| Message ID | 20220420082006.1096031-1-bmeng.cn@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: Fix BookE debug interrupt generation | expand |
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 {
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
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
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 {