Trap PMCR_EL0 or PMCR accesses to EL2 when MDCR_EL2.TPMCR is set.
Similar to MDCR_EL2.TPM, MDCR_EL2.TPMCR allows trapping EL0 and EL1
accesses to the PMCR register to EL2.

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

diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
index 0f295b1376..3aacd4f652 100644
--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -252,6 +252,26 @@ 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,
+                                   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 ret;
+}
+
 static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
                                            const ARMCPRegInfo *ri,
                                            bool isread)
@@ -1192,14 +1212,14 @@ void define_pm_cpregs(ARMCPU *cpu)
             .fgt = FGT_PMCR_EL0,
             .type = ARM_CP_IO | ARM_CP_ALIAS,
             .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmcr),
-            .accessfn = pmreg_access,
+            .accessfn = pmreg_access_pmcr,
             .readfn = pmcr_read, .raw_readfn = raw_read,
             .writefn = pmcr_write, .raw_writefn = raw_write,
         };
         const ARMCPRegInfo pmcr64 = {
             .name = "PMCR_EL0", .state = ARM_CP_STATE_AA64,
             .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 0,
-            .access = PL0_RW, .accessfn = pmreg_access,
+            .access = PL0_RW, .accessfn = pmreg_access_pmcr,
             .fgt = FGT_PMCR_EL0,
             .type = ARM_CP_IO,
             .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
-- 
2.34.1

On 22.07.2025 16:19, Smail AIDER via wrote:
> Trap PMCR_EL0 or PMCR accesses to EL2 when MDCR_EL2.TPMCR is set.
> Similar to MDCR_EL2.TPM, MDCR_EL2.TPMCR allows trapping EL0 and EL1
> accesses to the PMCR register to EL2.

Ping?

Has this patch been forgotten?

Thanks,

/mjt

On Sun, 10 Aug 2025 at 08:57, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 22.07.2025 16:19, Smail AIDER via wrote:
> > Trap PMCR_EL0 or PMCR accesses to EL2 when MDCR_EL2.TPMCR is set.
> > Similar to MDCR_EL2.TPM, MDCR_EL2.TPMCR allows trapping EL0 and EL1
> > accesses to the PMCR register to EL2.
>
> Ping?
>
> Has this patch been forgotten?

This is v1, it got code review comments, and there is a v2 on list.

The v2 on my list but it's passed the "worth putting into 10.1"
point already and hasn't yet been reviewed. I was assuming RTH would
review as he looked at v1.

-- PMM

On 7/22/25 06:19, Smail AIDER via wrote:
> Trap PMCR_EL0 or PMCR accesses to EL2 when MDCR_EL2.TPMCR is set.
> Similar to MDCR_EL2.TPM, MDCR_EL2.TPMCR allows trapping EL0 and EL1
> accesses to the PMCR register to EL2.
> 
> Signed-off-by: Smail AIDER <smail.aider@huawei.com>
> ---
>   target/arm/cpregs-pmu.c | 24 ++++++++++++++++++++++--
>   1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
> index 0f295b1376..3aacd4f652 100644
> --- a/target/arm/cpregs-pmu.c
> +++ b/target/arm/cpregs-pmu.c
> @@ -252,6 +252,26 @@ 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,
> +                                   bool isread)
> +{
> +    int el = arm_current_el(env);
> +    uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
> +

This could be improved by not computing these until they're needed.

> +    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;
> +    }

Or by creating a common helper for pmreg_access and pmreg_access_pmcr such that those 
values only need computing once within the common helper.  That could make the code more 
obvious as well.

E.g.

static CPAccessResult do_pmreg_access(CPUARMState *env, bool with_tpmcr)
{
     int el = arm_current_el(env);

     if (el == 0 && !(env->cp15.c9_pmuserenr & 1)) {
         return CP_ACCESS_TRAP_EL1;
     }

     if (el < 2) {
         uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);

         if (mdcr_el2 & MDCR_TPM) {
             return CP_ACCESS_TRAP_EL2;
         }
         if (with_tpmcr && (mdcr_el2 & MDCR_TPMCR)) {
             return CP_ACCESS_TRAP_EL2;
         }
     }

     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TPM)) {
         return CP_ACCESS_TRAP_EL3;
     }
     return CP_ACCESS_OK;
}

static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
                                    bool isread)
{
     return do_pmreg_access(env, false);
}

static CPAccessResult pmreg_access_pmcr(CPUARMState *env, const ARMCPRegInfo *ri,
                                         bool isread)
{
     return do_pmreg_access(env, true);
}


r~