| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Aug 2019 17:11:23 +0100
From: Julien <julien.thierry.kdev@...il.com>
To: Marc Zyngier <maz@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Jason Cooper <jason@...edaemon.net>,
Rob Herring <robh+dt@...nel.org>
Cc: John Garry <john.garry@...wei.com>,
Shameerali Kolothum Thodi
<shameerali.kolothum.thodi@...wei.com>,
Lokesh Vutla <lokeshvutla@...com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 05/12] irqchip/gic: Prepare for more than 16 PPIs
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@...nel.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
Powered by blists - more mailing lists