linux-kernel - Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

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: <5694C72B.2060702@arm.com>
Date:	Tue, 12 Jan 2016 09:28:11 +0000
From:	Marc Zyngier <marc.zyngier@....com>
To:	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Noam Camus <noamc@...hip.com>
CC:	"linux-snps-arc@...ts.infradead.org" 
	<linux-snps-arc@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Chris Metcalf <cmetcalf@...hip.com>,
	"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>
Subject: Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

On 12/01/16 09:12, Vineet Gupta wrote:

[...]

>> HANDLE_DOMAIN_IRQ is not mandatory at all - a number of architectures
>> had something open-coded in the past (with some drawbacks and/or bugs),
>> and this config option is just one of the ways to get it right.
>>
>> MIPS/PPC perform the reverse lookup directly, without using this
>> infrastructure, and that would be a valid approach for ARC too.
> 
> I'd rather use the generic infrastructure and improve it if needed !
> 
>>>> Marc, I hope you are back from holidays. When u get a chance could you please
>>>> respond to above.
>>>>
>>>> Also I tool a stab at it anyways.
>>>> 1. Enabled HANDLE_DOMAIN_IRQ
>>>> 2. arch_do_IRQ() calls handle_domain_irq(NULL, hwirq, regs) since that code
>>>> doesn't know about domains.
>>>>
>>>> It fails w/o irq_set_default_host() being called.
>> Well, that's expected. unless you use the underlying primitive
>> (__handle_domain_irq) and pass false as the parameter for lookup (see
>> handle_IRQ() in arch/arm/kernel/irq.c). This is what we use for "legacy"
>> platforms that do not support domains.
> 
> Right I saw that and that causes virq = hwirq - kind of defeats the purpose if we
> were doing this on ARC.

You can forget about this if you convert all your platforms to using
domains. But if you have to deal with a transition period (which, in the
ARM case, will last exactly forever because some platforms will never be
converted to DT), this comes in handy.

>>
>>>> If we go back to sunxi example you quoted above, it relies on driver passing the
>>>> domain to handle_domain_irq(). IMHO it is simpler if we had the default domain.
>>>>
>>>> So long story short, ARC can be made to use handle_domain_irq() w/o the song and
>>>> dance of registering another callback from irqchip code if we retained the default
>>>> domain setting.
>> This only works if you can guarantee that you will never have another
>> irqchip calling irq_set_default_host()... If your system is always
>> simple enough to guarantee that, why not.
> 
> ATM we can certainly assume that.
> 
> However with ARM approach two irqchips can still call set_handle_irq() and only
> the first one succeeds (and others return silently). That seems wrong to me -
> irq_xxx.c will still use the handler registered by say irq_sun41.c ?

Oh, this is by no mean foolproof. We do rely on irqchips installing
their primary handler only if they are truly the primary irqchip (they
do not have a parent). Anything else will fail badly.

So if you're confident that you can ensure that noone will set de
default domain twice, you'll be fine.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...