[PATCH v4 37/42] xen/riscv: sync with IMSIC VS-file injections on vCPU migration

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

vcpu_imsic_inject() writes an interrupt identity to the SETIPNUM_LE
register of a vCPU's h/w guest interrupt file under the read side of
vsfile_lock, and makes the write visible before releasing it. A
migration publishes the new file under the write side and drains the
old file only afterwards, so an injection either starts after the
publish and goes to the new file, or writes to the old file before the
publish. In the latter case, the write has to have taken effect in the
old file by the time the file is drained, but nothing ensures that:
even a completed write to an interrupt file may take effect in it only
later, as the AIA spec allows that to be delayed arbitrarily. So the
pending bit may be set only after imsic_vsfile_local_drain() has saved
and cleared the old file, which loses the interrupt, or even after the
file has been handed out to another vCPU.

What the AIA spec does guarantee is that the effects of the writes to
a single interrupt file occur in the global memory order of the writes.
So reserve the last identity of every guest interrupt file, which the
guest isn't given any more, and before draining the old file, write
that identity to it from the pCPU owning it and wait for it to become
pending. The injections which came before the publish precede that
write in the global memory order, so by then they have taken effect in
the file too.

As riscv,num-guest-ids is now used for the reserved identity, a missing
property is taken to mean as many identities as riscv,num-ids, which is
what the DT binding says, rather than GUEST_IMSIC_MAX_MSIS of them.

The AIA spec requires the number of identities of an interrupt file to
be one less than a multiple of 64, and Linux refuses an IMSIC which
doesn't follow that. So the guest is told about the largest such number
of identities below the reserved one, e.g. 191 of the 255 identities
QEMU provides, rather than about all the others.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Before making a final decision on this patch, please review:
https://github.com/riscv/riscv-aia/issues/172

At the time this patch was submitted, there had been no response from the
AIA specification maintainers. Therefore, for now, I consider the approach
proposed in this patch to be a viable solution.

The way this is implemented will prevent the V100 platform from booting.
Let's discuss it first.

If this synchronization is actually needed, then let's go back to making V100
tolerate MSI reservation failures for the VS-file instead of panicking.
---
Changes in v4:
 - New patch.
---
---
 xen/arch/riscv/imsic.c             | 106 +++++++++++++++++++++++++----
 xen/arch/riscv/include/asm/imsic.h |   7 ++
 2 files changed, 101 insertions(+), 12 deletions(-)

diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
index e2b3bfbd6619..e72a710b2d28 100644
--- a/xen/arch/riscv/imsic.c
+++ b/xen/arch/riscv/imsic.c
@@ -129,6 +129,18 @@ do {                                \
     csr_write(CSR_VSIREG, v);       \
 } while (0)
 
+#define imsic_vs_csr_read(c)        \
+({                                  \
+    csr_write(CSR_VSISELECT, c);    \
+    csr_read(CSR_VSIREG);           \
+})
+
+#define imsic_vs_csr_clear(c, v)    \
+do {                                \
+    csr_write(CSR_VSISELECT, c);    \
+    csr_clear(CSR_VSIREG, v);       \
+} while (0)
+
 /*
  * On RV64 a 64-bit EIx group is the even-numbered register alone, whereas on
  * RV32 it is split into the even-numbered (low half) and the following
@@ -430,11 +442,6 @@ static int __init imsic_parse_node(const struct 
dt_device_node *node,
         return -ENOENT;
     }
 
-    if ( dt_property_read_u32(node, "riscv,num-guest-ids", &tmp) )
-        guest_num_msis = min(GUEST_IMSIC_MAX_MSIS, tmp);
-    else
-        guest_num_msis = GUEST_IMSIC_MAX_MSIS;
-
     if ( (imsic_cfg.nr_ids < IMSIC_MIN_ID) ||
          (imsic_cfg.nr_ids > IMSIC_MAX_ID) )
     {
@@ -443,6 +450,39 @@ static int __init imsic_parse_node(const struct 
dt_device_node *node,
         return -ENOENT;
     }
 
+    /*
+     * A guest interrupt file implements as many identities as
+     * riscv,num-guest-ids says, or as many as the S-file does if it is
+     * absent.
+     */
+    if ( !dt_property_read_u32(node, "riscv,num-guest-ids", &tmp) )
+        tmp = imsic_cfg.nr_ids;
+
+    /*
+     * One identity more than the minimum is needed, as the last one is
+     * reserved below.
+     */
+    if ( (tmp <= IMSIC_MIN_ID) || (tmp > IMSIC_MAX_ID) )
+    {
+        printk(XENLOG_ERR
+               "%s: invalid number of guest interrupt identities\n",
+               node->name);
+        return -ENOENT;
+    }
+
+    /*
+     * Reserve the last identity of a guest interrupt file for synchronizing
+     * with the injections into the file (see imsic_vsfile_local_sync()), and
+     * give the guest at most GUEST_IMSIC_MAX_MSIS of the others. The AIA
+     * spec requires the number of identities to be one less than a multiple
+     * of 64, so round it down to the largest such number below
+     * guest_sync_id.
+     */
+    BUILD_BUG_ON((GUEST_IMSIC_MAX_MSIS & IMSIC_MIN_ID) != IMSIC_MIN_ID);
+    imsic_cfg.guest_sync_id = tmp;
+    guest_num_msis = min(GUEST_IMSIC_MAX_MSIS,
+                         ROUNDDOWN(tmp, IMSIC_MIN_ID + 1) - 1);
+
     imsic_cfg.nr_eix = DIV_ROUND_UP(imsic_cfg.nr_ids + 1, 64);
 
     /* Compute base address */
@@ -701,18 +741,58 @@ static void imsic_call_on_cpu(unsigned int cpu, void 
(*func)(void *),
 }
 
 /*
- * Ensure that all the MSIs the APLIC has already generated for the hart this
- * runs on have really reached the hart's IMSIC.
+ * Wait until the writes made to guest interrupt file @hgei of this hart 
through
+ * SETIPNUM_LE before this call have taken effect in it.
+ */
+static void imsic_vsfile_local_sync(unsigned int hgei)
+{
+    unsigned int id = imsic_cfg.guest_sync_id;
+    unsigned int isel =
+        (id / BITS_PER_LONG) * (BITS_PER_LONG / IMSIC_EIPx_BITS) + IMSIC_EIP0;
+    unsigned long bit = BIT(id % BITS_PER_LONG, UL);
+    void __iomem *va = imsic_cfg.msi[smp_processor_id()].files_va +
+                       hgei * IMSIC_MMIO_PAGE_SZ;
+    struct imsic_vsfile_ctx ctx;
+
+    imsic_vsfile_select(hgei, &ctx);
+
+    imsic_vs_csr_clear(isel, bit);
+
+    /*
+     * For FENCE, a CSR write counts as device output, which writel() doesn't
+     * order before its own write: should the IMSIC see the two the other way
+     * round, the clear would undo the write and the loop would never end.
+     */
+    RISCV_FENCE(o, o);
+    writel(id, va + IMSIC_MMIO_SETIPNUM_LE);
+
+    while ( !(imsic_vs_csr_read(isel) & bit) )
+        cpu_relax();
+
+    /* The identity is Xen's, so it mustn't be left pending in the file. */
+    imsic_vs_csr_clear(isel, bit);
+
+    imsic_vsfile_deselect(&ctx);
+}
+
+/*
+ * Ensure that all the MSIs already sent to the guest interrupt file @data 
names
+ * on the hart this runs on, by the h/w APLIC or by vAPLIC, have reached it.
  *
- * The barrier is the one described by the AIA specification in
- * "Synchronizing interactions between a hart and the APLIC": ask the APLIC to
- * send an MSI to the hart itself and wait until it shows up as pending in the
- * hart's own interrupt file. As it says nothing about MSIs on their way to
+ * For the h/w APLIC, the barrier is the one described by the AIA specification
+ * in "Synchronizing interactions between a hart and the APLIC": ask the APLIC
+ * to send an MSI to the hart itself and wait until it shows up as pending in
+ * the hart's own interrupt file. As it says nothing about MSIs on their way to
  * any other hart, it has to be executed by the pCPU owning the interrupt file
  * the MSIs were being sent to.
+ *
+ * vAPLIC sends its MSIs through SETIPNUM_LE, which that barrier doesn't cover,
+ * so they are waited for separately.
  */
-static void cf_check imsic_aplic_sync(void *unused)
+static void cf_check imsic_aplic_sync(void *data)
 {
+    const struct imsic_vsfile_data *idata = data;
+
     imsic_local_eix_update(imsic_sync_id(&imsic_cfg), 1, true, false);
 
     /*
@@ -727,6 +807,8 @@ static void cf_check imsic_aplic_sync(void *unused)
 
     while ( !imsic_local_is_pending(imsic_sync_id(&imsic_cfg)) )
         cpu_relax();
+
+    imsic_vsfile_local_sync(idata->hgei);
 }
 
 /*
diff --git a/xen/arch/riscv/include/asm/imsic.h 
b/xen/arch/riscv/include/asm/imsic.h
index 8245ec6eb695..f372761b8853 100644
--- a/xen/arch/riscv/include/asm/imsic.h
+++ b/xen/arch/riscv/include/asm/imsic.h
@@ -66,6 +66,13 @@ struct imsic_config {
      */
     unsigned int nr_eix;
 
+    /*
+     * Interrupt identity reserved in every h/w guest interrupt file for
+     * synchronizing with the injections into the file. It is never given to a
+     * guest.
+     */
+    unsigned int guest_sync_id;
+
     /* MSI */
     const struct imsic_msi *msi;
 
-- 
2.56.1