* [PATCH 01/11] powerpc/xive: Replace pr_devel() by pr_debug() to ease debug
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 02/11] powerpc/xive: Introduce an helper to print out interrupt characteristics Cédric Le Goater
` (10 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
These routines are not on hot code paths and pr_debug() is easier to
activate. Also add a '0x' prefix to hex printed values (HW IRQ number).
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 29 +++++++++++------------
arch/powerpc/sysdev/xive/spapr.c | 38 +++++++++++++++----------------
2 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index c5d75c02ad8b..7280ff3fef2d 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -451,6 +451,8 @@ static void xive_do_source_set_mask(struct xive_irq_data *xd,
{
u64 val;
+ pr_debug("%s: HW 0x%x %smask\n", __func__, xd->hw_irq, mask ? "" : "un");
+
/*
* If the interrupt had P set, it may be in a queue.
*
@@ -612,8 +614,8 @@ static unsigned int xive_irq_startup(struct irq_data *d)
xd->saved_p = false;
xd->stale_p = false;
- pr_devel("xive_irq_startup: irq %d [0x%x] data @%p\n",
- d->irq, hw_irq, d);
+
+ pr_debug("%s: irq %d [0x%x] data @%p\n", __func__, d->irq, hw_irq, d);
/* Pick a target */
target = xive_pick_irq_target(d, irq_data_get_affinity_mask(d));
@@ -654,8 +656,7 @@ static void xive_irq_shutdown(struct irq_data *d)
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
unsigned int hw_irq = (unsigned int)irqd_to_hwirq(d);
- pr_devel("xive_irq_shutdown: irq %d [0x%x] data @%p\n",
- d->irq, hw_irq, d);
+ pr_debug("%s: irq %d [0x%x] data @%p\n", __func__, d->irq, hw_irq, d);
if (WARN_ON(xd->target == XIVE_INVALID_TARGET))
return;
@@ -679,7 +680,7 @@ static void xive_irq_unmask(struct irq_data *d)
{
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
- pr_devel("xive_irq_unmask: irq %d data @%p\n", d->irq, xd);
+ pr_debug("%s: irq %d data @%p\n", __func__, d->irq, xd);
xive_do_source_set_mask(xd, false);
}
@@ -688,7 +689,7 @@ static void xive_irq_mask(struct irq_data *d)
{
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
- pr_devel("xive_irq_mask: irq %d data @%p\n", d->irq, xd);
+ pr_debug("%s: irq %d data @%p\n", __func__, d->irq, xd);
xive_do_source_set_mask(xd, true);
}
@@ -702,7 +703,7 @@ static int xive_irq_set_affinity(struct irq_data *d,
u32 target, old_target;
int rc = 0;
- pr_debug("%s: irq %d/%x\n", __func__, d->irq, hw_irq);
+ pr_debug("%s: irq %d/0x%x\n", __func__, d->irq, hw_irq);
/* Is this valid ? */
if (cpumask_any_and(cpumask, cpu_online_mask) >= nr_cpu_ids)
@@ -975,7 +976,7 @@ EXPORT_SYMBOL_GPL(is_xive_irq);
void xive_cleanup_irq_data(struct xive_irq_data *xd)
{
- pr_debug("%s for HW %x\n", __func__, xd->hw_irq);
+ pr_debug("%s for HW 0x%x\n", __func__, xd->hw_irq);
if (xd->eoi_mmio) {
iounmap(xd->eoi_mmio);
@@ -1211,8 +1212,8 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
pr_err("Failed to map IPI CPU %d\n", cpu);
return -EIO;
}
- pr_devel("CPU %d HW IPI %x, virq %d, trig_mmio=%p\n", cpu,
- xc->hw_ipi, xive_ipi_irq, xc->ipi_data.trig_mmio);
+ pr_debug("CPU %d HW IPI 0x%x, virq %d, trig_mmio=%p\n", cpu,
+ xc->hw_ipi, xive_ipi_irq, xc->ipi_data.trig_mmio);
/* Unmask it */
xive_do_source_set_mask(&xc->ipi_data, false);
@@ -1390,7 +1391,7 @@ static int xive_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
if (rc)
return rc;
- pr_debug("%s %d/%lx #%d\n", __func__, virq, hwirq, nr_irqs);
+ pr_debug("%s %d/0x%lx #%d\n", __func__, virq, hwirq, nr_irqs);
for (i = 0; i < nr_irqs; i++) {
/* TODO: call xive_irq_domain_map() */
@@ -1505,7 +1506,7 @@ static void xive_setup_cpu(void)
#ifdef CONFIG_SMP
void xive_smp_setup_cpu(void)
{
- pr_devel("SMP setup CPU %d\n", smp_processor_id());
+ pr_debug("SMP setup CPU %d\n", smp_processor_id());
/* This will have already been done on the boot CPU */
if (smp_processor_id() != boot_cpuid)
@@ -1651,10 +1652,10 @@ bool __init xive_core_init(struct device_node *np, const struct xive_ops *ops,
ppc_md.get_irq = xive_get_irq;
__xive_enabled = true;
- pr_devel("Initializing host..\n");
+ pr_debug("Initializing host..\n");
xive_init_host(np);
- pr_devel("Initializing boot CPU..\n");
+ pr_debug("Initializing boot CPU..\n");
/* Allocate per-CPU data and queues */
xive_prepare_cpu(smp_processor_id());
diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
index f143b6f111ac..77943dc70860 100644
--- a/arch/powerpc/sysdev/xive/spapr.c
+++ b/arch/powerpc/sysdev/xive/spapr.c
@@ -173,7 +173,7 @@ static long plpar_int_get_source_info(unsigned long flags,
} while (plpar_busy_delay(rc));
if (rc) {
- pr_err("H_INT_GET_SOURCE_INFO lisn=%ld failed %ld\n", lisn, rc);
+ pr_err("H_INT_GET_SOURCE_INFO lisn=0x%lx failed %ld\n", lisn, rc);
return rc;
}
@@ -182,8 +182,8 @@ static long plpar_int_get_source_info(unsigned long flags,
*trig_page = retbuf[2];
*esb_shift = retbuf[3];
- pr_devel("H_INT_GET_SOURCE_INFO flags=%lx eoi=%lx trig=%lx shift=%lx\n",
- retbuf[0], retbuf[1], retbuf[2], retbuf[3]);
+ pr_debug("H_INT_GET_SOURCE_INFO lisn=0x%lx flags=0x%lx eoi=0x%lx trig=0x%lx shift=0x%lx\n",
+ lisn, retbuf[0], retbuf[1], retbuf[2], retbuf[3]);
return 0;
}
@@ -200,8 +200,8 @@ static long plpar_int_set_source_config(unsigned long flags,
long rc;
- pr_devel("H_INT_SET_SOURCE_CONFIG flags=%lx lisn=%lx target=%lx prio=%lx sw_irq=%lx\n",
- flags, lisn, target, prio, sw_irq);
+ pr_debug("H_INT_SET_SOURCE_CONFIG flags=0x%lx lisn=0x%lx target=%ld prio=%ld sw_irq=%ld\n",
+ flags, lisn, target, prio, sw_irq);
do {
@@ -210,7 +210,7 @@ static long plpar_int_set_source_config(unsigned long flags,
} while (plpar_busy_delay(rc));
if (rc) {
- pr_err("H_INT_SET_SOURCE_CONFIG lisn=%ld target=%lx prio=%lx failed %ld\n",
+ pr_err("H_INT_SET_SOURCE_CONFIG lisn=0x%lx target=%ld prio=%ld failed %ld\n",
lisn, target, prio, rc);
return rc;
}
@@ -227,7 +227,7 @@ static long plpar_int_get_source_config(unsigned long flags,
unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
long rc;
- pr_devel("H_INT_GET_SOURCE_CONFIG flags=%lx lisn=%lx\n", flags, lisn);
+ pr_debug("H_INT_GET_SOURCE_CONFIG flags=0x%lx lisn=0x%lx\n", flags, lisn);
do {
rc = plpar_hcall(H_INT_GET_SOURCE_CONFIG, retbuf, flags, lisn,
@@ -235,7 +235,7 @@ static long plpar_int_get_source_config(unsigned long flags,
} while (plpar_busy_delay(rc));
if (rc) {
- pr_err("H_INT_GET_SOURCE_CONFIG lisn=%ld failed %ld\n",
+ pr_err("H_INT_GET_SOURCE_CONFIG lisn=0x%lx failed %ld\n",
lisn, rc);
return rc;
}
@@ -244,8 +244,8 @@ static long plpar_int_get_source_config(unsigned long flags,
*prio = retbuf[1];
*sw_irq = retbuf[2];
- pr_devel("H_INT_GET_SOURCE_CONFIG target=%lx prio=%lx sw_irq=%lx\n",
- retbuf[0], retbuf[1], retbuf[2]);
+ pr_debug("H_INT_GET_SOURCE_CONFIG target=%ld prio=%ld sw_irq=%ld\n",
+ retbuf[0], retbuf[1], retbuf[2]);
return 0;
}
@@ -273,8 +273,8 @@ static long plpar_int_get_queue_info(unsigned long flags,
*esn_page = retbuf[0];
*esn_size = retbuf[1];
- pr_devel("H_INT_GET_QUEUE_INFO page=%lx size=%lx\n",
- retbuf[0], retbuf[1]);
+ pr_debug("H_INT_GET_QUEUE_INFO cpu=%ld prio=%ld page=0x%lx size=0x%lx\n",
+ target, priority, retbuf[0], retbuf[1]);
return 0;
}
@@ -289,8 +289,8 @@ static long plpar_int_set_queue_config(unsigned long flags,
{
long rc;
- pr_devel("H_INT_SET_QUEUE_CONFIG flags=%lx target=%lx priority=%lx qpage=%lx qsize=%lx\n",
- flags, target, priority, qpage, qsize);
+ pr_debug("H_INT_SET_QUEUE_CONFIG flags=0x%lx target=%ld priority=0x%lx qpage=0x%lx qsize=0x%lx\n",
+ flags, target, priority, qpage, qsize);
do {
rc = plpar_hcall_norets(H_INT_SET_QUEUE_CONFIG, flags, target,
@@ -298,7 +298,7 @@ static long plpar_int_set_queue_config(unsigned long flags,
} while (plpar_busy_delay(rc));
if (rc) {
- pr_err("H_INT_SET_QUEUE_CONFIG cpu=%ld prio=%ld qpage=%lx returned %ld\n",
+ pr_err("H_INT_SET_QUEUE_CONFIG cpu=%ld prio=%ld qpage=0x%lx returned %ld\n",
target, priority, qpage, rc);
return rc;
}
@@ -315,7 +315,7 @@ static long plpar_int_sync(unsigned long flags, unsigned long lisn)
} while (plpar_busy_delay(rc));
if (rc) {
- pr_err("H_INT_SYNC lisn=%ld returned %ld\n", lisn, rc);
+ pr_err("H_INT_SYNC lisn=0x%lx returned %ld\n", lisn, rc);
return rc;
}
@@ -333,8 +333,8 @@ static long plpar_int_esb(unsigned long flags,
unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
long rc;
- pr_devel("H_INT_ESB flags=%lx lisn=%lx offset=%lx in=%lx\n",
- flags, lisn, offset, in_data);
+ pr_debug("H_INT_ESB flags=0x%lx lisn=0x%lx offset=0x%lx in=0x%lx\n",
+ flags, lisn, offset, in_data);
do {
rc = plpar_hcall(H_INT_ESB, retbuf, flags, lisn, offset,
@@ -342,7 +342,7 @@ static long plpar_int_esb(unsigned long flags,
} while (plpar_busy_delay(rc));
if (rc) {
- pr_err("H_INT_ESB lisn=%ld offset=%ld returned %ld\n",
+ pr_err("H_INT_ESB lisn=0x%lx offset=0x%lx returned %ld\n",
lisn, offset, rc);
return rc;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 02/11] powerpc/xive: Introduce an helper to print out interrupt characteristics
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
2021-11-05 10:26 ` [PATCH 01/11] powerpc/xive: Replace pr_devel() by pr_debug() to ease debug Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 03/11] powerpc/xive: Activate StoreEOI on P10 Cédric Le Goater
` (9 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
and extend output of debugfs and xmon with addresses of the ESB
management and trigger pages.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 54 +++++++++++++++----------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 7280ff3fef2d..3d558cad1f19 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -227,6 +227,19 @@ static void xive_esb_write(struct xive_irq_data *xd, u32 offset, u64 data)
out_be64(xd->eoi_mmio + offset, data);
}
+static void xive_irq_data_dump(struct xive_irq_data *xd, char *buffer, size_t size)
+{
+ u64 val = xive_esb_read(xd, XIVE_ESB_GET);
+
+ snprintf(buffer, size, "flags=%c%c%c PQ=%c%c 0x%016llx 0x%016llx",
+ xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
+ xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
+ xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
+ val & XIVE_ESB_VAL_P ? 'P' : '-',
+ val & XIVE_ESB_VAL_Q ? 'Q' : '-',
+ xd->trig_page, xd->eoi_page);
+}
+
#ifdef CONFIG_XMON
static notrace void xive_dump_eq(const char *name, struct xive_q *q)
{
@@ -252,11 +265,10 @@ notrace void xmon_xive_do_dump(int cpu)
#ifdef CONFIG_SMP
{
- u64 val = xive_esb_read(&xc->ipi_data, XIVE_ESB_GET);
+ char buffer[128];
- xmon_printf("IPI=0x%08x PQ=%c%c ", xc->hw_ipi,
- val & XIVE_ESB_VAL_P ? 'P' : '-',
- val & XIVE_ESB_VAL_Q ? 'Q' : '-');
+ xive_irq_data_dump(&xc->ipi_data, buffer, sizeof(buffer));
+ xmon_printf("IPI=0x%08x %s", xc->hw_ipi, buffer);
}
#endif
xive_dump_eq("EQ", &xc->queue[xive_irq_priority]);
@@ -291,15 +303,11 @@ int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
d = xive_get_irq_data(hw_irq);
if (d) {
- struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
- u64 val = xive_esb_read(xd, XIVE_ESB_GET);
-
- xmon_printf("flags=%c%c%c PQ=%c%c",
- xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
- xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
- xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
- val & XIVE_ESB_VAL_P ? 'P' : '-',
- val & XIVE_ESB_VAL_Q ? 'Q' : '-');
+ char buffer[128];
+
+ xive_irq_data_dump(irq_data_get_irq_handler_data(d),
+ buffer, sizeof(buffer));
+ xmon_printf("%s", buffer);
}
xmon_printf("\n");
@@ -1703,11 +1711,10 @@ static void xive_debug_show_cpu(struct seq_file *m, int cpu)
#ifdef CONFIG_SMP
{
- u64 val = xive_esb_read(&xc->ipi_data, XIVE_ESB_GET);
+ char buffer[128];
- seq_printf(m, "IPI=0x%08x PQ=%c%c ", xc->hw_ipi,
- val & XIVE_ESB_VAL_P ? 'P' : '-',
- val & XIVE_ESB_VAL_Q ? 'Q' : '-');
+ xive_irq_data_dump(&xc->ipi_data, buffer, sizeof(buffer));
+ seq_printf(m, "IPI=0x%08x %s", xc->hw_ipi, buffer);
}
#endif
{
@@ -1734,8 +1741,7 @@ static void xive_debug_show_irq(struct seq_file *m, struct irq_data *d)
u32 target;
u8 prio;
u32 lirq;
- struct xive_irq_data *xd;
- u64 val;
+ char buffer[128];
rc = xive_ops->get_irq_config(hw_irq, &target, &prio, &lirq);
if (rc) {
@@ -1746,14 +1752,8 @@ static void xive_debug_show_irq(struct seq_file *m, struct irq_data *d)
seq_printf(m, "IRQ 0x%08x : target=0x%x prio=%02x lirq=0x%x ",
hw_irq, target, prio, lirq);
- xd = irq_data_get_irq_handler_data(d);
- val = xive_esb_read(xd, XIVE_ESB_GET);
- seq_printf(m, "flags=%c%c%c PQ=%c%c",
- xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
- xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
- xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
- val & XIVE_ESB_VAL_P ? 'P' : '-',
- val & XIVE_ESB_VAL_Q ? 'Q' : '-');
+ xive_irq_data_dump(irq_data_get_irq_handler_data(d), buffer, sizeof(buffer));
+ seq_puts(m, buffer);
seq_puts(m, "\n");
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 03/11] powerpc/xive: Activate StoreEOI on P10
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
2021-11-05 10:26 ` [PATCH 01/11] powerpc/xive: Replace pr_devel() by pr_debug() to ease debug Cédric Le Goater
2021-11-05 10:26 ` [PATCH 02/11] powerpc/xive: Introduce an helper to print out interrupt characteristics Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create() Cédric Le Goater
` (8 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
StoreEOI (the capability to EOI with a store) requires load-after-store
ordering in some cases to be reliable. P10 introduced a new offset for
load operations to enforce correct ordering and the XIVE driver has
the required support since kernel 5.8, commit b1f9be9392f0
("powerpc/xive: Enforce load-after-store ordering when StoreEOI is active")
Since skiboot v7, StoreEOI support is advertised on P10 with a new flag
on the PowerNV platform. See skiboot commit 4bd7d84afe46 ("xive/p10:
Introduce a new OPAL_XIVE_IRQ_STORE_EOI2 flag"). When detected,
activate the feature.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/opal-api.h | 1 +
arch/powerpc/sysdev/xive/native.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 0b63ba7d5917..a2bc4b95e703 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -1094,6 +1094,7 @@ enum {
OPAL_XIVE_IRQ_SHIFT_BUG = 0x00000008, /* P9 DD1.0 workaround */
OPAL_XIVE_IRQ_MASK_VIA_FW = 0x00000010, /* P9 DD1.0 workaround */
OPAL_XIVE_IRQ_EOI_VIA_FW = 0x00000020, /* P9 DD1.0 workaround */
+ OPAL_XIVE_IRQ_STORE_EOI2 = 0x00000040,
};
/* Flags for OPAL_XIVE_GET/SET_QUEUE_INFO */
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 1aec282cd650..7ec8911dad57 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -63,6 +63,8 @@ int xive_native_populate_irq_data(u32 hw_irq, struct xive_irq_data *data)
opal_flags = be64_to_cpu(flags);
if (opal_flags & OPAL_XIVE_IRQ_STORE_EOI)
data->flags |= XIVE_IRQ_FLAG_STORE_EOI;
+ if (opal_flags & OPAL_XIVE_IRQ_STORE_EOI2)
+ data->flags |= XIVE_IRQ_FLAG_STORE_EOI;
if (opal_flags & OPAL_XIVE_IRQ_LSI)
data->flags |= XIVE_IRQ_FLAG_LSI;
data->eoi_page = be64_to_cpu(eoi_page);
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create()
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (2 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 03/11] powerpc/xive: Activate StoreEOI on P10 Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-18 9:21 ` Michael Ellerman
2021-11-05 10:26 ` [PATCH 05/11] powerpc/xive: Change the debugfs file 'xive' into a directory Cédric Le Goater
` (7 subsequent siblings)
11 siblings, 1 reply; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
and fix some compile issues when !CONFIG_DEBUG_FS.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 3d558cad1f19..b71cc1020296 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -227,6 +227,7 @@ static void xive_esb_write(struct xive_irq_data *xd, u32 offset, u64 data)
out_be64(xd->eoi_mmio + offset, data);
}
+#if defined(CONFIG_XMON) || defined(CONFIG_DEBUG_FS)
static void xive_irq_data_dump(struct xive_irq_data *xd, char *buffer, size_t size)
{
u64 val = xive_esb_read(xd, XIVE_ESB_GET);
@@ -239,6 +240,7 @@ static void xive_irq_data_dump(struct xive_irq_data *xd, char *buffer, size_t si
val & XIVE_ESB_VAL_Q ? 'Q' : '-',
xd->trig_page, xd->eoi_page);
}
+#endif
#ifdef CONFIG_XMON
static notrace void xive_dump_eq(const char *name, struct xive_q *q)
@@ -1701,6 +1703,7 @@ static int __init xive_off(char *arg)
}
__setup("xive=off", xive_off);
+#ifdef CONFIG_DEBUG_FS
static void xive_debug_show_cpu(struct seq_file *m, int cpu)
{
struct xive_cpu *xc = per_cpu(xive_cpu, cpu);
@@ -1779,10 +1782,18 @@ static int xive_core_debug_show(struct seq_file *m, void *private)
}
DEFINE_SHOW_ATTRIBUTE(xive_core_debug);
+static void xive_core_debugfs_create(void)
+{
+ debugfs_create_file("xive", 0400, arch_debugfs_dir,
+ NULL, &xive_core_debug_fops);
+}
+
+#endif /* CONFIG_DEBUG_FS */
+
int xive_core_debug_init(void)
{
- if (xive_enabled())
- debugfs_create_file("xive", 0400, arch_debugfs_dir,
- NULL, &xive_core_debug_fops);
+ if (xive_enabled() && IS_ENABLED(CONFIG_DEBUG_FS))
+ xive_core_debugfs_create();
+
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create()
2021-11-05 10:26 ` [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create() Cédric Le Goater
@ 2021-11-18 9:21 ` Michael Ellerman
2021-11-18 15:22 ` Cédric Le Goater
0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2021-11-18 9:21 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Cédric Le Goater
Cédric Le Goater <clg@kaod.org> writes:
> and fix some compile issues when !CONFIG_DEBUG_FS.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 3d558cad1f19..b71cc1020296 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
...
> @@ -1779,10 +1782,18 @@ static int xive_core_debug_show(struct seq_file *m, void *private)
> }
> DEFINE_SHOW_ATTRIBUTE(xive_core_debug);
>
> +static void xive_core_debugfs_create(void)
> +{
> + debugfs_create_file("xive", 0400, arch_debugfs_dir,
> + NULL, &xive_core_debug_fops);
> +}
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
> int xive_core_debug_init(void)
> {
> - if (xive_enabled())
> - debugfs_create_file("xive", 0400, arch_debugfs_dir,
> - NULL, &xive_core_debug_fops);
> + if (xive_enabled() && IS_ENABLED(CONFIG_DEBUG_FS))
> + xive_core_debugfs_create();
> +
> return 0;
> }
For skiroot_defconfig this gives me:
arch/powerpc/sysdev/xive/common.c: In function ‘xive_core_init’:
arch/powerpc/sysdev/xive/common.c:1676:2: error: implicit declaration of function ‘xive_core_debugfs_create’; did you mean ‘xive_core_debug_init’? [-Werror=implicit-function-declaration]
1676 | xive_core_debugfs_create();
| ^~~~~~~~~~~~~~~~~~~~~~~~
| xive_core_debug_init
cc1: all warnings being treated as errors
We need an empty inline stub of xive_core_debugfs_create() for the
CONFIG_DEBUG_FS=n case.
I'm wondering though why do we have xive_core_debug_init() at all, why
don't we just initialise the debugfs files in xive_core_init()?
cheers
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create()
2021-11-18 9:21 ` Michael Ellerman
@ 2021-11-18 15:22 ` Cédric Le Goater
0 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-18 15:22 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
On 11/18/21 10:21, Michael Ellerman wrote:
> Cédric Le Goater <clg@kaod.org> writes:
>
>> and fix some compile issues when !CONFIG_DEBUG_FS.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
>> index 3d558cad1f19..b71cc1020296 100644
>> --- a/arch/powerpc/sysdev/xive/common.c
>> +++ b/arch/powerpc/sysdev/xive/common.c
> ...
>> @@ -1779,10 +1782,18 @@ static int xive_core_debug_show(struct seq_file *m, void *private)
>> }
>> DEFINE_SHOW_ATTRIBUTE(xive_core_debug);
>>
>> +static void xive_core_debugfs_create(void)
>> +{
>> + debugfs_create_file("xive", 0400, arch_debugfs_dir,
>> + NULL, &xive_core_debug_fops);
>> +}
>> +
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>> int xive_core_debug_init(void)
>> {
>> - if (xive_enabled())
>> - debugfs_create_file("xive", 0400, arch_debugfs_dir,
>> - NULL, &xive_core_debug_fops);
>> + if (xive_enabled() && IS_ENABLED(CONFIG_DEBUG_FS))
>> + xive_core_debugfs_create();
>> +
>> return 0;
>> }
>
> For skiroot_defconfig this gives me:
>
> arch/powerpc/sysdev/xive/common.c: In function ‘xive_core_init’:
> arch/powerpc/sysdev/xive/common.c:1676:2: error: implicit declaration of function ‘xive_core_debugfs_create’; did you mean ‘xive_core_debug_init’? [-Werror=implicit-function-declaration]
> 1676 | xive_core_debugfs_create();
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> | xive_core_debug_init
> cc1: all warnings being treated as errors
>
>
> We need an empty inline stub of xive_core_debugfs_create() for the
> CONFIG_DEBUG_FS=n case.
I thought IS_ENABLED(CONFIG_DEBUG_FS)) was protecting compile from
that issue. Do you want a v2 for the whole ?
Or, I can send a replacement for patch 4 only.
> I'm wondering though why do we have xive_core_debug_init() at all, why
> don't we just initialise the debugfs files in xive_core_init()?
I think I tried that and there was an ordering issue.
Thanks,
C.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 05/11] powerpc/xive: Change the debugfs file 'xive' into a directory
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (3 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create() Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 06/11] powerpc/xive: Rename the 'cpus' debugfs file to 'ipis' Cédric Le Goater
` (6 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
Use a 'cpus' file to dump CPU states and 'interrupts' to dump IRQ states.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 36 +++++++++++++++++++++----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index b71cc1020296..0b34ad5748ee 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1760,17 +1760,10 @@ static void xive_debug_show_irq(struct seq_file *m, struct irq_data *d)
seq_puts(m, "\n");
}
-static int xive_core_debug_show(struct seq_file *m, void *private)
+static int xive_irq_debug_show(struct seq_file *m, void *private)
{
unsigned int i;
struct irq_desc *desc;
- int cpu;
-
- if (xive_ops->debug_show)
- xive_ops->debug_show(m, private);
-
- for_each_possible_cpu(cpu)
- xive_debug_show_cpu(m, cpu);
for_each_irq_desc(i, desc) {
struct irq_data *d = irq_domain_get_irq_data(xive_irq_domain, i);
@@ -1780,12 +1773,33 @@ static int xive_core_debug_show(struct seq_file *m, void *private)
}
return 0;
}
-DEFINE_SHOW_ATTRIBUTE(xive_core_debug);
+DEFINE_SHOW_ATTRIBUTE(xive_irq_debug);
+
+static int xive_cpu_debug_show(struct seq_file *m, void *private)
+{
+ int cpu;
+
+ if (xive_ops->debug_show)
+ xive_ops->debug_show(m, private);
+
+ for_each_possible_cpu(cpu)
+ xive_debug_show_cpu(m, cpu);
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(xive_cpu_debug);
static void xive_core_debugfs_create(void)
{
- debugfs_create_file("xive", 0400, arch_debugfs_dir,
- NULL, &xive_core_debug_fops);
+ struct dentry *xive_dir;
+
+ xive_dir = debugfs_create_dir("xive", arch_debugfs_dir);
+ if (IS_ERR(xive_dir))
+ return;
+
+ debugfs_create_file("cpus", 0400, xive_dir,
+ NULL, &xive_cpu_debug_fops);
+ debugfs_create_file("interrupts", 0400, xive_dir,
+ NULL, &xive_irq_debug_fops);
}
#endif /* CONFIG_DEBUG_FS */
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 06/11] powerpc/xive: Rename the 'cpus' debugfs file to 'ipis'
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (4 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 05/11] powerpc/xive: Change the debugfs file 'xive' into a directory Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 07/11] powerpc/xive: Add a debugfs file to dump EQs Cédric Le Goater
` (5 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
and remove the EQ entries output which is not very useful since only
the next two events of the queue are taken into account. We will
improve the dump of the EQ in the next patches.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 0b34ad5748ee..c5167f284da5 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1704,11 +1704,11 @@ static int __init xive_off(char *arg)
__setup("xive=off", xive_off);
#ifdef CONFIG_DEBUG_FS
-static void xive_debug_show_cpu(struct seq_file *m, int cpu)
+static void xive_debug_show_ipi(struct seq_file *m, int cpu)
{
struct xive_cpu *xc = per_cpu(xive_cpu, cpu);
- seq_printf(m, "CPU %d:", cpu);
+ seq_printf(m, "CPU %d: ", cpu);
if (xc) {
seq_printf(m, "pp=%02x CPPR=%02x ", xc->pending_prio, xc->cppr);
@@ -1720,19 +1720,6 @@ static void xive_debug_show_cpu(struct seq_file *m, int cpu)
seq_printf(m, "IPI=0x%08x %s", xc->hw_ipi, buffer);
}
#endif
- {
- struct xive_q *q = &xc->queue[xive_irq_priority];
- u32 i0, i1, idx;
-
- if (q->qpage) {
- idx = q->idx;
- i0 = be32_to_cpup(q->qpage + idx);
- idx = (idx + 1) & q->msk;
- i1 = be32_to_cpup(q->qpage + idx);
- seq_printf(m, "EQ idx=%d T=%d %08x %08x ...",
- q->idx, q->toggle, i0, i1);
- }
- }
}
seq_puts(m, "\n");
}
@@ -1775,7 +1762,7 @@ static int xive_irq_debug_show(struct seq_file *m, void *private)
}
DEFINE_SHOW_ATTRIBUTE(xive_irq_debug);
-static int xive_cpu_debug_show(struct seq_file *m, void *private)
+static int xive_ipi_debug_show(struct seq_file *m, void *private)
{
int cpu;
@@ -1783,10 +1770,10 @@ static int xive_cpu_debug_show(struct seq_file *m, void *private)
xive_ops->debug_show(m, private);
for_each_possible_cpu(cpu)
- xive_debug_show_cpu(m, cpu);
+ xive_debug_show_ipi(m, cpu);
return 0;
}
-DEFINE_SHOW_ATTRIBUTE(xive_cpu_debug);
+DEFINE_SHOW_ATTRIBUTE(xive_ipi_debug);
static void xive_core_debugfs_create(void)
{
@@ -1796,8 +1783,8 @@ static void xive_core_debugfs_create(void)
if (IS_ERR(xive_dir))
return;
- debugfs_create_file("cpus", 0400, xive_dir,
- NULL, &xive_cpu_debug_fops);
+ debugfs_create_file("ipis", 0400, xive_dir,
+ NULL, &xive_ipi_debug_fops);
debugfs_create_file("interrupts", 0400, xive_dir,
NULL, &xive_irq_debug_fops);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 07/11] powerpc/xive: Add a debugfs file to dump EQs
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (5 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 06/11] powerpc/xive: Rename the 'cpus' debugfs file to 'ipis' Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 08/11] powerpc/xive: Add a debugfs toggle for StoreEOI Cédric Le Goater
` (4 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
The XIVE driver under Linux uses a single interrupt priority and only
one event queue is configured per CPU. Expose the contents under
a 'xive/eqs/cpuX' debugfs file.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 37 +++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index c5167f284da5..75c683ffae7e 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1775,9 +1775,40 @@ static int xive_ipi_debug_show(struct seq_file *m, void *private)
}
DEFINE_SHOW_ATTRIBUTE(xive_ipi_debug);
+static void xive_eq_debug_show_one(struct seq_file *m, struct xive_q *q, u8 prio)
+{
+ int i;
+
+ seq_printf(m, "EQ%d idx=%d T=%d\n", prio, q->idx, q->toggle);
+ if (q->qpage) {
+ for (i = 0; i < q->msk + 1; i++) {
+ if (!(i % 8))
+ seq_printf(m, "%05d ", i);
+ seq_printf(m, "%08x%s", be32_to_cpup(q->qpage + i),
+ (i + 1) % 8 ? " " : "\n");
+ }
+ }
+ seq_puts(m, "\n");
+}
+
+static int xive_eq_debug_show(struct seq_file *m, void *private)
+{
+ int cpu = (long)m->private;
+ struct xive_cpu *xc = per_cpu(xive_cpu, cpu);
+
+ if (xc)
+ xive_eq_debug_show_one(m, &xc->queue[xive_irq_priority],
+ xive_irq_priority);
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(xive_eq_debug);
+
static void xive_core_debugfs_create(void)
{
struct dentry *xive_dir;
+ struct dentry *xive_eq_dir;
+ long cpu;
+ char name[16];
xive_dir = debugfs_create_dir("xive", arch_debugfs_dir);
if (IS_ERR(xive_dir))
@@ -1787,6 +1818,12 @@ static void xive_core_debugfs_create(void)
NULL, &xive_ipi_debug_fops);
debugfs_create_file("interrupts", 0400, xive_dir,
NULL, &xive_irq_debug_fops);
+ xive_eq_dir = debugfs_create_dir("eqs", xive_dir);
+ for_each_possible_cpu(cpu) {
+ snprintf(name, sizeof(name), "cpu%ld", cpu);
+ debugfs_create_file(name, 0400, xive_eq_dir, (void *)cpu,
+ &xive_eq_debug_fops);
+ }
}
#endif /* CONFIG_DEBUG_FS */
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 08/11] powerpc/xive: Add a debugfs toggle for StoreEOI
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (6 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 07/11] powerpc/xive: Add a debugfs file to dump EQs Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 09/11] powerpc/xive: Add a kernel parameter " Cédric Le Goater
` (3 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
It can be used to deactivate temporarily StoreEOI for tests or
performance on platforms supporting the feature (POWER10)
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 75c683ffae7e..11e2aaa13965 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -84,6 +84,16 @@ static DEFINE_PER_CPU(struct xive_cpu *, xive_cpu);
/* An invalid CPU target */
#define XIVE_INVALID_TARGET (-1)
+/*
+ * Global toggle to switch on/off StoreEOI
+ */
+static bool xive_store_eoi = true;
+
+static bool xive_is_store_eoi(struct xive_irq_data *xd)
+{
+ return xd->flags & XIVE_IRQ_FLAG_STORE_EOI && xive_store_eoi;
+}
+
/*
* Read the next entry in a queue, return its content if it's valid
* or 0 if there is no new entry.
@@ -208,7 +218,7 @@ static notrace u8 xive_esb_read(struct xive_irq_data *xd, u32 offset)
{
u64 val;
- if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
+ if (offset == XIVE_ESB_SET_PQ_10 && xive_is_store_eoi(xd))
offset |= XIVE_ESB_LD_ST_MO;
if ((xd->flags & XIVE_IRQ_FLAG_H_INT_ESB) && xive_ops->esb_rw)
@@ -233,7 +243,7 @@ static void xive_irq_data_dump(struct xive_irq_data *xd, char *buffer, size_t si
u64 val = xive_esb_read(xd, XIVE_ESB_GET);
snprintf(buffer, size, "flags=%c%c%c PQ=%c%c 0x%016llx 0x%016llx",
- xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
+ xive_is_store_eoi(xd) ? 'S' : ' ',
xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
val & XIVE_ESB_VAL_P ? 'P' : '-',
@@ -395,7 +405,7 @@ static void xive_do_source_eoi(struct xive_irq_data *xd)
xd->stale_p = false;
/* If the XIVE supports the new "store EOI facility, use it */
- if (xd->flags & XIVE_IRQ_FLAG_STORE_EOI) {
+ if (xive_is_store_eoi(xd)) {
xive_esb_write(xd, XIVE_ESB_STORE_EOI, 0);
return;
}
@@ -1824,6 +1834,7 @@ static void xive_core_debugfs_create(void)
debugfs_create_file(name, 0400, xive_eq_dir, (void *)cpu,
&xive_eq_debug_fops);
}
+ debugfs_create_bool("store-eoi", 0600, xive_dir, &xive_store_eoi);
}
#endif /* CONFIG_DEBUG_FS */
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 09/11] powerpc/xive: Add a kernel parameter for StoreEOI
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (7 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 08/11] powerpc/xive: Add a debugfs toggle for StoreEOI Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 10/11] powerpc/xive: Add a debugfs toggle for save-restore Cédric Le Goater
` (2 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
StoreEOI is activated by default on platforms supporting the feature
(POWER10) and will be used as soon as firmware advertises its
availability. The kernel parameter provides a way to deactivate its
use. It can be still be reactivated through debugfs.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 13 +++++++++++++
Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
2 files changed, 19 insertions(+)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 11e2aaa13965..67fd3a306369 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1713,6 +1713,19 @@ static int __init xive_off(char *arg)
}
__setup("xive=off", xive_off);
+static int __init xive_store_eoi_cmdline(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ if (strncmp(arg, "off", 3) == 0) {
+ pr_info("StoreEOI disabled on kernel command line\n");
+ xive_store_eoi = false;
+ }
+ return 0;
+}
+__setup("xive.store-eoi=", xive_store_eoi_cmdline);
+
#ifdef CONFIG_DEBUG_FS
static void xive_debug_show_ipi(struct seq_file *m, int cpu)
{
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 43dc35fe5bc0..10fa093251e8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6387,6 +6387,12 @@
controller on both pseries and powernv
platforms. Only useful on POWER9 and above.
+ xive.store-eoi=off [PPC]
+ By default on POWER10 and above, the kernel will use
+ stores for EOI handling when the XIVE interrupt mode
+ is active. This option allows the XIVE driver to use
+ loads instead, as on POWER9.
+
xhci-hcd.quirks [USB,KNL]
A hex value specifying bitmask with supplemental xhci
host controller quirks. Meaning of each bit can be
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 10/11] powerpc/xive: Add a debugfs toggle for save-restore
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (8 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 09/11] powerpc/xive: Add a kernel parameter " Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-05 10:26 ` [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter Cédric Le Goater
2021-11-25 9:36 ` [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Michael Ellerman
11 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
On POWER10, the automatic "save & restore" of interrupt context is
always available. Provide a way to deactivate it for tests or
performance.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/xive-internal.h | 1 +
arch/powerpc/sysdev/xive/common.c | 1 +
arch/powerpc/sysdev/xive/native.c | 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
index 504e7edce358..e0941bc64430 100644
--- a/arch/powerpc/sysdev/xive/xive-internal.h
+++ b/arch/powerpc/sysdev/xive/xive-internal.h
@@ -72,5 +72,6 @@ static inline u32 xive_alloc_order(u32 queue_shift)
}
extern bool xive_cmdline_disabled;
+extern bool xive_has_save_restore;
#endif /* __XIVE_INTERNAL_H */
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 67fd3a306369..39142df828a0 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1848,6 +1848,7 @@ static void xive_core_debugfs_create(void)
&xive_eq_debug_fops);
}
debugfs_create_bool("store-eoi", 0600, xive_dir, &xive_store_eoi);
+ debugfs_create_bool("save-restore", 0600, xive_dir, &xive_has_save_restore);
}
#endif /* CONFIG_DEBUG_FS */
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 7ec8911dad57..d6a091dc1bce 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -41,7 +41,7 @@ static u32 xive_queue_shift;
static u32 xive_pool_vps = XIVE_INVALID_VP;
static struct kmem_cache *xive_provision_cache;
static bool xive_has_single_esc;
-static bool xive_has_save_restore;
+bool xive_has_save_restore;
int xive_native_populate_irq_data(u32 hw_irq, struct xive_irq_data *data)
{
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (9 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 10/11] powerpc/xive: Add a debugfs toggle for save-restore Cédric Le Goater
@ 2021-11-05 10:26 ` Cédric Le Goater
2021-11-11 10:41 ` Michael Ellerman
2021-11-25 9:36 ` [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Michael Ellerman
11 siblings, 1 reply; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-05 10:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
On processors with a XIVE interrupt controller (POWER9 and above), the
kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
doorbell is generally preferred to using the XIVE IC because it is
faster. There are cases where we want to avoid doorbells and use XIVE
only, for debug or performance. Only useful on POWER9 and above.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/include/asm/dbell.h | 1 +
arch/powerpc/kernel/dbell.c | 17 +++++++++++++++++
arch/powerpc/platforms/powernv/smp.c | 7 +++++--
arch/powerpc/platforms/pseries/smp.c | 3 +++
Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
5 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
index 3e9da22a2779..07775aa3e81b 100644
--- a/arch/powerpc/include/asm/dbell.h
+++ b/arch/powerpc/include/asm/dbell.h
@@ -90,6 +90,7 @@ static inline void ppc_msgsync(void)
#endif /* CONFIG_PPC_BOOK3S */
extern void doorbell_exception(struct pt_regs *regs);
+extern bool doorbell_disabled;
static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
{
diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
index 5545c9cd17c1..681ee4775629 100644
--- a/arch/powerpc/kernel/dbell.c
+++ b/arch/powerpc/kernel/dbell.c
@@ -38,6 +38,23 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
set_irq_regs(old_regs);
}
+
+bool doorbell_disabled;
+
+static int __init doorbell_cmdline(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ if (strncmp(arg, "off", 3) == 0) {
+ pr_info("Doorbell disabled on kernel command line\n");
+ doorbell_disabled = true;
+ }
+
+ return 0;
+}
+__setup("doorbell=", doorbell_cmdline);
+
#else /* CONFIG_SMP */
DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
{
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index cbb67813cd5d..1311bda9446a 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -338,10 +338,13 @@ static void __init pnv_smp_probe(void)
ic_cause_ipi = smp_ops->cause_ipi;
WARN_ON(!ic_cause_ipi);
- if (cpu_has_feature(CPU_FTR_ARCH_300))
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (doorbell_disabled)
+ return;
smp_ops->cause_ipi = doorbell_global_ipi;
- else
+ } else {
smp_ops->cause_ipi = pnv_cause_ipi;
+ }
}
}
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index f47429323eee..3bc9e6aaf645 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -229,6 +229,9 @@ static __init void pSeries_smp_probe(void)
return;
}
+ if (doorbell_disabled)
+ return;
+
/*
* Under PowerVM, FSCR[MSGP] is enabled as guest vCPU siblings are
* gang scheduled on the same physical core, so doorbells are always
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 10fa093251e8..2e1284febe39 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1041,6 +1041,16 @@
The filter can be disabled or changed to another
driver later using sysfs.
+ doorbell=off [PPC]
+ On processors with a XIVE interrupt controller
+ (POWER9 and above), the kernel can use either
+ doorbells or XIVE to generate CPU IPIs. Sending
+ doorbell is generally preferred to using the XIVE
+ IC because it is faster. There are cases where
+ we want to avoid doorbells and use XIVE only,
+ for debug or performance. Only useful on
+ POWER9 and above.
+
driver_async_probe= [KNL]
List of driver names to be probed asynchronously.
Format: <driver_name1>,<driver_name2>...
--
2.31.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter
2021-11-05 10:26 ` [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter Cédric Le Goater
@ 2021-11-11 10:41 ` Michael Ellerman
2021-11-11 16:01 ` Cédric Le Goater
0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2021-11-11 10:41 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Cédric Le Goater
Cédric Le Goater <clg@kaod.org> writes:
> On processors with a XIVE interrupt controller (POWER9 and above), the
> kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
> doorbell is generally preferred to using the XIVE IC because it is
> faster. There are cases where we want to avoid doorbells and use XIVE
> only, for debug or performance. Only useful on POWER9 and above.
How much do we want this?
Kernel command line args are a bit of a pain, they tend to be poorly
tested, because someone has to explicitly enable them at boot time, and
then reboot to test the other case.
When would we want to enable this? Can we make the kernel smarter about
when to use doorbells and make it automated?
Could we make it a runtime switch?
cheers
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/include/asm/dbell.h | 1 +
> arch/powerpc/kernel/dbell.c | 17 +++++++++++++++++
> arch/powerpc/platforms/powernv/smp.c | 7 +++++--
> arch/powerpc/platforms/pseries/smp.c | 3 +++
> Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
> 5 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
> index 3e9da22a2779..07775aa3e81b 100644
> --- a/arch/powerpc/include/asm/dbell.h
> +++ b/arch/powerpc/include/asm/dbell.h
> @@ -90,6 +90,7 @@ static inline void ppc_msgsync(void)
> #endif /* CONFIG_PPC_BOOK3S */
>
> extern void doorbell_exception(struct pt_regs *regs);
> +extern bool doorbell_disabled;
>
> static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
> {
> diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
> index 5545c9cd17c1..681ee4775629 100644
> --- a/arch/powerpc/kernel/dbell.c
> +++ b/arch/powerpc/kernel/dbell.c
> @@ -38,6 +38,23 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
>
> set_irq_regs(old_regs);
> }
> +
> +bool doorbell_disabled;
> +
> +static int __init doorbell_cmdline(char *arg)
> +{
> + if (!arg)
> + return -EINVAL;
> +
> + if (strncmp(arg, "off", 3) == 0) {
> + pr_info("Doorbell disabled on kernel command line\n");
> + doorbell_disabled = true;
> + }
> +
> + return 0;
> +}
> +__setup("doorbell=", doorbell_cmdline);
> +
> #else /* CONFIG_SMP */
> DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
> {
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index cbb67813cd5d..1311bda9446a 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -338,10 +338,13 @@ static void __init pnv_smp_probe(void)
> ic_cause_ipi = smp_ops->cause_ipi;
> WARN_ON(!ic_cause_ipi);
>
> - if (cpu_has_feature(CPU_FTR_ARCH_300))
> + if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> + if (doorbell_disabled)
> + return;
> smp_ops->cause_ipi = doorbell_global_ipi;
> - else
> + } else {
> smp_ops->cause_ipi = pnv_cause_ipi;
> + }
> }
> }
>
> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
> index f47429323eee..3bc9e6aaf645 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -229,6 +229,9 @@ static __init void pSeries_smp_probe(void)
> return;
> }
>
> + if (doorbell_disabled)
> + return;
> +
> /*
> * Under PowerVM, FSCR[MSGP] is enabled as guest vCPU siblings are
> * gang scheduled on the same physical core, so doorbells are always
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 10fa093251e8..2e1284febe39 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1041,6 +1041,16 @@
> The filter can be disabled or changed to another
> driver later using sysfs.
>
> + doorbell=off [PPC]
> + On processors with a XIVE interrupt controller
> + (POWER9 and above), the kernel can use either
> + doorbells or XIVE to generate CPU IPIs. Sending
> + doorbell is generally preferred to using the XIVE
> + IC because it is faster. There are cases where
> + we want to avoid doorbells and use XIVE only,
> + for debug or performance. Only useful on
> + POWER9 and above.
> +
> driver_async_probe= [KNL]
> List of driver names to be probed asynchronously.
> Format: <driver_name1>,<driver_name2>...
> --
> 2.31.1
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter
2021-11-11 10:41 ` Michael Ellerman
@ 2021-11-11 16:01 ` Cédric Le Goater
2021-11-18 9:24 ` Michael Ellerman
0 siblings, 1 reply; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-11 16:01 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, Nicholas Piggin
On 11/11/21 11:41, Michael Ellerman wrote:
> Cédric Le Goater <clg@kaod.org> writes:
>> On processors with a XIVE interrupt controller (POWER9 and above), the
>> kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
>> doorbell is generally preferred to using the XIVE IC because it is
>> faster. There are cases where we want to avoid doorbells and use XIVE
>> only, for debug or performance. Only useful on POWER9 and above.
>
> How much do we want this?
Yes. Thanks for asking. It is a recent need.
Here is some background I should have added in the first place. May be
for a v2.
We have different ways of doing IPIs on POWER9 and above processors,
depending on the platform and the underlying hypervisor.
- PowerNV uses global doorbells
- pSeries/KVM uses XIVE only because local doorbells are not
efficient, as there are emulated in the KVM hypervisor
- pSeries/PowerVM uses XIVE for remote cores and local doorbells for
threads on same core (SMT4 or 8)
This recent commit 5b06d1679f2f ("powerpc/pseries: Use doorbells even
if XIVE is available") introduced the optimization for PowerVM and
commit 107c55005fbd ("powerpc/pseries: Add KVM guest doorbell
restrictions") restricted the optimization.
We would like a way to turn off the optimization.
> Kernel command line args are a bit of a pain, they tend to be poorly
> tested, because someone has to explicitly enable them at boot time,
> and then reboot to test the other case.
True. The "xive=off" parameter was poorly tested initially.
> When would we want to enable this?
For bring-up, for debug, for tests. I have been using a similar switch
to compare the XIVE interrupt controller performance with doorbells on
POWER9 and P0WER10.
A new need arises with PowerVM, some configurations will behave as KVM
(local doorbell are unsupported) and the doorbell=off parameter is a
simple way to handle this case today.
> Can we make the kernel smarter about when to use doorbells and make
> it automated?
I don't think we want to probe all IPI methods to detect how well
local doorbells are supported on the platform. Do we ?
A machine property/feature would be cleaner. It is a global CPU
property but I don't know where to put it. Ideas ?
> Could we make it a runtime switch?
We can. See the patch below. It covers the need for test/performance
but it won't work on a PowerVM system not supporting local doorbells
since boot will fail as soon as secondaries are started. We need a way
to take a decision early on which method to activate.
Thanks
C.
From dcac8528c89b689217515032f3329ba5ea10085d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
Date: Fri, 5 Nov 2021 12:23:48 +0100
Subject: [PATCH] powerpc/xive: Add a debugfs toggle to select xive for IPIs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
For performance tests only.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/common.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 39142df828a018..9ee36b95f9c545 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1826,6 +1826,30 @@ static int xive_eq_debug_show(struct seq_file *m, void *private)
}
DEFINE_SHOW_ATTRIBUTE(xive_eq_debug);
+static int xive_ipi_cause_debug_set(void *data, u64 val)
+{
+ static void (*do_ipi)(int cpu);
+
+ if (val) {
+ do_ipi = smp_ops->cause_ipi;
+ smp_ops->cause_ipi = xive_cause_ipi;
+ } else {
+ if (do_ipi)
+ smp_ops->cause_ipi = do_ipi;
+ }
+
+ return 0;
+}
+
+static int xive_ipi_cause_debug_get(void *data, u64 *val)
+{
+ *val = xive_cause_ipi == smp_ops->cause_ipi;
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(xive_ipi_cause_debug_fops, xive_ipi_cause_debug_get,
+ xive_ipi_cause_debug_set, "%llu\n");
+
static void xive_core_debugfs_create(void)
{
struct dentry *xive_dir;
@@ -1849,6 +1873,8 @@ static void xive_core_debugfs_create(void)
}
debugfs_create_bool("store-eoi", 0600, xive_dir, &xive_store_eoi);
debugfs_create_bool("save-restore", 0600, xive_dir, &xive_has_save_restore);
+ debugfs_create_file("ipi-cause", 0600, xive_dir,
+ NULL, &xive_ipi_cause_debug_fops);
}
#endif /* CONFIG_DEBUG_FS */
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter
2021-11-11 16:01 ` Cédric Le Goater
@ 2021-11-18 9:24 ` Michael Ellerman
2021-11-18 15:26 ` Cédric Le Goater
0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2021-11-18 9:24 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev, Nicholas Piggin
Cédric Le Goater <clg@kaod.org> writes:
> On 11/11/21 11:41, Michael Ellerman wrote:
>> Cédric Le Goater <clg@kaod.org> writes:
>>> On processors with a XIVE interrupt controller (POWER9 and above), the
>>> kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
>>> doorbell is generally preferred to using the XIVE IC because it is
>>> faster. There are cases where we want to avoid doorbells and use XIVE
>>> only, for debug or performance. Only useful on POWER9 and above.
>>
>> How much do we want this?
>
> Yes. Thanks for asking. It is a recent need.
>
> Here is some background I should have added in the first place. May be
> for a v2.
>
> We have different ways of doing IPIs on POWER9 and above processors,
> depending on the platform and the underlying hypervisor.
>
> - PowerNV uses global doorbells
>
> - pSeries/KVM uses XIVE only because local doorbells are not
> efficient, as there are emulated in the KVM hypervisor
>
> - pSeries/PowerVM uses XIVE for remote cores and local doorbells for
> threads on same core (SMT4 or 8)
>
> This recent commit 5b06d1679f2f ("powerpc/pseries: Use doorbells even
> if XIVE is available") introduced the optimization for PowerVM and
> commit 107c55005fbd ("powerpc/pseries: Add KVM guest doorbell
> restrictions") restricted the optimization.
>
> We would like a way to turn off the optimization.
Just for test/debug though?
>> Kernel command line args are a bit of a pain, they tend to be poorly
>> tested, because someone has to explicitly enable them at boot time,
>> and then reboot to test the other case.
>
> True. The "xive=off" parameter was poorly tested initially.
>
>> When would we want to enable this?
>
> For bring-up, for debug, for tests. I have been using a similar switch
> to compare the XIVE interrupt controller performance with doorbells on
> POWER9 and P0WER10.
>
> A new need arises with PowerVM, some configurations will behave as KVM
> (local doorbell are unsupported) and the doorbell=off parameter is a
> simple way to handle this case today.
That's the first I've heard of that, what PowerVM configurations have
non-working doorbells?
>> Can we make the kernel smarter about when to use doorbells and make
>> it automated?
>
> I don't think we want to probe all IPI methods to detect how well
> local doorbells are supported on the platform. Do we ?
We don't *want to*, but sounds like we might have to if they are not
working in some configurations as you mentioned above.
cheers
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter
2021-11-18 9:24 ` Michael Ellerman
@ 2021-11-18 15:26 ` Cédric Le Goater
0 siblings, 0 replies; 19+ messages in thread
From: Cédric Le Goater @ 2021-11-18 15:26 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, Nicholas Piggin
On 11/18/21 10:24, Michael Ellerman wrote:
> Cédric Le Goater <clg@kaod.org> writes:
>> On 11/11/21 11:41, Michael Ellerman wrote:
>>> Cédric Le Goater <clg@kaod.org> writes:
>>>> On processors with a XIVE interrupt controller (POWER9 and above), the
>>>> kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
>>>> doorbell is generally preferred to using the XIVE IC because it is
>>>> faster. There are cases where we want to avoid doorbells and use XIVE
>>>> only, for debug or performance. Only useful on POWER9 and above.
>>>
>>> How much do we want this?
>>
>> Yes. Thanks for asking. It is a recent need.
>>
>> Here is some background I should have added in the first place. May be
>> for a v2.
>>
>> We have different ways of doing IPIs on POWER9 and above processors,
>> depending on the platform and the underlying hypervisor.
>>
>> - PowerNV uses global doorbells
>>
>> - pSeries/KVM uses XIVE only because local doorbells are not
>> efficient, as there are emulated in the KVM hypervisor
>>
>> - pSeries/PowerVM uses XIVE for remote cores and local doorbells for
>> threads on same core (SMT4 or 8)
>>
>> This recent commit 5b06d1679f2f ("powerpc/pseries: Use doorbells even
>> if XIVE is available") introduced the optimization for PowerVM and
>> commit 107c55005fbd ("powerpc/pseries: Add KVM guest doorbell
>> restrictions") restricted the optimization.
>>
>> We would like a way to turn off the optimization.
>
> Just for test/debug though?
Yes. For now, this is the main target.
>>> Kernel command line args are a bit of a pain, they tend to be poorly
>>> tested, because someone has to explicitly enable them at boot time,
>>> and then reboot to test the other case.
>>
>> True. The "xive=off" parameter was poorly tested initially.
>>
>>> When would we want to enable this?
>>
>> For bring-up, for debug, for tests. I have been using a similar switch
>> to compare the XIVE interrupt controller performance with doorbells on
>> POWER9 and P0WER10.
>>
>> A new need arises with PowerVM, some configurations will behave as KVM
>> (local doorbell are unsupported) and the doorbell=off parameter is a
>> simple way to handle this case today.
>
> That's the first I've heard of that, what PowerVM configurations have
> non-working doorbells?
It's not released yet.
>>> Can we make the kernel smarter about when to use doorbells and make
>>> it automated?
>>
>> I don't think we want to probe all IPI methods to detect how well
>> local doorbells are supported on the platform. Do we ?
>
> We don't *want to*, but sounds like we might have to if they are not
> working in some configurations as you mentioned above.
Yeah. This is too early. I will keep that patch for internal use
until we have clarified.
Thanks,
C.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV
2021-11-05 10:26 [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV Cédric Le Goater
` (10 preceding siblings ...)
2021-11-05 10:26 ` [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter Cédric Le Goater
@ 2021-11-25 9:36 ` Michael Ellerman
11 siblings, 0 replies; 19+ messages in thread
From: Michael Ellerman @ 2021-11-25 9:36 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev
On Fri, 5 Nov 2021 11:26:25 +0100, Cédric Le Goater wrote:
> This series tries to improve diagnostic support in the XIVE driver. It
> adds pr_debug() primitives that can be activated at run-time and changes
> the debugfs xive entry to expose more information :
>
> /sys/kernel/debug/powerpc/xive/
> ├── eqs/
> │ ├── cpu0
> │ ├── cpu1
> │ ├── cpu2
> . .
> │ └── cpu99
> ├── interrupts
> ├── ipis
> ├── save-restore
> └── store-eoi
>
> [...]
Patches 1-10 applied to powerpc/next.
[01/11] powerpc/xive: Replace pr_devel() by pr_debug() to ease debug
https://git.kernel.org/powerpc/c/44b9c8ddcbc351d47ead974f0870d09bfc74b3f7
[02/11] powerpc/xive: Introduce an helper to print out interrupt characteristics
https://git.kernel.org/powerpc/c/bd5b00c6cf0c37fce1bcd94390044d7e1dd638e7
[03/11] powerpc/xive: Activate StoreEOI on P10
https://git.kernel.org/powerpc/c/756c52c632f5c2b054bb54b1ea9177329e4b8ce5
[04/11] powerpc/xive: Introduce xive_core_debugfs_create()
https://git.kernel.org/powerpc/c/412877dfae3dc12733bc711ccbd3d02338803865
[05/11] powerpc/xive: Change the debugfs file 'xive' into a directory
https://git.kernel.org/powerpc/c/baed14de78b5ee3ca04eae43c5b16e3eeb6e33a8
[06/11] powerpc/xive: Rename the 'cpus' debugfs file to 'ipis'
https://git.kernel.org/powerpc/c/33e1d4a152ce55272b54a16884461218d12d4f1b
[07/11] powerpc/xive: Add a debugfs file to dump EQs
https://git.kernel.org/powerpc/c/08f3f610214f395561bbda03344e641579f6e917
[08/11] powerpc/xive: Add a debugfs toggle for StoreEOI
https://git.kernel.org/powerpc/c/d7bc1e376cb786e9e8483455584d89cad4b5808f
[09/11] powerpc/xive: Add a kernel parameter for StoreEOI
https://git.kernel.org/powerpc/c/c21ee04f11ae068aa132cce56d09f618d4a66259
[10/11] powerpc/xive: Add a debugfs toggle for save-restore
https://git.kernel.org/powerpc/c/1e7684dc4fc70271c8bf86d397bd4fbfb3581e65
cheers
^ permalink raw reply [flat|nested] 19+ messages in thread