Re: [PATCH v2 11/21] target/ppc: Remove msr_gs macro

[Richard Henderson]

On 5/2/22 07:39, Víctor Colombo wrote:

-((value >> MSR_GS) & 1) != msr_gs) {
+!(value & env->msr & R_MSR_GS_MASK)) {


This isn't right.  I think you wanted

  (value ^ env->msr) & R_MSR_GS_MASK


r~

[PATCH v2 11/21] target/ppc: Remove msr_gs macro

[Víctor Colombo]

msr_gs macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.

Suggested-by: Richard Henderson 
Signed-off-by: Víctor Colombo 

---

v2: Remove M_MSR_GS and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo 
---
 target/ppc/cpu.h | 2 +-
 target/ppc/helper_regs.c | 2 +-
 target/ppc/mmu_helper.c  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 059a00ed65..4b69cd666d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt1*/
 #define MSR_LE   0  /* Little-endian mode   1 hflags */
 
+FIELD(MSR, GS, MSR_GS, 1)
 FIELD(MSR, POW, MSR_POW, 1)
 FIELD(MSR, CE, MSR_CE, 1)
 FIELD(MSR, ILE, MSR_ILE, 1)
@@ -479,7 +480,6 @@ FIELD(MSR, LE, MSR_LE, 1)
 #define msr_hv   (0)
 #endif
 #define msr_cm   ((env->msr >> MSR_CM)   & 1)
-#define msr_gs   ((env->msr >> MSR_GS)   & 1)
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 940f0207a0..88fcc01589 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -233,7 +233,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, 
int alter_hv)
 }
 if ((env->mmu_model == POWERPC_MMU_BOOKE ||
  env->mmu_model == POWERPC_MMU_BOOKE206) &&
-((value >> MSR_GS) & 1) != msr_gs) {
+!(value & env->msr & R_MSR_GS_MASK)) {
 cpu_interrupt_exittb(cs);
 }
 if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 142a717255..5bb5c71038 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -935,7 +935,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
 }
 
 if (((env->spr[SPR_BOOKE_MAS0] & MAS0_ATSEL) == MAS0_ATSEL_LRAT) &&
-!msr_gs) {
+!FIELD_EX64(env->msr, MSR, GS)) {
 /* XXX we don't support direct LRAT setting yet */
 fprintf(stderr, "cpu: don't support LRAT setting yet\n");
 return;
@@ -962,7 +962,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
POWERPC_EXCP_INVAL_INVAL, GETPC());
 }
 
-if (msr_gs) {
+if (FIELD_EX64(env->msr, MSR, GS)) {
 cpu_abort(env_cpu(env), "missing HV implementation\n");
 }
 
-- 
2.25.1