Re: [PATCH 09/10] target/arm: Support 64-bit event counters for FEAT_PMUv3p5

[Peter Maydell]

On Sat, 20 Aug 2022 at 19:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 8/11/22 10:16, Peter Maydell wrote:
> > +static bool pmevcntr_is_64_bit(CPUARMState *env, int counter)
> > +{
> > +    /* Return true if the specified event counter is configured to be 64 
> > bit */
> > +
> > +    /* This isn't intended to be used with the cycle counter */
> > +    assert(counter < 31);
> > +
> > +    if (!cpu_isar_feature(any_pmuv3p5, env_archcpu(env))) {
> > +        return false;
> > +    }
> > +
> > +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> > +        /*
> > +         * MDCR_EL2.HLP still applies even when EL2 is disabled in the
> > +         * current security state, so we don't use arm_mdcr_el2_eff() here.
> > +         */
> > +        bool hlp = env->cp15.mdcr_el2 & MDCR_HLP;
> > +        int hpmn = env->cp15.mdcr_el2 & MDCR_HPMN;
>
> The specs could be improved here, as the top of MDCR_EL2 says it doesn't 
> apply if EL2
> isn't enabled in the security state, HLP has the exception noted above, but 
> HPMN does not.
>   I conclude that HPMN is missing the exception, because nothing else makes 
> sense.

I suspect (but haven't thought through) that not all of the things HPMN
controls apply when EL2 is disabled, so it's probably more complicated
than a blanket "applies even when EL2 is disabled" tag the way HLP does it.

thanks
-- PMM