Hi Leonid,
On 27/08/2025 19:24, Leonid Komarianskyi wrote:
Currently, many common functions perform the same operations to calculate
GIC register addresses. This patch consolidates the similar code into
a separate helper function to improve maintainability and reduce duplication.
This refactoring also simplifies the implementation of eSPI support in future
changes.
Signed-off-by: Leonid Komarianskyi <leonid_komarians...@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
---
Changes in V4:
- no changes
Changes in V3:
- changed panic() in get_addr_by_offset() to printing warning and
ASSERT_UNREACHABLE()
- added verification of return pointer from get_addr_by_offset() in the
callers
- moved invocation of get_addr_by_offset() from spinlock guards, since
it is not necessarry
- added RB from Volodymyr Babchuk
Procces remark, here you said the Reviewed-by from Volodymyr was added
in v3. However, given the changes you made this should have been
invalidated (reviewed-by means the person read the code and confirmed it
is correct).
I see Volodymyr confirmed his reviewed-by on v3. So no issue, but this
should have been clarified in the changelog.
Changes in V2:
- no changes
---
xen/arch/arm/gic-v3.c | 114 +++++++++++++++++++++++----------
xen/arch/arm/include/asm/irq.h | 1 +
2 files changed, 81 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index cd3e1acf79..a959fefebe 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -445,17 +445,67 @@ static void gicv3_dump_state(const struct vcpu *v)
}
}
+static void __iomem *get_addr_by_offset(struct irq_desc *irqd, u32 offset)
+{
+ switch ( irqd->irq )
+ {
+ case 0 ... (NR_GIC_LOCAL_IRQS - 1):
+ switch ( offset )
+ {
+ case GICD_ISENABLER:
+ case GICD_ICENABLER:
+ case GICD_ISPENDR:
+ case GICD_ICPENDR:
+ case GICD_ISACTIVER:
+ case GICD_ICACTIVER:
+ return (GICD_RDIST_SGI_BASE + offset);
+ case GICD_ICFGR:
+ return (GICD_RDIST_SGI_BASE + GICR_ICFGR1);
+ case GICD_IPRIORITYR:
+ return (GICD_RDIST_SGI_BASE + GICR_IPRIORITYR0 + irqd->irq);
+ default:
+ break;
+ }
+ case NR_GIC_LOCAL_IRQS ... SPI_MAX_INTID:
+ switch ( offset )
+ {
+ case GICD_ISENABLER:
+ case GICD_ICENABLER:
+ case GICD_ISPENDR:
+ case GICD_ICPENDR:
+ case GICD_ISACTIVER:
+ case GICD_ICACTIVER:
+ return (GICD + offset + (irqd->irq / 32) * 4);
+ case GICD_ICFGR:
+ return (GICD + GICD_ICFGR + (irqd->irq / 16) * 4);
+ case GICD_IROUTER:
+ return (GICD + GICD_IROUTER + irqd->irq * 8);
+ case GICD_IPRIORITYR:
+ return (GICD + GICD_IPRIORITYR + irqd->irq);
+ default:
+ break;
+ }
+ default:
+ break;
+ }
+
+ /* Something went wrong, we shouldn't be able to reach here */
+ printk(XENLOG_WARNING "GICv3: WARNING: Invalid offset 0x%x for IRQ#%d",
NIT: I am not expecting the interrupt to be < 0. So it would be
preferable to use %u.
Acked-by: Julien Grall <jgr...@amazon.com>
Cheers,
--
Julien Grall
