[PATCH v2 10/39] xen/riscv: build the target hart index via aplic_hart_field()

Oleksii Kurochko <[email protected]>
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <3c7418d64a730ec5e4b2cfd0ecfbbbc6fe0bf7d8.1787838835.git.oleksii.kurochko@gmail.com>
aplic_set_irq_affinity() open-coded the packing of the group and hart
indices into the target register, and got two things wrong along the
way:

 - imsic_config.msi[] is indexed by logical CPU id, but the index was
   run through cpuid_to_hartid() first. On any platform where the two
   spaces differ this picks another CPU's interrupt file, or reads past
   the array;

 - the same hart id was then used verbatim as the low hart index, and
   the group index was derived from msi[].base_addr alone. The hart
   index bits live in msi[].offset, the base address only covers the
   MMIO regset, which may hold the files of several harts. Both indices
   have to come out of base_addr + offset.

aplic_hart_field() already extracts them that way, and is what the vAPLIC
target path uses, so call it here as well and insert the result with
MASK_INSR(APLIC_TARGET_HART_IDX) instead of a bare shift, which keeps the
value from spilling out of the 14-bit field. This also drops the last
in-tree duplicate of the AIA hart index formula. So drop defintion of
APLIC_TARGET_HART_IDX_SHIFT.

No functional change on a single-group platform whose hart ids match
their CPU ids and whose IMSIC regset holds one file per hart.

Fixes: d4676a1398bc ("xen/riscv: implementation of aplic and imsic operations")
Signed-off-by: Oleksii Kurochko <[email protected]>
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/aplic.c             | 30 ++++++------------------------
 xen/arch/riscv/include/asm/aplic.h |  1 -
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index 66ba4986a9ff..319a954f6f3c 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -325,9 +325,7 @@ static unsigned int aplic_get_cpu_from_mask(const cpumask_t *cpumask)
 static void cf_check aplic_set_irq_affinity(struct irq_desc *desc, const cpumask_t *mask)
 {
     unsigned int cpu;
-    uint64_t group_index, base_ppn;
-    uint32_t hhxw, lhxw, hhxs, value;
-    const struct imsic_config *imsic = aplic.imsic_cfg;
+    uint32_t value;
 
     /*
      * TODO: Currently, APLIC is supported only with MSI interrupts.
@@ -340,27 +338,11 @@ static void cf_check aplic_set_irq_affinity(struct irq_desc *desc, const cpumask
 
     ASSERT(spin_is_locked(&desc->lock));
 
-    cpu = cpuid_to_hartid(aplic_get_cpu_from_mask(mask));
-    hhxw = imsic->group_index_bits;
-    lhxw = imsic->hart_index_bits;
-    /*
-     * Although this variable is used only once in the calculation of
-     * group_index, and it might seem that hhxs could be defined as:
-     *   hhxs = imsic->group_index_shift - IMSIC_MMIO_PAGE_SHIFT;
-     * and then the addition of IMSIC_MMIO_PAGE_SHIFT could be omitted
-     * when calculating the group index.
-     * It was done intentionally this way to follow the formula from
-     * the AIA specification for calculating the MSI address.
-     */
-    hhxs = imsic->group_index_shift - IMSIC_MMIO_PAGE_SHIFT * 2;
-    base_ppn = imsic->msi[cpu].base_addr >> IMSIC_MMIO_PAGE_SHIFT;
-
-    /* Update hart and EEID in the target register */
-    group_index = (base_ppn >> (hhxs + IMSIC_MMIO_PAGE_SHIFT)) &
-                  (BIT(hhxw, UL) - 1);
-    value = desc->irq;
-    value |= cpu << APLIC_TARGET_HART_IDX_SHIFT;
-    value |= group_index << (lhxw + APLIC_TARGET_HART_IDX_SHIFT);
+    cpu = aplic_get_cpu_from_mask(mask);
+
+    /* Update hart index and EIID in the target register */
+    value = MASK_INSR(aplic_hart_field(cpu), APLIC_TARGET_HART_IDX) |
+            (desc->irq & APLIC_TARGET_EIID);
 
     spin_lock(&aplic.lock);
 
diff --git a/xen/arch/riscv/include/asm/aplic.h b/xen/arch/riscv/include/asm/aplic.h
index babba386071f..d629e1c83887 100644
--- a/xen/arch/riscv/include/asm/aplic.h
+++ b/xen/arch/riscv/include/asm/aplic.h
@@ -92,7 +92,6 @@
 #define APLIC_TARGET_BASE               0x3004
 #define APLIC_TARGET_LAST               0x3ffc
 #define  APLIC_TARGET_HART_IDX          GENMASK(31, 18)
-#define  APLIC_TARGET_HART_IDX_SHIFT    18
 #define  APLIC_TARGET_GUEST_IDX         GENMASK(17, 12)
 /* Bit 11 is reserved and reads as zero */
 #define  APLIC_TARGET_EIID              GENMASK(10, 0)
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.