Re: [Qemu-devel] [PATCH v3 15/22] target/arm: Add ARM_FEATURE_V7VE for v7 Virtualization Extensions

[Peter Maydell]

On 17 April 2018 at 15:23, Aaron Lindsay <address@hidden> 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

In any case we'd retain ARM_FEATURE_EL2 for "and really
has EL2/Hyp mode", and we'd want to do an audit of current
uses of various feature bits to see whether they followed
the new rules.

(For AArch64 things are a bit less awkward because the
architecture allows the idea of an implementation that
has EL2 but not EL3.)

thanks
-- PMM