Skip to contentSkip to conversation

FAILED: patch "[PATCH] RISC-V: KVM: Serialize virtual interrupt pending state" failed to apply to 6.6-stable tree

PATCH New [email protected] 2 participants 2 messages Jul 25 – Jul 27

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x d024a0a7879e6f37c0152aacf6d8e37b214a1738
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026072503-pointed-oops-b948@gregkh' --subject-prefix 'PATCH 6.6.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d024a0a7879e6f37c0152aacf6d8e37b214a1738 Mon Sep 17 00:00:00 2001
From: Xie Bo <[email protected]>
Date: Wed, 15 Jul 2026 10:03:59 +0800
Subject: [PATCH] RISC-V: KVM: Serialize virtual interrupt pending state
updates

KVM RISC-V tracks guest local interrupt state with two bitmaps:

- irqs_pending: interrupts that should be visible to the guest
- irqs_pending_mask: interrupts whose pending state changed

The current code updates those bitmaps with independent atomic bitops and assumes a multiple-producer, single-consumer protocol. That model does not actually hold.

kvm_riscv_vcpu_sync_interrupts() is not a pure consumer. When the guest changes guest-visible HVIP state, sync_interrupts() writes both irqs_pending and irqs_pending_mask to reflect the new guest state back into KVM state. As a result, irqs_pending and irqs_pending_mask form a single logical state transition, but they are not updated atomically as a pair.

This allows a race where a newly injected interrupt is lost. For example:

CPU0 CPU1
---- ----
kvm_riscv_vcpu_set_interrupt(VS_SOFT)
set_bit(VS_SOFT, irqs_pending)
kvm_riscv_vcpu_sync_interrupts()
sees guest-cleared HVIP.VSSIP sets irqs_pending_mask clear_bit(IRQ_VS_SOFT, irqs_pending)
set_bit(VS_SOFT, irqs_pending_mask)
kvm_vcpu_kick()

After that interleaving, a later flush can update HVIP without VSSIP even though a new virtual interrupt was injected. In practice, the guest can remain blocked in WFI with work pending.

The same pending/mask protocol is shared by VS soft interrupts, PMU overflow delivery, and AIA high interrupt synchronization, so the race is not limited to one interrupt source.

Fix this by serializing all updates to irqs_pending and irqs_pending_mask with a per-vCPU raw spinlock. This keeps the pending bit and the dirty mask as one state transition across:

- set/unset interrupt
- guest HVIP sync
- interrupt flush to guest CSR state
- vCPU reset
- AIA CSR writes that clear dirty state

Use non-atomic bitmap operations while holding the lock. Hold the lock across the AIA sync, flush, and pending checks as well, so both bitmap words share the same serialization domain.

This intentionally replaces the existing lockless protocol instead of trying to repair it with additional barriers. The problem is not memory ordering on a single field; it is that two separate bitmaps encode one shared state machine while both producers and sync paths can modify them. A per-vCPU raw spinlock keeps the fix small, local, and suitable for backporting.

Fixes cce69aff689e ("RISC-V: KVM: Implement VCPU interrupts and requests handling")
arch/riscv/include/asm/kvm_host.h +5 -5
index 60017ceec9d2..e2d5808169e4 100644
@@ -209,13 +209,13 @@ struct kvm_vcpu_arch {
 	/*
 	 * VCPU interrupts
 	 *
-	 * We have a lockless approach for tracking pending VCPU interrupts
-	 * implemented using atomic bitops. The irqs_pending bitmap represent
-	 * pending interrupts whereas irqs_pending_mask represent bits changed
-	 * in irqs_pending. Our approach is modeled around multiple producer
-	 * and single consumer problem where the consumer is the VCPU itself.
+	 * The irqs_pending bitmap represents pending interrupts whereas
+	 * irqs_pending_mask represents bits changed in irqs_pending. Updates
+	 * to these bitmaps are serialized so vcpu interrupt sync/flush cannot
+	 * drop a newly injected interrupt while syncing guest-visible HVIP.
 	 */
 #define KVM_RISCV_VCPU_NR_IRQS	64
+	raw_spinlock_t irqs_pending_lock;
 	DECLARE_BITMAP(irqs_pending, KVM_RISCV_VCPU_NR_IRQS);
 	DECLARE_BITMAP(irqs_pending_mask, KVM_RISCV_VCPU_NR_IRQS);
 
arch/riscv/kvm/aia.c +28 -7
index bafb009c5ce5..9a653b4ad40a 100644
@@ -53,12 +53,15 @@ void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
 	struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
 	unsigned long mask, val;
 
+	lockdep_assert_held(&vcpu->arch.irqs_pending_lock);
+
 	if (!kvm_riscv_aia_available())
 		return;
 
-	if (READ_ONCE(vcpu->arch.irqs_pending_mask[1])) {
-		mask = xchg_acquire(&vcpu->arch.irqs_pending_mask[1], 0);
-		val = READ_ONCE(vcpu->arch.irqs_pending[1]) & mask;
+	mask = vcpu->arch.irqs_pending_mask[1];
+	if (mask) {
+		vcpu->arch.irqs_pending_mask[1] = 0;
+		val = vcpu->arch.irqs_pending[1] & mask;
 
 		csr->hviph &= ~mask;
 		csr->hviph |= val;
@@ -69,6 +72,8 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
 {
 	struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
 
+	lockdep_assert_held(&vcpu->arch.irqs_pending_lock);
+
 	if (kvm_riscv_aia_available())
 		csr->vsieh = ncsr_read(CSR_VSIEH);
 }
@@ -77,13 +82,22 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
 bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
 {
 	unsigned long seip;
+#ifdef CONFIG_32BIT
+	unsigned long flags;
+	bool pending;
+#endif
 
 	if (!kvm_riscv_aia_available())
 		return false;
 
 #ifdef CONFIG_32BIT
-	if (READ_ONCE(vcpu->arch.irqs_pending[1]) &
-	    (vcpu->arch.aia_context.guest_csr.vsieh & upper_32_bits(mask)))
+	raw_spin_lock_irqsave(&vcpu->arch.irqs_pending_lock, flags);
+	pending = vcpu->arch.irqs_pending[1] &
+		  (vcpu->arch.aia_context.guest_csr.vsieh &
+		   upper_32_bits(mask));
+	raw_spin_unlock_irqrestore(&vcpu->arch.irqs_pending_lock, flags);
+
+	if (pending)
 		return true;
 #endif
 
@@ -207,6 +221,9 @@ int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
 {
 	struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
 	unsigned long regs_max = sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long);
+#ifdef CONFIG_32BIT
+	unsigned long flags;
+#endif
 
 	if (!riscv_isa_extension_available(vcpu->arch.isa, SSAIA))
 		return -ENOENT;
@@ -219,8 +236,12 @@ int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
 		((unsigned long *)csr)[reg_num] = val;
 
 #ifdef CONFIG_32BIT
-		if (reg_num == KVM_REG_RISCV_CSR_AIA_REG(siph))
-			WRITE_ONCE(vcpu->arch.irqs_pending_mask[1], 0);
+		if (reg_num == KVM_REG_RISCV_CSR_AIA_REG(siph)) {
+			raw_spin_lock_irqsave(&vcpu->arch.irqs_pending_lock, flags);
+			vcpu->arch.irqs_pending_mask[1] = 0;
+			raw_spin_unlock_irqrestore(&vcpu->arch.irqs_pending_lock,
+						   flags);
+		}
 #endif
 	}
 
arch/riscv/kvm/vcpu.c +48 -20

164 lines not on this page · raw message

arch/riscv/kvm/vcpu_onereg.c +6 -2
index 61988382570f..99b9107b1ac1 100644
@@ -286,6 +286,7 @@ static int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
 {
 	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
 	unsigned long regs_max = sizeof(struct kvm_riscv_csr) / sizeof(unsigned long);
+	unsigned long flags;
 
 	if (reg_num >= regs_max)
 		return -ENOENT;
@@ -299,8 +300,11 @@ static int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
 
 	((unsigned long *)csr)[reg_num] = reg_val;
 
-	if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
-		WRITE_ONCE(vcpu->arch.irqs_pending_mask[0], 0);
+	if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
+		raw_spin_lock_irqsave(&vcpu->arch.irqs_pending_lock, flags);
+		vcpu->arch.irqs_pending_mask[0] = 0;
+		raw_spin_unlock_irqrestore(&vcpu->arch.irqs_pending_lock, flags);
+	}
 
 	return 0;
 }
#1 lore raw

PATCH 1/1 RISC-V: KVM: Serialize virtual interrupt pending state updates

KVM RISC-V tracks guest local interrupt state with two bitmaps:

- irqs_pending: interrupts that should be visible to the guest
- irqs_pending_mask: interrupts whose pending state changed

The current code updates those bitmaps with independent atomic bitops and assumes a multiple-producer, single-consumer protocol. That model does not actually hold.

kvm_riscv_vcpu_sync_interrupts() is not a pure consumer. When the guest changes guest-visible HVIP state, sync_interrupts() writes both irqs_pending and irqs_pending_mask to reflect the new guest state back into KVM state. As a result, irqs_pending and irqs_pending_mask form a single logical state transition, but they are not updated atomically as a pair.

This allows a race where a newly injected interrupt is lost. For example:

CPU0 CPU1
---- ----
kvm_riscv_vcpu_set_interrupt(VS_SOFT)
set_bit(VS_SOFT, irqs_pending)
kvm_riscv_vcpu_sync_interrupts()
sees guest-cleared HVIP.VSSIP sets irqs_pending_mask clear_bit(IRQ_VS_SOFT, irqs_pending)
set_bit(VS_SOFT, irqs_pending_mask)
kvm_vcpu_kick()

After that interleaving, a later flush can update HVIP without VSSIP even though a new virtual interrupt was injected. In practice, the guest can remain blocked in WFI with work pending.

The same pending/mask protocol is shared by VS soft interrupts, PMU overflow delivery, and AIA high interrupt synchronization, so the race is not limited to one interrupt source.

Fix this by serializing all updates to irqs_pending and irqs_pending_mask with a per-vCPU raw spinlock. This keeps the pending bit and the dirty mask as one state transition across:

- set/unset interrupt
- guest HVIP sync
- interrupt flush to guest CSR state
- vCPU reset
- AIA CSR writes that clear dirty state

Use non-atomic bitmap operations while holding the lock. Hold the lock across the AIA sync, flush, and pending checks as well, so both bitmap words share the same serialization domain.

This intentionally replaces the existing lockless protocol instead of trying to repair it with additional barriers. The problem is not memory ordering on a single field; it is that two separate bitmaps encode one shared state machine while both producers and sync paths can modify them. A per-vCPU raw spinlock keeps the fix small, local, and suitable for backporting.

Fixes cce69aff689e ("RISC-V: KVM: Implement VCPU interrupts and requests handling")

[ bo: Drop IRQ_PMU_OVF handling, which is not present in 6.6.y, and
adapt the reset and CSR helpers to their older signatures. ]
(cherry picked from commit d024a0a7879e6f37c0152aacf6d8e37b214a1738)

---
 arch/riscv/include/asm/kvm_host.h | 10 ++---
 arch/riscv/kvm/aia.c              | 35 +++++++++++++----
 arch/riscv/kvm/vcpu.c             | 64 ++++++++++++++++++++++---------
 arch/riscv/kvm/vcpu_onereg.c      |  8 +++-
 4 files changed, 85 insertions(+), 32 deletions(-)
arch/riscv/include/asm/kvm_host.h +5 -5
index 459e61a..d2d7f9b 100644
@@ -202,13 +202,13 @@ struct kvm_vcpu_arch {
 	/*
 	 * VCPU interrupts
 	 *
-	 * We have a lockless approach for tracking pending VCPU interrupts
-	 * implemented using atomic bitops. The irqs_pending bitmap represent
-	 * pending interrupts whereas irqs_pending_mask represent bits changed
-	 * in irqs_pending. Our approach is modeled around multiple producer
-	 * and single consumer problem where the consumer is the VCPU itself.
+	 * The irqs_pending bitmap represents pending interrupts whereas
+	 * irqs_pending_mask represents bits changed in irqs_pending. Updates
+	 * to these bitmaps are serialized so vcpu interrupt sync/flush cannot
+	 * drop a newly injected interrupt while syncing guest-visible HVIP.
 	 */
 #define KVM_RISCV_VCPU_NR_IRQS	64
+	raw_spinlock_t irqs_pending_lock;
 	DECLARE_BITMAP(irqs_pending, KVM_RISCV_VCPU_NR_IRQS);
 	DECLARE_BITMAP(irqs_pending_mask, KVM_RISCV_VCPU_NR_IRQS);
 
arch/riscv/kvm/aia.c +28 -7
index 74bb274..be13309 100644
@@ -71,12 +71,15 @@ void kvm_riscv_vcpu_aia_flush_interrupts(struct kvm_vcpu *vcpu)
 	struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
 	unsigned long mask, val;
 
+	lockdep_assert_held(&vcpu->arch.irqs_pending_lock);
+
 	if (!kvm_riscv_aia_available())
 		return;
 
-	if (READ_ONCE(vcpu->arch.irqs_pending_mask[1])) {
-		mask = xchg_acquire(&vcpu->arch.irqs_pending_mask[1], 0);
-		val = READ_ONCE(vcpu->arch.irqs_pending[1]) & mask;
+	mask = vcpu->arch.irqs_pending_mask[1];
+	if (mask) {
+		vcpu->arch.irqs_pending_mask[1] = 0;
+		val = vcpu->arch.irqs_pending[1] & mask;
 
 		csr->hviph &= ~mask;
 		csr->hviph |= val;
@@ -87,6 +90,8 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
 {
 	struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
 
+	lockdep_assert_held(&vcpu->arch.irqs_pending_lock);
+
 	if (kvm_riscv_aia_available())
 		csr->vsieh = csr_read(CSR_VSIEH);
 }
@@ -96,13 +101,22 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
 {
 	int hgei;
 	unsigned long seip;
+#ifdef CONFIG_32BIT
+	unsigned long flags;
+	bool pending;
+#endif
 
 	if (!kvm_riscv_aia_available())
 		return false;
 
 #ifdef CONFIG_32BIT
-	if (READ_ONCE(vcpu->arch.irqs_pending[1]) &
-	    (vcpu->arch.aia_context.guest_csr.vsieh & upper_32_bits(mask)))
+	raw_spin_lock_irqsave(&vcpu->arch.irqs_pending_lock, flags);
+	pending = vcpu->arch.irqs_pending[1] &
+		  (vcpu->arch.aia_context.guest_csr.vsieh &
+		   upper_32_bits(mask));
+	raw_spin_unlock_irqrestore(&vcpu->arch.irqs_pending_lock, flags);
+
+	if (pending)
 		return true;
 #endif
 
@@ -190,6 +204,9 @@ int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
 			       unsigned long val)
 {
 	struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
+#ifdef CONFIG_32BIT
+	unsigned long flags;
+#endif
 
 	if (reg_num >= sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long))
 		return -ENOENT;
@@ -198,8 +215,12 @@ int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
 		((unsigned long *)csr)[reg_num] = val;
 
 #ifdef CONFIG_32BIT
-		if (reg_num == KVM_REG_RISCV_CSR_AIA_REG(siph))
-			WRITE_ONCE(vcpu->arch.irqs_pending_mask[1], 0);
+		if (reg_num == KVM_REG_RISCV_CSR_AIA_REG(siph)) {
+			raw_spin_lock_irqsave(&vcpu->arch.irqs_pending_lock, flags);
+			vcpu->arch.irqs_pending_mask[1] = 0;
+			raw_spin_unlock_irqrestore(&vcpu->arch.irqs_pending_lock,
+						   flags);
+		}
 #endif
 	}
 
arch/riscv/kvm/vcpu.c +46 -18

155 lines not on this page · raw message

arch/riscv/kvm/vcpu_onereg.c +6 -2
index d520b25..0e58531 100644
@@ -361,6 +361,7 @@ static int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
 					  unsigned long reg_val)
 {
 	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+	unsigned long flags;
 
 	if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
 		return -ENOENT;
@@ -372,8 +373,11 @@ static int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
 
 	((unsigned long *)csr)[reg_num] = reg_val;
 
-	if (reg_num == KVM_REG_RISCV_CSR_REG(sip))
-		WRITE_ONCE(vcpu->arch.irqs_pending_mask[0], 0);
+	if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
+		raw_spin_lock_irqsave(&vcpu->arch.irqs_pending_lock, flags);
+		vcpu->arch.irqs_pending_mask[0] = 0;
+		raw_spin_unlock_irqrestore(&vcpu->arch.irqs_pending_lock, flags);
+	}
 
 	return 0;
 }
Signature · 1 line
2.54.0