On Thu, Jun 04, 2020 at 17:09:30 +0100, Peter Maydell wrote: > On Thu, 4 Jun 2020 at 17:03, Leif Lindholm <l...@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