* [PATCH 1/5] powerpc: Pack arch_hw_breakpoint to avoid holes in struct
@ 2012-09-06 5:17 Michael Neuling
2012-09-06 5:17 ` [PATCH 2/5] powerpc: Use consistent name info for arch_hw_breakpoint Michael Neuling
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Michael Neuling @ 2012-09-06 5:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Michael Neuling, linuxppc-dev
No functional change
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/include/asm/hw_breakpoint.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index be04330..39b323e 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -27,10 +27,10 @@
#ifdef CONFIG_HAVE_HW_BREAKPOINT
struct arch_hw_breakpoint {
- bool extraneous_interrupt;
- u8 len; /* length of the target data symbol */
- int type;
unsigned long address;
+ int type;
+ u8 len; /* length of the target data symbol */
+ bool extraneous_interrupt;
};
#include <linux/kdebug.h>
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/5] powerpc: Use consistent name info for arch_hw_breakpoint 2012-09-06 5:17 [PATCH 1/5] powerpc: Pack arch_hw_breakpoint to avoid holes in struct Michael Neuling @ 2012-09-06 5:17 ` Michael Neuling 2012-09-06 5:17 ` [PATCH 3/5] powerpc: Use the XDABR hcall Michael Neuling ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-06 5:17 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Michael Neuling, linuxppc-dev Change bp_info to info to be consistent with the rest of this file. Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/kernel/hw_breakpoint.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 956a4c4..6767445 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -294,7 +294,7 @@ int __kprobes single_step_dabr_instruction(struct die_args *args) { struct pt_regs *regs = args->regs; struct perf_event *bp = NULL; - struct arch_hw_breakpoint *bp_info; + struct arch_hw_breakpoint *info; bp = current->thread.last_hit_ubp; /* @@ -304,16 +304,16 @@ int __kprobes single_step_dabr_instruction(struct die_args *args) if (!bp) return NOTIFY_DONE; - bp_info = counter_arch_bp(bp); + info = counter_arch_bp(bp); /* * We shall invoke the user-defined callback function in the single * stepping handler to confirm to 'trigger-after-execute' semantics */ - if (!bp_info->extraneous_interrupt) + if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(bp_info->address | bp_info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION); current->thread.last_hit_ubp = NULL; /* -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] powerpc: Use the XDABR hcall 2012-09-06 5:17 [PATCH 1/5] powerpc: Pack arch_hw_breakpoint to avoid holes in struct Michael Neuling 2012-09-06 5:17 ` [PATCH 2/5] powerpc: Use consistent name info for arch_hw_breakpoint Michael Neuling @ 2012-09-06 5:17 ` Michael Neuling 2012-09-06 5:17 ` [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling 2012-09-06 5:17 ` [PATCH 5/5] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling 3 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-06 5:17 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Michael Neuling, linuxppc-dev We never use the XDABR hcall since we check for DABR hcall first. XDABR syscall is better since it allows us to also set the DABRX. Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/platforms/pseries/setup.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 51ecac9..36b7744 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -529,10 +529,10 @@ static void __init pSeries_init_early(void) if (firmware_has_feature(FW_FEATURE_LPAR)) hvc_vio_init_early(); #endif - if (firmware_has_feature(FW_FEATURE_DABR)) - ppc_md.set_dabr = pseries_set_dabr; - else if (firmware_has_feature(FW_FEATURE_XDABR)) + if (firmware_has_feature(FW_FEATURE_XDABR)) ppc_md.set_dabr = pseries_set_xdabr; + else if (firmware_has_feature(FW_FEATURE_DABR)) + ppc_md.set_dabr = pseries_set_dabr; pSeries_cmo_feature_init(); iommu_init_early_pSeries(); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-06 5:17 [PATCH 1/5] powerpc: Pack arch_hw_breakpoint to avoid holes in struct Michael Neuling 2012-09-06 5:17 ` [PATCH 2/5] powerpc: Use consistent name info for arch_hw_breakpoint Michael Neuling 2012-09-06 5:17 ` [PATCH 3/5] powerpc: Use the XDABR hcall Michael Neuling @ 2012-09-06 5:17 ` Michael Neuling 2012-09-06 5:35 ` Geert Uytterhoeven 2012-09-06 5:17 ` [PATCH 5/5] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling 3 siblings, 1 reply; 15+ messages in thread From: Michael Neuling @ 2012-09-06 5:17 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Michael Neuling, linuxppc-dev Rework set_dabr to take a DABRX value as well. We are not actually changing any functionality at this stage, just preparing for that. The SET_XDABR hcall checks to make sure DABRX is non-zero, and if it is it barfs. So in this case when we are clearing both DABR and DABRX, just set 1 bit to make sure the hcall doesn't fail. Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/include/asm/debug.h | 2 +- arch/powerpc/include/asm/hw_breakpoint.h | 2 +- arch/powerpc/include/asm/machdep.h | 3 ++- arch/powerpc/include/asm/processor.h | 1 + arch/powerpc/include/asm/reg.h | 3 +++ arch/powerpc/kernel/hw_breakpoint.c | 12 ++++++------ arch/powerpc/kernel/process.c | 14 +++++++------- arch/powerpc/kernel/ptrace.c | 3 +++ arch/powerpc/kernel/signal.c | 2 +- arch/powerpc/platforms/cell/beat.c | 4 ++-- arch/powerpc/platforms/cell/beat.h | 2 +- arch/powerpc/platforms/ps3/setup.c | 6 ++---- arch/powerpc/platforms/pseries/setup.c | 9 ++++++--- arch/powerpc/xmon/xmon.c | 4 ++-- 14 files changed, 38 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h index 716d2f0..32de257 100644 --- a/arch/powerpc/include/asm/debug.h +++ b/arch/powerpc/include/asm/debug.h @@ -44,7 +44,7 @@ static inline int debugger_dabr_match(struct pt_regs *regs) { return 0; } static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; } #endif -extern int set_dabr(unsigned long dabr); +extern int set_dabr(unsigned long dabr, unsigned long dabrx); #ifdef CONFIG_PPC_ADV_DEBUG_REGS extern void do_send_trap(struct pt_regs *regs, unsigned long address, unsigned long error_code, int signal_code, int brkpt); diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index 39b323e..c6f48eb 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -61,7 +61,7 @@ extern void ptrace_triggered(struct perf_event *bp, struct perf_sample_data *data, struct pt_regs *regs); static inline void hw_breakpoint_disable(void) { - set_dabr(0); + set_dabr(0, 0); } extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs); diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 42ce570..236b477 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -180,7 +180,8 @@ struct machdep_calls { void (*enable_pmcs)(void); /* Set DABR for this platform, leave empty for default implemenation */ - int (*set_dabr)(unsigned long dabr); + int (*set_dabr)(unsigned long dabr, + unsigned long dabrx); #ifdef CONFIG_PPC32 /* XXX for now */ /* A general init function, called by ppc_init in init/main.c. diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index 54b73a2..17b58e5 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -219,6 +219,7 @@ struct thread_struct { #endif /* CONFIG_HAVE_HW_BREAKPOINT */ #endif unsigned long dabr; /* Data address breakpoint register */ + unsigned long dabrx; /* ... extension */ #ifdef CONFIG_ALTIVEC /* Complete AltiVec register set */ vector128 vr[32] __attribute__((aligned(16))); diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 6386086..334be34 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -208,6 +208,9 @@ #define SPRN_DABRX 0x3F7 /* Data Address Breakpoint Register Extension */ #define DABRX_USER (1UL << 0) #define DABRX_KERNEL (1UL << 1) +#define DABRX_HYP (1UL << 2) +#define DABRX_BTI (1UL << 3) +#define DABRX_ALL (DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER) #define SPRN_DAR 0x013 /* Data Address Register */ #define SPRN_DBCR 0x136 /* e300 Data Breakpoint Control Reg */ #define SPRN_DSISR 0x012 /* Data Storage Interrupt Status Register */ diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 6767445..6891d79 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -73,7 +73,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp) * If so, DABR will be populated in single_step_dabr_instruction(). */ if (current->thread.last_hit_ubp != bp) - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); return 0; } @@ -97,7 +97,7 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp) } *slot = NULL; - set_dabr(0); + set_dabr(0, 0); } /* @@ -197,7 +197,7 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs) info = counter_arch_bp(tsk->thread.last_hit_ubp); regs->msr &= ~MSR_SE; - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); tsk->thread.last_hit_ubp = NULL; } @@ -215,7 +215,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args) unsigned long dar = regs->dar; /* Disable breakpoints during exception handling */ - set_dabr(0); + set_dabr(0, 0); /* * The counter may be concurrently released but that can only @@ -281,7 +281,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); out: rcu_read_unlock(); return rc; @@ -313,7 +313,7 @@ int __kprobes single_step_dabr_instruction(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); current->thread.last_hit_ubp = NULL; /* diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 1a1f2dd..53c32a9 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -283,7 +283,7 @@ void do_dabr(struct pt_regs *regs, unsigned long address, return; /* Clear the DABR */ - set_dabr(0); + set_dabr(0, 0); /* Deliver the signal to userspace */ info.si_signo = SIGTRAP; @@ -364,18 +364,19 @@ static void set_debug_reg_defaults(struct thread_struct *thread) { if (thread->dabr) { thread->dabr = 0; - set_dabr(0); + thread->dabrx = 0; + set_dabr(0, 0); } } #endif /* !CONFIG_HAVE_HW_BREAKPOINT */ #endif /* CONFIG_PPC_ADV_DEBUG_REGS */ -int set_dabr(unsigned long dabr) +int set_dabr(unsigned long dabr, unsigned long dabrx) { __get_cpu_var(current_dabr) = dabr; if (ppc_md.set_dabr) - return ppc_md.set_dabr(dabr); + return ppc_md.set_dabr(dabr, dabrx); /* XXX should we have a CPU_FTR_HAS_DABR ? */ #ifdef CONFIG_PPC_ADV_DEBUG_REGS @@ -385,9 +386,8 @@ int set_dabr(unsigned long dabr) #endif #elif defined(CONFIG_PPC_BOOK3S) mtspr(SPRN_DABR, dabr); + mtspr(SPRN_DABRX, dabrx); #endif - - return 0; } @@ -480,7 +480,7 @@ struct task_struct *__switch_to(struct task_struct *prev, */ #ifndef CONFIG_HAVE_HW_BREAKPOINT if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr)) - set_dabr(new->thread.dabr); + set_dabr(new->thread.dabr, new->thread.dabrx); #endif /* CONFIG_HAVE_HW_BREAKPOINT */ #endif diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index c10fc28..79d8e56 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -960,6 +960,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, thread->ptrace_bps[0] = bp; ptrace_put_breakpoints(task); thread->dabr = data; + thread->dabrx = DABRX_ALL; return 0; } @@ -983,6 +984,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, /* Move contents to the DABR register */ task->thread.dabr = data; + task->thread.dabrx = DABRX_ALL; #else /* CONFIG_PPC_ADV_DEBUG_REGS */ /* As described above, it was assumed 3 bits were passed with the data * address, but we will assume only the mode bits will be passed @@ -1397,6 +1399,7 @@ static long ppc_set_hwdebug(struct task_struct *child, dabr |= DABR_DATA_WRITE; child->thread.dabr = dabr; + child->thread.dabrx = DABRX_ALL; return 1; #endif /* !CONFIG_PPC_ADV_DEBUG_DVCS */ diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c index 5c023c9..b936b45 100644 --- a/arch/powerpc/kernel/signal.c +++ b/arch/powerpc/kernel/signal.c @@ -130,7 +130,7 @@ static int do_signal(struct pt_regs *regs) * triggered inside the kernel. */ if (current->thread.dabr) - set_dabr(current->thread.dabr); + set_dabr(current->thread.dabr, current->thread.dabrx); #endif /* Re-enable the breakpoints for the signal stack */ thread_change_pc(current, regs); diff --git a/arch/powerpc/platforms/cell/beat.c b/arch/powerpc/platforms/cell/beat.c index 852592b..affcf56 100644 --- a/arch/powerpc/platforms/cell/beat.c +++ b/arch/powerpc/platforms/cell/beat.c @@ -136,9 +136,9 @@ ssize_t beat_nvram_get_size(void) return BEAT_NVRAM_SIZE; } -int beat_set_xdabr(unsigned long dabr) +int beat_set_xdabr(unsigned long dabr, unsigned long dabrx) { - if (beat_set_dabr(dabr, DABRX_KERNEL | DABRX_USER)) + if (beat_set_dabr(dabr, dabrx)) return -1; return 0; } diff --git a/arch/powerpc/platforms/cell/beat.h b/arch/powerpc/platforms/cell/beat.h index 32c8efc..bfcb8e3 100644 --- a/arch/powerpc/platforms/cell/beat.h +++ b/arch/powerpc/platforms/cell/beat.h @@ -32,7 +32,7 @@ void beat_get_rtc_time(struct rtc_time *); ssize_t beat_nvram_get_size(void); ssize_t beat_nvram_read(char *, size_t, loff_t *); ssize_t beat_nvram_write(char *, size_t, loff_t *); -int beat_set_xdabr(unsigned long); +int beat_set_xdabr(unsigned long, unsigned long); void beat_power_save(void); void beat_kexec_cpu_down(int, int); diff --git a/arch/powerpc/platforms/ps3/setup.c b/arch/powerpc/platforms/ps3/setup.c index 2d664c5..ef7de10 100644 --- a/arch/powerpc/platforms/ps3/setup.c +++ b/arch/powerpc/platforms/ps3/setup.c @@ -184,11 +184,9 @@ early_param("ps3flash", early_parse_ps3flash); #define prealloc_ps3flash_bounce_buffer() do { } while (0) #endif -static int ps3_set_dabr(unsigned long dabr) +static int ps3_set_dabr(unsigned long dabr, unsigned long dabrx) { - enum {DABR_USER = 1, DABR_KERNEL = 2,}; - - return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0; + return lv1_set_dabr(dabr, dabrx) ? -1 : 0; } static void __init ps3_setup_arch(void) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 36b7744..5ea8e4f 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -416,16 +416,19 @@ static int __init pSeries_init_panel(void) } machine_arch_initcall(pseries, pSeries_init_panel); -static int pseries_set_dabr(unsigned long dabr) +static int pseries_set_dabr(unsigned long dabr, unsigned long dabrx) { return plpar_hcall_norets(H_SET_DABR, dabr); } -static int pseries_set_xdabr(unsigned long dabr) +static int pseries_set_xdabr(unsigned long dabr, unsigned long dabrx) { + /* Have to set at least one bit in the DABRX according to PAPR */ + if (dabrx == 0 && dabr == 0) + dabrx = H_DABRX_USER; /* We want to catch accesses from kernel and userspace */ return plpar_hcall_norets(H_SET_XDABR, dabr, - H_DABRX_KERNEL | H_DABRX_USER); + dabrx & (H_DABRX_KERNEL | H_DABRX_USER)); } #define CMO_CHARACTERISTICS_TOKEN 44 diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 9b49c65..987f441 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -740,7 +740,7 @@ static void insert_bpts(void) static void insert_cpu_bpts(void) { if (dabr.enabled) - set_dabr(dabr.address | (dabr.enabled & 7)); + set_dabr(dabr.address | (dabr.enabled & 7), DABRX_ALL); if (iabr && cpu_has_feature(CPU_FTR_IABR)) mtspr(SPRN_IABR, iabr->address | (iabr->enabled & (BP_IABR|BP_IABR_TE))); @@ -768,7 +768,7 @@ static void remove_bpts(void) static void remove_cpu_bpts(void) { - set_dabr(0); + set_dabr(0, 0); if (cpu_has_feature(CPU_FTR_IABR)) mtspr(SPRN_IABR, 0); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-06 5:17 ` [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling @ 2012-09-06 5:35 ` Geert Uytterhoeven 2012-09-07 3:37 ` Michael Neuling 0 siblings, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2012-09-06 5:35 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev On Thu, Sep 6, 2012 at 7:17 AM, Michael Neuling <mikey@neuling.org> wrote: > Rework set_dabr to take a DABRX value as well. We are not actually > changing any functionality at this stage, just preparing for that. You are changing functionality. > #define DABRX_USER (1UL << 0) > #define DABRX_KERNEL (1UL << 1) > +#define DABRX_HYP (1UL << 2) > +#define DABRX_BTI (1UL << 3) > +#define DABRX_ALL (DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER) > --- a/arch/powerpc/platforms/cell/beat.c > +++ b/arch/powerpc/platforms/cell/beat.c > @@ -136,9 +136,9 @@ ssize_t beat_nvram_get_size(void) > return BEAT_NVRAM_SIZE; > } > > -int beat_set_xdabr(unsigned long dabr) > +int beat_set_xdabr(unsigned long dabr, unsigned long dabrx) > { > - if (beat_set_dabr(dabr, DABRX_KERNEL | DABRX_USER)) > + if (beat_set_dabr(dabr, dabrx)) > return -1; > return 0; > } > --- a/arch/powerpc/platforms/ps3/setup.c > +++ b/arch/powerpc/platforms/ps3/setup.c > @@ -184,11 +184,9 @@ early_param("ps3flash", early_parse_ps3flash); > #define prealloc_ps3flash_bounce_buffer() do { } while (0) > #endif > > -static int ps3_set_dabr(unsigned long dabr) > +static int ps3_set_dabr(unsigned long dabr, unsigned long dabrx) > { > - enum {DABR_USER = 1, DABR_KERNEL = 2,}; > - > - return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0; > + return lv1_set_dabr(dabr, dabrx) ? -1 : 0; > } > - set_dabr(dabr.address | (dabr.enabled & 7)); > + set_dabr(dabr.address | (dabr.enabled & 7), DABRX_ALL); Before, beat_set_dabr() and lv1_set_dabr() would have been called with dabrx = 3 (DABRX_KERNEL | DABRX_USER). Now they're called with dabrx = 15 (DABRX_ALL = DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER). No idea what's the impact of this... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-06 5:35 ` Geert Uytterhoeven @ 2012-09-07 3:37 ` Michael Neuling 2012-09-07 5:26 ` Geert Uytterhoeven 0 siblings, 1 reply; 15+ messages in thread From: Michael Neuling @ 2012-09-07 3:37 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: linuxppc-dev Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Sep 6, 2012 at 7:17 AM, Michael Neuling <mikey@neuling.org> wrote: > > Rework set_dabr to take a DABRX value as well. We are not actually > > changing any functionality at this stage, just preparing for that. > > You are changing functionality. You are right.. I'll fix that up.. Sorry. > > > #define DABRX_USER (1UL << 0) > > #define DABRX_KERNEL (1UL << 1) > > +#define DABRX_HYP (1UL << 2) > > +#define DABRX_BTI (1UL << 3) > > +#define DABRX_ALL (DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER) > > > --- a/arch/powerpc/platforms/cell/beat.c > > +++ b/arch/powerpc/platforms/cell/beat.c > > @@ -136,9 +136,9 @@ ssize_t beat_nvram_get_size(void) > > return BEAT_NVRAM_SIZE; > > } > > > > -int beat_set_xdabr(unsigned long dabr) > > +int beat_set_xdabr(unsigned long dabr, unsigned long dabrx) > > { > > - if (beat_set_dabr(dabr, DABRX_KERNEL | DABRX_USER)) > > + if (beat_set_dabr(dabr, dabrx)) > > return -1; > > return 0; > > } > > > --- a/arch/powerpc/platforms/ps3/setup.c > > +++ b/arch/powerpc/platforms/ps3/setup.c > > @@ -184,11 +184,9 @@ early_param("ps3flash", early_parse_ps3flash); > > #define prealloc_ps3flash_bounce_buffer() do { } while (0) > > #endif > > > > -static int ps3_set_dabr(unsigned long dabr) > > +static int ps3_set_dabr(unsigned long dabr, unsigned long dabrx) > > { > > - enum {DABR_USER = 1, DABR_KERNEL = 2,}; > > - > > - return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0; > > + return lv1_set_dabr(dabr, dabrx) ? -1 : 0; > > } > > > - set_dabr(dabr.address | (dabr.enabled & 7)); > > + set_dabr(dabr.address | (dabr.enabled & 7), DABRX_ALL); > > Before, beat_set_dabr() and lv1_set_dabr() would have been called with dabrx = 3 > (DABRX_KERNEL | DABRX_USER). Now they're called with dabrx = 15 > (DABRX_ALL = DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER). > > No idea what's the impact of this... Do you know if the ps3 hypervisor will allow us to set DABRX_BTI or DABRX_HYP? phyp wont. Mikey ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-07 3:37 ` Michael Neuling @ 2012-09-07 5:26 ` Geert Uytterhoeven 2012-09-07 5:43 ` Michael Neuling ` (4 more replies) 0 siblings, 5 replies; 15+ messages in thread From: Geert Uytterhoeven @ 2012-09-07 5:26 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev Hi Mikey, On Fri, 7 Sep 2012, Michael Neuling wrote: > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Thu, Sep 6, 2012 at 7:17 AM, Michael Neuling <mikey@neuling.org> w= rote: > > > Rework set_dabr to take a DABRX value as well. We are not actually > > > changing any functionality at this stage, just preparing for that. > >=20 > > You are changing functionality. >=20 > You are right.. I'll fix that up.. Sorry. >=20 > > > #define DABRX_USER (1UL << 0) > > > #define DABRX_KERNEL (1UL << 1) > > > +#define DABRX_HYP (1UL << 2) > > > +#define DABRX_BTI (1UL << 3) > > > +#define DABRX_ALL (DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DA= BRX_USER) > >=20 > > > --- a/arch/powerpc/platforms/cell/beat.c > > > +++ b/arch/powerpc/platforms/cell/beat.c > > > @@ -136,9 +136,9 @@ ssize_t beat_nvram_get_size(void) > > > return BEAT_NVRAM_SIZE; > > > } > > > > > > -int beat_set_xdabr(unsigned long dabr) > > > +int beat_set_xdabr(unsigned long dabr, unsigned long dabrx) > > > { > > > - if (beat_set_dabr(dabr, DABRX_KERNEL | DABRX_USER)) > > > + if (beat_set_dabr(dabr, dabrx)) > > > return -1; > > > return 0; > > > } > >=20 > > > --- a/arch/powerpc/platforms/ps3/setup.c > > > +++ b/arch/powerpc/platforms/ps3/setup.c > > > @@ -184,11 +184,9 @@ early_param("ps3flash", early_parse_ps3flash); > > > #define prealloc_ps3flash_bounce_buffer() do { } while (0) > > > #endif > > > > > > -static int ps3_set_dabr(unsigned long dabr) > > > +static int ps3_set_dabr(unsigned long dabr, unsigned long dabrx) > > > { > > > - enum {DABR_USER =3D 1, DABR_KERNEL =3D 2,}; > > > - > > > - return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0= ; > > > + return lv1_set_dabr(dabr, dabrx) ? -1 : 0; > > > } > >=20 > > > - set_dabr(dabr.address | (dabr.enabled & 7)); > > > + set_dabr(dabr.address | (dabr.enabled & 7), DABRX_A= LL); > >=20 > > Before, beat_set_dabr() and lv1_set_dabr() would have been called wit= h dabrx =3D 3 > > (DABRX_KERNEL | DABRX_USER). Now they're called with dabrx =3D 15 > > (DABRX_ALL =3D DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER). > >=20 > > No idea what's the impact of this... >=20 > Do you know if the ps3 hypervisor will allow us to set DABRX_BTI or > DABRX_HYP? phyp wont. =20 According to the documenation, all bits but DABRX_USER, DABRX_KERNEL, and DABRX_BTI must be zero. This implies DABRX_HYP cannot be set. BTW, the requirement that DABRX_USER and DABRX_KERNEL cannot both be zero at the same time is also there, cfr. your comment and check in pseries_set_xdabr(). Unfortunately, I cannot test it. With kind regards, Geert Uytterhoeven Software Architect Technology and Software Centre Europe Sony Belgium, bijkantoor van Sony Europe Limited. Da Vincilaan 7-D1 =C2=B7 B-1935 Zaventem =C2=B7 Belgium Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Sony Europe Limited. A company registered in England and Wales. Registered office: The Heights, Brooklands, Weybridge, Surrey. KT13 0XW. United Kingdom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-07 5:26 ` Geert Uytterhoeven @ 2012-09-07 5:43 ` Michael Neuling 2012-09-10 12:27 ` Geert Uytterhoeven 2012-09-07 7:24 ` [PATCH v2 0/3] powerpc: HW filtering of breakpoint for user/kernel/hypervisor events Michael Neuling ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Michael Neuling @ 2012-09-07 5:43 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: linuxppc-dev Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote: > Hi Mikey, > > On Fri, 7 Sep 2012, Michael Neuling wrote: > > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Thu, Sep 6, 2012 at 7:17 AM, Michael Neuling <mikey@neuling.org> wrote: > > > > Rework set_dabr to take a DABRX value as well. We are not actually > > > > changing any functionality at this stage, just preparing for that. > > > > > > You are changing functionality. > > > > You are right.. I'll fix that up.. Sorry. > > > > > > #define DABRX_USER (1UL << 0) > > > > #define DABRX_KERNEL (1UL << 1) > > > > +#define DABRX_HYP (1UL << 2) > > > > +#define DABRX_BTI (1UL << 3) > > > > +#define DABRX_ALL (DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER) > > > > > > > --- a/arch/powerpc/platforms/cell/beat.c > > > > +++ b/arch/powerpc/platforms/cell/beat.c > > > > @@ -136,9 +136,9 @@ ssize_t beat_nvram_get_size(void) > > > > return BEAT_NVRAM_SIZE; > > > > } > > > > > > > > -int beat_set_xdabr(unsigned long dabr) > > > > +int beat_set_xdabr(unsigned long dabr, unsigned long dabrx) > > > > { > > > > - if (beat_set_dabr(dabr, DABRX_KERNEL | DABRX_USER)) > > > > + if (beat_set_dabr(dabr, dabrx)) > > > > return -1; > > > > return 0; > > > > } > > > > > > > --- a/arch/powerpc/platforms/ps3/setup.c > > > > +++ b/arch/powerpc/platforms/ps3/setup.c > > > > @@ -184,11 +184,9 @@ early_param("ps3flash", early_parse_ps3flash); > > > > #define prealloc_ps3flash_bounce_buffer() do { } while (0) > > > > #endif > > > > > > > > -static int ps3_set_dabr(unsigned long dabr) > > > > +static int ps3_set_dabr(unsigned long dabr, unsigned long dabrx) > > > > { > > > > - enum {DABR_USER = 1, DABR_KERNEL = 2,}; > > > > - > > > > - return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0; > > > > + return lv1_set_dabr(dabr, dabrx) ? -1 : 0; > > > > } > > > > > > > - set_dabr(dabr.address | (dabr.enabled & 7)); > > > > + set_dabr(dabr.address | (dabr.enabled & 7), DABRX_ALL); > > > > > > Before, beat_set_dabr() and lv1_set_dabr() would have been called with dabrx = 3 > > > (DABRX_KERNEL | DABRX_USER). Now they're called with dabrx = 15 > > > (DABRX_ALL = DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER). > > > > > > No idea what's the impact of this... > > > > Do you know if the ps3 hypervisor will allow us to set DABRX_BTI or > > DABRX_HYP? phyp wont. > > According to the documenation, all bits but DABRX_USER, DABRX_KERNEL, and > DABRX_BTI must be zero. This implies DABRX_HYP cannot be set. > > BTW, the requirement that DABRX_USER and DABRX_KERNEL cannot both be zero > at the same time is also there, cfr. your comment and check in > pseries_set_xdabr(). > > Unfortunately, I cannot test it. OK thanks, I'll mask appropriately. Any place we can get a copy of the PS3 HV doc you're quoting from? Mikey ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-07 5:43 ` Michael Neuling @ 2012-09-10 12:27 ` Geert Uytterhoeven 0 siblings, 0 replies; 15+ messages in thread From: Geert Uytterhoeven @ 2012-09-10 12:27 UTC (permalink / raw) To: Michael Neuling; +Cc: linuxppc-dev Hi Mikey, On Fri, 7 Sep 2012, Michael Neuling wrote: > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote: > > On Fri, 7 Sep 2012, Michael Neuling wrote: > > > Do you know if the ps3 hypervisor will allow us to set DABRX_BTI or > > > DABRX_HYP? phyp wont. =20 > >=20 > > According to the documenation, all bits but DABRX_USER, DABRX_KERNEL,= and > > DABRX_BTI must be zero. This implies DABRX_HYP cannot be set. > >=20 > > BTW, the requirement that DABRX_USER and DABRX_KERNEL cannot both be = zero > > at the same time is also there, cfr. your comment and check in > > pseries_set_xdabr(). > >=20 > > Unfortunately, I cannot test it. >=20 > OK thanks, I'll mask appropriately. >=20 > Any place we can get a copy of the PS3 HV doc you're quoting from? I'm afraid not. With kind regards, Geert Uytterhoeven Software Architect Technology and Software Centre Europe Sony Belgium, bijkantoor van Sony Europe Limited. Da Vincilaan 7-D1 =C2=B7 B-1935 Zaventem =C2=B7 Belgium Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Sony Europe Limited. A company registered in England and Wales. Registered office: The Heights, Brooklands, Weybridge, Surrey. KT13 0XW. United Kingdom ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 0/3] powerpc: HW filtering of breakpoint for user/kernel/hypervisor events 2012-09-07 5:26 ` Geert Uytterhoeven 2012-09-07 5:43 ` Michael Neuling @ 2012-09-07 7:24 ` Michael Neuling 2012-09-07 7:24 ` [PATCH 1/3] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling ` (2 subsequent siblings) 4 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-07 7:24 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Geert Uytterhoeven, Michael Neuling, linuxppc-dev This is in response to Geerts concerns. Only posting last two patches again as first 3 are already in benh's next tree. Added another patch to cleanup some #defines that we can now remove Michael Neuling (3): powerpc: Rework set_dabr so it can take a DABRX value as well powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor powerpc: cleanup old DABRX #defines arch/powerpc/include/asm/debug.h | 2 +- arch/powerpc/include/asm/hvcall.h | 5 ----- arch/powerpc/include/asm/hw_breakpoint.h | 3 ++- arch/powerpc/include/asm/machdep.h | 3 ++- arch/powerpc/include/asm/processor.h | 1 + arch/powerpc/include/asm/reg.h | 3 +++ arch/powerpc/kernel/hw_breakpoint.c | 19 +++++++++++++------ arch/powerpc/kernel/process.c | 14 +++++++------- arch/powerpc/kernel/ptrace.c | 3 +++ arch/powerpc/kernel/signal.c | 2 +- arch/powerpc/platforms/cell/beat.c | 4 ++-- arch/powerpc/platforms/cell/beat.h | 2 +- arch/powerpc/platforms/ps3/setup.c | 10 +++++++--- arch/powerpc/platforms/pseries/setup.c | 14 +++++++++----- arch/powerpc/xmon/xmon.c | 4 ++-- 15 files changed, 54 insertions(+), 35 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] powerpc: Rework set_dabr so it can take a DABRX value as well 2012-09-07 5:26 ` Geert Uytterhoeven 2012-09-07 5:43 ` Michael Neuling 2012-09-07 7:24 ` [PATCH v2 0/3] powerpc: HW filtering of breakpoint for user/kernel/hypervisor events Michael Neuling @ 2012-09-07 7:24 ` Michael Neuling 2012-09-07 7:24 ` [PATCH 2/3] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling 2012-09-07 7:24 ` [PATCH 3/3] powerpc: cleanup old DABRX #defines Michael Neuling 4 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-07 7:24 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Geert Uytterhoeven, Michael Neuling, linuxppc-dev Rework set_dabr to take a DABRX value as well. Both the pseries and PS3 hypervisors do some checks on the DABRX values that are passed in the hcall. This patch stops bogus values from being passed to hypervisor. Also, in the case where we are clearing the breakpoint, where DABR and DABRX are zero, we modify the DABRX value to make it valid so that the hcall won't fail. Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/include/asm/debug.h | 2 +- arch/powerpc/include/asm/hw_breakpoint.h | 2 +- arch/powerpc/include/asm/machdep.h | 3 ++- arch/powerpc/include/asm/processor.h | 1 + arch/powerpc/include/asm/reg.h | 3 +++ arch/powerpc/kernel/hw_breakpoint.c | 12 ++++++------ arch/powerpc/kernel/process.c | 14 +++++++------- arch/powerpc/kernel/ptrace.c | 3 +++ arch/powerpc/kernel/signal.c | 2 +- arch/powerpc/platforms/cell/beat.c | 4 ++-- arch/powerpc/platforms/cell/beat.h | 2 +- arch/powerpc/platforms/ps3/setup.c | 10 +++++++--- arch/powerpc/platforms/pseries/setup.c | 14 +++++++++----- arch/powerpc/xmon/xmon.c | 4 ++-- 14 files changed, 46 insertions(+), 30 deletions(-) diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h index 716d2f0..32de257 100644 --- a/arch/powerpc/include/asm/debug.h +++ b/arch/powerpc/include/asm/debug.h @@ -44,7 +44,7 @@ static inline int debugger_dabr_match(struct pt_regs *regs) { return 0; } static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; } #endif -extern int set_dabr(unsigned long dabr); +extern int set_dabr(unsigned long dabr, unsigned long dabrx); #ifdef CONFIG_PPC_ADV_DEBUG_REGS extern void do_send_trap(struct pt_regs *regs, unsigned long address, unsigned long error_code, int signal_code, int brkpt); diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index 39b323e..c6f48eb 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -61,7 +61,7 @@ extern void ptrace_triggered(struct perf_event *bp, struct perf_sample_data *data, struct pt_regs *regs); static inline void hw_breakpoint_disable(void) { - set_dabr(0); + set_dabr(0, 0); } extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs); diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 42ce570..236b477 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -180,7 +180,8 @@ struct machdep_calls { void (*enable_pmcs)(void); /* Set DABR for this platform, leave empty for default implemenation */ - int (*set_dabr)(unsigned long dabr); + int (*set_dabr)(unsigned long dabr, + unsigned long dabrx); #ifdef CONFIG_PPC32 /* XXX for now */ /* A general init function, called by ppc_init in init/main.c. diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index 54b73a2..17b58e5 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -219,6 +219,7 @@ struct thread_struct { #endif /* CONFIG_HAVE_HW_BREAKPOINT */ #endif unsigned long dabr; /* Data address breakpoint register */ + unsigned long dabrx; /* ... extension */ #ifdef CONFIG_ALTIVEC /* Complete AltiVec register set */ vector128 vr[32] __attribute__((aligned(16))); diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 6386086..334be34 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -208,6 +208,9 @@ #define SPRN_DABRX 0x3F7 /* Data Address Breakpoint Register Extension */ #define DABRX_USER (1UL << 0) #define DABRX_KERNEL (1UL << 1) +#define DABRX_HYP (1UL << 2) +#define DABRX_BTI (1UL << 3) +#define DABRX_ALL (DABRX_BTI | DABRX_HYP | DABRX_KERNEL | DABRX_USER) #define SPRN_DAR 0x013 /* Data Address Register */ #define SPRN_DBCR 0x136 /* e300 Data Breakpoint Control Reg */ #define SPRN_DSISR 0x012 /* Data Storage Interrupt Status Register */ diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 6767445..6891d79 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -73,7 +73,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp) * If so, DABR will be populated in single_step_dabr_instruction(). */ if (current->thread.last_hit_ubp != bp) - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); return 0; } @@ -97,7 +97,7 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp) } *slot = NULL; - set_dabr(0); + set_dabr(0, 0); } /* @@ -197,7 +197,7 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs) info = counter_arch_bp(tsk->thread.last_hit_ubp); regs->msr &= ~MSR_SE; - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); tsk->thread.last_hit_ubp = NULL; } @@ -215,7 +215,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args) unsigned long dar = regs->dar; /* Disable breakpoints during exception handling */ - set_dabr(0); + set_dabr(0, 0); /* * The counter may be concurrently released but that can only @@ -281,7 +281,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); out: rcu_read_unlock(); return rc; @@ -313,7 +313,7 @@ int __kprobes single_step_dabr_instruction(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION); + set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); current->thread.last_hit_ubp = NULL; /* diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 1a1f2dd..53c32a9 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -283,7 +283,7 @@ void do_dabr(struct pt_regs *regs, unsigned long address, return; /* Clear the DABR */ - set_dabr(0); + set_dabr(0, 0); /* Deliver the signal to userspace */ info.si_signo = SIGTRAP; @@ -364,18 +364,19 @@ static void set_debug_reg_defaults(struct thread_struct *thread) { if (thread->dabr) { thread->dabr = 0; - set_dabr(0); + thread->dabrx = 0; + set_dabr(0, 0); } } #endif /* !CONFIG_HAVE_HW_BREAKPOINT */ #endif /* CONFIG_PPC_ADV_DEBUG_REGS */ -int set_dabr(unsigned long dabr) +int set_dabr(unsigned long dabr, unsigned long dabrx) { __get_cpu_var(current_dabr) = dabr; if (ppc_md.set_dabr) - return ppc_md.set_dabr(dabr); + return ppc_md.set_dabr(dabr, dabrx); /* XXX should we have a CPU_FTR_HAS_DABR ? */ #ifdef CONFIG_PPC_ADV_DEBUG_REGS @@ -385,9 +386,8 @@ int set_dabr(unsigned long dabr) #endif #elif defined(CONFIG_PPC_BOOK3S) mtspr(SPRN_DABR, dabr); + mtspr(SPRN_DABRX, dabrx); #endif - - return 0; } @@ -480,7 +480,7 @@ struct task_struct *__switch_to(struct task_struct *prev, */ #ifndef CONFIG_HAVE_HW_BREAKPOINT if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr)) - set_dabr(new->thread.dabr); + set_dabr(new->thread.dabr, new->thread.dabrx); #endif /* CONFIG_HAVE_HW_BREAKPOINT */ #endif diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index c10fc28..79d8e56 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -960,6 +960,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, thread->ptrace_bps[0] = bp; ptrace_put_breakpoints(task); thread->dabr = data; + thread->dabrx = DABRX_ALL; return 0; } @@ -983,6 +984,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, /* Move contents to the DABR register */ task->thread.dabr = data; + task->thread.dabrx = DABRX_ALL; #else /* CONFIG_PPC_ADV_DEBUG_REGS */ /* As described above, it was assumed 3 bits were passed with the data * address, but we will assume only the mode bits will be passed @@ -1397,6 +1399,7 @@ static long ppc_set_hwdebug(struct task_struct *child, dabr |= DABR_DATA_WRITE; child->thread.dabr = dabr; + child->thread.dabrx = DABRX_ALL; return 1; #endif /* !CONFIG_PPC_ADV_DEBUG_DVCS */ diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c index 5c023c9..b936b45 100644 --- a/arch/powerpc/kernel/signal.c +++ b/arch/powerpc/kernel/signal.c @@ -130,7 +130,7 @@ static int do_signal(struct pt_regs *regs) * triggered inside the kernel. */ if (current->thread.dabr) - set_dabr(current->thread.dabr); + set_dabr(current->thread.dabr, current->thread.dabrx); #endif /* Re-enable the breakpoints for the signal stack */ thread_change_pc(current, regs); diff --git a/arch/powerpc/platforms/cell/beat.c b/arch/powerpc/platforms/cell/beat.c index 852592b..affcf56 100644 --- a/arch/powerpc/platforms/cell/beat.c +++ b/arch/powerpc/platforms/cell/beat.c @@ -136,9 +136,9 @@ ssize_t beat_nvram_get_size(void) return BEAT_NVRAM_SIZE; } -int beat_set_xdabr(unsigned long dabr) +int beat_set_xdabr(unsigned long dabr, unsigned long dabrx) { - if (beat_set_dabr(dabr, DABRX_KERNEL | DABRX_USER)) + if (beat_set_dabr(dabr, dabrx)) return -1; return 0; } diff --git a/arch/powerpc/platforms/cell/beat.h b/arch/powerpc/platforms/cell/beat.h index 32c8efc..bfcb8e3 100644 --- a/arch/powerpc/platforms/cell/beat.h +++ b/arch/powerpc/platforms/cell/beat.h @@ -32,7 +32,7 @@ void beat_get_rtc_time(struct rtc_time *); ssize_t beat_nvram_get_size(void); ssize_t beat_nvram_read(char *, size_t, loff_t *); ssize_t beat_nvram_write(char *, size_t, loff_t *); -int beat_set_xdabr(unsigned long); +int beat_set_xdabr(unsigned long, unsigned long); void beat_power_save(void); void beat_kexec_cpu_down(int, int); diff --git a/arch/powerpc/platforms/ps3/setup.c b/arch/powerpc/platforms/ps3/setup.c index 2d664c5..3f509f8 100644 --- a/arch/powerpc/platforms/ps3/setup.c +++ b/arch/powerpc/platforms/ps3/setup.c @@ -184,11 +184,15 @@ early_param("ps3flash", early_parse_ps3flash); #define prealloc_ps3flash_bounce_buffer() do { } while (0) #endif -static int ps3_set_dabr(unsigned long dabr) +static int ps3_set_dabr(unsigned long dabr, unsigned long dabrx) { - enum {DABR_USER = 1, DABR_KERNEL = 2,}; + /* Have to set at least one bit in the DABRX */ + if (dabrx == 0 && dabr == 0) + dabrx = DABRX_USER; + /* hypervisor only allows us to set BTI, Kernel and user */ + dabrx &= DABRX_BTI | DABRX_KERNEL | DABRX_USER; - return lv1_set_dabr(dabr, DABR_KERNEL | DABR_USER) ? -1 : 0; + return lv1_set_dabr(dabr, dabrx) ? -1 : 0; } static void __init ps3_setup_arch(void) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 36b7744..b90deaf 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -416,16 +416,20 @@ static int __init pSeries_init_panel(void) } machine_arch_initcall(pseries, pSeries_init_panel); -static int pseries_set_dabr(unsigned long dabr) +static int pseries_set_dabr(unsigned long dabr, unsigned long dabrx) { return plpar_hcall_norets(H_SET_DABR, dabr); } -static int pseries_set_xdabr(unsigned long dabr) +static int pseries_set_xdabr(unsigned long dabr, unsigned long dabrx) { - /* We want to catch accesses from kernel and userspace */ - return plpar_hcall_norets(H_SET_XDABR, dabr, - H_DABRX_KERNEL | H_DABRX_USER); + /* Have to set at least one bit in the DABRX according to PAPR */ + if (dabrx == 0 && dabr == 0) + dabrx = DABRX_USER; + /* PAPR says we can only set kernel and user bits */ + dabrx &= H_DABRX_KERNEL | H_DABRX_USER; + + return plpar_hcall_norets(H_SET_XDABR, dabr, dabrx); } #define CMO_CHARACTERISTICS_TOKEN 44 diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 9b49c65..987f441 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -740,7 +740,7 @@ static void insert_bpts(void) static void insert_cpu_bpts(void) { if (dabr.enabled) - set_dabr(dabr.address | (dabr.enabled & 7)); + set_dabr(dabr.address | (dabr.enabled & 7), DABRX_ALL); if (iabr && cpu_has_feature(CPU_FTR_IABR)) mtspr(SPRN_IABR, iabr->address | (iabr->enabled & (BP_IABR|BP_IABR_TE))); @@ -768,7 +768,7 @@ static void remove_bpts(void) static void remove_cpu_bpts(void) { - set_dabr(0); + set_dabr(0, 0); if (cpu_has_feature(CPU_FTR_IABR)) mtspr(SPRN_IABR, 0); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor 2012-09-07 5:26 ` Geert Uytterhoeven ` (2 preceding siblings ...) 2012-09-07 7:24 ` [PATCH 1/3] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling @ 2012-09-07 7:24 ` Michael Neuling 2012-09-07 7:24 ` [PATCH 3/3] powerpc: cleanup old DABRX #defines Michael Neuling 4 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-07 7:24 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Geert Uytterhoeven, Michael Neuling, linuxppc-dev Currently we mark the DABRX to interrupt on all matches (hypervisor/kernel/user and then filter in software. We can be a lot smarter now that we can set the DABRX dynamically. This sets the DABRX based on the flags passed by the user. Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/include/asm/hw_breakpoint.h | 1 + arch/powerpc/kernel/hw_breakpoint.c | 15 +++++++++++---- arch/powerpc/platforms/pseries/setup.c | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index c6f48eb..4234245 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -28,6 +28,7 @@ struct arch_hw_breakpoint { unsigned long address; + unsigned long dabrx; int type; u8 len; /* length of the target data symbol */ bool extraneous_interrupt; diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 6891d79..a89cae4 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -73,7 +73,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp) * If so, DABR will be populated in single_step_dabr_instruction(). */ if (current->thread.last_hit_ubp != bp) - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); return 0; } @@ -170,6 +170,13 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp) info->address = bp->attr.bp_addr; info->len = bp->attr.bp_len; + info->dabrx = DABRX_ALL; + if (bp->attr.exclude_user) + info->dabrx &= ~DABRX_USER; + if (bp->attr.exclude_kernel) + info->dabrx &= ~DABRX_KERNEL; + if (bp->attr.exclude_hv) + info->dabrx &= ~DABRX_HYP; /* * Since breakpoint length can be a maximum of HW_BREAKPOINT_LEN(8) @@ -197,7 +204,7 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs) info = counter_arch_bp(tsk->thread.last_hit_ubp); regs->msr &= ~MSR_SE; - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); tsk->thread.last_hit_ubp = NULL; } @@ -281,7 +288,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); out: rcu_read_unlock(); return rc; @@ -313,7 +320,7 @@ int __kprobes single_step_dabr_instruction(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); current->thread.last_hit_ubp = NULL; /* diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index b90deaf..40b30e4 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -427,7 +427,7 @@ static int pseries_set_xdabr(unsigned long dabr, unsigned long dabrx) if (dabrx == 0 && dabr == 0) dabrx = DABRX_USER; /* PAPR says we can only set kernel and user bits */ - dabrx &= H_DABRX_KERNEL | H_DABRX_USER; + dabrx &= DABRX_KERNEL | DABRX_USER; return plpar_hcall_norets(H_SET_XDABR, dabr, dabrx); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] powerpc: cleanup old DABRX #defines 2012-09-07 5:26 ` Geert Uytterhoeven ` (3 preceding siblings ...) 2012-09-07 7:24 ` [PATCH 2/3] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling @ 2012-09-07 7:24 ` Michael Neuling 4 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-07 7:24 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Geert Uytterhoeven, Michael Neuling, linuxppc-dev These are no longer used so get rid of them Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/include/asm/hvcall.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index 423cf9e..7a86706 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -152,11 +152,6 @@ #define H_VASI_RESUMED 5 #define H_VASI_COMPLETED 6 -/* DABRX flags */ -#define H_DABRX_HYPERVISOR (1UL<<(63-61)) -#define H_DABRX_KERNEL (1UL<<(63-62)) -#define H_DABRX_USER (1UL<<(63-63)) - /* Each control block has to be on a 4K boundary */ #define H_CB_ALIGNMENT 4096 -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor 2012-09-06 5:17 [PATCH 1/5] powerpc: Pack arch_hw_breakpoint to avoid holes in struct Michael Neuling ` (2 preceding siblings ...) 2012-09-06 5:17 ` [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling @ 2012-09-06 5:17 ` Michael Neuling 2012-09-06 5:31 ` Michael Neuling 3 siblings, 1 reply; 15+ messages in thread From: Michael Neuling @ 2012-09-06 5:17 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Michael Neuling, linuxppc-dev Currently we mark the DABRX to interrupt on all matches (hypervisor/kernel/user and then filter in software. We can be a lot smarter now that we can set the DABRX dynamically. This sets the DABRX based on the flags passed by the user. Signed-off-by: Michael Neuling <mikey@neuling.org> --- arch/powerpc/include/asm/hw_breakpoint.h | 1 + arch/powerpc/kernel/hw_breakpoint.c | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index c6f48eb..4234245 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -28,6 +28,7 @@ struct arch_hw_breakpoint { unsigned long address; + unsigned long dabrx; int type; u8 len; /* length of the target data symbol */ bool extraneous_interrupt; diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 6891d79..a89cae4 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -73,7 +73,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp) * If so, DABR will be populated in single_step_dabr_instruction(). */ if (current->thread.last_hit_ubp != bp) - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); return 0; } @@ -170,6 +170,13 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp) info->address = bp->attr.bp_addr; info->len = bp->attr.bp_len; + info->dabrx = DABRX_ALL; + if (bp->attr.exclude_user) + info->dabrx &= ~DABRX_USER; + if (bp->attr.exclude_kernel) + info->dabrx &= ~DABRX_KERNEL; + if (bp->attr.exclude_hv) + info->dabrx &= ~DABRX_HYP; /* * Since breakpoint length can be a maximum of HW_BREAKPOINT_LEN(8) @@ -197,7 +204,7 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs) info = counter_arch_bp(tsk->thread.last_hit_ubp); regs->msr &= ~MSR_SE; - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); tsk->thread.last_hit_ubp = NULL; } @@ -281,7 +288,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); out: rcu_read_unlock(); return rc; @@ -313,7 +320,7 @@ int __kprobes single_step_dabr_instruction(struct die_args *args) if (!info->extraneous_interrupt) perf_bp_event(bp, regs); - set_dabr(info->address | info->type | DABR_TRANSLATION, DABRX_ALL); + set_dabr(info->address | info->type | DABR_TRANSLATION, info->dabrx); current->thread.last_hit_ubp = NULL; /* -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor 2012-09-06 5:17 ` [PATCH 5/5] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling @ 2012-09-06 5:31 ` Michael Neuling 0 siblings, 0 replies; 15+ messages in thread From: Michael Neuling @ 2012-09-06 5:31 UTC (permalink / raw) To: Benjamin Herrenschmidt, linuxppc-dev Michael Neuling <mikey@neuling.org> wrote: > Currently we mark the DABRX to interrupt on all matches > (hypervisor/kernel/user and then filter in software. We can be a lot > smarter now that we can set the DABRX dynamically. > > This sets the DABRX based on the flags passed by the user. > > Signed-off-by: Michael Neuling <mikey@neuling.org> For what it's worth, I have this test case now: http://neuling.org/devel/junkcode/hw_brk_test.c Mikey ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-09-10 12:27 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-06 5:17 [PATCH 1/5] powerpc: Pack arch_hw_breakpoint to avoid holes in struct Michael Neuling 2012-09-06 5:17 ` [PATCH 2/5] powerpc: Use consistent name info for arch_hw_breakpoint Michael Neuling 2012-09-06 5:17 ` [PATCH 3/5] powerpc: Use the XDABR hcall Michael Neuling 2012-09-06 5:17 ` [PATCH 4/5] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling 2012-09-06 5:35 ` Geert Uytterhoeven 2012-09-07 3:37 ` Michael Neuling 2012-09-07 5:26 ` Geert Uytterhoeven 2012-09-07 5:43 ` Michael Neuling 2012-09-10 12:27 ` Geert Uytterhoeven 2012-09-07 7:24 ` [PATCH v2 0/3] powerpc: HW filtering of breakpoint for user/kernel/hypervisor events Michael Neuling 2012-09-07 7:24 ` [PATCH 1/3] powerpc: Rework set_dabr so it can take a DABRX value as well Michael Neuling 2012-09-07 7:24 ` [PATCH 2/3] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling 2012-09-07 7:24 ` [PATCH 3/3] powerpc: cleanup old DABRX #defines Michael Neuling 2012-09-06 5:17 ` [PATCH 5/5] powerpc: Dynamically calculate the dabrx based on kernel/user/hypervisor Michael Neuling 2012-09-06 5:31 ` Michael Neuling
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).