| 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
| ||
|
Message-ID: <56694272.5050706@arm.com>
Date: Thu, 10 Dec 2015 09:14:26 +0000
From: Marc Zyngier <marc.zyngier@....com>
To: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>,
tglx@...utronix.de, jason@...edaemon.net, rjw@...ysocki.net
CC: Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Will Deacon <will.deacon@....com>,
Catalin Marinas <Catalin.Marinas@....com>,
hanjun.guo@...aro.org, tomasz.nowicki@...aro.org,
graeme.gregory@...aro.org, dhdang@....com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org
Subject: Re: [PATCH v6 4/4] gicv2m: acpi: Introducing GICv2m ACPI support
On 09/12/15 19:24, Suravee Suthikulpanit wrote:
> This patch introduces gicv2m_acpi_init(), which uses information
> in MADT GIC MSI frames structure to initialize GICv2m driver.
> It also exposes gicv2m_init() function, which simplifies callers
> to a single GICv2m init function.
>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
> Signed-off-by: Hanjun Guo <hanjun.guo@...aro.org>
> ---
> drivers/irqchip/irq-gic-v2m.c | 114 +++++++++++++++++++++++++++++++++++++++-
> drivers/irqchip/irq-gic.c | 6 ++-
> include/linux/irqchip/arm-gic.h | 3 +-
> 3 files changed, 120 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> index 779c390..e01a495 100644
> --- a/drivers/irqchip/irq-gic-v2m.c
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -15,9 +15,11 @@
>
> #define pr_fmt(fmt) "GICv2m: " fmt
>
> +#include <linux/acpi.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
> +#include <linux/msi.h>
> #include <linux/of_address.h>
> #include <linux/of_pci.h>
> #include <linux/slab.h>
> @@ -138,6 +140,11 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
> fwspec.param[0] = 0;
> fwspec.param[1] = hwirq - 32;
> fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> + } else if (is_fwnode_irqchip(domain->parent->fwnode)) {
> + fwspec.fwnode = domain->parent->fwnode;
> + fwspec.param_count = 2;
> + fwspec.param[0] = hwirq;
> + fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> } else {
> return -EINVAL;
> }
> @@ -255,6 +262,8 @@ static void gicv2m_teardown(void)
> kfree(v2m->bm);
> iounmap(v2m->base);
> of_node_put(to_of_node(v2m->fwnode));
> + if (is_fwnode_irqchip(v2m->fwnode))
> + irq_domain_free_fwnode(v2m->fwnode);
> kfree(v2m);
> }
> }
> @@ -373,11 +382,16 @@ static struct of_device_id gicv2m_device_id[] = {
> {},
> };
>
> -int __init gicv2m_of_init(struct device_node *node, struct irq_domain *parent)
> +static int __init gicv2m_of_init(struct fwnode_handle *parent_handle,
> + struct irq_domain *parent)
> {
> int ret = 0;
> + struct device_node *node = to_of_node(parent_handle);
> struct device_node *child;
>
> + if (!node)
> + return -EINVAL;
> +
I don't think we need this, see below.
> for (child = of_find_matching_node(node, gicv2m_device_id); child;
> child = of_find_matching_node(child, gicv2m_device_id)) {
> u32 spi_start = 0, nr_spis = 0;
> @@ -411,3 +425,101 @@ int __init gicv2m_of_init(struct device_node *node, struct irq_domain *parent)
> gicv2m_teardown();
> return ret;
> }
> +
> +#ifdef CONFIG_ACPI
> +static int acpi_num_msi;
> +
> +static struct fwnode_handle *gicv2m_get_fwnode(struct device *dev)
> +{
> + struct v2m_data *data;
> +
> + if (WARN_ON(acpi_num_msi <= 0))
> + return NULL;
> +
> + /* We only return the fwnode of the first MSI frame. */
> + data = list_first_entry_or_null(&v2m_nodes, struct v2m_data, entry);
> + if (!data)
> + return NULL;
> +
> + return data->fwnode;
> +}
> +
> +static int __init
> +acpi_parse_madt_msi(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + int ret;
> + struct resource res;
> + u32 spi_start = 0, nr_spis = 0;
> + struct acpi_madt_generic_msi_frame *m;
> + struct fwnode_handle *fwnode;
> +
> + m = (struct acpi_madt_generic_msi_frame *)header;
> + if (BAD_MADT_ENTRY(m, end))
> + return -EINVAL;
> +
> + res.start = m->base_address;
> + res.end = m->base_address + SZ_4K;
> +
> + if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) {
> + spi_start = m->spi_base;
> + nr_spis = m->spi_count;
> +
> + pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n",
> + spi_start, nr_spis);
> + }
> +
> + fwnode = irq_domain_alloc_fwnode((void *)m->base_address);
> + if (!fwnode) {
> + pr_err("Unable to allocate GICv2m domain token\n");
> + return -EINVAL;
> + }
> +
> + ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res);
> + if (ret)
> + irq_domain_free_fwnode(fwnode);
> +
> + return ret;
> +}
> +
> +static int __init gicv2m_acpi_init(struct irq_domain *parent)
> +{
> + int ret;
> +
> + if (acpi_num_msi > 0)
> + return 0;
> +
> + acpi_num_msi = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_MSI_FRAME,
> + acpi_parse_madt_msi, 0);
> +
> + if (acpi_num_msi <= 0)
> + goto err_out;
> +
> + ret = gicv2m_allocate_domains(parent);
> + if (ret)
> + goto err_out;
> +
> + pci_msi_register_fwnode_provider(&gicv2m_get_fwnode);
> +
> + return 0;
> +
> +err_out:
> + gicv2m_teardown();
> + return -EINVAL;
> +}
> +#else /* CONFIG_ACPI */
> +static int __init gicv2m_acpi_init(struct irq_domain *parent)
> +{
> + return -EINVAL;
> +}
> +#endif /* CONFIG_ACPI */
> +
> +int __init gicv2m_init(struct fwnode_handle *parent_handle,
> + struct irq_domain *parent)
> +{
> + int ret = gicv2m_of_init(parent_handle, parent);
> +
> + if (ret)
> + ret = gicv2m_acpi_init(parent);
> + return ret;
This should really read:
if (is_of_node(parent_handle))
return gicv2m_of_init(parent_handle, parent);
return gicv2m_acpi_init(parent);
and you can loose the test for NULL in gicv2m_of_init().
> +}
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index fcd327f..644e8bb 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1234,7 +1234,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> }
>
> if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
> - gicv2m_of_init(node, gic_data[gic_cnt].domain);
> + gicv2m_init(&node->fwnode, gic_data[gic_cnt].domain);
>
> gic_cnt++;
> return 0;
> @@ -1359,6 +1359,10 @@ static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
> __gic_init_bases(0, -1, dist_base, cpu_base, 0, domain_handle);
>
> acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
> +
> + if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
> + gicv2m_init(NULL, gic_data[0].domain);
> +
> return 0;
> }
> IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index bae69e5..febc6c3 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -106,7 +106,8 @@ int gic_cpu_if_down(unsigned int gic_nr);
> void gic_init(unsigned int nr, int start,
> void __iomem *dist , void __iomem *cpu);
>
> -int gicv2m_of_init(struct device_node *node, struct irq_domain *parent);
> +int gicv2m_init(struct fwnode_handle *parent_handle,
> + struct irq_domain *parent);
>
> void gic_send_sgi(unsigned int cpu_id, unsigned int irq);
> int gic_get_cpu_id(unsigned int cpu);
>
Apart from the fairly minor issue above:
Reviewed-by: Marc Zyngier <marc.zyngier@....com>
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists