[PATCH 00/20] target/ppc: Remove hidden usages of *env

[PATCH 00/20] target/ppc: Remove hidden usages of *env

[Víctor Colombo]

By running the grep command `git grep -nr 'define \(fpscr\|msr\)_[a-z0-9]\+\>'`
we can find multiple macros that use `env->fpscr` and `env->msr` but doesn't 
take *env as a parameter.

Richard Henderson said [1] that these macros hiding the usage of *env "are evil".
This patch series remove them and substitute with an explicit usage of *env by
adding macros in the same style of FP_* ones (e.g. FP_FI defined in cpu.h).

Patch 20 (target/ppc: Add unused M_MSR_* macros) implements unused macros, the
same that were removed in patch 02 (target/ppc: Remove unused msr_* macros). I
did that to keep the changes consistent with what was already present before.

[1]: https://lists.gnu.org/archive/html/qemu-ppc/2021-11/msg00280.html

Víctor Colombo (20):
  target/ppc: Remove fpscr_* macros from cpu.h
  target/ppc: Remove unused msr_* macros
  target/ppc: Substitute msr_pr macro with new M_MSR_PR macro
  target/ppc: Substitute msr_le macro with new M_MSR_LE macro
  target/ppc: Substitute msr_ds macro with new M_MSR_DS macro
  target/ppc: Substitute msr_ile macro with new M_MSR_ILE macro
  target/ppc: Substitute msr_ee macro with new M_MSR_EE macro
  target/ppc: Substitute msr_ce macro with new M_MSR_CE macro
  target/ppc: Substitute msr_pow macro with new M_MSR_POW macro
  target/ppc: Substitute msr_me macro with new M_MSR_ME macro
  target/ppc: Substitute msr_gs macro with new M_MSR_GS macro
  target/ppc: Substitute msr_fp macro with new M_MSR_FP macro
  target/ppc: Substitute msr_cm macro with new M_MSR_CM macro
  target/ppc: Substitute msr_ir macro with new M_MSR_IR macro
  target/ppc: Substitute msr_dr macro with new M_MSR_DR macro
  target/ppc: Substitute msr_ep macro with new M_MSR_EP macro
  target/ppc: Substitute msr_fe macro with new M_MSR_FE macro
  target/ppc: Substitute msr_ts macro with new M_MSR_TS macro
  target/ppc: Substitute msr_hv macro with new M_MSR_HV macro
  target/ppc: Add unused M_MSR_* macros

 hw/ppc/pegasos2.c        |   2 +-
 hw/ppc/spapr.c           |   2 +-
 target/ppc/cpu.c         |   2 +-
 target/ppc/cpu.h         | 125 ++++++++++++++++-----------------------
 target/ppc/cpu_init.c    |  21 ++++---
 target/ppc/excp_helper.c |  53 +++++++++--------
 target/ppc/fpu_helper.c  |  28 ++++-----
 target/ppc/gdbstub.c     |   2 +-
 target/ppc/helper_regs.c |  12 ++--
 target/ppc/kvm.c         |   7 ++-
 target/ppc/machine.c     |   2 +-
 target/ppc/mem_helper.c  |  23 +++----
 target/ppc/misc_helper.c |   2 +-
 target/ppc/mmu-radix64.c |  10 ++--
 target/ppc/mmu_common.c  |  38 ++++++------
 target/ppc/mmu_helper.c  |   6 +-
 16 files changed, 161 insertions(+), 174 deletions(-)

-- 
2.25.1

[PATCH 01/20] target/ppc: Remove fpscr_* macros from cpu.h

[Víctor Colombo]

fpscr_* defined macros are hiding the usage of *env behind them.
Substitute the usage of these macros with `env->fpscr & FP_*` to make
the code cleaner.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.c        |  2 +-
 target/ppc/cpu.h        | 29 -----------------------------
 target/ppc/fpu_helper.c | 28 ++++++++++++++--------------
 3 files changed, 15 insertions(+), 44 deletions(-)

diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index d7b42bae52..401b6f9e63 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -88,7 +88,7 @@ static inline void fpscr_set_rounding_mode(CPUPPCState *env)
     int rnd_type;
 
     /* Set rounding mode */
-    switch (fpscr_rn) {
+    switch (env->fpscr & FP_RN) {
     case 0:
         /* Best approximation (round to nearest) */
         rnd_type = float_round_nearest_even;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c2b6c987c0..ad31e51d69 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -713,41 +713,12 @@ enum {
 #define FPSCR_NI     2  /* Floating-point non-IEEE mode                      */
 #define FPSCR_RN1    1
 #define FPSCR_RN0    0  /* Floating-point rounding control                   */
-#define fpscr_drn    (((env->fpscr) & FP_DRN) >> FPSCR_DRN0)
-#define fpscr_fex    (((env->fpscr) >> FPSCR_FEX)    & 0x1)
-#define fpscr_vx     (((env->fpscr) >> FPSCR_VX)     & 0x1)
-#define fpscr_ox     (((env->fpscr) >> FPSCR_OX)     & 0x1)
-#define fpscr_ux     (((env->fpscr) >> FPSCR_UX)     & 0x1)
-#define fpscr_zx     (((env->fpscr) >> FPSCR_ZX)     & 0x1)
-#define fpscr_xx     (((env->fpscr) >> FPSCR_XX)     & 0x1)
-#define fpscr_vxsnan (((env->fpscr) >> FPSCR_VXSNAN) & 0x1)
-#define fpscr_vxisi  (((env->fpscr) >> FPSCR_VXISI)  & 0x1)
-#define fpscr_vxidi  (((env->fpscr) >> FPSCR_VXIDI)  & 0x1)
-#define fpscr_vxzdz  (((env->fpscr) >> FPSCR_VXZDZ)  & 0x1)
-#define fpscr_vximz  (((env->fpscr) >> FPSCR_VXIMZ)  & 0x1)
-#define fpscr_vxvc   (((env->fpscr) >> FPSCR_VXVC)   & 0x1)
-#define fpscr_fpcc   (((env->fpscr) >> FPSCR_FPCC)   & 0xF)
-#define fpscr_vxsoft (((env->fpscr) >> FPSCR_VXSOFT) & 0x1)
-#define fpscr_vxsqrt (((env->fpscr) >> FPSCR_VXSQRT) & 0x1)
-#define fpscr_vxcvi  (((env->fpscr) >> FPSCR_VXCVI)  & 0x1)
-#define fpscr_ve     (((env->fpscr) >> FPSCR_VE)     & 0x1)
-#define fpscr_oe     (((env->fpscr) >> FPSCR_OE)     & 0x1)
-#define fpscr_ue     (((env->fpscr) >> FPSCR_UE)     & 0x1)
-#define fpscr_ze     (((env->fpscr) >> FPSCR_ZE)     & 0x1)
-#define fpscr_xe     (((env->fpscr) >> FPSCR_XE)     & 0x1)
-#define fpscr_ni     (((env->fpscr) >> FPSCR_NI)     & 0x1)
-#define fpscr_rn     (((env->fpscr) >> FPSCR_RN0)    & 0x3)
 /* Invalid operation exception summary */
 #define FPSCR_IX     ((1 << FPSCR_VXSNAN) | (1 << FPSCR_VXISI)  | \
                       (1 << FPSCR_VXIDI)  | (1 << FPSCR_VXZDZ)  | \
                       (1 << FPSCR_VXIMZ)  | (1 << FPSCR_VXVC)   | \
                       (1 << FPSCR_VXSOFT) | (1 << FPSCR_VXSQRT) | \
                       (1 << FPSCR_VXCVI))
-/* exception summary */
-#define fpscr_ex  (((env->fpscr) >> FPSCR_XX) & 0x1F)
-/* enabled exception summary */
-#define fpscr_eex (((env->fpscr) >> FPSCR_XX) & ((env->fpscr) >> FPSCR_XE) &  \
-                   0x1F)
 
 #define FP_DRN2         (1ull << FPSCR_DRN2)
 #define FP_DRN1         (1ull << FPSCR_DRN1)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 99281cc37a..f6c8318a71 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -202,7 +202,7 @@ static void finish_invalid_op_excp(CPUPPCState *env, int op, uintptr_t retaddr)
     env->fpscr |= FP_VX;
     /* Update the floating-point exception summary */
     env->fpscr |= FP_FX;
-    if (fpscr_ve != 0) {
+    if (env->fpscr & FP_VE) {
         /* Update the floating-point enabled exception summary */
         env->fpscr |= FP_FEX;
         if (fp_exceptions_enabled(env)) {
@@ -216,7 +216,7 @@ static void finish_invalid_op_arith(CPUPPCState *env, int op,
                                     bool set_fpcc, uintptr_t retaddr)
 {
     env->fpscr &= ~(FP_FR | FP_FI);
-    if (fpscr_ve == 0) {
+    if (!(env->fpscr & FP_VE)) {
         if (set_fpcc) {
             env->fpscr &= ~FP_FPCC;
             env->fpscr |= (FP_C | FP_FU);
@@ -286,7 +286,7 @@ static void float_invalid_op_vxvc(CPUPPCState *env, bool set_fpcc,
     /* Update the floating-point exception summary */
     env->fpscr |= FP_FX;
     /* We must update the target FPR before raising the exception */
-    if (fpscr_ve != 0) {
+    if (env->fpscr & FP_VE) {
         CPUState *cs = env_cpu(env);
 
         cs->exception_index = POWERPC_EXCP_PROGRAM;
@@ -303,7 +303,7 @@ static void float_invalid_op_vxcvi(CPUPPCState *env, bool set_fpcc,
 {
     env->fpscr |= FP_VXCVI;
     env->fpscr &= ~(FP_FR | FP_FI);
-    if (fpscr_ve == 0) {
+    if (!(env->fpscr & FP_VE)) {
         if (set_fpcc) {
             env->fpscr &= ~FP_FPCC;
             env->fpscr |= (FP_C | FP_FU);
@@ -318,7 +318,7 @@ static inline void float_zero_divide_excp(CPUPPCState *env, uintptr_t raddr)
     env->fpscr &= ~(FP_FR | FP_FI);
     /* Update the floating-point exception summary */
     env->fpscr |= FP_FX;
-    if (fpscr_ze != 0) {
+    if (env->fpscr & FP_ZE) {
         /* Update the floating-point enabled exception summary */
         env->fpscr |= FP_FEX;
         if (fp_exceptions_enabled(env)) {
@@ -336,7 +336,7 @@ static inline void float_overflow_excp(CPUPPCState *env)
     env->fpscr |= FP_OX;
     /* Update the floating-point exception summary */
     env->fpscr |= FP_FX;
-    if (fpscr_oe != 0) {
+    if (env->fpscr & FP_OE) {
         /* XXX: should adjust the result */
         /* Update the floating-point enabled exception summary */
         env->fpscr |= FP_FEX;
@@ -356,7 +356,7 @@ static inline void float_underflow_excp(CPUPPCState *env)
     env->fpscr |= FP_UX;
     /* Update the floating-point exception summary */
     env->fpscr |= FP_FX;
-    if (fpscr_ue != 0) {
+    if (env->fpscr & FP_UE) {
         /* XXX: should adjust the result */
         /* Update the floating-point enabled exception summary */
         env->fpscr |= FP_FEX;
@@ -374,7 +374,7 @@ static inline void float_inexact_excp(CPUPPCState *env)
     env->fpscr |= FP_XX;
     /* Update the floating-point exception summary */
     env->fpscr |= FP_FX;
-    if (fpscr_xe != 0) {
+    if (env->fpscr & FP_XE) {
         /* Update the floating-point enabled exception summary */
         env->fpscr |= FP_FEX;
         /* We must update the target FPR before raising the exception */
@@ -2274,7 +2274,7 @@ VSX_MADDQ(XSNMSUBQPO, NMSUB_FLGS, 0)
         vxvc = svxvc;                                                         \
         if (flags & float_flag_invalid_snan) {                                \
             float_invalid_op_vxsnan(env, GETPC());                            \
-            vxvc &= fpscr_ve == 0;                                            \
+            vxvc &= !(env->fpscr & FP_VE);                                    \
         }                                                                     \
         if (vxvc) {                                                           \
             float_invalid_op_vxvc(env, 0, GETPC());                           \
@@ -2375,7 +2375,7 @@ static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb,
         if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||
             float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {
             vxsnan_flag = true;
-            if (fpscr_ve == 0 && ordered) {
+            if (!(env->fpscr & FP_VE) && ordered) {
                 vxvc_flag = true;
             }
         } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||
@@ -2440,7 +2440,7 @@ static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa,
         if (float128_is_signaling_nan(xa->f128, &env->fp_status) ||
             float128_is_signaling_nan(xb->f128, &env->fp_status)) {
             vxsnan_flag = true;
-            if (fpscr_ve == 0 && ordered) {
+            if (!(env->fpscr & FP_VE) && ordered) {
                 vxvc_flag = true;
             }
         } else if (float128_is_quiet_nan(xa->f128, &env->fp_status) ||
@@ -2590,7 +2590,7 @@ void helper_##name(CPUPPCState *env,                                          \
         t.VsrD(0) = xb->VsrD(0);                                              \
     }                                                                         \
                                                                               \
-    vex_flag = fpscr_ve & vxsnan_flag;                                        \
+    vex_flag = (env->fpscr & FP_VE) && vxsnan_flag;                           \
     if (vxsnan_flag) {                                                        \
         float_invalid_op_vxsnan(env, GETPC());                                \
     }                                                                         \
@@ -3320,7 +3320,7 @@ void helper_xsrqpi(CPUPPCState *env, uint32_t opcode,
     if (r == 0 && rmc == 0) {
         rmode = float_round_ties_away;
     } else if (r == 0 && rmc == 0x3) {
-        rmode = fpscr_rn;
+        rmode = env->fpscr & FP_RN;
     } else if (r == 1) {
         switch (rmc) {
         case 0:
@@ -3374,7 +3374,7 @@ void helper_xsrqpxp(CPUPPCState *env, uint32_t opcode,
     if (r == 0 && rmc == 0) {
         rmode = float_round_ties_away;
     } else if (r == 0 && rmc == 0x3) {
-        rmode = fpscr_rn;
+        rmode = env->fpscr & FP_RN;
     } else if (r == 1) {
         switch (rmc) {
         case 0:
-- 
2.25.1

[PATCH 02/20] target/ppc: Remove unused msr_* macros

[Víctor Colombo]

Some msr_* macros are not used anywhere. Remove them as part of
the work to remove all hidden usage of *env.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index ad31e51d69..106b555b86 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -463,23 +463,14 @@ typedef enum {
 #define HFSCR_MSGP     PPC_BIT(53) /* Privileged Message Send Facilities */
 #define HFSCR_IC_MSGP  0xA
 
-#define msr_sf   ((env->msr >> MSR_SF)   & 1)
-#define msr_isf  ((env->msr >> MSR_ISF)  & 1)
 #if defined(TARGET_PPC64)
 #define msr_hv   ((env->msr >> MSR_HV)   & 1)
 #else
 #define msr_hv   (0)
 #endif
 #define msr_cm   ((env->msr >> MSR_CM)   & 1)
-#define msr_icm  ((env->msr >> MSR_ICM)  & 1)
 #define msr_gs   ((env->msr >> MSR_GS)   & 1)
-#define msr_ucle ((env->msr >> MSR_UCLE) & 1)
-#define msr_vr   ((env->msr >> MSR_VR)   & 1)
-#define msr_spe  ((env->msr >> MSR_SPE)  & 1)
-#define msr_vsx  ((env->msr >> MSR_VSX)  & 1)
-#define msr_key  ((env->msr >> MSR_KEY)  & 1)
 #define msr_pow  ((env->msr >> MSR_POW)  & 1)
-#define msr_tgpr ((env->msr >> MSR_TGPR) & 1)
 #define msr_ce   ((env->msr >> MSR_CE)   & 1)
 #define msr_ile  ((env->msr >> MSR_ILE)  & 1)
 #define msr_ee   ((env->msr >> MSR_EE)   & 1)
@@ -487,25 +478,13 @@ typedef enum {
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_me   ((env->msr >> MSR_ME)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
-#define msr_se   ((env->msr >> MSR_SE)   & 1)
-#define msr_dwe  ((env->msr >> MSR_DWE)  & 1)
-#define msr_uble ((env->msr >> MSR_UBLE) & 1)
-#define msr_be   ((env->msr >> MSR_BE)   & 1)
-#define msr_de   ((env->msr >> MSR_DE)   & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
-#define msr_al   ((env->msr >> MSR_AL)   & 1)
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
 #define msr_ir   ((env->msr >> MSR_IR)   & 1)
 #define msr_dr   ((env->msr >> MSR_DR)   & 1)
-#define msr_is   ((env->msr >> MSR_IS)   & 1)
 #define msr_ds   ((env->msr >> MSR_DS)   & 1)
-#define msr_pe   ((env->msr >> MSR_PE)   & 1)
-#define msr_px   ((env->msr >> MSR_PX)   & 1)
-#define msr_pmm  ((env->msr >> MSR_PMM)  & 1)
-#define msr_ri   ((env->msr >> MSR_RI)   & 1)
 #define msr_le   ((env->msr >> MSR_LE)   & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
-#define msr_tm   ((env->msr >> MSR_TM)   & 1)
 
 #define DBCR0_ICMP (1 << 27)
 #define DBCR0_BRT (1 << 26)
-- 
2.25.1

[PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 hw/ppc/pegasos2.c        |  2 +-
 hw/ppc/spapr.c           |  2 +-
 target/ppc/cpu.h         |  3 ++-
 target/ppc/cpu_init.c    |  4 ++--
 target/ppc/excp_helper.c |  6 +++---
 target/ppc/mem_helper.c  |  4 ++--
 target/ppc/mmu-radix64.c |  4 ++--
 target/ppc/mmu_common.c  | 23 ++++++++++++-----------
 8 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 56bf203dfd..27ed54a71d 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -461,7 +461,7 @@ static void pegasos2_hypercall(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
     /* The TCG path should also be holding the BQL at this point */
     g_assert(qemu_mutex_iothread_locked());
 
-    if (msr_pr) {
+    if (env->msr & M_MSR_PR) {
         qemu_log_mask(LOG_GUEST_ERROR, "Hypercall made with MSR[PR]=1\n");
         env->gpr[3] = H_PRIVILEGE;
     } else if (env->gpr[3] == KVMPPC_H_RTAS) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 22569305d2..c947494099 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1269,7 +1269,7 @@ static void emulate_spapr_hypercall(PPCVirtualHypervisor *vhyp,
 
     g_assert(!vhyp_cpu_in_nested(cpu));
 
-    if (msr_pr) {
+    if (env->msr & M_MSR_PR) {
         hcall_dprintf("Hypercall made with MSR[PR]=1\n");
         env->gpr[3] = H_PRIVILEGE;
     } else {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 106b555b86..2ad023e981 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,8 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_PR (1ull << MSR_PR)
+
 /* PMU bits */
 #define MMCR0_FC     PPC_BIT(32)         /* Freeze Counters  */
 #define MMCR0_PMAO   PPC_BIT(56)         /* Perf Monitor Alert Ocurred */
@@ -474,7 +476,6 @@ typedef enum {
 #define msr_ce   ((env->msr >> MSR_CE)   & 1)
 #define msr_ile  ((env->msr >> MSR_ILE)  & 1)
 #define msr_ee   ((env->msr >> MSR_EE)   & 1)
-#define msr_pr   ((env->msr >> MSR_PR)   & 1)
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_me   ((env->msr >> MSR_ME)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d42e2ba8e0..6e2b23a859 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6303,7 +6303,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
         if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
             (env->spr[SPR_LPCR] & LPCR_EEE)) {
             bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-            if (heic == 0 || !msr_hv || msr_pr) {
+            if (!heic || !msr_hv || (env->msr & M_MSR_PR)) {
                 return true;
             }
         }
@@ -6517,7 +6517,7 @@ static bool cpu_has_work_POWER10(CPUState *cs)
         if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
             (env->spr[SPR_LPCR] & LPCR_EEE)) {
             bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-            if (heic == 0 || !msr_hv || msr_pr) {
+            if (!heic || !msr_hv || (env->msr & M_MSR_PR)) {
                 return true;
             }
         }
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d3e2cfcd71..10cd381be2 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1738,7 +1738,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
         bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
         bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
         /* HEIC blocks delivery to the hypervisor */
-        if ((async_deliver && !(heic && msr_hv && !msr_pr)) ||
+        if ((async_deliver && !(heic && msr_hv && !(env->msr & M_MSR_PR))) ||
             (env->has_hv_mode && msr_hv == 0 && !lpes0)) {
             if (books_vhyp_promotes_external_to_hvirt(cpu)) {
                 powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
@@ -1818,7 +1818,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
              * EBB exception must be taken in problem state and
              * with BESCR_GE set.
              */
-            if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) {
+            if ((env->msr & M_MSR_PR) && (env->spr[SPR_BESCR] & BESCR_GE)) {
                 env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
 
                 if (env->spr[SPR_BESCR] & BESCR_PMEO) {
@@ -2094,7 +2094,7 @@ static void do_ebb(CPUPPCState *env, int ebb_excp)
         env->spr[SPR_BESCR] |= BESCR_EEO;
     }
 
-    if (msr_pr == 1) {
+    if (env->msr & M_MSR_PR) {
         powerpc_excp(cpu, ebb_excp);
     } else {
         env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index c4ff8fd632..bd219e9c9c 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -613,10 +613,10 @@ void helper_tbegin(CPUPPCState *env)
         (1ULL << TEXASR_FAILURE_PERSISTENT) |
         (1ULL << TEXASR_NESTING_OVERFLOW) |
         (msr_hv << TEXASR_PRIVILEGE_HV) |
-        (msr_pr << TEXASR_PRIVILEGE_PR) |
+        (!!(env->msr & M_MSR_PR) << TEXASR_PRIVILEGE_PR) |
         (1ULL << TEXASR_FAILURE_SUMMARY) |
         (1ULL << TEXASR_TFIAR_EXACT);
-    env->spr[SPR_TFIAR] = env->nip | (msr_hv << 1) | msr_pr;
+    env->spr[SPR_TFIAR] = env->nip | (msr_hv << 1) | !!(env->msr & M_MSR_PR);
     env->spr[SPR_TFHAR] = env->nip + 4;
     env->crf[0] = 0xB; /* 0b1010 = transaction failure */
 }
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 5414fd63c1..d7b8b97ee7 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -191,12 +191,12 @@ static bool ppc_radix64_check_prot(PowerPCCPU *cpu, MMUAccessType access_type,
     }
 
     /* Determine permissions allowed by Encoded Access Authority */
-    if (!partition_scoped && (pte & R_PTE_EAA_PRIV) && msr_pr) {
+    if (!partition_scoped && (pte & R_PTE_EAA_PRIV) && (env->msr & M_MSR_PR)) {
         *prot = 0;
     } else if (mmuidx_pr(mmu_idx) || (pte & R_PTE_EAA_PRIV) ||
                partition_scoped) {
         *prot = ppc_radix64_get_prot_eaa(pte);
-    } else { /* !msr_pr && !(pte & R_PTE_EAA_PRIV) && !partition_scoped */
+    } else { /* !M_MSR_PR && !(pte & R_PTE_EAA_PRIV) && !partition_scoped */
         *prot = ppc_radix64_get_prot_eaa(pte);
         *prot &= ppc_radix64_get_prot_amr(cpu); /* Least combined permissions */
     }
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index e9c5b14c0f..fef2b11733 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -273,8 +273,8 @@ static inline void bat_size_prot(CPUPPCState *env, target_ulong *blp,
     bl = (*BATu & 0x00001FFC) << 15;
     valid = 0;
     prot = 0;
-    if (((msr_pr == 0) && (*BATu & 0x00000002)) ||
-        ((msr_pr != 0) && (*BATu & 0x00000001))) {
+    if ((!(env->msr & M_MSR_PR) && (*BATu & 0x00000002)) ||
+        ((env->msr & M_MSR_PR) && (*BATu & 0x00000001))) {
         valid = 1;
         pp = *BATl & 0x00000003;
         if (pp != 0) {
@@ -368,16 +368,17 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
     PowerPCCPU *cpu = env_archcpu(env);
     hwaddr hash;
     target_ulong vsid;
-    int ds, pr, target_page_bits;
+    int ds, target_page_bits;
+    bool pr;
     int ret;
     target_ulong sr, pgidx;
 
-    pr = msr_pr;
+    pr = env->msr & M_MSR_PR;
     ctx->eaddr = eaddr;
 
     sr = env->sr[eaddr >> 28];
-    ctx->key = (((sr & 0x20000000) && (pr != 0)) ||
-                ((sr & 0x40000000) && (pr == 0))) ? 1 : 0;
+    ctx->key = (((sr & 0x20000000) && pr) ||
+                ((sr & 0x40000000) && !pr)) ? 1 : 0;
     ds = sr & 0x80000000 ? 1 : 0;
     ctx->nx = sr & 0x10000000 ? 1 : 0;
     vsid = sr & 0x00FFFFFF;
@@ -386,8 +387,8 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
                   "Check segment v=" TARGET_FMT_lx " %d " TARGET_FMT_lx
                   " nip=" TARGET_FMT_lx " lr=" TARGET_FMT_lx
                   " ir=%d dr=%d pr=%d %d t=%d\n",
-                  eaddr, (int)(eaddr >> 28), sr, env->nip, env->lr, (int)msr_ir,
-                  (int)msr_dr, pr != 0 ? 1 : 0,
+                  eaddr, (int)(eaddr >> 28), sr, env->nip, env->lr,
+                  (int)msr_ir, (int)msr_dr, pr ? 1 : 0,
                   access_type == MMU_DATA_STORE, type);
     pgidx = (eaddr & ~SEGMENT_MASK_256M) >> target_page_bits;
     hash = vsid ^ pgidx;
@@ -530,7 +531,7 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
 
     ret = -1;
     raddr = (hwaddr)-1ULL;
-    pr = msr_pr;
+    pr = env->msr & M_MSR_PR;
     for (i = 0; i < env->nb_tlb; i++) {
         tlb = &env->tlb.tlbe[i];
         if (ppcemb_tlb_check(env, tlb, &raddr, address,
@@ -618,7 +619,7 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
 
 found_tlb:
 
-    if (msr_pr != 0) {
+    if (env->msr & M_MSR_PR) {
         prot2 = tlb->prot & 0xF;
     } else {
         prot2 = (tlb->prot >> 4) & 0xF;
@@ -768,7 +769,7 @@ static bool mmubooke206_get_as(CPUPPCState *env,
         return true;
     } else {
         *as_out = msr_ds;
-        *pr_out = msr_pr;
+        *pr_out = env->msr & M_MSR_PR;
         return false;
     }
 }
-- 
2.25.1

[PATCH 04/20] target/ppc: Substitute msr_le macro with new M_MSR_LE macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h        |  2 +-
 target/ppc/cpu_init.c   |  2 +-
 target/ppc/gdbstub.c    |  2 +-
 target/ppc/mem_helper.c | 16 ++++++++--------
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2ad023e981..d25a778b7c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
 #define M_MSR_PR (1ull << MSR_PR)
+#define M_MSR_LE (1ull << MSR_LE)
 
 /* PMU bits */
 #define MMCR0_FC     PPC_BIT(32)         /* Freeze Counters  */
@@ -484,7 +485,6 @@ typedef enum {
 #define msr_ir   ((env->msr >> MSR_IR)   & 1)
 #define msr_dr   ((env->msr >> MSR_DR)   & 1)
 #define msr_ds   ((env->msr >> MSR_DS)   & 1)
-#define msr_le   ((env->msr >> MSR_LE)   & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
 #define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 6e2b23a859..9dddc0e8f6 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7210,7 +7210,7 @@ static bool ppc_cpu_is_big_endian(CPUState *cs)
 
     cpu_synchronize_state(cs);
 
-    return !msr_le;
+    return !(env->msr & M_MSR_LE);
 }
 
 #ifdef CONFIG_TCG
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 1252429a2a..df1dcd90f0 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -95,7 +95,7 @@ static int ppc_gdb_register_len(int n)
 void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len)
 {
 #ifndef CONFIG_USER_ONLY
-    if (!msr_le) {
+    if (!(env->msr & M_MSR_LE)) {
         /* do nothing */
     } else if (len == 4) {
         bswap32s((uint32_t *)mem_buf);
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index bd219e9c9c..8ff99a6568 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -33,9 +33,9 @@
 static inline bool needs_byteswap(const CPUPPCState *env)
 {
 #if TARGET_BIG_ENDIAN
-  return msr_le;
+  return env->msr & M_MSR_LE;
 #else
-  return !msr_le;
+  return !(env->msr & M_MSR_LE);
 #endif
 }
 
@@ -470,8 +470,8 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env, target_ulong addr,
 #endif
 
 /*
- * We use msr_le to determine index ordering in a vector.  However,
- * byteswapping is not simply controlled by msr_le.  We also need to
+ * We use MSR_LE to determine index ordering in a vector.  However,
+ * byteswapping is not simply controlled by MSR_LE.  We also need to
  * take into account endianness of the target.  This is done for the
  * little-endian PPC64 user-mode target.
  */
@@ -484,7 +484,7 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env, target_ulong addr,
         int adjust = HI_IDX * (n_elems - 1);                    \
         int sh = sizeof(r->element[0]) >> 1;                    \
         int index = (addr & 0xf) >> sh;                         \
-        if (msr_le) {                                           \
+        if (env->msr & M_MSR_LE) {                              \
             index = n_elems - index - 1;                        \
         }                                                       \
                                                                 \
@@ -511,7 +511,7 @@ LVE(lvewx, cpu_ldl_data_ra, bswap32, u32)
         int adjust = HI_IDX * (n_elems - 1);                            \
         int sh = sizeof(r->element[0]) >> 1;                            \
         int index = (addr & 0xf) >> sh;                                 \
-        if (msr_le) {                                                   \
+        if (env->msr & M_MSR_LE) {                                      \
             index = n_elems - index - 1;                                \
         }                                                               \
                                                                         \
@@ -545,7 +545,7 @@ void helper_##name(CPUPPCState *env, target_ulong addr,                 \
     t.s128 = int128_zero();                                             \
     if (nb) {                                                           \
         nb = (nb >= 16) ? 16 : nb;                                      \
-        if (msr_le && !lj) {                                            \
+        if ((env->msr & M_MSR_LE) && !lj) {                             \
             for (i = 16; i > 16 - nb; i--) {                            \
                 t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC());   \
                 addr = addr_add(env, addr, 1);                          \
@@ -576,7 +576,7 @@ void helper_##name(CPUPPCState *env, target_ulong addr,           \
     }                                                             \
                                                                   \
     nb = (nb >= 16) ? 16 : nb;                                    \
-    if (msr_le && !lj) {                                          \
+    if ((env->msr & M_MSR_LE) && !lj) {                           \
         for (i = 16; i > 16 - nb; i--) {                          \
             cpu_stb_data_ra(env, addr, xt->VsrB(i - 1), GETPC()); \
             addr = addr_add(env, addr, 1);                        \
-- 
2.25.1

[PATCH 05/20] target/ppc: Substitute msr_ds macro with new M_MSR_DS macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h        | 2 +-
 target/ppc/mmu_common.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index d25a778b7c..e81f1f2d68 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
 #define M_MSR_PR (1ull << MSR_PR)
+#define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
 
 /* PMU bits */
@@ -484,7 +485,6 @@ typedef enum {
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
 #define msr_ir   ((env->msr >> MSR_IR)   & 1)
 #define msr_dr   ((env->msr >> MSR_DR)   & 1)
-#define msr_ds   ((env->msr >> MSR_DS)   & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
 #define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index fef2b11733..b7865d24b2 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -768,7 +768,7 @@ static bool mmubooke206_get_as(CPUPPCState *env,
         *pr_out = !!(epidr & EPID_EPR);
         return true;
     } else {
-        *as_out = msr_ds;
+        *as_out = env->msr & M_MSR_DS;
         *pr_out = env->msr & M_MSR_PR;
         return false;
     }
-- 
2.25.1

[PATCH 06/20] target/ppc: Substitute msr_ile macro with new M_MSR_ILE macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e81f1f2d68..95c28c3c1b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,7 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_ILE (1ull << MSR_ILE)
 #define M_MSR_PR (1ull << MSR_PR)
 #define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
@@ -476,7 +477,6 @@ typedef enum {
 #define msr_gs   ((env->msr >> MSR_GS)   & 1)
 #define msr_pow  ((env->msr >> MSR_POW)  & 1)
 #define msr_ce   ((env->msr >> MSR_CE)   & 1)
-#define msr_ile  ((env->msr >> MSR_ILE)  & 1)
 #define msr_ee   ((env->msr >> MSR_EE)   & 1)
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_me   ((env->msr >> MSR_ME)   & 1)
@@ -2677,7 +2677,7 @@ static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
     } else if (pcc->lpcr_mask & LPCR_ILE) {
         ile = !!(env->spr[SPR_LPCR] & LPCR_ILE);
     } else {
-        ile = !!(msr_ile);
+        ile = !!(env->msr & M_MSR_ILE);
     }
 
     return ile;
-- 
2.25.1

[PATCH 07/20] target/ppc: Substitute msr_ee macro with new M_MSR_EE macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         |  2 +-
 target/ppc/cpu_init.c    | 15 ++++++++++-----
 target/ppc/excp_helper.c |  2 +-
 target/ppc/kvm.c         |  3 ++-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 95c28c3c1b..bfde66ed66 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
 #define M_MSR_ILE (1ull << MSR_ILE)
+#define M_MSR_EE (1ull << MSR_EE)
 #define M_MSR_PR (1ull << MSR_PR)
 #define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
@@ -477,7 +478,6 @@ typedef enum {
 #define msr_gs   ((env->msr >> MSR_GS)   & 1)
 #define msr_pow  ((env->msr >> MSR_POW)  & 1)
 #define msr_ce   ((env->msr >> MSR_CE)   & 1)
-#define msr_ee   ((env->msr >> MSR_EE)   & 1)
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_me   ((env->msr >> MSR_ME)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 9dddc0e8f6..4d949ab1f1 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5949,7 +5949,8 @@ static bool cpu_has_work_POWER7(CPUState *cs)
         }
         return false;
     } else {
-        return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+        return (env->msr & M_MSR_EE) &&
+               (cs->interrupt_request & CPU_INTERRUPT_HARD);
     }
 }
 
@@ -6120,7 +6121,8 @@ static bool cpu_has_work_POWER8(CPUState *cs)
         }
         return false;
     } else {
-        return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+        return (env->msr & M_MSR_EE) &&
+               (cs->interrupt_request & CPU_INTERRUPT_HARD);
     }
 }
 
@@ -6337,7 +6339,8 @@ static bool cpu_has_work_POWER9(CPUState *cs)
         }
         return false;
     } else {
-        return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+        return (env->msr & M_MSR_EE) &&
+               (cs->interrupt_request & CPU_INTERRUPT_HARD);
     }
 }
 
@@ -6551,7 +6554,8 @@ static bool cpu_has_work_POWER10(CPUState *cs)
         }
         return false;
     } else {
-        return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+        return (env->msr & M_MSR_EE) &&
+               (cs->interrupt_request & CPU_INTERRUPT_HARD);
     }
 }
 
@@ -7119,7 +7123,8 @@ static bool ppc_cpu_has_work(CPUState *cs)
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
 
-    return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+    return (env->msr & M_MSR_EE) &&
+           (cs->interrupt_request & CPU_INTERRUPT_HARD);
 }
 
 static void ppc_cpu_reset(DeviceState *dev)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 10cd381be2..39d1c2a543 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1709,7 +1709,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
      * clear when coming out of some power management states (in order
      * for them to become a 0x100).
      */
-    async_deliver = (msr_ee != 0) || env->resume_as_sreset;
+    async_deliver = (env->msr & M_MSR_EE) || env->resume_as_sreset;
 
     /* Hypervisor decrementer exception */
     if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index a3c31b4e48..1ca18f21b2 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1351,7 +1351,8 @@ static int kvmppc_handle_halt(PowerPCCPU *cpu)
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
 
-    if (!(cs->interrupt_request & CPU_INTERRUPT_HARD) && (msr_ee)) {
+    if (!(cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+        (env->msr & M_MSR_EE)) {
         cs->halted = 1;
         cs->exception_index = EXCP_HLT;
     }
-- 
2.25.1

[PATCH 08/20] target/ppc: Substitute msr_ce macro with new M_MSR_CE macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         | 2 +-
 target/ppc/excp_helper.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index bfde66ed66..3f10c1f5b2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,7 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_CE (1ull << MSR_CE)
 #define M_MSR_ILE (1ull << MSR_ILE)
 #define M_MSR_EE (1ull << MSR_EE)
 #define M_MSR_PR (1ull << MSR_PR)
@@ -477,7 +478,6 @@ typedef enum {
 #define msr_cm   ((env->msr >> MSR_CM)   & 1)
 #define msr_gs   ((env->msr >> MSR_GS)   & 1)
 #define msr_pow  ((env->msr >> MSR_POW)  & 1)
-#define msr_ce   ((env->msr >> MSR_CE)   & 1)
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_me   ((env->msr >> MSR_ME)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 39d1c2a543..6dcaa79516 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1748,7 +1748,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
             return;
         }
     }
-    if (msr_ce != 0) {
+    if (env->msr & M_MSR_CE) {
         /* External critical interrupt */
         if (env->pending_interrupts & (1 << PPC_INTERRUPT_CEXT)) {
             powerpc_excp(cpu, POWERPC_EXCP_CRITICAL);
-- 
2.25.1

[PATCH 09/20] target/ppc: Substitute msr_pow macro with new M_MSR_POW macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         |  2 +-
 target/ppc/excp_helper.c | 12 ++++++------
 target/ppc/helper_regs.c |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 3f10c1f5b2..b79c00dd65 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,7 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_POW (1ull << MSR_POW)
 #define M_MSR_CE (1ull << MSR_CE)
 #define M_MSR_ILE (1ull << MSR_ILE)
 #define M_MSR_EE (1ull << MSR_EE)
@@ -477,7 +478,6 @@ typedef enum {
 #endif
 #define msr_cm   ((env->msr >> MSR_CM)   & 1)
 #define msr_gs   ((env->msr >> MSR_GS)   & 1)
-#define msr_pow  ((env->msr >> MSR_POW)  & 1)
 #define msr_fp   ((env->msr >> MSR_FP)   & 1)
 #define msr_me   ((env->msr >> MSR_ME)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 6dcaa79516..91af089d2e 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -661,7 +661,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_ITLB:      /* Instruction TLB error                    */
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
-        if (msr_pow) {
+        if (env->msr & M_MSR_POW) {
             cpu_abort(cs, "Trying to deliver power-saving system reset "
                       "exception %d with no HV support\n", excp);
         }
@@ -853,7 +853,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
-        if (msr_pow) {
+        if (env->msr & M_MSR_POW) {
             cpu_abort(cs, "Trying to deliver power-saving system reset "
                       "exception %d with no HV support\n", excp);
         }
@@ -1038,7 +1038,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
-        if (msr_pow) {
+        if (env->msr & M_MSR_POW) {
             cpu_abort(cs, "Trying to deliver power-saving system reset "
                       "exception %d with no HV support\n", excp);
         }
@@ -1248,7 +1248,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
         env->spr[SPR_BOOKE_ESR] = ESR_SPV;
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
-        if (msr_pow) {
+        if (env->msr & M_MSR_POW) {
             cpu_abort(cs, "Trying to deliver power-saving system reset "
                       "exception %d with no HV support\n", excp);
         }
@@ -1507,7 +1507,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
         /* A power-saving exception sets ME, otherwise it is unchanged */
-        if (msr_pow) {
+        if (env->msr & M_MSR_POW) {
             /* indicate that we resumed from power save mode */
             msr |= 0x10000;
             new_msr |= ((target_ulong)1 << MSR_ME);
@@ -1519,7 +1519,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
              */
             new_msr |= (target_ulong)MSR_HVB;
         } else {
-            if (msr_pow) {
+            if (env->msr & M_MSR_POW) {
                 cpu_abort(cs, "Trying to deliver power-saving system reset "
                           "exception %d with no HV support\n", excp);
             }
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 9a691d6833..a9bedfce50 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -261,7 +261,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
     env->msr = value;
     hreg_compute_hflags(env);
 #if !defined(CONFIG_USER_ONLY)
-    if (unlikely(msr_pow == 1)) {
+    if (unlikely(env->msr & M_MSR_POW)) {
         if (!env->pending_interrupts && (*env->check_pow)(env)) {
             cs->halted = 1;
             excp = EXCP_HALTED;
-- 
2.25.1

[PATCH 10/20] target/ppc: Substitute msr_me macro with new M_MSR_ME macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         |  2 +-
 target/ppc/excp_helper.c | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b79c00dd65..aa20a604ab 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -358,6 +358,7 @@ typedef enum {
 #define M_MSR_ILE (1ull << MSR_ILE)
 #define M_MSR_EE (1ull << MSR_EE)
 #define M_MSR_PR (1ull << MSR_PR)
+#define M_MSR_ME (1ull << MSR_ME)
 #define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
 
@@ -479,7 +480,6 @@ typedef enum {
 #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_me   ((env->msr >> MSR_ME)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 91af089d2e..0548b493a7 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -444,7 +444,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
         srr1 = SPR_40x_SRR3;
         break;
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
-        if (msr_me == 0) {
+        if (!(env->msr & M_MSR_ME)) {
             /*
              * Machine check exception is not enabled.  Enter
              * checkstop state.
@@ -575,7 +575,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_CRITICAL:    /* Critical input                         */
         break;
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
-        if (msr_me == 0) {
+        if (!(env->msr & M_MSR_ME)) {
             /*
              * Machine check exception is not enabled.  Enter
              * checkstop state.
@@ -748,7 +748,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
 
     switch (excp) {
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
-        if (msr_me == 0) {
+        if (!(env->msr & M_MSR_ME)) {
             /*
              * Machine check exception is not enabled.  Enter
              * checkstop state.
@@ -933,7 +933,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
 
     switch (excp) {
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
-        if (msr_me == 0) {
+        if (!(env->msr & M_MSR_ME)) {
             /*
              * Machine check exception is not enabled.  Enter
              * checkstop state.
@@ -1128,7 +1128,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
         srr1 = SPR_BOOKE_CSRR1;
         break;
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
-        if (msr_me == 0) {
+        if (!(env->msr & M_MSR_ME)) {
             /*
              * Machine check exception is not enabled.  Enter
              * checkstop state.
@@ -1366,7 +1366,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 
     switch (excp) {
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
-        if (msr_me == 0) {
+        if (!(env->msr & M_MSR_ME)) {
             /*
              * Machine check exception is not enabled.  Enter
              * checkstop state.
-- 
2.25.1

[PATCH 11/20] target/ppc: Substitute msr_gs macro with new M_MSR_GS macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 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 aa20a604ab..15f5d059a3 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,7 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_GS (1ull << MSR_GS)
 #define M_MSR_POW (1ull << MSR_POW)
 #define M_MSR_CE (1ull << MSR_CE)
 #define M_MSR_ILE (1ull << MSR_ILE)
@@ -478,7 +479,6 @@ typedef enum {
 #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 a9bedfce50..2742938abf 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 & M_MSR_GS)) {
         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..9044b7b036 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) {
+        !(env->msr & M_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 (env->msr & M_MSR_GS) {
         cpu_abort(env_cpu(env), "missing HV implementation\n");
     }
 
-- 
2.25.1

[PATCH 12/20] target/ppc: Substitute msr_fp macro with new M_MSR_FP macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         |  2 +-
 target/ppc/excp_helper.c | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 15f5d059a3..634c05a9d2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -359,6 +359,7 @@ typedef enum {
 #define M_MSR_ILE (1ull << MSR_ILE)
 #define M_MSR_EE (1ull << MSR_EE)
 #define M_MSR_PR (1ull << MSR_PR)
+#define M_MSR_FP (1ull << MSR_FP)
 #define M_MSR_ME (1ull << MSR_ME)
 #define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
@@ -479,7 +480,6 @@ typedef enum {
 #define msr_hv   (0)
 #endif
 #define msr_cm   ((env->msr >> MSR_CM)   & 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)
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0548b493a7..3e52061cd6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -478,7 +478,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -615,7 +615,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -788,7 +788,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -973,7 +973,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -1171,7 +1171,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -1434,7 +1434,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
-- 
2.25.1

[PATCH 13/20] target/ppc: Substitute msr_cm macro with new M_MSR_CM macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h        | 2 +-
 target/ppc/mmu_common.c | 2 +-
 target/ppc/mmu_helper.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 634c05a9d2..e26530fa09 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,7 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_CM (1ull << MSR_CM)
 #define M_MSR_GS (1ull << MSR_GS)
 #define M_MSR_POW (1ull << MSR_POW)
 #define M_MSR_CE (1ull << MSR_CE)
@@ -479,7 +480,6 @@ typedef enum {
 #else
 #define msr_hv   (0)
 #endif
-#define msr_cm   ((env->msr >> MSR_CM)   & 1)
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index b7865d24b2..a82649f2ff 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -692,7 +692,7 @@ int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
     hwaddr mask;
     uint32_t tlb_pid;
 
-    if (!msr_cm) {
+    if (!(env->msr & M_MSR_CM)) {
         /* In 32bit mode we can only address 32bit EAs */
         address = (uint32_t)address;
     }
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 9044b7b036..6c49920d0a 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1003,7 +1003,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
     /* Add a mask for page attributes */
     mask |= MAS2_ACM | MAS2_VLE | MAS2_W | MAS2_I | MAS2_M | MAS2_G | MAS2_E;
 
-    if (!msr_cm) {
+    if (!(env->msr & M_MSR_CM)) {
         /*
          * Executing a tlbwe instruction in 32-bit mode will set bits
          * 0:31 of the TLB EPN field to zero.
-- 
2.25.1

[PATCH 14/20] target/ppc: Substitute msr_ir macro with new M_MSR_IR macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         |  2 +-
 target/ppc/helper_regs.c |  2 +-
 target/ppc/mmu_common.c  | 11 ++++++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e26530fa09..cc0b5d72de 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -362,6 +362,7 @@ typedef enum {
 #define M_MSR_PR (1ull << MSR_PR)
 #define M_MSR_FP (1ull << MSR_FP)
 #define M_MSR_ME (1ull << MSR_ME)
+#define M_MSR_IR (1ull << MSR_IR)
 #define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
 
@@ -483,7 +484,6 @@ typedef enum {
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
-#define msr_ir   ((env->msr >> MSR_IR)   & 1)
 #define msr_dr   ((env->msr >> MSR_DR)   & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 2742938abf..fa8f213cd5 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -227,7 +227,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
         value &= ~MSR_HVB;
         value |= env->msr & MSR_HVB;
     }
-    if (((value >> MSR_IR) & 1) != msr_ir ||
+    if (!(value & env->msr & M_MSR_IR) ||
         ((value >> MSR_DR) & 1) != msr_dr) {
         cpu_interrupt_exittb(cs);
     }
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index a82649f2ff..918c15f78d 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -388,7 +388,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
                   " nip=" TARGET_FMT_lx " lr=" TARGET_FMT_lx
                   " ir=%d dr=%d pr=%d %d t=%d\n",
                   eaddr, (int)(eaddr >> 28), sr, env->nip, env->lr,
-                  (int)msr_ir, (int)msr_dr, pr ? 1 : 0,
+                  !!(env->msr & M_MSR_IR), (int)msr_dr, pr ? 1 : 0,
                   access_type == MMU_DATA_STORE, type);
     pgidx = (eaddr & ~SEGMENT_MASK_256M) >> target_page_bits;
     hash = vsid ^ pgidx;
@@ -626,7 +626,8 @@ found_tlb:
     }
 
     /* Check the address space */
-    if ((access_type == MMU_INST_FETCH ? msr_ir : msr_dr) != (tlb->attr & 1)) {
+    if ((access_type == MMU_INST_FETCH ?
+        !!(env->msr & M_MSR_IR) : msr_dr) != (tlb->attr & 1)) {
         qemu_log_mask(CPU_LOG_MMU, "%s: AS doesn't match\n", __func__);
         return -1;
     }
@@ -839,7 +840,7 @@ found_tlb:
     if (access_type == MMU_INST_FETCH) {
         /* There is no way to fetch code using epid load */
         assert(!use_epid);
-        as = msr_ir;
+        as = env->msr & M_MSR_IR;
     }
 
     if (as != ((tlb->mas1 & MAS1_TS) >> MAS1_TS_SHIFT)) {
@@ -1169,7 +1170,7 @@ int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t *ctx,
                                      int mmu_idx)
 {
     int ret = -1;
-    bool real_mode = (type == ACCESS_CODE && msr_ir == 0)
+    bool real_mode = (type == ACCESS_CODE && !(env->msr & M_MSR_IR))
         || (type != ACCESS_CODE && msr_dr == 0);
 
     switch (env->mmu_model) {
@@ -1231,7 +1232,7 @@ static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
     bool use_epid = mmubooke206_get_as(env, mmu_idx, &epid, &as, &pr);
 
     if (access_type == MMU_INST_FETCH) {
-        as = msr_ir;
+        as = env->msr & M_MSR_IR;
     }
     env->spr[SPR_BOOKE_MAS0] = env->spr[SPR_BOOKE_MAS4] & MAS4_TLBSELD_MASK;
     env->spr[SPR_BOOKE_MAS1] = env->spr[SPR_BOOKE_MAS4] & MAS4_TSIZED_MASK;
-- 
2.25.1

[PATCH 15/20] target/ppc: Substitute msr_dr macro with new M_MSR_DR macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         | 2 +-
 target/ppc/helper_regs.c | 2 +-
 target/ppc/mmu_common.c  | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index cc0b5d72de..3a5218a2cd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -363,6 +363,7 @@ typedef enum {
 #define M_MSR_FP (1ull << MSR_FP)
 #define M_MSR_ME (1ull << MSR_ME)
 #define M_MSR_IR (1ull << MSR_IR)
+#define M_MSR_DR (1ull << MSR_DR)
 #define M_MSR_DS (1ull << MSR_DS)
 #define M_MSR_LE (1ull << MSR_LE)
 
@@ -484,7 +485,6 @@ typedef enum {
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
-#define msr_dr   ((env->msr >> MSR_DR)   & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
 #define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index fa8f213cd5..1c67fbf7c1 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -228,7 +228,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
         value |= env->msr & MSR_HVB;
     }
     if (!(value & env->msr & M_MSR_IR) ||
-        ((value >> MSR_DR) & 1) != msr_dr) {
+        !(value & env->msr & M_MSR_DR)) {
         cpu_interrupt_exittb(cs);
     }
     if ((env->mmu_model == POWERPC_MMU_BOOKE ||
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 918c15f78d..dbb657d9bc 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -388,7 +388,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
                   " nip=" TARGET_FMT_lx " lr=" TARGET_FMT_lx
                   " ir=%d dr=%d pr=%d %d t=%d\n",
                   eaddr, (int)(eaddr >> 28), sr, env->nip, env->lr,
-                  !!(env->msr & M_MSR_IR), (int)msr_dr, pr ? 1 : 0,
+                  !!(env->msr & M_MSR_IR), !!(env->msr & M_MSR_DR), pr ? 1 : 0,
                   access_type == MMU_DATA_STORE, type);
     pgidx = (eaddr & ~SEGMENT_MASK_256M) >> target_page_bits;
     hash = vsid ^ pgidx;
@@ -627,7 +627,7 @@ found_tlb:
 
     /* Check the address space */
     if ((access_type == MMU_INST_FETCH ?
-        !!(env->msr & M_MSR_IR) : msr_dr) != (tlb->attr & 1)) {
+        !!(env->msr & M_MSR_IR) : !!(env->msr & M_MSR_DR)) != (tlb->attr & 1)) {
         qemu_log_mask(CPU_LOG_MMU, "%s: AS doesn't match\n", __func__);
         return -1;
     }
@@ -1171,7 +1171,7 @@ int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t *ctx,
 {
     int ret = -1;
     bool real_mode = (type == ACCESS_CODE && !(env->msr & M_MSR_IR))
-        || (type != ACCESS_CODE && msr_dr == 0);
+        || (type != ACCESS_CODE && !(env->msr & M_MSR_DR));
 
     switch (env->mmu_model) {
     case POWERPC_MMU_SOFT_6xx:
-- 
2.25.1

[PATCH 16/20] target/ppc: Substitute msr_ep macro with new M_MSR_EP macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         | 2 +-
 target/ppc/helper_regs.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 3a5218a2cd..1767a3a430 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -362,6 +362,7 @@ typedef enum {
 #define M_MSR_PR (1ull << MSR_PR)
 #define M_MSR_FP (1ull << MSR_FP)
 #define M_MSR_ME (1ull << MSR_ME)
+#define M_MSR_EP (1ull << MSR_EP)
 #define M_MSR_IR (1ull << MSR_IR)
 #define M_MSR_DR (1ull << MSR_DR)
 #define M_MSR_DS (1ull << MSR_DS)
@@ -484,7 +485,6 @@ typedef enum {
 #endif
 #define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
 #define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
-#define msr_ep   ((env->msr >> MSR_EP)   & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
 #define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 1c67fbf7c1..f9d2e123cf 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -241,8 +241,8 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
         /* Swap temporary saved registers with GPRs */
         hreg_swap_gpr_tgpr(env);
     }
-    if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
-        env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
+    if (unlikely(!(value & env->msr & M_MSR_EP))) {
+        env->excp_prefix = !!(value & M_MSR_EP) * 0xFFF00000;
     }
     /*
      * If PR=1 then EE, IR and DR must be 1
-- 
2.25.1

[PATCH 17/20] target/ppc: Substitute msr_fe macro with new M_MSR_FE macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         |  5 +++--
 target/ppc/excp_helper.c | 12 ++++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 1767a3a430..b957fc95e0 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -362,6 +362,9 @@ typedef enum {
 #define M_MSR_PR (1ull << MSR_PR)
 #define M_MSR_FP (1ull << MSR_FP)
 #define M_MSR_ME (1ull << MSR_ME)
+#define M_MSR_FE0 (1ull << MSR_FE0)
+#define M_MSR_FE1 (1ull << MSR_FE1)
+#define M_MSR_FE (M_MSR_FE0 | M_MSR_FE1)
 #define M_MSR_EP (1ull << MSR_EP)
 #define M_MSR_IR (1ull << MSR_IR)
 #define M_MSR_DR (1ull << MSR_DR)
@@ -483,8 +486,6 @@ typedef enum {
 #else
 #define msr_hv   (0)
 #endif
-#define msr_fe0  ((env->msr >> MSR_FE0)  & 1)
-#define msr_fe1  ((env->msr >> MSR_FE1)  & 1)
 #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
 #define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 3e52061cd6..88e5eb91f1 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -478,7 +478,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
+            if (!(env->msr & M_MSR_FE) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -615,7 +615,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
+            if (!(env->msr & M_MSR_FE) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -788,7 +788,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
+            if (!(env->msr & M_MSR_FE) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -973,7 +973,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
+            if (!(env->msr & M_MSR_FE) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -1171,7 +1171,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
+            if (!(env->msr & M_MSR_FE) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
@@ -1434,7 +1434,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
-            if ((msr_fe0 == 0 && msr_fe1 == 0) || !(env->msr & M_MSR_FP)) {
+            if (!(env->msr & M_MSR_FE) || !(env->msr & M_MSR_FP)) {
                 trace_ppc_excp_fp_ignore();
                 powerpc_reset_excp_state(cpu);
                 return;
-- 
2.25.1

[PATCH 18/20] target/ppc: Substitute msr_ts macro with new M_MSR_TS macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h     | 4 +++-
 target/ppc/kvm.c     | 4 ++--
 target/ppc/machine.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b957fc95e0..ee00b27818 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,9 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_TS0 (1ull << MSR_TS0)
+#define M_MSR_TS1 (1ull << MSR_TS1)
+#define M_MSR_TS (M_MSR_TS0 | M_MSR_TS1)
 #define M_MSR_CM (1ull << MSR_CM)
 #define M_MSR_GS (1ull << MSR_GS)
 #define M_MSR_POW (1ull << MSR_POW)
@@ -486,7 +489,6 @@ typedef enum {
 #else
 #define msr_hv   (0)
 #endif
-#define msr_ts   ((env->msr >> MSR_TS1)  & 3)
 
 #define DBCR0_ICMP (1 << 27)
 #define DBCR0_BRT (1 << 26)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 1ca18f21b2..3cccac41fd 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -973,7 +973,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         }
 
 #ifdef TARGET_PPC64
-        if (msr_ts) {
+        if (env->msr & M_MSR_TS) {
             for (i = 0; i < ARRAY_SIZE(env->tm_gpr); i++) {
                 kvm_set_one_reg(cs, KVM_REG_PPC_TM_GPR(i), &env->tm_gpr[i]);
             }
@@ -1281,7 +1281,7 @@ int kvm_arch_get_registers(CPUState *cs)
         }
 
 #ifdef TARGET_PPC64
-        if (msr_ts) {
+        if (env->msr & M_MSR_TS) {
             for (i = 0; i < ARRAY_SIZE(env->tm_gpr); i++) {
                 kvm_get_one_reg(cs, KVM_REG_PPC_TM_GPR(i), &env->tm_gpr[i]);
             }
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index e673944597..51832c4bde 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -417,7 +417,7 @@ static bool tm_needed(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
     CPUPPCState *env = &cpu->env;
-    return msr_ts;
+    return env->msr & M_MSR_TS;
 }
 
 static const VMStateDescription vmstate_tm = {
-- 
2.25.1

[PATCH 19/20] target/ppc: Substitute msr_hv macro with new M_MSR_HV macro

[Víctor Colombo]

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h         | 11 +++++------
 target/ppc/cpu_init.c    |  4 ++--
 target/ppc/excp_helper.c |  9 +++++----
 target/ppc/mem_helper.c  |  5 +++--
 target/ppc/misc_helper.c |  2 +-
 target/ppc/mmu-radix64.c |  6 +++---
 6 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index ee00b27818..3cbecc96d8 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,11 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#if defined(TARGET_PPC64)
+#define M_MSR_HV (1ull << MSR_HV)
+#else
+#define M_MSR_HV 0
+#endif
 #define M_MSR_TS0 (1ull << MSR_TS0)
 #define M_MSR_TS1 (1ull << MSR_TS1)
 #define M_MSR_TS (M_MSR_TS0 | M_MSR_TS1)
@@ -484,12 +489,6 @@ typedef enum {
 #define HFSCR_MSGP     PPC_BIT(53) /* Privileged Message Send Facilities */
 #define HFSCR_IC_MSGP  0xA
 
-#if defined(TARGET_PPC64)
-#define msr_hv   ((env->msr >> MSR_HV)   & 1)
-#else
-#define msr_hv   (0)
-#endif
-
 #define DBCR0_ICMP (1 << 27)
 #define DBCR0_BRT (1 << 26)
 #define DBSR_ICMP (1 << 27)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 4d949ab1f1..d2a7911a86 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6305,7 +6305,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
         if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
             (env->spr[SPR_LPCR] & LPCR_EEE)) {
             bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-            if (!heic || !msr_hv || (env->msr & M_MSR_PR)) {
+            if (!heic || !(env->msr & M_MSR_HV) || (env->msr & M_MSR_PR)) {
                 return true;
             }
         }
@@ -6520,7 +6520,7 @@ static bool cpu_has_work_POWER10(CPUState *cs)
         if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
             (env->spr[SPR_LPCR] & LPCR_EEE)) {
             bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-            if (!heic || !msr_hv || (env->msr & M_MSR_PR)) {
+            if (!heic || !(env->msr & M_MSR_HV) || (env->msr & M_MSR_PR)) {
                 return true;
             }
         }
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 88e5eb91f1..06bee7689b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1715,7 +1715,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
     if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
         /* LPCR will be clear when not supported so this will work */
         bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
-        if ((async_deliver || msr_hv == 0) && hdice) {
+        if ((async_deliver || !(env->msr & M_MSR_HV)) && hdice) {
             /* HDEC clears on delivery */
             env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
             powerpc_excp(cpu, POWERPC_EXCP_HDECR);
@@ -1727,7 +1727,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
     if (env->pending_interrupts & (1 << PPC_INTERRUPT_HVIRT)) {
         /* LPCR will be clear when not supported so this will work */
         bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
-        if ((async_deliver || msr_hv == 0) && hvice) {
+        if ((async_deliver || !(env->msr & M_MSR_HV)) && hvice) {
             powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
             return;
         }
@@ -1738,8 +1738,9 @@ static void ppc_hw_interrupt(CPUPPCState *env)
         bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
         bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
         /* HEIC blocks delivery to the hypervisor */
-        if ((async_deliver && !(heic && msr_hv && !(env->msr & M_MSR_PR))) ||
-            (env->has_hv_mode && msr_hv == 0 && !lpes0)) {
+        if ((async_deliver && !(heic && (env->msr & M_MSR_HV) &&
+            !(env->msr & M_MSR_PR))) ||
+            (env->has_hv_mode && !(env->msr & M_MSR_HV) && !lpes0)) {
             if (books_vhyp_promotes_external_to_hvirt(cpu)) {
                 powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
             } else {
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 8ff99a6568..d3964a7eb0 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -612,11 +612,12 @@ void helper_tbegin(CPUPPCState *env)
     env->spr[SPR_TEXASR] =
         (1ULL << TEXASR_FAILURE_PERSISTENT) |
         (1ULL << TEXASR_NESTING_OVERFLOW) |
-        (msr_hv << TEXASR_PRIVILEGE_HV) |
+        (!!(env->msr & M_MSR_HV) << TEXASR_PRIVILEGE_HV) |
         (!!(env->msr & M_MSR_PR) << TEXASR_PRIVILEGE_PR) |
         (1ULL << TEXASR_FAILURE_SUMMARY) |
         (1ULL << TEXASR_TFIAR_EXACT);
-    env->spr[SPR_TFIAR] = env->nip | (msr_hv << 1) | !!(env->msr & M_MSR_PR);
+    env->spr[SPR_TFIAR] = env->nip | (!!(env->msr & M_MSR_HV) << 1) |
+                          !!(env->msr & M_MSR_PR);
     env->spr[SPR_TFHAR] = env->nip + 4;
     env->crf[0] = 0xB; /* 0b1010 = transaction failure */
 }
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 06aa716cab..90b97abb07 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -73,7 +73,7 @@ void helper_hfscr_facility_check(CPUPPCState *env, uint32_t bit,
                                  const char *caller, uint32_t cause)
 {
 #ifdef TARGET_PPC64
-    if ((env->msr_mask & MSR_HVB) && !msr_hv &&
+    if ((env->msr_mask & MSR_HVB) && !(env->msr & M_MSR_HV) &&
                                      !(env->spr[SPR_HFSCR] & (1UL << bit))) {
         raise_hv_fu_exception(env, bit, caller, cause, GETPC());
     }
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index d7b8b97ee7..09b202071d 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -37,7 +37,7 @@ static bool ppc_radix64_get_fully_qualified_addr(const CPUPPCState *env,
         return false;
     }
 
-    if (msr_hv) { /* MSR[HV] -> Hypervisor/bare metal */
+    if (env->msr & M_MSR_HV) { /* MSR[HV] -> Hypervisor/bare metal */
         switch (eaddr & R_EADDR_QUADRANT) {
         case R_EADDR_QUADRANT0:
             *lpid = 0;
@@ -305,7 +305,7 @@ static bool validate_pate(PowerPCCPU *cpu, uint64_t lpid, ppc_v3_pate_t *pate)
     if (!(pate->dw0 & PATE0_HR)) {
         return false;
     }
-    if (lpid == 0 && !msr_hv) {
+    if (lpid == 0 && !(env->msr & M_MSR_HV)) {
         return false;
     }
     if ((pate->dw0 & PATE1_R_PRTS) < 5) {
@@ -430,7 +430,7 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
     *g_page_size = PRTBE_R_GET_RTS(prtbe0);
     base_addr = prtbe0 & PRTBE_R_RPDB;
     nls = prtbe0 & PRTBE_R_RPDS;
-    if (msr_hv || vhyp_flat_addressing(cpu)) {
+    if ((env->msr & M_MSR_HV) || vhyp_flat_addressing(cpu)) {
         /*
          * Can treat process table addresses as real addresses
          */
-- 
2.25.1

[PATCH 20/20] target/ppc: Add unused M_MSR_* macros

[Víctor Colombo]

Add M_MSR_* macros for msr bits that had an unused msr_* before.

Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
---
 target/ppc/cpu.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 3cbecc96d8..dda289a121 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,9 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define M_MSR_SF (1ull << MSR_SF)
+#define M_MSR_TAG (1ull << MSR_TAG)
+#define M_MSR_ISF (1ull << MSR_ISF)
 #if defined(TARGET_PPC64)
 #define M_MSR_HV (1ull << MSR_HV)
 #else
@@ -360,10 +363,20 @@ typedef enum {
 #endif
 #define M_MSR_TS0 (1ull << MSR_TS0)
 #define M_MSR_TS1 (1ull << MSR_TS1)
+#define M_MSR_TM (1ull << MSR_TM)
 #define M_MSR_TS (M_MSR_TS0 | M_MSR_TS1)
 #define M_MSR_CM (1ull << MSR_CM)
+#define M_MSR_ICM (1ull << MSR_ICM)
 #define M_MSR_GS (1ull << MSR_GS)
+#define M_MSR_UCLE (1ull << MSR_UCLE)
+#define M_MSR_VR (1ull << MSR_VR)
+#define M_MSR_SPE (1ull << MSR_SPE)
+#define M_MSR_VSX (1ull << MSR_VSX)
+#define M_MSR_S (1ull << MSR_S)
+#define M_MSR_KEY (1ull << MSR_KEY)
 #define M_MSR_POW (1ull << MSR_POW)
+#define M_MSR_WE (1ull << MSR_WE)
+#define M_MSR_TGPR (1ull << MSR_TGPR)
 #define M_MSR_CE (1ull << MSR_CE)
 #define M_MSR_ILE (1ull << MSR_ILE)
 #define M_MSR_EE (1ull << MSR_EE)
@@ -373,10 +386,21 @@ typedef enum {
 #define M_MSR_FE0 (1ull << MSR_FE0)
 #define M_MSR_FE1 (1ull << MSR_FE1)
 #define M_MSR_FE (M_MSR_FE0 | M_MSR_FE1)
+#define M_MSR_SE (1ull << MSR_SE)
+#define M_MSR_DWE (1ull << MSR_DWE)
+#define M_MSR_UBLE (1ull << MSR_UBLE)
+#define M_MSR_BE (1ull << MSR_BE)
+#define M_MSR_DE (1ull << MSR_DE)
+#define M_MSR_AL (1ull << MSR_AL)
 #define M_MSR_EP (1ull << MSR_EP)
 #define M_MSR_IR (1ull << MSR_IR)
 #define M_MSR_DR (1ull << MSR_DR)
+#define M_MSR_IS (1ull << MSR_IS)
 #define M_MSR_DS (1ull << MSR_DS)
+#define M_MSR_PE (1ull << MSR_PE)
+#define M_MSR_PX (1ull << MSR_PX)
+#define M_MSR_PMM (1ull << MSR_PMM)
+#define M_MSR_RI (1ull << MSR_RI)
 #define M_MSR_LE (1ull << MSR_LE)
 
 /* PMU bits */
-- 
2.25.1

Re: [PATCH 04/20] target/ppc: Substitute msr_le macro with new M_MSR_LE macro

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 6253 bytes --]

On Fri, 22 Apr 2022, Víctor Colombo wrote:
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
> ---
> target/ppc/cpu.h        |  2 +-
> target/ppc/cpu_init.c   |  2 +-
> target/ppc/gdbstub.c    |  2 +-
> target/ppc/mem_helper.c | 16 ++++++++--------
> 4 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 2ad023e981..d25a778b7c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -354,6 +354,7 @@ typedef enum {
> #define MSR_LE   0  /* Little-endian mode                           1 hflags */
>
> #define M_MSR_PR (1ull << MSR_PR)
> +#define M_MSR_LE (1ull << MSR_LE)

We have BIT(nr) and BIT_ULL(nr) macros for this. Would it be simpler to 
use that instead of adding another set of defines? (Both ways are somewhat 
comfusing and needs care when to use which so I'm not sure the additional 
define makes it simpler or not.) We even have PPC_BIT(bit) macro which 
follows the numbering used in the docs (which is big endian/backwards) but 
these bits are not numbered that way here. Is it worth fixing it to match 
docs? That might cause trouble elsewhere where current defines are used 
though or in case of FPSCR the numbering is not even the same in docs for 
32 bit and 64 bit so at least in that case keeping the current numbering 
may be simpler.

Regards,
BALATON Zoltan

> /* PMU bits */
> #define MMCR0_FC     PPC_BIT(32)         /* Freeze Counters  */
> @@ -484,7 +485,6 @@ typedef enum {
> #define msr_ir   ((env->msr >> MSR_IR)   & 1)
> #define msr_dr   ((env->msr >> MSR_DR)   & 1)
> #define msr_ds   ((env->msr >> MSR_DS)   & 1)
> -#define msr_le   ((env->msr >> MSR_LE)   & 1)
> #define msr_ts   ((env->msr >> MSR_TS1)  & 3)
>
> #define DBCR0_ICMP (1 << 27)
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 6e2b23a859..9dddc0e8f6 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7210,7 +7210,7 @@ static bool ppc_cpu_is_big_endian(CPUState *cs)
>
>     cpu_synchronize_state(cs);
>
> -    return !msr_le;
> +    return !(env->msr & M_MSR_LE);
> }
>
> #ifdef CONFIG_TCG
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index 1252429a2a..df1dcd90f0 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -95,7 +95,7 @@ static int ppc_gdb_register_len(int n)
> void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len)
> {
> #ifndef CONFIG_USER_ONLY
> -    if (!msr_le) {
> +    if (!(env->msr & M_MSR_LE)) {
>         /* do nothing */
>     } else if (len == 4) {
>         bswap32s((uint32_t *)mem_buf);
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index bd219e9c9c..8ff99a6568 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -33,9 +33,9 @@
> static inline bool needs_byteswap(const CPUPPCState *env)
> {
> #if TARGET_BIG_ENDIAN
> -  return msr_le;
> +  return env->msr & M_MSR_LE;
> #else
> -  return !msr_le;
> +  return !(env->msr & M_MSR_LE);
> #endif
> }
>
> @@ -470,8 +470,8 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env, target_ulong addr,
> #endif
>
> /*
> - * We use msr_le to determine index ordering in a vector.  However,
> - * byteswapping is not simply controlled by msr_le.  We also need to
> + * We use MSR_LE to determine index ordering in a vector.  However,
> + * byteswapping is not simply controlled by MSR_LE.  We also need to
>  * take into account endianness of the target.  This is done for the
>  * little-endian PPC64 user-mode target.
>  */
> @@ -484,7 +484,7 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env, target_ulong addr,
>         int adjust = HI_IDX * (n_elems - 1);                    \
>         int sh = sizeof(r->element[0]) >> 1;                    \
>         int index = (addr & 0xf) >> sh;                         \
> -        if (msr_le) {                                           \
> +        if (env->msr & M_MSR_LE) {                              \
>             index = n_elems - index - 1;                        \
>         }                                                       \
>                                                                 \
> @@ -511,7 +511,7 @@ LVE(lvewx, cpu_ldl_data_ra, bswap32, u32)
>         int adjust = HI_IDX * (n_elems - 1);                            \
>         int sh = sizeof(r->element[0]) >> 1;                            \
>         int index = (addr & 0xf) >> sh;                                 \
> -        if (msr_le) {                                                   \
> +        if (env->msr & M_MSR_LE) {                                      \
>             index = n_elems - index - 1;                                \
>         }                                                               \
>                                                                         \
> @@ -545,7 +545,7 @@ void helper_##name(CPUPPCState *env, target_ulong addr,                 \
>     t.s128 = int128_zero();                                             \
>     if (nb) {                                                           \
>         nb = (nb >= 16) ? 16 : nb;                                      \
> -        if (msr_le && !lj) {                                            \
> +        if ((env->msr & M_MSR_LE) && !lj) {                             \
>             for (i = 16; i > 16 - nb; i--) {                            \
>                 t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC());   \
>                 addr = addr_add(env, addr, 1);                          \
> @@ -576,7 +576,7 @@ void helper_##name(CPUPPCState *env, target_ulong addr,           \
>     }                                                             \
>                                                                   \
>     nb = (nb >= 16) ? 16 : nb;                                    \
> -    if (msr_le && !lj) {                                          \
> +    if ((env->msr & M_MSR_LE) && !lj) {                           \
>         for (i = 16; i > 16 - nb; i--) {                          \
>             cpu_stb_data_ra(env, addr, xt->VsrB(i - 1), GETPC()); \
>             addr = addr_add(env, addr, 1);                        \
>

Re: [PATCH 00/20] target/ppc: Remove hidden usages of *env

[Richard Henderson]

On 4/22/22 11:54, Víctor Colombo wrote:
> By running the grep command `git grep -nr 'define \(fpscr\|msr\)_[a-z0-9]\+\>'`
> we can find multiple macros that use `env->fpscr` and `env->msr` but doesn't
> take *env as a parameter.
> 
> Richard Henderson said [1] that these macros hiding the usage of *env "are evil".
> This patch series remove them and substitute with an explicit usage of *env by
> adding macros in the same style of FP_* ones (e.g. FP_FI defined in cpu.h).
> 
> Patch 20 (target/ppc: Add unused M_MSR_* macros) implements unused macros, the
> same that were removed in patch 02 (target/ppc: Remove unused msr_* macros). I
> did that to keep the changes consistent with what was already present before.

Oh frabjous day! Callooh! Callay!


r~

Re: [PATCH 01/20] target/ppc: Remove fpscr_* macros from cpu.h

[Richard Henderson]

On 4/22/22 11:54, Víctor Colombo wrote:
> fpscr_* defined macros are hiding the usage of *env behind them.
> Substitute the usage of these macros with `env->fpscr & FP_*` to make
> the code cleaner.
> 
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br>
> ---
>   target/ppc/cpu.c        |  2 +-
>   target/ppc/cpu.h        | 29 -----------------------------
>   target/ppc/fpu_helper.c | 28 ++++++++++++++--------------
>   3 files changed, 15 insertions(+), 44 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 02/20] target/ppc: Remove unused msr_* macros

[Richard Henderson]

On 4/22/22 11:54, Víctor Colombo wrote:
> Some msr_* macros are not used anywhere. Remove them as part of
> the work to remove all hidden usage of *env.
> 
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: Víctor Colombo<victor.colombo@eldorado.org.br>
> ---
>   target/ppc/cpu.h | 21 ---------------------
>   1 file changed, 21 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[Richard Henderson]

On 4/22/22 11:54, Víctor Colombo wrote:
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
> ---
>   hw/ppc/pegasos2.c        |  2 +-
>   hw/ppc/spapr.c           |  2 +-
>   target/ppc/cpu.h         |  3 ++-
>   target/ppc/cpu_init.c    |  4 ++--
>   target/ppc/excp_helper.c |  6 +++---
>   target/ppc/mem_helper.c  |  4 ++--
>   target/ppc/mmu-radix64.c |  4 ++--
>   target/ppc/mmu_common.c  | 23 ++++++++++++-----------
>   8 files changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index 56bf203dfd..27ed54a71d 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -461,7 +461,7 @@ static void pegasos2_hypercall(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
>       /* The TCG path should also be holding the BQL at this point */
>       g_assert(qemu_mutex_iothread_locked());
>   
> -    if (msr_pr) {
> +    if (env->msr & M_MSR_PR) {

I'm not sure I'm keen on the M_ prefix, but I'll defer to Cedric or Daniel if they're ok 
with it.

In general there are inconsistencies with the use of MSR_PR (1 vs 1ull), which makes it 
tempting to replace MSR_PR the bit number with MSR_PR the mask and leave off the M_ 
prefix.  It's somewhat easy for MSR_PR, since missed conversions will certainly result in 
compiler warnings for out-of-range shift (the same would not be true with bits 0-6, LE 
through EP).

Another possibility would be to use hw/registerfields.h.  Missed conversions are missing 
symbol errors.  You'd write FIELD_EX64(env->msr, MSR, PR) in cases like this and 
R_MSR_PR_MASK in cases like cpu_init.c.  It's more verbose for single bits like this, but 
much easier to work with multi-bit fields like MSR.TS.


r~

Re: [PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[Víctor Colombo]

Hello everyone! Thanks Zoltan and Richard for your kind reviews!

On 26/04/2022 18:29, Richard Henderson wrote:
> On 4/22/22 11:54, Víctor Colombo wrote:
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
>> ---
>>   hw/ppc/pegasos2.c        |  2 +-
>>   hw/ppc/spapr.c           |  2 +-
>>   target/ppc/cpu.h         |  3 ++-
>>   target/ppc/cpu_init.c    |  4 ++--
>>   target/ppc/excp_helper.c |  6 +++---
>>   target/ppc/mem_helper.c  |  4 ++--
>>   target/ppc/mmu-radix64.c |  4 ++--
>>   target/ppc/mmu_common.c  | 23 ++++++++++++-----------
>>   8 files changed, 25 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index 56bf203dfd..27ed54a71d 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -461,7 +461,7 @@ static void 
>> pegasos2_hypercall(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
>>       /* The TCG path should also be holding the BQL at this point */
>>       g_assert(qemu_mutex_iothread_locked());
>>
>> -    if (msr_pr) {
>> +    if (env->msr & M_MSR_PR) {
> 
> I'm not sure I'm keen on the M_ prefix, but I'll defer to Cedric or 
> Daniel if they're ok
> with it.
> 
> In general there are inconsistencies with the use of MSR_PR (1 vs 1ull), 
> which makes it
> tempting to replace MSR_PR the bit number with MSR_PR the mask and leave 
> off the M_
> prefix.  It's somewhat easy for MSR_PR, since missed conversions will 
> certainly result in
> compiler warnings for out-of-range shift (the same would not be true 
> with bits 0-6, LE
> through EP). >
> Another possibility would be to use hw/registerfields.h.  Missed 
> conversions are missing
> symbol errors.  You'd write FIELD_EX64(env->msr, MSR, PR) in cases like 
> this and
> R_MSR_PR_MASK in cases like cpu_init.c.  It's more verbose for single 
> bits like this, but
> much easier to work with multi-bit fields like MSR.TS.
> 
Thanks for your ideas! I think I'll go with this second one. It'll allow
to remove the !!(val) occurrences that I introduced in this series, so
the code quality will probably be improved.

It'll also be a 'safer' change that will require less rework on other
parts that I didn't intend to touch in this patch series.

I''l work on it!
> 
> r~

--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[Cédric Le Goater]

On 4/27/22 19:00, Víctor Colombo wrote:
> Hello everyone! Thanks Zoltan and Richard for your kind reviews!
> 
> On 26/04/2022 18:29, Richard Henderson wrote:
>> On 4/22/22 11:54, Víctor Colombo wrote:
>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
>>> ---
>>>   hw/ppc/pegasos2.c        |  2 +-
>>>   hw/ppc/spapr.c           |  2 +-
>>>   target/ppc/cpu.h         |  3 ++-
>>>   target/ppc/cpu_init.c    |  4 ++--
>>>   target/ppc/excp_helper.c |  6 +++---
>>>   target/ppc/mem_helper.c  |  4 ++--
>>>   target/ppc/mmu-radix64.c |  4 ++--
>>>   target/ppc/mmu_common.c  | 23 ++++++++++++-----------
>>>   8 files changed, 25 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>>> index 56bf203dfd..27ed54a71d 100644
>>> --- a/hw/ppc/pegasos2.c
>>> +++ b/hw/ppc/pegasos2.c
>>> @@ -461,7 +461,7 @@ static void pegasos2_hypercall(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
>>>       /* The TCG path should also be holding the BQL at this point */
>>>       g_assert(qemu_mutex_iothread_locked());
>>>
>>> -    if (msr_pr) {
>>> +    if (env->msr & M_MSR_PR) {
>>
>> I'm not sure I'm keen on the M_ prefix, but I'll defer to Cedric or Daniel if they're ok
>> with it.
>>
>> In general there are inconsistencies with the use of MSR_PR (1 vs 1ull), which makes it
>> tempting to replace MSR_PR the bit number with MSR_PR the mask and leave off the M_
>> prefix.  It's somewhat easy for MSR_PR, since missed conversions will certainly result in
>> compiler warnings for out-of-range shift (the same would not be true with bits 0-6, LE
>> through EP). >
>> Another possibility would be to use hw/registerfields.h.  Missed conversions are missing
>> symbol errors.  You'd write FIELD_EX64(env->msr, MSR, PR) in cases like this and
>> R_MSR_PR_MASK in cases like cpu_init.c.  It's more verbose for single bits like this, but
>> much easier to work with multi-bit fields like MSR.TS.
>>
> Thanks for your ideas! I think I'll go with this second one. It'll allow
> to remove the !!(val) occurrences that I introduced in this series, so
> the code quality will probably be improved.
> 
> It'll also be a 'safer' change that will require less rework on other
> parts that I didn't intend to touch in this patch series.


The registerfield API is certainly a good choice for cleanups.

Is there a way to adapt the PPC_BIT macros and keep the PPC bit
ordering ? It might be easier to forget about it. Which is what
the Linux kernel does in many places.


Device models are also impacted :

   include/hw/pci-host/pnv_phb*_regs.h
   include/hw/ppc/xive*_regs.h

Something I have been wanting to change for a while are these macros :

     static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
     {
         return (word & mask) >> ctz64(mask);
     }
     
     static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
                                     uint64_t value)
     {
         return (word & ~mask) | ((value << ctz64(mask)) & mask);
     }

Thanks,

C.
     

Re: [PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[Víctor Colombo]

On 28/04/2022 03:46, Cédric Le Goater wrote:
> On 4/27/22 19:00, Víctor Colombo wrote:
>> Hello everyone! Thanks Zoltan and Richard for your kind reviews!
>>
>> On 26/04/2022 18:29, Richard Henderson wrote:
>>> On 4/22/22 11:54, Víctor Colombo wrote:
>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
>>>> ---
>>>>   hw/ppc/pegasos2.c        |  2 +-
>>>>   hw/ppc/spapr.c           |  2 +-
>>>>   target/ppc/cpu.h         |  3 ++-
>>>>   target/ppc/cpu_init.c    |  4 ++--
>>>>   target/ppc/excp_helper.c |  6 +++---
>>>>   target/ppc/mem_helper.c  |  4 ++--
>>>>   target/ppc/mmu-radix64.c |  4 ++--
>>>>   target/ppc/mmu_common.c  | 23 ++++++++++++-----------
>>>>   8 files changed, 25 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>>>> index 56bf203dfd..27ed54a71d 100644
>>>> --- a/hw/ppc/pegasos2.c
>>>> +++ b/hw/ppc/pegasos2.c
>>>> @@ -461,7 +461,7 @@ static void 
>>>> pegasos2_hypercall(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
>>>>       /* The TCG path should also be holding the BQL at this point */
>>>>       g_assert(qemu_mutex_iothread_locked());
>>>>
>>>> -    if (msr_pr) {
>>>> +    if (env->msr & M_MSR_PR) {
>>>
>>> I'm not sure I'm keen on the M_ prefix, but I'll defer to Cedric or 
>>> Daniel if they're ok
>>> with it.
>>>
>>> In general there are inconsistencies with the use of MSR_PR (1 vs 
>>> 1ull), which makes it
>>> tempting to replace MSR_PR the bit number with MSR_PR the mask and 
>>> leave off the M_
>>> prefix.  It's somewhat easy for MSR_PR, since missed conversions will 
>>> certainly result in
>>> compiler warnings for out-of-range shift (the same would not be true 
>>> with bits 0-6, LE
>>> through EP). >
>>> Another possibility would be to use hw/registerfields.h.  Missed 
>>> conversions are missing
>>> symbol errors.  You'd write FIELD_EX64(env->msr, MSR, PR) in cases 
>>> like this and
>>> R_MSR_PR_MASK in cases like cpu_init.c.  It's more verbose for single 
>>> bits like this, but
>>> much easier to work with multi-bit fields like MSR.TS.
>>>
>> Thanks for your ideas! I think I'll go with this second one. It'll allow
>> to remove the !!(val) occurrences that I introduced in this series, so
>> the code quality will probably be improved.
>>
>> It'll also be a 'safer' change that will require less rework on other
>> parts that I didn't intend to touch in this patch series.
> 
> 
> The registerfield API is certainly a good choice for cleanups.
> 
> Is there a way to adapt the PPC_BIT macros and keep the PPC bit
> ordering ? It might be easier to forget about it. Which is what
> the Linux kernel does in many places.

Hello Cédric.

It would probably be easier to change this if we went with Zoltan's
idea. Just 'invert' the MSR_* values to match the ISA order and use
env->msr & PPC_BIT(MSR_*). However registerfield API expects it to be
in the "0 is the rightmost" order,
so we can't easily go with it and just invert the MSR_* values.

A solution I could think that might be easy is: rename PPC_BIT to
PPC_BIT_ULL (behaves like BIT_ULL but 'inverted'), and create a new
PPC_BIT macro that just inverts the bit value

#define PPC_BIT_ULL(bit) (0x8000000000000000ULL >> (bit))
#define PPC_BIT(bit) (63 - (bit))

and change MSR_* to use it

#define MSR_LE PPC_BIT(63)

> 
> 
> Device models are also impacted :
> 
>    include/hw/pci-host/pnv_phb*_regs.h
>    include/hw/ppc/xive*_regs.h
> 
> Something I have been wanting to change for a while are these macros :
> 
>      static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
>      {
>          return (word & mask) >> ctz64(mask);
>      }
> 
>      static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
>                                      uint64_t value)
>      {
>          return (word & ~mask) | ((value << ctz64(mask)) & mask);
>      }
> 
> Thanks,
> 
> C.
> 

Thanks!

--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[Richard Henderson]

On 4/28/22 07:56, Víctor Colombo wrote:
> A solution I could think that might be easy is: rename PPC_BIT to
> PPC_BIT_ULL (behaves like BIT_ULL but 'inverted'), and create a new
> PPC_BIT macro that just inverts the bit value
> 
> #define PPC_BIT_ULL(bit) (0x8000000000000000ULL >> (bit))
> #define PPC_BIT(bit) (63 - (bit))

There's also room for a big-endian set of registerfield macros.
(Don't forget s390x does the same thing, so "PPC" isn't appropriate generically.)

Something like

#define BE_FIELD_W(reg, field, regwidth, start, length) \
     FIELD(reg, field, regwidth - start - length, length)
#define BE_FIELD32(reg, field, start, length) \
     BE_FIELD_(reg, field, 32, start, length)
#define BE_FIELD64(reg, field, start, length) \
     BE_FIELD_(reg, field, 64, start, length)

at which point the usual FIELD_EX* and FIELD_DP* macros will work.


r~

Re: [PATCH 03/20] target/ppc: Substitute msr_pr macro with new M_MSR_PR macro

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 5141 bytes --]

On Thu, 28 Apr 2022, Víctor Colombo wrote:
> On 28/04/2022 03:46, Cédric Le Goater wrote:
>> On 4/27/22 19:00, Víctor Colombo wrote:
>>> Hello everyone! Thanks Zoltan and Richard for your kind reviews!
>>> 
>>> On 26/04/2022 18:29, Richard Henderson wrote:
>>>> On 4/22/22 11:54, Víctor Colombo wrote:
>>>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
>>>>> ---
>>>>>   hw/ppc/pegasos2.c        |  2 +-
>>>>>   hw/ppc/spapr.c           |  2 +-
>>>>>   target/ppc/cpu.h         |  3 ++-
>>>>>   target/ppc/cpu_init.c    |  4 ++--
>>>>>   target/ppc/excp_helper.c |  6 +++---
>>>>>   target/ppc/mem_helper.c  |  4 ++--
>>>>>   target/ppc/mmu-radix64.c |  4 ++--
>>>>>   target/ppc/mmu_common.c  | 23 ++++++++++++-----------
>>>>>   8 files changed, 25 insertions(+), 23 deletions(-)
>>>>> 
>>>>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>>>>> index 56bf203dfd..27ed54a71d 100644
>>>>> --- a/hw/ppc/pegasos2.c
>>>>> +++ b/hw/ppc/pegasos2.c
>>>>> @@ -461,7 +461,7 @@ static void pegasos2_hypercall(PPCVirtualHypervisor 
>>>>> *vhyp, PowerPCCPU *cpu)
>>>>>       /* The TCG path should also be holding the BQL at this point */
>>>>>       g_assert(qemu_mutex_iothread_locked());
>>>>> 
>>>>> -    if (msr_pr) {
>>>>> +    if (env->msr & M_MSR_PR) {
>>>> 
>>>> I'm not sure I'm keen on the M_ prefix, but I'll defer to Cedric or 
>>>> Daniel if they're ok
>>>> with it.
>>>> 
>>>> In general there are inconsistencies with the use of MSR_PR (1 vs 1ull), 
>>>> which makes it
>>>> tempting to replace MSR_PR the bit number with MSR_PR the mask and leave 
>>>> off the M_
>>>> prefix.  It's somewhat easy for MSR_PR, since missed conversions will 
>>>> certainly result in
>>>> compiler warnings for out-of-range shift (the same would not be true with 
>>>> bits 0-6, LE
>>>> through EP). >
>>>> Another possibility would be to use hw/registerfields.h.  Missed 
>>>> conversions are missing
>>>> symbol errors.  You'd write FIELD_EX64(env->msr, MSR, PR) in cases like 
>>>> this and
>>>> R_MSR_PR_MASK in cases like cpu_init.c.  It's more verbose for single 
>>>> bits like this, but
>>>> much easier to work with multi-bit fields like MSR.TS.
>>>> 
>>> Thanks for your ideas! I think I'll go with this second one. It'll allow
>>> to remove the !!(val) occurrences that I introduced in this series, so
>>> the code quality will probably be improved.
>>> 
>>> It'll also be a 'safer' change that will require less rework on other
>>> parts that I didn't intend to touch in this patch series.
>> 
>> 
>> The registerfield API is certainly a good choice for cleanups.
>> 
>> Is there a way to adapt the PPC_BIT macros and keep the PPC bit
>> ordering ? It might be easier to forget about it. Which is what
>> the Linux kernel does in many places.
>
> Hello Cédric.
>
> It would probably be easier to change this if we went with Zoltan's
> idea. Just 'invert' the MSR_* values to match the ISA order and use
> env->msr & PPC_BIT(MSR_*). However registerfield API expects it to be
> in the "0 is the rightmost" order,
> so we can't easily go with it and just invert the MSR_* values.

One thing I'm a bit worried about with registerfields macros is that they 
use deposit64 and extract64 which have an IMO unneeded assert so this 
means it adds an expression evaluation at every invocation of these 
(hopefully the function overhead is optimisied out by inlining) which 
might have some performance impact. So I still prefer the PPC_BIT macro 
but changing the MSR_* defines might introduce bugs when not done 
carefully so I'm nor sure it worths it.

Do we have some performance benchmarks that could be used to evaluate the 
changes for performance impact? There was some Summer of Code project for 
this but I think it was abandoned. It would be useful to run that as part 
of CI testing maybe.

Regards,
BALATON Zoltan

> A solution I could think that might be easy is: rename PPC_BIT to
> PPC_BIT_ULL (behaves like BIT_ULL but 'inverted'), and create a new
> PPC_BIT macro that just inverts the bit value
>
> #define PPC_BIT_ULL(bit) (0x8000000000000000ULL >> (bit))
> #define PPC_BIT(bit) (63 - (bit))
>
> and change MSR_* to use it
>
> #define MSR_LE PPC_BIT(63)
>
>> 
>> 
>> Device models are also impacted :
>>
>>    include/hw/pci-host/pnv_phb*_regs.h
>>    include/hw/ppc/xive*_regs.h
>> 
>> Something I have been wanting to change for a while are these macros :
>>
>>      static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
>>      {
>>          return (word & mask) >> ctz64(mask);
>>      }
>>
>>      static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
>>                                      uint64_t value)
>>      {
>>          return (word & ~mask) | ((value << ctz64(mask)) & mask);
>>      }
>> 
>> Thanks,
>> 
>> C.
>> 
>
> Thanks!
>
> --
> Víctor Cora Colombo
> Instituto de Pesquisas ELDORADO
> Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
>
>