linux-kernel - Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55AF55C2.4060807@linaro.org>
Date:	Wed, 22 Jul 2015 16:35:14 +0800
From:	Hanjun Guo <hanjun.guo@...aro.org>
To:	Marc Zyngier <marc.zyngier@....com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@....com>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Jason Cooper <jason@...edaemon.net>,
	"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>,
	Tomasz Nowicki <tomasz.nowicki@...aro.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>
Subject: Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

On 07/22/2015 02:12 AM, Marc Zyngier wrote:
> On 21/07/15 19:05, Lorenzo Pieralisi wrote:
>> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
>>> Now that the basic ACPI GSI code is irq domain aware, make sure
>>> that the ACPI support in the GIC doesn't pointlessly deviate from
>>> the DT path.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
>>> ---
>>>   drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>>>   include/linux/irqchip/arm-gic.h |  2 +-
>>>   2 files changed, 7 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index b41ccf5..f5d365d 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>>   {
>>>   	unsigned long ret = 0;
>>>
>>> -	if (irq_domain_get_of_node(d) != controller)
>>> -		return -EINVAL;
>>>   	if (intsize < 3)
>>>   		return -EINVAL;
>>>
>>> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>>>
>>>   void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>   			   void __iomem *dist_base, void __iomem *cpu_base,
>>> -			   u32 percpu_offset, struct device_node *node)
>>> +			   u32 percpu_offset, void *domain_token)
>>>   {
>>>   	irq_hw_number_t hwirq_base;
>>>   	struct gic_chip_data *gic;
>>> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>   		gic_irqs = 1020;
>>>   	gic->gic_irqs = gic_irqs;
>>>
>>> -	if (node) {		/* DT case */
>>> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>> +	if (domain_token) {		/* DT/ACPI case */
>>> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>>>   						    &gic_irq_domain_hierarchy_ops,
>>>   						    gic);
>>>   	} else {		/* Non-DT case */
>>> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>   			irq_base = irq_start;
>>>   		}
>>>
>>> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
>>> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>>>   					hwirq_base, &gic_irq_domain_ops, gic);
>>>   	}
>>>
>>> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>>>   	}
>>>
>>>   	/*
>>> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>>> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
>>> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>>> +	 * Initialize zero GIC instance (no multi-GIC support).
>>>   	 */
>>> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>>> -	irq_set_default_host(gic_data[0].domain);
>>> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
>>
>> Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
>> use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
>> to be careful anyway.
>
> Yeah, I noticed that one too, but couldn't imagine the PIC being
> migrated to that model just yet. It looks like it would be pretty
> harmless to set ACPI_IRQ_MODEL_PIC to 1, and introduce
> ACPI_IRQ_MODEL_ILLEGAL as zero.

I think this will be a problem, because acpi_irq_model_id enum actually
is defined by the ACPI spec, and the value is used to report to BIOS
the current interrupt model used by OS, see section 5.8.1 _PIC Method
in ACPI 6.0:

0 – PIC mode
1 – APIC mode
2 – SAPIC mode
Other values –Reserved

so we can't set ACPI_IRQ_MODEL_PIC to 1 as it may break the firmware,
also _PIC method actually is not needed for ARM platform at all, we
don't need to report to firmware the interrupt model used by OS on
ARM, it only used by legacy IA platforms, actually I'm planning to
remove acpi_irq_model_id on ARM64.

So to me acpi_irq_model_id is suitable for the token, can we use
another one as the token? how about the GIC ID in the MADT table?
and this also can be used for x86 (IOAPIC IDs) too.

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/