target/ppc/helper_regs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
From: Bin Meng <bin.meng@windriver.com>
Per E500 core reference manual [1], chapter 8.4.4 "Branch Taken Debug
Event" and chapter 8.4.5 "Instruction Complete Debug Event":
"A branch taken debug event occurs if both MSR[DE] and DBCR0[BRT]
are set ... Branch taken debug events are not recognized if MSR[DE]
is cleared when the branch instruction executes."
"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."
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://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
Changes in v2:
- update commit message to use E500CORERM instead of PowerISA 2.07
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
Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks, Daniel On 4/20/22 22:17, Bin Meng wrote: > From: Bin Meng <bin.meng@windriver.com> > > Per E500 core reference manual [1], chapter 8.4.4 "Branch Taken Debug > Event" and chapter 8.4.5 "Instruction Complete Debug Event": > > "A branch taken debug event occurs if both MSR[DE] and DBCR0[BRT] > are set ... Branch taken debug events are not recognized if MSR[DE] > is cleared when the branch instruction executes." > > "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." > > 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://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > --- > > Changes in v2: > - update commit message to use E500CORERM instead of PowerISA 2.07 > > 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 20/04/2022 22:17, Bin Meng wrote: > From: Bin Meng<bin.meng@windriver.com> > > Per E500 core reference manual [1], chapter 8.4.4 "Branch Taken Debug > Event" and chapter 8.4.5 "Instruction Complete Debug Event": > > "A branch taken debug event occurs if both MSR[DE] and DBCR0[BRT] > are set ... Branch taken debug events are not recognized if MSR[DE] > is cleared when the branch instruction executes." > > "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." > > 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://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf > > Signed-off-by: Bin Meng<bin.meng@windriver.com> > --- > > Changes in v2: > - update commit message to use E500CORERM instead of PowerISA 2.07 > > 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) { There was a discussion some time ago that was better to avoid hidden uses of *env, so it may be better to change msr_de to ((env->msr >> MSR_DE) & 1) or to (env->msr & BIT_ULL(MSR_DE)) > hflags |= 1 << HFLAGS_SE; > } > - if (dbcr0 & DBCR0_BRT) { > + if ((dbcr0 & DBCR0_BRT) && msr_de) { Here as well > hflags |= 1 << HFLAGS_BE; > } > } else { > -- > 2.25.1 > > Apart from that, Reviewed-by: Lucas Mateus Castro <lucas.araujo@eldorado.org.br> -- Lucas Mateus M. Araujo e Castro Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> Departamento Computação Embarcada Analista de Software Trainee Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
On 4/25/22 16:16, Lucas Mateus Martins Araujo e Castro wrote: > > On 20/04/2022 22:17, Bin Meng wrote: >> From: Bin Meng<bin.meng@windriver.com> >> >> Per E500 core reference manual [1], chapter 8.4.4 "Branch Taken Debug >> Event" and chapter 8.4.5 "Instruction Complete Debug Event": >> >> "A branch taken debug event occurs if both MSR[DE] and DBCR0[BRT] >> are set ... Branch taken debug events are not recognized if MSR[DE] >> is cleared when the branch instruction executes." >> >> "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." >> >> 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://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf >> >> Signed-off-by: Bin Meng<bin.meng@windriver.com> >> --- >> >> Changes in v2: >> - update commit message to use E500CORERM instead of PowerISA 2.07 >> >> 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) { > There was a discussion some time ago that was better to avoid hidden uses of *env, so it may be better to change msr_de to ((env->msr >> MSR_DE) & 1) or to (env->msr & BIT_ULL(MSR_DE)) I think the "target/ppc: Remove hidden usages of *env" patchset : https://patchwork.ozlabs.org/project/qemu-ppc/list/?series=296440 will take some time to merge and there will certainly be followups. This patch can go first. Thanks, C. >> hflags |= 1 << HFLAGS_SE; >> } >> - if (dbcr0 & DBCR0_BRT) { >> + if ((dbcr0 & DBCR0_BRT) && msr_de) { > Here as well >> hflags |= 1 << HFLAGS_BE; >> } >> } else { >> -- >> 2.25.1 >> >> > Apart from that, > Reviewed-by: Lucas Mateus Castro <lucas.araujo@eldorado.org.br> > -- > Lucas Mateus M. Araujo e Castro > Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> > Departamento Computação Embarcada > Analista de Software Trainee > Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
Bin Meng <bmeng.cn@gmail.com> writes: > From: Bin Meng <bin.meng@windriver.com> > > Per E500 core reference manual [1], chapter 8.4.4 "Branch Taken Debug > Event" and chapter 8.4.5 "Instruction Complete Debug Event": > > "A branch taken debug event occurs if both MSR[DE] and DBCR0[BRT] > are set ... Branch taken debug events are not recognized if MSR[DE] > is cleared when the branch instruction executes." > > "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." > > 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://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf > > Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
On 4/21/22 03:17, Bin Meng wrote: > From: Bin Meng <bin.meng@windriver.com> > > Per E500 core reference manual [1], chapter 8.4.4 "Branch Taken Debug > Event" and chapter 8.4.5 "Instruction Complete Debug Event": > > "A branch taken debug event occurs if both MSR[DE] and DBCR0[BRT] > are set ... Branch taken debug events are not recognized if MSR[DE] > is cleared when the branch instruction executes." > > "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." > > 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://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf > > Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > > Changes in v2: > - update commit message to use E500CORERM instead of PowerISA 2.07 > > 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 {
© 2016 - 2024 Red Hat, Inc.