On Tue, Apr 27, 2021 at 10:38:00AM +0100, Peter Maydell wrote: > On Tue, 27 Apr 2021 at 10:24, Andrew Jones <drjo...@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