| 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, 11 Sep 2014 13:01:35 +0100
From: Marc Zyngier <marc.zyngier@....com>
To: Grant Likely <grant.likely@...aro.org>
CC: Tomasz Nowicki <tomasz.nowicki@...aro.org>,
"hanjun.guo@...aro.org" <hanjun.guo@...aro.org>,
Catalin Marinas <Catalin.Marinas@....com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Mark Rutland <Mark.Rutland@....com>,
Olof Johansson <olof@...om.net>,
"graeme.gregory@...aro.org" <graeme.gregory@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
Sudeep Holla <Sudeep.Holla@....com>,
Will Deacon <Will.Deacon@....com>,
Jason Cooper <jason@...edaemon.net>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Robert Richter <rric@...nel.org>,
Lv Zheng <lv.zheng@...el.com>,
Robert Moore <robert.moore@...el.com>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
Liviu Dudau <Liviu.Dudau@....com>,
Randy Dunlap <rdunlap@...radead.org>,
Charles Garcia-Tobin <Charles.Garcia-Tobin@....com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>
Subject: Re: [PATCH v3 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support
Hi Grant,
On 11/09/14 12:48, Grant Likely wrote:
> On Tue, 02 Sep 2014 13:48:37 +0200, Tomasz Nowicki <tomasz.nowicki@...aro.org> wrote:
>> On 01.09.2014 19:35, Marc Zyngier wrote:
>>> On 01/09/14 15:57, Hanjun Guo wrote:
>>>> From: Tomasz Nowicki <tomasz.nowicki@...aro.org>
>>>>
>>>> ACPI kernel uses MADT table for proper GIC initialization. It needs to
>>>> parse GIC related subtables, collect CPU interface and distributor
>>>> addresses and call driver initialization function (which is hardware
>>>> abstraction agnostic). In a similar way, FDT initialize GICv1/2.
>>>>
>>>> NOTE: This commit allow to initialize GICv1/2 only.
>>>
>>> I cannot help but notice that there is no support for KVM here. It'd be
>>> good to add a note to that effect, so that people do not expect
>>> virtualization support to be working when booting with ACPI.
>>
>> yes, it is worth mentioning!
>>
>>>
>>>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@...aro.org>
>>>> Signed-off-by: Hanjun Guo <hanjun.guo@...aro.org>
>>>> ---
>>>> arch/arm64/include/asm/acpi.h | 2 -
>>>> arch/arm64/kernel/acpi.c | 23 +++++++
>>>> arch/arm64/kernel/irq.c | 5 ++
>>>> drivers/irqchip/irq-gic.c | 114 ++++++++++++++++++++++++++++++++++
>>>> include/linux/irqchip/arm-gic-acpi.h | 33 ++++++++++
>>>> 5 files changed, 175 insertions(+), 2 deletions(-)
>>>> create mode 100644 include/linux/irqchip/arm-gic-acpi.h
>>>>
>>>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>>>> index a867467..5d2ab63 100644
>>>> --- a/arch/arm64/include/asm/acpi.h
>>>> +++ b/arch/arm64/include/asm/acpi.h
>>>> @@ -97,8 +97,6 @@ void __init acpi_smp_init_cpus(void);
>>>> extern int (*acpi_suspend_lowlevel)(void);
>>>> #define acpi_wakeup_address 0
>>>>
>>>> -#define ACPI_MAX_GIC_CPU_INTERFACE_ENTRIES 65535
>>>> -
>>>> #else
>>>>
>>>> static inline bool acpi_psci_present(void) { return false; }
>>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>>> index 354b912..b3b82b0 100644
>>>> --- a/arch/arm64/kernel/acpi.c
>>>> +++ b/arch/arm64/kernel/acpi.c
>>>> @@ -23,6 +23,7 @@
>>>> #include <linux/irqdomain.h>
>>>> #include <linux/bootmem.h>
>>>> #include <linux/smp.h>
>>>> +#include <linux/irqchip/arm-gic-acpi.h>
>>>>
>>>> #include <asm/cputype.h>
>>>> #include <asm/cpu_ops.h>
>>>> @@ -313,6 +314,28 @@ void __init acpi_boot_table_init(void)
>>>> pr_err("Can't find FADT or error happened during parsing FADT\n");
>>>> }
>>>>
>>>> +void __init acpi_gic_init(void)
>>>> +{
>>>> + struct acpi_table_header *table;
>>>> + acpi_status status;
>>>> + acpi_size tbl_size;
>>>> + int err;
>>>> +
>>>> + status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, &table, &tbl_size);
>>>> + if (ACPI_FAILURE(status)) {
>>>> + const char *msg = acpi_format_exception(status);
>>>> +
>>>> + pr_err("Failed to get MADT table, %s\n", msg);
>>>> + return;
>>>> + }
>>>> +
>>>> + err = gic_v2_acpi_init(table);
>>>> + if (err)
>>>> + pr_err("Failed to initialize GIC IRQ controller");
>>>
>>> What will happen when you get to implement GICv3 support? Another entry
>>> like this? Why isn't this entirely contained in the GIC driver? Do I
>>> sound like a stuck record?
>>
>> There will be another call to GICv3 init:
>> [...]
>> err = gic_v3_acpi_init(table);
>> if (err)
>> err = gic_v2_acpi_init(table);
>> if (err)
>> pr_err("Failed to initialize GIC IRQ controller");
>> [...]
>> This is the main reason I put common code here.
>>
>>>
>>>> +
>>>> + early_acpi_os_unmap_memory((char *)table, tbl_size);
>>>> +}
>>>> +
>>>> /*
>>>> * acpi_suspend_lowlevel() - save kernel state and suspend.
>>>> *
>>>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>>>> index 0f08dfd..c074d60 100644
>>>> --- a/arch/arm64/kernel/irq.c
>>>> +++ b/arch/arm64/kernel/irq.c
>>>> @@ -28,6 +28,7 @@
>>>> #include <linux/irqchip.h>
>>>> #include <linux/seq_file.h>
>>>> #include <linux/ratelimit.h>
>>>> +#include <linux/irqchip/arm-gic-acpi.h>
>>>>
>>>> unsigned long irq_err_count;
>>>>
>>>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
>>>> void __init init_IRQ(void)
>>>> {
>>>> irqchip_init();
>>>> +
>>>> + if (!handle_arch_irq)
>>>> + acpi_gic_init();
>>>> +
>>>
>>> Why isn't this called from irqchip_init? It would seem like the logical
>>> spot to probe an interrupt controller.
>>
>> irqchip.c is OF dependent, I want to decouple these from the very
>> beginning.
>
> It doesn't have to be that way, but given that ARM64 is the only
> platform in the foreseeable future that will use ACPI irq setup, it
> doesn't make sense to put it in there.
I have a different perspective. There is no reason to pollute the arch
code with something that is essentially platform specific.
irqchip_init is the logical place to probe for an irqchip, and I fail to
see the point of sticking this code somewhere else. Why would ACPI be so
special that it requires additional hooks in the arch code?
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