linux-kernel - Re: [RFC Part2 v1 01/21] irqdomain: Introduce new interfaces to support hierarchy irqdomains

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]

Date:	Thu, 18 Sep 2014 15:28:44 +0800
From:	Jiang Liu <jiang.liu@...ux.intel.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Yinghai Lu <yinghai@...nel.org>,
	Borislav Petkov <bp@...en8.de>,
	Grant Likely <grant.likely@...aro.org>,
	Marc Zyngier <marc.zyngier@....com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tony Luck <tony.luck@...el.com>,
	Joerg Roedel <joro@...tes.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	x86@...nel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC Part2 v1 01/21] irqdomain: Introduce new interfaces to support
 hierarchy irqdomains

On 2014/9/17 1:43, Thomas Gleixner wrote:
> Jiang,
> 
> On Thu, 11 Sep 2014, Jiang Liu wrote:
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>>  	/* Create mapping */
>> -	virq = irq_create_mapping(domain, hwirq);
>> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>> +	if (domain->ops->alloc)
>> +		virq = irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE,
>> +					     irq_data);
>> +	else
>> +#endif
>> +		virq = irq_create_mapping(domain, hwirq);
> 
> I'd prefer to get rid of the #ifdef CONFIG...s in the code. So this
> can be written:
> 
>         if (irq_domain_has_hierarchy(domain))
> 		virq = irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE,
> 					     irq_data);
> 	else
> 		virq = irq_create_mapping(domain, hwirq);
Sure, will kill the ifdef. 	

> 	   
> 
>>  	if (!virq)
>>  		return virq;
>>  
>> @@ -540,7 +542,11 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>>  		return 0;
>>  
>>  	if (hwirq < domain->revmap_direct_max_irq) {
>> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>> +		data = irq_domain_get_irq_data(domain, hwirq);
>> +#else
>>  		data = irq_get_irq_data(hwirq);
>> +#endif
> 
> Similar here. Make irq_domain_get_irq_data() map to irq_get_irq_data() for
> the non hierarchy mode so you end up with a single line:
> 
> -  		data = irq_get_irq_data(hwirq);
> +		data = irq_domain_get_irq_data(domain, hwirq);
Sure.

>> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>> +/**
>> + * irq_domain_alloc_irqs - Allocate IRQs from domain
>> + * @domain: domain to allocate from
>> + * @irq_base: allocate specified IRQ nubmer if irq_base >= 0
>> + * @nr_irqs: number of IRQs to allocate
>> + * @node: NUMA node id for memory allocation
>> + * @arg: domain specific argument
>> + * @realloc: IRQ descriptors have already been allocated if true
>> + *
>> + * Allocate IRQ numbers and initialized all data structures to support
>> + * hiearchy IRQ domains.
>> + * Parameter @realloc is mainly to support legacy IRQs.
> 
> What's the issue with the legacy irqs? So this has the interrupt
> descriptors allocated already. Are they already wired up for serving
> interrupts and what's the state of those lines?
Function arch_early_ioapic_init() will allocate irq descriptors and
irq_cfg structures for all legacy IRQ for three purposes:
1) To support ISA IRQs managed by 8259.
2) To reserve vectors on all CPUs for legacy IRQs
3) Prepare data structures to support pre_init_apic_IRQ0().
We will kill pre_init_apic_IRQ0() soon, so item 3 above won't be needed
anymore.

When __irq_domain_alloc_irqs() is called, only irq descriptor and
irq_cfg have been allocated, but the interrupt controller hardware
should be untouched yet.

> 
>> + * Returns error code or allocated IRQ number
> 
> Can you please add some documentation how the hierarchical allocation
> is supposed to work and how the domains are connected. That should
> probably go to Documentation/IRQ-domains.txt.
Sure, I will do my best to add documentations for it.
> 
> Other than that this looks pretty good! Nice work!
Thanks!
Gerry
> 
> Thanks,
> 
> 	tglx
> 
--
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/