Skip to contentSkip to conversation

PS3: Fix hardware watchpoints

PATCH In review Geoff Levand 3 participants 4 messages Mar 14, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 PS3: Fix hardware watchpoints

From: Arnd Bergmann <[email protected]>

Add PS3 support for the PowerPC processor's Data Address Breakpoint Register (DABR).

---
Paul, please consider for 2.6.21.

 arch/powerpc/platforms/ps3/setup.c |    7 +++++++
 1 files changed, 7 insertions(+)
arch/powerpc/platforms/ps3/setup.c +7 -0
@@ -137,6 +137,12 @@ early_param("ps3fb", early_parse_ps3fb);
 #define prealloc_ps3fb_videomemory()	do { } while (0)
 #endif
 
+static int ps3_set_dabr(u64 dabr)
+{
+	enum {DABR_USER = 1, DABR_KERNEL = 2,};
+
+	return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0;
+}
 
 static void __init ps3_setup_arch(void)
 {
@@ -234,6 +240,7 @@ define_machine(ps3) {
 	.get_boot_time			= ps3_get_boot_time,
 	.set_rtc_time			= ps3_set_rtc_time,
 	.get_rtc_time			= ps3_get_rtc_time,
+	.set_dabr			= ps3_set_dabr,
 	.calibrate_decr			= ps3_calibrate_decr,
 	.progress			= ps3_progress,
 	.restart			= ps3_restart,
#1 lore raw
Quoted 8 lines · Geoff Levand

Add PS3 support for the PowerPC processor's Data Address Breakpoint Register (DABR).

---
Paul, please consider for 2.6.21.

It's nice and small, but it isn't really a bugfix as far as I can see... What's the downside of it not going into 2.6.21, apart from some inconvenience in debugging things?

Paul.

#2 lore raw
Quoted 8 lines · Paul Mackerras
Quoted 6 lines · Geoff Levand

Add PS3 support for the PowerPC processor's Data Address Breakpoint Register (DABR).

Quoted 6 lines
Quoted 2 lines

---
Paul, please consider for 2.6.21.

It's nice and small, but it isn't really a bugfix as far as I can see... What's the downside of it not going into 2.6.21, apart from some inconvenience in debugging things?

It makes it possible for a regular user to hang the partition, because the kernel falls back to access the actual SPR, which is forbidden by the hypervisor.

My original changelog text was:

The lv1 hypervisor silently kills the partition if the kernel attempts to access the dabr register directly. Instead, it needs to be set using an hcall. Since both pseries and celleb have the same function, the guess is that the implementation on ps3 is also the same.

How about making the actual changelog text something like this:

----
The lv1 hypervisor silently kills the partition if the kernel attempts to access the dabr register directly, which can happen when gdb adds a hardware breakpoint.
Instead, the kernel needs to be set using an hcall, the same way as it does for pseries and celleb.
----

The real question however should be if there is a point in merging any ps3 patches into 2.6.21 at this point. Geoff, my understanding is that the current ps3 platform code in 2.6.21-rc still relies on additional patches from you in order to even boot, right?

Maybe it's better to just queue them for 2.6.22 now, independent of the severity of bugs fixed by a given patch.

	Arnd <><
#3 lore raw
Quoted 16 lines · Arnd Bergmann
Quoted 14 lines · Paul Mackerras
Quoted 8 lines · Geoff Levand

Add PS3 support for the PowerPC processor's Data Address Breakpoint Register (DABR).

---
Paul, please consider for 2.6.21.

It's nice and small, but it isn't really a bugfix as far as I can see... What's the downside of it not going into 2.6.21, apart from some inconvenience in debugging things?

...

Quoted 2 lines

Maybe it's better to just queue them for 2.6.22 now, independent of the severity of bugs fixed by a given patch.

It doesn't matter so much, that's why I just said to consider for 2.6.21.

Paul, please just add to your for-2.6.22 branch.

-Geoff