[PATCH v3] RISC-V: KVM: Serialize and atomize the IMSIC's TOPEI read-and-clear operation

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH v3] RISC-V: KVM: Serialize and atomize the IMSIC's TOPEI read-and-clear operation
@ 2026-09-23  9:16 BillXiang
  2026-11-13  5:36 ` Anup Patel
  0 siblings, 1 reply; 3+ messages in thread
From: BillXiang @ 2026-09-23  9:16 UTC (permalink / raw)
  To: anup
  Cc: xb, atish.patra, pjw, palmer, aou, alex, pbonzini, graf,
	kvm-riscv, kvm, linux-riscv, linux-kernel, xiangwencheng

The TOPEI read-and-clear must be atomic with respect to
interrupt injection (set_bit in EIP). Otherwise an
interrupt injected between the topei read and the
clear_bit could be silently dropped.

v3:
 - fix undefined eflags

v2:
 - fix patch file curruption

v1: https://lore.kernel.org/kvm-riscv/20260923070332.1376132-1-xiangwencheng@lanxincomputing.com/

Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
---
 arch/riscv/kvm/aia_imsic.c | 42 +++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
index c1af23e79ae0..c440088651ec 100644
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -607,26 +607,25 @@ static void imsic_vsfile_cleanup(struct imsic *imsic)
 		kvm_riscv_aia_free_hgei(old_vsfile_cpu, old_vsfile_hgei);
 }
 
-static void imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
+static void __imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
 {
 	struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
 	struct imsic_mrif *mrif = imsic->swfile;
-	unsigned long flags;
-
-	/*
-	 * The critical section is necessary during external interrupt
-	 * updates to avoid the risk of losing interrupts due to potential
-	 * interruptions between reading topei and updating pending status.
-	 */
-
-	raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
 
 	if (imsic_mrif_atomic_read(mrif, &mrif->eidelivery) &&
 	    imsic_mrif_topei(mrif, imsic->nr_eix, imsic->nr_msis))
 		kvm_riscv_vcpu_set_interrupt(vcpu, IRQ_VS_EXT);
 	else
 		kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_EXT);
+}
+
+static void imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
+{
+	struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
+	unsigned long flags;
 
+	raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
+	__imsic_swfile_extirq_update(vcpu);
 	raw_spin_unlock_irqrestore(&imsic->swfile_extirq_lock, flags);
 }
 
@@ -912,12 +911,15 @@ int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu, unsigned long isel,
 	struct imsic_mrif_eix *eix;
 	int r, rc = KVM_INSN_CONTINUE_NEXT_SEPC;
 	struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
+	unsigned long flags;
 
 	/* If IMSIC vCPU state not initialized then forward to user space */
 	if (!imsic)
 		return KVM_INSN_EXIT_TO_USER_SPACE;
 
 	if (isel == KVM_RISCV_AIA_IMSIC_TOPEI) {
+		raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
+
 		/* Read pending and enabled interrupt with highest priority */
 		topei = imsic_mrif_topei(imsic->swfile, imsic->nr_eix,
 					 imsic->nr_msis);
@@ -934,16 +936,20 @@ int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu, unsigned long isel,
 					  eix->eip);
 			}
 		}
+		if (wr_mask)
+			__imsic_swfile_extirq_update(vcpu);
+
+		raw_spin_unlock_irqrestore(&imsic->swfile_extirq_lock, flags);
 	} else {
 		r = imsic_mrif_rmw(imsic->swfile, imsic->nr_eix, isel,
 				   val, new_val, wr_mask);
 		/* Forward unknown IMSIC register to user-space */
 		if (r)
 			rc = (r == -ENOENT) ? 0 : KVM_INSN_ILLEGAL_TRAP;
-	}
 
-	if (wr_mask)
-		imsic_swfile_extirq_update(vcpu);
+		if (wr_mask)
+			imsic_swfile_extirq_update(vcpu);
+	}
 
 	return rc;
 }
@@ -1050,9 +1056,17 @@ 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);
 	} else {
+		/*
+		 * Serialize the EIP set + external-irq update against the
+		 * TOPEI read-and-clear path so that an injected interrupt
+		 * cannot be silently lost when the guest acknowledges a
+		 * different (or the same) interrupt concurrently.
+		 */
+		raw_spin_lock(&imsic->swfile_extirq_lock);
 		eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)];
 		set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip);
-		imsic_swfile_extirq_update(vcpu);
+		__imsic_swfile_extirq_update(vcpu);
+		raw_spin_unlock(&imsic->swfile_extirq_lock);
 	}
 
 	read_unlock_irqrestore(&imsic->vsfile_lock, flags);
-- 
2.53.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] RISC-V: KVM: Serialize and atomize the IMSIC's TOPEI read-and-clear operation
  2026-09-23  9:16 [PATCH v3] RISC-V: KVM: Serialize and atomize the IMSIC's TOPEI read-and-clear operation BillXiang
@ 2026-11-13  5:36 ` Anup Patel
  2026-11-13  8:26   ` BillXiang
  0 siblings, 1 reply; 3+ messages in thread
From: Anup Patel @ 2026-11-13  5:36 UTC (permalink / raw)
  To: BillXiang
  Cc: xb, atish.patra, pjw, palmer, aou, alex, pbonzini, graf,
	kvm-riscv, kvm, linux-riscv, linux-kernel

On Wed, Sep 23, 2026 at 2:47 PM BillXiang
<xiangwencheng@lanxincomputing.com> wrote:
>
> The TOPEI read-and-clear must be atomic with respect to
> interrupt injection (set_bit in EIP). Otherwise an
> interrupt injected between the topei read and the
> clear_bit could be silently dropped.
>
> v3:
>  - fix undefined eflags
>
> v2:
>  - fix patch file curruption
>
> v1: https://lore.kernel.org/kvm-riscv/20260923070332.1376132-1-xiangwencheng@lanxincomputing.com/
>
> Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
> ---
>  arch/riscv/kvm/aia_imsic.c | 42 +++++++++++++++++++++++++-------------
>  1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
> index c1af23e79ae0..c440088651ec 100644
> --- a/arch/riscv/kvm/aia_imsic.c
> +++ b/arch/riscv/kvm/aia_imsic.c
> @@ -607,26 +607,25 @@ static void imsic_vsfile_cleanup(struct imsic *imsic)
>                 kvm_riscv_aia_free_hgei(old_vsfile_cpu, old_vsfile_hgei);
>  }
>
> -static void imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
> +static void __imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
>  {
>         struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
>         struct imsic_mrif *mrif = imsic->swfile;
> -       unsigned long flags;
> -
> -       /*
> -        * The critical section is necessary during external interrupt
> -        * updates to avoid the risk of losing interrupts due to potential
> -        * interruptions between reading topei and updating pending status.
> -        */
> -
> -       raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
>
>         if (imsic_mrif_atomic_read(mrif, &mrif->eidelivery) &&
>             imsic_mrif_topei(mrif, imsic->nr_eix, imsic->nr_msis))
>                 kvm_riscv_vcpu_set_interrupt(vcpu, IRQ_VS_EXT);
>         else
>                 kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_EXT);
> +}
> +
> +static void imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
> +{
> +       struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
> +       unsigned long flags;
>
> +       raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
> +       __imsic_swfile_extirq_update(vcpu);
>         raw_spin_unlock_irqrestore(&imsic->swfile_extirq_lock, flags);
>  }
>
> @@ -912,12 +911,15 @@ int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu, unsigned long isel,
>         struct imsic_mrif_eix *eix;
>         int r, rc = KVM_INSN_CONTINUE_NEXT_SEPC;
>         struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
> +       unsigned long flags;
>
>         /* If IMSIC vCPU state not initialized then forward to user space */
>         if (!imsic)
>                 return KVM_INSN_EXIT_TO_USER_SPACE;
>
>         if (isel == KVM_RISCV_AIA_IMSIC_TOPEI) {
> +               raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
> +
>                 /* Read pending and enabled interrupt with highest priority */
>                 topei = imsic_mrif_topei(imsic->swfile, imsic->nr_eix,
>                                          imsic->nr_msis);
> @@ -934,16 +936,20 @@ int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu, unsigned long isel,
>                                           eix->eip);
>                         }
>                 }
> +               if (wr_mask)
> +                       __imsic_swfile_extirq_update(vcpu);

The __imsic_swfile_extirq_update() also calls imsic_mrif_topei() so it is
better to pass topei value to __imsic_swfile_extirq_update().

> +
> +               raw_spin_unlock_irqrestore(&imsic->swfile_extirq_lock, flags);
>         } else {
>                 r = imsic_mrif_rmw(imsic->swfile, imsic->nr_eix, isel,
>                                    val, new_val, wr_mask);
>                 /* Forward unknown IMSIC register to user-space */
>                 if (r)
>                         rc = (r == -ENOENT) ? 0 : KVM_INSN_ILLEGAL_TRAP;
> -       }
>
> -       if (wr_mask)
> -               imsic_swfile_extirq_update(vcpu);
> +               if (wr_mask)
> +                       imsic_swfile_extirq_update(vcpu);
> +       }
>
>         return rc;
>  }
> @@ -1050,9 +1056,17 @@ 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);
>         } else {
> +               /*
> +                * Serialize the EIP set + external-irq update against the
> +                * TOPEI read-and-clear path so that an injected interrupt
> +                * cannot be silently lost when the guest acknowledges a
> +                * different (or the same) interrupt concurrently.
> +                */
> +               raw_spin_lock(&imsic->swfile_extirq_lock);
>                 eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)];
>                 set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip);
> -               imsic_swfile_extirq_update(vcpu);
> +               __imsic_swfile_extirq_update(vcpu);
> +               raw_spin_unlock(&imsic->swfile_extirq_lock);
>         }
>
>         read_unlock_irqrestore(&imsic->vsfile_lock, flags);
> --
> 2.53.0

Regards,
Anup

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] RISC-V: KVM: Serialize and atomize the IMSIC's TOPEI read-and-clear operation
  2026-11-13  5:36 ` Anup Patel
@ 2026-11-13  8:26   ` BillXiang
  0 siblings, 0 replies; 3+ messages in thread
From: BillXiang @ 2026-11-13  8:26 UTC (permalink / raw)
  To: Anup Patel
  Cc: xb, atish.patra, pjw, palmer, aou, alex, pbonzini, graf,
	kvm-riscv, kvm, linux-riscv, linux-kernel

On 11/13/2026 1:36 PM, Anup Patel wrote:
> On Wed, Sep 23, 2026 at 2:47 PM BillXiang
> <xiangwencheng@lanxincomputing.com> wrote:
>>
>> The TOPEI read-and-clear must be atomic with respect to
>> interrupt injection (set_bit in EIP). Otherwise an
>> interrupt injected between the topei read and the
>> clear_bit could be silently dropped.
>>
>> v3:
>>   - fix undefined eflags
>>
>> v2:
>>   - fix patch file curruption
>>
>> v1: https://lore.kernel.org/kvm-riscv/20260923070332.1376132-1-xiangwencheng@lanxincomputing.com/
>>
>> Signed-off-by: BillXiang <xiangwencheng@lanxincomputing.com>
>> ---
>>   arch/riscv/kvm/aia_imsic.c | 42 +++++++++++++++++++++++++-------------
>>   1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
>> index c1af23e79ae0..c440088651ec 100644
>> --- a/arch/riscv/kvm/aia_imsic.c
>> +++ b/arch/riscv/kvm/aia_imsic.c
>> @@ -607,26 +607,25 @@ static void imsic_vsfile_cleanup(struct imsic *imsic)
>>                  kvm_riscv_aia_free_hgei(old_vsfile_cpu, old_vsfile_hgei);
>>   }
>>
>> -static void imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
>> +static void __imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
>>   {
>>          struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
>>          struct imsic_mrif *mrif = imsic->swfile;
>> -       unsigned long flags;
>> -
>> -       /*
>> -        * The critical section is necessary during external interrupt
>> -        * updates to avoid the risk of losing interrupts due to potential
>> -        * interruptions between reading topei and updating pending status.
>> -        */
>> -
>> -       raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
>>
>>          if (imsic_mrif_atomic_read(mrif, &mrif->eidelivery) &&
>>              imsic_mrif_topei(mrif, imsic->nr_eix, imsic->nr_msis))
>>                  kvm_riscv_vcpu_set_interrupt(vcpu, IRQ_VS_EXT);
>>          else
>>                  kvm_riscv_vcpu_unset_interrupt(vcpu, IRQ_VS_EXT);
>> +}
>> +
>> +static void imsic_swfile_extirq_update(struct kvm_vcpu *vcpu)
>> +{
>> +       struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
>> +       unsigned long flags;
>>
>> +       raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
>> +       __imsic_swfile_extirq_update(vcpu);
>>          raw_spin_unlock_irqrestore(&imsic->swfile_extirq_lock, flags);
>>   }
>>
>> @@ -912,12 +911,15 @@ int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu, unsigned long isel,
>>          struct imsic_mrif_eix *eix;
>>          int r, rc = KVM_INSN_CONTINUE_NEXT_SEPC;
>>          struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
>> +       unsigned long flags;
>>
>>          /* If IMSIC vCPU state not initialized then forward to user space */
>>          if (!imsic)
>>                  return KVM_INSN_EXIT_TO_USER_SPACE;
>>
>>          if (isel == KVM_RISCV_AIA_IMSIC_TOPEI) {
>> +               raw_spin_lock_irqsave(&imsic->swfile_extirq_lock, flags);
>> +
>>                  /* Read pending and enabled interrupt with highest priority */
>>                  topei = imsic_mrif_topei(imsic->swfile, imsic->nr_eix,
>>                                           imsic->nr_msis);
>> @@ -934,16 +936,20 @@ int kvm_riscv_vcpu_aia_imsic_rmw(struct kvm_vcpu *vcpu, unsigned long isel,
>>                                            eix->eip);
>>                          }
>>                  }
>> +               if (wr_mask)
>> +                       __imsic_swfile_extirq_update(vcpu);
> 
> The __imsic_swfile_extirq_update() also calls imsic_mrif_topei() so it is
> better to pass topei value to __imsic_swfile_extirq_update().

Thanks for the review. I'll incorporate it into v4.

Regards,
Bill

> 
>> +
>> +               raw_spin_unlock_irqrestore(&imsic->swfile_extirq_lock, flags);
>>          } else {
>>                  r = imsic_mrif_rmw(imsic->swfile, imsic->nr_eix, isel,
>>                                     val, new_val, wr_mask);
>>                  /* Forward unknown IMSIC register to user-space */
>>                  if (r)
>>                          rc = (r == -ENOENT) ? 0 : KVM_INSN_ILLEGAL_TRAP;
>> -       }
>>
>> -       if (wr_mask)
>> -               imsic_swfile_extirq_update(vcpu);
>> +               if (wr_mask)
>> +                       imsic_swfile_extirq_update(vcpu);
>> +       }
>>
>>          return rc;
>>   }
>> @@ -1050,9 +1056,17 @@ 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);
>>          } else {
>> +               /*
>> +                * Serialize the EIP set + external-irq update against the
>> +                * TOPEI read-and-clear path so that an injected interrupt
>> +                * cannot be silently lost when the guest acknowledges a
>> +                * different (or the same) interrupt concurrently.
>> +                */
>> +               raw_spin_lock(&imsic->swfile_extirq_lock);
>>                  eix = &imsic->swfile->eix[iid / BITS_PER_TYPE(u64)];
>>                  set_bit(iid & (BITS_PER_TYPE(u64) - 1), eix->eip);
>> -               imsic_swfile_extirq_update(vcpu);
>> +               __imsic_swfile_extirq_update(vcpu);
>> +               raw_spin_unlock(&imsic->swfile_extirq_lock);
>>          }
>>
>>          read_unlock_irqrestore(&imsic->vsfile_lock, flags);
>> --
>> 2.53.0
> 
> Regards,
> Anup

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-13  8:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23  9:16 [PATCH v3] RISC-V: KVM: Serialize and atomize the IMSIC's TOPEI read-and-clear operation BillXiang
2026-11-13  5:36 ` Anup Patel
2026-11-13  8:26   ` BillXiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®