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.