[PATCH] target/ppc: Fix BookE debug interrupt generation

[PATCH] target/ppc: Fix BookE debug interrupt generation

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Per PowerISA v2.07 [1], Book III-E, chapter 7.6 "Interrupt definitions"

"When in Internal Debug Mode with MSR.DE=0, then Instruction Complete
and Branch Taken debug events cannot occur, and no DBSR status bits
are set and no subsequent imprecise Debug interrupt will occur."

Current codes do not check MSR.DE bit before setting HFLAGS_SE and
HFLAGS_BE flag, which would cause the immediate debug interrupt to
be generated, e.g.: when DBCR0.ICMP bit is set by guest software
and MSR.DE is not set.

[1] https://ibm.ent.box.com/s/jd5w15gz301s5b5dt375mshpq9c3lh4u

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 target/ppc/helper_regs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 9a691d6833..77bc57415c 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -63,10 +63,10 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
 
     if (ppc_flags & POWERPC_FLAG_DE) {
         target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
-        if (dbcr0 & DBCR0_ICMP) {
+        if ((dbcr0 & DBCR0_ICMP) && msr_de) {
             hflags |= 1 << HFLAGS_SE;
         }
-        if (dbcr0 & DBCR0_BRT) {
+        if ((dbcr0 & DBCR0_BRT) && msr_de) {
             hflags |= 1 << HFLAGS_BE;
         }
     } else {
-- 
2.25.1

Re: [PATCH] target/ppc: Fix BookE debug interrupt generation

[Fabiano Rosas]

Bin Meng <bmeng.cn@gmail.com> writes:

> From: Bin Meng <bin.meng@windriver.com>
>
> Per PowerISA v2.07 [1], Book III-E, chapter 7.6 "Interrupt definitions"

Which BookE board are you concerned about? I don't think we have any
BookE ISA v2.07 in QEMU currently.

> "When in Internal Debug Mode with MSR.DE=0, then Instruction Complete
> and Branch Taken debug events cannot occur, and no DBSR status bits
> are set and no subsequent imprecise Debug interrupt will occur."
>
> Current codes do not check MSR.DE bit before setting HFLAGS_SE and
> HFLAGS_BE flag, which would cause the immediate debug interrupt to
> be generated, e.g.: when DBCR0.ICMP bit is set by guest software
> and MSR.DE is not set.
>

The rationale and the change itself look ok.

> [1] https://ibm.ent.box.com/s/jd5w15gz301s5b5dt375mshpq9c3lh4u
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  target/ppc/helper_regs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 9a691d6833..77bc57415c 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -63,10 +63,10 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
>  
>      if (ppc_flags & POWERPC_FLAG_DE) {
>          target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
> -        if (dbcr0 & DBCR0_ICMP) {
> +        if ((dbcr0 & DBCR0_ICMP) && msr_de) {
>              hflags |= 1 << HFLAGS_SE;
>          }
> -        if (dbcr0 & DBCR0_BRT) {
> +        if ((dbcr0 & DBCR0_BRT) && msr_de) {
>              hflags |= 1 << HFLAGS_BE;
>          }
>      } else {

Re: [PATCH] target/ppc: Fix BookE debug interrupt generation

[Bin Meng]

On Wed, Apr 20, 2022 at 9:50 PM Fabiano Rosas <farosas@linux.ibm.com> wrote:
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Per PowerISA v2.07 [1], Book III-E, chapter 7.6 "Interrupt definitions"
>
> Which BookE board are you concerned about? I don't think we have any
> BookE ISA v2.07 in QEMU currently.

It's actually a PPC E500 core, but I am too lazy to dig out the E500
manual from Freescale/NXP :(

Let me know if I need to replace the link to an E500 manual.

>
> > "When in Internal Debug Mode with MSR.DE=0, then Instruction Complete
> > and Branch Taken debug events cannot occur, and no DBSR status bits
> > are set and no subsequent imprecise Debug interrupt will occur."
> >
> > Current codes do not check MSR.DE bit before setting HFLAGS_SE and
> > HFLAGS_BE flag, which would cause the immediate debug interrupt to
> > be generated, e.g.: when DBCR0.ICMP bit is set by guest software
> > and MSR.DE is not set.
> >
>
> The rationale and the change itself look ok.
>
> > [1] https://ibm.ent.box.com/s/jd5w15gz301s5b5dt375mshpq9c3lh4u
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  target/ppc/helper_regs.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >

Regards,
Bin

Re: [PATCH] target/ppc: Fix BookE debug interrupt generation

[Fabiano Rosas]

Bin Meng <bmeng.cn@gmail.com> writes:

> On Wed, Apr 20, 2022 at 9:50 PM Fabiano Rosas <farosas@linux.ibm.com> wrote:
>>
>> Bin Meng <bmeng.cn@gmail.com> writes:
>>
>> > From: Bin Meng <bin.meng@windriver.com>
>> >
>> > Per PowerISA v2.07 [1], Book III-E, chapter 7.6 "Interrupt definitions"
>>
>> Which BookE board are you concerned about? I don't think we have any
>> BookE ISA v2.07 in QEMU currently.
>
> It's actually a PPC E500 core, but I am too lazy to dig out the E500
> manual from Freescale/NXP :(

Here it is: https://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf

You can keep it =)

> Let me know if I need to replace the link to an E500 manual.

Yes, please. And the description as well:

  "An instruction complete debug event occurs when any instruction
  completes execution so long as MSR[DE] and DBCR0[ICMP] are both set..."
  
  "Instruction complete debug events are not recognized if MSR[DE] is
  cleared at the time of the instruction execution."

Otherwise a few years from now someone will use the git log as reference
and will get confused.

Thanks.

>>
>> > "When in Internal Debug Mode with MSR.DE=0, then Instruction Complete
>> > and Branch Taken debug events cannot occur, and no DBSR status bits
>> > are set and no subsequent imprecise Debug interrupt will occur."
>> >
>> > Current codes do not check MSR.DE bit before setting HFLAGS_SE and
>> > HFLAGS_BE flag, which would cause the immediate debug interrupt to
>> > be generated, e.g.: when DBCR0.ICMP bit is set by guest software
>> > and MSR.DE is not set.
>> >
>>
>> The rationale and the change itself look ok.
>>
>> > [1] https://ibm.ent.box.com/s/jd5w15gz301s5b5dt375mshpq9c3lh4u
>> >
>> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> > ---
>> >
>> >  target/ppc/helper_regs.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>
> Regards,
> Bin