> > > > -- > > > > v2: > > > > Fixes based on Christophe Leroy's comments: > > > > - Fix commit message formatting > > > > - Move more DAWR code into dawr.c > > > > --- > > > > arch/powerpc/Kconfig | 5 ++ > > > > arch/powerpc/include/asm/hw_breakpoint.h | 20 ++++--- > > > > arch/powerpc/kernel/Makefile | 1 + > > > > arch/powerpc/kernel/dawr.c | 75 > > > > ++++++++++++++++++++++++ > > > > arch/powerpc/kernel/hw_breakpoint.c | 56 ------------------ > > > > arch/powerpc/kvm/Kconfig | 1 + > > > > 6 files changed, 94 insertions(+), 64 deletions(-) > > > > create mode 100644 arch/powerpc/kernel/dawr.c > > > > > > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > > > > index 2711aac246..f4b19e48cc 100644 > > > > --- a/arch/powerpc/Kconfig > > > > +++ b/arch/powerpc/Kconfig > > > > @@ -242,6 +242,7 @@ config PPC > > > > select SYSCTL_EXCEPTION_TRACE > > > > select THREAD_INFO_IN_TASK > > > > select VIRT_TO_BUS if !PPC64 > > > > + select PPC_DAWR_FORCE_ENABLE if PPC64 || PERF > > > > > > What's PERF ? Did you mean PERF_EVENTS ? > > > > > > Then what you mean is: > > > - Selected all the time for PPC64 > > > - Selected for PPC32 when PERF is also selected. > > > > > > Is that what you want ? At first I thought it was linked to P9. > > > > This is wrong. I think we just want PPC64. PERF is a red herring. > > Are you sure ? Michael suggested PERF || KVM as far as I remember.
It was mpe but I think it was wrong. We could make it more selective with something like: PPC64 && (HW_BREAK || PPC_ADV_DEBUG_REGS || PERF) but I think that will end up back in the larger mess of https://github.com/linuxppc/issues/issues/128 I don't think the minor size gain is is worth delving in that mess, hence I made it simply PPC64 which is hopefully an improvement on what we have since it eliminates 32bit. > > > > #else /* CONFIG_HAVE_HW_BREAKPOINT */ > > > > static inline void hw_breakpoint_disable(void) { } > > > > static inline void thread_change_pc(struct task_struct *tsk, > > > > struct pt_regs *regs) { } > > > > -static inline bool dawr_enabled(void) { return false; } > > > > + > > > > #endif /* CONFIG_HAVE_HW_BREAKPOINT */ > > > > + > > > > +extern bool dawr_force_enable; > > > > + > > > > +#ifdef CONFIG_PPC_DAWR_FORCE_ENABLE > > > > +extern bool dawr_enabled(void); > > > > > > Functions should not be 'extern'. I'm sure checkpatch --strict will tell > > > you. > > > > That's not what's currently being done in this header file. I'm keeping > > with > > the style of that file. > > style is not defined on a per file basis. There is the style from the > past and the nowadays style. If you keep old style just because the file > includes old style statements, then the code will never improve. > > All new patches should come with clean 'checkpatch' report and follow > new style. Having mixed styles in a file is not a problem, that's the > way to improvement. See arch/powerpc/mm/mmu_decl.h as an exemple. I'm not sure that's mpe's policy. mpe? > > > > + > > > > +static ssize_t dawr_write_file_bool(struct file *file, > > > > + const char __user *user_buf, > > > > + size_t count, loff_t *ppos) > > > > +{ > > > > + struct arch_hw_breakpoint null_brk = {0, 0, 0}; > > > > + size_t rc; > > > > + > > > > + /* Send error to user if they hypervisor won't allow us to write > > > > DAWR */ > > > > + if ((!dawr_force_enable) && > > > > + (firmware_has_feature(FW_FEATURE_LPAR)) && > > > > + (set_dawr(&null_brk) != H_SUCCESS)) > > > > > > The above is not real clear. > > > set_dabr() returns 0, H_SUCCESS is not used there. > > > > It pseries_set_dawr() will return a hcall number. > > Right, then it maybe means set_dawr() should be fixes ? Sorry, I don't understand this. > > This code hasn't changed. I'm just moving it. > > Right, but could be an improvment for another patch. > As far as I remember you are the one who wrote that code at first place, > arent't you ? Yep, classic crap Mikey code :-) Mikey