Re: [PATCH 03/10] target/arm: Don't mishandle count when enabling or disabling PMU counters

[Richard Henderson]

On 8/11/22 10:16, Peter Maydell wrote:
> +static void mdcr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                           uint64_t value)
> +{
> +    /*
> +     * Some MDCR_EL3 bits affect whether PMU counters are running:

Typo el3.

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

r~

> +     * if we are trying to change any of those then we must
> +     * bracket this update with PMU start/finish calls.
> +     */
> +    bool pmu_op = (env->cp15.mdcr_el2 ^ value) & MDCR_EL2_PMU_ENABLE_BITS;
> +
> +    if (pmu_op) {
> +        pmu_op_start(env);
> +    }
> +    env->cp15.mdcr_el2 = value;
> +    if (pmu_op) {
> +        pmu_op_finish(env);
> +    }
>   }
>   
>   static const ARMCPRegInfo v8_cp_reginfo[] = {
> @@ -7669,6 +7713,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>           ARMCPRegInfo mdcr_el2 = {
>               .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
>               .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> +            .writefn = mdcr_el2_write,
>               .access = PL2_RW, .resetvalue = pmu_num_counters(env),
>               .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
>           };