On Thu, Jun 04, 2020 at 01:55:44PM +0100, Leif Lindholm wrote: > Hi there, > > (all this done on current HEAD: 66234fee9c) > > I was looking through the definition of the aarch64 "max" cpu, and > noticed it invokes aarch64_a57_initfn as a template, followed by > overriding some feature and ID fields to enable additional > functionality. > > I then noticed it does not override kvm_target, which hence remains > set as QEMU_KVM_ARM_TARGET_CORTEX_A57. Of course, this only happens on > the else side of the if (kvm_enabled()) branch, so doesn't affect it. > > However, while looking at this, I noticed aarch64_a72_initfn doesn't > initialise kvm_target at all. > > Then I looked at the definition of QEMU_KVM_ARM_TARGET_CORTEX_A57, > found there was also a KVM_ARM_TARGET_CORTEX_A57, and then I noticed > there exists a KVM_ARM_TARGET_CORTEX_A57 (in > linux-headers/asm-arm64/kvm.h), and *then* I noticed there exists a > KVM_ARM_TARGET_GENERIC_V8 definition there as well - plus a comment > saying "please don't add any more targets unless you really need to". > Then I noticed there isn't a corresponding > QEMU_KVM_ARM_TARGET_GENERIC_V8 in target/arm/kvm-consts.h. > > 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) > - 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...) > > 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
I'd rather not do that. AArch64 KVM doesn't support CPU models, so we shouldn't try to fake it by allowing '-cpu cortex-a72' to work, even when using it on a host with Cortex-A72 CPUs. I'd rather all AArch64 KVM users use 'host' or 'max' (which is the same as 'host' when used with KVM) until CPU models truly work. > - alternatively drop the explicit settings for A57/A53 The A57 and A53 KVM targets would have to be exactly the same as the KVM generic target in order to do that. And, even then we can't remove them from KVM, as they're API, so it doesn't help much to change them in QEMU. > - drop the call from aarch64_max_initfn to aarch64_a57_initfn, and > copy the relevant bits into the former for the !kvm case I don't have a strong preference here, but if the naming is what's troublesome, then I'd think we're better off creating something like an aarch64_aXX_initfn() function and then calling it from both a57 and max, and anywhere else it fits. Thanks, drew
