On 24/09/2019 11:49, Andrew Murray wrote: > On Mon, Sep 23, 2019 at 07:25:36PM +0100, Marc Zyngier wrote: >> While GICv4.0 mandates 16 bit worth of VPEIDs, GICv4.1 allows smaller > > s/VPEIDs/vPEIDs/ > >> implementations to be built. Add the required glue to dynamically >> compute the limit. >> >> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> >> --- >> drivers/irqchip/irq-gic-v3-its.c | 11 ++++++++++- >> drivers/irqchip/irq-gic-v3.c | 3 +++ >> include/linux/irqchip/arm-gic-v3.h | 5 +++++ >> 3 files changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index c94eb287393b..17b77a0b9d97 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -119,7 +119,16 @@ struct its_node { >> #define ITS_ITT_ALIGN SZ_256 >> >> /* The maximum number of VPEID bits supported by VLPI commands */ >> -#define ITS_MAX_VPEID_BITS (16) >> +#define ITS_MAX_VPEID_BITS \ >> + ({ \ >> + int nvpeid = 16; \ >> + if (gic_rdists->has_rvpeid && \ > > We use rvpeid as a way of determining if this is a GICv4.1, are there any > other means of determining this? If we use it in this way, is there any > benefit to having a has_gicv4_1 type of flag instead? RVPEID *is* the way to discover a GICv4.1. To be clear, if we adopted the ARM ARM nomenclature to describe extensions, GICv4.1 would be called GIC-RVPEID, and that'd be it. > Also for 'insane' configurations we set has_rvpeid to false, thus preventing > this feature. Does it make sense to do that? It makes perfect sense. RVPEID *and* VLPI are set to false, and we don't do *any* direct injection, because it simply cannot work. > GICD_TYPER2 is reserved in GICv4, however I understand this reads as RES0, > can we just rely on that instead? (We read it below anyway). Yes. In general for the GIC, any RESERVED register is RAZ/WI. > >> + gic_rdists->gicd_typer2 & GICD_TYPER2_VIL) \ >> + nvpeid = 1 + (gic_rdists->gicd_typer2 & \ >> + GICD_TYPER2_VID); \ >> + \ >> + nvpeid; \ >> + }) >> #define ITS_MAX_VPEID (1 << (ITS_MAX_VPEID_BITS)) >> >> /* Convert page order to size in bytes */ >> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >> index 0b545e2c3498..fb6360161d6c 100644 >> --- a/drivers/irqchip/irq-gic-v3.c >> +++ b/drivers/irqchip/irq-gic-v3.c >> @@ -1556,6 +1556,9 @@ static int __init gic_init_bases(void __iomem *dist_base, >> >> pr_info("%d SPIs implemented\n", GIC_LINE_NR - 32); >> pr_info("%d Extended SPIs implemented\n", GIC_ESPI_NR); >> + >> + gic_data.rdists.gicd_typer2 = readl_relaxed(gic_data.dist_base + GICD_TYPER2); >> + >> gic_data.domain = irq_domain_create_tree(handle, &gic_irq_domain_ops, >> &gic_data); >> irq_domain_update_bus_token(gic_data.domain, DOMAIN_BUS_WIRED); >> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h >> index b34e0c113697..71730b9def0c 100644 >> --- a/include/linux/irqchip/arm-gic-v3.h >> +++ b/include/linux/irqchip/arm-gic-v3.h >> @@ -13,6 +13,7 @@ >> #define GICD_CTLR 0x0000 >> #define GICD_TYPER 0x0004 >> #define GICD_IIDR 0x0008 >> +#define GICD_TYPER2 0x000C >> #define GICD_STATUSR 0x0010 >> #define GICD_SETSPI_NSR 0x0040 >> #define GICD_CLRSPI_NSR 0x0048 >> @@ -89,6 +90,9 @@ >> #define GICD_TYPER_ESPIS(typer) \ >> (((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0) >> >> +#define GICD_TYPER2_VIL (1U << 7) >> +#define GICD_TYPER2_VID GENMASK(4, 0) > > Given that the 4th bit is reserved for future expansion and values greater > than 0xF are reserved, is there value in changing this to GENMASK(3, 0)? No, I'd rather leave the field to match the specification, and discard values that go beyond 16 in the ITS_MAX_VPEID_BITS macro. Thanks, M. -- Jazz is not dead, it just smells funny... _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm