RISC-V: KVM: Move HGEI[E|P] CSR access to IMSIC virtualization

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>
Reviewed-by: Nutty Liu <liujingqi@lanxincomputing.com>
Link: https://lore.kernel.org/r/20250707035345.17494-3-apatel@ventanamicro.com
Signed-off-by: Anup Patel <anup@brainfault.org>

diff --git a/arch/riscv/include/asm/kvm_aia.h b/arch/riscv/include/asm/kvm_aia.h
index 3b643b9efc07b9ad7f35c304b81ff6ac71eb21d1..5acce285e56e1ea68871f892e6404a4218ec4269 100644 (file)
--- 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/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 85cfebc32e4cf58a818c37c4dc4ac1c47eaf0506..bcbf8b1ec1156176acf8a326fbc5335f29243482 100644 (file)
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -306,6 +306,9 @@ static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu)
        return IS_ENABLED(CONFIG_GUEST_PERF_EVENTS) && !!vcpu;
 }
 
+static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
+static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
+
 #define KVM_RISCV_GSTAGE_TLB_MIN_ORDER         12
 
 void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,

diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index 19afd1f235377682488304d28900bfcbc93e4e61..dad3181856600f13bd5b1d2bf36f21766da4711c 100644 (file)
--- 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 29ef9c2133a93317f0f9af284cbbdcbc3b140fa4..2ff865943ebbcdbd6d22851fc67b3dce2a71daa7 100644 (file)
--- a/arch/riscv/kvm/aia_imsic.c
+++ b/arch/riscv/kvm/aia_imsic.c
@@ -676,6 +676,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 e0a01af426ff2a85888da456b39f54e557039647..0462863206ca7e1b976094b2826ed89b1bf87413 100644 (file)
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -207,16 +207,6 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
        return kvm_riscv_vcpu_timer_pending(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)
 {
        return (kvm_riscv_vcpu_has_interrupts(vcpu, -1UL) &&