All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
@ 2012-05-30  7:43 Joakim Tjernlund
  2012-05-30  7:59 ` Dan Malek
  2012-05-30 13:26 ` Bob Cochran
  0 siblings, 2 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-30  7:43 UTC (permalink / raw)
  To: linuxppc-dev, support, Bob Cochran

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-30  7:43 [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL Joakim Tjernlund
@ 2012-05-30  7:59 ` Dan Malek
  2012-05-30 12:08   ` Re[2]: " Abatron Support
  2012-05-30 13:26 ` Bob Cochran
  1 sibling, 1 reply; 33+ messages in thread
From: Dan Malek @ 2012-05-30  7:59 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, Bob Cochran, support


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re[2]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-30  7:59 ` Dan Malek
@ 2012-05-30 12:08   ` Abatron Support
  2012-05-31  9:05     ` Joakim Tjernlund
  0 siblings, 1 reply; 33+ messages in thread
From: Abatron Support @ 2012-05-30 12:08 UTC (permalink / raw)
  To: Dan Malek; +Cc: linuxppc-dev, Bob Cochran

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-30  7:43 [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL Joakim Tjernlund
  2012-05-30  7:59 ` Dan Malek
@ 2012-05-30 13:26 ` Bob Cochran
  2012-06-01  9:14   ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 33+ messages in thread
From: Bob Cochran @ 2012-05-30 13:26 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re[2]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-30 12:08   ` Re[2]: " Abatron Support
@ 2012-05-31  9:05     ` Joakim Tjernlund
  2012-05-31  9:30       ` Re[4]: " Abatron Support
  2012-06-01  9:12       ` Re[2]: " Benjamin Herrenschmidt
  0 siblings, 2 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-31  9:05 UTC (permalink / raw)
  To: Support; +Cc: linuxppc-dev, Dan Malek, Bob Cochran

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re[4]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31  9:05     ` Joakim Tjernlund
@ 2012-05-31  9:30       ` Abatron Support
  2012-05-31  9:56         ` Joakim Tjernlund
  2012-06-01  9:12       ` Re[2]: " Benjamin Herrenschmidt
  1 sibling, 1 reply; 33+ messages in thread
From: Abatron Support @ 2012-05-31  9:30 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, Dan Malek, Bob Cochran


> 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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re[4]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31  9:30       ` Re[4]: " Abatron Support
@ 2012-05-31  9:56         ` Joakim Tjernlund
  2012-05-31 17:47           ` Scott Wood
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-31  9:56 UTC (permalink / raw)
  To: Support; +Cc: linuxppc-dev, Dan Malek, Bob Cochran

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31  9:56         ` Joakim Tjernlund
@ 2012-05-31 17:47           ` Scott Wood
  2012-05-31 21:38             ` Joakim Tjernlund
  0 siblings, 1 reply; 33+ messages in thread
From: Scott Wood @ 2012-05-31 17:47 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 17:47           ` Scott Wood
@ 2012-05-31 21:38             ` Joakim Tjernlund
  2012-05-31 21:43               ` Scott Wood
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-31 21:38 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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?

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 21:38             ` Joakim Tjernlund
@ 2012-05-31 21:43               ` Scott Wood
  2012-05-31 22:14                 ` Joakim Tjernlund
  0 siblings, 1 reply; 33+ messages in thread
From: Scott Wood @ 2012-05-31 21:43 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 21:43               ` Scott Wood
@ 2012-05-31 22:14                 ` Joakim Tjernlund
  2012-05-31 22:16                   ` Scott Wood
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-31 22:14 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 22:14                 ` Joakim Tjernlund
@ 2012-05-31 22:16                   ` Scott Wood
  2012-05-31 22:33                     ` Joakim Tjernlund
  2012-05-31 22:35                     ` Joakim Tjernlund
  0 siblings, 2 replies; 33+ messages in thread
From: Scott Wood @ 2012-05-31 22:16 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 22:16                   ` Scott Wood
@ 2012-05-31 22:33                     ` Joakim Tjernlund
  2012-05-31 22:35                     ` Joakim Tjernlund
  1 sibling, 0 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-31 22:33 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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 :)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 22:16                   ` Scott Wood
  2012-05-31 22:33                     ` Joakim Tjernlund
@ 2012-05-31 22:35                     ` Joakim Tjernlund
  2012-07-20  8:27                       ` Zang Roy-R61911
  1 sibling, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-05-31 22:35 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re[2]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31  9:05     ` Joakim Tjernlund
  2012-05-31  9:30       ` Re[4]: " Abatron Support
@ 2012-06-01  9:12       ` Benjamin Herrenschmidt
  2012-06-01 10:34         ` Joakim Tjernlund
  1 sibling, 1 reply; 33+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-01  9:12 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-30 13:26 ` Bob Cochran
@ 2012-06-01  9:14   ` Benjamin Herrenschmidt
  2012-06-01 10:38     ` Joakim Tjernlund
  2012-06-01 16:28     ` Scott Wood
  0 siblings, 2 replies; 33+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-01  9:14 UTC (permalink / raw)
  To: Bob Cochran; +Cc: linuxppc-dev, support

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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re[2]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01  9:12       ` Re[2]: " Benjamin Herrenschmidt
@ 2012-06-01 10:34         ` Joakim Tjernlund
  0 siblings, 0 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-06-01 10:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01  9:14   ` Benjamin Herrenschmidt
@ 2012-06-01 10:38     ` Joakim Tjernlund
  2012-06-01 16:28     ` Scott Wood
  1 sibling, 0 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-06-01 10:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Bob Cochran, support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01  9:14   ` Benjamin Herrenschmidt
  2012-06-01 10:38     ` Joakim Tjernlund
@ 2012-06-01 16:28     ` Scott Wood
  2012-06-01 22:30       ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 33+ messages in thread
From: Scott Wood @ 2012-06-01 16:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Bob Cochran, support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01 16:28     ` Scott Wood
@ 2012-06-01 22:30       ` Benjamin Herrenschmidt
  2012-06-01 22:42         ` Scott Wood
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-01 22:30 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Bob Cochran, support

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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01 22:30       ` Benjamin Herrenschmidt
@ 2012-06-01 22:42         ` Scott Wood
  2012-06-01 23:24           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 33+ messages in thread
From: Scott Wood @ 2012-06-01 22:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Bob Cochran, support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01 22:42         ` Scott Wood
@ 2012-06-01 23:24           ` Benjamin Herrenschmidt
  2012-06-02 18:29             ` Joakim Tjernlund
  0 siblings, 1 reply; 33+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-01 23:24 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Bob Cochran, support

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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-01 23:24           ` Benjamin Herrenschmidt
@ 2012-06-02 18:29             ` Joakim Tjernlund
  2012-06-02 21:21               ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-06-02 18:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Bob Cochran, support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-02 18:29             ` Joakim Tjernlund
@ 2012-06-02 21:21               ` Benjamin Herrenschmidt
  2012-06-03  9:20                 ` Joakim Tjernlund
                                   ` (2 more replies)
  0 siblings, 3 replies; 33+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-02 21:21 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev, Bob Cochran, support

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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-02 21:21               ` Benjamin Herrenschmidt
@ 2012-06-03  9:20                 ` Joakim Tjernlund
  2012-06-04  9:06                 ` Joakim Tjernlund
       [not found]                 ` <OF7810EBD9.B191A242-ONC1257A13.0031E9E3-C1257A13.00320D10@LocalDomain>
  2 siblings, 0 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-06-03  9:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Bob Cochran, support

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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-06-02 21:21               ` Benjamin Herrenschmidt
  2012-06-03  9:20                 ` Joakim Tjernlund
@ 2012-06-04  9:06                 ` Joakim Tjernlund
       [not found]                 ` <OF7810EBD9.B191A242-ONC1257A13.0031E9E3-C1257A13.00320D10@LocalDomain>
  2 siblings, 0 replies; 33+ messages in thread
From: Joakim Tjernlund @ 2012-06-04  9:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Bob Cochran, support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
       [not found]                 ` <OF7810EBD9.B191A242-ONC1257A13.0031E9E3-C1257A13.00320D10@LocalDomain>
@ 2012-07-11 14:24                   ` Joakim Tjernlund
  2012-07-11 14:45                     ` Kumar Gala
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-07-11 14:24 UTC (permalink / raw)
  Cc: Scott Wood, support, Bob Cochran, linuxppc-dev

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-07-11 14:24                   ` Joakim Tjernlund
@ 2012-07-11 14:45                     ` Kumar Gala
  0 siblings, 0 replies; 33+ messages in thread
From: Kumar Gala @ 2012-07-11 14:45 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev, Bob Cochran, support


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=

^ permalink raw reply	[flat|nested] 33+ messages in thread

* RE: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-05-31 22:35                     ` Joakim Tjernlund
@ 2012-07-20  8:27                       ` Zang Roy-R61911
  2012-07-20  8:37                         ` Joakim Tjernlund
  0 siblings, 1 reply; 33+ messages in thread
From: Zang Roy-R61911 @ 2012-07-20  8:27 UTC (permalink / raw)
  To: Joakim Tjernlund, Wood Scott-B07421
  Cc: linuxppc-dev, Dan Malek, Bob Cochran, Support



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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* RE: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-07-20  8:27                       ` Zang Roy-R61911
@ 2012-07-20  8:37                         ` Joakim Tjernlund
  2013-06-25  0:51                           ` Scott Wood
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2012-07-20  8:37 UTC (permalink / raw)
  To: Zang Roy-R61911
  Cc: Wood Scott-B07421, Dan Malek, Support, Bob Cochran, linuxppc-dev

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2012-07-20  8:37                         ` Joakim Tjernlund
@ 2013-06-25  0:51                           ` Scott Wood
  2013-06-25  6:00                             ` Joakim Tjernlund
  0 siblings, 1 reply; 33+ messages in thread
From: Scott Wood @ 2013-06-25  0:51 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: Wood Scott-B07421, Zang Roy-R61911, Bob Cochran, linuxppc-dev,
	Dan Malek, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2013-06-25  0:51                           ` Scott Wood
@ 2013-06-25  6:00                             ` Joakim Tjernlund
  2013-06-26 21:42                               ` Scott Wood
  0 siblings, 1 reply; 33+ messages in thread
From: Joakim Tjernlund @ 2013-06-25  6:00 UTC (permalink / raw)
  To: Scott Wood
  Cc: Wood Scott-B07421, Zang Roy-R61911, Bob Cochran, linuxppc-dev,
	Dan Malek, Support

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
  2013-06-25  6:00                             ` Joakim Tjernlund
@ 2013-06-26 21:42                               ` Scott Wood
  0 siblings, 0 replies; 33+ messages in thread
From: Scott Wood @ 2013-06-26 21:42 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: Wood Scott-B07421, Zang Roy-R61911, Bob Cochran, linuxppc-dev,
	Dan Malek, Support

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=

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2013-06-26 21:43 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-30  7:43 [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL Joakim Tjernlund
2012-05-30  7:59 ` Dan Malek
2012-05-30 12:08   ` Re[2]: " Abatron Support
2012-05-31  9:05     ` Joakim Tjernlund
2012-05-31  9:30       ` Re[4]: " Abatron Support
2012-05-31  9:56         ` Joakim Tjernlund
2012-05-31 17:47           ` Scott Wood
2012-05-31 21:38             ` Joakim Tjernlund
2012-05-31 21:43               ` Scott Wood
2012-05-31 22:14                 ` Joakim Tjernlund
2012-05-31 22:16                   ` Scott Wood
2012-05-31 22:33                     ` Joakim Tjernlund
2012-05-31 22:35                     ` Joakim Tjernlund
2012-07-20  8:27                       ` Zang Roy-R61911
2012-07-20  8:37                         ` Joakim Tjernlund
2013-06-25  0:51                           ` Scott Wood
2013-06-25  6:00                             ` Joakim Tjernlund
2013-06-26 21:42                               ` Scott Wood
2012-06-01  9:12       ` Re[2]: " Benjamin Herrenschmidt
2012-06-01 10:34         ` Joakim Tjernlund
2012-05-30 13:26 ` Bob Cochran
2012-06-01  9:14   ` Benjamin Herrenschmidt
2012-06-01 10:38     ` Joakim Tjernlund
2012-06-01 16:28     ` Scott Wood
2012-06-01 22:30       ` Benjamin Herrenschmidt
2012-06-01 22:42         ` Scott Wood
2012-06-01 23:24           ` Benjamin Herrenschmidt
2012-06-02 18:29             ` Joakim Tjernlund
2012-06-02 21:21               ` Benjamin Herrenschmidt
2012-06-03  9:20                 ` Joakim Tjernlund
2012-06-04  9:06                 ` Joakim Tjernlund
     [not found]                 ` <OF7810EBD9.B191A242-ONC1257A13.0031E9E3-C1257A13.00320D10@LocalDomain>
2012-07-11 14:24                   ` Joakim Tjernlund
2012-07-11 14:45                     ` Kumar Gala

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.