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

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



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 <oleksii.kurochko@xxxxxxxxx>
---
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




 


Rackspace

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