| 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: Wed, 05 Aug 2015 13:57:14 +0100
From: Marc Zyngier <marc.zyngier@....com>
To: Hanjun Guo <hanjun.guo@...aro.org>,
Jason Cooper <jason@...edaemon.net>,
Will Deacon <Will.Deacon@....com>,
Catalin Marinas <Catalin.Marinas@....com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>
CC: Thomas Gleixner <tglx@...utronix.de>,
Jiang Liu <jiang.liu@...ux.intel.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
Timur Tabi <timur@...eaurora.org>,
Tomasz Nowicki <tomasz.nowicki@...aro.org>,
"grant.likely@...aro.org" <grant.likely@...aro.org>,
Mark Brown <broonie@...nel.org>, Wei Huang <wei@...hat.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>
Subject: Re: [PATCH v4 01/10] irqchip / GIC: Add GIC version support in ACPI
MADT
On 05/08/15 13:40, Hanjun Guo wrote:
> On 08/04/2015 08:06 PM, Marc Zyngier wrote:
>> On 29/07/15 11:08, Hanjun Guo wrote:
>>> There is a field added in ACPI 6.0 MADT table to indicate the
>>> GIC version, so parse the table to get its value for later use.
>>>
>>> If GIC version presented in MADT is 0, we need to fallback to
>>> hardware discovery to get the GIC version.
>>>
>>> In ACPI MADT table there is no compatible strings to indicate
>>> various irqchips and also ACPI doesn't support irqchips which
>>> are not compatible with ARM GIC spec, so GIC version can be used
>>> to load different GIC drivers which is needed for the later patch.
>>>
>>> Signed-off-by: Hanjun Guo <hanjun.guo@...aro.org>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> drivers/irqchip/Kconfig | 3 +
>>> drivers/irqchip/Makefile | 1 +
>>> drivers/irqchip/irq-gic-acpi.c | 109 +++++++++++++++++++++++++++++++++++
>>> include/linux/irqchip/arm-gic-acpi.h | 1 +
>>> 5 files changed, 115 insertions(+)
>>> create mode 100644 drivers/irqchip/irq-gic-acpi.c
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 318175f..f2ff61f 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -16,6 +16,7 @@ config ARM64
>>> select ARM_AMBA
>>> select ARM_ARCH_TIMER
>>> select ARM_GIC
>>> + select ARM_GIC_ACPI if ACPI
>>> select AUDIT_ARCH_COMPAT_GENERIC
>>> select ARM_GIC_V2M if PCI_MSI
>>> select ARM_GIC_V3
>>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>> index 120d815..557ec2f 100644
>>> --- a/drivers/irqchip/Kconfig
>>> +++ b/drivers/irqchip/Kconfig
>>> @@ -47,6 +47,9 @@ config ARM_VIC_NR
>>> The maximum number of VICs available in the system, for
>>> power management.
>>>
>>> +config ARM_GIC_ACPI
>>> + bool
>>> +
>>> config ATMEL_AIC_IRQ
>>> bool
>>> select GENERIC_IRQ_CHIP
>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>> index 11d08c9..383f421 100644
>>> --- a/drivers/irqchip/Makefile
>>> +++ b/drivers/irqchip/Makefile
>>> @@ -23,6 +23,7 @@ obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o
>>> obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o
>>> obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o
>>> obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
>>> +obj-$(CONFIG_ARM_GIC_ACPI) += irq-gic-acpi.o
>>> obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
>>> obj-$(CONFIG_ARM_VIC) += irq-vic.o
>>> obj-$(CONFIG_ATMEL_AIC_IRQ) += irq-atmel-aic-common.o irq-atmel-aic.o
>>> diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c
>>> new file mode 100644
>>> index 0000000..6537b43
>>> --- /dev/null
>>> +++ b/drivers/irqchip/irq-gic-acpi.c
>>> @@ -0,0 +1,109 @@
>>> +/*
>>> + * ACPI based support for ARM GIC init
>>> + *
>>> + * Copyright (C) 2015, Linaro Ltd.
>>> + * Author: Hanjun Guo <hanjun.guo@...aro.org>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#define pr_fmt(fmt) "ACPI: GIC: " fmt
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/init.h>
>>> +#include <linux/irqchip/arm-gic-acpi.h>
>>> +#include <linux/irqchip/arm-gic-v3.h>
>>> +
>>> +/* GIC version presented in MADT GIC distributor structure */
>>> +static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE;
>>> +
>>> +static phys_addr_t dist_phy_base __initdata;
>>> +
>>> +static int __init
>>> +acpi_gic_parse_distributor(struct acpi_subtable_header *header,
>>> + const unsigned long end)
>>> +{
>>> + struct acpi_madt_generic_distributor *dist;
>>> +
>>> + dist = (struct acpi_madt_generic_distributor *)header;
>>> +
>>> + if (BAD_MADT_ENTRY(dist, end))
>>> + return -EINVAL;
>>> +
>>> + gic_version = dist->version;
>>> + dist_phy_base = dist->base_address;
>>> + return 0;
>>> +}
>>> +
>>> +static int __init
>>> +match_gic_redist(struct acpi_subtable_header *header, const unsigned long end)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static bool __init acpi_gic_redist_is_present(void)
>>> +{
>>> + int count;
>>> +
>>> + /* scan MADT table to find if we have redistributor entries */
>>> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
>>> + match_gic_redist, 0);
>>> +
>>> + /* that's true if we have at least one GIC redistributor entry */
>>> + return count > 0;
>>> +}
>>> +
>>> +static int __init acpi_gic_version_init(void)
>>> +{
>>> + int count;
>>> + u32 reg;
>>> + void __iomem *dist_base;
>>> +
>>> + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
>>> + acpi_gic_parse_distributor, 0);
>>> +
>>> + if (count <= 0) {
>>> + pr_err("No valid GIC distributor entry exists\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (gic_version >= ACPI_MADT_GIC_VERSION_RESERVED) {
>>> + pr_err("Invalid GIC version %d in MADT\n", gic_version);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /*
>>> + * when the GIC version is 0, we fallback to hardware discovery.
>>> + * this is also needed to keep compatiable with ACPI 5.1,
>>> + * which has no gic_version field in distributor structure and
>>> + * reserved as 0.
>>> + *
>>> + * For hardware discovery, the offset for GICv1/2 and GICv3/4 to
>>> + * get the GIC version is different (0xFE8 for GICv1/2 and 0xFFE8
>>> + * for GICv3/4), so we need to handle it separately.
>>> + */
>>> + if (gic_version == ACPI_MADT_GIC_VERSION_NONE) {
>>> + /* it's GICv3/v4 if redistributor is present */
>>> + if (acpi_gic_redist_is_present()) {
>>> + dist_base = ioremap(dist_phy_base,
>>> + ACPI_GICV3_DIST_MEM_SIZE);
>>> + if (!dist_base)
>>> + return -ENOMEM;
>>> +
>>> + reg = readl_relaxed(dist_base + GICD_PIDR2) &
>>> + GIC_PIDR2_ARCH_MASK;
>>> + if (reg == GIC_PIDR2_ARCH_GICv3)
>>> + gic_version = ACPI_MADT_GIC_VERSION_V3;
>>> + else
>>> + gic_version = ACPI_MADT_GIC_VERSION_V4;
>>
>> Frankly, why should we care? If there is no redistributor, this is a V2.
>> If there are redistributors, then it is a V3/V4, and it shouldn't matter
>> which one this is as the GICv3 driver can find out all by itself. Also,
>> the GICv4 feature only matter for KVM, which can probe this on its own.
>
> For hardware discovery, it works.
>
> I use the gic_version in later patch to ioremap physical base address of
> GICR (which 2 or 4 64k pages), but I can get it from register in later
> patch then remove the code above.
>
> For gic version in GICD subtable, we must consider V4 as firmware may
> just present ACPI_MADT_GIC_VERSION_V4 as the GIC version, that's why
> we need to match V4 in the GICv3 driver.
My point is: the GICv3 driver doesn't give a damn. At all. And it
shouldn't! The *only* difference is the size of the redistributor, and
this can be worked out *inside the GIC driver* by looking at:
- GICR_PIDR2.ArchRev (and not GICD_PIDR2)
- GICR_TYPER.VLPIS
Whatever the firmware says is absolutely irrelevant, and even if we
needed to override what the HW reports (because it is broken), we can
implement it as a quirk inside the driver itself, without any of this
ioremap dance that doesn't even do the right thing.
So to "we must consider V4", my answer is no. Certainly not at this
location.
Thanks,
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