[PATCH] powerpc/xive: Fix/improve verbose debug output

[PATCH] powerpc/xive: Fix/improve verbose debug output

[Benjamin Herrenschmidt]

The existing verbose debug code doesn't build when enabled.

This fixes it and generally improves the output to make it
more useful.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/sysdev/xive/common.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 6a98efb..2305aa9 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -143,7 +143,6 @@ static u32 xive_scan_interrupts(struct xive_cpu *xc, bool just_peek)
 		struct xive_q *q;
 
 		prio = ffs(xc->pending_prio) - 1;
-		DBG_VERBOSE("scan_irq: trying prio %d\n", prio);
 
 		/* Try to fetch */
 		irq = xive_read_eq(&xc->queue[prio], just_peek);
@@ -171,12 +170,18 @@ static u32 xive_scan_interrupts(struct xive_cpu *xc, bool just_peek)
 	}
 
 	/* If nothing was found, set CPPR to 0xff */
-	if (irq == 0)
+	if (irq == 0) {
 		prio = 0xff;
+		DBG_VERBOSE("scan_irq(%d): nothing found\n", just_peek);
+	} else {
+		DBG_VERBOSE("scan_irq(%d): found irq %d prio %d\n",
+			    just_peek, irq, prio);
+	}
 
 	/* Update HW CPPR to match if necessary */
 	if (prio != xc->cppr) {
-		DBG_VERBOSE("scan_irq: adjusting CPPR to %d\n", prio);
+		DBG_VERBOSE("scan_irq(%d): adjusting CPPR %d->%d\n",
+			    just_peek, xc->cppr, prio);
 		xc->cppr = prio;
 		out_8(xive_tima + xive_tima_offset + TM_CPPR, prio);
 	}
@@ -260,7 +265,7 @@ static unsigned int xive_get_irq(void)
 	/* Scan our queue(s) for interrupts */
 	irq = xive_scan_interrupts(xc, false);
 
-	DBG_VERBOSE("get_irq: got irq 0x%x, new pending=0x%02x\n",
+	DBG_VERBOSE("get_irq: got irq %d new pending=0x%02x\n",
 	    irq, xc->pending_prio);
 
 	/* Return pending interrupt if any */
@@ -282,7 +287,7 @@ static unsigned int xive_get_irq(void)
 static void xive_do_queue_eoi(struct xive_cpu *xc)
 {
 	if (xive_scan_interrupts(xc, true) != 0) {
-		DBG_VERBOSE("eoi: pending=0x%02x\n", xc->pending_prio);
+		DBG_VERBOSE("eoi_irq: more pending !\n");
 		force_external_irq_replay();
 	}
 }
@@ -327,11 +332,13 @@ void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd)
 			in_be64(xd->eoi_mmio);
 		else {
 			eoi_val = xive_poke_esb(xd, XIVE_ESB_SET_PQ_00);
-			DBG_VERBOSE("eoi_val=%x\n", offset, eoi_val);
+			DBG_VERBOSE("hwirq 0x%x eoi_val=%x\n", hw_irq, eoi_val);
 
 			/* Re-trigger if needed */
-			if ((eoi_val & XIVE_ESB_VAL_Q) && xd->trig_mmio)
+			if ((eoi_val & XIVE_ESB_VAL_Q) && xd->trig_mmio) {
+				DBG_VERBOSE(" -> eoi retrigger !\n");
 				out_be64(xd->trig_mmio, 0);
+			}
 		}
 	}
 }
@@ -380,10 +387,15 @@ static void xive_do_source_set_mask(struct xive_irq_data *xd,
 	if (mask) {
 		val = xive_poke_esb(xd, XIVE_ESB_SET_PQ_01);
 		xd->saved_p = !!(val & XIVE_ESB_VAL_P);
-	} else if (xd->saved_p)
-		xive_poke_esb(xd, XIVE_ESB_SET_PQ_10);
-	else
-		xive_poke_esb(xd, XIVE_ESB_SET_PQ_00);
+		DBG_VERBOSE("masking val=%llx, sp=%d\n",
+			    val, xd->saved_p);
+	} else {
+		DBG_VERBOSE("unmasking sp=%d\n", xd->saved_p);
+		if (xd->saved_p)
+			xive_poke_esb(xd, XIVE_ESB_SET_PQ_10);
+		else
+			xive_poke_esb(xd, XIVE_ESB_SET_PQ_00);
+	}
 }
 
 /*
@@ -526,6 +538,7 @@ static unsigned int xive_irq_startup(struct irq_data *d)
 
 	pr_devel("xive_irq_startup: irq %d [0x%x] data @%p\n",
 		 d->irq, hw_irq, d);
+	pr_devel("  eoi_mmio=%p trig_mmio=%p\n", xd->eoi_mmio, xd->trig_mmio);
 
 #ifdef CONFIG_PCI_MSI
 	/*
@@ -754,6 +767,8 @@ static int xive_irq_retrigger(struct irq_data *d)
 	if (WARN_ON(xd->flags & XIVE_IRQ_FLAG_LSI))
 		return 0;
 
+	DBG_VERBOSE("retrigger irq %d\n", d->irq);
+
 	/*
 	 * To perform a retrigger, we first set the PQ bits to
 	 * 11, then perform an EOI.

Re: [PATCH] powerpc/xive: Fix/improve verbose debug output

[Michael Ellerman]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> The existing verbose debug code doesn't build when enabled.

So why don't we convert all the DBG_VERBOSE() to pr_devel()? 

If there's non-verbose debug that we think would be useful to
differentiate from verbose then those could be pr_debug() - which means
they'll be jump labelled off in most production kernels, but still able
to be enabled.

cheers

Re: [PATCH] powerpc/xive: Fix/improve verbose debug output

[Benjamin Herrenschmidt]

On Fri, 2017-04-28 at 13:07 +1000, Michael Ellerman wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> 
> > The existing verbose debug code doesn't build when enabled.
> 
> So why don't we convert all the DBG_VERBOSE() to pr_devel()? 

pr_devel provides a bunch of debug at init/setup/mask/unmask etc... but
the system is still usable

DBG_VERBOSE starts spewing stuff on every interrupt and eoi, the system
is no longer usable.

> If there's non-verbose debug that we think would be useful to
> differentiate from verbose then those could be pr_debug() - which means
> they'll be jump labelled off in most production kernels, but still able
> to be enabled.

Maybe... I don't like the giant "debug" switch accross the whole
kernel, though.

Ben.

Re: [PATCH] powerpc/xive: Fix/improve verbose debug output

[Michael Ellerman]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Fri, 2017-04-28 at 13:07 +1000, Michael Ellerman wrote:
>> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
>> 
>> > The existing verbose debug code doesn't build when enabled.
>> 
>> So why don't we convert all the DBG_VERBOSE() to pr_devel()? 
>
> pr_devel provides a bunch of debug at init/setup/mask/unmask etc... but
> the system is still usable

OK so those could be converted to pr_debug().

> DBG_VERBOSE starts spewing stuff on every interrupt and eoi, the system
> is no longer usable.

And those could stay at pr_devel(), requiring a #define DEBUG and
recompile to enable.

>> If there's non-verbose debug that we think would be useful to
>> differentiate from verbose then those could be pr_debug() - which means
>> they'll be jump labelled off in most production kernels, but still able
>> to be enabled.
>
> Maybe... I don't like the giant "debug" switch accross the whole
> kernel, though.

Not sure what you mean. You can enable pr_debug()s individually, by
function, by module, by file, or for the whole kernel.

To enable everything in xive you'd do:

# echo 'file *xive* +p' > /sys/kernel/debug/dynamic_debug/control

Or boot with: loglevel=8 dyndbg="file *xive* +p"

cheers

Re: [PATCH] powerpc/xive: Fix/improve verbose debug output

[Benjamin Herrenschmidt]

On Fri, 2017-04-28 at 16:34 +1000, Michael Ellerman wrote:
> > > If there's non-verbose debug that we think would be useful to
> > > differentiate from verbose then those could be pr_debug() - which means
> > > they'll be jump labelled off in most production kernels, but still able
> > > to be enabled.
> > 
> > Maybe... I don't like the giant "debug" switch accross the whole
> > kernel, though.
> 
> Not sure what you mean. You can enable pr_debug()s individually, by
> function, by module, by file, or for the whole kernel.
> 
> To enable everything in xive you'd do:
> 
> # echo 'file *xive* +p' > /sys/kernel/debug/dynamic_debug/control
> 
> Or boot with: loglevel=8 dyndbg="file *xive* +p"

Ah that's new goodness I wasn't aware of. Anyway, I can spin that
later, not planning on doing any work today ;-)

Cheers,
Ben.

RE: [PATCH] powerpc/xive: Fix/improve verbose debug output

[David Laight]

From: Michael Ellerman
> Sent: 28 April 2017 07:34
...
> Not sure what you mean. You can enable pr_debug()s individually, by
> function, by module, by file, or for the whole kernel.

It is sort of a shame that you can't turn pr_info() off in the same way.

	David