* [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®