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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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),
>