Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

On Thu, Jun 04, 2020 at 14:10:08 +0100, Peter Maydell wrote:
> [added kvm-arm to the cc list; the kernel folks tend to hang out
> there, not on qemu-devel, so KVM related questions are usually
> worth raising there as well.]
> 
> On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > However, while looking at this, I noticed aarch64_a72_initfn doesn't
> > initialise kvm_target at all.
> 
> Yep. The kernel doesn't support "give me a CPU that looks like
> a Cortex-A72".
> 
> > So, then I decided to actually test things, and found that
> > (with -enable-kvm):
> > - on Cortex-A53 hardware
> >   - "max" kvm_target gets initialized to 4 (KVM_ARM_TARGET_CORTEX_A53)
> >     by kvm_arm_get_host_cpu_features (as returned from the kernel for
> >     vm_arm_create_scratch_host_vcpu)
> >   - cortex-A72 fails to start with "KVM is not supported for this guest
> >     CPU type"
> >   (fair enough, it's later than A53)
> 
> Untested, but I assume that -cpu cortex-a53 works on the A53...

Yes.

> > - on Cortex-A72 hardware
> >   - "max" kvm_target gets initialized to 5 (KVM_ARM_TARGET_GENERIC_V8)
> >     by kvm_arm_get_host_cpu_features
> >   - "cortex-A72" fails to start (umm...)
> 
> ...and fails on the A72 host.

>From an explicit software test. If I initialize kvm_target to
KVM_ARM_TARGET_GENERIC_V8, I can certainly run EDK2.

> > However ... if I haven't managed to confuse myself somewhere in here
> > (which is completely possible), would it be OK if I submitted a set of
> > patches that:
> > - add a QEMU_KVM_ARM_TARGET_GENERIC_V8 to match the kernel one
> > - set kvm_target for Cortex-A72 to QEMU_KVM_ARM_TARGET_GENERIC_V8
> 
> This would be wrong -- it would mean that you could tell QEMU "give
> me a guest CPU that's a Cortex-A72" and it would not error on
> non-A72 hardware but not actually give a guest CPU that looks
> like a Cortex-A72.
> 
>  * If what you want is "give me something that works" then that's
>    -cpu host or -cpu max.

That's what I thought until I saw the manual A57/A53 setting of
kvm_target.

>  * If what you want is "give me something that's always this kind of
>    CPU regardless of the host hardware" then that's a lot of kernel
>    dev work nobody's been enthusiastic enough to undertake yet
>    (notably the "what do we do about CPU errata workarounds" question
>    would need to be solved...)
> 
>  * If what you want is "allow me to say '-cpu cortex-a72' and have
>    it work on an A72 host and not anywhere else" then I guess we could
>    implement that on the QEMU side by querying the MIDR and checking
>    whether it was what we expected.

I don't really.

> >   - alternatively drop the explicit settings for A57/A53
> 
> These explicit settings are correct, because for these CPUs
> the kernel does have a "give me what I want in particular"
> setting (which it will fail on the wrong h/w), and also as
> back-compat for older kernels that predate the GENERIC_V8
> define and only recognize the explicit "give me an A53" value.

Right. But then I got somewhat confused also by how
https://git.qemu.org/?p=qemu.git;a=blob;f=target/arm/kvm64.c#l494
doesn't explicitly list KVM_ARM_TARGET_CORTEX_A53.

> > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and
> >   copy the relevant bits into the former for the !kvm case
> 
> Not sure why you care about this -- it's an implementation
> detail of the TCG handling of the 'max' CPU. There's an argument
> for disentangling TCG 'max' so it's not literally implemented
> as "a57 plus newer stuff", granted.

So ... the reason I care is because I'm adding a new cpu in my local
branch, figured cpu64.c was a good starting point, and then followed a
long string of repeating the questions "why?" and "why not?" while
trying to understand why things were set up the way they are.

And I ended up in a state where it looks like we do some things for
A57 that we don't do for A53, and we do even fewer things for A72, but
then we do (end up doing) all of the A57 bits again for max (for TCG
only), and then overwriting them.

Then I tried to build some sort of consistent working model in my head
and send some questions off to the list rather than try to send out
patches straight away as I figured the likelihood was high I had
missed or misunderstood something.

I guess what I'm really asking is if there is some legacy in here
that can be cleaned up to make the expected behaviour for a new CPU
more clear from looking at available code? And if there are specific
legacy things that need to be kept around for compatibility that
should not be implemented by new CPUs, if they could have some nice
warnings attached.

Regards,

Leif