ARM: ux500: Initialize irq affinity
Subsystems (3)
PATCH 1/1 ARM: ux500: Initialize irq affinity
Use irq_set_affinity() to initialize the kernel view of irq affinity when programming the GIC registers.
--- arch/arm/mach-ux500/cpu.c | 7 +++++++ arch/arm/mach-ux500/include/mach/irqs-db8500.h | 4 ++++ 2 files changed, 11 insertions(+), 0 deletions(-)
arch/arm/mach-ux500/cpu.c +7 -0
@@ -28,6 +28,7 @@ void __init ux500_init_irq(void) {void __iomem *dist_base;void __iomem *cpu_base; +int i;if (cpu_is_u5500 ()) { dist_base =__io_address (U5500_GIC_DIST_BASE );
@@ -40,6 +41,12 @@ void __init ux500_init_irq(void)gic_init (0 ,29 , dist_base, cpu_base); +for (i =IRQ_SHPI_START ; i <=IRQ_SHPI_END ; i++) { +if (i >=IRQ_MODEM_START && i <=IRQ_MODEM_END ) +continue ; +irq_set_affinity (i,cpumask_of (0 )); + } +/* * Init clocks here so that they are available for system timer * initialization.
arch/arm/mach-ux500/include/mach/irqs-db8500.h +4 -0
@@ -8,6 +8,10 @@#ifndef __MACH_IRQS_DB8500_H#define __MACH_IRQS_DB8500_H +#define IRQ_SHPI_END (IRQ_SHPI_START + 127) +#define IRQ_MODEM_START (IRQ_SHPI_START + 64) +#define IRQ_MODEM_END (IRQ_SHPI_START + 83) +#define IRQ_DB8500_MTU0 (IRQ_SHPI_START + 4)#define IRQ_DB8500_SPI2 (IRQ_SHPI_START + 6)#define IRQ_DB8500_PMU (IRQ_SHPI_START + 7)
Signature · 1 line
1.7.8.3
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 2 lines · Per Fransson
Use
irq_set_affinity()to initialize the kernel view of irq affinity when programming the GIC registers.
Why is this necessary? And if it is necessary, why do you think it should be ux500 specific?
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 5 lines · Russell King - ARM Linux
Quoted 2 lines · Per Fransson
Use
irq_set_affinity()to initialize the kernel view of irq affinity when programming the GIC registers.Why is this necessary?
Necessary: we don't quite know. (Maybe should have been RFC, but this patch is better than being silent of it...)
On the Ux500 all shared peripheral IRQs (32 thru 127) are delivered to CPU0 (hardwired, will not work on CPU1), I have heard that this is sort of what everybody does but don't know for sure. (Marc?)
Currently the gic_set_affinity() in common/gic.c isn't called for any IRQs so they are left in their power-on state (from U-Boot or similar) which is essentially correct (we checked dumps of the registers).
However when we look at the irq descriptors in e.g. crash:
crash> irq -s
IRQ NAME AFFINITY
36 Nomadik Timer Tick 0
43 uart-pl011 0-1
44 nmk-i2c 0-1
46 pl022 0-1
53 nmk-i2c 0-1
54 nmk-i2c 0-1
(... etc ...)So the descriptors think the IRQs are shared except for the timer. Which isn't true. So this patch makes the map match reality.
Quoted 2 lines
And if it is necessary, why do you think it
should be ux500 specific?
Good question. If this is the case for all SMP gic users maybe we should make the code #ifdef CONFIG_SMP inside gic.c instead? (Sans the check for the modem IRQs, they should probably also be made affine to CPU0.)
Yours,
Linus Walleij
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Russell King - ARM Linux
Quoted 2 lines · Per Fransson
Use
irq_set_affinity()to initialize the kernel view of irq affinity when programming the GIC registers.
Quoted 8 lines
Quoted 1 line
Why is this necessary?
Necessary: we don't quite know. (Maybe should have been RFC, but this patch is better than being silent of it...)
On the Ux500 all shared peripheral IRQs (32 thru 127) are delivered to CPU0 (hardwired, will not work on CPU1), I have heard that this is sort of what everybody does but don't know for sure. (Marc?)
Not as far as I know. My platforms seems to happily deliver interrupts on other cores by setting the SMP affinity (just tested on my EB11MP).
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 ARM: ux500: Initialize irq affinity
In reply to Linus WalleijHi Linus,
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Russell King - ARM Linux
Quoted 2 lines · Per Fransson
Use
irq_set_affinity()to initialize the kernel view of irq affinity when programming the GIC registers.
Quoted 8 lines
Quoted 1 line
Why is this necessary?
Necessary: we don't quite know. (Maybe should have been RFC, but this patch is better than being silent of it...)
On the Ux500 all shared peripheral IRQs (32 thru 127) are delivered to CPU0 (hardwired, will not work on CPU1), I have heard that this is sort of what everybody does but don't know for sure. (Marc?)
Oo-er, I've not heard of any platforms doing this before, but it is permitted by the GIC spec so we should deal with it.
Quoted 17 lines
Currently the
gic_set_affinity()incommon/gic.cisn't called for any IRQs so they are left in their power-on state (from U-Boot or similar) which is essentially correct (we checked dumps of the registers).However when we look at the irq descriptors in e.g. crash:
crash> irq -s IRQ NAME AFFINITY 36 Nomadik Timer Tick 0 43 uart-pl011 0-1 44 nmk-i2c 0-1 46 pl022 0-1 53 nmk-i2c 0-1 54 nmk-i2c 0-1 (... etc ...)
Quoted 3 lines
So the descriptors think the IRQs are shared except for the timer. Which isn't true. So this patch makes the map match reality.
Right. So these are SPIs that are configured statically in hardware. They should therefore be identifiable by writing zeros to the ICDIPTRn registers at the distributor and then reading back to see if any of the bits are set. If they are, we can create a cpumask for each interrupt and set the affinity to that.
The next question is, is gic_dist_init the right place to do this or is it too early (we need to update the irq_desc)?
Quoted 7 lines
Quoted 2 lines
And if it is necessary, why do you think it
should be ux500 specific?Good question. If this is the case for all SMP gic users maybe we should make the code #ifdef
CONFIG_SMPinsidegic.cinstead? (Sans the check for the modem IRQs, they should probably also be made affine to CPU0.)
Although I think you're the only platform doing this, we should aim to handle it in the GIC driver at some initialisation stage.
Will
PATCH 1/1 ARM: ux500: Initialize irq affinity
In reply to Linus WalleijQuoted 10 lines · Linus Walleij
Quoted 5 lines · Russell King - ARM Linux
Quoted 2 lines · Per Fransson
Use
irq_set_affinity()to initialize the kernel view of irq affinity when programming the GIC registers.Why is this necessary?
Necessary: we don't quite know. (Maybe should have been RFC, but this patch is better than being silent of it...)
I'm afraid Per Fransson took the thread off the mailing lists, so my reply to him also wasn't public. Here's what I said:
| The affinity starts off as "all CPUs", and the implementation is free | to route the IRQ to any of those CPUs. On x86, some hardware is smart | enough to route the IRQ to one of a set of CPUs, choosing which CPU | to target in hardware. | | We don't have that ability, and trying to do that in software proved to | be very problematic to get it to work satisfactorily. Given that there | is a userspace daemon which helps to solve the problem, it was decided | that we wouldn't bother with this i the kernel. | | So, we have to choose a CPU to route the IRQ to from the mask being | requested - and the requested mask may contain anything from a single CPU | to multiple CPUs. | | We choose to route it to the first online CPU in the set. That fits with | the model, and does not require us to report back which CPU it's currently | routed to. Indeed, architectures such as x86 can't tell you which CPU | the next interrupt will hit. | | So we shouldn't even try to do this on ARM. Yes, we end up with all IRQs | targetting CPU0 at boot, and with the masks showing 'all CPUs' but that's | not a problem. Really not a problem.
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Linus Walleij
Quoted 4 lines · Russell King - ARM Linux
Quoted 2 lines · Per Fransson
Use
irq_set_affinity()to initialize the kernel view of irq affinity when programming the GIC registers.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Why is this necessary?
Necessary: we don't quite know. (Maybe should have been RFC, but this patch is better than being silent of it...)
I'm afraid Per Fransson took the thread off the mailing lists, so my reply to him also wasn't public. ?Here's what I said:
Oh, I did do that. My apologies.
PATCH 1/1 ARM: ux500: Initialize irq affinity
In reply to Russell King - ARM LinuxHi Russell,
Quoted 4 lines · Russell King - ARM Linux
| So, we have to choose a CPU to route the IRQ to from the mask being | requested - and the requested mask may contain anything from a single CPU | to multiple CPUs. |
Quoted 8 lines
| We choose to route it to the first online CPU in the set. That fits with | the model, and does not require us to report back which CPU it's currently | routed to. Indeed, architectures such as x86 can't tell you which CPU | the next interrupt will hit. | | So we shouldn't even try to do this on ARM. Yes, we end up with all IRQs | targetting CPU0 at boot, and with the masks showing 'all CPUs' but that's | not a problem. Really not a problem.
I think this *could* be a problem if, as in Linus's case, some interrupts have a hardcoded affinity enforced by the hardware (i.e. there are sticky bits in the GIC distributor CPU targets registers). If we boot on a CPU that is different from the hardcoded affinity, we could end up initialising an interrupt with multiple targets. I suspect this will just lead to spurious interrupts at the end of the day, but there's a potential performance impact too.
So we could change gic_dist_init to ignore interrupts with sticky bits when setting up the targets (assuming that the hardcoded target is limited to a single CPU...) although we also need a way to prevent those interrupts being moved later on.
For the time being we can just limit ux500 to booting on (physical) CPU 0 and wait and see if anybody hardcodes their targets to point at secondary CPUs in the future.
Will
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 7 lines · Will Deacon
I think this *could* be a problem if, as in Linus's case, some interrupts have a hardcoded affinity enforced by the hardware (i.e. there are sticky bits in the GIC distributor CPU targets registers). If we boot on a CPU that is different from the hardcoded affinity, we could end up initialising an interrupt with multiple targets. I suspect this will just lead to spurious interrupts at the end of the day, but there's a potential performance impact too.
If we boot on a CPU which is incapable of receiving interrupts from attached devices, we have a much larger problem than just affinity. We potentially can't initialize those devices until their dependent CPU comes online - we certainly can't start to use those devices until that happens.
That's something we just don't deal with - and as far as userspace goes, trying to set an affinity to just the offline CPU will return an error code.
So, if you have interrupts which can only be serviced by a group of CPUs, at least one of those CPUs must remain online at all times that those dependent devices could produce an interrupt.
I think what we want instead is for the IRQ subsystem to record on a per-IRQ basis which CPUs the interrupt /can/ be routed to, and if:
affinity & cpus_online & allowable_cpus
becomes empty, the attempt to set the affinity is failed. Also, a callback in the hotplug code to error out if
cpus_online & allowable_cpus & ~offlining_cpu
would become an empty set.
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 9 lines · Russell King - ARM Linux
Quoted 7 lines · Will Deacon
I think this *could* be a problem if, as in Linus's case, some interrupts have a hardcoded affinity enforced by the hardware (i.e. there are sticky bits in the GIC distributor CPU targets registers). If we boot on a CPU that is different from the hardcoded affinity, we could end up initialising an interrupt with multiple targets. I suspect this will just lead to spurious interrupts at the end of the day, but there's a potential performance impact too.
Quoted 5 lines
If we boot on a CPU which is incapable of receiving interrupts from attached devices, we have a much larger problem than just affinity. We potentially can't initialize those devices until their dependent CPU comes online - we certainly can't start to use those devices until that happens.
My initial reading of the GIC spec was that you would always be able to augment the set of targets, but taking another look this doesn't appear to be the case. So yes, if/when we encounter a platform doing this for a CPU other than the boot CPU then we'll be in trouble.
Quoted 3 lines
That's something we just don't deal with - and as far as userspace goes, trying to set an affinity to just the offline CPU will return an error code.
Although we can currently end up reporting that an interrupt is affine only to an offline CPU (patch 7292/1 fixes this).
Quoted 7 lines
So, if you have interrupts which can only be serviced by a group of CPUs, at least one of those CPUs must remain online at all times that those dependent devices could produce an interrupt.
I think what we want instead is for the IRQ subsystem to record on a per-IRQ basis which CPUs the interrupt /can/ be routed to, and if:
Quoted 8 lines
affinity &
cpus_online&allowable_cpusbecomes empty, the attempt to set the affinity is failed. Also, a callback in the hotplug code to error out if
cpus_online&allowable_cpus& ~offlining_cpuwould become an empty set.
That looks like a good idea to me. I suggest we wait until somebody builds such a monster and get them to implement the solution :) (unless the ux500 is capable of booting on a CPU other than 0?).
Will
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 13 lines · Will Deacon
[Russell]
Quoted 12 lines
So, if you have interrupts which can only be serviced by a group of CPUs, at least one of those CPUs must remain online at all times that those dependent devices could produce an interrupt.
I think what we want instead is for the IRQ subsystem to record on a per-IRQ basis which CPUs the interrupt /can/ be routed to, and if:
? ? ? affinity &
cpus_online&allowable_cpusbecomes empty, the attempt to set the affinity is failed. ?Also, a callback in the hotplug code to error out if
Quoted 7 lines
Quoted 3 lines
? ? ?
cpus_online&allowable_cpus& ~offlining_cpuwould become an empty set.
That looks like a good idea to me. I suggest we wait until somebody builds such a monster and get them to implement the solution :) (unless the ux500 is capable of booting on a CPU other than 0?).
No it needs to boot on 0. So let's let this thread sleep ...
Thanks,
Linus Walleij
PATCH 1/1 ARM: ux500: Initialize irq affinity
In reply to Linus WalleijQuoted 3 lines · Linus Walleij
On the Ux500 all shared peripheral IRQs (32 thru 127) are delivered to CPU0 (hardwired, will not work on CPU1), I have heard that this is sort of what everybody does but don't know for sure. (Marc?)
BTW, if you can't route IRQs to a CPU, then we'll need a hook to refuse the affinity call.
Quoted 4 lines
Currently the
gic_set_affinity()incommon/gic.cisn't called for any IRQs so they are left in their power-on state (from U-Boot or similar) which is essentially correct (we checked dumps of the registers).
No they aren't. See the gic code:
cpu = cpu_logical_map(smp_processor_id());
cpumask = 1 << cpu;
cpumask |= cpumask << 8;
cpumask |= cpumask << 16;
/*
* Set all global interrupts to this CPU only.
*/
for (i = 32; i < gic_irqs; i += 4)
writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);At boot, we program _all_ the GIC IRQs to point@the boot CPU as part of the initialization, which will be logical CPU 0.
Quoted 11 lines
However when we look at the irq descriptors in e.g. crash:
crash> irq -s IRQ NAME AFFINITY 36 Nomadik Timer Tick 0 43 uart-pl011 0-1 44 nmk-i2c 0-1 46 pl022 0-1 53 nmk-i2c 0-1 54 nmk-i2c 0-1 (... etc ...)
Yes, and as I said in my previous email, having the affinity mask report more CPUs than we set the hardware to is perfectly legal. What you've pointed out above is that setting an affinity mask of _just_ CPU1 should be made fail on Ux500.
In other words, when setting an affinity, if:
affinity & online_mask & ~invalid_cpus
is zero, then we should fail the request to set the affinity.
But that doesn't mean that setting the affinity to ~0 (all CPUs, including the invalid CPUs) should be failed.
PATCH 1/1 ARM: ux500: Initialize irq affinity
Quoted 11 lines · Russell King - ARM Linux
Quoted 4 lines · Linus Walleij
Currently the
gic_set_affinity()incommon/gic.cisn't called for any IRQs so they are left in their power-on state (from U-Boot or similar) which is essentially correct (we checked dumps of the registers).No they aren't. ?See the gic code:
(...)
At boot, we program _all_ the GIC IRQs to point at the boot CPU as part of the initialization, which will be logical CPU 0.
Yes, I was dead wrong. And I even read that code just hours ago :-(
Quoted 12 lines
Quoted 11 lines
However when we look at the irq descriptors in e.g. crash:
? ? crash> irq -s
? ? IRQ NAME ? ? ? ? ? ? ? ? AFFINITY
? ? ?36 Nomadik Timer Tick ? 0
? ? ?43 uart-pl011 ? ? ? ? ? 0-1
? ? ?44 nmk-i2c ? ? ? ? ? ? ?0-1
? ? ?46 pl022 ? ? ? ? ? ? ? ?0-1
? ? ?53 nmk-i2c ? ? ? ? ? ? ?0-1
? ? ?54 nmk-i2c ? ? ? ? ? ? ?0-1
? ? (... etc ...)
Quoted 2 lines
Yes, and as I said in my previous email, having the affinity mask report more CPUs than we set the hardware to is perfectly legal.
OK I see, then it's no problem.
Yours,
Linus Walleij