linux-kernel - Re: [PATCH 3.18-rc3 v9 3/5] irqchip: gic: Introduce plumbing for IPI FIQ

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:	Wed, 26 Nov 2014 15:48:47 +0000
From:	Daniel Thompson <daniel.thompson@...aro.org>
To:	Tim Sander <tim@...eglstein.org>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	Russell King <linux@....linux.org.uk>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	patches@...aro.org, linaro-kernel@...ts.linaro.org,
	John Stultz <john.stultz@...aro.org>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	Dirk Behme <dirk.behme@...bosch.com>,
	Daniel Drake <drake@...lessm.com>,
	Dmitry Pervushin <dpervushin@...il.com>,
	Marc Zyngier <marc.zyngier@....com>,
	Marek Vasut <marex@...x.de>
Subject: Re: [PATCH 3.18-rc3 v9 3/5] irqchip: gic: Introduce plumbing for
 IPI FIQ

On 26/11/14 15:09, Tim Sander wrote:
> I would be quite happy if grouping support for gic would be mainlined.
> Then only the dance to get the old gic version 1 working with fiqs would be 
> needed...

You mention "the dance"...

Are you familiar with this work from Marek Vasut?
  https://lkml.org/lkml/2014/7/15/550

Marek blushed a bit when it was written and it wasn't very popular in
code review... however it does arranges memory to mapped in a manner
that allows FIQ to be deployed by the kernel on early gic v1 devices.


>> +/*
>> + * Shift an interrupt between Group 0 and Group 1.
>> + *
>> + * In addition to changing the group we also modify the priority to
>> + * match what "ARM strongly recommends" for a system where no Group 1
>> + * interrupt must ever preempt a Group 0 interrupt.
>> + *
>> + * If is safe to call this function on systems which do not support
>> + * grouping (it will have no effect).
>> + */
>> +static void gic_set_group_irq(void __iomem *base, unsigned int hwirq,
>> +				int group)
>> +{
>> +	unsigned int grp_reg = hwirq / 32 * 4;
>> +	u32 grp_mask = BIT(hwirq % 32);
>> +	u32 grp_val;
>> +
>> +	unsigned int pri_reg = (hwirq / 4) * 4;
>> +	u32 pri_mask = BIT(7 + ((hwirq % 4) * 8));
>> +	u32 pri_val;
>> +
>> +	/*
>> +	 * Systems which do not support grouping will have not have
>> +	 * the EnableGrp1 bit set.
>> +	 */
>> +	if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)))
>> +		return;
>> +
>> +	raw_spin_lock(&irq_controller_lock);
>> +
> Assumption: The interrupt in question is not masked over here?

At present this function is called only during initialization and all
interrupts are globally disabled at that stage in the boot.


>> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
>> +	pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg);
>> +
>> +	if (group) {
>> +		grp_val |= grp_mask;
>> +		pri_val |= pri_mask;
>> +	} else {
>> +		grp_val &= ~grp_mask;
>> +		pri_val &= ~pri_mask;
>> +	}
>> +
>> +	writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg);
> If the assumption is true, then there is a race if the interrupt in question 
> hits here with undefined priority setting. Recomended workaround would be
> masking the interrupt in question.

An interesting question!

Firstly, as mentioned above, such a race is impossible with the code
proposed so far.

I do have some code sitting written by untested that makes it possible
to set the group based on a flag passed during request_irq() (something
requested by tglx in a review from a month or two back). That also means
the interrupt is disabled during the call.

I think that means that neither now nor in the immediate future would
such a race be possible.


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