Re: [PATCH RESEND v2 5/6] target/arm/cpu: Enable 'el2' to work with host/max cpu

[Andrew Jones]

On Tue, Apr 27, 2021 at 10:38:00AM +0100, Peter Maydell wrote:
> On Tue, 27 Apr 2021 at 10:24, Andrew Jones <drjones@redhat.com> wrote:
> > I feel like there are way too many ways to track this feature now. If I
> > didn't lose count we have
> >
> >  1) cpu->has_el2
> >  2) cpu->env & ARM_FEATURE_EL2
> >  3) (for mach-virt) vms->virt
> >  4) possibly (and probably should) some ID register bits
> >
> > I realize the first three are already in use for TCG, but I'm guessing
> > we'll want to clean those up. What's the plan going forward? I presume
> > it'll be (4), but maybe something like (1) and/or (3) will stick around
> > for convenience. I'm pretty sure we want to avoid (2).
> 
> For new features added we want to use ID register bits. However,
> a lot of older pre-existing features are keyed off ARM_FEATURE_FOO
> flag bits. Trying to convert an ARM_FEATURE flag to use ID registers
> isn't necessarily 100% trivial (for instance, the ARM_FEATURE flag
> is always checkable regardless of AArch64 vs AArch32, but the ID
> register checks often need to be split up into separate ones checking
> the AArch32 or the AArch64 ID register value). So we aren't really
> doing conversion of existing flags. (I did a few which were easy
> because there were only a handful of checks of them.) As a side note,
> some features really can't be checked in ID registers, like
> ARM_FEATURE_V8_1M, so env->features is not going to go away completely.
> 
> The only use of cpu->has_foo should be for the QOM property -- the
> arm_cpu_realizefn() should look at it and then either clear the
> ARM_FEATURE flag or update the ID register bits depending on
> which one the feature is using.
> 
> vms->virt is for the board code (and controls more things than
> just whether the CPU itself has EL2).
>

Thanks for the summary, Peter. For this series, do you recommend
attempting to convert from ARM_FEATURE_EL2 to feature bits first? Or keep
the ARM_FEATURE flag? Also, while I agree we can't use vms->virt for the
same purposes as the CPU feature (however that's tracked), do you agree
vms->virt should be true when the CPU feature is enabled?

Thanks,
drew