[RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

[Joakim Tjernlund]

Scott Wood <scottwood at freescale.com> wrote on 2012/06/01 00:16:53:
>
> On 05/31/2012 05:14 PM, Joakim Tjernlund wrote:
> > Scott Wood <scottwood at freescale.com> wrote on 2012/05/31 23:43:34:
> >>
> >> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> >>> Scott Wood <scottwood at freescale.com> wrote on 2012/05/31 19:47:53:
> >>>>
> >>>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> >>>>> Abatron Support <support at abatron.ch> wrote on 2012/05/31 11:30:57:
> >>>>>>
> >>>>>>
> >>>>>>> Abatron Support <support at abatron.ch> wrote on 2012/05/30 14:08:26:
> >>>>>>>>
> >>>>>>>>>> I have tested this briefly with BDI2000 on P2010(e500) and
> >>>>>>>>>> it works for me. I don't know if there are any bad side effects,
> >>>>>>>>>> therfore
> >>>>>>>>>> this RFC.
> >>>>>>>>
> >>>>>>>>> We used to have MSR_DE surrounded by CONFIG_something
> >>>>>>>>> to ensure it wasn't set under normal operation.  IIRC, if MSR_DE
> >>>>>>>>> is set, you will have problems with software debuggers that
> >>>>>>>>> utilize the the debugging registers in the chip itself.  You only want
> >>>>>>>>> to force this to be set when using the BDI, not at other times.
> >>>>>>>>
> >>>>>>>> This MSR_DE is also of interest and used for software debuggers that
> >>>>>>>> make use of the debug registers. Only if MSR_DE is set then debug
> >>>>>>>> interrupts are generated. If a debug event leads to a debug interrupt
> >>>>>>>> handled by a software debugger or if it leads to a debug halt handled
> >>>>>>>> by a JTAG tool is selected with DBCR0_EDM / DBCR0_IDM.
> >>>>>>>>
> >>>>>>>> The "e500 Core Family Reference Manual" chapter "Chapter 8
> >>>>>>>> Debug Support" explains in detail the effect of MSR_DE.
> >>>>>>
> >>>>>>> So what is the verdict on this? I don't buy into Dan argument without some
> >>>>>>> hard data.
> >>>>>>
> >>>>>> What I tried to mention is that handling the MSR_DE correct is not only
> >>>>>> an emulator (JTAG debugger) requirement. Also a software debugger may
> >>>>>> depend on a correct handled MSR_DE bit.
> >>>>>
> >>>>> Yes, that made sense to me too. How would SW debuggers work if the kernel keeps
> >>>>> turning off MSR_DE first chance it gets?
> >>>>
> >>>> The kernel selectively enables MSR_DE when it wants to debug.  I'm not
> >>>> sure if anything will be bothered by leaving it on all the time.  This
> >>>> is something we need for virtualization as well, so a hypervisor can
> >>>> debug the guest.
> >>>
> >>> hmm, I read that as you as in favour of the patch?
> >>
> >> I'd want some confirmation that it doesn't break anything, and that
> >> there aren't any other places that need MSR_DE that this doesn't cover,
> >> but in general yes.
> >
> > Then you need to test drive the patch :)
>
> I was thinking more along the lines of someone who's more familiar with
> the relevant parts of the code confirming that it's really OK, not just
> testing that it doesn't blow up in my face.

Still needs a test run, just throw it in :)