linux-kernel - Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc

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: <20120411205707.68AC13E08B6@localhost>
Date:	Wed, 11 Apr 2012 14:57:07 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Andreas Schwab <schwab@...ux-m68k.org>
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	devicetree-discuss@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	Rob Herring <rob.herring@...xeda.com>,
	Milton Miller <miltonm@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linuxppc-dev@...ts.ozlabs.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead

On Wed, 11 Apr 2012 15:29:42 +1000, Benjamin Herrenschmidt <benh@...nel.crashing.org> wrote:
> On Wed, 2012-04-11 at 11:33 +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2012-04-11 at 11:13 +1000, Benjamin Herrenschmidt wrote:
> > > On Sat, 2012-04-07 at 14:27 +0200, Andreas Schwab wrote:
> > > > Benjamin Herrenschmidt <benh@...nel.crashing.org> writes:
> > > > 
> > > > > It's arguable that this irq_set_irq_type(,NONE) shouln't be there but
> > > > > still ... it's been around for ever and things worked :-) So something
> > > > > -else- is causing the problem and I'd like to understand what exactly.
> > > > 
> > > > AFAICS before a09b659cd68c10ec6a30cb91ebd2c327fcd5bfe5
> > > > irq_set_irq_type(,NONE) was actually a no-op.
> > > 
> > > So I'm still a bit baffled... ie, I understand some of what's happening
> > > but not why it breaks things, I haven't yet managed to reproduce but I
> > > haven't tried too hard just yet (was away from the HW) :
> > 
> > Allright, I have a repro-case, I'll dig.
> 
> Ok, so it's Grant's fault :-)

I pretty much expected it would be.  :-p

> So basically, it's quite subtle and I'm only 99% sure of the details but
> I believe what happens is:
> 
>  - The audio interrupts get virq 64 and 65 (so above NR_IRQS)
> 
>  - The reverse map isn't pre-filled at map time (we should probably do
> it nowadays), we do it lazily so ...

Hmmm... I though I had merged a patch that does that.

/me goes to look again...

Okay, I did write that patch, but I never merged it because it didn't
get much review and I was already nervous about the other irq_domain
changes.  I'll post it again and ask for feedback.

>  - The whole business with irq_virq_count needs fixing. Basically the
> default value shouldn't be NR_IRQ. I suggest making it 0 and have all
> the use sites do something like:
> 
> 	max = irq_virq_count ? irq_virq_count : nr_irqs;
> 
> (Grant, can you take care of that ?)

Yeah, I've got a different way to fix it though.  There is exactly one
user of irq_virq_count in-tree right now: PS3.  Also, irq_virq_count
is only useful for the NOMAP mapping.  So, instead of having a single
global irq_virq_count values, I've dropped it entirely and added a
max_irq argument to irq_domain_add_nomap().  That makes it a property
of an individual nomap irq domain instead of a global system settting.

Hopefully I'll have a draft patch ready today.

g.
--
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/