| 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: <55C20B83.10808@linaro.org>
Date: Wed, 05 Aug 2015 21:11:31 +0800
From: Hanjun Guo <hanjun.guo@...aro.org>
To: Marc Zyngier <marc.zyngier@....com>,
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 08/05/2015 08:57 PM, Marc Zyngier wrote:
> 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.
Sorry, I didn't make it clear. what I mean is that when GIC version
is presented as V4, we can match the GICv3 driver with it, and
everything will be the same in the driver itself.
Thanks
Hanjun
--
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