Consolidating GIC per-cpu interrupts
Subsystems (6)
v11 cover Consolidating GIC per-cpu interrupts
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...
As the previous solution which tried to map PPIs to normal interrupts has been proved to be buggy, I've opted to a much simpler scheme (based on Russell's input).
The proposed solution is to handle the interrupt using the same path as SPIs, with a common handler for all PPIs. Each PPI can be requested using gic_request_ppi(), similar to request_irq(). The local timer code is updated to reflect these changes.
Patches against v3.1-rc1 + Will Deacon's TWD patch ("ARM: twd:
register clockevents device before enabling PPI"). Tested on PB11MP, VE, OMAP4 (Panda) and Tegra (Harmony). As this patch series is quite different from the previous one, I've dropped all previous acks from platform maintainers.
PATCH v11 1/4 ARM: gic: consolidate PPI handling
PPI handling is a bit of an odd beast. It uses its own low level handling code and is hardwired to the local timers (hence lacking a registration interface).
Instead, switch the low handling to the normal SPI handling code.
This also allows the removal of some duplicated code.
--- arch/arm/common/gic.c | 69 +++++++++++++++++++- 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 | 19 +----- arch/arm/include/asm/localtimer.h | 6 +- arch/arm/include/asm/smp.h | 5 -- arch/arm/kernel/irq.c | 3 - arch/arm/kernel/smp.c | 27 ++------ arch/arm/mach-exynos4/include/mach/entry-macro.S | 6 +-- arch/arm/mach-msm/board-msm8x60.c | 11 --- arch/arm/mach-msm/include/mach/entry-macro-qgic.S | 73 +-------------------- arch/arm/mach-omap2/include/mach/entry-macro.S | 14 +---- arch/arm/mach-shmobile/entry-intc.S | 3 - arch/arm/mach-shmobile/include/mach/entry-macro.S | 3 - 14 files changed, 82 insertions(+), 167 deletions(-)
arch/arm/common/gic.c +68 -1
@@ -28,10 +28,14 @@#include <linux/smp.h> #include <linux/cpumask.h> #include <linux/io.h> +#include <linux/interrupt.h> +#include <linux/percpu.h> +#include <linux/slab.h> #include <asm/irq.h> #include <asm/mach/irq.h> #include <asm/hardware/gic.h> +#include <asm/localtimer.h> static DEFINE_SPINLOCK (irq_controller_lock);
@@ -255,12 +259,39 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)irq_set_chained_handler (irq, gic_handle_cascade_irq); } +#ifdef CONFIG_LOCAL_TIMERS +#define gic_ppi_handler percpu_timer_handler +#else +static irqreturn_t gic_ppi_handler (int irq,void *dev_id) +{ +return IRQ_NONE ; +} +#endif + +#define PPI_IRQACT (nr) \ + { \ + .handler = gic_ppi_handler, \ + .flags = IRQF_PERCPU | IRQF_TIMER, \ + .irq = nr, \ + .name = "PPI-" # nr, \ + } + +static struct irqaction ppi_irqaction_template[16 ]__initdata = { +PPI_IRQACT (0 ),PPI_IRQACT (1 ),PPI_IRQACT (2 ),PPI_IRQACT (3 ), +PPI_IRQACT (4 ),PPI_IRQACT (5 ),PPI_IRQACT (6 ),PPI_IRQACT (7 ), +PPI_IRQACT (8 ),PPI_IRQACT (9 ),PPI_IRQACT (10 ),PPI_IRQACT (11 ), +PPI_IRQACT (12 ),PPI_IRQACT (13 ),PPI_IRQACT (14 ),PPI_IRQACT (15 ), +}; + +static struct irqaction *ppi_irqaction; +static void __init gic_dist_init (struct gic_chip_data *gic,unsigned int irq_start) {unsigned int gic_irqs, irq_limit, i;void __iomem *base = gic->dist_base;u32 cpumask =1 <<smp_processor_id (); +u32 nrppis =0 , ppi_base =0 ; cpumask |= cpumask <<8 ; cpumask |= cpumask <<16 ;
@@ -277,6 +308,28 @@ static void __init gic_dist_init(struct gic_chip_data *gic, gic_irqs =1020 ;/* +* Nobody would be insane enough to use PPIs on a secondary +* GIC, right? +*/ +if (gic == &gic_data[0 ]) { + nrppis =16 - (irq_start &15 ); + ppi_base = gic->irq_offset +32 - nrppis; + + ppi_irqaction =kmemdup (&ppi_irqaction_template[16 - nrppis], +sizeof (*ppi_irqaction) * nrppis, +GFP_KERNEL ); + +if (nrppis && !ppi_irqaction) { +pr_err ("GIC: Can't allocate PPI memory" ); + nrppis =0 ; + ppi_base =0 ; + } + } + +pr_info ("Configuring GIC with %d sources (%d PPIs)\n" , + gic_irqs, (gic == &gic_data[0 ]) ? nrppis :0 ); + +/* * Set all global interrupts to be level triggered, active low. */ for (i =32 ; i < gic_irqs; i +=16 )
@@ -311,7 +364,21 @@ static void __init gic_dist_init(struct gic_chip_data *gic,/* * Setup the Linux IRQ subsystem. */ -for (i = irq_start; i < irq_limit; i++) { +for (i =0 ; i < nrppis; i++) { +int ppi = i + ppi_base; +int err; + +irq_set_chip_and_handler (ppi, &gic_chip, handle_percpu_irq); +irq_set_chip_data (ppi, gic); +irq_set_status_flags (ppi,IRQ_NOAUTOEN |IRQ_NOPROBE ); + + err =setup_irq (ppi, &ppi_irqaction[i]); +if (err) +pr_err ("GIC: can't setup IRQ%d PPI%d (%d)\n" , + i, ppi, err); + } + +for (i = irq_start + nrppis; i < irq_limit; i++) {irq_set_chip_and_handler (i, &gic_chip, handle_fasteoi_irq);irq_set_chip_data (i, gic);set_irq_flags (i,IRQF_VALID |IRQF_PROBE );
arch/arm/include/asm/entry-macro-multi.S +0 -7
@@ -25,13 +25,6 @@ movne r1, sp adrne lr, BSYM(1b) bne do_IPI - -#ifdef CONFIG_LOCAL_TIMERS - test_for_ltirq r0, r2, r6, lr - movne r0, sp - adrne lr, BSYM(1b) - bne do_local_timer -#endif #endif 9997: .endm
arch/arm/include/asm/hardirq.h +0 -3
@@ -9,9 +9,6 @@ typedef struct { unsigned int __softirq_pending; -#ifdef CONFIG_LOCAL_TIMERS - unsigned int local_timer_irqs; -#endif #ifdef CONFIG_SMP unsigned int ipi_irqs[NR_IPI]; #endif
arch/arm/include/asm/hardware/entry-macro-gic.S +2 -17
@@ -22,15 +22,11 @@ * interrupt controller spec. To wit: * * Interrupts 0-15 are IPI - * 16-28 are reserved - * 29-31 are local. We allow 30 to be used for the watchdog. + * 16-31 are local. We allow 30 to be used for the watchdog. * 32-1020 are global * 1021-1022 are reserved * 1023 is "spurious" (no interrupt) * - * For now, we ignore all local interrupts so only return an interrupt if it's - * between 30 and 1020. The test_for_ipi routine below will pick up on IPIs. - * * A simple read from the controller will tell us the number of the highest * priority enabled interrupt. We then just need to check whether it is in the * valid range for an IRQ (30-1020 inclusive).
@@ -43,7 +39,7 @@ ldr \tmp, =1021 bic \irqnr, \irqstat, #0x1c00 - cmp \irqnr, #29 + cmp \irqnr, #15 cmpcc \irqnr, \irqnr cmpne \irqnr, \tmp cmpcs \irqnr, \irqnr
@@ -62,14 +58,3 @@ strcc \irqstat, [\base, #GIC_CPU_EOI] cmpcs \irqnr, \irqnr .endm - -/* As above, this assumes that irqstat and base are preserved.. */ - - .macro test_for_ltirq, irqnr, irqstat, base, tmp - bic \irqnr, \irqstat, #0x1c00 - mov \tmp, #0 - cmp \irqnr, #29 - moveq \tmp, #1 - streq \irqstat, [\base, #GIC_CPU_EOI] - cmp \tmp, #0 - .endm
arch/arm/include/asm/localtimer.h +4 -2
@@ -10,6 +10,8 @@#ifndef __ASM_ARM_LOCALTIMER_H#define __ASM_ARM_LOCALTIMER_H +#include <linux/interrupt.h> +struct clock_event_device ;/*
@@ -18,9 +20,9 @@ struct clock_event_device;void percpu_timer_setup (void );/* -* Called from assembly, this is the local timer IRQ handler +* Per-cpu timer IRQ handler */ -asmlinkage void do_local_timer(struct pt_regs *); +irqreturn_t percpu_timer_handler (int irq,void *dev_id);#ifdef CONFIG_LOCAL_TIMERS
arch/arm/include/asm/smp.h +0 -5
@@ -88,9 +88,4 @@ extern void platform_cpu_enable(unsigned int cpu);extern void arch_send_call_function_single_ipi (int cpu);extern void arch_send_call_function_ipi_mask (const struct cpumask *mask); -/* -* show local interrupt info -*/ -extern void show_local_irqs (struct seq_file *,int ); -#endif /* ifndef __ASM_ARM_SMP_H */
arch/arm/kernel/irq.c +0 -3
@@ -59,9 +59,6 @@ int arch_show_interrupts(struct seq_file *p, int prec)#ifdef CONFIG_SMP show_ipi_list (p, prec);#endif -#ifdef CONFIG_LOCAL_TIMERS -show_local_irqs (p, prec); -#endif seq_printf (p,"%*s: %10lu\n" , prec,"Err" , irq_err_count);return 0 ; }
arch/arm/kernel/smp.c +5 -22
@@ -453,10 +453,6 @@ u64 smp_irq_stat_cpu(unsigned int cpu)for (i =0 ; i <NR_IPI ; i++) sum +=__get_irq_stat (cpu, ipi_irqs[i]); -#ifdef CONFIG_LOCAL_TIMERS - sum +=__get_irq_stat (cpu, local_timer_irqs); -#endif -return sum; }
@@ -474,29 +470,16 @@ static void ipi_timer(void) }#ifdef CONFIG_LOCAL_TIMERS -asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs) +irqreturn_t percpu_timer_handler (int irq,void *dev_id) { -struct pt_regs *old_regs =set_irq_regs (regs); -int cpu =smp_processor_id (); +struct clock_event_device *evt = &__get_cpu_var (percpu_clockevent);if (local_timer_ack ()) { -__inc_irq_stat (cpu, local_timer_irqs); -ipi_timer (); + evt->event_handler (evt); +return IRQ_HANDLED ; } -set_irq_regs (old_regs); -} - -void show_local_irqs (struct seq_file *p,int prec) -{ -unsigned int cpu; - -seq_printf (p,"%*s: " , prec,"LOC" ); - -for_each_present_cpu (cpu) -seq_printf (p,"%10u " ,__get_irq_stat (cpu, local_timer_irqs)); - -seq_printf (p," Local timer interrupts\n" ); +return IRQ_NONE ; }#endif
arch/arm/mach-exynos4/include/mach/entry-macro.S +1 -5
@@ -55,7 +55,7 @@ bic \irqnr, \irqstat, #0x1c00 - cmp \irqnr, #29 + cmp \irqnr, #15 cmpcc \irqnr, \irqnr cmpne \irqnr, \tmp cmpcs \irqnr, \irqnr
@@ -77,7 +77,3 @@ cmpcs \irqnr, \irqnr .endm - /* As above, this assumes that irqstat and base are preserved.. */ - - .macro test_for_ltirq, irqnr, irqstat, base, tmp - .endm
arch/arm/mach-msm/board-msm8x60.c +0 -11
@@ -36,8 +36,6 @@ static void __init msm8x60_map_io(void)static void __init msm8x60_init_irq (void ) { -unsigned int i; -gic_init (0 ,GIC_PPI_START ,MSM_QGIC_DIST_BASE , (void *)MSM_QGIC_CPU_BASE );
@@ -49,15 +47,6 @@ static void __init msm8x60_init_irq(void)*/ if (!machine_is_msm8x60_sim ())writel (0x0000FFFF ,MSM_QGIC_DIST_BASE +GIC_DIST_ENABLE_SET ); - -/* FIXME: Not installing AVS_SVICINT and AVS_SVICINTSWDONE yet -* as they are configured as level, which does not play nice with -* handle_percpu_irq. -*/ -for (i =GIC_PPI_START ; i <GIC_SPI_START ; i++) { -if (i !=AVS_SVICINT && i !=AVS_SVICINTSWDONE ) -irq_set_handler (i, handle_percpu_irq); - } }static void __init msm8x60_init (void )
arch/arm/mach-msm/include/mach/entry-macro-qgic.S +1 -72
82 lines not on this page · raw message
arch/arm/mach-omap2/include/mach/entry-macro.S +1 -13
@@ -78,7 +78,7 @@ 4401: ldr \irqstat, [\base, #GIC_CPU_INTACK] ldr \tmp, =1021 bic \irqnr, \irqstat, #0x1c00 - cmp \irqnr, #29 + cmp \irqnr, #15 cmpcc \irqnr, \irqnr cmpne \irqnr, \tmp cmpcs \irqnr, \irqnr
@@ -101,18 +101,6 @@ it cs cmpcs \irqnr, \irqnr .endm - - /* As above, this assumes that irqstat and base are preserved */ - - .macro test_for_ltirq, irqnr, irqstat, base, tmp - bic \irqnr, \irqstat, #0x1c00 - mov \tmp, #0 - cmp \irqnr, #29 - itt eq - moveq \tmp, #1 - streq \irqstat, [\base, #GIC_CPU_EOI] - cmp \tmp, #0 - .endm #endif /* CONFIG_SMP */ #else /* MULTI_OMAP2 */
arch/arm/mach-shmobile/entry-intc.S +0 -3
@@ -51,7 +51,4 @@ .macro test_for_ipi, irqnr, irqstat, base, tmp .endm - .macro test_for_ltirq, irqnr, irqstat, base, tmp - .endm - arch_irq_handler shmobile_handle_irq_intc
arch/arm/mach-shmobile/include/mach/entry-macro.S +0 -3
@@ -27,8 +27,5 @@ .macro test_for_ipi, irqnr, irqstat, base, tmp .endm - .macro test_for_ltirq, irqnr, irqstat, base, tmp - .endm - .macro arch_ret_to_user, tmp1, tmp2 .endm
Signature · 1 line
1.7.0.4
PATCH v11 2/4 ARM: gic: Add PPI registration interface
In reply to Marc ZyngierThis patch makes it possible to request (and free) a PPI. It thus makes it useable for more than just the local timers.
Update TWD and MSM timers to use that functionnality.
Based on an earlier patch by Russell King.
--- arch/arm/common/gic.c | 94 ++++++++++++++++++++++++++++------ arch/arm/include/asm/hardware/gic.h | 5 ++- arch/arm/include/asm/localtimer.h | 11 ++++- arch/arm/include/asm/smp_twd.h | 1 + arch/arm/kernel/smp.c | 4 +- arch/arm/kernel/smp_twd.c | 15 +++++- arch/arm/mach-msm/timer.c | 56 +++++++++++---------- 7 files changed, 137 insertions(+), 49 deletions(-)
arch/arm/common/gic.c +77 -17
@@ -35,7 +35,6 @@#include <asm/irq.h> #include <asm/mach/irq.h> #include <asm/hardware/gic.h> -#include <asm/localtimer.h> static DEFINE_SPINLOCK (irq_controller_lock);
@@ -259,14 +258,25 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)irq_set_chained_handler (irq, gic_handle_cascade_irq); } -#ifdef CONFIG_LOCAL_TIMERS -#define gic_ppi_handler percpu_timer_handler -#else +struct gic_action { +irq_handler_t handler; +void *data; +}; + +static DEFINE_PER_CPU(struct gic_action *, gic_ppi_action); +static unsigned int gic_nr_ppis, gic_ppi_base; +static irqreturn_t gic_ppi_handler (int irq,void *dev_id) { +unsigned int ppi = irq - gic_ppi_base; +struct gic_action *act; + + act = &__get_cpu_var (gic_ppi_action)[ppi]; +if (likely (act->handler)) +return act->handler (irq, act->data); +return IRQ_NONE ; } -#endif #define PPI_IRQACT (nr) \ { \
@@ -291,6 +301,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,unsigned int gic_irqs, irq_limit, i;void __iomem *base = gic->dist_base;u32 cpumask =1 <<smp_processor_id (); +u32 dist_ctr, nrcpus;u32 nrppis =0 , ppi_base =0 ; cpumask |= cpumask <<8 ;
@@ -302,11 +313,14 @@ static void __init gic_dist_init(struct gic_chip_data *gic,* Find out how many interrupts are supported. * The GIC only supports up to 1020 interrupt sources. */ - gic_irqs =readl_relaxed (base +GIC_DIST_CTR ) &0x1f ; - gic_irqs = (gic_irqs +1 ) *32 ; + dist_ctr =readl_relaxed (base +GIC_DIST_CTR ); + gic_irqs = ((dist_ctr &0x1f ) +1 ) *32 ;if (gic_irqs >1020 ) gic_irqs =1020 ; +/* Find out how many CPUs are supported (8 max). */ + nrcpus = ((dist_ctr >>5 ) &7 ) +1 ; +/* * Nobody would be insane enough to use PPIs on a secondary * GIC, right?
@@ -324,6 +338,19 @@ static void __init gic_dist_init(struct gic_chip_data *gic, nrppis =0 ; ppi_base =0 ; } + +for (i =0 ; i < nrcpus; i++) { +struct gic_action **ppiacts; + + ppiacts = &per_cpu (gic_ppi_action, i); + *ppiacts =kzalloc (sizeof (*gic_ppi_action) * nrppis, +GFP_KERNEL ); +if (!*ppiacts) +pr_err ("GIC: Can't allocate PPI CPU%d memory" , i); + } + + gic_nr_ppis = nrppis; + gic_ppi_base = ppi_base; } pr_info("Configuring GIC with %d sources (%d PPIs)\n",
@@ -387,6 +414,49 @@ static void __init gic_dist_init(struct gic_chip_data *gic,writel_relaxed (1 , base +GIC_DIST_CTRL ); } +int gic_request_ppi (unsigned int irq,irq_handler_t handler,void *data) +{ +struct gic_action *act; +unsigned long flags; +unsigned int ppi = irq - gic_ppi_base; +int ret = -EBUSY ; + +if (ppi >= gic_nr_ppis) +return -EINVAL ; + +local_irq_save (flags); + act = &__get_cpu_var (gic_ppi_action)[ppi]; +if (!act->handler) { + act->handler = handler; + act->data = data; + +gic_unmask_irq (irq_get_irq_data (irq)); + ret =0 ; + } +local_irq_restore (flags); + +return ret; +} + +void gic_free_ppi (unsigned int irq,void *data) +{ +struct gic_action *act; +unsigned long flags; +unsigned int ppi = irq - gic_ppi_base; + +if (ppi >= gic_nr_ppis) +return ; + +local_irq_save (flags); + act = &__get_cpu_var (gic_ppi_action)[ppi]; +if (act->data == data) { +gic_mask_irq (irq_get_irq_data (irq)); + act->handler =NULL ; + act->data =NULL ; + } +local_irq_restore (flags); +} +static void __cpuinit gic_cpu_init (struct gic_chip_data *gic) {void __iomem *dist_base = gic->dist_base;
@@ -436,16 +506,6 @@ void __cpuinit gic_secondary_init(unsigned int gic_nr)gic_cpu_init (&gic_data[gic_nr]); } -void __cpuinit gic_enable_ppi (unsigned int irq) -{ -unsigned long flags; - -local_irq_save (flags); -irq_set_status_flags (irq,IRQ_NOPROBE ); -gic_unmask_irq (irq_get_irq_data (irq)); -local_irq_restore (flags); -} -#ifdef CONFIG_SMP void gic_raise_softirq (const struct cpumask *mask,unsigned int irq) {
arch/arm/include/asm/hardware/gic.h +4 -1
@@ -33,6 +33,8 @@#define GIC_DIST_SOFTINT 0xf00#ifndef __ASSEMBLY__ +#include <linux/interrupt.h> +extern void __iomem *gic_cpu_base_addr;extern struct irq_chip gic_arch_extn;
@@ -40,7 +42,8 @@ void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);void gic_secondary_init (unsigned int );void gic_cascade_irq (unsigned int gic_nr,unsigned int irq);void gic_raise_softirq (const struct cpumask *mask,unsigned int irq); -void gic_enable_ppi (unsigned int ); +int gic_request_ppi (unsigned int irq,irq_handler_t handler,void *data); +void gic_free_ppi (unsigned int irq,void *data);struct gic_chip_data {unsigned int irq_offset;
arch/arm/include/asm/localtimer.h +10 -1
@@ -24,7 +24,6 @@ void percpu_timer_setup(void);*/ irqreturn_t percpu_timer_handler (int irq,void *dev_id); -#ifdef CONFIG_LOCAL_TIMERS #ifdef CONFIG_HAVE_ARM_TWD
@@ -32,6 +31,7 @@ irqreturn_t percpu_timer_handler(int irq, void *dev_id);#include "smp_twd.h" #define local_timer_ack () twd_timer_ack() +#define local_timer_stop (c) twd_timer_stop((c))#else
@@ -41,6 +41,11 @@ irqreturn_t percpu_timer_handler(int irq, void *dev_id);*/ int local_timer_ack (void ); +/* +* Stop the local timer +*/ +void local_timer_stop (struct clock_event_device *); +#endif /*
@@ -54,6 +59,10 @@ static inline int local_timer_setup(struct clock_event_device *evt) {return -ENXIO ; } + +static inline void local_timer_stop (struct clock_event_device *evt) +{ +}#endif #endif
arch/arm/include/asm/smp_twd.h +1 -0
@@ -24,5 +24,6 @@ extern void __iomem *twd_base;int twd_timer_ack (void );void twd_timer_setup (struct clock_event_device *); +void twd_timer_stop (struct clock_event_device *);#endif
arch/arm/kernel/smp.c +2 -2
@@ -472,7 +472,7 @@ static void ipi_timer(void)#ifdef CONFIG_LOCAL_TIMERS irqreturn_t percpu_timer_handler (int irq,void *dev_id) { -struct clock_event_device *evt = &__get_cpu_var (percpu_clockevent); +struct clock_event_device *evt = dev_id;if (local_timer_ack ()) { evt->event_handler (evt);
@@ -533,7 +533,7 @@ 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); + local_timer_stop(evt); }#endif
arch/arm/kernel/smp_twd.c +13 -2
@@ -19,6 +19,7 @@#include <linux/io.h> #include <asm/smp_twd.h> +#include <asm/localtimer.h> #include <asm/hardware/gic.h> /* set up by the platform code */
@@ -80,6 +81,12 @@ int twd_timer_ack(void)return 0 ; } +void twd_timer_stop (struct clock_event_device *clk) +{ +twd_set_mode (CLOCK_EVT_MODE_UNUSED , clk); +gic_free_ppi (clk->irq, clk); +} +static void __cpuinit twd_calibrate_rate (void ) {unsigned long count;
@@ -124,6 +131,8 @@ static void __cpuinit twd_calibrate_rate(void)*/ void __cpuinit twd_timer_setup (struct clock_event_device *clk) { +int err; +twd_calibrate_rate (); clk->name ="local_timer" ;
@@ -139,6 +148,8 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)clockevents_register_device (clk); -/* Make sure our local interrupt controller has this enabled */ -gic_enable_ppi (clk->irq); + err =gic_request_ppi (clk->irq, percpu_timer_handler, clk); +if (err) +pr_err ("%s: can't register interrupt %d on cpu %d (%d)\n" , + clk->name, clk->irq,smp_processor_id (), err); }
arch/arm/mach-msm/timer.c +30 -26
119 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH v11 3/4 ARM: local timers: drop local_timer_ack()
In reply to Marc ZyngierOnly TWD uses local_timer_ack() to perform something useful. Make it the real (private) interrupt handler, remove the common interrupt handler as well as the useless stubs from MCT and MSM.
--- arch/arm/include/asm/localtimer.h | 12 ------------ arch/arm/include/asm/smp_twd.h | 1 - arch/arm/kernel/smp.c | 14 -------------- arch/arm/kernel/smp_twd.c | 17 +++++++---------- arch/arm/mach-exynos4/mct.c | 6 ------ arch/arm/mach-msm/timer.c | 6 ------ 6 files changed, 7 insertions(+), 49 deletions(-)
arch/arm/include/asm/localtimer.h +0 -12
@@ -19,29 +19,17 @@ struct clock_event_device;*/ void percpu_timer_setup (void ); -/* -* Per-cpu timer IRQ handler -*/ -irqreturn_t percpu_timer_handler (int irq,void *dev_id); -#ifdef CONFIG_LOCAL_TIMERS #ifdef CONFIG_HAVE_ARM_TWD #include "smp_twd.h" -#define local_timer_ack () twd_timer_ack()#define local_timer_stop (c) twd_timer_stop((c))#else /* -* Platform provides this to acknowledge a local timer IRQ. -* Returns true if the local timer IRQ is to be processed. -*/ -int local_timer_ack (void ); - -/* * Stop the local timer */ void local_timer_stop (struct clock_event_device *);
arch/arm/include/asm/smp_twd.h +0 -1
@@ -22,7 +22,6 @@ struct clock_event_device;extern void __iomem *twd_base; -int twd_timer_ack (void );void twd_timer_setup (struct clock_event_device *);void twd_timer_stop (struct clock_event_device *);
arch/arm/kernel/smp.c +0 -14
@@ -469,20 +469,6 @@ static void ipi_timer(void)irq_exit (); } -#ifdef CONFIG_LOCAL_TIMERS -irqreturn_t percpu_timer_handler (int irq,void *dev_id) -{ -struct clock_event_device *evt = dev_id; - -if (local_timer_ack ()) { - evt->event_handler (evt); -return IRQ_HANDLED ; - } - -return IRQ_NONE ; -} -#endif -#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST static void smp_timer_broadcast (const struct cpumask *mask) {
arch/arm/kernel/smp_twd.c +7 -10
@@ -65,20 +65,17 @@ static int twd_set_next_event(unsigned long evt,return 0 ; } -/* -* local_timer_ack: checks for a local timer interrupt. -* -* If a local timer interrupt has occurred, acknowledge and return 1. -* Otherwise, return 0. -*/ -int twd_timer_ack (void ) +static irqreturn_t twd_timer_handler (int irq,void *dev_id) { +struct clock_event_device *clk = dev_id; +if (__raw_readl (twd_base +TWD_TIMER_INTSTAT )) {__raw_writel (1 , twd_base +TWD_TIMER_INTSTAT ); -return 1 ; + clk->event_handler (clk); +return IRQ_HANDLED ; } -return 0 ; +return IRQ_NONE ; }void twd_timer_stop (struct clock_event_device *clk)
@@ -148,7 +145,7 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)clockevents_register_device (clk); - err =gic_request_ppi (clk->irq, percpu_timer_handler, clk); + err =gic_request_ppi (clk->irq, twd_timer_handler, clk);if (err)pr_err ("%s: can't register interrupt %d on cpu %d (%d)\n" , clk->name, clk->irq,smp_processor_id (), err);
arch/arm/mach-exynos4/mct.c +0 -6
@@ -393,12 +393,6 @@ void __cpuinit local_timer_setup(struct clock_event_device *evt) {exynos4_mct_tick_init (evt); } - -int local_timer_ack (void ) -{ -return 0 ; -} -#endif /* CONFIG_LOCAL_TIMERS */ static void __init exynos4_timer_resources (void )
arch/arm/mach-msm/timer.c +0 -6
@@ -309,12 +309,6 @@ void local_timer_stop(struct clock_event_device *evt) evt->set_mode (CLOCK_EVT_MODE_UNUSED , evt);gic_free_ppi (evt->irq, evt); } - -inline int local_timer_ack (void ) -{ -return 1 ; -} -#endif struct sys_timer msm_timer = {
Signature · 1 line
1.7.0.4
PATCH v11 4/4 ARM: gic: add compute_irqnr macro for exynos4
In reply to Marc Zyngierexynos4 has a full copy of entry-macro-gic.S, just for the sake of an offset added to the IRQ number read from the GIC.
Add a compute_irqnr macro to entry-macro-gic.S so that any platform can add it's own hook without having to copy the whole file again.
--- arch/arm/include/asm/hardware/entry-macro-gic.S | 3 + arch/arm/mach-exynos4/include/mach/entry-macro.S | 63 ++------------------- 2 files changed, 9 insertions(+), 57 deletions(-)
arch/arm/include/asm/hardware/entry-macro-gic.S +3 -0
@@ -43,6 +43,9 @@ cmpcc \irqnr, \irqnr cmpne \irqnr, \tmp cmpcs \irqnr, \irqnr +#ifdef HAVE_GIC_BASE_OFFSET + compute_irqnr \irqnr, \tmp +#endif .endm /* We assume that irqstat (the raw value of the IRQ acknowledge
arch/arm/mach-exynos4/include/mach/entry-macro.S +6 -57
77 lines not on this page · raw message
Signature · 1 line
1.7.0.4
v11 cover Consolidating GIC per-cpu interrupts
In reply to Marc ZyngierThomas,
Could you _please_ look Marcs patch series and give an opinion on it. I've also attached my patch to this reply too, which is an alternative approach to Marcs.
Quoted 66 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...As the previous solution which tried to map PPIs to normal interrupts has been proved to be buggy, I've opted to a much simpler scheme (based on Russell's input).
The proposed solution is to handle the interrupt using the same path as SPIs, with a common handler for all PPIs. Each PPI can be requested using
gic_request_ppi(), similar torequest_irq(). The local timer code is updated to reflect these changes.Patches against v3.1-rc1 + Will Deacon's TWD patch ("ARM: twd:
register clockevents device before enabling PPI"). Tested on PB11MP, VE, OMAP4 (Panda) and Tegra (Harmony). As this patch series is quite different from the previous one, I've dropped all previous acks from platform maintainers.Quoted 1 line
From v10:
- Fixed exynos4 compilation
Quoted 1 line
From v9:
- Fixed PPI request in the MSM timer code
- Fixed UP/non-local-timer build
- Moved some bits and pieces from patch 1 to patch 2Marc Zyngier (4):
ARM: gic: consolidate PPI handling
ARM: gic: Add PPI registration interface
ARM: local timers: droplocal_timer_ack()
ARM: gic: addcompute_irqnrmacro for exynos4arch/arm/common/gic.c | 153 +++++++++++++++++++-- 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 | 22 +--- arch/arm/include/asm/hardware/gic.h | 5 +- arch/arm/include/asm/localtimer.h | 19 ++-- arch/arm/include/asm/smp.h | 5 - arch/arm/include/asm/smp_twd.h | 2 +- arch/arm/kernel/irq.c | 3 - arch/arm/kernel/smp.c | 33 +----- arch/arm/kernel/smp_twd.c | 30 +++-- arch/arm/mach-exynos4/include/mach/entry-macro.S | 67 +-------- arch/arm/mach-exynos4/mct.c | 6 - 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 | 56 ++++---- arch/arm/mach-omap2/include/mach/entry-macro.S | 14 +-- arch/arm/mach-shmobile/entry-intc.S | 3 - arch/arm/mach-shmobile/include/mach/entry-macro.S | 3 - 19 files changed, 214 insertions(+), 301 deletions(-)
Quoted 4 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-------------- next part --------------
arch/arm/common/gic.c | 70 ++++++++++++++++++++- arch/arm/include/asm/entry-macro-multi.S | 5 +- arch/arm/include/asm/hardware/entry-macro-gic.S | 17 +++-- arch/arm/include/asm/hardware/gic.h | 5 +- arch/arm/include/asm/localtimer.h | 14 +---- arch/arm/include/asm/smp_twd.h | 2 +- arch/arm/kernel/smp.c | 18 +----- arch/arm/kernel/smp_twd.c | 41 +++++++----- arch/arm/mach-exynos4/include/mach/entry-macro.S | 8 +- arch/arm/mach-msm/include/mach/entry-macro-qgic.S | 14 ++-- arch/arm/mach-msm/timer.c | 20 +++++- arch/arm/mach-omap2/include/mach/entry-macro.S | 9 +-- arch/arm/mach-shmobile/entry-intc.S | 2 +- arch/arm/mach-shmobile/include/mach/entry-macro.S | 2 +- 14 files changed, 144 insertions(+), 83 deletions(-)
arch/arm/common/gic.c +67 -3
@@ -27,12 +27,16 @@#include <linux/list.h> #include <linux/smp.h> #include <linux/cpumask.h> +#include <linux/interrupt.h> #include <linux/io.h> #include <asm/irq.h> #include <asm/mach/irq.h> #include <asm/hardware/gic.h> +#define GIC_FIRST_PPI 16 +#define NR_PPI 16 +static DEFINE_SPINLOCK (irq_controller_lock);/* Address of GIC 0 CPU interface */
@@ -376,14 +380,74 @@ void __cpuinit gic_secondary_init(unsigned int gic_nr)gic_cpu_init (&gic_data[gic_nr]); } -void __cpuinit gic_enable_ppi (unsigned int irq) +struct gic_action { +irq_handler_t handler; +void *data; +}; + +static DEFINE_PER_CPU(struct gic_action[NR_PPI], gic_ppi_action); + +asmlinkage void __exception_irq_entry gic_call_ppi(unsigned int irq, +struct pt_regs *regs) +{ +unsigned int ppi = irq -GIC_FIRST_PPI ; +struct gic_action *act; + + act = &__get_cpu_var (gic_ppi_action)[ppi]; +if (act->handler) { +struct pt_regs *old_regs =set_irq_regs (regs); + +/* FIXME: need to deal with PPI IRQ stats better.. */ +__inc_irq_stat (smp_processor_id (), local_timer_irqs); + +irq_enter (); + act->handler (irq, act->data); +irq_exit (); + +set_irq_regs (old_regs); + } +} + +int gic_request_ppi (unsigned int irq,irq_handler_t handler,void *data) { +struct gic_action *act;unsigned long flags; +unsigned int ppi = irq -GIC_FIRST_PPI ; +int ret = -EBUSY ; + +if (ppi >=NR_PPI ) +return -EINVAL ;local_irq_save (flags); -irq_set_status_flags (irq,IRQ_NOPROBE ); -gic_unmask_irq (irq_get_irq_data (irq)); + act = &__get_cpu_var (gic_ppi_action)[ppi]; +if (!act->handler) { + act->handler = handler; + act->data = data; + +irq_set_status_flags (irq,IRQ_NOPROBE ); +gic_unmask_irq (irq_get_irq_data (irq)); + }local_irq_restore (flags); + +return ret; +} + +void gic_free_ppi (unsigned int irq,void *data) +{ +struct gic_action *act; +unsigned long flags; +unsigned int ppi = irq -GIC_FIRST_PPI ; + +if (ppi <NR_PPI ) { +local_irq_save (flags); + act = &__get_cpu_var (gic_ppi_action)[ppi]; +if (act->data == data) { +gic_mask_irq (irq_get_irq_data (irq)); + act->handler =NULL ; + act->data =NULL ; + } +local_irq_restore (flags); + } }#ifdef CONFIG_SMP
arch/arm/include/asm/entry-macro-multi.S +1 -4
@@ -27,10 +27,7 @@ bne do_IPI #ifdef CONFIG_LOCAL_TIMERS - test_for_ltirq r0, r2, r6, lr - movne r0, sp - adrne lr, BSYM(1b) - bne do_local_timer + test_for_ppi r0, r2, r6, lr, sp, BSYM(1b) #endif #endif 9997:
arch/arm/include/asm/hardware/entry-macro-gic.S +10 -7
@@ -63,13 +63,16 @@ cmpcs \irqnr, \irqnr .endm -/* As above, this assumes that irqstat and base are preserved.. */ +/* + * We will have checked for normal IRQs (32+) and IPIs (0-15) so only + * PPIs are left here. + */ - .macro test_for_ltirq, irqnr, irqstat, base, tmp + .macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym bic \irqnr, \irqstat, #0x1c00 - mov \tmp, #0 - cmp \irqnr, #29 - moveq \tmp, #1 - streq \irqstat, [\base, #GIC_CPU_EOI] - cmp \tmp, #0 + cmp \irqnr, #32 + strcc \irqstat, [\base, #GIC_CPU_EOI] + movcc r1, \regs + adrcc lr, \sym + bcc gic_call_ppi .endm
arch/arm/include/asm/hardware/gic.h +4 -1
@@ -33,6 +33,8 @@#define GIC_DIST_SOFTINT 0xf00#ifndef __ASSEMBLY__ +#include <linux/interrupt.h> +extern void __iomem *gic_cpu_base_addr;extern struct irq_chip gic_arch_extn;
@@ -40,7 +42,8 @@ void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);void gic_secondary_init (unsigned int );void gic_cascade_irq (unsigned int gic_nr,unsigned int irq);void gic_raise_softirq (const struct cpumask *mask,unsigned int irq); -void gic_enable_ppi (unsigned int ); +int gic_request_ppi (unsigned int ,irq_handler_t ,void *); +void gic_free_ppi (unsigned int ,void *);#endif #endif
arch/arm/include/asm/localtimer.h +2 -12
@@ -17,27 +17,17 @@ struct clock_event_device;*/ void percpu_timer_setup (void ); -/* -* Called from assembly, this is the local timer IRQ handler -*/ -asmlinkage void do_local_timer(struct pt_regs *); - -#ifdef CONFIG_LOCAL_TIMERS #ifdef CONFIG_HAVE_ARM_TWD #include "smp_twd.h" -#define local_timer_ack () twd_timer_ack() +#define local_timer_stop twd_timer_stop#else -/* -* Platform provides this to acknowledge a local timer IRQ. -* Returns true if the local timer IRQ is to be processed. -*/ -int local_timer_ack (void ); +int local_timer_stop (struct clock_event_device *);#endif
arch/arm/include/asm/smp_twd.h +1 -1
@@ -22,7 +22,7 @@ struct clock_event_device;extern void __iomem *twd_base; -int twd_timer_ack (void );void twd_timer_setup (struct clock_event_device *); +void twd_timer_stop (struct clock_event_device *);#endif
arch/arm/kernel/smp.c +1 -17
@@ -458,19 +458,6 @@ static void ipi_timer(void) }#ifdef CONFIG_LOCAL_TIMERS -asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs) -{ -struct pt_regs *old_regs =set_irq_regs (regs); -int cpu =smp_processor_id (); - -if (local_timer_ack ()) { -__inc_irq_stat (cpu, local_timer_irqs); -ipi_timer (); - } - -set_irq_regs (old_regs); -} -void show_local_irqs (struct seq_file *p,int prec) {unsigned int cpu;
@@ -531,10 +518,7 @@ void __cpuinit percpu_timer_setup(void)*/ 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); +local_timer_stop (&per_cpu (percpu_clockevent,smp_processor_id ())); }#endif
arch/arm/kernel/smp_twd.c +24 -17
65 lines not on this page · raw message
arch/arm/mach-exynos4/include/mach/entry-macro.S +4 -4
@@ -74,11 +74,11 @@ /* As above, this assumes that irqstat and base are preserved.. */ - .macro test_for_ltirq, irqnr, irqstat, base, tmp + .macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym bic \irqnr, \irqstat, #0x1c00 - mov \tmp, #0 cmp \irqnr, #29 - moveq \tmp, #1 streq \irqstat, [\base, #GIC_CPU_EOI] - cmp \tmp, #0 + moveq r1, \regs + adreq lr, \sym + bcc gic_call_ppi .endm
arch/arm/mach-msm/include/mach/entry-macro-qgic.S +7 -7
@@ -78,11 +78,11 @@ /* As above, this assumes that irqstat and base are preserved.. */ - .macro test_for_ltirq, irqnr, irqstat, base, tmp - bic \irqnr, \irqstat, #0x1c00 - mov \tmp, #0 - cmp \irqnr, #16 - moveq \tmp, #1 - streq \irqstat, [\base, #GIC_CPU_EOI] - cmp \tmp, #0 + .macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym + bic \irqnr, \irqstat, #0x1c00 + cmp \irqnr, #16 + streq \irqstat, [\base, #GIC_CPU_EOI] + moveq r1, \regs + adreq lr, \sym + beq gic_call_ppi .endm
arch/arm/mach-msm/timer.c +17 -3
@@ -271,9 +271,19 @@ static void __init msm_timer_init(void) }#ifdef CONFIG_SMP +static irqreturn_t local_timer_irq (int irq,void *data) +{ +struct clock_event_device *evt = data; + + evt->event_handler (evt); + +return IRQ_HANDLED ; +} +int __cpuinit local_timer_setup (struct clock_event_device *evt) {struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER ]; +int ret;/* Use existing clock_event for cpu 0 */ if (!smp_processor_id ())
@@ -300,15 +310,19 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt) local_clock_event = evt; -gic_enable_ppi (clock->irq.irq); + ret =gic_request_ppi (evt->irq, local_timer_irq, evt); +if (ret) +pr_err ("CPU%u: unable to request local timer interrupt\n" , +smp_processor_id ());clockevents_register_device (evt); return 0; } -inline int local_timer_ack (void ) +void local_timer_stop (struct clock_event_device *evt) { -return 1 ; + clk->set_mode (CLOCK_EVT_MODE_UNUSED , clk); +gic_free_ppi (clk->irq, clk); }#endif
arch/arm/mach-omap2/include/mach/entry-macro.S +4 -5
@@ -104,14 +104,13 @@ /* As above, this assumes that irqstat and base are preserved */ - .macro test_for_ltirq, irqnr, irqstat, base, tmp + .macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym bic \irqnr, \irqstat, #0x1c00 - mov \tmp, #0 cmp \irqnr, #29 - itt eq - moveq \tmp, #1 streq \irqstat, [\base, #GIC_CPU_EOI] - cmp \tmp, #0 + moveq r1, \regs + adreq lr, \sym + beq gic_call_ppi .endm #endif /* CONFIG_SMP */
arch/arm/mach-shmobile/entry-intc.S +1 -1
@@ -51,7 +51,7 @@ .macro test_for_ipi, irqnr, irqstat, base, tmp .endm - .macro test_for_ltirq, irqnr, irqstat, base, tmp + .macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym .endm arch_irq_handler shmobile_handle_irq_intc
arch/arm/mach-shmobile/include/mach/entry-macro.S +1 -1
@@ -27,7 +27,7 @@ .macro test_for_ipi, irqnr, irqstat, base, tmp .endm - .macro test_for_ltirq, irqnr, irqstat, base, tmp + .macro test_for_ppi, irqnr, irqstat, base, tmp, regs, sym .endm .macro arch_ret_to_user, tmp1, tmp2
v11 cover Consolidating GIC per-cpu interrupts
B1;2601;0cRussell,
Quoted 5 lines · Russell King - ARM Linux
Thomas,
Could you _please_ look Marcs patch series and give an opinion on it. I've also attached my patch to this reply too, which is an alternative approach to Marcs.
I don't have fundamental objections to Marcs or your approach. In fact they are very similar.
The main difference is that Marc sets up regular interrupts with handle_percpu_irq instead of going through a separate entry point. The only downside is that it exposes the PPI interrupts to the generic irq API, so nothing can prevent stupid drivers to call disable/enable_irq & al. I'm not sure how much of an issue that is in reality. If it matters we can add a flag to the core code which excludes such interrupts from being accessed.
Another thing, which sticks out compared to other percpu interrupt users in arch/* is that you provide the ability to assign different handlers on different CPUs to a given PPI interrupt number. Most other percpu implementations setup the interrupt with a unique percpu aware handler and just enable/disable it per core in the low level setup/shutdown code. Is running different handlers on different cores a real requirement or just a nice feature with no usecase?
Thanks,
tglx
v11 cover Consolidating GIC per-cpu interrupts
Thomas,
Quoted 7 lines · Thomas Gleixner
Another thing, which sticks out compared to other percpu interrupt users in
arch/*is that you provide the ability to assign different handlers on different CPUs to a given PPI interrupt number. Most other percpu implementations setup the interrupt with a unique percpu aware handler and just enable/disable it per core in the low level setup/shutdown code. Is running different handlers on different cores a real requirement or just a nice feature with no usecase?
At the moment, it sort of falls into the second category. MSM has "asymmetric" timers (each core has its private timer on a different PPI), but that doesn't mandate having separate handlers per core, unless someone decides to connect something on another CPU, using the same PPI... The architecture would probably allow it.
But a clear requirement we have is that the handler has to be called with a per-cpu dev_id pointer (we use this to obtain the clock_event_device in the timer handler, for example). Which makes having something similar to request_irq() quite the natural thing.
Cheers,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
v11 cover Consolidating GIC per-cpu interrupts
Marc,
Quoted 12 lines · Marc Zyngier
Thomas,
Quoted 7 lines · Thomas Gleixner
Another thing, which sticks out compared to other percpu interrupt users in
arch/*is that you provide the ability to assign different handlers on different CPUs to a given PPI interrupt number. Most other percpu implementations setup the interrupt with a unique percpu aware handler and just enable/disable it per core in the low level setup/shutdown code. Is running different handlers on different cores a real requirement or just a nice feature with no usecase?
Quoted 5 lines
At the moment, it sort of falls into the second category. MSM has "asymmetric" timers (each core has its private timer on a different PPI), but that doesn't mandate having separate handlers per core, unless someone decides to connect something on another CPU, using the same PPI... The architecture would probably allow it.
The question is whether we really want to allow it from the OS side. That makes irq accounting an utter mess as you end up with devA,B,C,D on the same interrupt line and each counts on the corresponding CPU0,1,2,3
Quoted 4 lines
But a clear requirement we have is that the handler has to be called with a per-cpu
dev_idpointer (we use this to obtain theclock_event_devicein the timer handler, for example). Which makes having something similar torequest_irq()quite the natural thing.
That makes a lot of sense, but it requires your extra percpu handler registration/free interface ....
Looking at the other PERCPU irq users there might be a general interest for this.
If we can apply the following set of (sane) restricitions to this:
- interrupt is never shared
- interrupt is never threaded
- handler is common for all CPUs
then we could do something like the patch below. Warning, this is incomplete and requires a bunch of other changes like adding per cpu aware enable/disable functions and excluding the other interfaces from fiddling with such an interrupt.
So a request/setup_irq() of such an interrupt would require the following steps.
irq_set_percpu_devid(irq);
This would set: IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOPROBE | IRQ_PER_CPU_DEVID and
irq_set_handler(irq, handle_irq_per_cpu_devid); setup/request_percpu_irq(irq, .....);
The dev_id pointer for those interrupts would be a percpu pointer which holds the real dev_ids, e.g. the percpu clockevents
Due to the restricted nature of those interrupts we probably can ignore nested disable/enable_percpu_irq() calls and just keep the *_percpu_irq API to a bare minimum.
Thoughts ?
Thanks,
tglx
include/linux/interrupt.h +15 -11
@@ -95,6 +97,7 @@ typedef irqreturn_t (*irq_handler_t)(int* @flags: flags (see IRQF_* above) * @name: name of the device * @dev_id: cookie to identify the device +* @percpu_dev_id: cookie to identify the device * @next: pointer to the next irqaction for shared interrupts * @irq: interrupt number * @dir: pointer to the proc/irq/NN/name entry
@@ -104,17 +107,20 @@ typedef irqreturn_t (*irq_handler_t)(int* @thread_mask: bitmask for keeping track of @thread activity */ struct irqaction { -irq_handler_t handler; -unsigned long flags; -void *dev_id; -struct irqaction *next; -int irq; -irq_handler_t thread_fn; -struct task_struct *thread; -unsigned long thread_flags; -unsigned long thread_mask; -const char *name; -struct proc_dir_entry *dir; +irq_handler_t handler; +unsigned long flags; +union { +void *dev_id; +void __percpu *percpu_dev_id; + }; +struct irqaction *next; +int irq; +irq_handler_t thread_fn; +struct task_struct *thread; +unsigned long thread_flags; +unsigned long thread_mask; +const char *name; +struct proc_dir_entry *dir; }____cacheline_internodealigned_in_smp ;extern irqreturn_t no_action (int cpl,void *dev_id);
kernel/irq/chip.c +31 -0
@@ -544,6 +544,37 @@ handle_percpu_irq(unsigned int irq, stru chip->irq_eoi (&desc->irq_data); } +/** +* handle_percpu_devid_irq - Per CPU local irq handler with per cpu dev ids +* @irq: the interrupt number +* @desc: the interrupt description structure for this irq +* +* Per CPU interrupts on SMP machines without locking requirements. Same as +* handle_percpu_irq() above but with the following extras: +* +* action->dev_id is a pointer to percpu variables which contain the +* real device id for the cpu on which this handler is called +*/ +void handle_percpu_devid_irq (unsigned int irq,struct irq_desc *desc) +{ +struct irq_chip *chip =irq_desc_get_chip (desc); +struct irqaction *action = desc->action; +void *dev_id =__this_cpu_ptr (action->percpu_dev_id); +irqreturn_t res; + +kstat_incr_irqs_this_cpu (irq, desc); + +if (chip->irq_ack) + chip->irq_ack (&desc->irq_data); + +trace_irq_handler_entry (irq, action); + res = action->handler (irq, dev_id); +trace_irq_handler_exit (irq, action, res); + +if (chip->irq_eoi) + chip->irq_eoi (&desc->irq_data); +} +void __irq_set_handler (unsigned int irq,irq_flow_handler_t handle,int is_chained,const char *name)
v11 cover Consolidating GIC per-cpu interrupts
Thomas,
Quoted 60 lines · Thomas Gleixner
Marc,
Quoted 12 lines · Marc Zyngier
Thomas,
Quoted 7 lines · Thomas Gleixner
Another thing, which sticks out compared to other percpu interrupt users in
arch/*is that you provide the ability to assign different handlers on different CPUs to a given PPI interrupt number. Most other percpu implementations setup the interrupt with a unique percpu aware handler and just enable/disable it per core in the low level setup/shutdown code. Is running different handlers on different cores a real requirement or just a nice feature with no usecase?Quoted 5 lines
At the moment, it sort of falls into the second category. MSM has "asymmetric" timers (each core has its private timer on a different PPI), but that doesn't mandate having separate handlers per core, unless someone decides to connect something on another CPU, using the same PPI... The architecture would probably allow it.
The question is whether we really want to allow it from the OS side. That makes irq accounting an utter mess as you end up with devA,B,C,D on the same interrupt line and each counts on the corresponding CPU0,1,2,3
Quoted 4 lines
But a clear requirement we have is that the handler has to be called with a per-cpu
dev_idpointer (we use this to obtain theclock_event_devicein the timer handler, for example). Which makes having something similar torequest_irq()quite the natural thing.That makes a lot of sense, but it requires your extra percpu handler registration/free interface ....
Looking at the other PERCPU irq users there might be a general interest for this.
If we can apply the following set of (sane) restricitions to this:
- interrupt is never shared
- interrupt is never threaded
- handler is common for all CPUsthen we could do something like the patch below. Warning, this is incomplete and requires a bunch of other changes like adding per cpu aware enable/disable functions and excluding the other interfaces from fiddling with such an interrupt.
So a request/setup_irq() of such an interrupt would require the following steps.
irq_set_percpu_devid(irq);This would set:
IRQ_NOAUTOEN|IRQ_PER_CPU|IRQ_NOPROBE|IRQ_PER_CPU_DEVIDandirq_set_handler(irq, handle_irq_per_cpu_devid); setup/request_percpu_irq(irq, .....);
Quoted 8 lines
The
dev_idpointer for those interrupts would be a percpu pointer which holds the realdev_ids, e.g. the percpu clockeventsDue to the restricted nature of those interrupts we probably can ignore nested disable/enable_percpu_irq() calls and just keep the
*_percpu_irqAPI to a bare minimum.Thoughts ?
I quite like it. Specially if it can be useful to other architectures. Let me glue all that together to get a feel of how it would work, and I'll get back to you.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...