Create a common helper for pmreg_access and pmreg_access_pmcr to improve
readability.

Signed-off-by: Smail AIDER <smail.aider@huawei.com>
---
 target/arm/cpregs-pmu.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
index 3aacd4f652..adaaaf2517 100644
--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -228,22 +228,28 @@ static bool event_supported(uint16_t number)
     return supported_event_map[number] != UNSUPPORTED_EVENT;
 }
 
-static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
-                                   bool isread)
+static CPAccessResult do_pmreg_access(CPUARMState *env, bool is_pmcr)
 {
     /*
      * Performance monitor registers user accessibility is controlled
-     * by PMUSERENR. MDCR_EL2.TPM and MDCR_EL3.TPM allow configurable
+     * by PMUSERENR. MDCR_EL2.TPM/TPMCR and MDCR_EL3.TPM allow configurable
      * trapping to EL2 or EL3 for other accesses.
      */
     int el = arm_current_el(env);
-    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
+    uint64_t mdcr_el2;
 
     if (el == 0 && !(env->cp15.c9_pmuserenr & 1)) {
         return CP_ACCESS_TRAP_EL1;
     }
-    if (el < 2 && (mdcr_el2 & MDCR_TPM)) {
-        return CP_ACCESS_TRAP_EL2;
+    if (el < 2) {
+        mdcr_el2 = arm_mdcr_el2_eff(env);
+
+        if (mdcr_el2 & MDCR_TPM) {
+            return CP_ACCESS_TRAP_EL2;
+        }
+        if (is_pmcr && (mdcr_el2 & MDCR_TPMCR)) {
+            return CP_ACCESS_TRAP_EL2;
+        }
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TPM)) {
         return CP_ACCESS_TRAP_EL3;
@@ -252,24 +258,16 @@ static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
     return CP_ACCESS_OK;
 }
 
-/*
- * Check for traps to PMCR register, which are controlled by PMUSERENR.EN
- * for EL1, MDCR_EL2.TPM/TPMCR for EL2 and MDCR_EL3.TPM for EL3.
- */
-static CPAccessResult pmreg_access_pmcr(CPUARMState *env, const ARMCPRegInfo *ri,
+static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                    bool isread)
 {
-    int el = arm_current_el(env);
-    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
-
-    CPAccessResult ret = pmreg_access(env, ri, isread);
-
-    /* Check MDCR_TPMCR if not already trapped to EL1 */
-    if (ret != CP_ACCESS_TRAP_EL1 && el < 2 && (mdcr_el2 & MDCR_TPMCR)) {
-        ret = CP_ACCESS_TRAP_EL2;
-    }
+    return do_pmreg_access(env, false);
+}
 
-    return ret;
+static CPAccessResult pmreg_access_pmcr(CPUARMState *env, const ARMCPRegInfo *ri,
+                                   bool isread)
+{
+    return do_pmreg_access(env, true);
 }
 
 static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
-- 
2.34.1

On 8/1/25 19:06, Smail AIDER wrote:
> Create a common helper for pmreg_access and pmreg_access_pmcr to improve
> readability.
> 
> Signed-off-by: Smail AIDER <smail.aider@huawei.com>
> ---
>   target/arm/cpregs-pmu.c | 40 +++++++++++++++++++---------------------
>   1 file changed, 19 insertions(+), 21 deletions(-)

The squash of these two patch is what I asked for.

> 
> diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
> index 3aacd4f652..adaaaf2517 100644
> --- a/target/arm/cpregs-pmu.c
> +++ b/target/arm/cpregs-pmu.c
> @@ -228,22 +228,28 @@ static bool event_supported(uint16_t number)
>       return supported_event_map[number] != UNSUPPORTED_EVENT;
>   }
>   
> -static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
> -                                   bool isread)
> +static CPAccessResult do_pmreg_access(CPUARMState *env, bool is_pmcr)
>   {
>       /*
>        * Performance monitor registers user accessibility is controlled
> -     * by PMUSERENR. MDCR_EL2.TPM and MDCR_EL3.TPM allow configurable
> +     * by PMUSERENR. MDCR_EL2.TPM/TPMCR and MDCR_EL3.TPM allow configurable
>        * trapping to EL2 or EL3 for other accesses.
>        */
>       int el = arm_current_el(env);
> -    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
> +    uint64_t mdcr_el2;
>   
>       if (el == 0 && !(env->cp15.c9_pmuserenr & 1)) {
>           return CP_ACCESS_TRAP_EL1;
>       }
> -    if (el < 2 && (mdcr_el2 & MDCR_TPM)) {
> -        return CP_ACCESS_TRAP_EL2;
> +    if (el < 2) {
> +        mdcr_el2 = arm_mdcr_el2_eff(env);

Move the declaration here too:

	uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);

With that,

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


r~