linux-kernel - Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev

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: <4E775912.3060502@arm.com>
Date:	Mon, 19 Sep 2011 16:00:34 +0100
From:	Marc Zyngier <marc.zyngier@....com>
To:	Abhijeet Dharmapurikar <adharmap@...eaurora.org>
CC:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts

On 19/09/11 10:28, Marc Zyngier wrote:
> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>>  > + *  @devname: An ascii name for the claiming device
>>  > + *  @dev_id: A percpu cookie passed back to the handler function
>>  > + *
>>  > + *  This call allocates interrupt resources, but doesn't
>>  > + *  automatically enable the interrupt. It has to be done on each
>>  > + *  CPU using enable_percpu_irq().
>>  > + *
>>  > + *  Dev_id must be globally unique. It is a per-cpu variable, and
>>  > + *  the handler gets called with the interrupted CPU's instance of
>>  > + *  that variable.
>>  > + */
>>  > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>>  > +                   const char *devname, void __percpu *dev_id)
>>
>> Can we add irqflags argument. I think it will be useful to pass flags,
>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>> The chip could use a set_type callback for ppi's too.
> 
> We're entering dangerous territory here. While this would work with the
> GIC (the interrupt type is at the distributor level), you could easily
> imagine an interrupt controller with the PPI configuration at the CPU
> interface level... In that case, calling set_type from __setup_irq()
> would end up doing the wrong thing, and I'd hate the API to give the
> idea it can do things it may not do in the end...
> 
> Furthermore, do we actually have a GIC implementation where PPI
> configuration isn't read-only? I only know about the ARM implementation,
> and the Qualcomm may well be different (the spec says it's
> implementation defined).

Replying to myself after a quick investigation... Looks like the Qualcomm
implementation does exactly what is mentioned above:

arch/arm/mach-msm/platsmp.c:
void __cpuinit platform_secondary_init(unsigned int cpu)
{
	/* Configure edge-triggered PPIs */
	writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
	[...]

The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
is a banked register (otherwise we would not do it in platform_secondary_init(),
right?) So doing a set_type() from __setup_irq() would be just wrong. It really
needs to be done on a per-CPU basis.

Do you agree?

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

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