linux-kernel - Re: [PATCH 1/2] ARM: gic: add irq

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: <4E84DCF4.4020904@gmail.com>
Date:	Thu, 29 Sep 2011 16:02:44 -0500
From:	Rob Herring <robherring2@...il.com>
To:	Grant Likely <grant.likely@...retlab.ca>
CC:	linux-arm-kernel@...ts.infradead.org,
	devicetree-discuss@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	marc.zyngier@....com, thomas.abraham@...aro.org,
	jamie@...ieiles.com, b-cousson@...com, shawn.guo@...aro.org
Subject: Re: [PATCH 1/2] ARM: gic: add irq_domain support

On 09/29/2011 12:15 PM, Grant Likely wrote:
> On Wed, Sep 28, 2011 at 10:53:55PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@...xeda.com>
>>
>> Convert the gic interrupt controller to use irq domains in preparation
>> for device-tree binding and MULTI_IRQ. This allows for translation between
>> GIC interrupt IDs and Linux irq numbers.
>>
>> The meaning of irq_offset has changed. It now is just the number of skipped
>> GIC interrupt IDs for the controller. It will be 16 for primary GIC and 32
>> for secondary GICs.
> 
> [...]
> 
>>  /*
>> @@ -81,7 +82,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
>>   */
>>  static void gic_mask_irq(struct irq_data *d)
>>  {
>> -	u32 mask = 1 << (d->irq % 32);
>> +	u32 mask = 1 << (gic_irq(d) % 32);
> 
> This can probably change simply to d->hwirq if irq_offset is
> eliminated as I describe below.
> 
>>  void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
>>  	void __iomem *dist_base, void __iomem *cpu_base)
>>  {
>>  	struct gic_chip_data *gic;
>> +	struct irq_domain *domain;
>> +	int gic_irqs;
>>  
>>  	BUG_ON(gic_nr >= MAX_GIC_NR);
>>  
>>  	gic = &gic_data[gic_nr];
>> +	domain = &gic->domain;
>>  	gic->dist_base = dist_base;
>>  	gic->cpu_base = cpu_base;
>> -	gic->irq_offset = (irq_start - 1) & ~31;
>>  
>> -	if (gic_nr == 0)
>> +	/*
>> +	 * For primary GICs, skip over SGIs.
>> +	 * For secondary GICs, skip over PPIs, too.
>> +	 */
>> +	if (gic_nr == 0) {
>>  		gic_cpu_base_addr = cpu_base;
>> +		gic->irq_offset = 16;
>> +		irq_start = (irq_start & ~31) + 16;
> 
> With the switch to irq_domain, there should no longer be any need for
> a ~31 mask on the irq_start number.  Yes, you'll want to make sure
> that it doesn't allocate below irq 16, but the driver should
> completely use the irq_domain to manage the mapping from linux-irq
> number to hwirq number.  The ~31 mask appears to have been an
> optimization to quickly calculate hwirq number from the linux one, but
> that value is now found in irq_data->hwirq.

I started out exactly this way removing irq_offset. The problem is the
core irq domain code assumes that hwirq starts at 0, but for the gic it
starts at 16 or 32.

It could be fixed in the domain core, but that would effectively mean
moving irq_offset into the core.

The masking here is mainly to not break exynos. Everyone else passes in
a value of 16-31 which I could just hardcode to 16. So when exynos gets
all the interrupts converted over to domains, this can be removed. I
looked at it briefly and it doesn't look trivial.

> 
> irq_offset could almost be dropped entirely outside of probe because
> it no longer matters.  Instead, populate the to_irq() hook so that the
> hwirq value gets set correctly from the outset.
> 
> It would be better in general to have a consistent view of what the
> hwirq number means regardless of if it is a root GIC.

Agreed.

Rob
--
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/