Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Peter Maydell]

On Thu, 4 Jun 2020 at 16:38, Leif Lindholm <leif@nuviainc.com> wrote:
> On Thu, Jun 04, 2020 at 14:10:08 +0100, Peter Maydell wrote:
> > On Thu, 4 Jun 2020 at 13:55, Leif Lindholm <leif@nuviainc.com> wrote:
> > > 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.

That would be asking for a -cpu max, though, not for an A53.

> > > 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.

> > >   - 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.

That list is supposed to contain "all CPUs which might be
present on a host kernel which doesn't support the
PREFERRED_TARGET ioctl". That ioctl went in in Linux kernel
commit 42c4e0c77ac91, and in a kernel source tree
"git show 42c4e0c77ac91:arch/arm64/include/uapi/asm/kvm.h"
tells us that indeed at that point the only 3 CPUs supported
were AEM_V8, FOUNDATION_V8 and CORTEX_A57. Once the host
kernel supported the PREFERRED_TARGET ioctl, we could query
it to ask "what kind of CPU are you?" in that function rather
than having to guess.

> 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.

(we don't overwrite them, we augment 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.

At this point I'd take another step backwards and ask
"why are you trying to add a new CPU?". Are you after a
TCG emulation of it, or are you trying to use KVM? If you're
using KVM, generally QEMU is set up so you don't need to tell
it about new CPU types at all. Mostly the cpu definitions for
specific CPUs in cpu64.c are there because we want them for TCG.

thanks
-- PMM