[RFC] Debugging with a HW probe.

[RFC] Debugging with a HW probe.

[Jimi Xenidis]

On the XenPPC project we've been playing around with using GDB to
source debug Linux (and Xen) using a RiscWatch HW probe.

On IBM POWER4 and greater processors (and possibly POWER3 and *Star)
there is an instruction called ATTN (asm (".long 0x200")) that will
have the process call out to the HW probe.  This instruction is used
by RiscWatch software to set "soft breakpoints".

We have found it useful to teach xmon to make this call so we can then
debug the SW thru the probe. Is this useful to anyone else?

Below is a quick attempt at a formalized patch, tho' I am torn between
making it ATTN specific or just making a generic HW Probe Service.

All comments welcome.

BTW: We even have some python that will let GDB "talk to" a RiscWatch
     probe and fully source-level debug a Linux kernel (even the
     initial relocatable parts with a few tricks), this code is
     currently part of the our Xen tree (you need mercurial to get
     it):
       http://xenbits.xensource.com/ext/xenppc-unstable.hg

     The python is self contained and in:
       tools/gpproxy



-JX
----

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 8d48e9e..b84f03d 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -82,6 +82,13 @@ config XMON_DEFAULT
 	  xmon is normally disabled unless booted with 'xmon=on'.
 	  Use 'xmon=off' to disable xmon init during runtime.
 
+config XMON_ATTN
+       bool "Use Support Processor Attention Instruction"
+       depends on XMON && PPC64
+       help
+         If you have a hardware probe attached to your processor you
+         can 'contact' it with a the 'A' command at the xmon prompt.
+
 config IRQSTACKS
 	bool "Use separate kernel stacks when processing interrupts"
 	depends on PPC64
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 4735b41..ea1d732 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -37,6 +37,7 @@ #include <asm/cputable.h>
 #include <asm/rtas.h>
 #include <asm/sstep.h>
 #include <asm/bug.h>
+#include <asm/firmware.h>
 
 #ifdef CONFIG_PPC64
 #include <asm/hvcall.h>
@@ -175,7 +176,12 @@ #define isalnum(c)	(('0' <= (c) && (c) <
 #define isspace(c)	(c == ' ' || c == '\t' || c == 10 || c == 13 || c == 0)
 
 static char *help_string = "\
-Commands:\n\
+Commands:\n"
+#ifdef CONFIG_XMON_ATTN
+  "\
+  A     Get the attention of the hardware probe, assuming you have one\n"
+#endif
+  "\
   b	show breakpoints\n\
   bd	set data breakpoint\n\
   bi	set instruction breakpoint\n\
@@ -223,6 +229,70 @@ #endif
 
 static struct pt_regs *xmon_regs;
 
+#ifdef CONFIG_XMON_ATTN
+static int xmon_hw_probe_enabled;
+/* try to keep this funtion from being inlined so its easier to move
+ * around the ATTN instruction */
+static noinline void xmon_hw_probe(void)
+{
+	if (!xmon_hw_probe_enabled)
+		return;
+	ATTN();
+}
+static void xmon_en_hw_probe(int enable)
+{
+	ulong hid0;
+	int threaded = 0;
+
+	/* hopefully the hypervisor has set us up */
+	if (firmware_has_feature(FW_FEATURE_LPAR))
+		return;
+	/* should this be a feature? */
+	switch (PVR_VER(mfspr(SPRN_PVR))) {
+	default:
+		return;
+
+	case PV_POWER4:
+	case PV_POWER4p:
+	case PV_POWER5:
+	case PV_POWER5p:
+	case PV_BE:
+		/* stop both threads */
+		threaded = 1;
+	case PV_SSTAR:		
+	case PV_970:
+	case PV_970FX:
+	case PV_970MP:
+		break;
+	}
+	hid0 = mfspr(SPRN_HID0);
+	if (enable) {
+		/* make sure that on threaded processors we stop both
+		 * threads */
+		hid0 |= HID0_ATTN | (threaded ? HID0_QATTN : 0);
+		xmon_hw_probe_enabled = 1;
+	} else {
+		/* only turn the feature off */
+		hid0 &= ~HID0_ATTN;
+		xmon_hw_probe_enabled = 0;
+	}
+	/* many processors require the following sequence */
+	asm volatile(
+		"sync\n"
+		"mtspr     %1, %0\n"
+		"mfspr     %0, %1\n"
+		"mfspr     %0, %1\n"
+		"mfspr     %0, %1\n"
+		"mfspr     %0, %1\n"
+		"mfspr     %0, %1\n"
+		"mfspr     %0, %1\n"
+		"isync" : "=&r" (hid0) : "i" (SPRN_HID0), "0" (hid0):
+		"memory");
+}
+#else
+#define xmon_en_hw_probe(x)
+#endif
+
 static inline void sync(void)
 {
 	asm volatile("sync; isync");
@@ -834,6 +904,11 @@ #ifdef CONFIG_PPC_STD_MMU
 			dump_segments();
 			break;
 #endif
+#ifdef CONFIG_XMON_ATTN
+		case 'A':
+			xmon_hw_probe();
+			break;
+#endif
 		default:
 			printf("Unrecognized command: ");
 		        do {
@@ -2565,6 +2640,7 @@ void xmon_init(int enable)
 		__debugger_dabr_match = NULL;
 		__debugger_fault_handler = NULL;
 	}
+	xmon_en_hw_probe(enable);
 	xmon_map_scc();
 }
 
diff --git a/drivers/net/pcmcia/axnet_cs.c b/drivers/net/pcmcia/axnet_cs.c
index 2ea66ac..33a7922 100644
diff --git a/include/asm-powerpc/reg.h b/include/asm-powerpc/reg.h
index bd467bf..ce82965 100644
--- a/include/asm-powerpc/reg.h
+++ b/include/asm-powerpc/reg.h
@@ -207,6 +207,8 @@ #define SPRN_EAR	0x11A		/* External Addr
 #define SPRN_HASH1	0x3D2		/* Primary Hash Address Register */
 #define SPRN_HASH2	0x3D3		/* Secondary Hash Address Resgister */
 #define SPRN_HID0	0x3F0		/* Hardware Implementation Register 0 */
+#define HID0_QATTN	(1UL<<35)	/* Sup. Proc. attn insn all threads */
+#define HID0_ATTN	(1UL<<32)	/* Sup. Proc. attn insn */
 #define HID0_EMCP	(1<<31)		/* Enable Machine Check pin */
 #define HID0_EBA	(1<<29)		/* Enable Bus Address Parity */
 #define HID0_EBD	(1<<28)		/* Enable Bus Data Parity */
diff --git a/include/asm-powerpc/xmon.h b/include/asm-powerpc/xmon.h
index 43f7129..cd8f48e 100644
--- a/include/asm-powerpc/xmon.h
+++ b/include/asm-powerpc/xmon.h
@@ -8,5 +8,12 @@ extern int xmon(struct pt_regs *excp);
 extern void xmon_printf(const char *fmt, ...);
 extern void xmon_init(int);
 
+/*
+ * Support Processor Attention Instruction introduced in POWER
+ * architecture processors as of RS64, tho may not be supported by
+ * POWER 3.
+ */
+#define ATTN() asm volatile(".long 0x00000200; nop")
+
 #endif
 #endif

Re: [RFC] Debugging with a HW probe.

[Olof Johansson]

Hi,

On Sun, Aug 06, 2006 at 10:42:16AM -0400, Jimi Xenidis wrote:

> On IBM POWER4 and greater processors (and possibly POWER3 and *Star)
> there is an instruction called ATTN (asm (".long 0x200")) that will
> have the process call out to the HW probe.  This instruction is used
> by RiscWatch software to set "soft breakpoints".
> 
> We have found it useful to teach xmon to make this call so we can then
> debug the SW thru the probe. Is this useful to anyone else?

Being able to correlate software events with the hardware debugger looks
quite useful, yes.

> Below is a quick attempt at a formalized patch, tho' I am torn between
> making it ATTN specific or just making a generic HW Probe Service.

I would say make it generic, maybe with cputables entry or ppc_md
member, especially with the powerpc merge and other platforms coming in,
having it more generic could be useful.

Also, that way you could get rid of the PVR check within xmon.

> All comments welcome.

A couple below.

> +static void xmon_en_hw_probe(int enable)
> +{
> +	ulong hid0;
> +	int threaded = 0;
> +
> +	/* hopefully the hypervisor has set us up */
> +	if (firmware_has_feature(FW_FEATURE_LPAR))
> +		return;
> +	/* should this be a feature? */
> +	switch (PVR_VER(mfspr(SPRN_PVR))) {
> +	default:
> +		return;
> +
> +	case PV_POWER4:
> +	case PV_POWER4p:
> +	case PV_POWER5:
> +	case PV_POWER5p:
> +	case PV_BE:
> +		/* stop both threads */
> +		threaded = 1;

POWER4 multithreaded? I don't think so?

> +	case PV_SSTAR:		

Trailing whitespace. Also, doesn't sstar have HMT?

> +	case PV_970:
> +	case PV_970FX:
> +	case PV_970MP:
> +		break;
> +	}
> +	hid0 = mfspr(SPRN_HID0);
> +	if (enable) {
> +		/* make sure that on threaded processors we stop both
> +		 * threads */
> +		hid0 |= HID0_ATTN | (threaded ? HID0_QATTN : 0);
> +		xmon_hw_probe_enabled = 1;
> +	} else {
> +		/* only turn the feature off */
> +		hid0 &= ~HID0_ATTN;
> +		xmon_hw_probe_enabled = 0;
> +	}
> +	/* many processors require the following sequence */
> +	asm volatile(
> +		"sync\n"
> +		"mtspr     %1, %0\n"
> +		"mfspr     %0, %1\n"
> +		"mfspr     %0, %1\n"
> +		"mfspr     %0, %1\n"
> +		"mfspr     %0, %1\n"
> +		"mfspr     %0, %1\n"
> +		"mfspr     %0, %1\n"
> +		"isync" : "=&r" (hid0) : "i" (SPRN_HID0), "0" (hid0):
> +		"memory");
> +}
> +#else
> +#define xmon_en_hw_probe(x)
> +#endif
> +
>  static inline void sync(void)
>  {
>  	asm volatile("sync; isync");
> @@ -834,6 +904,11 @@ #ifdef CONFIG_PPC_STD_MMU
>  			dump_segments();
>  			break;
>  #endif
> +#ifdef CONFIG_XMON_ATTN
> +		case 'A':
> +			xmon_hw_probe();
> +			break;
> +#endif
>  		default:
>  			printf("Unrecognized command: ");
>  		        do {
> @@ -2565,6 +2640,7 @@ void xmon_init(int enable)
>  		__debugger_dabr_match = NULL;
>  		__debugger_fault_handler = NULL;
>  	}
> +	xmon_en_hw_probe(enable);

Please only enable this right before the call.

>  	xmon_map_scc();
>  }
>  
> diff --git a/drivers/net/pcmcia/axnet_cs.c b/drivers/net/pcmcia/axnet_cs.c
> index 2ea66ac..33a7922 100644
> diff --git a/include/asm-powerpc/reg.h b/include/asm-powerpc/reg.h
> index bd467bf..ce82965 100644
> --- a/include/asm-powerpc/reg.h
> +++ b/include/asm-powerpc/reg.h
> @@ -207,6 +207,8 @@ #define SPRN_EAR	0x11A		/* External Addr
>  #define SPRN_HASH1	0x3D2		/* Primary Hash Address Register */
>  #define SPRN_HASH2	0x3D3		/* Secondary Hash Address Resgister */
>  #define SPRN_HID0	0x3F0		/* Hardware Implementation Register 0 */
> +#define HID0_QATTN	(1UL<<35)	/* Sup. Proc. attn insn all threads */
> +#define HID0_ATTN	(1UL<<32)	/* Sup. Proc. attn insn */
>  #define HID0_EMCP	(1<<31)		/* Enable Machine Check pin */
>  #define HID0_EBA	(1<<29)		/* Enable Bus Address Parity */
>  #define HID0_EBD	(1<<28)		/* Enable Bus Data Parity */
> diff --git a/include/asm-powerpc/xmon.h b/include/asm-powerpc/xmon.h
> index 43f7129..cd8f48e 100644
> --- a/include/asm-powerpc/xmon.h
> +++ b/include/asm-powerpc/xmon.h
> @@ -8,5 +8,12 @@ extern int xmon(struct pt_regs *excp);
>  extern void xmon_printf(const char *fmt, ...);
>  extern void xmon_init(int);
>  
> +/*
> + * Support Processor Attention Instruction introduced in POWER
> + * architecture processors as of RS64, tho may not be supported by
> + * POWER 3.
> + */
> +#define ATTN() asm volatile(".long 0x00000200; nop")

At least my toolchain understands "attn ; nop"?



-Olof

Re: [RFC] Debugging with a HW probe.

[Paul Mackerras]

Jimi Xenidis writes:

> +static noinline void xmon_hw_probe(void)
> +{
> +	if (!xmon_hw_probe_enabled)
> +		return;
> +	ATTN();
> +}

I can't see how we can get to call this routine with
xmon_hw_probe_enabled equal to 0...  What am I missing?

Paul.

Re: [RFC] Debugging with a HW probe.

[Jimi Xenidis]

On Aug 8, 2006, at 12:48 AM, Paul Mackerras wrote:

> Jimi Xenidis writes:
>
>> +static noinline void xmon_hw_probe(void)
>> +{
>> +	if (!xmon_hw_probe_enabled)
>> +		return;
>> +	ATTN();
>> +}
>
> I can't see how we can get to call this routine with
> xmon_hw_probe_enabled equal to 0...  What am I missing?
>

@@ -834,6 +904,11 @@ #ifdef CONFIG_PPC_STD_MMU
  			dump_segments();
  			break;
  #endif
+#ifdef CONFIG_XMON_ATTN
+		case 'A':
+			xmon_hw_probe();
+			break;
+#endif
  		default:
  			printf("Unrecognized command: ");
  		        do {

The static is used to avoid the illegal instruction trap that will/ 
should occur if we execute this instruction and the hid bit is not set.

-JX

Re: [RFC] Debugging with a HW probe.

[Jimi Xenidis]

On Aug 7, 2006, at 7:06 PM, Olof Johansson wrote:

> > --- a/include/asm-powerpc/xmon.h
> > +++ b/include/asm-powerpc/xmon.h
> > @@ -8,5 +8,12 @@ extern int xmon(struct pt_regs *excp);
> >  extern void xmon_printf(const char *fmt, ...);
> >  extern void xmon_init(int);
> >
> > +/*
> > + * Support Processor Attention Instruction introduced in POWER
> > + * architecture processors as of RS64, tho may not be supported by
> > + * POWER 3.
> > + */
> > +#define ATTN() asm volatile(".long 0x00000200; nop")
>
> At least my toolchain understands "attn ; nop"?

looks like it was added in 2003:
   2003-06-10  Gary Hade <garyhade@us.ibm.com>
   	    Alan Modra  <amodra@bigpond.net.au>

	* ppc-opc.c (DQ, RAQ, RSQ, RTQ): Define.
	(insert_dq, extract_dq, insert_raq, insert_rtq, insert_rsq): New.
	(powerpc_opcodes): Add "attn", "lq" and "stq".

am happy to put in symbolically.

Take 2: [RFC] Debugging with a HW probe.

[Jimi Xenidis]

A rework of the original patch, some on and off list comments had
suggested that this be a generic service, and to make it very hard
to unintentionally turn it on.  I fact we should make sure that the
feature is indeed turned off.

Any suggestions on how to operate on the HID bits of all CPUs based on
the value of the config _and_ 'hw_probe_enabled' would be welcom.

I'd also like to wait for Olof's:
  http://ozlabs.org/pipermail/linuxppc-dev/2006-August/025024.html
patch to make it to the tree (or not).

Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>

---
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index e29ef77..bc4cdf9 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -61,6 +61,17 @@ config KGDB_CONSOLE
 	  over the gdb stub.
 	  If unsure, say N.
 
+config ENABLE_HW_PROBE
+       bool "Allow instructions that contact a hardware probe (dangerous)"
+       depends on PPC64
+       help
+         If you enable this AND you add "hwprobe" to the cmdline, the
+         processor will enable instructions that contact the hardware
+         probe.  These instructions ca be used in all processor modes
+         _including_ user mode and are only useful for kernel
+         development and debugging.  DO NOT enable this unless you
+         plan to use it.
+
 config XMON
 	bool "Include xmon kernel debugger"
 	depends on DEBUGGER && !PPC_ISERIES
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index bf2005b..02c5b83 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -68,6 +68,9 @@ int __initdata iommu_is_off;
 int __initdata iommu_force_on;
 unsigned long tce_alloc_start, tce_alloc_end;
 #endif
+#ifdef CONFIG_ENABLE_HW_PROBE
+int hw_probe_enabled;
+#endif
 
 typedef u32 cell_t;
 
@@ -693,6 +696,11 @@ #ifdef CONFIG_PPC64
 	if (of_get_flat_dt_prop(node, "linux,iommu-force-on", NULL) != NULL)
 		iommu_force_on = 1;
 #endif
+#ifdef CONFIG_HW_PROBE_ENABLE
+	if (of_get_flat_dt_prop(node, "linux,hw-probe-enable", NULL) != NULL) {
+		hw_probe_enabled = 1;
+		DBG("HW Probe will be enabled\n");
+#endif
 
 	/* mem=x on the command line is the preferred mechanism */
  	lprop = of_get_flat_dt_prop(node, "linux,memory-limit", NULL);
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 90972ef..26428de 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -587,6 +587,14 @@ #ifdef CONFIG_PPC64
 			RELOC(iommu_force_on) = 1;
 	}
 #endif
+#ifdef CONFIG_HW_PROBE_ENABLE
+	opt = strstr(RELOC(prom_cmd_line), RELOC("hwprobe"));
+	if (opt) {
+		prom_printf("WARNING! HW Probe will be activated!\n");
+		prom_setprop(_prom->chosen, "/chosen",
+			     "linux,hw-probe-enable", NULL, 0);
+	}
+#endif
 }
 
 #ifdef CONFIG_PPC_PSERIES
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 179b10c..51a1e4e 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -189,7 +189,12 @@ #endif
   dd	dump double values\n\
   dr	dump stream of raw bytes\n\
   e	print exception information\n\
-  f	flush cache\n\
+  f	flush cache\n"
+#ifdef CONFIG_ENABLE_HW_PROBE
+  "\
+  H     Contact hardware probe, if available\n"
+#endif
+  "\
   la	lookup symbol+offset of specified address\n\
   ls	lookup address of specified symbol\n\
   m	examine/change memory\n\
@@ -217,6 +222,18 @@ #endif
   zh	halt\n"
 ;
 
+#ifdef CONFIG_ENABLE_HW_PROBE
+/* try to keep this funtion from being inlined so its easier to move
+ * around the ATTN instruction */
+extern int hw_probe_enabled;
+static noinline void xmon_hw_probe(void)
+{
+	if (!hw_probe_enabled)
+		return;
+	ATTN();
+}
+#endif
+
 static struct pt_regs *xmon_regs;
 
 static inline void sync(void)
@@ -819,6 +836,11 @@ #endif /* CONFIG_SMP */
 			if (cpu_cmd())
 				return 0;
 			break;
+#ifdef CONFIG_ENABLE_HW_PROBE
+		case 'H':
+			xmon_hw_probe();
+			break;
+#endif
 		case 'z':
 			bootcmds();
 			break;
diff --git a/include/asm-powerpc/reg.h b/include/asm-powerpc/reg.h
index cf73475..478097e 100644
--- a/include/asm-powerpc/reg.h
+++ b/include/asm-powerpc/reg.h
@@ -207,6 +207,13 @@ #define SPRN_EAR	0x11A		/* External Addr
 #define SPRN_HASH1	0x3D2		/* Primary Hash Address Register */
 #define SPRN_HASH2	0x3D3		/* Secondary Hash Address Resgister */
 #define SPRN_HID0	0x3F0		/* Hardware Implementation Register 0 */
+#ifdef __ASSEMBLY__
+#define HID0_QATTN	(1<<35)		/* Sup. Proc. attn insn all threads */
+#define HID0_ATTN	(1<<32)		/* Sup. Proc. attn insn */
+#else
+#define HID0_QATTN	(1UL<<35)	/* Sup. Proc. attn insn all threads */
+#define HID0_ATTN	(1UL<<32)	/* Sup. Proc. attn insn */
+#endif
 #define HID0_EMCP	(1<<31)		/* Enable Machine Check pin */
 #define HID0_EBA	(1<<29)		/* Enable Bus Address Parity */
 #define HID0_EBD	(1<<28)		/* Enable Bus Data Parity */
@@ -641,6 +648,13 @@ extern void ppc64_runlatch_off(void);
 extern unsigned long scom970_read(unsigned int address);
 extern void scom970_write(unsigned int address, unsigned long value);
 
+/*
+ * Support Processor Attention Instruction instroduced in POWER
+ * architecture processors as of RS64, tho may not be supported by
+ * POWER 3.
+ */
+#define ATTN() asm volatile("attn; nop")
+
 #else
 #define ppc64_runlatch_on()
 #define ppc64_runlatch_off()

Re: Take 2: [RFC] Debugging with a HW probe.

[Milton Miller]

On Aug 14, 2006, at 5:16 AM, Jimi Xenidis jimix at watson.ibm.com  
wrote:

> A rework of the original patch, some on and off list comments had
> suggested that this be a generic service, and to make it very hard
> to unintentionally turn it on.  I fact we should make sure that the
> feature is indeed turned off.
>
> Any suggestions on how to operate on the HID bits of all CPUs based on
> the value of the config _and_ 'hw_probe_enabled' would be welcom.
>
> I'd also like to wait for Olof's:
>   http://ozlabs.org/pipermail/linuxppc-dev/2006-August/025024.html
> patch to make it to the tree (or not).
>
> Signed-off-by: Jimi Xenidis <jimix at watson.ibm.com>

[sorry for the list archive patch munging]

> ---
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index e29ef77..bc4cdf9 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -61,6 +61,17 @@ config KGDB_CONSOLE
>           over the gdb stub.
>           If unsure, say N.
>
> +config ENABLE_HW_PROBE
> +       bool "Allow instructions that contact a hardware probe 
> (dangerous)"
> +       depends on PPC64

Not having this depend on DEBUGGER but in the middle of things that do 
will
get you scorn from the auto-indenting police.

Since we can only call this from xmon, should it depend on XMON (and be
placed after that)?

> +       help
> +         If you enable this AND you add "hwprobe" to the cmdline, the
> +         processor will enable instructions that contact the hardware
> +         probe.  These instructions ca be used in all processor modes

can

> +         _including_ user mode and are only useful for kernel

not sure this _highlighting_ is used elsewhere in Kconfig help ...

Should we mention that a hardware probe is required to continue 
exectuion?
In other words, its not just contact, but signal and wait for a hw 
probe?

> +         development and debugging.  DO NOT enable this unless you
> +         plan to use it.
> +
>  config XMON
>         bool "Include xmon kernel debugger"
>         depends on DEBUGGER && !PPC_ISERIES
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index bf2005b..02c5b83 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -68,6 +68,9 @@ int __initdata iommu_is_off;
>  int __initdata iommu_force_on;
>  unsigned long tce_alloc_start, tce_alloc_end;
>  #endif
> +#ifdef CONFIG_ENABLE_HW_PROBE
> +int hw_probe_enabled;
> +#endif
>
>  typedef u32 cell_t;
>
> @@ -693,6 +696,11 @@ #ifdef CONFIG_PPC64
>         if (of_get_flat_dt_prop(node, "linux,iommu-force-on", NULL) != 
> NULL)
>                 iommu_force_on = 1;
>  #endif
> +#ifdef CONFIG_HW_PROBE_ENABLE
> +       if (of_get_flat_dt_prop(node, "linux,hw-probe-enable", NULL) 
> != NULL) {
> +               hw_probe_enabled = 1;
> +               DBG("HW Probe will be enabled\n");
> +#endif

No.  [see next comment]

>
>         /* mem=x on the command line is the preferred mechanism */
>         lprop = of_get_flat_dt_prop(node, "linux,memory-limit", NULL);
> diff --git a/arch/powerpc/kernel/prom_init.c 
> b/arch/powerpc/kernel/prom_init.c
> index 90972ef..26428de 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -587,6 +587,14 @@ #ifdef CONFIG_PPC64
>                         RELOC(iommu_force_on) = 1;
>         }
>  #endif
> +#ifdef CONFIG_HW_PROBE_ENABLE
> +       opt = strstr(RELOC(prom_cmd_line), RELOC("hwprobe"));
> +       if (opt) {
> +               prom_printf("WARNING! HW Probe will be activated!\n");
> +               prom_setprop(_prom->chosen, "/chosen",
> +                            "linux,hw-probe-enable", NULL, 0);
> +       }
> +#endif
>  }

Please, PLEASE do NOT do this.

prom_init.c is only used by one of the many flat device tree generators,
namely the open-firmware client.  Adding a property like this requires 
us
to update all the other clients.

And there is no reason to parse it this early.

Instead, parse it from the command line like the other early parsing.

I thing a generic early_param would be fine.  However, xmon_init is an
early_parm in setup-common, so if you really require it before the first
call, then you could parse it next to mem= at the bottom of
early_init_dt_scan_chosen.

Which .h is hw_probe_enabled in?  (none?)

>
>  #ifdef CONFIG_PPC_PSERIES
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 179b10c..51a1e4e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -189,7 +189,12 @@ #endif
>    dd   dump double values\n\
>    dr   dump stream of raw bytes\n\
>    e    print exception information\n\
> -  f    flush cache\n\
> +  f    flush cache\n"
> +#ifdef CONFIG_ENABLE_HW_PROBE
> +  "\
> +  H     Contact hardware probe, if available\n"
> +#endif
> +  "\

While this style does keep the lines aligned in the source, it adds 
veritcal
almost-whitespace.  And I notice a different choice was made at the 
bottom.

>    la   lookup symbol+offset of specified address\n\
>    ls   lookup address of specified symbol\n\
>    m    examine/change memory\n\
> @@ -217,6 +222,18 @@ #endif
>    zh   halt\n"
>  ;
>
> +#ifdef CONFIG_ENABLE_HW_PROBE
> +/* try to keep this funtion from being inlined so its easier to move
> + * around the ATTN instruction */
> +extern int hw_probe_enabled;
> +static noinline void xmon_hw_probe(void)
> +{
> +       if (!hw_probe_enabled)
> +               return;
> +       ATTN();
> +}
> +#endif
> +
>  static struct pt_regs *xmon_regs;
>
>  static inline void sync(void)
> @@ -819,6 +836,11 @@ #endif /* CONFIG_SMP */
>                         if (cpu_cmd())
>                                 return 0;
>                         break;
> +#ifdef CONFIG_ENABLE_HW_PROBE
> +               case 'H':
> +                       xmon_hw_probe();
> +                       break;
> +#endif
>                 case 'z':
>                         bootcmds();
>                         break;
> diff --git a/include/asm-powerpc/reg.h b/include/asm-powerpc/reg.h
> index cf73475..478097e 100644
> --- a/include/asm-powerpc/reg.h
> +++ b/include/asm-powerpc/reg.h
> @@ -207,6 +207,13 @@ #define SPRN_EAR   0x11A           /* External 
> Addr
>  #define SPRN_HASH1     0x3D2           /* Primary Hash Address 
> Register */
>  #define SPRN_HASH2     0x3D3           /* Secondary Hash Address 
> Resgister */
>  #define SPRN_HID0      0x3F0           /* Hardware Implementation 
> Register 0 */
> +#ifdef __ASSEMBLY__
> +#define HID0_QATTN     (1<<35)         /* Sup. Proc. attn insn all 
> threads */
> +#define HID0_ATTN      (1<<32)         /* Sup. Proc. attn insn */
> +#else
> +#define HID0_QATTN     (1UL<<35)       /* Sup. Proc. attn insn all 
> threads */
> +#define HID0_ATTN      (1UL<<32)       /* Sup. Proc. attn insn */
> +#endif
>  #define HID0_EMCP      (1<<31)         /* Enable Machine Check pin */
>  #define HID0_EBA       (1<<29)         /* Enable Bus Address Parity */
>  #define HID0_EBD       (1<<28)         /* Enable Bus Data Parity */
> @@ -641,6 +648,13 @@ extern void ppc64_runlatch_off(void);
>  extern unsigned long scom970_read(unsigned int address);
>  extern void scom970_write(unsigned int address, unsigned long value);
>
> +/*
> + * Support Processor Attention Instruction instroduced in POWER
> + * architecture processors as of RS64, tho may not be supported by
> + * POWER 3.
> + */
> +#define ATTN() asm volatile("attn; nop")
> +


Fairly certian POWER3 does NOT implement this, but I don't have book IV 
handy.
Does one of the processors require the nop ?



>  #else
>  #define ppc64_runlatch_on()
>  #define ppc64_runlatch_off()
>

milton

Re: Take 2: [RFC] Debugging with a HW probe.

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

On Tue, 2006-08-22 at 01:04 -0500, Milton Miller wrote:
> On Aug 14, 2006, at 5:16 AM, Jimi Xenidis jimix at watson.ibm.com  
> > @@ -693,6 +696,11 @@ #ifdef CONFIG_PPC64
> >         if (of_get_flat_dt_prop(node, "linux,iommu-force-on", NULL) != 
> > NULL)
> >                 iommu_force_on = 1;
> >  #endif
> > +#ifdef CONFIG_HW_PROBE_ENABLE
> > +       if (of_get_flat_dt_prop(node, "linux,hw-probe-enable", NULL) 
> > != NULL) {
> > +               hw_probe_enabled = 1;
> > +               DBG("HW Probe will be enabled\n");
> > +#endif
> 
> No.  [see next comment]
> 
> >
> >         /* mem=x on the command line is the preferred mechanism */
> >         lprop = of_get_flat_dt_prop(node, "linux,memory-limit", NULL);
> > diff --git a/arch/powerpc/kernel/prom_init.c 
> > b/arch/powerpc/kernel/prom_init.c
> > index 90972ef..26428de 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -587,6 +587,14 @@ #ifdef CONFIG_PPC64
> >                         RELOC(iommu_force_on) = 1;
> >         }
> >  #endif
> > +#ifdef CONFIG_HW_PROBE_ENABLE
> > +       opt = strstr(RELOC(prom_cmd_line), RELOC("hwprobe"));
> > +       if (opt) {
> > +               prom_printf("WARNING! HW Probe will be activated!\n");
> > +               prom_setprop(_prom->chosen, "/chosen",
> > +                            "linux,hw-probe-enable", NULL, 0);
> > +       }
> > +#endif
> >  }
> 
> Please, PLEASE do NOT do this.
> 
> prom_init.c is only used by one of the many flat device tree generators,
> namely the open-firmware client.  Adding a property like this requires 
> us
> to update all the other clients.
> 
> And there is no reason to parse it this early.
> 
> Instead, parse it from the command line like the other early parsing.
> 
> I thing a generic early_param would be fine.  However, xmon_init is an
> early_parm in setup-common, so if you really require it before the first
> call, then you could parse it next to mem= at the bottom of
> early_init_dt_scan_chosen.

What Milton said, we learnt that the hard way with kexec :)

Except that we don't do mem= in early_init_dt_scan_chosen anymore (get a
newer kernel Milton!).

The early_param parsing is done very early, and the xmon= parsing does
not jump into xmon, it waits until a little later before doing it. So an
early_param should be fine, ie. it will be parsed before xmon ever runs.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: Take 2: [RFC] Debugging with a HW probe.

[Jimi Xenidis]

On Aug 22, 2006, at 2:04 AM, Milton Miller wrote:

>
> On Aug 14, 2006, at 5:16 AM, Jimi Xenidis jimix at watson.ibm.com   
> wrote:
>>
>> Signed-off-by: Jimi Xenidis <jimix at watson.ibm.com>
>
> [sorry for the list archive patch munging]
>
>> ---
>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
>> index e29ef77..bc4cdf9 100644
>> --- a/arch/powerpc/Kconfig.debug
>> +++ b/arch/powerpc/Kconfig.debug
>> @@ -61,6 +61,17 @@ config KGDB_CONSOLE
>>           over the gdb stub.
>>           If unsure, say N.
>>
>> +config ENABLE_HW_PROBE
>> +       bool "Allow instructions that contact a hardware probe  
>> (dangerous)"
>> +       depends on PPC64
>
> Not having this depend on DEBUGGER but in the middle of things that  
> do will
> get you scorn from the auto-indenting police.
ACK

>
> Since we can only call this from xmon, should it depend on XMON  
> (and be
> placed after that)?

If the HW probe is available, XMON will use it.
However, enabling the HW probe also means that I can (as a developer)  
insert ATTN() anywhere in my code and have the probe stop there. I  
even have kernels where BUG() contains ATTN(), but thats another patch.

>
>> +       help
>> +         If you enable this AND you add "hwprobe" to the cmdline,  
>> the
>> +         processor will enable instructions that contact the  
>> hardware
>> +         probe.  These instructions ca be used in all processor  
>> modes
>
> can
ACK

>
>> +         _including_ user mode and are only useful for kernel
>
> not sure this _highlighting_ is used elsewhere in Kconfig help ...

I was looking for an example and found "_will not boot_" in this very  
file.

>
> Should we mention that a hardware probe is required to continue  
> exectuion?
> In other words, its not just contact, but signal and wait for a hw  
> probe?
Hows this?
          If you enable this AND you add "hwprobe" to the cmdline, the
          processor will enable instructions that signal and wait for
          the hardware probe, _stopping_ the processor.  These
          instructions can be used in all processor modes _including_
          user mode and are only useful for kernel development and
          debugging.  DO NOT enable this unless you plan to use it.  If
          you DO NOT have a hardware probe, answer N.

[snip]
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -587,6 +587,14 @@ #ifdef CONFIG_PPC64
>>                         RELOC(iommu_force_on) = 1;
>>         }
>>  #endif
>> +#ifdef CONFIG_HW_PROBE_ENABLE
>> +       opt = strstr(RELOC(prom_cmd_line), RELOC("hwprobe"));
>> +       if (opt) {
>> +               prom_printf("WARNING! HW Probe will be activated! 
>> \n");
>> +               prom_setprop(_prom->chosen, "/chosen",
>> +                            "linux,hw-probe-enable", NULL, 0);
>> +       }
>> +#endif
>>  }
>
> Please, PLEASE do NOT do this.

Ok, I won't. I somehow missed "early_param()" that should do just fine.

>
> Which .h is hw_probe_enabled in?  (none?)

Correct, this RFC is to get valuable feedback (thank you), the  
setting the global is the current goal, I'm waiting for some other  
patches from other people to get in before we start going after HID  
bits that this global, may or may not be part of.

>
>>
>>  #ifdef CONFIG_PPC_PSERIES
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index 179b10c..51a1e4e 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -189,7 +189,12 @@ #endif
>>    dd   dump double values\n\
>>    dr   dump stream of raw bytes\n\
>>    e    print exception information\n\
>> -  f    flush cache\n\
>> +  f    flush cache\n"
>> +#ifdef CONFIG_ENABLE_HW_PROBE
>> +  "\
>> +  H     Contact hardware probe, if available\n"
>> +#endif
>> +  "\
>
> While this style does keep the lines aligned in the source, it adds  
> veritcal
> almost-whitespace.

Are you expressing "dislike"?

>   And I notice a different choice was made at the bottom.
I borrowed from the CONFIG_SMP above and avoided putting '"' in col 0  
but am happy to compress.

[snip]
>> @@ -641,6 +648,13 @@ extern void ppc64_runlatch_off(void);
>>  extern unsigned long scom970_read(unsigned int address);
>>  extern void scom970_write(unsigned int address, unsigned long  
>> value);
>>
>> +/*
>> + * Support Processor Attention Instruction instroduced in POWER
>> + * architecture processors as of RS64, tho may not be supported by
>> + * POWER 3.
>> + */
>> +#define ATTN() asm volatile("attn; nop")
>> +
>
>
> Fairly certian POWER3 does NOT implement this, but I don't have  
> book IV handy.
The book4 mentions "BPUBKT" and when asked no one could give a  
definite answerm, I figured I'd try an see at some point.  Having  
said that, you give me a s/Fairly/Absolutely/ and its GONE!

> Does one of the processors require the nop ?

The nop is there so that once in the probe, depending on your probe  
interface, you can easily skip it by adding 4 to the PC knowing you  
would get a nop.  This is not entirely necessary but some processor/ 
probes have a PC and a "next PC".  I have had little experience with  
this probe and wanted to make sure I had something to set both to if  
it was needed.

Thanks for your feedback.
-JX

Re: [RFC] Debugging with a HW probe.

[Jimi Xenidis]

On Aug 8, 2006, at 9:22 PM, Milton Miller wrote:

> On Sun Aug  6 2006 09:42:16 AM CDT, Jimi Xenidis wrote:
>> On the XenPPC project we've been playing around with using GDB to
>> source debug Linux (and Xen) using a RiscWatch HW probe.
>
>> We have found it useful to teach xmon to make this call so we can  
>> then
>> debug the SW thru the probe. Is this useful to anyone else
>
> Since you only invoke the attention on command from the debugger, how
> is this better than just invoking the stop command from the probe?   
> Is it you
> are not dispatched out?  That you are not at a random point in the  
> input
> polling?

Thats exactly it!
If I get the probe to stop the CPU I could stop in the OS or in a  
user app.  Since we are working on Xen it could be Xen or any number  
of OSes or user apps on OSes.
When interrogating the machine state, the probe (almost) respects the  
modes of the CPU and MMU so memory accesses are translated (if xlate  
is on).  Since, probe "soft breakpoints" are ATTN instructions as  
well they get written into memory by the probe.

Having SW "call out" allows me to make sure that I'm in the address  
space I care about.
Since I only invoke it from xmon it assumes you have xmon support and  
to really be useful you need to SysReq to xmon.
I could be useful to make this part of SysReq, but I think we want to  
make sure that its use is entirely intentional.
You can also insert 'asm volatile(".long 0x200;nop");' in you own  
code or even add it to BUG()/panic() and skip the suffering the xmon/ 
SysReq trap.

BTW: I say "almost" because the probe does not respect the truncating  
of the upper bits when xlate is off and 0xC00... addresses are  
useless, we hacked the gdb stub to work around that.
BTW2: (I'm sure obvious to some), Since the probe writes ATTN  
instructions, I do not know what would happen if the probe wrote ATTN  
to an unmapped page, this is why debugging user space might be useless.
-JX

Re: [RFC] Debugging with a HW probe.

[Milton Miller]

On Sun Aug  6 2006 09:42:16 AM CDT, Jimi Xenidis wrote:
> On the XenPPC project we've been playing around with using GDB to
> source debug Linux (and Xen) using a RiscWatch HW probe.

> We have found it useful to teach xmon to make this call so we can then
> debug the SW thru the probe. Is this useful to anyone else

Since you only invoke the attention on command from the debugger, how
is this better than just invoking the stop command from the probe?  Is it you
are not dispatched out?  That you are not at a random point in the input
polling?

milton