Ping for code review, please?
thanks
-- PMM
On Tue, 18 Mar 2025 at 11:42, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> Our KVM code includes backwards compatibility support for ancient
> kernels which don't support the KVM_ARM_PREFERRED_TARGET ioctl. This
> ioctl was introduced in kernel commit 42c4e0c77ac91 in September
> 2013 and is in v3.12, so it's reasonable to assume it's present.
>
> (We already dropped support for kernels without KVM_CAP_DEVICE_CTRL,
> a feature added to the kernel in April 2013, in our commit
> 84f298ea3e; so there are only about six months' worth of kernels,
> from v3.9 to v3.11, that we don't already fail to run on and that
> this commit is dropping handling for.)
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> target/arm/kvm_arm.h | 7 +----
> target/arm/arm-qmp-cmds.c | 2 +-
> target/arm/kvm.c | 55 ++++++---------------------------------
> 3 files changed, 10 insertions(+), 54 deletions(-)
>
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index 05c3de8cd46..5f17fc2f3d5 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -97,10 +97,6 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu);
> #ifdef CONFIG_KVM
> /**
> * kvm_arm_create_scratch_host_vcpu:
> - * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
> - * QEMU_KVM_ARM_TARGET_NONE) to try as fallback if the kernel does not
> - * know the PREFERRED_TARGET ioctl. Passing NULL is the same as passing
> - * an empty array.
> * @fdarray: filled in with kvmfd, vmfd, cpufd file descriptors in that order
> * @init: filled in with the necessary values for creating a host
> * vcpu. If NULL is provided, will not init the vCPU (though the cpufd
> @@ -113,8 +109,7 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu);
> * Returns: true on success (and fdarray and init are filled in),
> * false on failure (and fdarray and init are not valid).
> */
> -bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
> - int *fdarray,
> +bool kvm_arm_create_scratch_host_vcpu(int *fdarray,
> struct kvm_vcpu_init *init);
>
> /**
> diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
> index 883c0a0e8cc..a1a944adb43 100644
> --- a/target/arm/arm-qmp-cmds.c
> +++ b/target/arm/arm-qmp-cmds.c
> @@ -46,7 +46,7 @@ static inline void gic_cap_kvm_probe(GICCapability *v2,
> GICCapability *v3)
> #ifdef CONFIG_KVM
> int fdarray[3];
>
> - if (!kvm_arm_create_scratch_host_vcpu(NULL, fdarray, NULL)) {
> + if (!kvm_arm_create_scratch_host_vcpu(fdarray, NULL)) {
> return;
> }
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index da30bdbb234..568561c6d54 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -100,8 +100,7 @@ static int kvm_arm_vcpu_finalize(ARMCPU *cpu, int feature)
> return kvm_vcpu_ioctl(CPU(cpu), KVM_ARM_VCPU_FINALIZE, &feature);
> }
>
> -bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
> - int *fdarray,
> +bool kvm_arm_create_scratch_host_vcpu(int *fdarray,
> struct kvm_vcpu_init *init)
> {
> int ret = 0, kvmfd = -1, vmfd = -1, cpufd = -1;
> @@ -150,40 +149,13 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t
> *cpus_to_try,
> struct kvm_vcpu_init preferred;
>
> ret = ioctl(vmfd, KVM_ARM_PREFERRED_TARGET, &preferred);
> - if (!ret) {
> - init->target = preferred.target;
> + if (ret < 0) {
> + goto err;
> }
> + init->target = preferred.target;
> }
> - if (ret >= 0) {
> - ret = ioctl(cpufd, KVM_ARM_VCPU_INIT, init);
> - if (ret < 0) {
> - goto err;
> - }
> - } else if (cpus_to_try) {
> - /* Old kernel which doesn't know about the
> - * PREFERRED_TARGET ioctl: we know it will only support
> - * creating one kind of guest CPU which is its preferred
> - * CPU type.
> - */
> - struct kvm_vcpu_init try;
> -
> - while (*cpus_to_try != QEMU_KVM_ARM_TARGET_NONE) {
> - try.target = *cpus_to_try++;
> - memcpy(try.features, init->features, sizeof(init->features));
> - ret = ioctl(cpufd, KVM_ARM_VCPU_INIT, &try);
> - if (ret >= 0) {
> - break;
> - }
> - }
> - if (ret < 0) {
> - goto err;
> - }
> - init->target = try.target;
> - } else {
> - /* Treat a NULL cpus_to_try argument the same as an empty
> - * list, which means we will fail the call since this must
> - * be an old kernel which doesn't support PREFERRED_TARGET.
> - */
> + ret = ioctl(cpufd, KVM_ARM_VCPU_INIT, init);
> + if (ret < 0) {
> goto err;
> }
>
> @@ -259,17 +231,6 @@ static bool
> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> uint64_t features = 0;
> int err;
>
> - /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
> - * we know these will only support creating one kind of guest CPU,
> - * which is its preferred CPU type. Fortunately these old kernels
> - * support only a very limited number of CPUs.
> - */
> - static const uint32_t cpus_to_try[] = {
> - KVM_ARM_TARGET_AEM_V8,
> - KVM_ARM_TARGET_FOUNDATION_V8,
> - KVM_ARM_TARGET_CORTEX_A57,
> - QEMU_KVM_ARM_TARGET_NONE
> - };
> /*
> * target = -1 informs kvm_arm_create_scratch_host_vcpu()
> * to use the preferred target
> @@ -300,7 +261,7 @@ static bool
> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> features |= 1ULL << ARM_FEATURE_PMU;
> }
>
> - if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
> + if (!kvm_arm_create_scratch_host_vcpu(fdarray, &init)) {
> return false;
> }
>
> @@ -1835,7 +1796,7 @@ uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
>
> probed = true;
>
> - if (!kvm_arm_create_scratch_host_vcpu(NULL, fdarray, &init)) {
> + if (!kvm_arm_create_scratch_host_vcpu(fdarray, &init)) {
> error_report("failed to create scratch VCPU with SVE enabled");
> abort();
> }
> --
> 2.43.0
