* [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-09-25 15:51 Andrew Jones
2026-10-09 11:45 ` Anup Patel
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Jones @ 2026-09-25 15:51 UTC (permalink / raw)
To: kvm-riscv; +Cc: anup, atish.patra, pjw, palmer, kvm, linux-riscv, linux-kernel
kvm_riscv_vcpu_aia_imsic_put() can run from the scheduler preemption
notifier while the raw runqueue lock is held. Taking vsfile_lock, an
rwlock_t, there violates the lockdep wait-context rules.
Protect the CPU/HGEI pair with a separate raw spinlock. All updates to
that pair retain vsfile_lock and take the new lock, while scheduler-out
uses only the new lock around the HGEIE update. This keeps the wakeup
setup serialized with VS-file publication and release without bringing
wider IMSIC operations into a raw critical section.
Fixes: 4cec89db80ba ("RISC-V: KVM: Move HGEI[E|P] CSR access to IMSIC virtualization")
Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
---
arch/riscv/kvm/aia_imsic.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
index c1af23e79ae0..1ccb7eac9f45 100644
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -48,6 +48,8 @@ struct imsic {
/* IMSIC VS-file */
rwlock_t vsfile_lock;
+ /* Writers hold both locks; scheduler-out only takes hgei_lock. */
+ raw_spinlock_t hgei_lock;
int vsfile_cpu;
int vsfile_hgei;
void __iomem *vsfile_va;
@@ -582,6 +584,17 @@ static void imsic_vsfile_local_update(int vsfile_hgei, u32 nr_eix,
csr_write(CSR_VSISELECT, old_vsiselect);
}
+static void imsic_vsfile_set_hgei(struct imsic *imsic, int cpu, int hgei)
+{
+ unsigned long flags;
+
+ /* Serialize the CPU/HGEI pair with scheduler-out's wakeup setup. */
+ raw_spin_lock_irqsave(&imsic->hgei_lock, flags);
+ imsic->vsfile_cpu = cpu;
+ imsic->vsfile_hgei = hgei;
+ raw_spin_unlock_irqrestore(&imsic->hgei_lock, flags);
+}
+
static void imsic_vsfile_cleanup(struct imsic *imsic)
{
int old_vsfile_hgei, old_vsfile_cpu;
@@ -596,7 +609,7 @@ static void imsic_vsfile_cleanup(struct imsic *imsic)
write_lock_irqsave(&imsic->vsfile_lock, flags);
old_vsfile_hgei = imsic->vsfile_hgei;
old_vsfile_cpu = imsic->vsfile_cpu;
- imsic->vsfile_cpu = imsic->vsfile_hgei = -1;
+ imsic_vsfile_set_hgei(imsic, -1, -1);
imsic->vsfile_va = NULL;
imsic->vsfile_pa = 0;
write_unlock_irqrestore(&imsic->vsfile_lock, flags);
@@ -727,10 +740,10 @@ void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu)
if (!kvm_vcpu_is_blocking(vcpu))
return;
- read_lock_irqsave(&imsic->vsfile_lock, flags);
+ raw_spin_lock_irqsave(&imsic->hgei_lock, flags);
if (imsic->vsfile_cpu > -1)
csr_set(CSR_HGEIE, BIT(imsic->vsfile_hgei));
- read_unlock_irqrestore(&imsic->vsfile_lock, flags);
+ raw_spin_unlock_irqrestore(&imsic->hgei_lock, flags);
}
void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
@@ -747,7 +760,7 @@ void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
write_lock_irqsave(&imsic->vsfile_lock, flags);
old_vsfile_hgei = imsic->vsfile_hgei;
old_vsfile_cpu = imsic->vsfile_cpu;
- imsic->vsfile_cpu = imsic->vsfile_hgei = -1;
+ imsic_vsfile_set_hgei(imsic, -1, -1);
imsic->vsfile_va = NULL;
imsic->vsfile_pa = 0;
write_unlock_irqrestore(&imsic->vsfile_lock, flags);
@@ -861,8 +874,7 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu)
/* Update new IMSIC VS-file details in IMSIC context */
write_lock_irqsave(&imsic->vsfile_lock, flags);
- imsic->vsfile_hgei = new_vsfile_hgei;
- imsic->vsfile_cpu = vcpu->cpu;
+ imsic_vsfile_set_hgei(imsic, vcpu->cpu, new_vsfile_hgei);
imsic->vsfile_va = new_vsfile_va;
imsic->vsfile_pa = new_vsfile_pa;
write_unlock_irqrestore(&imsic->vsfile_lock, flags);
@@ -1112,6 +1124,7 @@ int kvm_riscv_vcpu_aia_imsic_init(struct kvm_vcpu *vcpu)
/* Setup IMSIC context */
imsic->nr_msis = kvm->arch.aia.nr_ids + 1;
rwlock_init(&imsic->vsfile_lock);
+ raw_spin_lock_init(&imsic->hgei_lock);
imsic->nr_eix = BITS_TO_U64(imsic->nr_msis);
imsic->nr_hw_eix = BITS_TO_U64(kvm_riscv_aia_max_ids);
imsic->vsfile_hgei = imsic->vsfile_cpu = -1;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
2026-09-25 15:51 [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state Andrew Jones
@ 2026-10-09 11:45 ` Anup Patel
2026-10-09 13:44 ` Andrew Jones
0 siblings, 1 reply; 5+ messages in thread
From: Anup Patel @ 2026-10-09 11:45 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm-riscv, atish.patra, pjw, palmer, kvm, linux-riscv, linux-kernel
On Fri, Sep 25, 2026 at 5:51 PM Andrew Jones
<andrew.jones@oss.qualcomm.com> wrote:
>
> kvm_riscv_vcpu_aia_imsic_put() can run from the scheduler preemption
> notifier while the raw runqueue lock is held. Taking vsfile_lock, an
> rwlock_t, there violates the lockdep wait-context rules.
>
> Protect the CPU/HGEI pair with a separate raw spinlock. All updates to
> that pair retain vsfile_lock and take the new lock, while scheduler-out
> uses only the new lock around the HGEIE update. This keeps the wakeup
> setup serialized with VS-file publication and release without bringing
> wider IMSIC operations into a raw critical section.
Instead of introducing another lock, why not replace
read_lock_irqsave(&imsic->vsfile_lock, flags) with
write_lock_irqsave(&imsic->vsfile_lock, flags) in
kvm_riscv_vcpu_aia_imsic_put() ??
Regards,
Anup
>
> Fixes: 4cec89db80ba ("RISC-V: KVM: Move HGEI[E|P] CSR access to IMSIC virtualization")
> Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
> ---
> arch/riscv/kvm/aia_imsic.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
> index c1af23e79ae0..1ccb7eac9f45 100644
> --- a/arch/riscv/kvm/aia_imsic.c
> +++ b/arch/riscv/kvm/aia_imsic.c
> @@ -48,6 +48,8 @@ struct imsic {
>
> /* IMSIC VS-file */
> rwlock_t vsfile_lock;
> + /* Writers hold both locks; scheduler-out only takes hgei_lock. */
> + raw_spinlock_t hgei_lock;
> int vsfile_cpu;
> int vsfile_hgei;
> void __iomem *vsfile_va;
> @@ -582,6 +584,17 @@ static void imsic_vsfile_local_update(int vsfile_hgei, u32 nr_eix,
> csr_write(CSR_VSISELECT, old_vsiselect);
> }
>
> +static void imsic_vsfile_set_hgei(struct imsic *imsic, int cpu, int hgei)
> +{
> + unsigned long flags;
> +
> + /* Serialize the CPU/HGEI pair with scheduler-out's wakeup setup. */
> + raw_spin_lock_irqsave(&imsic->hgei_lock, flags);
> + imsic->vsfile_cpu = cpu;
> + imsic->vsfile_hgei = hgei;
> + raw_spin_unlock_irqrestore(&imsic->hgei_lock, flags);
> +}
> +
> static void imsic_vsfile_cleanup(struct imsic *imsic)
> {
> int old_vsfile_hgei, old_vsfile_cpu;
> @@ -596,7 +609,7 @@ static void imsic_vsfile_cleanup(struct imsic *imsic)
> write_lock_irqsave(&imsic->vsfile_lock, flags);
> old_vsfile_hgei = imsic->vsfile_hgei;
> old_vsfile_cpu = imsic->vsfile_cpu;
> - imsic->vsfile_cpu = imsic->vsfile_hgei = -1;
> + imsic_vsfile_set_hgei(imsic, -1, -1);
> imsic->vsfile_va = NULL;
> imsic->vsfile_pa = 0;
> write_unlock_irqrestore(&imsic->vsfile_lock, flags);
> @@ -727,10 +740,10 @@ void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu)
> if (!kvm_vcpu_is_blocking(vcpu))
> return;
>
> - read_lock_irqsave(&imsic->vsfile_lock, flags);
> + raw_spin_lock_irqsave(&imsic->hgei_lock, flags);
> if (imsic->vsfile_cpu > -1)
> csr_set(CSR_HGEIE, BIT(imsic->vsfile_hgei));
> - read_unlock_irqrestore(&imsic->vsfile_lock, flags);
> + raw_spin_unlock_irqrestore(&imsic->hgei_lock, flags);
> }
>
> void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
> @@ -747,7 +760,7 @@ void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
> write_lock_irqsave(&imsic->vsfile_lock, flags);
> old_vsfile_hgei = imsic->vsfile_hgei;
> old_vsfile_cpu = imsic->vsfile_cpu;
> - imsic->vsfile_cpu = imsic->vsfile_hgei = -1;
> + imsic_vsfile_set_hgei(imsic, -1, -1);
> imsic->vsfile_va = NULL;
> imsic->vsfile_pa = 0;
> write_unlock_irqrestore(&imsic->vsfile_lock, flags);
> @@ -861,8 +874,7 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu)
>
> /* Update new IMSIC VS-file details in IMSIC context */
> write_lock_irqsave(&imsic->vsfile_lock, flags);
> - imsic->vsfile_hgei = new_vsfile_hgei;
> - imsic->vsfile_cpu = vcpu->cpu;
> + imsic_vsfile_set_hgei(imsic, vcpu->cpu, new_vsfile_hgei);
> imsic->vsfile_va = new_vsfile_va;
> imsic->vsfile_pa = new_vsfile_pa;
> write_unlock_irqrestore(&imsic->vsfile_lock, flags);
> @@ -1112,6 +1124,7 @@ int kvm_riscv_vcpu_aia_imsic_init(struct kvm_vcpu *vcpu)
> /* Setup IMSIC context */
> imsic->nr_msis = kvm->arch.aia.nr_ids + 1;
> rwlock_init(&imsic->vsfile_lock);
> + raw_spin_lock_init(&imsic->hgei_lock);
> imsic->nr_eix = BITS_TO_U64(imsic->nr_msis);
> imsic->nr_hw_eix = BITS_TO_U64(kvm_riscv_aia_max_ids);
> imsic->vsfile_hgei = imsic->vsfile_cpu = -1;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
2026-10-09 11:45 ` Anup Patel
@ 2026-10-09 13:44 ` Andrew Jones
2026-10-09 16:37 ` Anup Patel
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Jones @ 2026-10-09 13:44 UTC (permalink / raw)
To: Anup Patel
Cc: kvm-riscv, atish.patra, pjw, palmer, kvm, linux-riscv, linux-kernel
Hi Anup,
On Fri, Oct 09, 2026 at 01:45:20PM +0200, Anup Patel wrote:
> On Fri, Sep 25, 2026 at 5:51 PM Andrew Jones
> <andrew.jones@oss.qualcomm.com> wrote:
> >
> > kvm_riscv_vcpu_aia_imsic_put() can run from the scheduler preemption
> > notifier while the raw runqueue lock is held. Taking vsfile_lock, an
> > rwlock_t, there violates the lockdep wait-context rules.
> >
> > Protect the CPU/HGEI pair with a separate raw spinlock. All updates to
> > that pair retain vsfile_lock and take the new lock, while scheduler-out
> > uses only the new lock around the HGEIE update. This keeps the wakeup
> > setup serialized with VS-file publication and release without bringing
> > wider IMSIC operations into a raw critical section.
>
> Instead of introducing another lock, why not replace
> read_lock_irqsave(&imsic->vsfile_lock, flags) with
> write_lock_irqsave(&imsic->vsfile_lock, flags) in
> kvm_riscv_vcpu_aia_imsic_put() ??
Unfortunately, write_lock_irqsave() has the same PREEMPT_RT issue, since
rwlock_t is sleepable on RT in both modes.
Thanks,
drew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
2026-10-09 13:44 ` Andrew Jones
@ 2026-10-09 16:37 ` Anup Patel
2026-10-17 12:29 ` Andrew Jones
0 siblings, 1 reply; 5+ messages in thread
From: Anup Patel @ 2026-10-09 16:37 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm-riscv, atish.patra, pjw, palmer, kvm, linux-riscv, linux-kernel
On Fri, Oct 9, 2026 at 3:44 PM Andrew Jones
<andrew.jones@oss.qualcomm.com> wrote:
>
> Hi Anup,
>
> On Fri, Oct 09, 2026 at 01:45:20PM +0200, Anup Patel wrote:
> > On Fri, Sep 25, 2026 at 5:51 PM Andrew Jones
> > <andrew.jones@oss.qualcomm.com> wrote:
> > >
> > > kvm_riscv_vcpu_aia_imsic_put() can run from the scheduler preemption
> > > notifier while the raw runqueue lock is held. Taking vsfile_lock, an
> > > rwlock_t, there violates the lockdep wait-context rules.
> > >
> > > Protect the CPU/HGEI pair with a separate raw spinlock. All updates to
> > > that pair retain vsfile_lock and take the new lock, while scheduler-out
> > > uses only the new lock around the HGEIE update. This keeps the wakeup
> > > setup serialized with VS-file publication and release without bringing
> > > wider IMSIC operations into a raw critical section.
> >
> > Instead of introducing another lock, why not replace
> > read_lock_irqsave(&imsic->vsfile_lock, flags) with
> > write_lock_irqsave(&imsic->vsfile_lock, flags) in
> > kvm_riscv_vcpu_aia_imsic_put() ??
>
> Unfortunately, write_lock_irqsave() has the same PREEMPT_RT issue, since
> rwlock_t is sleepable on RT in both modes.
>
I am not fond of nested locking, why not convert vsfile_lock into raw
spinlock ??
Regards,
Anup
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
2026-10-09 16:37 ` Anup Patel
@ 2026-10-17 12:29 ` Andrew Jones
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Jones @ 2026-10-17 12:29 UTC (permalink / raw)
To: Anup Patel
Cc: Andrew Jones, kvm-riscv, atish.patra, pjw, palmer, kvm,
linux-riscv, linux-kernel
On Fri, Oct 09, 2026 at 06:37:13PM +0200, Anup Patel wrote:
> On Fri, Oct 9, 2026 at 3:44 PM Andrew Jones
> <andrew.jones@oss.qualcomm.com> wrote:
> >
> > Hi Anup,
> >
> > On Fri, Oct 09, 2026 at 01:45:20PM +0200, Anup Patel wrote:
> > > On Fri, Sep 25, 2026 at 5:51 PM Andrew Jones
> > > <andrew.jones@oss.qualcomm.com> wrote:
> > > >
> > > > kvm_riscv_vcpu_aia_imsic_put() can run from the scheduler preemption
> > > > notifier while the raw runqueue lock is held. Taking vsfile_lock, an
> > > > rwlock_t, there violates the lockdep wait-context rules.
> > > >
> > > > Protect the CPU/HGEI pair with a separate raw spinlock. All updates to
> > > > that pair retain vsfile_lock and take the new lock, while scheduler-out
> > > > uses only the new lock around the HGEIE update. This keeps the wakeup
> > > > setup serialized with VS-file publication and release without bringing
> > > > wider IMSIC operations into a raw critical section.
> > >
> > > Instead of introducing another lock, why not replace
> > > read_lock_irqsave(&imsic->vsfile_lock, flags) with
> > > write_lock_irqsave(&imsic->vsfile_lock, flags) in
> > > kvm_riscv_vcpu_aia_imsic_put() ??
> >
> > Unfortunately, write_lock_irqsave() has the same PREEMPT_RT issue, since
> > rwlock_t is sleepable on RT in both modes.
> >
>
> I am not fond of nested locking, why not convert vsfile_lock into raw
> spinlock ??
>
We can do that. The reader concurrency the rwlock gave us was only for
tiny sections anyway. I'll send a v2.
Thanks,
drew
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-17 12:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-25 15:51 [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state Andrew Jones
2026-10-09 11:45 ` Anup Patel
2026-10-09 13:44 ` Andrew Jones
2026-10-09 16:37 ` Anup Patel
2026-10-17 12:29 ` Andrew Jones
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®