[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