| Message ID | 20200309085806.155823-4-ravi.bangoria@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc/watchpoint: Preparation for more than one watchpoint | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (ab326587bb5fb91cc97df9b9f48e9e1469f04621) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 48 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit : > So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1. > But future Power architecture is introducing 2nd DAWR and thus kernel > should be able to dynamically find actual number of watchpoints > supported by hw it's running on. Introduce function for the same. > Also convert HBP_NUM macro to HBP_NUM_MAX, which will now represent > maximum number of watchpoints supported by Powerpc. > > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> > --- > arch/powerpc/include/asm/cputable.h | 6 +++++- > arch/powerpc/include/asm/hw_breakpoint.h | 2 ++ > arch/powerpc/include/asm/processor.h | 2 +- > arch/powerpc/kernel/hw_breakpoint.c | 2 +- > arch/powerpc/kernel/process.c | 6 ++++++ > 5 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h > index 40a4d3c6fd99..c67b94f3334c 100644 > --- a/arch/powerpc/include/asm/cputable.h > +++ b/arch/powerpc/include/asm/cputable.h > @@ -614,7 +614,11 @@ enum { > }; > #endif /* __powerpc64__ */ > > -#define HBP_NUM 1 > +/* > + * Maximum number of hw breakpoint supported on powerpc. Number of > + * breakpoints supported by actual hw might be less than this. > + */ > +#define HBP_NUM_MAX 1 > > #endif /* !__ASSEMBLY__ */ > > diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h > index f2f8d8aa8e3b..741c4f7573c4 100644 > --- a/arch/powerpc/include/asm/hw_breakpoint.h > +++ b/arch/powerpc/include/asm/hw_breakpoint.h > @@ -43,6 +43,8 @@ struct arch_hw_breakpoint { > #define DABR_MAX_LEN 8 > #define DAWR_MAX_LEN 512 > > +extern int nr_wp_slots(void); 'extern' keyword is unneeded and irrelevant here. Please remove it. Even checkpatch is unhappy (https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12172//artifact/linux/checkpatch.log) > + > #ifdef CONFIG_HAVE_HW_BREAKPOINT > #include <linux/kdebug.h> > #include <asm/reg.h> > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h > index 8387698bd5b6..666b2825278c 100644 > --- a/arch/powerpc/include/asm/processor.h > +++ b/arch/powerpc/include/asm/processor.h > @@ -176,7 +176,7 @@ struct thread_struct { > int fpexc_mode; /* floating-point exception mode */ > unsigned int align_ctl; /* alignment handling control */ > #ifdef CONFIG_HAVE_HW_BREAKPOINT > - struct perf_event *ptrace_bps[HBP_NUM]; > + struct perf_event *ptrace_bps[HBP_NUM_MAX]; > /* > * Helps identify source of single-step exception and subsequent > * hw-breakpoint enablement > diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c > index d0854320bb50..e68798cee3fa 100644 > --- a/arch/powerpc/kernel/hw_breakpoint.c > +++ b/arch/powerpc/kernel/hw_breakpoint.c > @@ -38,7 +38,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg); > int hw_breakpoint_slots(int type) > { > if (type == TYPE_DATA) > - return HBP_NUM; > + return nr_wp_slots(); > return 0; /* no instruction breakpoints available */ > } > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index 110db94cdf3c..6d4b029532e2 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -835,6 +835,12 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a, > return true; > } > > +/* Returns total number of data breakpoints available. */ > +int nr_wp_slots(void) > +{ > + return HBP_NUM_MAX; > +} > + This is not worth a global function. At least it should be a static function located in hw_breakpoint.c. But it would be even better to have it as a static inline in asm/hw_breakpoint.h Christophe
>> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h >> index f2f8d8aa8e3b..741c4f7573c4 100644 >> --- a/arch/powerpc/include/asm/hw_breakpoint.h >> +++ b/arch/powerpc/include/asm/hw_breakpoint.h >> @@ -43,6 +43,8 @@ struct arch_hw_breakpoint { >> #define DABR_MAX_LEN 8 >> #define DAWR_MAX_LEN 512 >> +extern int nr_wp_slots(void); > > 'extern' keyword is unneeded and irrelevant here. Please remove it. Even checkpatch is unhappy (https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12172//artifact/linux/checkpatch.log) Sure. ... >> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c >> index 110db94cdf3c..6d4b029532e2 100644 >> --- a/arch/powerpc/kernel/process.c >> +++ b/arch/powerpc/kernel/process.c >> @@ -835,6 +835,12 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a, >> return true; >> } >> +/* Returns total number of data breakpoints available. */ >> +int nr_wp_slots(void) >> +{ >> + return HBP_NUM_MAX; >> +} >> + > > This is not worth a global function. At least it should be a static function located in hw_breakpoint.c. But it would be even better to have it as a static inline in asm/hw_breakpoint.h Makes sense. Will change it. Thanks.
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h index 40a4d3c6fd99..c67b94f3334c 100644 --- a/arch/powerpc/include/asm/cputable.h +++ b/arch/powerpc/include/asm/cputable.h @@ -614,7 +614,11 @@ enum { }; #endif /* __powerpc64__ */ -#define HBP_NUM 1 +/* + * Maximum number of hw breakpoint supported on powerpc. Number of + * breakpoints supported by actual hw might be less than this. + */ +#define HBP_NUM_MAX 1 #endif /* !__ASSEMBLY__ */ diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index f2f8d8aa8e3b..741c4f7573c4 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -43,6 +43,8 @@ struct arch_hw_breakpoint { #define DABR_MAX_LEN 8 #define DAWR_MAX_LEN 512 +extern int nr_wp_slots(void); + #ifdef CONFIG_HAVE_HW_BREAKPOINT #include <linux/kdebug.h> #include <asm/reg.h> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index 8387698bd5b6..666b2825278c 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -176,7 +176,7 @@ struct thread_struct { int fpexc_mode; /* floating-point exception mode */ unsigned int align_ctl; /* alignment handling control */ #ifdef CONFIG_HAVE_HW_BREAKPOINT - struct perf_event *ptrace_bps[HBP_NUM]; + struct perf_event *ptrace_bps[HBP_NUM_MAX]; /* * Helps identify source of single-step exception and subsequent * hw-breakpoint enablement diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index d0854320bb50..e68798cee3fa 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -38,7 +38,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg); int hw_breakpoint_slots(int type) { if (type == TYPE_DATA) - return HBP_NUM; + return nr_wp_slots(); return 0; /* no instruction breakpoints available */ } diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 110db94cdf3c..6d4b029532e2 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -835,6 +835,12 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a, return true; } +/* Returns total number of data breakpoints available. */ +int nr_wp_slots(void) +{ + return HBP_NUM_MAX; +} + #ifdef CONFIG_PPC_TRANSACTIONAL_MEM static inline bool tm_enabled(struct task_struct *tsk)
So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1. But future Power architecture is introducing 2nd DAWR and thus kernel should be able to dynamically find actual number of watchpoints supported by hw it's running on. Introduce function for the same. Also convert HBP_NUM macro to HBP_NUM_MAX, which will now represent maximum number of watchpoints supported by Powerpc. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/cputable.h | 6 +++++- arch/powerpc/include/asm/hw_breakpoint.h | 2 ++ arch/powerpc/include/asm/processor.h | 2 +- arch/powerpc/kernel/hw_breakpoint.c | 2 +- arch/powerpc/kernel/process.c | 6 ++++++ 5 files changed, 15 insertions(+), 3 deletions(-)