Re: kvm_target, QEMU_KVM_ARM_TARGET_GENERIC_V8 questions

[Leif Lindholm]

On Thu, Jun 04, 2020 at 17:09:30 +0100, Peter Maydell wrote:
> On Thu, 4 Jun 2020 at 17:03, Leif Lindholm <leif@nuviainc.com> wrote:
> > But there's also things like:
> > - a57_initfn explicitly setting kvm_target, then only being called
> >   from max_initfn for !kvm_enabled()
> 
> Expected -- a KVM 'max' is nothing to do with a TCG 'max':
>  * for KVM, -cpu max means "same as -cpu host"
>  * for TCG, -cpu max means "start with an A57, then add in all the
>    extra architectural features that have been added since then".

Sure. But why are we setting the kvm_target at all for the
!kvm_enabled() case?

> kvm_target being set by a57_initfn is specifically for the case
> where a KVM user is using "-cpu cortex-a57".
> 
> > - a57_initfn setting cpu->dtb_compatible to "arm,cortex-a57"
> 
> What else would it set it to?

Hmm, I had been hoping there was a generic v8a one - kvm64.c kind of
got my hopes up with "arm,arm-v8". Still, for "max", would it not be
useful to update it to the track the most architecturally advanced cpu
supported? At this point "arm,cortex-a72".

> > - a57 initfn setting cpu->midr, max_initfn overwriting parts of it
> 
> Also expected, TCG's -cpu max is "A57 with lots of extras".

Maybe I'm being too rigid in my thinking here, but it does kind of jar
to see code call a function called aarch64_a57_initfn to have it write
a value where it then throws away the only thing in the value that
made it a57.

> The way we create a TCG -cpu max is a bit odd, as the code was
> originally written in a situation where A57 was the most advanced
> TCG CPU we had and there were no extra architectural features
> supported by our CPU emulation. Today we have an A72 model as
> well and a lot of extra architectural features, so the "code
> borrowed" to "extras added" ratio looks a bit unbalanced.
> Cleaning it up would not be a bad idea.

I might start by doing that bit. It might make a lot of the above
niggles simply disappear.

Not entirely unrelated question:
Would you take added field definitions in target/arm/cpu.h for
features not yet emulated in QEMU but defined in released versions of
ARM ARM?

Regards,

Leif