Re: [PATCH v2 04/21] target/ppc: Remove msr_le macro

2022-05-02 Thread Richard Henderson

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

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

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


Reviewed-by: Richard Henderson 

r~



[PATCH v2 04/21] target/ppc: Remove msr_le macro

2022-05-02 Thread Víctor Colombo
msr_le macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.

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

---

v2: Remove M_MSR_LE and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo 
---
 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 21d1f14381..932c5f4bdd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -355,6 +355,7 @@ typedef enum {
 #define MSR_LE   0  /* Little-endian mode   1 hflags */
 
 FIELD(MSR, PR, MSR_PR, 1)
+FIELD(MSR, LE, MSR_LE, 1)
 
 /* PMU bits */
 #define MMCR0_FC PPC_BIT(32) /* Freeze Counters  */
@@ -485,7 +486,6 @@ FIELD(MSR, PR, MSR_PR, 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_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 ac16a64846..0c6b83406e 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 !FIELD_EX64(env->msr, MSR, LE);
 }
 
 #ifdef CONFIG_TCG
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 1252429a2a..1a0b9ca82c 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 (!FIELD_EX64(env->msr, 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 fba7f84b7a..9af135e88e 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 FIELD_EX64(env->msr, MSR, LE);
 #else
-  return !msr_le;
+  return !FIELD_EX64(env->msr, 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 (FIELD_EX64(env->msr, 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 (FIELD_EX64(env->msr, 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 (FIELD_EX64(env->msr, 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, 
  \
 }