On Apr 17 16:00, Peter Maydell wrote: > On 17 April 2018 at 15:23, Aaron Lindsay <alind...@codeaurora.org> wrote: > > On Apr 12 18:17, Peter Maydell wrote: > >> What's the difference between this and ARM_FEATURE_EL2 ? > > > > I use ARM_FEATURE_V7VE in a later patch to guard against implementing > > PMOVSSET on v7 machines which don't implement the virtualization > > extensions > > (http://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg04917.html). > > I could use ARM_FEATURE_EL2, but declaring that v7 machines supported > > EL2 didn't feel right. I don't feel strongly one way or the other - how > > do you prefer to handle this? > > So, the underlying issue here is that there's a QEMU specific > fudge going on. Architecturally, if the CPU implements the > Virtualization Extensions, then: > * it has Hyp mode > * it must also implement the Security Extensions > * on reset it starts in the Secure world > * it has LPAE > * it has some stuff that is not inherently tied to having EL2, > like the SDIV and UDIV instructions, and the presence of > PMOVSSET > > In an ideal world, we'd just have a feature flag that turned > all that on. Unfortunately, a combination of backwards compatibility > issues, the order in which various features were implemented > in QEMU, and the fact that KVM can't emulate a guest CPU with > the Security Extensions means that we want to be able to model > variants of some CPUs that don't really exist in real hardware: > Cortex-A15 and -A7 which only implement EL0/EL1 but still have > all the v7VE features that you can see from those ELs. But we > didn't really properly lay out guidelines for how the feature > bits should work in this case, with the result that we have > a bunch of local hacks (for instance get_S1prot() has a check > on the LPAE feature bit, since in practice that bit is set in > exactly the CPUs that have v7VE; and the UDIV/SDIV insns have > their own feature bits.) > > So we should probably sort out this mess first, either by: > > (a) state that we use ARM_FEATURE_LPAE for all checks for > features that are architecturally v7VE but which we want to > exist even on our v7VE-no-Hyp-no-Secure oddballs > (b) define an ARM_FEATURE_V7VE for them > (c) define separate feature bits for them individually
>From what I can tell, using ARM_FEATURE_LPAE to represent all the almost-v7ve misfits won't work well because ARM_FEATURE_ARM_DIV may be supported on some platforms for which ARM_FEATURE_LPAE is not (Cortex R5), and ARM_FEATURE_ARM_DIV is read from ID_ISAR0 in kvm_arm_get_host_cpu_features() (and may be set/not set independently of ARM_FEATURE_LPAE). It appears there is a need to independently distinguish between them. The same reasoning also seems to rule out option (b) "one ARM_FEATURE_V7VE to rule them all", leaving me with option (c). It almost seems silly to create ARM_FEATURE_PMOVSSET, but I'm not sure what else makes sense to do here. Am I missing something (I'm almost hoping I am)? -Aaron -- Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.