All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-09-25 15:51 ` Andrew Jones
  0 siblings, 0 replies; 15+ 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-09-25 15:51 ` Andrew Jones
  0 siblings, 0 replies; 15+ 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 related	[flat|nested] 15+ messages in thread
* [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-09-25 15:51 ` Andrew Jones
  0 siblings, 0 replies; 15+ 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


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [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
  -1 siblings, 0 replies; 15+ 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] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-09 11:45   ` Anup Patel
  0 siblings, 0 replies; 15+ 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
>

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-09 11:45   ` Anup Patel
  0 siblings, 0 replies; 15+ 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
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ 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
  -1 siblings, 0 replies; 15+ 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

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-09 13:44     ` Andrew Jones
  0 siblings, 0 replies; 15+ 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-09 13:44     ` Andrew Jones
  0 siblings, 0 replies; 15+ 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] 15+ 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
  -1 siblings, 0 replies; 15+ 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

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-09 16:37       ` Anup Patel
  0 siblings, 0 replies; 15+ 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-09 16:37       ` Anup Patel
  0 siblings, 0 replies; 15+ 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] 15+ 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
  -1 siblings, 0 replies; 15+ 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 

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-17 12:29         ` Andrew Jones
  0 siblings, 0 replies; 15+ 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 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] RISC-V: KVM: Use a raw lock for IMSIC wakeup state
@ 2026-10-17 12:29         ` Andrew Jones
  0 siblings, 0 replies; 15+ 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] 15+ messages in thread
end of thread, other threads:[~2026-10-17 12:29 UTC | newest]

Thread overview: 15+ 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-09-25 15:51 ` Andrew Jones
2026-09-25 15:51 ` Andrew Jones
2026-10-09 11:45 ` Anup Patel
2026-10-09 11:45   ` Anup Patel
2026-10-09 11:45   ` Anup Patel
2026-10-09 13:44   ` Andrew Jones
2026-10-09 13:44     ` Andrew Jones
2026-10-09 13:44     ` Andrew Jones
2026-10-09 16:37     ` Anup Patel
2026-10-09 16:37       ` Anup Patel
2026-10-09 16:37       ` Anup Patel
2026-10-17 12:29       ` Andrew Jones
2026-10-17 12:29         ` Andrew Jones
2026-10-17 12:29         ` Andrew Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.