| Message ID | 1390401773-12100-2-git-send-email-vijay.kilari@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On Wed, Jan 22, 2014 at 02:42:48PM +0000, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > > Add macros to enable and disable to manage PSTATE.D > for debugging. The macros local_dbg_save and local_dbg_restore > are moved to irqflags.h file > > KGDB boot tests fail because of PSTATE.D is masked. > unmask it for debugging support > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > --- > arch/arm64/include/asm/debug-monitors.h | 18 ++---------------- > arch/arm64/include/asm/irqflags.h | 22 ++++++++++++++++++++++ > arch/arm64/kernel/debug-monitors.c | 3 ++- > 3 files changed, 26 insertions(+), 17 deletions(-) > > diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h > index 6231479..bc48880 100644 > --- a/arch/arm64/include/asm/debug-monitors.h > +++ b/arch/arm64/include/asm/debug-monitors.h > @@ -43,22 +43,8 @@ enum debug_el { > #ifndef __ASSEMBLY__ > struct task_struct; > > -#define local_dbg_save(flags) \ > - do { \ > - typecheck(unsigned long, flags); \ > - asm volatile( \ > - "mrs %0, daif // local_dbg_save\n" \ > - "msr daifset, #8" \ > - : "=r" (flags) : : "memory"); \ > - } while (0) > - > -#define local_dbg_restore(flags) \ > - do { \ > - typecheck(unsigned long, flags); \ > - asm volatile( \ > - "msr daif, %0 // local_dbg_restore\n" \ > - : : "r" (flags) : "memory"); \ > - } while (0) > +#define local_dbg_enable() asm("msr daifclr, #8" : : : "memory") > +#define local_dbg_disable() asm("msr daifset, #8" : : : "memory") Any reason not to move these to irqflags.h too? > #define DBG_ARCH_ID_RESERVED 0 /* In case of ptrace ABI updates. */ > > diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h > index b2fcfbc..f163b11 100644 > --- a/arch/arm64/include/asm/irqflags.h > +++ b/arch/arm64/include/asm/irqflags.h > @@ -90,5 +90,27 @@ static inline int arch_irqs_disabled_flags(unsigned long flags) > return flags & PSR_I_BIT; > } > > +/* > + * save and restore debug state > + */ > +static inline unsigned long local_dbg_save(void) > +{ > + unsigned long flags; > + asm volatile( > + "mrs %0, daif // local_dbg_save" > + "msr daifset, #8" > + : "=r" (flags) : : "memory"); > + return flags; > +} > + > +static inline void local_dbg_restore(unsigned long flags) > +{ > + asm volatile( > + "msr daif, %0 // local_dbg_restore" > + : > + : "r" (flags) > + : "memory"); > +} > + > #endif > #endif > diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c > index 23586bd..774ad04 100644 > --- a/arch/arm64/kernel/debug-monitors.c > +++ b/arch/arm64/kernel/debug-monitors.c > @@ -51,7 +51,7 @@ u8 debug_monitors_arch(void) > static void mdscr_write(u32 mdscr) > { > unsigned long flags; > - local_dbg_save(flags); > + flags = local_dbg_save(); Why are you changing the API? This is now pointlessly different to irqs. Will
On Wed, Jan 22, 2014 at 11:01 PM, Will Deacon <will.deacon@arm.com> wrote: > Hello, > > On Wed, Jan 22, 2014 at 02:42:48PM +0000, vijay.kilari@gmail.com wrote: >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >> >> Add macros to enable and disable to manage PSTATE.D >> for debugging. The macros local_dbg_save and local_dbg_restore >> are moved to irqflags.h file >> >> KGDB boot tests fail because of PSTATE.D is masked. >> unmask it for debugging support >> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >> --- >> arch/arm64/include/asm/debug-monitors.h | 18 ++---------------- >> arch/arm64/include/asm/irqflags.h | 22 ++++++++++++++++++++++ >> arch/arm64/kernel/debug-monitors.c | 3 ++- >> 3 files changed, 26 insertions(+), 17 deletions(-) >> >> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h >> index 6231479..bc48880 100644 >> --- a/arch/arm64/include/asm/debug-monitors.h >> +++ b/arch/arm64/include/asm/debug-monitors.h >> @@ -43,22 +43,8 @@ enum debug_el { >> #ifndef __ASSEMBLY__ >> struct task_struct; >> >> -#define local_dbg_save(flags) \ >> - do { \ >> - typecheck(unsigned long, flags); \ >> - asm volatile( \ >> - "mrs %0, daif // local_dbg_save\n" \ >> - "msr daifset, #8" \ >> - : "=r" (flags) : : "memory"); \ >> - } while (0) >> - >> -#define local_dbg_restore(flags) \ >> - do { \ >> - typecheck(unsigned long, flags); \ >> - asm volatile( \ >> - "msr daif, %0 // local_dbg_restore\n" \ >> - : : "r" (flags) : "memory"); \ >> - } while (0) >> +#define local_dbg_enable() asm("msr daifclr, #8" : : : "memory") >> +#define local_dbg_disable() asm("msr daifset, #8" : : : "memory") > > Any reason not to move these to irqflags.h too? Can me moved to irqflags.h file > >> #define DBG_ARCH_ID_RESERVED 0 /* In case of ptrace ABI updates. */ >> >> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h >> index b2fcfbc..f163b11 100644 >> --- a/arch/arm64/include/asm/irqflags.h >> +++ b/arch/arm64/include/asm/irqflags.h >> @@ -90,5 +90,27 @@ static inline int arch_irqs_disabled_flags(unsigned long flags) >> return flags & PSR_I_BIT; >> } >> >> +/* >> + * save and restore debug state >> + */ >> +static inline unsigned long local_dbg_save(void) >> +{ >> + unsigned long flags; >> + asm volatile( >> + "mrs %0, daif // local_dbg_save" >> + "msr daifset, #8" >> + : "=r" (flags) : : "memory"); >> + return flags; >> +} >> + >> +static inline void local_dbg_restore(unsigned long flags) >> +{ >> + asm volatile( >> + "msr daif, %0 // local_dbg_restore" >> + : >> + : "r" (flags) >> + : "memory"); >> +} >> + >> #endif >> #endif >> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c >> index 23586bd..774ad04 100644 >> --- a/arch/arm64/kernel/debug-monitors.c >> +++ b/arch/arm64/kernel/debug-monitors.c >> @@ -51,7 +51,7 @@ u8 debug_monitors_arch(void) >> static void mdscr_write(u32 mdscr) >> { >> unsigned long flags; >> - local_dbg_save(flags); >> + flags = local_dbg_save(); > > Why are you changing the API? This is now pointlessly different to irqs. To be in line with arch_local_irq_save & arch_local_save_flags in irqflags.h, I moved this macros into functions and accordingly changed the caller. I could not find any code using this local_dbg_{save, restore} except from debug-monitors.c file If required, I can think of renaming local_dbg_{save,restore} as local_dbg_{save,restore}_flags > > Will
On Thu, Jan 23, 2014 at 04:46:36AM +0000, Vijay Kilari wrote: > On Wed, Jan 22, 2014 at 11:01 PM, Will Deacon <will.deacon@arm.com> wrote: > >> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c > >> index 23586bd..774ad04 100644 > >> --- a/arch/arm64/kernel/debug-monitors.c > >> +++ b/arch/arm64/kernel/debug-monitors.c > >> @@ -51,7 +51,7 @@ u8 debug_monitors_arch(void) > >> static void mdscr_write(u32 mdscr) > >> { > >> unsigned long flags; > >> - local_dbg_save(flags); > >> + flags = local_dbg_save(); > > > > Why are you changing the API? This is now pointlessly different to irqs. > > To be in line with arch_local_irq_save & arch_local_save_flags in irqflags.h, > I moved this macros into functions and accordingly changed the caller. > I could not find any code using this local_dbg_{save, restore} except > from debug-monitors.c file > > If required, I can think of renaming local_dbg_{save,restore} as > local_dbg_{save,restore}_flags No, I'd rather local_dbg_{save, restore} were aligned with local_irq_{save, restore} (as defined in include/linux/irqflags.h). Will
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h index 6231479..bc48880 100644 --- a/arch/arm64/include/asm/debug-monitors.h +++ b/arch/arm64/include/asm/debug-monitors.h @@ -43,22 +43,8 @@ enum debug_el { #ifndef __ASSEMBLY__ struct task_struct; -#define local_dbg_save(flags) \ - do { \ - typecheck(unsigned long, flags); \ - asm volatile( \ - "mrs %0, daif // local_dbg_save\n" \ - "msr daifset, #8" \ - : "=r" (flags) : : "memory"); \ - } while (0) - -#define local_dbg_restore(flags) \ - do { \ - typecheck(unsigned long, flags); \ - asm volatile( \ - "msr daif, %0 // local_dbg_restore\n" \ - : : "r" (flags) : "memory"); \ - } while (0) +#define local_dbg_enable() asm("msr daifclr, #8" : : : "memory") +#define local_dbg_disable() asm("msr daifset, #8" : : : "memory") #define DBG_ARCH_ID_RESERVED 0 /* In case of ptrace ABI updates. */ diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h index b2fcfbc..f163b11 100644 --- a/arch/arm64/include/asm/irqflags.h +++ b/arch/arm64/include/asm/irqflags.h @@ -90,5 +90,27 @@ static inline int arch_irqs_disabled_flags(unsigned long flags) return flags & PSR_I_BIT; } +/* + * save and restore debug state + */ +static inline unsigned long local_dbg_save(void) +{ + unsigned long flags; + asm volatile( + "mrs %0, daif // local_dbg_save" + "msr daifset, #8" + : "=r" (flags) : : "memory"); + return flags; +} + +static inline void local_dbg_restore(unsigned long flags) +{ + asm volatile( + "msr daif, %0 // local_dbg_restore" + : + : "r" (flags) + : "memory"); +} + #endif #endif diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c index 23586bd..774ad04 100644 --- a/arch/arm64/kernel/debug-monitors.c +++ b/arch/arm64/kernel/debug-monitors.c @@ -51,7 +51,7 @@ u8 debug_monitors_arch(void) static void mdscr_write(u32 mdscr) { unsigned long flags; - local_dbg_save(flags); + flags = local_dbg_save(); asm volatile("msr mdscr_el1, %0" :: "r" (mdscr)); local_dbg_restore(flags); } @@ -138,6 +138,7 @@ static void clear_os_lock(void *unused) { asm volatile("msr oslar_el1, %0" : : "r" (0)); isb(); + local_dbg_enable(); } static int os_lock_notify(struct notifier_block *self,