2020-09-01 14:46:44

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 00/16] arm/arm64: Turning IPIs into normal interrupts

For as long as SMP ARM has existed, IPIs have been handled as
something special. The arch code and the interrupt controller exchange
a couple of hooks (one to generate an IPI, another to handle it).

Although this is perfectly manageable, it prevents the use of features
that we could use if IPIs were Linux IRQs (such as pseudo-NMIs). It
also means that each interrupt controller driver has to follow an
architecture-specific interface instead of just implementing the base
irqchip functionalities. The arch code also duplicates a number of
things that the core irq code already does (such as calling
set_irq_regs(), irq_enter()...).

This series tries to remedy this on arm/arm64 by offering a new
registration interface where the irqchip gives the arch code a range
of interrupts to use for IPIs. The arch code requests these as normal
per-cpu interrupts.

The bulk of the work is at the interrupt controller level, where all 5
irqchips used on arm+SMP/arm64 get converted.

Finally, we drop the legacy registration interface as well as the
custom statistics accounting.

Note that I have had a look at providing a "generic" interface by
expanding the kernel/irq/ipi.c bag of helpers, but so far all
irqchips have very different requirements, so there is hardly anything
to consolidate for now. Maybe some as hip04 and the Marvell horror get
cleaned up (the latter certainly could do with a good dusting).

This has been tested on a bunch of 32 and 64bit guests (GICv2, GICv3),
as well as 64bit bare metal (GICv3). The RPi part has only been tested
in QEMU as a 64bit guest, while the HiSi and Marvell parts have only
been compile-tested.

I'm aiming for 5.10 for this, so any comment would be appreciated.

* From v2:
- Tidied up the arch code (__read_mostly for the IPI handling data,
WARN_ON_ONCE on setup and teardown), dropped spurious prototype
on 32bit
- Prevent IPIs from being forwarded to VCPUs
- Merged the GIC affinity fix, hence one less patch
- Added RBs from Valentin

* From v1:
- Clarified the effect of nesting irq_enter/exit (Russell)
- Changed the point where we tear IPIs down on (Valentin)
- IPIs are no longer accessible from DT
- HIP04 and Armada 370-XP have been converted, but are untested
- arch-specific kstat accounting is removed
- ARM's legacy interface is dropped

Marc Zyngier (16):
genirq: Add fasteoi IPI flow
genirq: Allow interrupts to be excluded from /proc/interrupts
arm64: Allow IPIs to be handled as normal interrupts
ARM: Allow IPIs to be handled as normal interrupts
irqchip/gic-v3: Describe the SGI range
irqchip/gic-v3: Configure SGIs as standard interrupts
irqchip/gic: Refactor SMP configuration
irqchip/gic: Configure SGIs as standard interrupts
irqchip/gic-common: Don't enable SGIs by default
irqchip/bcm2836: Configure mailbox interrupts as standard interrupts
irqchip/hip04: Configure IPIs as standard interrupts
irqchip/armada-370-xp: Configure IPIs as standard interrupts
arm64: Kill __smp_cross_call and co
arm64: Remove custom IRQ stat accounting
ARM: Kill __smp_cross_call and co
ARM: Remove custom IRQ stat accounting

arch/arm/Kconfig | 1 +
arch/arm/include/asm/hardirq.h | 17 --
arch/arm/include/asm/smp.h | 5 +-
arch/arm/kernel/smp.c | 135 +++++++++-----
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/hardirq.h | 9 -
arch/arm64/include/asm/irq_work.h | 4 +-
arch/arm64/include/asm/smp.h | 6 +-
arch/arm64/kernel/smp.c | 121 ++++++++-----
drivers/irqchip/irq-armada-370-xp.c | 262 +++++++++++++++++++---------
drivers/irqchip/irq-bcm2836.c | 151 +++++++++++++---
drivers/irqchip/irq-gic-common.c | 3 -
drivers/irqchip/irq-gic-v3.c | 104 ++++++-----
drivers/irqchip/irq-gic.c | 177 +++++++++++--------
drivers/irqchip/irq-hip04.c | 89 +++++-----
include/linux/irq.h | 5 +-
kernel/irq/chip.c | 27 +++
kernel/irq/debugfs.c | 1 +
kernel/irq/proc.c | 2 +-
kernel/irq/settings.h | 7 +
20 files changed, 720 insertions(+), 407 deletions(-)

--
2.27.0


2020-09-01 14:47:11

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 02/16] genirq: Allow interrupts to be excluded from /proc/interrupts

A number of architectures implement IPI statistics directly,
duplicating the core kstat_irqs accounting. As we move IPIs to
being actual IRQs, we would end-up with a confusing display
in /proc/interrupts (where the IPIs would appear twice).

In order to solve this, allow interrupts to be flagged as
"hidden", which excludes them from /proc/interrupts.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
include/linux/irq.h | 4 +++-
kernel/irq/debugfs.c | 1 +
kernel/irq/proc.c | 2 +-
kernel/irq/settings.h | 7 +++++++
4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 57205bbf46bf..63b9d962ee67 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -71,6 +71,7 @@ enum irqchip_irq_state;
* it from the spurious interrupt detection
* mechanism and from core side polling.
* IRQ_DISABLE_UNLAZY - Disable lazy irq disable
+ * IRQ_HIDDEN - Don't show up in /proc/interrupts
*/
enum {
IRQ_TYPE_NONE = 0x00000000,
@@ -97,13 +98,14 @@ enum {
IRQ_PER_CPU_DEVID = (1 << 17),
IRQ_IS_POLLED = (1 << 18),
IRQ_DISABLE_UNLAZY = (1 << 19),
+ IRQ_HIDDEN = (1 << 20),
};

#define IRQF_MODIFY_MASK \
(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \
- IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
+ IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY | IRQ_HIDDEN)

#define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)

diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index b95ff5d5f4bd..acabc0c0e46b 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -136,6 +136,7 @@ static const struct irq_bit_descr irqdesc_states[] = {
BIT_MASK_DESCR(_IRQ_PER_CPU_DEVID),
BIT_MASK_DESCR(_IRQ_IS_POLLED),
BIT_MASK_DESCR(_IRQ_DISABLE_UNLAZY),
+ BIT_MASK_DESCR(_IRQ_HIDDEN),
};

static const struct irq_bit_descr irqdesc_istates[] = {
diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 32c071d7bc03..72513ed2a5fc 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -485,7 +485,7 @@ int show_interrupts(struct seq_file *p, void *v)

rcu_read_lock();
desc = irq_to_desc(i);
- if (!desc)
+ if (!desc || irq_settings_is_hidden(desc))
goto outsparse;

if (desc->kstat_irqs)
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index e43795cd2ccf..403378b9947b 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -17,6 +17,7 @@ enum {
_IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
_IRQ_IS_POLLED = IRQ_IS_POLLED,
_IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
+ _IRQ_HIDDEN = IRQ_HIDDEN,
_IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
};

@@ -31,6 +32,7 @@ enum {
#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
#define IRQ_IS_POLLED GOT_YOU_MORON
#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
+#define IRQ_HIDDEN GOT_YOU_MORON
#undef IRQF_MODIFY_MASK
#define IRQF_MODIFY_MASK GOT_YOU_MORON

@@ -167,3 +169,8 @@ static inline void irq_settings_clr_disable_unlazy(struct irq_desc *desc)
{
desc->status_use_accessors &= ~_IRQ_DISABLE_UNLAZY;
}
+
+static inline bool irq_settings_is_hidden(struct irq_desc *desc)
+{
+ return desc->status_use_accessors & _IRQ_HIDDEN;
+}
--
2.27.0

2020-09-01 14:47:20

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 05/16] irqchip/gic-v3: Describe the SGI range

As we are about to start making use of SGIs in a more conventional
way, let's describe it is the GICv3 list of interrupt types.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 850842f27bee..f7c99a302d01 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -112,6 +112,7 @@ static DEFINE_PER_CPU(bool, has_rss);
#define DEFAULT_PMR_VALUE 0xf0

enum gic_intid_range {
+ SGI_RANGE,
PPI_RANGE,
SPI_RANGE,
EPPI_RANGE,
@@ -123,6 +124,8 @@ enum gic_intid_range {
static enum gic_intid_range __get_intid_range(irq_hw_number_t hwirq)
{
switch (hwirq) {
+ case 0 ... 15:
+ return SGI_RANGE;
case 16 ... 31:
return PPI_RANGE;
case 32 ... 1019:
@@ -148,15 +151,22 @@ static inline unsigned int gic_irq(struct irq_data *d)
return d->hwirq;
}

-static inline int gic_irq_in_rdist(struct irq_data *d)
+static inline bool gic_irq_in_rdist(struct irq_data *d)
{
- enum gic_intid_range range = get_intid_range(d);
- return range == PPI_RANGE || range == EPPI_RANGE;
+ switch (get_intid_range(d)) {
+ case SGI_RANGE:
+ case PPI_RANGE:
+ case EPPI_RANGE:
+ return true;
+ default:
+ return false;
+ }
}

static inline void __iomem *gic_dist_base(struct irq_data *d)
{
switch (get_intid_range(d)) {
+ case SGI_RANGE:
case PPI_RANGE:
case EPPI_RANGE:
/* SGI+PPI -> SGI_base for this CPU */
@@ -253,6 +263,7 @@ static void gic_enable_redist(bool enable)
static u32 convert_offset_index(struct irq_data *d, u32 offset, u32 *index)
{
switch (get_intid_range(d)) {
+ case SGI_RANGE:
case PPI_RANGE:
case SPI_RANGE:
*index = d->hwirq;
@@ -1277,6 +1288,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
chip = &gic_eoimode1_chip;

switch (__get_intid_range(hw)) {
+ case SGI_RANGE:
case PPI_RANGE:
case EPPI_RANGE:
irq_set_percpu_devid(irq);
--
2.27.0

2020-09-01 14:47:32

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 06/16] irqchip/gic-v3: Configure SGIs as standard interrupts

Change the way we deal with GICv3 SGIs by turning them into proper
IRQs, and calling into the arch code to register the interrupt range
instead of a callback.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3.c | 86 ++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 39 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index f7c99a302d01..717064588299 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -36,6 +36,8 @@
#define FLAGS_WORKAROUND_GICR_WAKER_MSM8996 (1ULL << 0)
#define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539 (1ULL << 1)

+#define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1)
+
struct redist_region {
void __iomem *redist_base;
phys_addr_t phys_base;
@@ -383,7 +385,7 @@ static int gic_irq_set_irqchip_state(struct irq_data *d,
{
u32 reg;

- if (d->hwirq >= 8192) /* PPI/SPI only */
+ if (d->hwirq >= 8192) /* SGI/PPI/SPI only */
return -EINVAL;

switch (which) {
@@ -583,6 +585,9 @@ static int gic_set_type(struct irq_data *d, unsigned int type)

static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
{
+ if (get_intid_range(d) == SGI_RANGE)
+ return -EINVAL;
+
if (vcpu)
irqd_set_forwarded_to_vcpu(d);
else
@@ -657,38 +662,14 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
if ((irqnr >= 1020 && irqnr <= 1023))
return;

- /* Treat anything but SGIs in a uniform way */
- if (likely(irqnr > 15)) {
- int err;
-
- if (static_branch_likely(&supports_deactivate_key))
- gic_write_eoir(irqnr);
- else
- isb();
-
- err = handle_domain_irq(gic_data.domain, irqnr, regs);
- if (err) {
- WARN_ONCE(true, "Unexpected interrupt received!\n");
- gic_deactivate_unhandled(irqnr);
- }
- return;
- }
- if (irqnr < 16) {
+ if (static_branch_likely(&supports_deactivate_key))
gic_write_eoir(irqnr);
- if (static_branch_likely(&supports_deactivate_key))
- gic_write_dir(irqnr);
-#ifdef CONFIG_SMP
- /*
- * Unlike GICv2, we don't need an smp_rmb() here.
- * The control dependency from gic_read_iar to
- * the ISB in gic_write_eoir is enough to ensure
- * that any shared data read by handle_IPI will
- * be read after the ACK.
- */
- handle_IPI(irqnr, regs);
-#else
- WARN_ONCE(true, "Unexpected SGI received!\n");
-#endif
+ else
+ isb();
+
+ if (handle_domain_irq(gic_data.domain, irqnr, regs)) {
+ WARN_ONCE(true, "Unexpected interrupt received!\n");
+ gic_deactivate_unhandled(irqnr);
}
}

@@ -1136,11 +1117,11 @@ static void gic_send_sgi(u64 cluster_id, u16 tlist, unsigned int irq)
gic_write_sgi1r(val);
}

-static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
+static void gic_ipi_send_mask(struct irq_data *d, const struct cpumask *mask)
{
int cpu;

- if (WARN_ON(irq >= 16))
+ if (WARN_ON(d->hwirq >= 16))
return;

/*
@@ -1154,7 +1135,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
u16 tlist;

tlist = gic_compute_target_list(&cpu, mask, cluster_id);
- gic_send_sgi(cluster_id, tlist, irq);
+ gic_send_sgi(cluster_id, tlist, d->hwirq);
}

/* Force the above writes to ICC_SGI1R_EL1 to be executed */
@@ -1163,10 +1144,24 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)

static void __init gic_smp_init(void)
{
- set_smp_cross_call(gic_raise_softirq);
+ struct irq_fwspec sgi_fwspec = {
+ .fwnode = gic_data.fwnode,
+ .param_count = 1,
+ };
+ int base_sgi;
+
cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_GIC_STARTING,
"irqchip/arm/gicv3:starting",
gic_starting_cpu, NULL);
+
+ /* Register all 8 non-secure SGIs */
+ base_sgi = __irq_domain_alloc_irqs(gic_data.domain, -1, 8,
+ NUMA_NO_NODE, &sgi_fwspec,
+ false, NULL);
+ if (WARN_ON(base_sgi <= 0))
+ return;
+
+ set_smp_ipi_range(base_sgi, 8);
}

static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
@@ -1215,6 +1210,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
}
#else
#define gic_set_affinity NULL
+#define gic_ipi_send_mask NULL
#define gic_smp_init() do { } while(0)
#endif

@@ -1257,6 +1253,7 @@ static struct irq_chip gic_chip = {
.irq_set_irqchip_state = gic_irq_set_irqchip_state,
.irq_nmi_setup = gic_irq_nmi_setup,
.irq_nmi_teardown = gic_irq_nmi_teardown,
+ .ipi_send_mask = gic_ipi_send_mask,
.flags = IRQCHIP_SET_TYPE_MASKED |
IRQCHIP_SKIP_SET_WAKE |
IRQCHIP_MASK_ON_SUSPEND,
@@ -1274,6 +1271,7 @@ static struct irq_chip gic_eoimode1_chip = {
.irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity,
.irq_nmi_setup = gic_irq_nmi_setup,
.irq_nmi_teardown = gic_irq_nmi_teardown,
+ .ipi_send_mask = gic_ipi_send_mask,
.flags = IRQCHIP_SET_TYPE_MASKED |
IRQCHIP_SKIP_SET_WAKE |
IRQCHIP_MASK_ON_SUSPEND,
@@ -1289,6 +1287,12 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,

switch (__get_intid_range(hw)) {
case SGI_RANGE:
+ irq_set_percpu_devid(irq);
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
+ handle_percpu_devid_fasteoi_ipi,
+ NULL, NULL);
+ break;
+
case PPI_RANGE:
case EPPI_RANGE:
irq_set_percpu_devid(irq);
@@ -1318,13 +1322,17 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
return 0;
}

-#define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1)
-
static int gic_irq_domain_translate(struct irq_domain *d,
struct irq_fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
+ if (fwspec->param_count == 1 && fwspec->param[0] < 16) {
+ *hwirq = fwspec->param[0];
+ *type = IRQ_TYPE_EDGE_RISING;
+ return 0;
+ }
+
if (is_of_node(fwspec->fwnode)) {
if (fwspec->param_count < 3)
return -EINVAL;
@@ -1656,9 +1664,9 @@ static int __init gic_init_bases(void __iomem *dist_base,

gic_update_rdist_properties();

- gic_smp_init();
gic_dist_init();
gic_cpu_init();
+ gic_smp_init();
gic_cpu_pm_init();

if (gic_dist_supports_lpis()) {
--
2.27.0

2020-09-01 14:53:46

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 13/16] arm64: Kill __smp_cross_call and co

The old IPI registration interface is now unused on arm64, so let's
get rid of it.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm64/include/asm/irq_work.h | 4 +---
arch/arm64/include/asm/smp.h | 7 ------
arch/arm64/kernel/smp.c | 38 ++++++-------------------------
3 files changed, 8 insertions(+), 41 deletions(-)

diff --git a/arch/arm64/include/asm/irq_work.h b/arch/arm64/include/asm/irq_work.h
index 8a1ef1907760..a1020285ea75 100644
--- a/arch/arm64/include/asm/irq_work.h
+++ b/arch/arm64/include/asm/irq_work.h
@@ -2,11 +2,9 @@
#ifndef __ASM_IRQ_WORK_H
#define __ASM_IRQ_WORK_H

-#include <asm/smp.h>
-
static inline bool arch_irq_work_has_interrupt(void)
{
- return !!__smp_cross_call;
+ return true;
}

#endif /* __ASM_IRQ_WORK_H */
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index 57c5db15f6b7..06bc8684f70c 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -71,13 +71,6 @@ extern void handle_IPI(int ipinr, struct pt_regs *regs);
*/
extern void smp_init_cpus(void);

-/*
- * Provide a function to raise an IPI cross call on CPUs in callmap.
- */
-extern void set_smp_cross_call(void (*)(const struct cpumask *, unsigned int));
-
-extern void (*__smp_cross_call)(const struct cpumask *, unsigned int);
-
/*
* Register IPI interrupts with the arch SMP code
*/
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 00c9db1b61b5..58fb155fb0ab 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -782,13 +782,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
}
}

-void (*__smp_cross_call)(const struct cpumask *, unsigned int);
-
-void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
-{
- __smp_cross_call = fn;
-}
-
static const char *ipi_types[NR_IPI] __tracepoint_string = {
#define S(x,s) [x] = s
S(IPI_RESCHEDULE, "Rescheduling interrupts"),
@@ -800,11 +793,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
S(IPI_WAKEUP, "CPU wake-up interrupts"),
};

-static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
-{
- trace_ipi_raise(target, ipi_types[ipinr]);
- __smp_cross_call(target, ipinr);
-}
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr);

void show_ipi_list(struct seq_file *p, int prec)
{
@@ -851,8 +840,7 @@ void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
#ifdef CONFIG_IRQ_WORK
void arch_irq_work_raise(void)
{
- if (__smp_cross_call)
- smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
+ smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
}
#endif

@@ -959,34 +947,23 @@ static void do_handle_IPI(int ipinr)
trace_ipi_exit_rcuidle(ipi_types[ipinr]);
}

-/* Legacy version, should go away once all irqchips have been converted */
-void handle_IPI(int ipinr, struct pt_regs *regs)
-{
- struct pt_regs *old_regs = set_irq_regs(regs);
-
- irq_enter();
- do_handle_IPI(ipinr);
- irq_exit();
-
- set_irq_regs(old_regs);
-}
-
static irqreturn_t ipi_handler(int irq, void *data)
{
do_handle_IPI(irq - ipi_irq_base);
return IRQ_HANDLED;
}

-static void ipi_send(const struct cpumask *target, unsigned int ipi)
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
{
- __ipi_send_mask(ipi_desc[ipi], target);
+ trace_ipi_raise(target, ipi_types[ipinr]);
+ __ipi_send_mask(ipi_desc[ipinr], target);
}

static void ipi_setup(int cpu)
{
int i;

- if (!ipi_irq_base)
+ if (WARN_ON_ONCE(!ipi_irq_base))
return;

for (i = 0; i < nr_ipi; i++)
@@ -997,7 +974,7 @@ static void ipi_teardown(int cpu)
{
int i;

- if (!ipi_irq_base)
+ if (WARN_ON_ONCE(!ipi_irq_base))
return;

for (i = 0; i < nr_ipi; i++)
@@ -1023,7 +1000,6 @@ void __init set_smp_ipi_range(int ipi_base, int n)
}

ipi_irq_base = ipi_base;
- __smp_cross_call = ipi_send;

/* Setup the boot CPU immediately */
ipi_setup(smp_processor_id());
--
2.27.0

2020-09-01 14:53:53

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 11/16] irqchip/hip04: Configure IPIs as standard interrupts

In order to switch the hip04 driver to provide standard interrupts
for IPIs, rework the way interrupts are allocated, making sure
the irqdomain covers the SGIs as well as the rest of the interrupt
range.

The driver is otherwise so old-school that it creates all interrupts
upfront (duh!), so there is hardly anything else to change, apart
from communicating the IPIs to the arch code.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-hip04.c | 89 +++++++++++++++++--------------------
1 file changed, 40 insertions(+), 49 deletions(-)

diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
index 130caa1c9d93..9b73dcfaf48d 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -171,6 +171,29 @@ static int hip04_irq_set_affinity(struct irq_data *d,

return IRQ_SET_MASK_OK;
}
+
+static void hip04_ipi_send_mask(struct irq_data *d, const struct cpumask *mask)
+{
+ int cpu;
+ unsigned long flags, map = 0;
+
+ raw_spin_lock_irqsave(&irq_controller_lock, flags);
+
+ /* Convert our logical CPU mask into a physical one. */
+ for_each_cpu(cpu, mask)
+ map |= hip04_cpu_map[cpu];
+
+ /*
+ * Ensure that stores to Normal memory are visible to the
+ * other CPUs before they observe us issuing the IPI.
+ */
+ dmb(ishst);
+
+ /* this always happens on GIC0 */
+ writel_relaxed(map << 8 | d->hwirq, hip04_data.dist_base + GIC_DIST_SOFTINT);
+
+ raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
+}
#endif

static void __exception_irq_entry hip04_handle_irq(struct pt_regs *regs)
@@ -182,19 +205,9 @@ static void __exception_irq_entry hip04_handle_irq(struct pt_regs *regs)
irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
irqnr = irqstat & GICC_IAR_INT_ID_MASK;

- if (likely(irqnr > 15 && irqnr <= HIP04_MAX_IRQS)) {
+ if (irqnr <= HIP04_MAX_IRQS)
handle_domain_irq(hip04_data.domain, irqnr, regs);
- continue;
- }
- if (irqnr < 16) {
- writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
-#ifdef CONFIG_SMP
- handle_IPI(irqnr, regs);
-#endif
- continue;
- }
- break;
- } while (1);
+ } while (irqnr > HIP04_MAX_IRQS);
}

static struct irq_chip hip04_irq_chip = {
@@ -205,6 +218,7 @@ static struct irq_chip hip04_irq_chip = {
.irq_set_type = hip04_irq_set_type,
#ifdef CONFIG_SMP
.irq_set_affinity = hip04_irq_set_affinity,
+ .ipi_send_mask = hip04_ipi_send_mask,
#endif
.flags = IRQCHIP_SET_TYPE_MASKED |
IRQCHIP_SKIP_SET_WAKE |
@@ -279,39 +293,17 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc)
writel_relaxed(1, base + GIC_CPU_CTRL);
}

-#ifdef CONFIG_SMP
-static void hip04_raise_softirq(const struct cpumask *mask, unsigned int irq)
-{
- int cpu;
- unsigned long flags, map = 0;
-
- raw_spin_lock_irqsave(&irq_controller_lock, flags);
-
- /* Convert our logical CPU mask into a physical one. */
- for_each_cpu(cpu, mask)
- map |= hip04_cpu_map[cpu];
-
- /*
- * Ensure that stores to Normal memory are visible to the
- * other CPUs before they observe us issuing the IPI.
- */
- dmb(ishst);
-
- /* this always happens on GIC0 */
- writel_relaxed(map << 8 | irq, hip04_data.dist_base + GIC_DIST_SOFTINT);
-
- raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
-}
-#endif
-
static int hip04_irq_domain_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
- if (hw < 32) {
+ if (hw < 16) {
+ irq_set_percpu_devid(irq);
+ irq_set_chip_and_handler(irq, &hip04_irq_chip,
+ handle_percpu_devid_fasteoi_ipi);
+ } else if (hw < 32) {
irq_set_percpu_devid(irq);
irq_set_chip_and_handler(irq, &hip04_irq_chip,
handle_percpu_devid_irq);
- irq_set_status_flags(irq, IRQ_NOAUTOEN);
} else {
irq_set_chip_and_handler(irq, &hip04_irq_chip,
handle_fasteoi_irq);
@@ -328,10 +320,13 @@ static int hip04_irq_domain_xlate(struct irq_domain *d,
unsigned long *out_hwirq,
unsigned int *out_type)
{
- unsigned long ret = 0;
-
if (irq_domain_get_of_node(d) != controller)
return -EINVAL;
+ if (intsize == 1 && intspec[0] < 16) {
+ *out_hwirq = intspec[0];
+ *out_type = IRQ_TYPE_EDGE_RISING;
+ return 0;
+ }
if (intsize < 3)
return -EINVAL;

@@ -344,7 +339,7 @@ static int hip04_irq_domain_xlate(struct irq_domain *d,

*out_type = intspec[2] & IRQ_TYPE_SENSE_MASK;

- return ret;
+ return 0;
}

static int hip04_irq_starting_cpu(unsigned int cpu)
@@ -361,7 +356,6 @@ static const struct irq_domain_ops hip04_irq_domain_ops = {
static int __init
hip04_of_init(struct device_node *node, struct device_node *parent)
{
- irq_hw_number_t hwirq_base = 16;
int nr_irqs, irq_base, i;

if (WARN_ON(!node))
@@ -390,24 +384,21 @@ hip04_of_init(struct device_node *node, struct device_node *parent)
nr_irqs = HIP04_MAX_IRQS;
hip04_data.nr_irqs = nr_irqs;

- nr_irqs -= hwirq_base; /* calculate # of irqs to allocate */
-
- irq_base = irq_alloc_descs(-1, hwirq_base, nr_irqs, numa_node_id());
+ irq_base = irq_alloc_descs(-1, 0, nr_irqs, numa_node_id());
if (irq_base < 0) {
pr_err("failed to allocate IRQ numbers\n");
return -EINVAL;
}

hip04_data.domain = irq_domain_add_legacy(node, nr_irqs, irq_base,
- hwirq_base,
+ 0,
&hip04_irq_domain_ops,
&hip04_data);
-
if (WARN_ON(!hip04_data.domain))
return -EINVAL;

#ifdef CONFIG_SMP
- set_smp_cross_call(hip04_raise_softirq);
+ set_smp_ipi_range(irq_base, 16);
#endif
set_handle_irq(hip04_handle_irq);

--
2.27.0

2020-09-01 14:54:04

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 10/16] irqchip/bcm2836: Configure mailbox interrupts as standard interrupts

In order to switch the bcm2836 driver to privide standard interrupts
for IPIs, it first needs to stop lying about the way things work.

The mailbox interrupt is actually a multiplexer, with enough
bits to store 32 pending interrupts per CPU. So let's turn it
into a chained irqchip.

Once this is done, we can instanciate the corresponding IPIs,
and pass them to the architecture code.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-bcm2836.c | 151 ++++++++++++++++++++++++++++------
1 file changed, 125 insertions(+), 26 deletions(-)

diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 2038693f074c..85df6ddad9be 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -10,6 +10,7 @@
#include <linux/of_irq.h>
#include <linux/irqchip.h>
#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
#include <linux/irqchip/irq-bcm2836.h>

#include <asm/exception.h>
@@ -89,12 +90,24 @@ static struct irq_chip bcm2836_arm_irqchip_gpu = {
.irq_unmask = bcm2836_arm_irqchip_unmask_gpu_irq,
};

+static void bcm2836_arm_irqchip_dummy_op(struct irq_data *d)
+{
+}
+
+static struct irq_chip bcm2836_arm_irqchip_dummy = {
+ .name = "bcm2836-dummy",
+ .irq_eoi = bcm2836_arm_irqchip_dummy_op,
+};
+
static int bcm2836_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
struct irq_chip *chip;

switch (hw) {
+ case LOCAL_IRQ_MAILBOX0:
+ chip = &bcm2836_arm_irqchip_dummy;
+ break;
case LOCAL_IRQ_CNTPSIRQ:
case LOCAL_IRQ_CNTPNSIRQ:
case LOCAL_IRQ_CNTHPIRQ:
@@ -127,17 +140,7 @@ __exception_irq_entry bcm2836_arm_irqchip_handle_irq(struct pt_regs *regs)
u32 stat;

stat = readl_relaxed(intc.base + LOCAL_IRQ_PENDING0 + 4 * cpu);
- if (stat & BIT(LOCAL_IRQ_MAILBOX0)) {
-#ifdef CONFIG_SMP
- void __iomem *mailbox0 = (intc.base +
- LOCAL_MAILBOX0_CLR0 + 16 * cpu);
- u32 mbox_val = readl(mailbox0);
- u32 ipi = ffs(mbox_val) - 1;
-
- writel(1 << ipi, mailbox0);
- handle_IPI(ipi, regs);
-#endif
- } else if (stat) {
+ if (stat) {
u32 hwirq = ffs(stat) - 1;

handle_domain_irq(intc.domain, hwirq, regs);
@@ -145,8 +148,35 @@ __exception_irq_entry bcm2836_arm_irqchip_handle_irq(struct pt_regs *regs)
}

#ifdef CONFIG_SMP
-static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
- unsigned int ipi)
+static struct irq_domain *ipi_domain;
+
+static void bcm2836_arm_irqchip_handle_ipi(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ int cpu = smp_processor_id();
+ u32 mbox_val;
+
+ chained_irq_enter(chip, desc);
+
+ mbox_val = readl_relaxed(intc.base + LOCAL_MAILBOX0_CLR0 + 16 * cpu);
+ if (mbox_val) {
+ int hwirq = ffs(mbox_val) - 1;
+ generic_handle_irq(irq_find_mapping(ipi_domain, hwirq));
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static void bcm2836_arm_irqchip_ipi_eoi(struct irq_data *d)
+{
+ int cpu = smp_processor_id();
+
+ writel_relaxed(BIT(d->hwirq),
+ intc.base + LOCAL_MAILBOX0_CLR0 + 16 * cpu);
+}
+
+static void bcm2836_arm_irqchip_ipi_send_mask(struct irq_data *d,
+ const struct cpumask *mask)
{
int cpu;
void __iomem *mailbox0_base = intc.base + LOCAL_MAILBOX0_SET0;
@@ -157,11 +187,45 @@ static void bcm2836_arm_irqchip_send_ipi(const struct cpumask *mask,
*/
smp_wmb();

- for_each_cpu(cpu, mask) {
- writel(1 << ipi, mailbox0_base + 16 * cpu);
+ for_each_cpu(cpu, mask)
+ writel_relaxed(BIT(d->hwirq), mailbox0_base + 16 * cpu);
+}
+
+static struct irq_chip bcm2836_arm_irqchip_ipi = {
+ .name = "IPI",
+ .irq_eoi = bcm2836_arm_irqchip_ipi_eoi,
+ .ipi_send_mask = bcm2836_arm_irqchip_ipi_send_mask,
+};
+
+static int bcm2836_arm_irqchip_ipi_alloc(struct irq_domain *d,
+ unsigned int virq,
+ unsigned int nr_irqs, void *args)
+{
+ int i;
+
+ for (i = 0; i < nr_irqs; i++) {
+ irq_set_percpu_devid(virq + i);
+ irq_domain_set_info(d, virq + i, i, &bcm2836_arm_irqchip_ipi,
+ d->host_data,
+ handle_percpu_devid_fasteoi_ipi,
+ NULL, NULL);
}
+
+ return 0;
}

+static void bcm2836_arm_irqchip_ipi_free(struct irq_domain *d,
+ unsigned int virq,
+ unsigned int nr_irqs)
+{
+ /* Not freeing IPIs */
+}
+
+static const struct irq_domain_ops ipi_domain_ops = {
+ .alloc = bcm2836_arm_irqchip_ipi_alloc,
+ .free = bcm2836_arm_irqchip_ipi_free,
+};
+
static int bcm2836_cpu_starting(unsigned int cpu)
{
bcm2836_arm_irqchip_unmask_per_cpu_irq(LOCAL_MAILBOX_INT_CONTROL0, 0,
@@ -175,25 +239,58 @@ static int bcm2836_cpu_dying(unsigned int cpu)
cpu);
return 0;
}
-#endif

-static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
- .xlate = irq_domain_xlate_onetwocell,
- .map = bcm2836_map,
-};
+#define BITS_PER_MBOX 32

-static void
-bcm2836_arm_irqchip_smp_init(void)
+static void bcm2836_arm_irqchip_smp_init(void)
{
-#ifdef CONFIG_SMP
+ struct irq_fwspec ipi_fwspec = {
+ .fwnode = intc.domain->fwnode,
+ .param_count = 1,
+ .param = {
+ [0] = LOCAL_IRQ_MAILBOX0,
+ },
+ };
+ int base_ipi, mux_irq;
+
+ mux_irq = irq_create_fwspec_mapping(&ipi_fwspec);
+ if (WARN_ON(mux_irq <= 0))
+ return;
+
+ ipi_domain = irq_domain_create_linear(intc.domain->fwnode,
+ BITS_PER_MBOX, &ipi_domain_ops,
+ NULL);
+ if (WARN_ON(!ipi_domain))
+ return;
+
+ ipi_domain->flags |= IRQ_DOMAIN_FLAG_IPI_SINGLE;
+ irq_domain_update_bus_token(ipi_domain, DOMAIN_BUS_IPI);
+
+ base_ipi = __irq_domain_alloc_irqs(ipi_domain, -1, BITS_PER_MBOX,
+ NUMA_NO_NODE, NULL,
+ false, NULL);
+
+ if (WARN_ON(!base_ipi))
+ return;
+
+ set_smp_ipi_range(base_ipi, BITS_PER_MBOX);
+
+ irq_set_chained_handler_and_data(mux_irq,
+ bcm2836_arm_irqchip_handle_ipi, NULL);
+
/* Unmask IPIs to the boot CPU. */
cpuhp_setup_state(CPUHP_AP_IRQ_BCM2836_STARTING,
"irqchip/bcm2836:starting", bcm2836_cpu_starting,
bcm2836_cpu_dying);
-
- set_smp_cross_call(bcm2836_arm_irqchip_send_ipi);
-#endif
}
+#else
+#define bcm2836_arm_irqchip_smp_init() do { } while(0)
+#endif
+
+static const struct irq_domain_ops bcm2836_arm_irqchip_intc_ops = {
+ .xlate = irq_domain_xlate_onetwocell,
+ .map = bcm2836_map,
+};

/*
* The LOCAL_IRQ_CNT* timer firings are based off of the external
@@ -232,6 +329,8 @@ static int __init bcm2836_arm_irqchip_l1_intc_of_init(struct device_node *node,
if (!intc.domain)
panic("%pOF: unable to create IRQ domain\n", node);

+ irq_domain_update_bus_token(intc.domain, DOMAIN_BUS_WIRED);
+
bcm2836_arm_irqchip_smp_init();

set_handle_irq(bcm2836_arm_irqchip_handle_irq);
--
2.27.0

2020-09-01 14:54:35

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 14/16] arm64: Remove custom IRQ stat accounting

Let's switch the arm64 code to the core accounting, which already
does everything we need.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm64/include/asm/hardirq.h | 9 ---------
arch/arm64/kernel/smp.c | 24 ++++++------------------
2 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
index 985493af704b..5ffa4bacdad3 100644
--- a/arch/arm64/include/asm/hardirq.h
+++ b/arch/arm64/include/asm/hardirq.h
@@ -13,21 +13,12 @@
#include <asm/kvm_arm.h>
#include <asm/sysreg.h>

-#define NR_IPI 7
-
typedef struct {
unsigned int __softirq_pending;
- unsigned int ipi_irqs[NR_IPI];
} ____cacheline_aligned irq_cpustat_t;

#include <linux/irq_cpustat.h> /* Standard mappings for irq_cpustat_t above */

-#define __inc_irq_stat(cpu, member) __IRQ_STAT(cpu, member)++
-#define __get_irq_stat(cpu, member) __IRQ_STAT(cpu, member)
-
-u64 smp_irq_stat_cpu(unsigned int cpu);
-#define arch_irq_stat_cpu smp_irq_stat_cpu
-
#define __ARCH_IRQ_EXIT_IRQS_DISABLED 1

struct nmi_ctx {
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 58fb155fb0ab..07b45466e0bb 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -72,7 +72,8 @@ enum ipi_msg_type {
IPI_CPU_CRASH_STOP,
IPI_TIMER,
IPI_IRQ_WORK,
- IPI_WAKEUP
+ IPI_WAKEUP,
+ NR_IPI
};

static int ipi_irq_base __read_mostly;
@@ -800,26 +801,15 @@ void show_ipi_list(struct seq_file *p, int prec)
unsigned int cpu, i;

for (i = 0; i < NR_IPI; i++) {
+ unsigned int irq = irq_desc_get_irq(ipi_desc[i]);
seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i,
prec >= 4 ? " " : "");
for_each_online_cpu(cpu)
- seq_printf(p, "%10u ",
- __get_irq_stat(cpu, ipi_irqs[i]));
+ seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
seq_printf(p, " %s\n", ipi_types[i]);
}
}

-u64 smp_irq_stat_cpu(unsigned int cpu)
-{
- u64 sum = 0;
- int i;
-
- for (i = 0; i < NR_IPI; i++)
- sum += __get_irq_stat(cpu, ipi_irqs[i]);
-
- return sum;
-}
-
void arch_send_call_function_ipi_mask(const struct cpumask *mask)
{
smp_cross_call(mask, IPI_CALL_FUNC);
@@ -892,10 +882,8 @@ static void do_handle_IPI(int ipinr)
{
unsigned int cpu = smp_processor_id();

- if ((unsigned)ipinr < NR_IPI) {
+ if ((unsigned)ipinr < NR_IPI)
trace_ipi_entry_rcuidle(ipi_types[ipinr]);
- __inc_irq_stat(cpu, ipi_irqs[ipinr]);
- }

switch (ipinr) {
case IPI_RESCHEDULE:
@@ -992,7 +980,7 @@ void __init set_smp_ipi_range(int ipi_base, int n)
int err;

err = request_percpu_irq(ipi_base + i, ipi_handler,
- "IPI", &irq_stat);
+ "IPI", &cpu_number);
WARN_ON(err);

ipi_desc[i] = irq_to_desc(ipi_base + i);
--
2.27.0

2020-09-01 14:54:57

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 12/16] irqchip/armada-370-xp: Configure IPIs as standard interrupts

To introduce IPIs as standard interrupts to the Armada 370-XP
driver, let's allocate a completely separate irqdomain and
irqchip combo that lives parallel to the "standard" one.

This effectively should be modelled as a chained interrupt
controller, but the code is in such a state that it is
pretty hard to shoehorn, as it would require the rewrite
of the MSI layer as well.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-armada-370-xp.c | 262 +++++++++++++++++++---------
1 file changed, 178 insertions(+), 84 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index c9bdc5221b82..d7eb2e93db8f 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -310,7 +310,134 @@ static inline int armada_370_xp_msi_init(struct device_node *node,
}
#endif

+static void armada_xp_mpic_perf_init(void)
+{
+ unsigned long cpuid = cpu_logical_map(smp_processor_id());
+
+ /* Enable Performance Counter Overflow interrupts */
+ writel(ARMADA_370_XP_INT_CAUSE_PERF(cpuid),
+ per_cpu_int_base + ARMADA_370_XP_INT_FABRIC_MASK_OFFS);
+}
+
#ifdef CONFIG_SMP
+static struct irq_domain *ipi_domain;
+
+static void armada_370_xp_ipi_mask(struct irq_data *d)
+{
+ u32 reg;
+ reg = readl(per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+ reg &= ~BIT(d->hwirq);
+ writel(reg, per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+}
+
+static void armada_370_xp_ipi_unmask(struct irq_data *d)
+{
+ u32 reg;
+ reg = readl(per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+ reg |= BIT(d->hwirq);
+ writel(reg, per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+}
+
+static void armada_370_xp_ipi_send_mask(struct irq_data *d,
+ const struct cpumask *mask)
+{
+ unsigned long map = 0;
+ int cpu;
+
+ /* Convert our logical CPU mask into a physical one. */
+ for_each_cpu(cpu, mask)
+ map |= 1 << cpu_logical_map(cpu);
+
+ /*
+ * Ensure that stores to Normal memory are visible to the
+ * other CPUs before issuing the IPI.
+ */
+ dsb();
+
+ /* submit softirq */
+ writel((map << 8) | d->hwirq, main_int_base +
+ ARMADA_370_XP_SW_TRIG_INT_OFFS);
+}
+
+static void armada_370_xp_ipi_eoi(struct irq_data *d)
+{
+ writel(~BIT(d->hwirq), per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
+}
+
+static struct irq_chip ipi_irqchip = {
+ .name = "IPI",
+ .irq_mask = armada_370_xp_ipi_mask,
+ .irq_unmask = armada_370_xp_ipi_unmask,
+ .irq_eoi = armada_370_xp_ipi_eoi,
+ .ipi_send_mask = armada_370_xp_ipi_send_mask,
+};
+
+static int armada_370_xp_ipi_alloc(struct irq_domain *d,
+ unsigned int virq,
+ unsigned int nr_irqs, void *args)
+{
+ int i;
+
+ for (i = 0; i < nr_irqs; i++) {
+ irq_set_percpu_devid(virq + i);
+ irq_domain_set_info(d, virq + i, i, &ipi_irqchip,
+ d->host_data,
+ handle_percpu_devid_fasteoi_ipi,
+ NULL, NULL);
+ }
+
+ return 0;
+}
+
+static void armada_370_xp_ipi_free(struct irq_domain *d,
+ unsigned int virq,
+ unsigned int nr_irqs)
+{
+ /* Not freeing IPIs */
+}
+
+static const struct irq_domain_ops ipi_domain_ops = {
+ .alloc = armada_370_xp_ipi_alloc,
+ .free = armada_370_xp_ipi_free,
+};
+
+static void ipi_resume(void)
+{
+ int i;
+
+ for (i = 0; i < IPI_DOORBELL_END; i++) {
+ int irq;
+
+ irq = irq_find_mapping(ipi_domain, i);
+ if (irq <= 0)
+ continue;
+ if (irq_percpu_is_enabled(irq)) {
+ struct irq_data *d;
+ d = irq_domain_get_irq_data(ipi_domain, irq);
+ armada_370_xp_ipi_unmask(d);
+ }
+ }
+}
+
+static __init void armada_xp_ipi_init(struct device_node *node)
+{
+ int base_ipi;
+
+ ipi_domain = irq_domain_create_linear(of_node_to_fwnode(node),
+ IPI_DOORBELL_END,
+ &ipi_domain_ops, NULL);
+ if (WARN_ON(!ipi_domain))
+ return;
+
+ irq_domain_update_bus_token(ipi_domain, DOMAIN_BUS_IPI);
+ base_ipi = __irq_domain_alloc_irqs(ipi_domain, -1, IPI_DOORBELL_END,
+ NUMA_NO_NODE, NULL, false, NULL);
+ if (WARN_ON(!base_ipi))
+ return;
+
+ set_smp_ipi_range(base_ipi, IPI_DOORBELL_END);
+}
+
static DEFINE_RAW_SPINLOCK(irq_controller_lock);

static int armada_xp_set_affinity(struct irq_data *d,
@@ -334,43 +461,6 @@ static int armada_xp_set_affinity(struct irq_data *d,

return IRQ_SET_MASK_OK;
}
-#endif
-
-static struct irq_chip armada_370_xp_irq_chip = {
- .name = "MPIC",
- .irq_mask = armada_370_xp_irq_mask,
- .irq_mask_ack = armada_370_xp_irq_mask,
- .irq_unmask = armada_370_xp_irq_unmask,
-#ifdef CONFIG_SMP
- .irq_set_affinity = armada_xp_set_affinity,
-#endif
- .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
-};
-
-static int armada_370_xp_mpic_irq_map(struct irq_domain *h,
- unsigned int virq, irq_hw_number_t hw)
-{
- armada_370_xp_irq_mask(irq_get_irq_data(virq));
- if (!is_percpu_irq(hw))
- writel(hw, per_cpu_int_base +
- ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
- else
- writel(hw, main_int_base + ARMADA_370_XP_INT_SET_ENABLE_OFFS);
- irq_set_status_flags(virq, IRQ_LEVEL);
-
- if (is_percpu_irq(hw)) {
- irq_set_percpu_devid(virq);
- irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
- handle_percpu_devid_irq);
- } else {
- irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
- handle_level_irq);
- irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(virq)));
- }
- irq_set_probe(virq);
-
- return 0;
-}

static void armada_xp_mpic_smp_cpu_init(void)
{
@@ -383,48 +473,16 @@ static void armada_xp_mpic_smp_cpu_init(void)
for (i = 0; i < nr_irqs; i++)
writel(i, per_cpu_int_base + ARMADA_370_XP_INT_SET_MASK_OFFS);

+ /* Disable all IPIs */
+ writel(0, per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
+
/* Clear pending IPIs */
writel(0, per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);

- /* Enable first 8 IPIs */
- writel(IPI_DOORBELL_MASK, per_cpu_int_base +
- ARMADA_370_XP_IN_DRBEL_MSK_OFFS);
-
/* Unmask IPI interrupt */
writel(0, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
}

-static void armada_xp_mpic_perf_init(void)
-{
- unsigned long cpuid = cpu_logical_map(smp_processor_id());
-
- /* Enable Performance Counter Overflow interrupts */
- writel(ARMADA_370_XP_INT_CAUSE_PERF(cpuid),
- per_cpu_int_base + ARMADA_370_XP_INT_FABRIC_MASK_OFFS);
-}
-
-#ifdef CONFIG_SMP
-static void armada_mpic_send_doorbell(const struct cpumask *mask,
- unsigned int irq)
-{
- int cpu;
- unsigned long map = 0;
-
- /* Convert our logical CPU mask into a physical one. */
- for_each_cpu(cpu, mask)
- map |= 1 << cpu_logical_map(cpu);
-
- /*
- * Ensure that stores to Normal memory are visible to the
- * other CPUs before issuing the IPI.
- */
- dsb();
-
- /* submit softirq */
- writel((map << 8) | irq, main_int_base +
- ARMADA_370_XP_SW_TRIG_INT_OFFS);
-}
-
static void armada_xp_mpic_reenable_percpu(void)
{
unsigned int irq;
@@ -445,6 +503,8 @@ static void armada_xp_mpic_reenable_percpu(void)

armada_370_xp_irq_unmask(data);
}
+
+ ipi_resume();
}

static int armada_xp_mpic_starting_cpu(unsigned int cpu)
@@ -462,7 +522,46 @@ static int mpic_cascaded_starting_cpu(unsigned int cpu)
enable_percpu_irq(parent_irq, IRQ_TYPE_NONE);
return 0;
}
+#else
+static void armada_xp_mpic_smp_cpu_init(void) {}
+static void ipi_resume(void) {}
+#endif
+
+static struct irq_chip armada_370_xp_irq_chip = {
+ .name = "MPIC",
+ .irq_mask = armada_370_xp_irq_mask,
+ .irq_mask_ack = armada_370_xp_irq_mask,
+ .irq_unmask = armada_370_xp_irq_unmask,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = armada_xp_set_affinity,
#endif
+ .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int armada_370_xp_mpic_irq_map(struct irq_domain *h,
+ unsigned int virq, irq_hw_number_t hw)
+{
+ armada_370_xp_irq_mask(irq_get_irq_data(virq));
+ if (!is_percpu_irq(hw))
+ writel(hw, per_cpu_int_base +
+ ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
+ else
+ writel(hw, main_int_base + ARMADA_370_XP_INT_SET_ENABLE_OFFS);
+ irq_set_status_flags(virq, IRQ_LEVEL);
+
+ if (is_percpu_irq(hw)) {
+ irq_set_percpu_devid(virq);
+ irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
+ handle_percpu_devid_irq);
+ } else {
+ irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
+ handle_level_irq);
+ irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(virq)));
+ }
+ irq_set_probe(virq);
+
+ return 0;
+}

static const struct irq_domain_ops armada_370_xp_mpic_irq_ops = {
.map = armada_370_xp_mpic_irq_map,
@@ -562,22 +661,15 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
#ifdef CONFIG_SMP
/* IPI Handling */
if (irqnr == 0) {
- u32 ipimask, ipinr;
+ unsigned long ipimask;
+ int ipi;

ipimask = readl_relaxed(per_cpu_int_base +
ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS)
& IPI_DOORBELL_MASK;

- writel(~ipimask, per_cpu_int_base +
- ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
-
- /* Handle all pending doorbells */
- for (ipinr = IPI_DOORBELL_START;
- ipinr < IPI_DOORBELL_END; ipinr++) {
- if (ipimask & (0x1 << ipinr))
- handle_IPI(ipinr, regs);
- }
- continue;
+ for_each_set_bit(ipi, &ipimask, IPI_DOORBELL_END)
+ handle_domain_irq(ipi_domain, ipi, regs);
}
#endif

@@ -636,6 +728,8 @@ static void armada_370_xp_mpic_resume(void)
writel(0, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
if (doorbell_mask_reg & PCI_MSI_DOORBELL_MASK)
writel(1, per_cpu_int_base + ARMADA_370_XP_INT_CLEAR_MASK_OFFS);
+
+ ipi_resume();
}

static struct syscore_ops armada_370_xp_mpic_syscore_ops = {
@@ -691,7 +785,7 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,
irq_set_default_host(armada_370_xp_mpic_domain);
set_handle_irq(armada_370_xp_handle_irq);
#ifdef CONFIG_SMP
- set_smp_cross_call(armada_mpic_send_doorbell);
+ armada_xp_ipi_init(node);
cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_ARMADA_XP_STARTING,
"irqchip/armada/ipi:starting",
armada_xp_mpic_starting_cpu, NULL);
--
2.27.0

2020-09-01 14:54:59

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 15/16] ARM: Kill __smp_cross_call and co

The old IPI registration interface is now unused on arm, so let's
get rid of it.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm/include/asm/smp.h | 6 ------
arch/arm/kernel/smp.c | 26 +++++++-------------------
2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 0e29730295ca..0ca55a607d0a 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -39,12 +39,6 @@ void handle_IPI(int ipinr, struct pt_regs *regs);
*/
extern void smp_init_cpus(void);

-
-/*
- * Provide a function to raise an IPI cross call on CPUs in callmap.
- */
-extern void set_smp_cross_call(void (*)(const struct cpumask *, unsigned int));
-
/*
* Register IPI interrupts with the arch SMP code
*/
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index f21f78483353..d51e64955a26 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -511,14 +511,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
}
}

-static void (*__smp_cross_call)(const struct cpumask *, unsigned int);
-
-void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
-{
- if (!__smp_cross_call)
- __smp_cross_call = fn;
-}
-
static const char *ipi_types[NR_IPI] __tracepoint_string = {
#define S(x,s) [x] = s
S(IPI_WAKEUP, "CPU wakeup interrupts"),
@@ -530,11 +522,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
S(IPI_COMPLETION, "completion interrupts"),
};

-static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
-{
- trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);
- __smp_cross_call(target, ipinr);
-}
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr);

void show_ipi_list(struct seq_file *p, int prec)
{
@@ -713,16 +701,17 @@ static irqreturn_t ipi_handler(int irq, void *data)
return IRQ_HANDLED;
}

-static void ipi_send(const struct cpumask *target, unsigned int ipi)
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
{
- __ipi_send_mask(ipi_desc[ipi], target);
+ trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);
+ __ipi_send_mask(ipi_desc[ipinr], target);
}

static void ipi_setup(int cpu)
{
int i;

- if (!ipi_irq_base)
+ if (WARN_ON_ONCE(!ipi_irq_base))
return;

for (i = 0; i < nr_ipi; i++)
@@ -733,7 +722,7 @@ static void ipi_teardown(int cpu)
{
int i;

- if (!ipi_irq_base)
+ if (WARN_ON_ONCE(!ipi_irq_base))
return;

for (i = 0; i < nr_ipi; i++)
@@ -759,7 +748,6 @@ void __init set_smp_ipi_range(int ipi_base, int n)
}

ipi_irq_base = ipi_base;
- set_smp_cross_call(ipi_send);

/* Setup the boot CPU immediately */
ipi_setup(smp_processor_id());
@@ -872,7 +860,7 @@ core_initcall(register_cpufreq_notifier);

static void raise_nmi(cpumask_t *mask)
{
- __smp_cross_call(mask, IPI_CPU_BACKTRACE);
+ __ipi_send_mask(ipi_desc[IPI_CPU_BACKTRACE], mask);
}

void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
--
2.27.0

2020-09-01 14:56:06

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

Let's switch the arm code to the core accounting, which already
does everything we need.

Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm/include/asm/hardirq.h | 17 -----------------
arch/arm/kernel/smp.c | 20 ++++----------------
2 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index 7a88f160b1fb..b95848ed2bc7 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -6,29 +6,12 @@
#include <linux/threads.h>
#include <asm/irq.h>

-/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
-#define NR_IPI 7
-
typedef struct {
unsigned int __softirq_pending;
-#ifdef CONFIG_SMP
- unsigned int ipi_irqs[NR_IPI];
-#endif
} ____cacheline_aligned irq_cpustat_t;

#include <linux/irq_cpustat.h> /* Standard mappings for irq_cpustat_t above */

-#define __inc_irq_stat(cpu, member) __IRQ_STAT(cpu, member)++
-#define __get_irq_stat(cpu, member) __IRQ_STAT(cpu, member)
-
-#ifdef CONFIG_SMP
-u64 smp_irq_stat_cpu(unsigned int cpu);
-#else
-#define smp_irq_stat_cpu(cpu) 0
-#endif
-
-#define arch_irq_stat_cpu smp_irq_stat_cpu
-
#define __ARCH_IRQ_EXIT_IRQS_DISABLED 1

#endif /* __ASM_HARDIRQ_H */
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index d51e64955a26..aead847ac8b9 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -65,6 +65,7 @@ enum ipi_msg_type {
IPI_CPU_STOP,
IPI_IRQ_WORK,
IPI_COMPLETION,
+ NR_IPI,
/*
* CPU_BACKTRACE is special and not included in NR_IPI
* or tracable with trace_ipi_*
@@ -529,27 +530,16 @@ void show_ipi_list(struct seq_file *p, int prec)
unsigned int cpu, i;

for (i = 0; i < NR_IPI; i++) {
+ unsigned int irq = irq_desc_get_irq(ipi_desc[i]);
seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);

for_each_online_cpu(cpu)
- seq_printf(p, "%10u ",
- __get_irq_stat(cpu, ipi_irqs[i]));
+ seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));

seq_printf(p, " %s\n", ipi_types[i]);
}
}

-u64 smp_irq_stat_cpu(unsigned int cpu)
-{
- u64 sum = 0;
- int i;
-
- for (i = 0; i < NR_IPI; i++)
- sum += __get_irq_stat(cpu, ipi_irqs[i]);
-
- return sum;
-}
-
void arch_send_call_function_ipi_mask(const struct cpumask *mask)
{
smp_cross_call(mask, IPI_CALL_FUNC);
@@ -630,10 +620,8 @@ static void do_handle_IPI(int ipinr)
{
unsigned int cpu = smp_processor_id();

- if ((unsigned)ipinr < NR_IPI) {
+ if ((unsigned)ipinr < NR_IPI)
trace_ipi_entry_rcuidle(ipi_types[ipinr]);
- __inc_irq_stat(cpu, ipi_irqs[ipinr]);
- }

switch (ipinr) {
case IPI_WAKEUP:
--
2.27.0

2020-09-02 07:47:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

Hi Marc,

I love your patch! Yet something to improve:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on arm/for-next v5.9-rc3 next-20200828]
[cannot apply to tip/irq/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Marc-Zyngier/arm-arm64-Turning-IPIs-into-normal-interrupts/20200901-225407
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm-realview_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

arch/arm/kernel/smp.c: In function 'show_ipi_list':
>> arch/arm/kernel/smp.c:537:27: error: implicit declaration of function 'kstat_irqs_cpu' [-Werror=implicit-function-declaration]
537 | seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
| ^~~~~~~~~~~~~~
arch/arm/kernel/smp.c: At top level:
arch/arm/kernel/smp.c:559:6: warning: no previous prototype for 'arch_irq_work_raise' [-Wmissing-prototypes]
559 | void arch_irq_work_raise(void)
| ^~~~~~~~~~~~~~~~~~~
arch/arm/kernel/smp.c:774:6: warning: no previous prototype for 'panic_smp_self_stop' [-Wmissing-prototypes]
774 | void panic_smp_self_stop(void)
| ^~~~~~~~~~~~~~~~~~~
arch/arm/kernel/smp.c:786:5: warning: no previous prototype for 'setup_profiling_timer' [-Wmissing-prototypes]
786 | int setup_profiling_timer(unsigned int multiplier)
| ^~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/780a93ca7a729bf2414f45d5322cc62f43ae4070
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marc-Zyngier/arm-arm64-Turning-IPIs-into-normal-interrupts/20200901-225407
git checkout 780a93ca7a729bf2414f45d5322cc62f43ae4070
vim +/kstat_irqs_cpu +537 arch/arm/kernel/smp.c

527
528 void show_ipi_list(struct seq_file *p, int prec)
529 {
530 unsigned int cpu, i;
531
532 for (i = 0; i < NR_IPI; i++) {
533 unsigned int irq = irq_desc_get_irq(ipi_desc[i]);
534 seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
535
536 for_each_online_cpu(cpu)
> 537 seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
538
539 seq_printf(p, " %s\n", ipi_types[i]);
540 }
541 }
542

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.04 kB)
.config.gz (20.54 kB)
Download all attachments

2020-09-02 20:22:47

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

On Wed, 02 Sep 2020 08:41:31 +0100,
kernel test robot <[email protected]> wrote:
>
> Hi Marc,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on arm64/for-next/core]
> [also build test ERROR on arm/for-next v5.9-rc3 next-20200828]
> [cannot apply to tip/irq/core]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Marc-Zyngier/arm-arm64-Turning-IPIs-into-normal-interrupts/20200901-225407
> base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> config: arm-realview_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> arch/arm/kernel/smp.c: In function 'show_ipi_list':
> >> arch/arm/kernel/smp.c:537:27: error: implicit declaration of function 'kstat_irqs_cpu' [-Werror=implicit-function-declaration]
> 537 | seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
> | ^~~~~~~~~~~~~~

Now fixed. Thanks for the heads up.

M.

--
Without deviation from the norm, progress is not possible.

2020-09-07 06:14:59

by [email protected]

[permalink] [raw]
Subject: RE: [PATCH v3 00/16] arm/arm64: Turning IPIs into normal interrupts

Hi Marc.

I am interested in this feature.
I have confirmed this patch works fine on Fujitsu FX1000. Thanks for creating it.
I look forward to the day when this patch is merged.
We plan to post a patch that implements IPI for CPU stop Interrupt (for crash dump) with pseudo NMI.

Tested-by: Hitomi Hasegawa <[email protected]>
Thanks.

-----Original Message-----
From: linux-arm-kernel <[email protected]> On Behalf Of Marc Zyngier
Sent: Tuesday, September 1, 2020 11:43 PM
To: [email protected]; [email protected]
Cc: Sumit Garg <[email protected]>; [email protected]; Florian Fainelli <[email protected]>; Russell King <[email protected]>; Jason Cooper <[email protected]>; Saravana Kannan <[email protected]>; Andrew Lunn <[email protected]>; Catalin Marinas <[email protected]>; Gregory Clement <[email protected]>; Thomas Gleixner <[email protected]>; Will Deacon <[email protected]>; Valentin Schneider <[email protected]>
Subject: [PATCH v3 00/16] arm/arm64: Turning IPIs into normal interrupts

For as long as SMP ARM has existed, IPIs have been handled as something special. The arch code and the interrupt controller exchange a couple of hooks (one to generate an IPI, another to handle it).

Although this is perfectly manageable, it prevents the use of features that we could use if IPIs were Linux IRQs (such as pseudo-NMIs). It also means that each interrupt controller driver has to follow an architecture-specific interface instead of just implementing the base irqchip functionalities. The arch code also duplicates a number of things that the core irq code already does (such as calling set_irq_regs(), irq_enter()...).

This series tries to remedy this on arm/arm64 by offering a new registration interface where the irqchip gives the arch code a range of interrupts to use for IPIs. The arch code requests these as normal per-cpu interrupts.

The bulk of the work is at the interrupt controller level, where all 5 irqchips used on arm+SMP/arm64 get converted.

Finally, we drop the legacy registration interface as well as the custom statistics accounting.

Note that I have had a look at providing a "generic" interface by expanding the kernel/irq/ipi.c bag of helpers, but so far all irqchips have very different requirements, so there is hardly anything to consolidate for now. Maybe some as hip04 and the Marvell horror get cleaned up (the latter certainly could do with a good dusting).

This has been tested on a bunch of 32 and 64bit guests (GICv2, GICv3), as well as 64bit bare metal (GICv3). The RPi part has only been tested in QEMU as a 64bit guest, while the HiSi and Marvell parts have only been compile-tested.

I'm aiming for 5.10 for this, so any comment would be appreciated.

* From v2:
- Tidied up the arch code (__read_mostly for the IPI handling data,
WARN_ON_ONCE on setup and teardown), dropped spurious prototype
on 32bit
- Prevent IPIs from being forwarded to VCPUs
- Merged the GIC affinity fix, hence one less patch
- Added RBs from Valentin

* From v1:
- Clarified the effect of nesting irq_enter/exit (Russell)
- Changed the point where we tear IPIs down on (Valentin)
- IPIs are no longer accessible from DT
- HIP04 and Armada 370-XP have been converted, but are untested
- arch-specific kstat accounting is removed
- ARM's legacy interface is dropped

Marc Zyngier (16):
genirq: Add fasteoi IPI flow
genirq: Allow interrupts to be excluded from /proc/interrupts
arm64: Allow IPIs to be handled as normal interrupts
ARM: Allow IPIs to be handled as normal interrupts
irqchip/gic-v3: Describe the SGI range
irqchip/gic-v3: Configure SGIs as standard interrupts
irqchip/gic: Refactor SMP configuration
irqchip/gic: Configure SGIs as standard interrupts
irqchip/gic-common: Don't enable SGIs by default
irqchip/bcm2836: Configure mailbox interrupts as standard interrupts
irqchip/hip04: Configure IPIs as standard interrupts
irqchip/armada-370-xp: Configure IPIs as standard interrupts
arm64: Kill __smp_cross_call and co
arm64: Remove custom IRQ stat accounting
ARM: Kill __smp_cross_call and co
ARM: Remove custom IRQ stat accounting

arch/arm/Kconfig | 1 +
arch/arm/include/asm/hardirq.h | 17 --
arch/arm/include/asm/smp.h | 5 +-
arch/arm/kernel/smp.c | 135 +++++++++-----
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/hardirq.h | 9 -
arch/arm64/include/asm/irq_work.h | 4 +-
arch/arm64/include/asm/smp.h | 6 +-
arch/arm64/kernel/smp.c | 121 ++++++++-----
drivers/irqchip/irq-armada-370-xp.c | 262 +++++++++++++++++++---------
drivers/irqchip/irq-bcm2836.c | 151 +++++++++++++---
drivers/irqchip/irq-gic-common.c | 3 -
drivers/irqchip/irq-gic-v3.c | 104 ++++++-----
drivers/irqchip/irq-gic.c | 177 +++++++++++--------
drivers/irqchip/irq-hip04.c | 89 +++++-----
include/linux/irq.h | 5 +-
kernel/irq/chip.c | 27 +++
kernel/irq/debugfs.c | 1 +
kernel/irq/proc.c | 2 +-
kernel/irq/settings.h | 7 +
20 files changed, 720 insertions(+), 407 deletions(-)

--
2.27.0


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2020-09-11 16:43:38

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v3 13/16] arm64: Kill __smp_cross_call and co

On Tue, Sep 01, 2020 at 03:43:21PM +0100, Marc Zyngier wrote:
> The old IPI registration interface is now unused on arm64, so let's
> get rid of it.
>
> Reviewed-by: Valentin Schneider <[email protected]>
> Signed-off-by: Marc Zyngier <[email protected]>

Acked-by: Catalin Marinas <[email protected]>

2020-09-11 16:45:48

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v3 14/16] arm64: Remove custom IRQ stat accounting

On Tue, Sep 01, 2020 at 03:43:22PM +0100, Marc Zyngier wrote:
> Let's switch the arm64 code to the core accounting, which already
> does everything we need.
>
> Reviewed-by: Valentin Schneider <[email protected]>
> Signed-off-by: Marc Zyngier <[email protected]>

Acked-by: Catalin Marinas <[email protected]>

2020-09-14 14:36:22

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH v3 10/16] irqchip/bcm2836: Configure mailbox interrupts as standard interrupts

Hi Marc,

On 01.09.2020 16:43, Marc Zyngier wrote:
> In order to switch the bcm2836 driver to privide standard interrupts
> for IPIs, it first needs to stop lying about the way things work.
>
> The mailbox interrupt is actually a multiplexer, with enough
> bits to store 32 pending interrupts per CPU. So let's turn it
> into a chained irqchip.
>
> Once this is done, we can instanciate the corresponding IPIs,
> and pass them to the architecture code.
>
> Signed-off-by: Marc Zyngier <[email protected]>

This one also fails. It breaks booting of Raspberry Pi 3b boards (both
in ARM and ARM64 mode):

NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = (ptrval)
[00000000] *pgd=80000000004003, *pmd=00000000
Internal error: Oops: 80000206 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.9.0-rc4+ #9166
Hardware name: BCM2835
PC is at 0x0
LR is at irq_percpu_enable+0x40/0x50
pc : [<00000000>]    lr : [<c0274638>]    psr: 600000d3
sp : c1201e00  ip : c1201e18  fp : c1201e14
r10: c0ef23dc  r9 : c120583c  r8 : 00000000
r7 : 00000011  r6 : eb032d00  r5 : 00000000  r4 : eb032d00
r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : eb032d18
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment user
Control: 30c5383d  Table: 00003000  DAC: fffffffd
Process swapper/0 (pid: 0, stack limit = 0x(ptrval))
Stack: (0xc1201e00 to 0xc1202000)
...
Backtrace:
[<c02745f8>] (irq_percpu_enable) from [<c0272498>]
(enable_percpu_irq+0xa4/0xd8)
 r5:c1204ec8 r4:00000000
[<c02723f4>] (enable_percpu_irq) from [<c020ded0>]
(ipi_setup.part.0+0x3c/0x48)
 r8:c107ba80 r7:00000018 r6:00000011 r5:c1204ee0 r4:00000001
[<c020de94>] (ipi_setup.part.0) from [<c1005684>]
(set_smp_ipi_range+0xd8/0xf8)
 r5:c1204ee0 r4:00000008
[<c10055ac>] (set_smp_ipi_range) from [<c1023388>]
(bcm2836_arm_irqchip_l1_intc_of_init+0x1c0/0x22c)
 r8:c1366820 r7:c1204ec8 r6:00000010 r5:00000001 r4:00000000
[<c10231c8>] (bcm2836_arm_irqchip_l1_intc_of_init) from [<c102f28c>]
(of_irq_init+0x18c/0x2dc)
 r9:00000000 r8:c1201f34 r7:c1204ec8 r6:c1201f2c r5:c1201f2c r4:eb004880
[<c102f100>] (of_irq_init) from [<c1022f4c>] (irqchip_init+0x1c/0x24)
 r10:0000006c r9:00000000 r8:00000000 r7:ffffffff r6:cccccccd r5:c0eb7abe
 r4:c103ba30
[<c1022f30>] (irqchip_init) from [<c1003960>] (init_IRQ+0x30/0x98)
[<c1003930>] (init_IRQ) from [<c1000ebc>] (start_kernel+0x3b4/0x628)
 r5:c0eb7abe r4:c12c1000
[<c1000b08>] (start_kernel) from [<00000000>] (0x0)
 r10:30c5387d r9:410fd034 r8:02600000 r7:00000000 r6:30c0387d r5:00000000
 r4:c1000330
Code: bad PC value
random: get_random_bytes called from init_oops_id+0x30/0x4c with crng_init=0
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---


> ---
> drivers/irqchip/irq-bcm2836.c | 151 ++++++++++++++++++++++++++++------
> 1 file changed, 125 insertions(+), 26 deletions(-)
>
> ...

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2020-09-14 16:20:43

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 10/16] irqchip/bcm2836: Configure mailbox interrupts as standard interrupts

Hi Marek,

On 2020-09-14 15:32, Marek Szyprowski wrote:
> Hi Marc,
>
> On 01.09.2020 16:43, Marc Zyngier wrote:
>> In order to switch the bcm2836 driver to privide standard interrupts
>> for IPIs, it first needs to stop lying about the way things work.
>>
>> The mailbox interrupt is actually a multiplexer, with enough
>> bits to store 32 pending interrupts per CPU. So let's turn it
>> into a chained irqchip.
>>
>> Once this is done, we can instanciate the corresponding IPIs,
>> and pass them to the architecture code.
>>
>> Signed-off-by: Marc Zyngier <[email protected]>
>
> This one also fails. It breaks booting of Raspberry Pi 3b boards (both
> in ARM and ARM64 mode):

Damn. This used to work. Looks like I was eager to delete stuff at
some point. Can you give this a go and let me know if that works
for you (only tested in QEMU with the raspi2 model):

diff --git a/drivers/irqchip/irq-bcm2836.c
b/drivers/irqchip/irq-bcm2836.c
index 85df6ddad9be..97838eb705f9 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -193,6 +193,8 @@ static void bcm2836_arm_irqchip_ipi_send_mask(struct
irq_data *d,

static struct irq_chip bcm2836_arm_irqchip_ipi = {
.name = "IPI",
+ .irq_mask = bcm2836_arm_irqchip_dummy_op,
+ .irq_unmask = bcm2836_arm_irqchip_dummy_op,
.irq_eoi = bcm2836_arm_irqchip_ipi_eoi,
.ipi_send_mask = bcm2836_arm_irqchip_ipi_send_mask,
};


Thanks again,

M.
--
Jazz is not dead. It just smells funny...

2020-09-14 19:14:08

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH v3 10/16] irqchip/bcm2836: Configure mailbox interrupts as standard interrupts

Hi Marc,

On 14.09.2020 18:10, Marc Zyngier wrote:
> On 2020-09-14 15:32, Marek Szyprowski wrote:
>> On 01.09.2020 16:43, Marc Zyngier wrote:
>>> In order to switch the bcm2836 driver to privide standard interrupts
>>> for IPIs, it first needs to stop lying about the way things work.
>>>
>>> The mailbox interrupt is actually a multiplexer, with enough
>>> bits to store 32 pending interrupts per CPU. So let's turn it
>>> into a chained irqchip.
>>>
>>> Once this is done, we can instanciate the corresponding IPIs,
>>> and pass them to the architecture code.
>>>
>>> Signed-off-by: Marc Zyngier <[email protected]>
>>
>> This one also fails. It breaks booting of Raspberry Pi 3b boards (both
>> in ARM and ARM64 mode):
>
> Damn. This used to work. Looks like I was eager to delete stuff at
> some point. Can you give this a go and let me know if that works
> for you (only tested in QEMU with the raspi2 model):
>
> diff --git a/drivers/irqchip/irq-bcm2836.c
> b/drivers/irqchip/irq-bcm2836.c
> index 85df6ddad9be..97838eb705f9 100644
> --- a/drivers/irqchip/irq-bcm2836.c
> +++ b/drivers/irqchip/irq-bcm2836.c
> @@ -193,6 +193,8 @@ static void
> bcm2836_arm_irqchip_ipi_send_mask(struct irq_data *d,
>
>  static struct irq_chip bcm2836_arm_irqchip_ipi = {
>      .name        = "IPI",
> +    .irq_mask    = bcm2836_arm_irqchip_dummy_op,
> +    .irq_unmask    = bcm2836_arm_irqchip_dummy_op,
>      .irq_eoi    = bcm2836_arm_irqchip_ipi_eoi,
>      .ipi_send_mask    = bcm2836_arm_irqchip_ipi_send_mask,
>  };
>
>
> Thanks again,

This fixes boot on my RPi3b. Thanks!

Tested-by: Marek Szyprowski <[email protected]>

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2020-09-16 20:41:12

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v3 00/16] arm/arm64: Turning IPIs into normal interrupts



On 9/1/2020 7:43 AM, Marc Zyngier wrote:
> For as long as SMP ARM has existed, IPIs have been handled as
> something special. The arch code and the interrupt controller exchange
> a couple of hooks (one to generate an IPI, another to handle it).
>
> Although this is perfectly manageable, it prevents the use of features
> that we could use if IPIs were Linux IRQs (such as pseudo-NMIs). It
> also means that each interrupt controller driver has to follow an
> architecture-specific interface instead of just implementing the base
> irqchip functionalities. The arch code also duplicates a number of
> things that the core irq code already does (such as calling
> set_irq_regs(), irq_enter()...).
>
> This series tries to remedy this on arm/arm64 by offering a new
> registration interface where the irqchip gives the arch code a range
> of interrupts to use for IPIs. The arch code requests these as normal
> per-cpu interrupts.
>
> The bulk of the work is at the interrupt controller level, where all 5
> irqchips used on arm+SMP/arm64 get converted.
>
> Finally, we drop the legacy registration interface as well as the
> custom statistics accounting.
>
> Note that I have had a look at providing a "generic" interface by
> expanding the kernel/irq/ipi.c bag of helpers, but so far all
> irqchips have very different requirements, so there is hardly anything
> to consolidate for now. Maybe some as hip04 and the Marvell horror get
> cleaned up (the latter certainly could do with a good dusting).
>
> This has been tested on a bunch of 32 and 64bit guests (GICv2, GICv3),
> as well as 64bit bare metal (GICv3). The RPi part has only been tested
> in QEMU as a 64bit guest, while the HiSi and Marvell parts have only
> been compile-tested.
>
> I'm aiming for 5.10 for this, so any comment would be appreciated.

FWIW, I boot tested this on a Brahma-B53 device (GIC-400) in 32-bit and
64-bit mode and on a Brahma-B15 device (GIC-400 as well) and both
devices worked in all 3 configurations:

Tested-by: Florian Fainelli <[email protected]>

All cores were brought up successfully. All of these devices use
PSCI/ATF FWIW.
--
Florian

2020-09-24 09:02:14

by Guillaume Tucker

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

Hi Marc,

On 01/09/2020 15:43, Marc Zyngier wrote:
> Let's switch the arm code to the core accounting, which already
> does everything we need.
>
> Reviewed-by: Valentin Schneider <[email protected]>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> arch/arm/include/asm/hardirq.h | 17 -----------------
> arch/arm/kernel/smp.c | 20 ++++----------------
> 2 files changed, 4 insertions(+), 33 deletions(-)

This appears to be causing a NULL pointer dereference on
beaglebone-black, it got bisected automatically several times.
None of the other platforms in the KernelCI labs appears to be
affected.

Here's the error in the full job log, with next-20200923:

https://storage.staging.kernelci.org/kernelci/staging.kernelci.org/staging-20200924.0/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-beaglebone-black.html#L460

and some meta-data:

https://staging.kernelci.org/test/case/id/5f6bea67f724eb1b34dce584/

The full bisection report is available here:

https://groups.io/g/kernelci-results-staging/message/2094

I've also run it again with a debug build to locate the problem,
see below.


> diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
> index 7a88f160b1fb..b95848ed2bc7 100644
> --- a/arch/arm/include/asm/hardirq.h
> +++ b/arch/arm/include/asm/hardirq.h
> @@ -6,29 +6,12 @@
> #include <linux/threads.h>
> #include <asm/irq.h>
>
> -/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
> -#define NR_IPI 7
> -
> typedef struct {
> unsigned int __softirq_pending;
> -#ifdef CONFIG_SMP
> - unsigned int ipi_irqs[NR_IPI];
> -#endif
> } ____cacheline_aligned irq_cpustat_t;
>
> #include <linux/irq_cpustat.h> /* Standard mappings for irq_cpustat_t above */
>
> -#define __inc_irq_stat(cpu, member) __IRQ_STAT(cpu, member)++
> -#define __get_irq_stat(cpu, member) __IRQ_STAT(cpu, member)
> -
> -#ifdef CONFIG_SMP
> -u64 smp_irq_stat_cpu(unsigned int cpu);
> -#else
> -#define smp_irq_stat_cpu(cpu) 0
> -#endif
> -
> -#define arch_irq_stat_cpu smp_irq_stat_cpu
> -
> #define __ARCH_IRQ_EXIT_IRQS_DISABLED 1
>
> #endif /* __ASM_HARDIRQ_H */
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index d51e64955a26..aead847ac8b9 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -65,6 +65,7 @@ enum ipi_msg_type {
> IPI_CPU_STOP,
> IPI_IRQ_WORK,
> IPI_COMPLETION,
> + NR_IPI,
> /*
> * CPU_BACKTRACE is special and not included in NR_IPI
> * or tracable with trace_ipi_*
> @@ -529,27 +530,16 @@ void show_ipi_list(struct seq_file *p, int prec)
> unsigned int cpu, i;
>
> for (i = 0; i < NR_IPI; i++) {
> + unsigned int irq = irq_desc_get_irq(ipi_desc[i]);

It looks like irq_desc_get_irq() gets called with a NULL
pointer (well, 0x0000001c):

(gdb) l *0xc030ef38
0xc030ef38 is in show_ipi_list (../include/linux/irqdesc.h:123).
118 return container_of(data->common, struct irq_desc, irq_common_data);
119 }
120
121 static inline unsigned int irq_desc_get_irq(struct irq_desc *desc)
122 {
123 return desc->irq_data.irq;
124 }
125
126 static inline struct irq_data *irq_desc_get_irq_data(struct irq_desc *desc)
127 {

Full job log: https://lava.baylibre.com/scheduler/job/142375#L727

I haven't looked any further but hopefully this should be a good
enough clue to find the root cause. I don't know if you have a
platform at hand to reproduce the issue, please let me know if
you need some help with debugging or testing a fix.

Hope this helps,
Guillaume


> seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
>
> for_each_online_cpu(cpu)
> - seq_printf(p, "%10u ",
> - __get_irq_stat(cpu, ipi_irqs[i]));
> + seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
>
> seq_printf(p, " %s\n", ipi_types[i]);
> }
> }
>
> -u64 smp_irq_stat_cpu(unsigned int cpu)
> -{
> - u64 sum = 0;
> - int i;
> -
> - for (i = 0; i < NR_IPI; i++)
> - sum += __get_irq_stat(cpu, ipi_irqs[i]);
> -
> - return sum;
> -}
> -
> void arch_send_call_function_ipi_mask(const struct cpumask *mask)
> {
> smp_cross_call(mask, IPI_CALL_FUNC);
> @@ -630,10 +620,8 @@ static void do_handle_IPI(int ipinr)
> {
> unsigned int cpu = smp_processor_id();
>
> - if ((unsigned)ipinr < NR_IPI) {
> + if ((unsigned)ipinr < NR_IPI)
> trace_ipi_entry_rcuidle(ipi_types[ipinr]);
> - __inc_irq_stat(cpu, ipi_irqs[ipinr]);
> - }
>
> switch (ipinr) {
> case IPI_WAKEUP:
>

2020-09-24 09:31:27

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

Hi Guillaume,

On Thu, 24 Sep 2020 10:00:09 +0100,
Guillaume Tucker <[email protected]> wrote:
>
> Hi Marc,
>
> On 01/09/2020 15:43, Marc Zyngier wrote:
> > Let's switch the arm code to the core accounting, which already
> > does everything we need.
> >
> > Reviewed-by: Valentin Schneider <[email protected]>
> > Signed-off-by: Marc Zyngier <[email protected]>
> > ---
> > arch/arm/include/asm/hardirq.h | 17 -----------------
> > arch/arm/kernel/smp.c | 20 ++++----------------
> > 2 files changed, 4 insertions(+), 33 deletions(-)
>
> This appears to be causing a NULL pointer dereference on
> beaglebone-black, it got bisected automatically several times.
> None of the other platforms in the KernelCI labs appears to be
> affected.

Hmm. My bet is that because this is a UP machine running an SMP
kernel, and I fell into the trap of forgetting about this 32bit
configuration.

I expect the following patch to fix it. Please give it a go if you can
(I'm away at the moment and can't test much, and do not have any
physical 32bit machine to test this on).

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 00327fa74b01..b4e3d336dc33 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -531,7 +531,12 @@ void show_ipi_list(struct seq_file *p, int prec)
unsigned int cpu, i;

for (i = 0; i < NR_IPI; i++) {
- unsigned int irq = irq_desc_get_irq(ipi_desc[i]);
+ unsigned int irq;
+
+ if (!ipi_desc[i])
+ continue;
+
+ irq = irq_desc_get_irq(ipi_desc[i]);
seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);

for_each_online_cpu(cpu)

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2020-09-24 13:13:10

by Guillaume Tucker

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

On 24/09/2020 10:29, Marc Zyngier wrote:
> Hi Guillaume,
>
> On Thu, 24 Sep 2020 10:00:09 +0100,
> Guillaume Tucker <[email protected]> wrote:
>>
>> Hi Marc,
>>
>> On 01/09/2020 15:43, Marc Zyngier wrote:
>>> Let's switch the arm code to the core accounting, which already
>>> does everything we need.
>>>
>>> Reviewed-by: Valentin Schneider <[email protected]>
>>> Signed-off-by: Marc Zyngier <[email protected]>
>>> ---
>>> arch/arm/include/asm/hardirq.h | 17 -----------------
>>> arch/arm/kernel/smp.c | 20 ++++----------------
>>> 2 files changed, 4 insertions(+), 33 deletions(-)
>>
>> This appears to be causing a NULL pointer dereference on
>> beaglebone-black, it got bisected automatically several times.
>> None of the other platforms in the KernelCI labs appears to be
>> affected.
>
> Hmm. My bet is that because this is a UP machine running an SMP
> kernel, and I fell into the trap of forgetting about this 32bit
> configuration.
>
> I expect the following patch to fix it. Please give it a go if you can
> (I'm away at the moment and can't test much, and do not have any
> physical 32bit machine to test this on).

OK thanks, that worked:

https://lava.baylibre.com/scheduler/job/143170

I've added this fix to the kernel branch used on
staging.kernelci.org which is based on linux-next, so it will get
fully verified a bit later today.

Guillaume


> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 00327fa74b01..b4e3d336dc33 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -531,7 +531,12 @@ void show_ipi_list(struct seq_file *p, int prec)
> unsigned int cpu, i;
>
> for (i = 0; i < NR_IPI; i++) {
> - unsigned int irq = irq_desc_get_irq(ipi_desc[i]);
> + unsigned int irq;
> +
> + if (!ipi_desc[i])
> + continue;
> +
> + irq = irq_desc_get_irq(ipi_desc[i]);
> seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
>
> for_each_online_cpu(cpu)
>
> Thanks,
>
> M.
>

2020-09-24 13:37:53

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

Hi Guillaume,

On Thu, Sep 24, 2020 at 6:01 AM Guillaume Tucker
<[email protected]> wrote:

> This appears to be causing a NULL pointer dereference on
> beaglebone-black, it got bisected automatically several times.
> None of the other platforms in the KernelCI labs appears to be
> affected.

Actually imx53-qsb is also affected:
https://storage.kernelci.org/next/master/next-20200924/arm/imx_v6_v7_defconfig/gcc-8/lab-pengutronix/baseline-imx53-qsrb.html

kernelci marks it Boot result: PASS though.

Shouldn't kernelci flag a warning or error instead?

Thanks

2020-09-24 14:24:01

by Guillaume Tucker

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

On 24/09/2020 14:34, Fabio Estevam wrote:
> Hi Guillaume,
>
> On Thu, Sep 24, 2020 at 6:01 AM Guillaume Tucker
> <[email protected]> wrote:
>
>> This appears to be causing a NULL pointer dereference on
>> beaglebone-black, it got bisected automatically several times.
>> None of the other platforms in the KernelCI labs appears to be
>> affected.
>
> Actually imx53-qsb is also affected:
> https://storage.kernelci.org/next/master/next-20200924/arm/imx_v6_v7_defconfig/gcc-8/lab-pengutronix/baseline-imx53-qsrb.html
>
> kernelci marks it Boot result: PASS though.
>
> Shouldn't kernelci flag a warning or error instead?

Thanks for bringing this up. The status in the HTML log file is
a very coarse one, in this case the board booted "fine" since it
reached a login prompt. The issue was detected later when
checking for errors in the kernel log.

But yes you're right, the issue is also impacting imx53-qsrb
indeed. I didn't spot that because it was only reported as a
regression on staging.kernelci.org, whereas imx53-qsrb is in the
Pengutronix lab which is not sending results there at the moment.

The failures can be found on the production web dashboard though,
but not as regressions:

beaglebone-black:

https://kernelci.org/test/case/id/5f6c7f1ab7c8c5472cbf9de9/

imx53-qsrb:

https://kernelci.org/test/case/id/5f6c7ea6f89a9d0f4dbf9ddf/


I need to investigate why that is the case, knowing that the
regression was detected correctly on staging which is the
development KernelCI instance:

https://staging.kernelci.org/test/plan/id/5f6bea67f724eb1b34dce581/


Thanks,
Guillaume


2020-09-28 09:02:27

by Guillaume Tucker

[permalink] [raw]
Subject: Re: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting

Hi Marc,

On 24/09/2020 14:09, Guillaume Tucker wrote:
> On 24/09/2020 10:29, Marc Zyngier wrote:
>> Hi Guillaume,
>>
>> On Thu, 24 Sep 2020 10:00:09 +0100,
>> Guillaume Tucker <[email protected]> wrote:
>>>
>>> Hi Marc,
>>>
>>> On 01/09/2020 15:43, Marc Zyngier wrote:
>>>> Let's switch the arm code to the core accounting, which already
>>>> does everything we need.
>>>>
>>>> Reviewed-by: Valentin Schneider <[email protected]>
>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>> ---
>>>> arch/arm/include/asm/hardirq.h | 17 -----------------
>>>> arch/arm/kernel/smp.c | 20 ++++----------------
>>>> 2 files changed, 4 insertions(+), 33 deletions(-)
>>>
>>> This appears to be causing a NULL pointer dereference on
>>> beaglebone-black, it got bisected automatically several times.
>>> None of the other platforms in the KernelCI labs appears to be
>>> affected.
>>
>> Hmm. My bet is that because this is a UP machine running an SMP
>> kernel, and I fell into the trap of forgetting about this 32bit
>> configuration.
>>
>> I expect the following patch to fix it. Please give it a go if you can
>> (I'm away at the moment and can't test much, and do not have any
>> physical 32bit machine to test this on).
>
> OK thanks, that worked:
>
> https://lava.baylibre.com/scheduler/job/143170
>
> I've added this fix to the kernel branch used on
> staging.kernelci.org which is based on linux-next, so it will get
> fully verified a bit later today.
>
> Guillaume
>
>
>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>> index 00327fa74b01..b4e3d336dc33 100644
>> --- a/arch/arm/kernel/smp.c
>> +++ b/arch/arm/kernel/smp.c
>> @@ -531,7 +531,12 @@ void show_ipi_list(struct seq_file *p, int prec)
>> unsigned int cpu, i;
>>
>> for (i = 0; i < NR_IPI; i++) {
>> - unsigned int irq = irq_desc_get_irq(ipi_desc[i]);
>> + unsigned int irq;
>> +
>> + if (!ipi_desc[i])
>> + continue;
>> +
>> + irq = irq_desc_get_irq(ipi_desc[i]);
>> seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
>>
>> for_each_online_cpu(cpu)

This fix has been all tested now, with no visible side effects:

https://staging.kernelci.org/test/job/kernelci/branch/staging.kernelci.org/kernel/staging-20200928.1/plan/baseline/

In the meantime, the same issue was detected (without the fix)
and bisected on sun5i-a13-olinuxino-micro and landed on the same
commit. A few more platforms are also impacted such as imx53-qsb
as mentioned by Fabio.

The commit is in your irqchip tree so I guess we should wait for
you to apply the fix. If you do make a separate commit to fix
the issue, please add:

Reported-by: kernelci.org bot <[email protected]>

and also:

Tested-by: Guillaume Tucker <[email protected]>

Thanks,
Guillaume