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

[PATCH v4 27/42] xen/riscv: introduce vaplic_reconfigure_target()



When a vCPU is migrated to a different pCPU, its IMSIC guest interrupt
file changes. Any APLIC interrupt previously configured to deliver an
MSI to the old interrupt file must be retargeted to the new one.

Implement vaplic_reconfigure_target() to walk the h/w interrupts routed
to the domain and rewrite the APLIC TARGET register of every one whose
guest-programmed target names the vCPU, building the new value with
aplic_msi_target_gen(). Each source is rewritten under its vaplic_source
lock, so that it can't race with a guest write of the same target
register. Nothing calls it yet.

This is correct only as part of step 3 of the sequence in the AIA spec's
"Migrating a virtual hart to a different guest interrupt file". Before
it is called, the new file must be cleared, with eidelivery set to 0,
and recorded in the vCPU's IMSIC state, which aplic_msi_target_gen()
takes the new target from. Once it returns, Xen must synchronize with
the APLIC.

That is also why an interrupt raised mid-retarget isn't lost. An MSI
sent before its source's TARGET register is rewritten is latched in the
old file's eip, the synchronization ensures it has arrived there, and
steps 4 and 5 merge that eip into the new file. One sent after the
rewrite lands in the new file and stays pending there until eidelivery
is restored (step 6).

Also assert in aplic_msi_target_gen() that the physical APLIC is in MSI
delivery mode, which all of its callers rely on.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
vaplic_reconfigure_target() gets its first user in "xen/riscv: introduce
IMSIC h/w interrupt file attaching to vcpu", and is then used by
"xen/riscv: remap interrupts to new IMSIC VS-file" too.
---
Changes in v4:
 - Walk hw_irqs rather than used_irqs: only a h/w source has a physical
   TARGET register to rewrite. The MSIs of a virtual source are injected
   by Xen into whichever interrupt file the vCPU has at that moment, so
   such a source needs no retargeting.
 - Explain in the commit message why an interrupt raised mid-retarget
   isn't lost.
 - Word the comment above aplic_msi_target_gen() as a requirement on
   whoever publishes a new interrupt file, as nothing calls
   vaplic_reconfigure_target() yet.
 - Move right before "xen/riscv: introduce IMSIC h/w interrupt file
   attaching to vcpu", its first user.
 - Don't call the zero value of target[] a reset value, as the AIA spec
   doesn't define one, and mention that skipping EIID 0 also keeps
   unprogrammed sources from being retargeted along with vCPU0.
---
Changes in v3:
 - Update the commit message.
 - Replace aplic_reconfigure_target() in aplic.c with
   vaplic_reconfigure_target() in vaplic.c: the sources to retarget are
   chosen from the guest's view of the target registers, decoded by the
   new vaplic_target_decode_vcpu(), instead of by matching the h/w
   registers against the old guest file id and pCPU, which drops the
   old_guest_file_id and old_cpu parameters. Each source is rewritten
   under its vaplic_source lock, so it can't race with a guest write of the
   same target register.
 - Hold vsfile_lock for reading while retargeting, as aplic_msi_target_gen()
   requires. Lock order is the vaplic_source lock, then vsfile_lock, then
   aplic.lock (taken by aplic_hw_write_reg()).
 - Replace the BUG_ON() on MSI delivery mode with an ASSERT() in
   aplic_msi_target_gen(), so that none of its callers can miss it; direct
   delivery mode isn't supported, so there's no way to get there in
   release builds either.
 - Update the comment above aplic_msi_target_gen() about why vsfile_lock
   has to be held.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/aplic.c              | 13 ++++++-
 xen/arch/riscv/include/asm/vaplic.h |  3 ++
 xen/arch/riscv/vaplic.c             | 60 +++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index 51d7eae4617a..825eb60bf444 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -129,7 +129,11 @@ static unsigned long aplic_hart_index(unsigned int cpu)
 /*
  * v->processor can't be used here: during a migration it names the new pCPU
  * before the interrupt file is moved there. The caller has to hold
- * vsfile_lock until the result is written to the h/w.
+ * vsfile_lock, so that the guest file id and the pCPU owning it are read as
+ * a consistent pair. A value may still be built from the old file just
+ * before a migration publishes the new one, so whoever publishes a new file
+ * has to call vaplic_reconfigure_target() afterwards, to rewrite every
+ * source targeting the vCPU.
  */
 uint32_t aplic_msi_target_gen(const struct vcpu *v, uint32_t base_val)
 {
@@ -140,6 +144,13 @@ uint32_t aplic_msi_target_gen(const struct vcpu *v, 
uint32_t base_val)
     ASSERT(rw_is_locked(&vimsic_state->vsfile_lock));
     ASSERT(vimsic_state->vsfile_cpu < NR_CPUS);
 
+    /*
+     * The callers rely on the physical APLIC being in MSI delivery mode.
+     * Checking that here rather than in each of them means none can be
+     * missed should direct delivery mode ever be supported.
+     */
+    ASSERT(aplic_msi_mode());
+
     hart_index = aplic_hart_index(vimsic_state->vsfile_cpu);
 
     base_val &= APLIC_TARGET_EIID;
diff --git a/xen/arch/riscv/include/asm/vaplic.h 
b/xen/arch/riscv/include/asm/vaplic.h
index 9a121904a346..843b78e80ad8 100644
--- a/xen/arch/riscv/include/asm/vaplic.h
+++ b/xen/arch/riscv/include/asm/vaplic.h
@@ -17,6 +17,7 @@
 #include <asm/intc.h>
 
 struct domain;
+struct vcpu;
 
 #define to_vaplic(d) container_of((d)->arch.vintc, struct vaplic, vintc)
 
@@ -50,4 +51,6 @@ struct vaplic {
 int domain_vaplic_init(struct domain *d);
 void domain_vaplic_deinit(struct domain *d);
 
+void vaplic_reconfigure_target(const struct vcpu *v);
+
 #endif /* RISCV_VAPLIC_H */
diff --git a/xen/arch/riscv/vaplic.c b/xen/arch/riscv/vaplic.c
index 7898455b4934..42489bf299b9 100644
--- a/xen/arch/riscv/vaplic.c
+++ b/xen/arch/riscv/vaplic.c
@@ -169,6 +169,66 @@ static void vaplic_target_write(const struct domain *d, 
unsigned int irqn,
     spin_unlock_irqrestore(&src->lock, flags);
 }
 
+/* Decode the target vCPU from a TARGET snapshot (NULL if none). */
+static struct vcpu *vaplic_target_decode_vcpu(const struct domain *d,
+                                              uint32_t target)
+{
+    unsigned int hart_idx = MASK_EXTR(target, APLIC_TARGET_HART_IDX);
+
+    /*
+     * Skip a source whose EIID is 0: interrupt identity 0 is never valid at
+     * an IMSIC, so such a source delivers nothing and needs no retargeting.
+     * The AIA spec leaves target[] UNSPECIFIED after reset, but the vAPLIC
+     * state is zero-initialized, so this also covers every source the guest
+     * hasn't programmed yet, which would otherwise decode as targeting vCPU0
+     * (hart index 0) and have its h/w TARGET register needlessly rewritten
+     * on every vCPU0 migration.
+     * The hart index range check is only a guard: the write path refuses to
+     * record values with an out-of-range hart index.
+     */
+    if ( !(target & APLIC_TARGET_EIID) || hart_idx >= d->max_vcpus )
+        return NULL;
+
+    return d->vcpu[hart_idx];
+}
+
+void vaplic_reconfigure_target(const struct vcpu *v)
+{
+    struct domain *d = v->domain;
+    struct vaplic *vaplic = to_vaplic(d);
+    struct vimsic_state *vimsic_state = v->arch.vimsic_state;
+    unsigned int irqn;
+
+    bitmap_for_each ( irqn, d->arch.vintc->hw_irqs, d->arch.vintc->nr_virqs )
+    {
+        struct vaplic_source *src = &vaplic->regs.sources[irqn];
+        unsigned long flags;
+        uint32_t target;
+
+        /* APLIC interrupt sources start from 1, there is no source 0. */
+        if ( !irqn )
+            continue;
+
+        spin_lock_irqsave(&src->lock, flags);
+
+        target = read_atomic(&src->target);
+
+        if ( vaplic_target_decode_vcpu(d, target) == v )
+        {
+            /*
+             * IRQs are already off under src->lock so no need for *_irqsave
+             * here.
+             */
+            read_lock(&vimsic_state->vsfile_lock);
+            aplic_hw_write_reg(offsetof(struct aplic_regs, target[irqn - 1]),
+                               aplic_msi_target_gen(v, target));
+            read_unlock(&vimsic_state->vsfile_lock);
+        }
+
+        spin_unlock_irqrestore(&src->lock, flags);
+    }
+}
+
 static uint32_t generate_auth_mask(const struct domain *currd,
                                    unsigned int word_idx)
 {
-- 
2.56.1




 


Rackspace

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