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

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

[Joakim Tjernlund]

Emulators such as BDI2000 and CodeWarrior needs to have MSR_DE set
in order to support break points.
This adds MSR_DE for kernel space only.
---

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.

 arch/powerpc/include/asm/reg.h       |    2 +-
 arch/powerpc/include/asm/reg_booke.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 7fdc2c0..25c8554 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -108,7 +108,7 @@
 #define MSR_USER64	MSR_USER32 | MSR_64BIT
 #elif defined(CONFIG_PPC_BOOK3S_32) || defined(CONFIG_8xx)
 /* Default MSR for kernel mode. */
-#define MSR_KERNEL	(MSR_ME|MSR_RI|MSR_IR|MSR_DR)
+#define MSR_KERNEL	(MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_DE)
 #define MSR_USER	(MSR_KERNEL|MSR_PR|MSR_EE)
 #endif
 
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 500fe1d..0cb259b 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -37,7 +37,7 @@
 #define MSR_KERNEL	(MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
 #define MSR_USER	(MSR_KERNEL|MSR_PR|MSR_EE)
 #else
-#define MSR_KERNEL	(MSR_ME|MSR_RI|MSR_CE)
+#define MSR_KERNEL	(MSR_ME|MSR_RI|MSR_CE|MSR_DE)
 #define MSR_USER	(MSR_KERNEL|MSR_PR|MSR_EE)
 #endif
 
-- 
1.7.3.4

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

[Dan Malek]

Hi Joakim.

On May 30, 2012, at 12:43 AM, Joakim Tjernlund wrote:

> 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.

Thanks.

	-- Dan

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

[Abatron Support]

>> 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.

Ruedi

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

[Joakim Tjernlund]

Abatron Support <support@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.

 Jocke

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

[Abatron Support]

> Abatron Support <support@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.

Ruedi

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

[Joakim Tjernlund]

Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
>
>
> > Abatron Support <support@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?

 Jocke

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

[Scott Wood]

On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
>>
>>
>>> Abatron Support <support@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.

-Scott

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

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
>
> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> > Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
> >>
> >>
> >>> Abatron Support <support@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?

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

[Scott Wood]

On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
>>
>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
>>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
>>>>
>>>>
>>>>> Abatron Support <support@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.

-Scott

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

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 23:43:34:
>
> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
> >>
> >> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> >>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
> >>>>
> >>>>
> >>>>> Abatron Support <support@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 :)

 Jocke

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

[Scott Wood]

On 05/31/2012 05:14 PM, Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 23:43:34:
>>
>> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
>>> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
>>>>
>>>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
>>>>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
>>>>>>
>>>>>>
>>>>>>> Abatron Support <support@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.

-Scott

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

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2012/06/01 00:16:53:
>
> On 05/31/2012 05:14 PM, Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 23:43:34:
> >>
> >> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> >>> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
> >>>>
> >>>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> >>>>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
> >>>>>>
> >>>>>>
> >>>>>>> Abatron Support <support@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 :)

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

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2012/06/01 00:16:53:
>
> On 05/31/2012 05:14 PM, Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 23:43:34:
> >>
> >> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> >>> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
> >>>>
> >>>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> >>>>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
> >>>>>>
> >>>>>>
> >>>>>>> Abatron Support <support@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.

It just occurred to me that you guys have this already in your Linux SDK so it can't be that bad.

 Jocke

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

[Zang Roy-R61911]

> -----Original Message-----
> From: linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.org
> [mailto:linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.or=
g]
> On Behalf Of Joakim Tjernlund
> Sent: Friday, June 01, 2012 6:36 AM
> To: Wood Scott-B07421
> Cc: linuxppc-dev@ozlabs.org; Dan Malek; Bob Cochran; Support
> Subject: Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
>=20
> Scott Wood <scottwood@freescale.com> wrote on 2012/06/01 00:16:53:
> >
> > On 05/31/2012 05:14 PM, Joakim Tjernlund wrote:
> > > Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 23:43:34:
> > >>
> > >> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> > >>> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
> > >>>>
> > >>>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> > >>>>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57=
:
> > >>>>>>
> > >>>>>>
> > >>>>>>> Abatron Support <support@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 time=
s.
> > >>>>>>>>
> > >>>>>>>> This MSR_DE is also of interest and used for software debugger=
s
> 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 n=
ot
> only
> > >>>>>> an emulator (JTAG debugger) requirement. Also a software debugge=
r
> may
> > >>>>>> depend on a correct handled MSR_DE bit.
> > >>>>>
> > >>>>> Yes, that made sense to me too. How would SW debuggers work if th=
e
> 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 c=
an
> > >>>> 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 cov=
er,
> > >> 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.
>=20
> It just occurred to me that you guys have this already in your Linux SDK =
so
> it can't be that bad.
No. MSR_DE is ONLY added when using CW debug in SDK.
Roy

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

[Joakim Tjernlund]

Zang Roy-R61911 <r61911@freescale.com> wrote on 2012/07/20 10:27:52:
>
>
>
> > -----Original Message-----
> > From: linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org
> > [mailto:linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org]
> > On Behalf Of Joakim Tjernlund
> > Sent: Friday, June 01, 2012 6:36 AM
> > To: Wood Scott-B07421
> > Cc: linuxppc-dev@ozlabs.org; Dan Malek; Bob Cochran; Support
> > Subject: Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> >
> > Scott Wood <scottwood@freescale.com> wrote on 2012/06/01 00:16:53:
> > >
> > > On 05/31/2012 05:14 PM, Joakim Tjernlund wrote:
> > > > Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 23:43:34:
> > > >>
> > > >> On 05/31/2012 04:38 PM, Joakim Tjernlund wrote:
> > > >>> Scott Wood <scottwood@freescale.com> wrote on 2012/05/31 19:47:53:
> > > >>>>
> > > >>>> On 05/31/2012 04:56 AM, Joakim Tjernlund wrote:
> > > >>>>> Abatron Support <support@abatron.ch> wrote on 2012/05/31 11:30:57:
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>> Abatron Support <support@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.
> >
> > It just occurred to me that you guys have this already in your Linux SDK so
> > it can't be that bad.
> No. MSR_DE is ONLY added when using CW debug in SDK.
> Roy
>

Yes, and I later found that user space debugging is busted if you turn on MSR_DE in
kernel.

 Jocke

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

[Scott Wood]

On Fri, Jul 20, 2012 at 10:37:17AM +0200, Joakim Tjernlund wrote:
> Zang Roy-R61911 <r61911@freescale.com> wrote on 2012/07/20 10:27:52:
> >
> >
> >
> > > -----Original Message-----
> > > From: linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org
> > > [mailto:linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org]
> > > On Behalf Of Joakim Tjernlund
> > > Sent: Friday, June 01, 2012 6:36 AM
> > > To: Wood Scott-B07421
> > > Cc: linuxppc-dev@ozlabs.org; Dan Malek; Bob Cochran; Support
> > > Subject: Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> > >
> > > It just occurred to me that you guys have this already in your Linux SDK so
> > > it can't be that bad.
> > No. MSR_DE is ONLY added when using CW debug in SDK.
> > Roy
> >
> 
> Yes, and I later found that user space debugging is busted if you turn on MSR_DE in
> kernel.

So, how should we handle the CONFIG_BDI_SWITCH patch?  It seems like it
should at least have a warning in the kconfig help text that it breaks
userspace debugging (to the point of causing a kernel oops if it's
tried).  Or maybe it can deselect CONFIG_PPC_ADV_DEBUG_REGS?

It'd also be nice to keep things like this, that are a consequence of how
external debug works on e500, separate from the Abatron-specific stuff.

-Scott

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

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2013/06/25 02:51:00:
> 
> On Fri, Jul 20, 2012 at 10:37:17AM +0200, Joakim Tjernlund wrote:
> > Zang Roy-R61911 <r61911@freescale.com> wrote on 2012/07/20 10:27:52:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: 
linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org
> > > > [
mailto:linuxppc-dev-bounces+tie-fei.zang=freescale.com@lists.ozlabs.org]
> > > > On Behalf Of Joakim Tjernlund
> > > > Sent: Friday, June 01, 2012 6:36 AM
> > > > To: Wood Scott-B07421
> > > > Cc: linuxppc-dev@ozlabs.org; Dan Malek; Bob Cochran; Support
> > > > Subject: Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> > > >
> > > > It just occurred to me that you guys have this already in your 
Linux SDK so
> > > > it can't be that bad.
> > > No. MSR_DE is ONLY added when using CW debug in SDK.
> > > Roy
> > >
> > 
> > Yes, and I later found that user space debugging is busted if you turn 
on MSR_DE in
> > kernel.
> 
> So, how should we handle the CONFIG_BDI_SWITCH patch?  It seems like it
> should at least have a warning in the kconfig help text that it breaks
> userspace debugging (to the point of causing a kernel oops if it's
> tried).  Or maybe it can deselect CONFIG_PPC_ADV_DEBUG_REGS?
> 
> It'd also be nice to keep things like this, that are a consequence of 
how
> external debug works on e500, separate from the Abatron-specific stuff.
> 

I was hoping the kernel would grow per context handling of MSR_DE. Then 
one could have
MSR_DE on in MSR_KERNEL but off in user space(unless gdb request it on a 
per process basis). 

 Jocke

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

[Scott Wood]

On 06/25/2013 01:00:23 AM, Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 2013/06/25 02:51:00:
> >
> > On Fri, Jul 20, 2012 at 10:37:17AM +0200, Joakim Tjernlund wrote:
> > > Zang Roy-R61911 <r61911@freescale.com> wrote on 2012/07/20 =20
> 10:27:52:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From:
> linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.org
> > > > > [
> mailto:linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.org=
]
> > > > > On Behalf Of Joakim Tjernlund
> > > > > Sent: Friday, June 01, 2012 6:36 AM
> > > > > To: Wood Scott-B07421
> > > > > Cc: linuxppc-dev@ozlabs.org; Dan Malek; Bob Cochran; Support
> > > > > Subject: Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> > > > >
> > > > > It just occurred to me that you guys have this already in your
> Linux SDK so
> > > > > it can't be that bad.
> > > > No. MSR_DE is ONLY added when using CW debug in SDK.
> > > > Roy
> > > >
> > >
> > > Yes, and I later found that user space debugging is busted if you =20
> turn
> on MSR_DE in
> > > kernel.
> >
> > So, how should we handle the CONFIG_BDI_SWITCH patch?  It seems =20
> like it
> > should at least have a warning in the kconfig help text that it =20
> breaks
> > userspace debugging (to the point of causing a kernel oops if it's
> > tried).  Or maybe it can deselect CONFIG_PPC_ADV_DEBUG_REGS?
> >
> > It'd also be nice to keep things like this, that are a consequence =20
> of
> how
> > external debug works on e500, separate from the Abatron-specific =20
> stuff.
> >
>=20
> I was hoping the kernel would grow per context handling of MSR_DE. =20
> Then
> one could have
> MSR_DE on in MSR_KERNEL but off in user space(unless gdb request it =20
> on a
> per process basis).

What if the external debugger wants debugging to work when in userspace =20
as well?  Plus, you wouldn't be able to debug from the very beginning =20
of an exception handler if the exception came from userspace.

-Scott=

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

[Benjamin Herrenschmidt]

On Thu, 2012-05-31 at 11:05 +0200, Joakim Tjernlund wrote:
> Abatron Support <support@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.

The kernel normally controls when to set or not set MSR:DE, at least
when using SW breakpoints. Setting it globally should remain some kind
of specific debug option.

In fact on some CPUs, we even leave user set dbcr settings and rely on
DE being off in kernel space to avoid user->kernel attacks via the debug
registers (I think we still do that on 64-bit BookE though it should
eventually change).

Cheers,
Ben.

>  Jocke
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

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

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/06/01 11:12:51:
>
> On Thu, 2012-05-31 at 11:05 +0200, Joakim Tjernlund wrote:
> > Abatron Support <support@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.
>
> The kernel normally controls when to set or not set MSR:DE, at least
> when using SW breakpoints. Setting it globally should remain some kind
> of specific debug option.
>
> In fact on some CPUs, we even leave user set dbcr settings and rely on
> DE being off in kernel space to avoid user->kernel attacks via the debug
> registers (I think we still do that on 64-bit BookE though it should
> eventually change).

hmm, would it not be better to always clear out/control dbcr settings and always have MSR:DE
on? It would be much easier to control dbcr, even dynamically, than MSR:DE

 Jocke

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

[Bob Cochran]

On 05/30/2012 03:43 AM, Joakim Tjernlund wrote:
> Emulators such as BDI2000 and CodeWarrior needs to have MSR_DE set
> in order to support break points.
> This adds MSR_DE for kernel space only.
> ---
>
> 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.
>
>   arch/powerpc/include/asm/reg.h       |    2 +-
>   arch/powerpc/include/asm/reg_booke.h |    2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
snip


I believe that additional patches are required for CodeWarrior to work 
properly (e.g., assembly start up).  I think the patches should come 
from Freescale.  For whatever reason, they include them in their SDK, 
but haven't submitted them for inclusion in the mainline.

As a developer on Freescale Power products, I would like to see 
Freescale offer up a CodeWarrior patch set, so I don't have to manage 
the patches myself when working outside the SDK (i.e., on a more recent 
kernel).

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

[Benjamin Herrenschmidt]

On Wed, 2012-05-30 at 09:26 -0400, Bob Cochran wrote:
> I believe that additional patches are required for CodeWarrior to
> work 
> properly (e.g., assembly start up).  I think the patches should come 
> from Freescale.  For whatever reason, they include them in their SDK, 
> but haven't submitted them for inclusion in the mainline.
> 
> As a developer on Freescale Power products, I would like to see 
> Freescale offer up a CodeWarrior patch set, so I don't have to manage 
> the patches myself when working outside the SDK (i.e., on a more
> recent 
> kernel).

Such patches would have a hard time getting upstream considering that
codewarrior is a commercial product.

Ben.

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

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/06/01 11:14:49:
>
> On Wed, 2012-05-30 at 09:26 -0400, Bob Cochran wrote:
> > I believe that additional patches are required for CodeWarrior to
> > work
> > properly (e.g., assembly start up).  I think the patches should come
> > from Freescale.  For whatever reason, they include them in their SDK,
> > but haven't submitted them for inclusion in the mainline.
> >
> > As a developer on Freescale Power products, I would like to see
> > Freescale offer up a CodeWarrior patch set, so I don't have to manage
> > the patches myself when working outside the SDK (i.e., on a more
> > recent
> > kernel).
>
> Such patches would have a hard time getting upstream considering that
> codewarrior is a commercial product.

Naa, Abatron BDI is also a commercial product and that is in the kernel. CW
changes pretty much just add the same settings for CONFIG_CW. I think Freescale
should just rename CONFIG_BDI_SWITCH to something generic and just use the same code.

 Jocke

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

[Scott Wood]

On 06/01/2012 04:14 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2012-05-30 at 09:26 -0400, Bob Cochran wrote:
>> I believe that additional patches are required for CodeWarrior to
>> work 
>> properly (e.g., assembly start up).  I think the patches should come 
>> from Freescale.  For whatever reason, they include them in their SDK, 
>> but haven't submitted them for inclusion in the mainline.
>>
>> As a developer on Freescale Power products, I would like to see 
>> Freescale offer up a CodeWarrior patch set, so I don't have to manage 
>> the patches myself when working outside the SDK (i.e., on a more
>> recent 
>> kernel).
> 
> Such patches would have a hard time getting upstream considering that
> codewarrior is a commercial product.

It's not really about CodeWarrior -- it's needed for any external debug
on these chips.

Those chips are commercial products too, BTW. :-)

-Scott

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

[Benjamin Herrenschmidt]

On Fri, 2012-06-01 at 11:28 -0500, Scott Wood wrote:
> 
> It's not really about CodeWarrior -- it's needed for any external
> debug
> on these chips.
> 
> Those chips are commercial products too, BTW. :-)

As long as it's not code to specifically interact with the CW software
it's ok.

I don't have a special axe to grind against CW (I use to love it under
ol' MacOS, though the new eclipse based one does seem to suck hard...
but then I never got a "licence" to use it past the demo anyway), it's
just that I don't want to start building SW interfaces to a foreign
tool.

BTW. My point of view is that this whole business about MSR:DE is a HW
design bug. There should be -no- (absolutely 0) interaction between the
SW state and the HW debugger for normal operations unless the user of
the debugger explicitly wants to change some state.

Cheers,
Ben.

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

[Scott Wood]

On 06/01/2012 05:30 PM, Benjamin Herrenschmidt wrote:
> BTW. My point of view is that this whole business about MSR:DE is a HW
> design bug. There should be -no- (absolutely 0) interaction between the
> SW state and the HW debugger for normal operations unless the user of
> the debugger explicitly wants to change some state.

I agree entirely, and e500mc at least has less of this than e500v2 (not
sure if it still needs MSR[DE], but supposedly it doesn't have the
requirement for there to be a valid instruction at the debug vector,
which is lots of fun when booting).  But this isn't exactly something
Freescale is going to replace existing chips over.

Getting all the way to zero interaction would require a completely
separate debug facility so software can debug at the same time.  I'd be
all for that (and let's throw in a third, for the hypervisor), but I'm
not the one that needs to be convinced.

-Scott

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

[Benjamin Herrenschmidt]

On Fri, 2012-06-01 at 17:42 -0500, Scott Wood wrote:
> On 06/01/2012 05:30 PM, Benjamin Herrenschmidt wrote:
> > BTW. My point of view is that this whole business about MSR:DE is a HW
> > design bug. There should be -no- (absolutely 0) interaction between the
> > SW state and the HW debugger for normal operations unless the user of
> > the debugger explicitly wants to change some state.
> 
> I agree entirely, and e500mc at least has less of this than e500v2 (not
> sure if it still needs MSR[DE], but supposedly it doesn't have the
> requirement for there to be a valid instruction at the debug vector,
> which is lots of fun when booting).  But this isn't exactly something
> Freescale is going to replace existing chips over.
> 
> Getting all the way to zero interaction would require a completely
> separate debug facility so software can debug at the same time.  I'd be
> all for that (and let's throw in a third, for the hypervisor), but I'm
> not the one that needs to be convinced.

You can find a good compromise. If you have some kind of SPR letting you
know now many DACs and IACs are available, you could essentially
"reserve" some for HW debug with the probe. Not as good as a fully
separate facility but still better than stepping on each other toes.

Things like DBCR should probably still be separated. There's no excuse
for the MSR:DE bullshit tho :-)

Cheers,
Ben.

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

[Joakim Tjernlund]

>
> On Fri, 2012-06-01 at 17:42 -0500, Scott Wood wrote:
> > On 06/01/2012 05:30 PM, Benjamin Herrenschmidt wrote:
> > > BTW. My point of view is that this whole business about MSR:DE is a HW
> > > design bug. There should be -no- (absolutely 0) interaction between the
> > > SW state and the HW debugger for normal operations unless the user of
> > > the debugger explicitly wants to change some state.
> >
> > I agree entirely, and e500mc at least has less of this than e500v2 (not
> > sure if it still needs MSR[DE], but supposedly it doesn't have the
> > requirement for there to be a valid instruction at the debug vector,
> > which is lots of fun when booting).  But this isn't exactly something
> > Freescale is going to replace existing chips over.
> >
> > Getting all the way to zero interaction would require a completely
> > separate debug facility so software can debug at the same time.  I'd be
> > all for that (and let's throw in a third, for the hypervisor), but I'm
> > not the one that needs to be convinced.
>
> You can find a good compromise. If you have some kind of SPR letting you
> know now many DACs and IACs are available, you could essentially
> "reserve" some for HW debug with the probe. Not as good as a fully
> separate facility but still better than stepping on each other toes.
>
> Things like DBCR should probably still be separated. There's no excuse
> for the MSR:DE bullshit tho :-)

hmm, where does this go w.r.t the patch? Got the feeling that the
best thing is to just turn MSR:DE on and be done with it?

 Jocke

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

[Benjamin Herrenschmidt]

On Sat, 2012-06-02 at 20:29 +0200, Joakim Tjernlund wrote:
> 
> hmm, where does this go w.r.t the patch? Got the feeling that the
> best thing is to just turn MSR:DE on and be done with it? 

Not unconditionally, we need to have a close look, that might be ok
specifically for BookE 32-bit, it's certainly not ok for BookE 64-bit at
this point.

For now, I'm ok with a debug CONFIG_* option.

Also do we know if MSR:DE has any performance impact on any CPU ? I know
having DACs enabled has a major impact on some for example.

Ben.

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

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/06/02 23:21:16:
>
> On Sat, 2012-06-02 at 20:29 +0200, Joakim Tjernlund wrote:
> >
> > hmm, where does this go w.r.t the patch? Got the feeling that the
> > best thing is to just turn MSR:DE on and be done with it?
>
> Not unconditionally, we need to have a close look, that might be ok
> specifically for BookE 32-bit, it's certainly not ok for BookE 64-bit at
> this point.
>
> For now, I'm ok with a debug CONFIG_* option.

OK, I will wrap this with the existing CONFIG_BDI_SWITCH and only for booke

>
> Also do we know if MSR:DE has any performance impact on any CPU ? I know
> having DACs enabled has a major impact on some for example.

No idea, this is something for Freescale to dwell on.

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

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/06/02 23:21:16:
>
> On Sat, 2012-06-02 at 20:29 +0200, Joakim Tjernlund wrote:
> >
> > hmm, where does this go w.r.t the patch? Got the feeling that the
> > best thing is to just turn MSR:DE on and be done with it?
>
> Not unconditionally, we need to have a close look, that might be ok
> specifically for BookE 32-bit, it's certainly not ok for BookE 64-bit at
> this point.
>
> For now, I'm ok with a debug CONFIG_* option.
>
> Also do we know if MSR:DE has any performance impact on any CPU ? I know
> having DACs enabled has a major impact on some for example.

I just sent a new patch, named
  [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
which will only add MSR_DE for booke under CONFIG_BDI_SWITCH

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

[Joakim Tjernlund]

Joakim Tjernlund/Transmode wrote on 2012/06/04 11:06:41:
>
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/06/02 23:21:16:
> >
> > On Sat, 2012-06-02 at 20:29 +0200, Joakim Tjernlund wrote:
> > >
> > > hmm, where does this go w.r.t the patch? Got the feeling that the
> > > best thing is to just turn MSR:DE on and be done with it?
> >
> > Not unconditionally, we need to have a close look, that might be ok
> > specifically for BookE 32-bit, it's certainly not ok for BookE 64-bit at
> > this point.
> >
> > For now, I'm ok with a debug CONFIG_* option.
> >
> > Also do we know if MSR:DE has any performance impact on any CPU ? I know
> > having DACs enabled has a major impact on some for example.
>
> I just sent a new patch, named
>   [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> which will only add MSR_DE for booke under CONFIG_BDI_SWITCH

Now I tried running gdb in user space and then I get this(with the MSR_DE patch):
root@P2020RDB ~ # ./gdb busybox
GNU gdb 6.8
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "powerpc-softfloat-linux-gnu"...
(no debugging symbols found)
(gdb) run
Starting program: /root/busybox
(no debugging syOops: Exception in kernel mode, sig: 5 [#1]
PREEMPT P1010 RDB
Modules linked in:
NIP: c00067dc LR: c02d1994 CTR: c004a97c
REGS: dfef9f10 TRAP: 2002   Not tainted  (3.4.0+)
MSR: 00021000 <CE,ME>  CR: 42002424  XER: 00000000
TASK = df9a0b10[429] 'gdb' THREAD: dfb9a000
GPR00: cc00cc00 dfb9bd70 df9a0b10 df9a0b10 df949ad0 c0396000 00000000 df949b0c
GPR08: 00000000 40000000 00002786 c03b0000 2b833af6 10493068 10490000 105a62a0
GPR16: 00000000 10490000 10490000 10490000 10490000 10490000 10490000 dfb2ce40
GPR24: df949ad0 c02e0000 c02e0000 c03b20a4 00000004 dfb9a000 c0399ce0 df9a0b10
NIP [c00067dc] __switch_to+0x74/0xc4
LR [c02d1994] __schedule+0x1a0/0x3c0
Call Trace:
[dfb9bd70] [dfb9a000] 0xdfb9a000 (unreliable)
[dfb9bd80] [c02d1994] __schedule+0x1a0/0x3c0
[dfb9bdc0] [c02d1cb8] preempt_schedule+0x54/0x74
[dfb9bdd0] [c0047fe0] try_to_wake_up+0x140/0x144
[dfb9bdf0] [c002b0bc] ptrace_resume+0x70/0xbc
[dfb9be00] [c002c044] ptrace_request+0x1e8/0x5f0
[dfb9bec0] [c00032c4] arch_ptrace+0x30/0x8f8
[dfb9bf10] [c002ba90] sys_ptrace+0xb4/0x3cc
[dfb9bf40] [c000cf4c] ret_from_syscall+0x0/0x3c
--- Exception: c01 at 0xfd607a4
    LR = 0x101a4900
Instruction dump:
41820040 80040234 7c184ba6 80040238 7c194ba6 8004023c 7c1c4ba6 80040240
7c1d4ba6 80040224 7c144ba6 80040228 <7c154ba6> 8004022c 7c164ba6 7c431378
---[ end trace 584ab4ed4087571b ]---

note: gdb[429] exited with preempt_count 268435458
mbols found)
(nBUG: scheduling while atomic: busybox/430/0x10000004
o debugging symbModules linked in:ols found)

When I remove the patch it works. I got no idea were it goes wrong,
any ideas?

 Jocke

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

[Kumar Gala]

On Jul 11, 2012, at 9:24 AM, Joakim Tjernlund wrote:

> Joakim Tjernlund/Transmode wrote on 2012/06/04 11:06:41:
>>=20
>> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2012/06/02 =
23:21:16:
>>>=20
>>> On Sat, 2012-06-02 at 20:29 +0200, Joakim Tjernlund wrote:
>>>>=20
>>>> hmm, where does this go w.r.t the patch? Got the feeling that the
>>>> best thing is to just turn MSR:DE on and be done with it?
>>>=20
>>> Not unconditionally, we need to have a close look, that might be ok
>>> specifically for BookE 32-bit, it's certainly not ok for BookE =
64-bit at
>>> this point.
>>>=20
>>> For now, I'm ok with a debug CONFIG_* option.
>>>=20
>>> Also do we know if MSR:DE has any performance impact on any CPU ? I =
know
>>> having DACs enabled has a major impact on some for example.
>>=20
>> I just sent a new patch, named
>>  [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
>> which will only add MSR_DE for booke under CONFIG_BDI_SWITCH
>=20
> Now I tried running gdb in user space and then I get this(with the =
MSR_DE patch):
> root@P2020RDB ~ # ./gdb busybox
> GNU gdb 6.8
> Copyright (C) 2008 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later =
<http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show =
copying"
> and "show warranty" for details.
> This GDB was configured as "powerpc-softfloat-linux-gnu"...
> (no debugging symbols found)
> (gdb) run
> Starting program: /root/busybox
> (no debugging syOops: Exception in kernel mode, sig: 5 [#1]
> PREEMPT P1010 RDB
> Modules linked in:
> NIP: c00067dc LR: c02d1994 CTR: c004a97c
> REGS: dfef9f10 TRAP: 2002   Not tainted  (3.4.0+)
> MSR: 00021000 <CE,ME>  CR: 42002424  XER: 00000000
> TASK =3D df9a0b10[429] 'gdb' THREAD: dfb9a000
> GPR00: cc00cc00 dfb9bd70 df9a0b10 df9a0b10 df949ad0 c0396000 00000000 =
df949b0c
> GPR08: 00000000 40000000 00002786 c03b0000 2b833af6 10493068 10490000 =
105a62a0
> GPR16: 00000000 10490000 10490000 10490000 10490000 10490000 10490000 =
dfb2ce40
> GPR24: df949ad0 c02e0000 c02e0000 c03b20a4 00000004 dfb9a000 c0399ce0 =
df9a0b10
> NIP [c00067dc] __switch_to+0x74/0xc4
> LR [c02d1994] __schedule+0x1a0/0x3c0
> Call Trace:
> [dfb9bd70] [dfb9a000] 0xdfb9a000 (unreliable)
> [dfb9bd80] [c02d1994] __schedule+0x1a0/0x3c0
> [dfb9bdc0] [c02d1cb8] preempt_schedule+0x54/0x74
> [dfb9bdd0] [c0047fe0] try_to_wake_up+0x140/0x144
> [dfb9bdf0] [c002b0bc] ptrace_resume+0x70/0xbc
> [dfb9be00] [c002c044] ptrace_request+0x1e8/0x5f0
> [dfb9bec0] [c00032c4] arch_ptrace+0x30/0x8f8
> [dfb9bf10] [c002ba90] sys_ptrace+0xb4/0x3cc
> [dfb9bf40] [c000cf4c] ret_from_syscall+0x0/0x3c
> --- Exception: c01 at 0xfd607a4
>    LR =3D 0x101a4900
> Instruction dump:
> 41820040 80040234 7c184ba6 80040238 7c194ba6 8004023c 7c1c4ba6 =
80040240
> 7c1d4ba6 80040224 7c144ba6 80040228 <7c154ba6> 8004022c 7c164ba6 =
7c431378
> ---[ end trace 584ab4ed4087571b ]---
>=20
> note: gdb[429] exited with preempt_count 268435458
> mbols found)
> (nBUG: scheduling while atomic: busybox/430/0x10000004
> o debugging symbModules linked in:ols found)
>=20
> When I remove the patch it works. I got no idea were it goes wrong,
> any ideas?
>=20
> Jocke

I was wondering if this was going to break user space debugging.  What's =
probably happening by having MSR_DE always set, you are getting debug =
exceptions while in the kernel because DBCR[ICMP] (instruction complete) =
is  set and the HW is taking an exception.  Previously MSR_DE would ONLY =
be set while in user mode and thus prevent "spurious" debug exceptions =
like this.

- k=