Re: [PATCH v2 03/21] target/ppc: Remove msr_pr macro

2022-05-02 Thread Richard Henderson

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

msr_pr 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_PR and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---


Reviewed-by: Richard Henderson 

r~




[PATCH v2 03/21] target/ppc: Remove msr_pr macro

2022-05-02 Thread Víctor Colombo
msr_pr 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_PR and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo 
---
 hw/ppc/pegasos2.c|  2 +-
 hw/ppc/spapr.c   |  2 +-
 target/ppc/cpu.h |  4 +++-
 target/ppc/cpu_init.c|  4 ++--
 target/ppc/excp_helper.c |  8 +---
 target/ppc/mem_helper.c  |  5 +++--
 target/ppc/mmu-radix64.c |  5 +++--
 target/ppc/mmu_common.c  | 23 ---
 8 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 56bf203dfd..9411ca6b16 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 (FIELD_EX64(env->msr, 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..fe9937e811 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 (FIELD_EX64(env->msr, 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..21d1f14381 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -25,6 +25,7 @@
 #include "exec/cpu-defs.h"
 #include "cpu-qom.h"
 #include "qom/object.h"
+#include "hw/registerfields.h"
 
 #define TCG_GUEST_DEFAULT_MO 0
 
@@ -353,6 +354,8 @@ typedef enum {
 #define MSR_RI   1  /* Recoverable interrupt1*/
 #define MSR_LE   0  /* Little-endian mode   1 hflags */
 
+FIELD(MSR, PR, MSR_PR, 1)
+
 /* PMU bits */
 #define MMCR0_FC PPC_BIT(32) /* Freeze Counters  */
 #define MMCR0_PMAO   PPC_BIT(56) /* Perf Monitor Alert Ocurred */
@@ -474,7 +477,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..ac16a64846 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 || FIELD_EX64(env->msr, 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 || FIELD_EX64(env->msr, MSR, PR)) {
 return true;
 }
 }
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d3e2cfcd71..7e8e34ef06 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1738,7 +1738,8 @@ 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 &&
+!FIELD_EX64(env->msr, 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 +1819,8 @@ 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 (FIELD_EX64(env->msr, MSR, PR) &&
+(env->spr[SPR_BESCR] & BESCR_GE)) {
 env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
 
 if (env->spr[SPR_BESCR] & BESCR_PMEO) {
@@ -2094,7 +2096,7 @@ static void do_ebb(CPUPPCState *env, int ebb_excp)