[PATCH v3 0/1] ARM_PMU: Trap PMCR when MDCR_EL2.TPMCR is set

[PATCH v3 0/1] ARM_PMU: Trap PMCR when MDCR_EL2.TPMCR is set

[Smail AIDER via]

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.

Changes from v2:
- Squashed patches, no other changes.

Changes from v1:
- Some refactoring to simplify code.

Smail AIDER (1):
  target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

 target/arm/cpregs-pmu.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Smail AIDER via]

From: Smail AIDER via <qemu-devel@nongnu.org>

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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20250722131925.2119169-1-smail.aider@huawei.com>
---
 target/arm/cpregs-pmu.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
index 9c4431c18b..13392ddc4c 100644
--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -228,22 +228,27 @@ 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);
 
     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) {
+        uint64_t 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,6 +257,18 @@ static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
     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);
+}
+
 static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
                                            const ARMCPRegInfo *ri,
                                            bool isread)
@@ -1187,14 +1204,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

Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Philippe Mathieu-Daudé]

Hi Smail,

(no need to Cc qemu-stable with this patch, it is a simple refactor)

On 11/8/25 13:21, Smail AIDER via wrote:
> From: Smail AIDER via <qemu-devel@nongnu.org>
> 
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20250722131925.2119169-1-smail.aider@huawei.com>
> ---
>   target/arm/cpregs-pmu.c | 33 +++++++++++++++++++++++++--------
>   1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
> index 9c4431c18b..13392ddc4c 100644
> --- a/target/arm/cpregs-pmu.c
> +++ b/target/arm/cpregs-pmu.c
> @@ -228,22 +228,27 @@ 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)

"bool is_pmcr" vs ...

> +static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
> +                                   bool isread)

... "bool isread".

I suppose we want to use "is_pmcr" here instead of "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);
> +}
> +
>   static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
>                                              const ARMCPRegInfo *ri,
>                                              bool isread)
> @@ -1187,14 +1204,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),

RE: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Smail AIDER via]

Hi Philippe,

It is not just some refactoring. The last patch v3 is a squash of two previous patches v1 and v2.
Maybe I need to change the history description if not clear (I was talking from v3 point of view).
The purpose of the series is the main description itself. Please check the v1 below:

https://patchew.org/QEMU/20250722131925.2119169-1-smail.aider@huawei.com/

Other than that, the argument (is_pmcr) is correct. "isread" is not used in this case.

--
Best Regards,
Smail AIDER
E-Mail: smail.aider@huawei.com
Operating System Researcher/Developer
Dresden Research Center, OS Kernel Lab
Huawei Technologies Co., Ltd

-----Original Message-----
From: Philippe Mathieu-Daudé <philmd@linaro.org> 
Sent: Monday, August 11, 2025 2:36 PM
To: Smail AIDER <smail.aider@huawei.com>; qemu-devel@nongnu.org
Cc: Alexander Spyridakis <alexander.spyridakis@huawei.com>; zhangyue (BA) <zhangyue165@huawei.com>; Liuyutao(DRC) <liuyutao2@huawei.com>; mjt@tls.msk.ru; Peter Maydell <peter.maydell@linaro.org>; qemu-arm@nongnu.org; richard.henderson@linaro.org
Subject: Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

Hi Smail,

(no need to Cc qemu-stable with this patch, it is a simple refactor)

On 11/8/25 13:21, Smail AIDER via wrote:
> From: Smail AIDER via <qemu-devel@nongnu.org>
> 
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20250722131925.2119169-1-smail.aider@huawei.com>
> ---
>   target/arm/cpregs-pmu.c | 33 +++++++++++++++++++++++++--------
>   1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
> index 9c4431c18b..13392ddc4c 100644
> --- a/target/arm/cpregs-pmu.c
> +++ b/target/arm/cpregs-pmu.c
> @@ -228,22 +228,27 @@ 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)

"bool is_pmcr" vs ...

> +static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
> +                                   bool isread)

... "bool isread".

I suppose we want to use "is_pmcr" here instead of "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);
> +}
> +
>   static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
>                                              const ARMCPRegInfo *ri,
>                                              bool isread)
> @@ -1187,14 +1204,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),

Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Philippe Mathieu-Daudé]

On 11/8/25 15:33, Smail AIDER wrote:
> Hi Philippe,
> 
> It is not just some refactoring. The last patch v3 is a squash of two previous patches v1 and v2.
> Maybe I need to change the history description if not clear (I was talking from v3 point of view).
> The purpose of the series is the main description itself. Please check the v1 below:
> 
> https://patchew.org/QEMU/20250722131925.2119169-1-smail.aider@huawei.com/

Then please add a Cc tag (maintainer can do it if this v3 is OK, no need 
for v4):

Cc: qemu-stable@nongnu.org

> Other than that, the argument (is_pmcr) is correct. "isread" is not used in this case.

Right, I missed it during review. Maybe we want to forward the arguments
for clarity?

  -static CPAccessResult pmreg_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
  -                                   bool isread)
  +static CPAccessResult do_pmreg_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
  +                                   bool isread, bool is_pmcr)

Anyhow I'll let Richard review. No objection.

> 
> --
> Best Regards,
> Smail AIDER
> E-Mail: smail.aider@huawei.com
> Operating System Researcher/Developer
> Dresden Research Center, OS Kernel Lab
> Huawei Technologies Co., Ltd
> 
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Monday, August 11, 2025 2:36 PM
> To: Smail AIDER <smail.aider@huawei.com>; qemu-devel@nongnu.org
> Cc: Alexander Spyridakis <alexander.spyridakis@huawei.com>; zhangyue (BA) <zhangyue165@huawei.com>; Liuyutao(DRC) <liuyutao2@huawei.com>; mjt@tls.msk.ru; Peter Maydell <peter.maydell@linaro.org>; qemu-arm@nongnu.org; richard.henderson@linaro.org
> Subject: Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set
> 
> Hi Smail,
> 
> (no need to Cc qemu-stable with this patch, it is a simple refactor)
> 
> On 11/8/25 13:21, Smail AIDER via wrote:
>> From: Smail AIDER via <qemu-devel@nongnu.org>
>>
>> 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>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-Id: <20250722131925.2119169-1-smail.aider@huawei.com>
>> ---
>>    target/arm/cpregs-pmu.c | 33 +++++++++++++++++++++++++--------
>>    1 file changed, 25 insertions(+), 8 deletions(-)
>>
>> diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
>> index 9c4431c18b..13392ddc4c 100644
>> --- a/target/arm/cpregs-pmu.c
>> +++ b/target/arm/cpregs-pmu.c
>> @@ -228,22 +228,27 @@ 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)
> 
> "bool is_pmcr" vs ...
> 
>> +static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                                   bool isread)
> 
> ... "bool isread".
> 
> I suppose we want to use "is_pmcr" here instead of "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);
>> +}
>> +
>>    static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
>>                                               const ARMCPRegInfo *ri,
>>                                               bool isread)
>> @@ -1187,14 +1204,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),
> 

Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Michael Tokarev]

On 11.08.2025 16:45, Philippe Mathieu-Daudé wrote:
> On 11/8/25 15:33, Smail AIDER wrote:

> Then please add a Cc tag (maintainer can do it if this v3 is OK, no need 
> for v4):
> 
> Cc: qemu-stable@nongnu.org

I'm picking it up already - it's on my watch list since the v1 :)

/mjt

RE: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Smail AIDER via]

Hi Michael,

Could you please remove "via" from the author name, it's not part of my name : ) I guess it was just a typo.

Thank you.

--
Best Regards,
Smail AIDER
E-Mail: smail.aider@huawei.com
Operating System Researcher/Developer
Dresden Research Center, OS Kernel Lab
Huawei Technologies Co., Ltd

-----Original Message-----
From: Michael Tokarev <mjt@tls.msk.ru> 
Sent: Monday, August 11, 2025 3:48 PM
To: Philippe Mathieu-Daudé <philmd@linaro.org>; Smail AIDER <smail.aider@huawei.com>; qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Subject: Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

On 11.08.2025 16:45, Philippe Mathieu-Daudé wrote:
> On 11/8/25 15:33, Smail AIDER wrote:

> Then please add a Cc tag (maintainer can do it if this v3 is OK, no need 
> for v4):
> 
> Cc: qemu-stable@nongnu.org

I'm picking it up already - it's on my watch list since the v1 :)

/mjt

Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Michael Tokarev]

On 12.08.2025 09:51, Smail AIDER wrote:
> Hi Michael,
> 
> Could you please remove "via" from the author name, it's not part of my name : ) I guess it was just a typo.

I'm not the one to do this and there's nothing to remove really -
it is the mailing list who does it, replacing From with
the mailing list itself.  It will be done by whomever
applies the patch to the master branch.  Don't worry
about it.

Thanks,

/mjt

Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Michael Tokarev]

On 11.08.2025 14:21, Smail AIDER via wrote:
> From: Smail AIDER via <qemu-devel@nongnu.org>
> 
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20250722131925.2119169-1-smail.aider@huawei.com>

Hi!  Please take a look at
https://gitlab.com/mjt0k/qemu/-/commit/842918ee3c1cb9ac687bb27920c8d9020c2c109b
for my back-port of this commit to v10.0.x series, -- does it look
sane?  I assume this change is targetting 10.0.x too, not just 10.1.x.

Thanks,

/mjt

RE: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

[Smail AIDER via]

Hi Michael,

Your back-port commit is sane. Yes, I suppose this change needs to be ported to other releases. It has already been applied to the next 10.2 by @Peter Maydell.

Thanks,
Smail AIDER

-----Original Message-----

Hi!  Please take a look at
https://gitlab.com/mjt0k/qemu/-/commit/842918ee3c1cb9ac687bb27920c8d9020c2c109b
for my back-port of this commit to v10.0.x series, -- does it look sane?  I assume this change is targetting 10.0.x too, not just 10.1.x.

Thanks,

/mjt

Re: [PATCH v3 0/1] ARM_PMU: Trap PMCR when MDCR_EL2.TPMCR is set

[Peter Maydell]

On Mon, 11 Aug 2025 at 12:21, Smail AIDER <smail.aider@huawei.com> 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.
>
> Changes from v2:
> - Squashed patches, no other changes.
>
> Changes from v1:
> - Some refactoring to simplify code.
>
> Smail AIDER (1):
>   target/arm: Trap PMCR when MDCR_EL2.TPMCR is set



Applied to target-arm.next for 10.2, thanks.

PS: a cover letter is not necessary for a single patch,
only for multiple patch series.

-- PMM