target/ppc/helper_regs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
POWERPC_MMU_BOOKE is not a mask and should not be tested with a
bitwise AND operator.
It went unnoticed because it only impacts the 601 CPU implementation
for which we don't have a known firmware image.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
target/ppc/helper_regs.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 7dca585dddeb..5b12cb03c961 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -156,7 +156,8 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
*/
unsigned immu_idx, dmmu_idx;
dmmu_idx = msr & (1 << MSR_PR) ? 0 : 1;
- if (env->mmu_model & POWERPC_MMU_BOOKE) {
+ if (env->mmu_model == POWERPC_MMU_BOOKE ||
+ env->mmu_model == POWERPC_MMU_BOOKE206) {
dmmu_idx |= msr & (1 << MSR_GS) ? 4 : 0;
immu_idx = dmmu_idx;
immu_idx |= msr & (1 << MSR_IS) ? 2 : 0;
@@ -237,7 +238,8 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
((value >> MSR_DR) & 1) != msr_dr) {
cpu_interrupt_exittb(cs);
}
- if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
+ if ((env->mmu_model == POWERPC_MMU_BOOKE ||
+ env->mmu_model == POWERPC_MMU_BOOKE206) &&
((value >> MSR_GS) & 1) != msr_gs) {
cpu_interrupt_exittb(cs);
}
--
2.31.1
On 1/24/22 09:16, Cédric Le Goater wrote: > POWERPC_MMU_BOOKE is not a mask and should not be tested with a > bitwise AND operator. > > It went unnoticed because it only impacts the 601 CPU implementation > for which we don't have a known firmware image. I forgot to change that. There is one here : https://github.com/artyom-tarasenko/openfirmware image : https://github.com/artyom-tarasenko/openfirmware/releases/download/40p-20190413/q40pofw-serial.rom Thanks, C. > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > target/ppc/helper_regs.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c > index 7dca585dddeb..5b12cb03c961 100644 > --- a/target/ppc/helper_regs.c > +++ b/target/ppc/helper_regs.c > @@ -156,7 +156,8 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env) > */ > unsigned immu_idx, dmmu_idx; > dmmu_idx = msr & (1 << MSR_PR) ? 0 : 1; > - if (env->mmu_model & POWERPC_MMU_BOOKE) { > + if (env->mmu_model == POWERPC_MMU_BOOKE || > + env->mmu_model == POWERPC_MMU_BOOKE206) { > dmmu_idx |= msr & (1 << MSR_GS) ? 4 : 0; > immu_idx = dmmu_idx; > immu_idx |= msr & (1 << MSR_IS) ? 2 : 0; > @@ -237,7 +238,8 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv) > ((value >> MSR_DR) & 1) != msr_dr) { > cpu_interrupt_exittb(cs); > } > - if ((env->mmu_model & POWERPC_MMU_BOOKE) && > + if ((env->mmu_model == POWERPC_MMU_BOOKE || > + env->mmu_model == POWERPC_MMU_BOOKE206) && > ((value >> MSR_GS) & 1) != msr_gs) { > cpu_interrupt_exittb(cs); > } >
On Mon, Jan 24, 2022 at 09:16:09AM +0100, Cédric le Goater wrote: > POWERPC_MMU_BOOKE is not a mask and should not be tested with a > bitwise AND operator. > > It went unnoticed because it only impacts the 601 CPU implementation > for which we don't have a known firmware image. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> The longer term fix, of course, is to entirely get rid of the unholy mix of enum and bitmask that is mmu_model. > --- > target/ppc/helper_regs.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c > index 7dca585dddeb..5b12cb03c961 100644 > --- a/target/ppc/helper_regs.c > +++ b/target/ppc/helper_regs.c > @@ -156,7 +156,8 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env) > */ > unsigned immu_idx, dmmu_idx; > dmmu_idx = msr & (1 << MSR_PR) ? 0 : 1; > - if (env->mmu_model & POWERPC_MMU_BOOKE) { > + if (env->mmu_model == POWERPC_MMU_BOOKE || > + env->mmu_model == POWERPC_MMU_BOOKE206) { > dmmu_idx |= msr & (1 << MSR_GS) ? 4 : 0; > immu_idx = dmmu_idx; > immu_idx |= msr & (1 << MSR_IS) ? 2 : 0; > @@ -237,7 +238,8 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv) > ((value >> MSR_DR) & 1) != msr_dr) { > cpu_interrupt_exittb(cs); > } > - if ((env->mmu_model & POWERPC_MMU_BOOKE) && > + if ((env->mmu_model == POWERPC_MMU_BOOKE || > + env->mmu_model == POWERPC_MMU_BOOKE206) && > ((value >> MSR_GS) & 1) != msr_gs) { > cpu_interrupt_exittb(cs); > } -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
© 2016 - 2024 Red Hat, Inc.