[PATCH] RISC-V: KVM: Order IMSIC VS-file write before releasing vsfile_lock

[Oleksii Kurochko]

kvm_riscv_vcpu_aia_imsic_inject() writes the interrupt identity to the
SETIPNUM register of the vCPU's IMSIC VS-file while holding vsfile_lock
for reading. The lock keeps the VS-file from being switched under the
injector: kvm_riscv_vcpu_aia_imsic_update() and
kvm_riscv_vcpu_aia_imsic_release() take it for writing to move the vCPU
to a new VS-file or to the SW-file, then read and clear the old VS-file
and free its HGEI line.

On RISC-V, writel() is followed only by mmiowb_set_pending(). The
trailing "fence o,w" is deferred to spin_unlock() by the mmiowb()
tracking, and rwlocks don't take part in that tracking:
queued_read_unlock() issues just "fence rw,w" before the AMO, which
doesn't order device output. So the SETIPNUM write can still be in
flight after another hart has observed vsfile_lock being released. If
the old VS-file is read and cleared before the write lands, the
interrupt is lost. If the HGEI line has already been handed out
again, the interrupt ends up in another vCPU's VS-file.

Add an explicit RISCV_FENCE(o, w) after the write to the VS-file, which
is what spin_unlock() would issue via mmiowb().

Fixes: db8b7e97d613 ("RISC-V: KVM: Add in-kernel virtualization of AIA IMSIC")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Why RISCV_FENCE(o, w) and not mmiowb():

- Since the mmiowb() rework in v5.2 (e.g. commit 60ca1e5a200c
  ("mmiowb: Hook up mmiowb helpers to spinlocks and generic I/O
  accessors")), mmiowb() is an implementation detail of the spinlock
  code. Explicit invocations were removed from drivers by commit
  fb24ea52f78e ("drivers: Remove explicit invocations of mmiowb()") and
  from the documentation by commit 915530396c78 ("Documentation: Kill
  all references to mmiowb()"), so adding a new direct caller would go
  against that.

- RISCV_FENCE(o, w) states exactly the ordering needed here (device
  output before the store that releases the lock), which is fine for
  arch-specific code, and it is the same instruction mmiowb() expands to
  on RISC-V. wmb() ("fence ow,ow") would also work but is stronger than
  required.

Scope: this only orders the write before the lock release. Whether a
write that is already in global memory order is guaranteed to have
taken effect in the old VS-file before kvm_riscv_vcpu_aia_imsic_update()
or kvm_riscv_vcpu_aia_imsic_release() reads and clears it is a separate
question, see https://github.com/riscv/riscv-aia/issues/172. If it
isn't, these paths will also need to drain the old VS-file, and such a
drain depends on the ordering added here.
---
 arch/riscv/kvm/aia_imsic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
index f17dc559a709..807bb73a129a 100644
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -1056,6 +1056,13 @@ int kvm_riscv_vcpu_aia_imsic_inject(struct kvm_vcpu *vcpu,
 
 	if (imsic->vsfile_cpu >= 0) {
 		writel(iid, imsic->vsfile_va + IMSIC_MMIO_SETIPNUM_LE);
+		/*
+		 * rwlocks don't participate in mmiowb() tracking, so order
+		 * the write to the VS-file before releasing vsfile_lock,
+		 * i.e. before the vCPU can be moved off this VS-file and
+		 * the VS-file read and cleared.
+		 */
+		RISCV_FENCE(o, w);
 	} else {
 		eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)];
 		set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip);

base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.56.0