[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 31/39] xen/riscv: remap interrupts to new IMSIC VS-file



Once the new IMSIC VS-file has been zeroed out, it has to be mapped into
the guest's G-stage in place of the old one. Also, if any interrupts at an
APLIC are forwarded by MSIs to the old interrupt file, reconfigure the
APLIC to send them to the new interrupt file.

Taking a h/w guest interrupt file (assigning it, zeroing it out, mapping
it into G-stage and recording it in the vCPU's IMSIC state) is factored
out into imsic_vsfile_acquire().

The relevant translation tables at all IOMMUs would also have to be
updated so that MSIs for this virtual interrupt file are sent to the new
physical interrupt file, but that is skipped for now as there is no IOMMU
support for RISC-V.

Synchronize with the APLIC to ensure that no straggler MSIs will arrive at
the old interrupt file by running imsic_aplic_sync() on the pCPU which owns
that file.

If the new file can't be mapped into the guest, release the guest
external interrupt number imsic_vsfile_acquire() took for it instead of
leaking it. vgein_release() takes the pCPU explicitly instead of relying
on v->processor, as on migration v->processor already refers to the new
pCPU while the number to be released may belong to the old one.

Keep BUG_ON("unimplemented") placeholder in imsic_migrate_vcpu() to guard
against silent incorrect behaviour or unexpected panics in guest VMs until
the function is fully implemented.

Implement imsic_update_state(), which records the vCPU's guest
interrupt file id together with the pCPU owning the file. The two are
read as a pair, e.g. by imsic_migrate_vcpu() and the IMSIC context
switch hooks, so they are updated under vsfile_lock as a single unit.

imsic_map_guest_file() and vgein_release() are stubs for now.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
imsic_vsfile_acquire() isn't introduced already by "xen/riscv: prepare new
IMSIC VS-file": there, taking the new file is just vgein_assign() and zeroing
the file out. What makes it worth a helper of its own, mapping the file into
the guest, recording it in the vCPU's IMSIC state and releasing it again with
vgein_release() if the mapping fails, only comes with this patch. The helper
is also shared with the first attach of a h/w file in "xen/riscv: introduce
IMSIC h/w interrupt file attaching to vcpu".
---
Changes in v3:
 - Update the commit message.
 - Move imsic_aplic_sync() and imsic_local_is_pending() to "xen/riscv:
   implement APLIC-IMSIC-hart sync barrier for vCPU migration" and only
   call the helper here.
 - compare old_vsfile_cpu against CPU_NONE instead of NR_CPUS.
 - Call vaplic_reconfigure_target() instead of aplic_reconfigure_target().
 - Update the comment at the start of the migration: at that point the new
   IMSIC VS-file is only being prepared, the interrupt producers are moved
   to it later.
 - Factor taking a h/w guest interrupt file (assign, zero out, map into
   G-stage and record in the per-vCPU IMSIC state) out of
   imsic_migrate_vcpu() into imsic_vsfile_acquire(), introduced here
   instead of in "xen/riscv: introduce IMSIC h/w interrupt file attaching
   to vcpu".
 - Crash the domain instead of BUG_ON()ing when no h/w guest interrupt
   file is free, and report which file could not be mapped.
 - Introduce the vgein_release() stub here, together with its first
   user: release the h/w guest interrupt file taken by
   imsic_vsfile_acquire() if mapping it into the guest fails. Drop the
   stray "\n" from its BUG_ON().
 - Clarify the comment ahead of imsic_vsfile_acquire() on why setting
   hstatus.VGEIN is left to the caller.
 - Implement imsic_update_state() here instead of leaving it a stub until
   "xen/riscv: introduce IMSIC h/w interrupt file attaching to vcpu": all
   it needs, vsfile_lock and CPU_NONE, is available already. The s/w
   IMSIC VS-file is recorded with vsfile_cpu == CPU_NONE rather than
   NR_CPUS.
 - Make imsic_update_state() static and drop its declaration from
   asm/imsic.h: imsic_vsfile_acquire() is its only user.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/aia.c               |   5 ++
 xen/arch/riscv/imsic.c             | 130 +++++++++++++++++++++++++++--
 xen/arch/riscv/include/asm/aia.h   |   1 +
 xen/arch/riscv/include/asm/imsic.h |   2 +
 4 files changed, 130 insertions(+), 8 deletions(-)

diff --git a/xen/arch/riscv/aia.c b/xen/arch/riscv/aia.c
index 229ab4b8678e..bd35078b7354 100644
--- a/xen/arch/riscv/aia.c
+++ b/xen/arch/riscv/aia.c
@@ -30,3 +30,8 @@ unsigned int vgein_assign(struct vcpu *v)
 
     return 0;
 }
+
+void vgein_release(struct vcpu *v, unsigned int vgein_id, unsigned int cpu)
+{
+    BUG_ON("unimplemented");
+}
diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
index 8e0273eab902..47854264edf3 100644
--- a/xen/arch/riscv/imsic.c
+++ b/xen/arch/riscv/imsic.c
@@ -29,6 +29,7 @@
 #include <asm/aia.h>
 #include <asm/aplic.h>
 #include <asm/imsic.h>
+#include <asm/vaplic.h>
 
 #define IMSIC_HART_SIZE(guest_bits) (BIT(guest_bits, U) * IMSIC_MMIO_PAGE_SZ)
 
@@ -156,6 +157,18 @@ unsigned int vcpu_guest_file_id(const struct vcpu *v)
     return ACCESS_ONCE(v->arch.vimsic_state->guest_file_id);
 }
 
+static void imsic_update_state(struct vcpu *v, unsigned int guest_file_id)
+{
+    unsigned long flags;
+    struct vimsic_state *vimsic_state = v->arch.vimsic_state;
+    unsigned int cpu = guest_file_id ? v->processor : CPU_NONE;
+
+    write_lock_irqsave(&vimsic_state->vsfile_lock, flags);
+    vimsic_state->guest_file_id = guest_file_id;
+    vimsic_state->vsfile_cpu = cpu;
+    write_unlock_irqrestore(&vimsic_state->vsfile_lock, flags);
+}
+
 void __init imsic_ids_local_delivery(bool enable)
 {
     if ( enable )
@@ -462,6 +475,11 @@ void cf_check imsic_ctxt_switch_to(struct vcpu *n)
     /* Nothing to do */
 }
 
+int imsic_map_guest_file(struct vcpu *v, unsigned int vsfile_id)
+{
+    return -EOPNOTSUPP;
+}
+
 int cf_check vcpu_imsic_init(struct vcpu *v)
 {
     struct vimsic_state *imsic_state;
@@ -882,11 +900,71 @@ int __init vimsic_make_domu_dt_node(struct kernel_info 
*kinfo,
     return fdt_end_node(fdt);
 }
 
+/*
+ * Take a h/w guest interrupt file on the pCPU @v runs on and make it the
+ * vCPU's one.
+ *
+ * hstatus.VGEIN is deliberately left alone: it must only point at a file
+ * which already holds the vCPU's interrupt state. The file taken here is
+ * zeroed, so for imsic_migrate_vcpu() hstatus.VGEIN can only be updated once
+ * the state of the old file has been copied into it. Hence it is up to the
+ * caller to set hstatus.VGEIN at the right moment.
+ *
+ * Returns the id of the file, or 0, with the domain crashed, if none could be
+ * taken.
+ */
+static unsigned int imsic_vsfile_acquire(struct vcpu *v)
+{
+    unsigned int cpu = v->processor;
+    struct imsic_vsfile_data vsfile_data = { .nr_eix = imsic_cfg.nr_eix };
+    unsigned int vsfile_id;
+    int rc;
+
+    vsfile_id = vgein_assign(v);
+    if ( !vsfile_id )
+    {
+        /*
+         * vgein_assign() returns 0 when no free h/w guest interrupt file is
+         * available. s/w guest interrupt files aren't supported yet, so such
+         * a vCPU can't be run.
+         */
+        domain_crash(v->domain,
+                     "%pv: no free h/w guest interrupt file on CPU%u\n",
+                     v, cpu);
+        return 0;
+    }
+
+    vsfile_data.hgei = vsfile_id;
+
+    /* The file could still hold the state of its previous owner */
+    imsic_call_on_cpu(cpu, imsic_vsfile_local_clear, &vsfile_data);
+
+    rc = imsic_map_guest_file(v, vsfile_id);
+    if ( rc )
+    {
+        vgein_release(v, vsfile_id, cpu);
+
+        /* Can't continue w/o correctly mapped IMSIC interrupt file */
+        domain_crash(v->domain,
+                     "%pv: failed to map h/w guest interrupt file %u: %d\n",
+                     v, vsfile_id, rc);
+        return 0;
+    }
+
+    imsic_update_state(v, vsfile_id);
+
+    return vsfile_id;
+}
+
 void imsic_migrate_vcpu(struct vcpu *v)
 {
     struct imsic_vsfile_data vsfile_data = {
         .nr_eix = imsic_cfg.nr_eix,
     };
+    struct vimsic_state *imsic_state = v->arch.vimsic_state;
+    unsigned long flags;
+    unsigned int old_vsfile_id;
+    unsigned int old_vsfile_cpu;
 
     /*
      * The scheduler can mark a freshly created vCPU's unit as migrated and
@@ -898,21 +976,57 @@ void imsic_migrate_vcpu(struct vcpu *v)
     if ( v->arch.last_cpu == CPU_NONE )
         return;
 
+    read_lock_irqsave(&imsic_state->vsfile_lock, flags);
+    old_vsfile_id = imsic_state->guest_file_id;
+    old_vsfile_cpu = imsic_state->vsfile_cpu;
+    read_unlock_irqrestore(&imsic_state->vsfile_lock, flags);
+
+    /*
+     * We don't support SW interrupt files at the moment. Bail out before
+     * anything is touched, as the old file has no owning pCPU in that case
+     * and there is nothing to retarget the producers away from.
+     */
+    if ( old_vsfile_cpu == CPU_NONE )
+        panic("IMSIC SW-file isn't supported\n");
+
     /*
      * At this point, all interrupt producers are still using the old IMSIC
-     * VS-file.
+     * VS-file. Allocate and clear the new one before redirecting anything
+     * to it.
      */
 
-    vsfile_data.hgei = vgein_assign(v);
+    /* Zero-out, map and start to use the new IMSIC VS-file */
+    vsfile_data.hgei = imsic_vsfile_acquire(v);
+    if ( !vsfile_data.hgei )
+        return;
 
-    /* We don't support SW interrupt files at the moment. */
-    BUG_ON(!vsfile_data.hgei);
+    /*
+     * TODO: Modify the relevant translation tables at all IOMMUs so that MSIs
+     *       for this virtual interrupt file are now sent to the new physical
+     *       interrupt file.
+     */
+    if ( iommu_enabled )
+        printk_once("IMSIC: IOMMU MSI retargeting is not implemented\n");
 
-    /* Zero-out new IMSIC VS-file */
-    imsic_call_on_cpu(v->processor, imsic_vsfile_local_clear, &vsfile_data);
+    /*
+     * If any interrupts at an APLIC are forwarded by MSIs to the old interrupt
+     * file, reconfigure the APLIC to send them to the new interrupt file.
+     */
+    vaplic_reconfigure_target(v);
 
-    /* Avoid an unused-function error until the barrier has a user. */
-    (void)imsic_aplic_sync;
+    /*
+     * Synchronizing interactions between a hart and the APLIC.
+     *
+     * The MSIs to be flushed are the ones still on their way to the old
+     * interrupt file, so the barrier has to be done by the pCPU which owns
+     * that file.
+     */
+    imsic_call_on_cpu(old_vsfile_cpu, imsic_aplic_sync, NULL);
+
+    /*
+     * At this point, all interrupt producers have been moved
+     * to the new IMSIC VS-file.
+     */
 
     BUG_ON("unimplemented");
 }
diff --git a/xen/arch/riscv/include/asm/aia.h b/xen/arch/riscv/include/asm/aia.h
index 53a1efb042f8..8e4eb2f6b14e 100644
--- a/xen/arch/riscv/include/asm/aia.h
+++ b/xen/arch/riscv/include/asm/aia.h
@@ -10,5 +10,6 @@ bool aia_usable(void);
 void aia_init(void);
 
 unsigned int vgein_assign(struct vcpu *v);
+void vgein_release(struct vcpu *v, unsigned int vgein_id, unsigned int cpu);
 
 #endif /* RISCV_AIA_H */
diff --git a/xen/arch/riscv/include/asm/imsic.h 
b/xen/arch/riscv/include/asm/imsic.h
index f86407e21765..e08b041eebbd 100644
--- a/xen/arch/riscv/include/asm/imsic.h
+++ b/xen/arch/riscv/include/asm/imsic.h
@@ -114,6 +114,8 @@ int vimsic_make_domu_dt_node(struct kernel_info *kinfo, 
unsigned int *phandle);
 void imsic_ctxt_switch_from(struct vcpu *p);
 void imsic_ctxt_switch_to(struct vcpu *n);
 
+int imsic_map_guest_file(struct vcpu *v, unsigned int vsfile_id);
+
 void imsic_migrate_vcpu(struct vcpu *v);
 
 #endif /* ASM_RISCV_IMSIC_H */
-- 
2.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.