[PATCH v3 35/39] xen/riscv: wake up a descheduled vCPU on a guest external interrupt

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

While a vCPU is running, MSIs written to its h/w IMSIC guest interrupt
file are delivered straight to VS-mode. Once the vCPU is descheduled
nobody observes that file anymore, so a guest blocked on such an
interrupt would stay blocked until some unrelated event happens to
schedule it again.

Let Xen observe the file in that window: on deschedule set the vCPU's
bit in HGEIE, which turns an interrupt pending in its VS-file into an
HS-level SGEI, and clear the bit again on schedule-in. HGEIP only
reports an interrupt file number, so to find the vCPU to kick, keep a
per-pCPU owners[] array indexed by file number and updated in
vgein_assign()/vgein_release() together with the VGEIN bitmap. The SGEI
handler clears the HGEIE bits it has seen before kicking their owners,
so that the interrupt doesn't keep firing until the vCPU is switched in.

The same applies to a vCPU which is migrated to another pCPU while it
isn't running. Its old interrupt file is released during the migration,
and the vCPU isn't switched in on the new pCPU until something wakes it
up, so nothing would observe the new interrupt file. Hence arm HGEIE for
the new interrupt file on the new pCPU in imsic_migrate_vcpu(), and
clear the bit of the old one in imsic_vsfile_local_read_clear(), which
already runs on the pCPU owning it.

SGEIs are enabled per vCPU through v->arch.hie: vcpu_csr_init() sets
IRQ_S_GEXT in it and csr_regs_ctxt_switch_to() loads it into the CSR.

vgein_release() is only called on vCPU migration: a vCPU going away
also has to free its VGEIN slot and drop the owners[] entry, but there
is no vCPU teardown path to hook it into yet.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in v3:
 - Drop the unrelated cpu_callback() change.
 - Read HGEIP and HGEIE in separate statements (MISRA C Rule 13.2).
 - Check that guest interrupt file ID 0 isn't reported once, ahead of
   the loop in hgei_interrupt(), instead of on every iteration.
 - Drop the VGEIN_DEBUG print from hgei_interrupt().
 - Explain in imsic_ctxt_switch_from() why setting the HGEIE bit last
   doesn't lose an interrupt.
 - Initialise hie with BIT(IRQ_S_GEXT, UL) and explain why SGEIs are
   enabled.
 - Arm HGEIE of the new interrupt file on v->processor, as the
   new_vsfile_cpu local variable is gone from imsic_migrate_vcpu().
 - Describe in the commit message why HGEIE is armed for a vCPU which
   is migrated while it isn't running.
 - Correct the commit message: vgein_release() is called on vCPU
   migration.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/aia.c                | 38 ++++++++++++++++++
 xen/arch/riscv/domain.c             |  7 ++++
 xen/arch/riscv/imsic.c              | 62 ++++++++++++++++++++++++++++-
 xen/arch/riscv/include/asm/aia.h    |  2 +
 xen/arch/riscv/include/asm/domain.h |  1 +
 xen/arch/riscv/traps.c              |  7 +++-
 6 files changed, 115 insertions(+), 2 deletions(-)

diff --git a/xen/arch/riscv/aia.c b/xen/arch/riscv/aia.c
index 1aca07c2f70f..d961c5c12239 100644
--- a/xen/arch/riscv/aia.c
+++ b/xen/arch/riscv/aia.c
@@ -18,6 +18,13 @@ struct vgein_ctrl {
     /* The least-significant bits are implemented first, apart from bit 0 */
     unsigned long bmp;
     spinlock_t lock;
+    /*
+     * Guest interrupt file IDs run from 1 to geilen inclusive (0 means that
+     * no guest external interrupt source is selected), and geilen can never
+     * exceed BITS_PER_LONG - 1, so indexing this array by the ID directly
+     * always fits.
+     */
+    struct vcpu *owners[BITS_PER_LONG];
     unsigned int geilen;
 };
 
@@ -138,7 +145,10 @@ unsigned int vgein_assign(struct vcpu *v)
     if ( vgein_id > vgein->geilen )
         vgein_id = 0;
     else
+    {
         __set_bit(vgein_id, bmp);
+        vgein->owners[vgein_id] = v;
+    }
 
     spin_unlock_irqrestore(&vgein->lock, flags);
 
@@ -161,6 +171,7 @@ void vgein_release(struct vcpu *v, unsigned int vgein_id, 
unsigned int cpu)
     spin_lock_irqsave(&vgein->lock, flags);
     if ( !__test_and_clear_bit(vgein_id, &vgein->bmp) )
         ASSERT_UNREACHABLE();
+    vgein->owners[vgein_id] = NULL;
     spin_unlock_irqrestore(&vgein->lock, flags);
 
 #ifdef VGEIN_DEBUG
@@ -168,3 +179,30 @@ void vgein_release(struct vcpu *v, unsigned int vgein_id, 
unsigned int cpu)
             __func__, v, vgein_id, cpu, vgein->bmp);
 #endif
 }
+
+void hgei_interrupt(void)
+{
+    unsigned long hgei_mask, flags;
+    struct vgein_ctrl *vgein = &this_cpu(vgein);
+
+    hgei_mask = csr_read(CSR_HGEIP);
+    hgei_mask &= csr_read(CSR_HGEIE);
+
+    csr_clear(CSR_HGEIE, hgei_mask);
+
+    spin_lock_irqsave(&vgein->lock, flags);
+
+    /*
+     * Bit 0 of HGEIP/HGEIE is read-only zero: guest interrupt file ID 0
+     * means that no guest external interrupt source is selected.
+     */
+    ASSERT(!(hgei_mask & 1));
+
+    for_each_set_bit ( guest_file_id, hgei_mask )
+    {
+        if ( vgein->owners[guest_file_id] )
+            vcpu_kick(vgein->owners[guest_file_id]);
+    }
+
+    spin_unlock_irqrestore(&vgein->lock, flags);
+}
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index f60f871aba98..6cb762bee33c 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -130,6 +130,12 @@ static void vcpu_csr_init(struct vcpu *v)
         v->arch.hstateen0 = (hstateen0 & csr_masks.hstateen0) |
                             csr_masks.ro_one.hstateen0;
     }
+
+    /*
+     * Enable SGEIs, so that a guest interrupt file marked in HGEIE while
+     * the vCPU is descheduled can raise an interrupt to Xen.
+     */
+    v->arch.hie = BIT(IRQ_S_GEXT, UL);
 }
 
 static void continue_new_vcpu(struct vcpu *prev)
@@ -386,6 +392,7 @@ static void csr_regs_ctxt_switch_to(struct vcpu *n)
     csr_write(CSR_HEDELEG, n->arch.hedeleg);
     csr_write(CSR_HIDELEG, n->arch.hideleg);
     csr_write(CSR_HVIP, n->arch.hvip);
+    csr_write(CSR_HIE, n->arch.hie);
     csr_write64(CSR_HENVCFG, n->arch.henvcfg);
     csr_write(CSR_HCOUNTEREN, n->arch.hcounteren);
     csr_write64(CSR_HTIMEDELTA, n->arch.htimedelta);
diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
index e35688f98bb1..40d2eff14d07 100644
--- a/xen/arch/riscv/imsic.c
+++ b/xen/arch/riscv/imsic.c
@@ -555,12 +555,37 @@ void cf_check imsic_ctxt_switch_from(struct vcpu *p)
 
     write_lock_irqsave(&imsic_state->vsfile_lock, flags);
     imsic_state->vsfile_cpu = p->processor;
+    /*
+     * Start to observe the VS-file from HS-mode: while the vCPU isn't
+     * running an interrupt pending in its VS-file is reported through HGEIP
+     * instead of being delivered to VS-mode, which lets Xen wake the vCPU up.
+     *
+     * An MSI is recorded in the VS-file's eip[] array until the guest claims
+     * it, and HGEIP isn't latched but reflects whether the file currently
+     * has a pending-and-enabled interrupt. Hence an interrupt which arrived
+     * before this point raises an SGEI as soon as the bit is set in HGEIE;
+     * nothing is lost in between.
+     */
+    csr_set(CSR_HGEIE, BIT(imsic_state->guest_file_id, UL));
     write_unlock_irqrestore(&imsic_state->vsfile_lock, flags);
 }
 
 void cf_check imsic_ctxt_switch_to(struct vcpu *n)
 {
-    /* Nothing to do */
+    struct vimsic_state *imsic_state = n->arch.vimsic_state;
+    unsigned long flags;
+
+    /* A s/w VS-file is never observed through HGEIP. */
+    if ( !vcpu_guest_file_id(n) )
+        return;
+
+    /*
+     * The vCPU is about to run, so hstatus.VGEIN delivers the VS-file's
+     * interrupts to it directly and there is nothing left for Xen to observe.
+     */
+    read_lock_irqsave(&imsic_state->vsfile_lock, flags);
+    csr_clear(CSR_HGEIE, BIT(imsic_state->guest_file_id, UL));
+    read_unlock_irqrestore(&imsic_state->vsfile_lock, flags);
 }
 
 int imsic_map_guest_file(struct vcpu *v, unsigned int vsfile_id)
@@ -695,6 +720,12 @@ static void cf_check imsic_vsfile_local_read_clear(void 
*data)
     struct imsic_mrif *mrif = idata->mrif;
     unsigned long new_hstatus, old_hstatus, old_vsiselect;
 
+    /*
+     * The HGEIE bit imsic_ctxt_switch_from() armed belongs to the old owner
+     * only.
+     */
+    csr_clear(CSR_HGEIE, BIT(idata->hgei, UL));
+
     /*
      * The file is reached by retargeting hstatus.VGEIN, and
      * imsic_vs_csr_swap() alters vsiselect. Both belong to the vCPU running
@@ -1044,6 +1075,21 @@ int __init vimsic_make_domu_dt_node(struct kernel_info 
*kinfo,
     return fdt_end_node(fdt);
 }
 
+/*
+ * Start to observe the interrupt file from HS-mode, the same way
+ * imsic_ctxt_switch_from() does it for a vCPU which is switched out.
+ *
+ * The counterpart, clearing the bit of the interrupt file which is left
+ * behind, is done by imsic_vsfile_local_read_clear(), which already runs on
+ * the pCPU owning that file.
+ */
+static void cf_check imsic_local_hgeie_set(void *data)
+{
+    const struct imsic_vsfile_data *idata = data;
+
+    csr_set(CSR_HGEIE, BIT(idata->hgei, UL));
+}
+
 static void cf_check imsic_vsfile_local_update(void *data)
 {
     unsigned int i;
@@ -1225,6 +1271,20 @@ void imsic_migrate_vcpu(struct vcpu *v)
     /* Restore register state in the new IMSIC VS-file */
     imsic_call_on_cpu(v->processor, imsic_vsfile_local_update, &vsfile_data);
 
+    /*
+     * imsic_ctxt_switch_from() armed HGEIE for the old interrupt file only.
+     * Unless the vCPU is being switched in right now (a migration done by
+     * sched_context_switch(), where is_running is already set), arm it for
+     * the new file too: a blocked vCPU would otherwise never be woken up by
+     * an interrupt pending in the new file, be it restored from the old one
+     * or arriving later.
+     *
+     * For a vCPU which is being switched in, imsic_ctxt_switch_to() clears the
+     * bit anyway, as interrupts are then delivered to the vCPU directly.
+     */
+    if ( !v->is_running )
+        imsic_call_on_cpu(v->processor, imsic_local_hgeie_set, &vsfile_data);
+
     /* Set VCPU HSTATUS.VGEIN to new IMSIC VS-file */
     vcpu_guest_cpu_user_regs(v)->hstatus &= ~HSTATUS_VGEIN;
     vcpu_guest_cpu_user_regs(v)->hstatus |=
diff --git a/xen/arch/riscv/include/asm/aia.h b/xen/arch/riscv/include/asm/aia.h
index 8e4eb2f6b14e..6a05bdd8c236 100644
--- a/xen/arch/riscv/include/asm/aia.h
+++ b/xen/arch/riscv/include/asm/aia.h
@@ -12,4 +12,6 @@ void aia_init(void);
 unsigned int vgein_assign(struct vcpu *v);
 void vgein_release(struct vcpu *v, unsigned int vgein_id, unsigned int cpu);
 
+void hgei_interrupt(void);
+
 #endif /* RISCV_AIA_H */
diff --git a/xen/arch/riscv/include/asm/domain.h 
b/xen/arch/riscv/include/asm/domain.h
index c33d83836f2e..a3fa637b40da 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -69,6 +69,7 @@ struct arch_vcpu {
     register_t hstateen0;
     uint64_t   htimedelta;
     register_t hvip;
+    register_t hie;
 
     register_t vsatp;
     register_t vscause;
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index dc938bae5b00..cf18f5185ff4 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -12,9 +12,10 @@
 #include <xen/sched.h>
 #include <xen/softirq.h>
 
-#include <asm/extable.h>
+#include <asm/aia.h>
 #include <asm/cpufeature.h>
 #include <asm/emulate.h>
+#include <asm/extable.h>
 #include <asm/intc.h>
 #include <asm/processor.h>
 #include <asm/riscv_encoding.h>
@@ -273,6 +274,10 @@ void do_trap(struct cpu_user_regs *cpu_regs)
                 timer_interrupt();
                 break;
 
+            case IRQ_S_GEXT:
+                hgei_interrupt();
+                break;
+
             default:
                 intr_handled = false;
                 break;
-- 
2.55.0