Consolidating GIC per-cpu interrupts

arch/arm/kernel/smp_twd.c static void twd_set_mode(enum clock_event_mode mode, · line 51

Quoted 8 lines · Marc Zyngier

arch/arm/kernel/smp_twd.c +4 -0

@@ -43,6 +45,10 @@ static void twd_set_mode(enum clock_event_mode mode,
 		ctrl = TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_ONESHOT;
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
+#ifdef CONFIG_ARM_GIC_PPI_MAP
+		disable_irq(clk->irq);
+		/* fall through */
+#endif

Eww, no. This is wrong. You can get an UNUSED event if the TWD clock event device gets swapped out for some other clock event device (eg, one with a higher preference.)

This is at best a hack around the problem. The existing code deals with hot unplug via this in arch/arm/kernel/smp.c:

/*
* The generic clock events code purposely does not stop the local timer
* on CPU_DEAD/CPU_DEAD_FROZEN hotplug events, so we have to do it
* manually here.
*/
static void percpu_timer_stop(void)

{
        unsigned int cpu = smp_processor_id();
        struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);

        evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
}

Maybe that needs a local_timer_stop() ->twd_timer_stop() call rather than trying to be clever by doing this via the set_mode() method. That would seem to be overall a cleaner solution.

Use the normal interrupt scheme for the local timers by using a remapped PPI interrupt.

Tested on a Pandaboard.

Cc Tony Lindgren <[email protected]>

Reviewed-by Santosh Shilimkar <[email protected]>

Tested-by Santosh Shilimkar <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

Use the normal interrupt scheme for the local timers by using a remapped PPI interrupt.

MSM already had a very similar scheme, though still mixing both GIC-specific and generic APIs.

Fixes and ideas courtesy of Stephen Boyd.

Cc David Brown <[email protected]>

Cc Daniel Walker <[email protected]>

Cc Bryan Huntsman <[email protected]>

Reviewed-and-Tested-by Stephen Boyd <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

It is sometimes usefull to compute the mapping of a PPI on a CPU that is not the one handling the interrupt (in the probe function of a driver, for example).

Quoted 3 lines · Marc Zyngier

It is sometimes usefull to compute the mapping of a PPI on a CPU that is not the one handling the interrupt (in the probe function of a driver, for example).

This is unsafe. Firstly, smp_processor_id() in preemptible contexts is not valid as you can be migrated to another CPU at any moment.

Secondly, if you have a driver probe function, and you want to register a PPI interrupt for a different CPU, while you can call the request function, the resulting ->unmask will be called on the calling CPU (assuming preempt is disabled and we don't schedule). You might get lucky and it may be called on your intended CPU...

That means the mask registers you access are on the local CPU, not the target CPU, so you end up with the wrong interrupt enabled.

As request_threaded_irq() contains a kzalloc() which can schedule, I can't see how you can safely request a particular per-CPU interrupt and guarantee that you'll enable it on the appropriate core.

Finally, note that using request_irq() from the secondary_startup() thread is probably a very bad idea - should it require to schedule, there is nothing for the secondary CPU to switch to - we're the secondary CPUs idle thread.

I think you'll need to use setup_irq() to register the PPI interrupts, especially for the TWD. However, that then gives you a problem when you come to tear it down on CPU hot unplug, as free_irq() can't be used - it'll try to kfree() the static irqaction structure.

So I think more thought is required on this entire patch set.

I have thought (before this patch set appeared) about making the PPI stuff entirely GIC specific, having an array of 16 function pointers in the GIC code to allow things like TWD to hook into. No preemption or sleep problems, no genirq overhead to their handling, etc. Then again, I think we do need irq_enter()..irq_exit().

Quoted 5 lines · On Fri, 8 Jul 2011 20:57:08 +0100 Russell King - ARM Linux…

Quoted 3 lines · Marc Zyngier

It is sometimes usefull to compute the mapping of a PPI on a CPU that is not the one handling the interrupt (in the probe function of a driver, for example).

Quoted 2 lines

This is unsafe. Firstly, smp_processor_id() in preemptible contexts is not valid as you can be migrated to another CPU at any moment.

Point taken. I think I can solve this by having a per-cpu variable instead of hijacking the gic_chip_data structure.

Quoted 5 lines

Secondly, if you have a driver probe function, and you want to register a PPI interrupt for a different CPU, while you can call the request function, the resulting ->unmask will be called on the calling CPU (assuming preempt is disabled and we don't schedule). You might get lucky and it may be called on your intended CPU...

I think you're going way beyond than the intended use of this function. It is only there to be able to compute the PPI->IRQ mapping, *not* to register the IRQ, which of course only make sense on the target CPU.

And to ensure that drivers do that safely, that would _have_ to be a separate function which takes care of all that. The code would look something like this:

        cpumask_t cpus_allowed = current->cpus_allowed;
        set_cpus_allowed(current, cpumask_of_cpu(cpu));
        BUG_ON(cpu != smp_processor_id());
	irq = gic_ppi_map(ppi);
	ret = request_irq(irq, ...);
	set_cpus_allowed(current, cpus_allowed);

(But not this: this depends on CPUMASK_OFFSTACK not being set.)

Quoted 4 lines · Russell King - ARM Linux

And to ensure that drivers do that safely, that would _have_ to be a separate function which takes care of all that. The code would look something like this:

Quoted 8 lines

        cpumask_t cpus_allowed = current->cpus_allowed;
        set_cpus_allowed(current, cpumask_of_cpu(cpu));
        BUG_ON(cpu != smp_processor_id());
      irq = gic_ppi_map(ppi);
      ret = request_irq(irq, ...);
      set_cpus_allowed(current, cpus_allowed);

(But not this: this depends on CPUMASK_OFFSTACK not being set.)

So, we can either do something like this:

int gic_request_ppi(unsigned int ppi, irq_handler_t handler,
unsigned long flags, const char *name, void *data)

{
        cpumask_var_t cpus_allowed;
        unsigned int irq, cpu = get_cpu();
        int ret;

        /* Allocate cpus_allowed mask */
        if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL)) {
                ret = -ENOMEM;
                goto err;
        }

        /* Copy current thread affinity */
        cpumask_copy(cups_allowed, &current->cpus_allowed);

        /* Bind to the current CPU */
        set_cpus_allowed_ptr(current, cpumask_of(cpu));
        irq = gic_ppi_map(ppi);
        ret = request_irq(irq, handler, flags, name, data);
        set_cpus_allowed_ptr(current, cpus_allowed);

        free_cpumask_var(cpus_allowed);

err:

        put_cpu();
        return ret;
}

void gic_free_ppi(unsigned int ppi, void *data)

{
        cpumask_var_t cpus_allowed;
        unsigned int irq, cpu = get_cpu();

        /* Allocate cpus_allowed mask */
        if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL))
                goto err; /* BUG */

        /* Copy current thread affinity */
        cpumask_copy(cups_allowed, &current->cpus_allowed);

        /* Bind to the current CPU */
        set_cpus_allowed_ptr(current, cpumask_of(cpu));
        irq = gic_ppi_map(ppi);
        free_irq(irq, data);
        set_cpus_allowed_ptr(current, cpus_allowed);

        free_cpumask_var(cpus_allowed);

err:

        put_cpu();
}

Or the below, which will need platform people to tweak their entry-macro stuff to allow through IRQs 16-31.

There's also the question about whether we should pass in the desired CPU number to these too, in case we have a requirement to ensure that we get the PPI on a specific CPU, or whether we only care about the _current_ CPU we happen to be running on.

That depends on what else PPIs get used for other than TWD.

You won't be surprised if I say that I prefer your first version. The second one, while much simpler, keeps the additional low level entry point (gic_call_ppi), and has to do its own accounting.

But more than that. MSM timers are implemented using the same code on both UP and SMP, with or without GIC. which means we have to request the interrupt using the same API. Your first version would work in that case (gic_ppi_map() on a non-PPI should return the same number).

Quoted 4 lines

There's also the question about whether we should pass in the desired CPU number to these too, in case we have a requirement to ensure that we get the PPI on a specific CPU, or whether we only care about the _current_ CPU we happen to be running on.

As long as we tie mapping and interrupt request together, there is no reason to offer that functionality. But DT may need to resolve the mappings independently (while creating the platform devices), and the driver would then request the mapped PPI. In that case, we need to ensure we're running on the right CPU.

Quoted 8 lines · Marc Zyngier

You won't be surprised if I say that I prefer your first version. The second one, while much simpler, keeps the additional low level entry point (gic_call_ppi), and has to do its own accounting.

But more than that. MSM timers are implemented using the same code on both UP and SMP, with or without GIC. which means we have to request the interrupt using the same API. Your first version would work in that case (gic_ppi_map() on a non-PPI should return the same number).

Who says gic_request_ppi() will exist on systems without GIC? Or even gic_ppi_map()?

Let me make the point plain: no driver must EVER use gic_ppi_map(). No driver must EVER call request_irq() any other genirq function for a PPI interrupt directly. They must all be wrapped in suitable containers to bind the current thread to the current CPU. Not doing so will lead to failures due to the wrong CPUs registers being hit.

Quoted 10 lines

Quoted 4 lines

There's also the question about whether we should pass in the desired CPU number to these too, in case we have a requirement to ensure that we get the PPI on a specific CPU, or whether we only care about the _current_ CPU we happen to be running on.

As long as we tie mapping and interrupt request together, there is no reason to offer that functionality. But DT may need to resolve the mappings independently (while creating the platform devices), and the driver would then request the mapped PPI. In that case, we need to ensure we're running on the right CPU.

You still don't get the issue. Really you don't. And you don't seem to get DT either.

DT has precisely nothing to do with this, and should never have anything to do with this kind of mapping. Mapping a PPI + CPU to a _Linux_ IRQ number is a _Linux_ specific operation. It's not something that will be acceptable to be represented in DT. What DT describes is the _hardware_. So, if DT wants to describe a PPI, then DT should describe PPI N on CPU N.

However, the basis of my argument is that this has got precisely nothing to do with the mapping of PPIs to IRQ numbers. It's about the using unique IRQ numbers to describe an IRQ which can _only_ be accessed on one particular CPU.

The PPIs are really not standard interrupts. Trying to treat them as such means that all the standard genirq interfaces will need to be _wrapped_ to ensure that they're bound to the particular CPU that you're interested in.

The reason for that is to ensure that you hit the right hardware registers for the IRQ number you're passing in. Using my previous example, it's no good if you pass in IRQ9 (PPI2 CPU1) but end up hitting IRQ11's (PPI2 CPU3) registers instead.

Plus, someone will have to audit drivers even more carefully to ensure that they're not trying to use the standard request_irq() (or any other of the genirq interfaces) with PPI interrupt numbers. Who's going to do that?

So, I believe your patches are fundamentally misdesigned on a technical level, are fragile, and therefore are not suitable for integration into mainline.

And that's exactly what it does:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.html

What I was trying to explain (and obviously failed to) is that the _Linux_ DT _code_ will try to resolve the PPI number and convert it to a _Linux_ IRQ number. Unless of course you don't encode it as an interrupt at all, which seems to be what you're aiming for.

Quoted 7 lines · Marc Zyngier

And that's exactly what it does:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg05026.html

What I was trying to explain (and obviously failed to) is that the _Linux_ DT _code_ will try to resolve the PPI number and convert it to a _Linux_ IRQ number. Unless of course you don't encode it as an interrupt at all, which seems to be what you're aiming for.

I'm not aiming for anything. I'm trying to get you to fully understand the issue I've raised with your patches. So, let's try a new scenario based on your statement above.

You have a device which happens to use a PPI. You obtain its IRQ number from DT, which tells you IRQ 9, because the DT information said PPI 2 CPU 1. So you pass IRQ 9 into the IRQ request function, but as you're running on CPU 3, you have no access to the hardware for IRQ 9.

Please describe in detail how, with your patches, PPI 2 CPU 1 gets enabled rather than PPI 2 CPU 3 when IRQ 9 is requested.

The current GIC per-cpu interrupts (aka PPIs) suffer from a number of problems:

- They use a completely separate scheme to handle the interrupts, mostly because the PPI concept doesn't really match the kernel view of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level assembly code.
- The local timer code can only be used by devices generating PPIs, and not SPIs.
- At least one platform (msm) has started implementing its own alternative scheme.
- Some low-level code gets duplicated, as usual...

The proposed solution is to let the GIC code expose the PPIs as something that the kernel can manage. Instead of having a single interrupt number shared on all cores, make the interrupt number be different on each CPU.

This enables the use of the normal kernel API (request_irq() and friends) and the elimination of some low level code. It also serves as the basis of the GIC device tree support code. On the other side, it causes quite a bit of churn in the timer code.

Previous versions of this series have received ACKs from some platform maintainers, but so far there has been very little comment on the core code. As I now have quite a large number of patches relying on this series (local timers as devices, device tree support for GIC and TWD), I'd be grateful for at least a hint indicating that this is going in the right direction. Or not.

Quoted 24 lines · Marc Zyngier

The current GIC per-cpu interrupts (aka PPIs) suffer from a number of problems:

- They use a completely separate scheme to handle the interrupts, mostly because the PPI concept doesn't really match the kernel view of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level assembly code.
- The local timer code can only be used by devices generating PPIs, and not SPIs.
- At least one platform (msm) has started implementing its own alternative scheme.
- Some low-level code gets duplicated, as usual...

The proposed solution is to let the GIC code expose the PPIs as something that the kernel can manage. Instead of having a single interrupt number shared on all cores, make the interrupt number be different on each CPU.

This enables the use of the normal kernel API (request_irq() and friends) and the elimination of some low level code. It also serves as the basis of the GIC device tree support code. On the other side, it causes quite a bit of churn in the timer code.

Quoted 7 lines

Previous versions of this series have received ACKs from some platform maintainers, but so far there has been very little comment on the core code. As I now have quite a large number of patches relying on this series (local timers as devices, device tree support for GIC and TWD), I'd be grateful for at least a hint indicating that this is going in the right direction. Or not.

I'm about to start mainlining Freescale i.MX6 core code. It will be wonderful if these great consolidation works could hit v3.1 merge window, so that I can base i.MX6 off them in the next cycle.

The current GIC per-cpu interrupts (aka PPIs) suffer from a number of problems:

- They use a completely separate scheme to handle the interrupts, mostly because the PPI concept doesn't really match the kernel view of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level assembly code.
- The local timer code can only be used by devices generating PPIs, and not SPIs.
- At least one platform (msm) has started implementing its own alternative scheme.
- Some low-level code gets duplicated, as usual...

The proposed solution is to let the GIC code expose the PPIs as something that the kernel can manage. Instead of having a single interrupt number shared on all cores, make the interrupt number be different on each CPU.

This enables the use of the normal kernel API (request_irq() and friends) and the elimination of some low level code. It also serves as the basis of the GIC device tree support code. On the other side, it causes quite a bit of churn in the timer code.

Previous versions of this series have received ACKs from some platform maintainers, but so far there has been very little comment on the core code. As I now have quite a large number of patches relying on this series (local timers as devices, device tree support for GIC and TWD), I'd be grateful for at least a hint indicating that this is going in the right direction. Or not.

This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2 (Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git

From v7:
- Rebased on v3.0-rc5
- Rename gic_ppi_to_vppi() to gic_ppi_map()
- Rename CONFIG_ARM_GIC_VPPI to CONFIG_ARM_GIC_PPI_MAP
- Add gic_ppi_map_on_cpu()

From v6:
- Fix compilation when !CONFIG_PM.

From v5:
- Update Exynos4 MCT to use the local timer infrastructure, now that it doesn't depend on PPIs anymore. Untested, as the board I have doesn't implement MCT (EVT0 syndrome).

From v4:
- Fix arch/arm/common/Kconfig indentation
- Fix MSM warning when building UP.

From v3:
- Fix yet another CPU_HOTPLUG problem: instead of calling free_irq() on timer halt, just disable the interrupt. On timer restart, detect that the interrupt has been requested already and just enable the interrupt back. This avoid doing a request_irq() with preemption disabled, which triggers an ugly warning. Reported by Stephen Boyd.

From v2:
- More MSM fixes
- Dropped MSM interrupt handler removal, as this requires more discussion, and will be addressed in a separate series.
- Moved introduction of percpu_timer_handler to a separate patch

From v1:
- Fix crash while hotpluging a CPU
- Use handle_percpu_irq() instead of handle_fasteoi_irq()
- MSM fixes courtesy of Stephen Boyd
- MSM switched to percpu_timer_handler()
- Remove local timer interrupt accounting
- Restructure patches #1 and #2

Marc Zyngier (14):
ARM: gic: add per-cpu interrupt mapping
ARM: smp: add interrupt handler for local timers
ARM: smp_twd: add support for remapped PPI interrupts
ARM: omap4: use remapped PPI interrupts for local timer
ARM: versatile: use remapped PPI interrupts for local timer
ARM: shmobile: use remapped PPI interrupts for local timer
ARM: ux500: use remapped PPI interrupts for local timer
ARM: tegra: use remapped PPI interrupts for local timer
ARM: msm: use remapped PPI interrupts for local timer
ARM: exynos4: use remapped PPI interrupts for local timer
ARM: gic: remove previous local timer interrupt handling
ARM: gic: add compute_irqnr macro for exynos4
ARM: SMP: automatically select ARM_GIC_PPI_MAP
ARM: gic: add gic_ppi_map_on_cpu()

 arch/arm/Kconfig                                  |    1 +
 arch/arm/common/Kconfig                           |    5 +
 arch/arm/common/gic.c                             |  152 +++++++++++++++++++--
 arch/arm/include/asm/entry-macro-multi.S          |    7 -
 arch/arm/include/asm/hardirq.h                    |    3 -
 arch/arm/include/asm/hardware/entry-macro-gic.S   |   31 ++---
 arch/arm/include/asm/hardware/gic.h               |   11 ++-
 arch/arm/include/asm/localtimer.h                 |    7 +-
 arch/arm/include/asm/smp.h                        |    5 -
 arch/arm/kernel/irq.c                             |   11 +-
 arch/arm/kernel/smp.c                             |   27 +---
 arch/arm/kernel/smp_twd.c                         |   25 +++-
 arch/arm/mach-exynos4/include/mach/entry-macro.S  |   70 +---------
 arch/arm/mach-exynos4/localtimer.c                |    3 +-
 arch/arm/mach-msm/board-msm8x60.c                 |   11 --
 arch/arm/mach-msm/include/mach/entry-macro-qgic.S |   73 +----------
 arch/arm/mach-msm/timer.c                         |   58 ++++----
 arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--

 arch/arm/mach-omap2/timer-mpu.c                   |    3 +-
 arch/arm/mach-shmobile/entry-intc.S               |    3 -
 arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
 arch/arm/mach-shmobile/localtimer.c               |    3 +-
 arch/arm/mach-tegra/localtimer.c                  |    3 +-
 arch/arm/mach-ux500/localtimer.c                  |    3 +-
 arch/arm/plat-versatile/localtimer.c              |    3 +-
 25 files changed, 253 insertions(+), 282 deletions(-)

Quoted 108 lines · Marc Zyngier

The current GIC per-cpu interrupts (aka PPIs) suffer from a number of problems:

- They use a completely separate scheme to handle the interrupts, mostly because the PPI concept doesn't really match the kernel view of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level assembly code.
- The local timer code can only be used by devices generating PPIs, and not SPIs.
- At least one platform (msm) has started implementing its own alternative scheme.
- Some low-level code gets duplicated, as usual...

The proposed solution is to let the GIC code expose the PPIs as something that the kernel can manage. Instead of having a single interrupt number shared on all cores, make the interrupt number be different on each CPU.

This enables the use of the normal kernel API (request_irq() and friends) and the elimination of some low level code. It also serves as the basis of the GIC device tree support code. On the other side, it causes quite a bit of churn in the timer code.

Previous versions of this series have received ACKs from some platform maintainers, but so far there has been very little comment on the core code. As I now have quite a large number of patches relying on this series (local timers as devices, device tree support for GIC and TWD), I'd be grateful for at least a hint indicating that this is going in the right direction. Or not.

This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2 (Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git

From v7:
- Rebased on v3.0-rc5
- Rename gic_ppi_to_vppi() to gic_ppi_map()
- Rename CONFIG_ARM_GIC_VPPI to CONFIG_ARM_GIC_PPI_MAP
- Add gic_ppi_map_on_cpu()

From v6:
- Fix compilation when !CONFIG_PM.

From v5:
- Update Exynos4 MCT to use the local timer infrastructure, now that it doesn't depend on PPIs anymore. Untested, as the board I have doesn't implement MCT (EVT0 syndrome).

From v4:
- Fix arch/arm/common/Kconfig indentation
- Fix MSM warning when building UP.

From v3:
- Fix yet another CPU_HOTPLUG problem: instead of calling free_irq() on timer halt, just disable the interrupt. On timer restart, detect that the interrupt has been requested already and just enable the interrupt back. This avoid doing a request_irq() with preemption disabled, which triggers an ugly warning. Reported by Stephen Boyd.

From v2:
- More MSM fixes
- Dropped MSM interrupt handler removal, as this requires more discussion, and will be addressed in a separate series.
- Moved introduction of percpu_timer_handler to a separate patch

From v1:
- Fix crash while hotpluging a CPU
- Use handle_percpu_irq() instead of handle_fasteoi_irq()
- MSM fixes courtesy of Stephen Boyd
- MSM switched to percpu_timer_handler()
- Remove local timer interrupt accounting
- Restructure patches #1 and #2

Marc Zyngier (14):
ARM: gic: add per-cpu interrupt mapping
ARM: smp: add interrupt handler for local timers
ARM: smp_twd: add support for remapped PPI interrupts
ARM: omap4: use remapped PPI interrupts for local timer
ARM: versatile: use remapped PPI interrupts for local timer
ARM: shmobile: use remapped PPI interrupts for local timer
ARM: ux500: use remapped PPI interrupts for local timer
ARM: tegra: use remapped PPI interrupts for local timer
ARM: msm: use remapped PPI interrupts for local timer
ARM: exynos4: use remapped PPI interrupts for local timer
ARM: gic: remove previous local timer interrupt handling
ARM: gic: add compute_irqnr macro for exynos4
ARM: SMP: automatically select ARM_GIC_PPI_MAP
ARM: gic: add gic_ppi_map_on_cpu()

 arch/arm/Kconfig                                  |    1 +
 arch/arm/common/Kconfig                           |    5 +
 arch/arm/common/gic.c                             |  152 +++++++++++++++++++--
 arch/arm/include/asm/entry-macro-multi.S          |    7 -
 arch/arm/include/asm/hardirq.h                    |    3 -
 arch/arm/include/asm/hardware/entry-macro-gic.S   |   31 ++---
 arch/arm/include/asm/hardware/gic.h               |   11 ++-
 arch/arm/include/asm/localtimer.h                 |    7 +-
 arch/arm/include/asm/smp.h                        |    5 -
 arch/arm/kernel/irq.c                             |   11 +-
 arch/arm/kernel/smp.c                             |   27 +---
 arch/arm/kernel/smp_twd.c                         |   25 +++-
 arch/arm/mach-exynos4/include/mach/entry-macro.S  |   70 +---------
 arch/arm/mach-exynos4/localtimer.c                |    3 +-
 arch/arm/mach-msm/board-msm8x60.c                 |   11 --
 arch/arm/mach-msm/include/mach/entry-macro-qgic.S |   73 +----------
 arch/arm/mach-msm/timer.c                         |   58 ++++----
 arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--

Quoted 8 lines

 arch/arm/mach-omap2/timer-mpu.c                   |    3 +-
 arch/arm/mach-shmobile/entry-intc.S               |    3 -
 arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
 arch/arm/mach-shmobile/localtimer.c               |    3 +-
 arch/arm/mach-tegra/localtimer.c                  |    3 +-
 arch/arm/mach-ux500/localtimer.c                  |    3 +-
 arch/arm/plat-versatile/localtimer.c              |    3 +-
 25 files changed, 253 insertions(+), 282 deletions(-)

Russell,

Unless you have any objection, I'd like to push this to Arnd's arm-soc tree. Would that be OK with you?

Quoted 120 lines · Marc Zyngier

Quoted 108 lines · Marc Zyngier

The current GIC per-cpu interrupts (aka PPIs) suffer from a number of problems:

- They use a completely separate scheme to handle the interrupts, mostly because the PPI concept doesn't really match the kernel view of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level assembly code.
- The local timer code can only be used by devices generating PPIs, and not SPIs.
- At least one platform (msm) has started implementing its own alternative scheme.
- Some low-level code gets duplicated, as usual...

The proposed solution is to let the GIC code expose the PPIs as something that the kernel can manage. Instead of having a single interrupt number shared on all cores, make the interrupt number be different on each CPU.

This enables the use of the normal kernel API (request_irq() and friends) and the elimination of some low level code. It also serves as the basis of the GIC device tree support code. On the other side, it causes quite a bit of churn in the timer code.

Previous versions of this series have received ACKs from some platform maintainers, but so far there has been very little comment on the core code. As I now have quite a large number of patches relying on this series (local timers as devices, device tree support for GIC and TWD), I'd be grateful for at least a hint indicating that this is going in the right direction. Or not.

This patch set is based on 3.0-rc5. Tested on PB-11MP, Tegra2 (Harmony) and SMDK-V310. Also available in the "gic_ppi_map" branch of
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git

From v7:
- Rebased on v3.0-rc5
- Rename gic_ppi_to_vppi() to gic_ppi_map()
- Rename CONFIG_ARM_GIC_VPPI to CONFIG_ARM_GIC_PPI_MAP
- Add gic_ppi_map_on_cpu()

From v6:
- Fix compilation when !CONFIG_PM.

From v5:
- Update Exynos4 MCT to use the local timer infrastructure, now that it doesn't depend on PPIs anymore. Untested, as the board I have doesn't implement MCT (EVT0 syndrome).

From v4:
- Fix arch/arm/common/Kconfig indentation
- Fix MSM warning when building UP.

From v3:
- Fix yet another CPU_HOTPLUG problem: instead of calling free_irq() on timer halt, just disable the interrupt. On timer restart, detect that the interrupt has been requested already and just enable the interrupt back. This avoid doing a request_irq() with preemption disabled, which triggers an ugly warning. Reported by Stephen Boyd.

From v2:
- More MSM fixes
- Dropped MSM interrupt handler removal, as this requires more discussion, and will be addressed in a separate series.
- Moved introduction of percpu_timer_handler to a separate patch

From v1:
- Fix crash while hotpluging a CPU
- Use handle_percpu_irq() instead of handle_fasteoi_irq()
- MSM fixes courtesy of Stephen Boyd
- MSM switched to percpu_timer_handler()
- Remove local timer interrupt accounting
- Restructure patches #1 and #2

Marc Zyngier (14):
ARM: gic: add per-cpu interrupt mapping
ARM: smp: add interrupt handler for local timers
ARM: smp_twd: add support for remapped PPI interrupts
ARM: omap4: use remapped PPI interrupts for local timer
ARM: versatile: use remapped PPI interrupts for local timer
ARM: shmobile: use remapped PPI interrupts for local timer
ARM: ux500: use remapped PPI interrupts for local timer
ARM: tegra: use remapped PPI interrupts for local timer
ARM: msm: use remapped PPI interrupts for local timer
ARM: exynos4: use remapped PPI interrupts for local timer
ARM: gic: remove previous local timer interrupt handling
ARM: gic: add compute_irqnr macro for exynos4
ARM: SMP: automatically select ARM_GIC_PPI_MAP
ARM: gic: add gic_ppi_map_on_cpu()

 arch/arm/Kconfig                                  |    1 +
 arch/arm/common/Kconfig                           |    5 +
 arch/arm/common/gic.c                             |  152 +++++++++++++++++++--
 arch/arm/include/asm/entry-macro-multi.S          |    7 -
 arch/arm/include/asm/hardirq.h                    |    3 -
 arch/arm/include/asm/hardware/entry-macro-gic.S   |   31 ++---
 arch/arm/include/asm/hardware/gic.h               |   11 ++-
 arch/arm/include/asm/localtimer.h                 |    7 +-
 arch/arm/include/asm/smp.h                        |    5 -
 arch/arm/kernel/irq.c                             |   11 +-
 arch/arm/kernel/smp.c                             |   27 +---
 arch/arm/kernel/smp_twd.c                         |   25 +++-
 arch/arm/mach-exynos4/include/mach/entry-macro.S  |   70 +---------
 arch/arm/mach-exynos4/localtimer.c                |    3 +-
 arch/arm/mach-msm/board-msm8x60.c                 |   11 --
 arch/arm/mach-msm/include/mach/entry-macro-qgic.S |   73 +----------
 arch/arm/mach-msm/timer.c                         |   58 ++++----
 arch/arm/mach-omap2/include/mach/entry-macro.S    |   14 +--

Quoted 8 lines

 arch/arm/mach-omap2/timer-mpu.c                   |    3 +-
 arch/arm/mach-shmobile/entry-intc.S               |    3 -
 arch/arm/mach-shmobile/include/mach/entry-macro.S |    3 -
 arch/arm/mach-shmobile/localtimer.c               |    3 +-
 arch/arm/mach-tegra/localtimer.c                  |    3 +-
 arch/arm/mach-ux500/localtimer.c                  |    3 +-
 arch/arm/plat-versatile/localtimer.c              |    3 +-
 25 files changed, 253 insertions(+), 282 deletions(-)

Russell,

Quoted 2 lines

Unless you have any objection, I'd like to push this to Arnd's arm-soc tree. Would that be OK with you?

I've yet to look at any of these patches in any depth - my immediate concern is gobbling up soo many IRQ numbers for the per-cpu stuff (which also eats up memory for all the irq structures) when this isn't really necessary.

Plus is this patch set core stuff or SoC stuff? It looks more core to me, so that means it should come through my tree rather than Arnd's.

Plus is this patch set core stuff or SoC stuff? It looks more core to me, so that means it should come through my tree rather than Arnd's.