Currently, the common AIA functions kvm_riscv_vcpu_aia_has_interrupts()
and kvm_riscv_aia_wakeon_hgei() lookup HGEI line using an array of VCPU
pointers before accessing HGEI[E|P] CSR which is slow and prone to race
conditions because there is a separate per-hart lock for the VCPU pointer
array and a separate per-VCPU rwlock for IMSIC VS-file (including HGEI
line) used by the VCPU. Due to these race conditions, it is observed
on QEMU RISC-V host that Guest VCPUs sleep in WFI and never wakeup even
with interrupt pending in the IMSIC VS-file because VCPUs were waiting
for HGEI wakeup on the wrong host CPU.
The IMSIC virtualization already keeps track of the HGEI line and the
associated IMSIC VS-file used by each VCPU so move the HGEI[E|P] CSR
access to IMSIC virtualization so that costly HGEI line lookup can be
avoided and likelihood of race-conditions when updating HGEI[E|P] CSR
is also reduced.
Reviewed-by: Atish Patra <atishp@rivosinc.com>
Tested-by: Atish Patra <atishp@rivosinc.com>
Tested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Fixes: 3385339296d1 ("RISC-V: KVM: Use IMSIC guest files when available")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
arch/riscv/include/asm/kvm_aia.h | 4 ++-
arch/riscv/kvm/aia.c | 51 +++++---------------------------
arch/riscv/kvm/aia_imsic.c | 45 ++++++++++++++++++++++++++++
arch/riscv/kvm/vcpu.c | 2 --
4 files changed, 55 insertions(+), 47 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
index 0a0f12496f00..b04ecdd1a860 100644
--- a/arch/riscv/include/asm/kvm_aia.h
+++ b/arch/riscv/include/asm/kvm_aia.h
@@ -87,6 +87,9 @@ DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
extern struct kvm_device_ops kvm_riscv_aia_device_ops;
+bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu);
+void kvm_riscv_vcpu_aia_imsic_load(struct kvm_vcpu *vcpu, int cpu);
+void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu);
void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu);
int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu);
@@ -161,7 +164,6 @@ void kvm_riscv_aia_destroy_vm(struct kvm *kvm);
int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
void __iomem **hgei_va, phys_addr_t *hgei_pa);
void kvm_riscv_aia_free_hgei(int cpu, int hgei);
-void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable);
void kvm_riscv_aia_enable(void);
void kvm_riscv_aia_disable(void);
diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index 19afd1f23537..dad318185660 100644
--- a/arch/riscv/kvm/aia.c
+++ b/arch/riscv/kvm/aia.c
@@ -30,28 +30,6 @@ unsigned int kvm_riscv_aia_nr_hgei;
unsigned int kvm_riscv_aia_max_ids;
DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
-static int aia_find_hgei(struct kvm_vcpu *owner)
-{
- int i, hgei;
- unsigned long flags;
- struct aia_hgei_control *hgctrl = get_cpu_ptr(&aia_hgei);
-
- raw_spin_lock_irqsave(&hgctrl->lock, flags);
-
- hgei = -1;
- for (i = 1; i <= kvm_riscv_aia_nr_hgei; i++) {
- if (hgctrl->owners[i] == owner) {
- hgei = i;
- break;
- }
- }
-
- raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
-
- put_cpu_ptr(&aia_hgei);
- return hgei;
-}
-
static inline unsigned long aia_hvictl_value(bool ext_irq_pending)
{
unsigned long hvictl;
@@ -95,7 +73,6 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
{
- int hgei;
unsigned long seip;
if (!kvm_riscv_aia_available())
@@ -114,11 +91,7 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
return false;
- hgei = aia_find_hgei(vcpu);
- if (hgei > 0)
- return !!(ncsr_read(CSR_HGEIP) & BIT(hgei));
-
- return false;
+ return kvm_riscv_vcpu_aia_imsic_has_interrupt(vcpu);
}
void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
@@ -164,6 +137,9 @@ void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
csr_write(CSR_HVIPRIO2H, csr->hviprio2h);
#endif
}
+
+ if (kvm_riscv_aia_initialized(vcpu->kvm))
+ kvm_riscv_vcpu_aia_imsic_load(vcpu, cpu);
}
void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
@@ -174,6 +150,9 @@ void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
if (!kvm_riscv_aia_available())
return;
+ if (kvm_riscv_aia_initialized(vcpu->kvm))
+ kvm_riscv_vcpu_aia_imsic_put(vcpu);
+
if (kvm_riscv_nacl_available()) {
nsh = nacl_shmem();
csr->vsiselect = nacl_csr_read(nsh, CSR_VSISELECT);
@@ -472,22 +451,6 @@ void kvm_riscv_aia_free_hgei(int cpu, int hgei)
raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
}
-void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable)
-{
- int hgei;
-
- if (!kvm_riscv_aia_available())
- return;
-
- hgei = aia_find_hgei(owner);
- if (hgei > 0) {
- if (enable)
- csr_set(CSR_HGEIE, BIT(hgei));
- else
- csr_clear(CSR_HGEIE, BIT(hgei));
- }
-}
-
static irqreturn_t hgei_interrupt(int irq, void *dev_id)
{
int i;
diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
index ea1a36836d9c..fda0346f0ea1 100644
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -677,6 +677,48 @@ static void imsic_swfile_update(struct kvm_vcpu *vcpu,
imsic_swfile_extirq_update(vcpu);
}
+bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu)
+{
+ struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
+ unsigned long flags;
+ bool ret = false;
+
+ /*
+ * The IMSIC SW-file directly injects interrupt via hvip so
+ * only check for interrupt when IMSIC VS-file is being used.
+ */
+
+ read_lock_irqsave(&imsic->vsfile_lock, flags);
+ if (imsic->vsfile_cpu > -1)
+ ret = !!(csr_read(CSR_HGEIP) & BIT(imsic->vsfile_hgei));
+ read_unlock_irqrestore(&imsic->vsfile_lock, flags);
+
+ return ret;
+}
+
+void kvm_riscv_vcpu_aia_imsic_load(struct kvm_vcpu *vcpu, int cpu)
+{
+ /*
+ * No need to explicitly clear HGEIE CSR bits because the
+ * hgei interrupt handler (aka hgei_interrupt()) will always
+ * clear it for us.
+ */
+}
+
+void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu)
+{
+ struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
+ unsigned long flags;
+
+ if (!kvm_vcpu_is_blocking(vcpu))
+ return;
+
+ read_lock_irqsave(&imsic->vsfile_lock, flags);
+ if (imsic->vsfile_cpu > -1)
+ csr_set(CSR_HGEIE, BIT(imsic->vsfile_hgei));
+ read_unlock_irqrestore(&imsic->vsfile_lock, flags);
+}
+
void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
{
unsigned long flags;
@@ -781,6 +823,9 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu)
* producers to the new IMSIC VS-file.
*/
+ /* Ensure HGEIE CSR bit is zero before using the new IMSIC VS-file */
+ csr_clear(CSR_HGEIE, BIT(new_vsfile_hgei));
+
/* Zero-out new IMSIC VS-file */
imsic_vsfile_local_clear(new_vsfile_hgei, imsic->nr_hw_eix);
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index fe028b4274df..b26bf35a0a19 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -211,12 +211,10 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
{
- kvm_riscv_aia_wakeon_hgei(vcpu, true);
}
void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
{
- kvm_riscv_aia_wakeon_hgei(vcpu, false);
}
int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
--
2.43.0On 7/7/2025 11:53 AM, Anup Patel wrote:
> Currently, the common AIA functions kvm_riscv_vcpu_aia_has_interrupts()
> and kvm_riscv_aia_wakeon_hgei() lookup HGEI line using an array of VCPU
> pointers before accessing HGEI[E|P] CSR which is slow and prone to race
> conditions because there is a separate per-hart lock for the VCPU pointer
> array and a separate per-VCPU rwlock for IMSIC VS-file (including HGEI
> line) used by the VCPU. Due to these race conditions, it is observed
> on QEMU RISC-V host that Guest VCPUs sleep in WFI and never wakeup even
> with interrupt pending in the IMSIC VS-file because VCPUs were waiting
> for HGEI wakeup on the wrong host CPU.
>
> The IMSIC virtualization already keeps track of the HGEI line and the
> associated IMSIC VS-file used by each VCPU so move the HGEI[E|P] CSR
> access to IMSIC virtualization so that costly HGEI line lookup can be
> avoided and likelihood of race-conditions when updating HGEI[E|P] CSR
> is also reduced.
>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> Tested-by: Atish Patra <atishp@rivosinc.com>
> Tested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Fixes: 3385339296d1 ("RISC-V: KVM: Use IMSIC guest files when available")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> arch/riscv/include/asm/kvm_aia.h | 4 ++-
> arch/riscv/kvm/aia.c | 51 +++++---------------------------
> arch/riscv/kvm/aia_imsic.c | 45 ++++++++++++++++++++++++++++
> arch/riscv/kvm/vcpu.c | 2 --
> 4 files changed, 55 insertions(+), 47 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> index 0a0f12496f00..b04ecdd1a860 100644
> --- a/arch/riscv/include/asm/kvm_aia.h
> +++ b/arch/riscv/include/asm/kvm_aia.h
> @@ -87,6 +87,9 @@ DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
>
> extern struct kvm_device_ops kvm_riscv_aia_device_ops;
>
> +bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu);
> +void kvm_riscv_vcpu_aia_imsic_load(struct kvm_vcpu *vcpu, int cpu);
> +void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu);
> void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu);
> int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu);
>
> @@ -161,7 +164,6 @@ void kvm_riscv_aia_destroy_vm(struct kvm *kvm);
> int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
> void __iomem **hgei_va, phys_addr_t *hgei_pa);
> void kvm_riscv_aia_free_hgei(int cpu, int hgei);
> -void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable);
>
> void kvm_riscv_aia_enable(void);
> void kvm_riscv_aia_disable(void);
> diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> index 19afd1f23537..dad318185660 100644
> --- a/arch/riscv/kvm/aia.c
> +++ b/arch/riscv/kvm/aia.c
> @@ -30,28 +30,6 @@ unsigned int kvm_riscv_aia_nr_hgei;
> unsigned int kvm_riscv_aia_max_ids;
> DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
>
> -static int aia_find_hgei(struct kvm_vcpu *owner)
> -{
> - int i, hgei;
> - unsigned long flags;
> - struct aia_hgei_control *hgctrl = get_cpu_ptr(&aia_hgei);
> -
> - raw_spin_lock_irqsave(&hgctrl->lock, flags);
> -
> - hgei = -1;
> - for (i = 1; i <= kvm_riscv_aia_nr_hgei; i++) {
> - if (hgctrl->owners[i] == owner) {
> - hgei = i;
> - break;
> - }
> - }
> -
> - raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> -
> - put_cpu_ptr(&aia_hgei);
> - return hgei;
> -}
> -
> static inline unsigned long aia_hvictl_value(bool ext_irq_pending)
> {
> unsigned long hvictl;
> @@ -95,7 +73,6 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
>
> bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> {
> - int hgei;
> unsigned long seip;
>
> if (!kvm_riscv_aia_available())
> @@ -114,11 +91,7 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
> return false;
>
> - hgei = aia_find_hgei(vcpu);
> - if (hgei > 0)
> - return !!(ncsr_read(CSR_HGEIP) & BIT(hgei));
> -
> - return false;
> + return kvm_riscv_vcpu_aia_imsic_has_interrupt(vcpu);
> }
>
> void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
> @@ -164,6 +137,9 @@ void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
> csr_write(CSR_HVIPRIO2H, csr->hviprio2h);
> #endif
> }
> +
> + if (kvm_riscv_aia_initialized(vcpu->kvm))
> + kvm_riscv_vcpu_aia_imsic_load(vcpu, cpu);
> }
>
> void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
> @@ -174,6 +150,9 @@ void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
> if (!kvm_riscv_aia_available())
> return;
>
> + if (kvm_riscv_aia_initialized(vcpu->kvm))
> + kvm_riscv_vcpu_aia_imsic_put(vcpu);
> +
> if (kvm_riscv_nacl_available()) {
> nsh = nacl_shmem();
> csr->vsiselect = nacl_csr_read(nsh, CSR_VSISELECT);
> @@ -472,22 +451,6 @@ void kvm_riscv_aia_free_hgei(int cpu, int hgei)
> raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> }
>
> -void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable)
> -{
> - int hgei;
> -
> - if (!kvm_riscv_aia_available())
> - return;
> -
> - hgei = aia_find_hgei(owner);
> - if (hgei > 0) {
> - if (enable)
> - csr_set(CSR_HGEIE, BIT(hgei));
> - else
> - csr_clear(CSR_HGEIE, BIT(hgei));
> - }
> -}
> -
> static irqreturn_t hgei_interrupt(int irq, void *dev_id)
> {
> int i;
> diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
> index ea1a36836d9c..fda0346f0ea1 100644
> --- a/arch/riscv/kvm/aia_imsic.c
> +++ b/arch/riscv/kvm/aia_imsic.c
> @@ -677,6 +677,48 @@ static void imsic_swfile_update(struct kvm_vcpu *vcpu,
> imsic_swfile_extirq_update(vcpu);
> }
>
> +bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu)
> +{
> + struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
> + unsigned long flags;
> + bool ret = false;
> +
> + /*
> + * The IMSIC SW-file directly injects interrupt via hvip so
> + * only check for interrupt when IMSIC VS-file is being used.
> + */
> +
> + read_lock_irqsave(&imsic->vsfile_lock, flags);
> + if (imsic->vsfile_cpu > -1)
> + ret = !!(csr_read(CSR_HGEIP) & BIT(imsic->vsfile_hgei));
> + read_unlock_irqrestore(&imsic->vsfile_lock, flags);
> +
> + return ret;
> +}
> +
> +void kvm_riscv_vcpu_aia_imsic_load(struct kvm_vcpu *vcpu, int cpu)
> +{
> + /*
> + * No need to explicitly clear HGEIE CSR bits because the
> + * hgei interrupt handler (aka hgei_interrupt()) will always
> + * clear it for us.
> + */
> +}
> +
> +void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu)
> +{
> + struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
> + unsigned long flags;
> +
> + if (!kvm_vcpu_is_blocking(vcpu))
> + return;
> +
> + read_lock_irqsave(&imsic->vsfile_lock, flags);
> + if (imsic->vsfile_cpu > -1)
> + csr_set(CSR_HGEIE, BIT(imsic->vsfile_hgei));
> + read_unlock_irqrestore(&imsic->vsfile_lock, flags);
> +}
> +
> void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
> {
> unsigned long flags;
> @@ -781,6 +823,9 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu)
> * producers to the new IMSIC VS-file.
> */
>
> + /* Ensure HGEIE CSR bit is zero before using the new IMSIC VS-file */
> + csr_clear(CSR_HGEIE, BIT(new_vsfile_hgei));
> +
> /* Zero-out new IMSIC VS-file */
> imsic_vsfile_local_clear(new_vsfile_hgei, imsic->nr_hw_eix);
>
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index fe028b4274df..b26bf35a0a19 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -211,12 +211,10 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>
> void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> {
> - kvm_riscv_aia_wakeon_hgei(vcpu, true);
> }
>
> void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> {
> - kvm_riscv_aia_wakeon_hgei(vcpu, false);
> }
>
Nitpick:
Should the above two empty functions be removed ?
Reviewed-by: Nutty Liu<liujingqi@lanxincomputing.com>
Thanks,
Nutty
> int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
On Mon, Jul 7, 2025 at 9:50 AM Nutty Liu <liujingqi@lanxincomputing.com> wrote:
>
> On 7/7/2025 11:53 AM, Anup Patel wrote:
> > Currently, the common AIA functions kvm_riscv_vcpu_aia_has_interrupts()
> > and kvm_riscv_aia_wakeon_hgei() lookup HGEI line using an array of VCPU
> > pointers before accessing HGEI[E|P] CSR which is slow and prone to race
> > conditions because there is a separate per-hart lock for the VCPU pointer
> > array and a separate per-VCPU rwlock for IMSIC VS-file (including HGEI
> > line) used by the VCPU. Due to these race conditions, it is observed
> > on QEMU RISC-V host that Guest VCPUs sleep in WFI and never wakeup even
> > with interrupt pending in the IMSIC VS-file because VCPUs were waiting
> > for HGEI wakeup on the wrong host CPU.
> >
> > The IMSIC virtualization already keeps track of the HGEI line and the
> > associated IMSIC VS-file used by each VCPU so move the HGEI[E|P] CSR
> > access to IMSIC virtualization so that costly HGEI line lookup can be
> > avoided and likelihood of race-conditions when updating HGEI[E|P] CSR
> > is also reduced.
> >
> > Reviewed-by: Atish Patra <atishp@rivosinc.com>
> > Tested-by: Atish Patra <atishp@rivosinc.com>
> > Tested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > Fixes: 3385339296d1 ("RISC-V: KVM: Use IMSIC guest files when available")
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > arch/riscv/include/asm/kvm_aia.h | 4 ++-
> > arch/riscv/kvm/aia.c | 51 +++++---------------------------
> > arch/riscv/kvm/aia_imsic.c | 45 ++++++++++++++++++++++++++++
> > arch/riscv/kvm/vcpu.c | 2 --
> > 4 files changed, 55 insertions(+), 47 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
> > index 0a0f12496f00..b04ecdd1a860 100644
> > --- a/arch/riscv/include/asm/kvm_aia.h
> > +++ b/arch/riscv/include/asm/kvm_aia.h
> > @@ -87,6 +87,9 @@ DECLARE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> >
> > extern struct kvm_device_ops kvm_riscv_aia_device_ops;
> >
> > +bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu);
> > +void kvm_riscv_vcpu_aia_imsic_load(struct kvm_vcpu *vcpu, int cpu);
> > +void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu);
> > void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu);
> > int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu);
> >
> > @@ -161,7 +164,6 @@ void kvm_riscv_aia_destroy_vm(struct kvm *kvm);
> > int kvm_riscv_aia_alloc_hgei(int cpu, struct kvm_vcpu *owner,
> > void __iomem **hgei_va, phys_addr_t *hgei_pa);
> > void kvm_riscv_aia_free_hgei(int cpu, int hgei);
> > -void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable);
> >
> > void kvm_riscv_aia_enable(void);
> > void kvm_riscv_aia_disable(void);
> > diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
> > index 19afd1f23537..dad318185660 100644
> > --- a/arch/riscv/kvm/aia.c
> > +++ b/arch/riscv/kvm/aia.c
> > @@ -30,28 +30,6 @@ unsigned int kvm_riscv_aia_nr_hgei;
> > unsigned int kvm_riscv_aia_max_ids;
> > DEFINE_STATIC_KEY_FALSE(kvm_riscv_aia_available);
> >
> > -static int aia_find_hgei(struct kvm_vcpu *owner)
> > -{
> > - int i, hgei;
> > - unsigned long flags;
> > - struct aia_hgei_control *hgctrl = get_cpu_ptr(&aia_hgei);
> > -
> > - raw_spin_lock_irqsave(&hgctrl->lock, flags);
> > -
> > - hgei = -1;
> > - for (i = 1; i <= kvm_riscv_aia_nr_hgei; i++) {
> > - if (hgctrl->owners[i] == owner) {
> > - hgei = i;
> > - break;
> > - }
> > - }
> > -
> > - raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > -
> > - put_cpu_ptr(&aia_hgei);
> > - return hgei;
> > -}
> > -
> > static inline unsigned long aia_hvictl_value(bool ext_irq_pending)
> > {
> > unsigned long hvictl;
> > @@ -95,7 +73,6 @@ void kvm_riscv_vcpu_aia_sync_interrupts(struct kvm_vcpu *vcpu)
> >
> > bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> > {
> > - int hgei;
> > unsigned long seip;
> >
> > if (!kvm_riscv_aia_available())
> > @@ -114,11 +91,7 @@ bool kvm_riscv_vcpu_aia_has_interrupts(struct kvm_vcpu *vcpu, u64 mask)
> > if (!kvm_riscv_aia_initialized(vcpu->kvm) || !seip)
> > return false;
> >
> > - hgei = aia_find_hgei(vcpu);
> > - if (hgei > 0)
> > - return !!(ncsr_read(CSR_HGEIP) & BIT(hgei));
> > -
> > - return false;
> > + return kvm_riscv_vcpu_aia_imsic_has_interrupt(vcpu);
> > }
> >
> > void kvm_riscv_vcpu_aia_update_hvip(struct kvm_vcpu *vcpu)
> > @@ -164,6 +137,9 @@ void kvm_riscv_vcpu_aia_load(struct kvm_vcpu *vcpu, int cpu)
> > csr_write(CSR_HVIPRIO2H, csr->hviprio2h);
> > #endif
> > }
> > +
> > + if (kvm_riscv_aia_initialized(vcpu->kvm))
> > + kvm_riscv_vcpu_aia_imsic_load(vcpu, cpu);
> > }
> >
> > void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
> > @@ -174,6 +150,9 @@ void kvm_riscv_vcpu_aia_put(struct kvm_vcpu *vcpu)
> > if (!kvm_riscv_aia_available())
> > return;
> >
> > + if (kvm_riscv_aia_initialized(vcpu->kvm))
> > + kvm_riscv_vcpu_aia_imsic_put(vcpu);
> > +
> > if (kvm_riscv_nacl_available()) {
> > nsh = nacl_shmem();
> > csr->vsiselect = nacl_csr_read(nsh, CSR_VSISELECT);
> > @@ -472,22 +451,6 @@ void kvm_riscv_aia_free_hgei(int cpu, int hgei)
> > raw_spin_unlock_irqrestore(&hgctrl->lock, flags);
> > }
> >
> > -void kvm_riscv_aia_wakeon_hgei(struct kvm_vcpu *owner, bool enable)
> > -{
> > - int hgei;
> > -
> > - if (!kvm_riscv_aia_available())
> > - return;
> > -
> > - hgei = aia_find_hgei(owner);
> > - if (hgei > 0) {
> > - if (enable)
> > - csr_set(CSR_HGEIE, BIT(hgei));
> > - else
> > - csr_clear(CSR_HGEIE, BIT(hgei));
> > - }
> > -}
> > -
> > static irqreturn_t hgei_interrupt(int irq, void *dev_id)
> > {
> > int i;
> > diff --git a/arch/riscv/kvm/aia_imsic.c b/arch/riscv/kvm/aia_imsic.c
> > index ea1a36836d9c..fda0346f0ea1 100644
> > --- a/arch/riscv/kvm/aia_imsic.c
> > +++ b/arch/riscv/kvm/aia_imsic.c
> > @@ -677,6 +677,48 @@ static void imsic_swfile_update(struct kvm_vcpu *vcpu,
> > imsic_swfile_extirq_update(vcpu);
> > }
> >
> > +bool kvm_riscv_vcpu_aia_imsic_has_interrupt(struct kvm_vcpu *vcpu)
> > +{
> > + struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
> > + unsigned long flags;
> > + bool ret = false;
> > +
> > + /*
> > + * The IMSIC SW-file directly injects interrupt via hvip so
> > + * only check for interrupt when IMSIC VS-file is being used.
> > + */
> > +
> > + read_lock_irqsave(&imsic->vsfile_lock, flags);
> > + if (imsic->vsfile_cpu > -1)
> > + ret = !!(csr_read(CSR_HGEIP) & BIT(imsic->vsfile_hgei));
> > + read_unlock_irqrestore(&imsic->vsfile_lock, flags);
> > +
> > + return ret;
> > +}
> > +
> > +void kvm_riscv_vcpu_aia_imsic_load(struct kvm_vcpu *vcpu, int cpu)
> > +{
> > + /*
> > + * No need to explicitly clear HGEIE CSR bits because the
> > + * hgei interrupt handler (aka hgei_interrupt()) will always
> > + * clear it for us.
> > + */
> > +}
> > +
> > +void kvm_riscv_vcpu_aia_imsic_put(struct kvm_vcpu *vcpu)
> > +{
> > + struct imsic *imsic = vcpu->arch.aia_context.imsic_state;
> > + unsigned long flags;
> > +
> > + if (!kvm_vcpu_is_blocking(vcpu))
> > + return;
> > +
> > + read_lock_irqsave(&imsic->vsfile_lock, flags);
> > + if (imsic->vsfile_cpu > -1)
> > + csr_set(CSR_HGEIE, BIT(imsic->vsfile_hgei));
> > + read_unlock_irqrestore(&imsic->vsfile_lock, flags);
> > +}
> > +
> > void kvm_riscv_vcpu_aia_imsic_release(struct kvm_vcpu *vcpu)
> > {
> > unsigned long flags;
> > @@ -781,6 +823,9 @@ int kvm_riscv_vcpu_aia_imsic_update(struct kvm_vcpu *vcpu)
> > * producers to the new IMSIC VS-file.
> > */
> >
> > + /* Ensure HGEIE CSR bit is zero before using the new IMSIC VS-file */
> > + csr_clear(CSR_HGEIE, BIT(new_vsfile_hgei));
> > +
> > /* Zero-out new IMSIC VS-file */
> > imsic_vsfile_local_clear(new_vsfile_hgei, imsic->nr_hw_eix);
> >
> > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> > index fe028b4274df..b26bf35a0a19 100644
> > --- a/arch/riscv/kvm/vcpu.c
> > +++ b/arch/riscv/kvm/vcpu.c
> > @@ -211,12 +211,10 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
> >
> > void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
> > {
> > - kvm_riscv_aia_wakeon_hgei(vcpu, true);
> > }
> >
> > void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
> > {
> > - kvm_riscv_aia_wakeon_hgei(vcpu, false);
> > }
> >
>
> Nitpick:
> Should the above two empty functions be removed ?
>
We cannot remove these functions but we can certainly
make them static inline. I will update in the next revision.
Thanks,
Anup
© 2016 - 2025 Red Hat, Inc.