Messages in this thread | | | Subject | Re: [PATCH v2 05/12] irqchip/gic: Prepare for more than 16 PPIs | From | Julien <> | Date | Thu, 22 Aug 2019 17:11:23 +0100 |
| |
Hi Marc,
On 06/08/19 11:01, Marc Zyngier wrote: > GICv3.1 allows up to 80 PPIs (16 legaci PPIs and 64 Extended PPIs), > meaning we can't just leave the old 16 hardcoded everywhere. > > We also need to add the infrastructure to discover the number of PPIs > on a per redistributor basis, although we still pretend there is only > 16 of them for now. > > No functional change. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > drivers/irqchip/irq-gic-common.c | 19 ++++++++++++------- > drivers/irqchip/irq-gic-common.h | 2 +- > drivers/irqchip/irq-gic-v3.c | 22 +++++++++++++++------- > drivers/irqchip/irq-gic.c | 2 +- > drivers/irqchip/irq-hip04.c | 2 +- > 5 files changed, 30 insertions(+), 17 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c > index 6900b6f0921c..14110db01c05 100644 > --- a/drivers/irqchip/irq-gic-common.c > +++ b/drivers/irqchip/irq-gic-common.c > @@ -128,26 +128,31 @@ void gic_dist_config(void __iomem *base, int gic_irqs, > sync_access(); > } > > -void gic_cpu_config(void __iomem *base, void (*sync_access)(void)) > +void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void)) > { > int i; > > /* > * Deal with the banked PPI and SGI interrupts - disable all > - * PPI interrupts, ensure all SGI interrupts are enabled. > - * Make sure everything is deactivated. > + * private interrupts. Make sure everything is deactivated. > */ > - writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR); > - writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR); > - writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET); > + for (i = 0; i < nr; i += 32) {
You added "nr" as argument but if "nr" isn't a multiple of 32 weird things might happen, no?
It would be worth specifying that somewhere, and checking it with a WARN().
Maybe it might be worth reducing the granularity to manipulating 16 irqs since there are 16 SGI + 16 PPI + 64 EPPI, but that might not be very useful right now.
Cheers,
Julien
> + writel_relaxed(GICD_INT_EN_CLR_X32, > + base + GIC_DIST_ACTIVE_CLEAR + i / 8); > + writel_relaxed(GICD_INT_EN_CLR_X32, > + base + GIC_DIST_ENABLE_CLEAR + i / 8); > + } > > /* > * Set priority on PPI and SGI interrupts > */ > - for (i = 0; i < 32; i += 4) > + for (i = 0; i < nr; i += 4) > writel_relaxed(GICD_INT_DEF_PRI_X4, > base + GIC_DIST_PRI + i * 4 / 4); > > + /* Ensure all SGI interrupts are now enabled */ > + writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET); > + > if (sync_access) > sync_access(); > } > diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h > index 5a46b6b57750..ccba8b0fe0f5 100644 > --- a/drivers/irqchip/irq-gic-common.h > +++ b/drivers/irqchip/irq-gic-common.h > @@ -22,7 +22,7 @@ int gic_configure_irq(unsigned int irq, unsigned int type, > void __iomem *base, void (*sync_access)(void)); > void gic_dist_config(void __iomem *base, int gic_irqs, > void (*sync_access)(void)); > -void gic_cpu_config(void __iomem *base, void (*sync_access)(void)); > +void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void)); > void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, > void *data); > void gic_enable_of_quirks(const struct device_node *np, > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 1ca4dde32034..e03fb6d7c2ce 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -51,6 +51,7 @@ struct gic_chip_data { > u32 nr_redist_regions; > u64 flags; > bool has_rss; > + unsigned int ppi_nr; > struct partition_desc *ppi_descs[16]; > }; > > @@ -812,19 +813,24 @@ static int gic_populate_rdist(void) > return -ENODEV; > } > > -static int __gic_update_vlpi_properties(struct redist_region *region, > - void __iomem *ptr) > +static int __gic_update_rdist_properties(struct redist_region *region, > + void __iomem *ptr) > { > u64 typer = gic_read_typer(ptr + GICR_TYPER); > gic_data.rdists.has_vlpis &= !!(typer & GICR_TYPER_VLPIS); > gic_data.rdists.has_direct_lpi &= !!(typer & GICR_TYPER_DirectLPIS); > + gic_data.ppi_nr = 16; > > return 1; > } > > -static void gic_update_vlpi_properties(void) > +static void gic_update_rdist_properties(void) > { > - gic_iterate_rdists(__gic_update_vlpi_properties); > + gic_data.ppi_nr = UINT_MAX; > + gic_iterate_rdists(__gic_update_rdist_properties); > + if (WARN_ON(gic_data.ppi_nr == UINT_MAX)) > + gic_data.ppi_nr = 0; > + pr_info("%d PPIs implemented\n", gic_data.ppi_nr); > pr_info("%sVLPI support, %sdirect LPI support\n", > !gic_data.rdists.has_vlpis ? "no " : "", > !gic_data.rdists.has_direct_lpi ? "no " : ""); > @@ -968,6 +974,7 @@ static int gic_dist_supports_lpis(void) > static void gic_cpu_init(void) > { > void __iomem *rbase; > + int i; > > /* Register ourselves with the rest of the world */ > if (gic_populate_rdist()) > @@ -978,9 +985,10 @@ static void gic_cpu_init(void) > rbase = gic_data_rdist_sgi_base(); > > /* Configure SGIs/PPIs as non-secure Group-1 */ > - writel_relaxed(~0, rbase + GICR_IGROUPR0); > + for (i = 0; i < gic_data.ppi_nr + 16; i += 32) > + writel_relaxed(~0, rbase + GICR_IGROUPR0 + i / 8); > > - gic_cpu_config(rbase, gic_redist_wait_for_rwp); > + gic_cpu_config(rbase, gic_data.ppi_nr + 16, gic_redist_wait_for_rwp); > > /* initialise system registers */ > gic_cpu_sys_reg_init(); > @@ -1449,7 +1457,7 @@ static int __init gic_init_bases(void __iomem *dist_base, > > set_handle_irq(gic_handle_irq); > > - gic_update_vlpi_properties(); > + gic_update_rdist_properties(); > > gic_smp_init(); > gic_dist_init(); > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index ab48760acabb..25c1ae69db30 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -543,7 +543,7 @@ static int gic_cpu_init(struct gic_chip_data *gic) > gic_cpu_map[i] &= ~cpu_mask; > } > > - gic_cpu_config(dist_base, NULL); > + gic_cpu_config(dist_base, 32, NULL); > > writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK); > gic_cpu_if_up(gic); > diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c > index 1626131834a6..130caa1c9d93 100644 > --- a/drivers/irqchip/irq-hip04.c > +++ b/drivers/irqchip/irq-hip04.c > @@ -273,7 +273,7 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc) > if (i != cpu) > hip04_cpu_map[i] &= ~cpu_mask; > > - gic_cpu_config(dist_base, NULL); > + gic_cpu_config(dist_base, 32, NULL); > > writel_relaxed(0xf0, base + GIC_CPU_PRIMASK); > writel_relaxed(1, base + GIC_CPU_CTRL); >
-- Julien Thierry
| |