[PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV

[PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV

[Cédric Le Goater]

Hello,

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

It also introduces 2 new kernel parameters :

    xive.store-eoi=off   to deactivate StoreEOI at boot but it still be
    			 reactivated through sysfs.
			 
    doorbell=off         to deactivate doorbells for CPU IPIs
                         and XIVE instead

The first is a workaround in case of a FW advertising the wrong
feature. It can be used to check performance also. The second is for
PowerVM development and tests when the LP-per-thread mode is
activated. Doorbells cannot be used in that case.

Finally, it activates StoreEOI support for the PowerNV platform. OPAL
for P10 has been released and we are free to use this extension.

Thanks,

C.


Cédric Le Goater (11):
  powerpc/xive: Replace pr_devel() by pr_debug() to ease debug
  powerpc/xive: Introduce an helper to print out interrupt
    characteristics
  powerpc/xive: Activate StoreEOI on P10
  powerpc/xive: Introduce xive_core_debugfs_create()
  powerpc/xive: Change the debugfs file 'xive' into a directory
  powerpc/xive: Rename the 'cpus' debugfs file to 'ipis'
  powerpc/xive: Add a debugfs file to dump EQs
  powerpc/xive: Add a debugfs toggle for StoreEOI
  powerpc/xive: Add a kernel parameter for StoreEOI
  powerpc/xive: Add a debugfs toggle for save-restore
  powerpc/smp: Add a doorbell=off kernel parameter

 arch/powerpc/include/asm/dbell.h              |   1 +
 arch/powerpc/include/asm/opal-api.h           |   1 +
 arch/powerpc/sysdev/xive/xive-internal.h      |   1 +
 arch/powerpc/kernel/dbell.c                   |  17 ++
 arch/powerpc/platforms/powernv/smp.c          |   7 +-
 arch/powerpc/platforms/pseries/smp.c          |   3 +
 arch/powerpc/sysdev/xive/common.c             | 211 ++++++++++++------
 arch/powerpc/sysdev/xive/native.c             |   4 +-
 arch/powerpc/sysdev/xive/spapr.c              |  38 ++--
 .../admin-guide/kernel-parameters.txt         |  16 ++
 10 files changed, 209 insertions(+), 90 deletions(-)

-- 
2.31.1

[PATCH 01/11] powerpc/xive: Replace pr_devel() by pr_debug() to ease debug

[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

[PATCH 02/11] powerpc/xive: Introduce an helper to print out interrupt characteristics

[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

[PATCH 03/11] powerpc/xive: Activate StoreEOI on P10

[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

[PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create()

[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

[PATCH 05/11] powerpc/xive: Change the debugfs file 'xive' into a directory

[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

[PATCH 06/11] powerpc/xive: Rename the 'cpus' debugfs file to 'ipis'

[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

[PATCH 07/11] powerpc/xive: Add a debugfs file to dump EQs

[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

[PATCH 08/11] powerpc/xive: Add a debugfs toggle for StoreEOI

[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

[PATCH 09/11] powerpc/xive: Add a kernel parameter for StoreEOI

[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

[PATCH 10/11] powerpc/xive: Add a debugfs toggle for save-restore

[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

[PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter

[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

Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter

[Michael Ellerman]

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

Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter

[Cédric Le Goater]

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 */

Re: [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create()

[Michael Ellerman]

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

Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter

[Michael Ellerman]

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

Re: [PATCH 04/11] powerpc/xive: Introduce xive_core_debugfs_create()

[Cédric Le Goater]

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.

Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter

[Cédric Le Goater]

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.

Re: [PATCH 00/11] powerpc/xive: Improve diagnostic and activate StoreEOI on P10 PowerNV

[Michael Ellerman]

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