target/arm/kvm_arm.h | 25 +++++++++++++------------ target/arm/cpu.c | 2 +- target/arm/cpu64.c | 10 +++++----- target/arm/kvm.c | 4 ++-- target/arm/kvm64.c | 14 +++++--------- 5 files changed, 26 insertions(+), 29 deletions(-)
Since commit d70c996df23f, when enabling the PMU we get:
$ qemu-system-aarch64 -cpu host,pmu=on -M virt,accel=kvm,gic-version=3
Segmentation fault (core dumped)
Thread 1 "qemu-system-aar" received signal SIGSEGV, Segmentation fault.
0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588
2588 ret = ioctl(s->fd, type, arg);
(gdb) bt
#0 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588
#1 0x0000aaaaaae31568 in kvm_check_extension (s=0x0, extension=126) at accel/kvm/kvm-all.c:916
#2 0x0000aaaaaafce254 in kvm_arm_pmu_supported (cpu=0xaaaaac214ab0) at target/arm/kvm.c:213
#3 0x0000aaaaaafc0f94 in arm_set_pmu (obj=0xaaaaac214ab0, value=true, errp=0xffffffffe438) at target/arm/cpu.c:1111
#4 0x0000aaaaab5533ac in property_set_bool (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", opaque=0xaaaaac222730, errp=0xffffffffe438) at qom/object.c:2170
#5 0x0000aaaaab5512f0 in object_property_set (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1328
#6 0x0000aaaaab551e10 in object_property_parse (obj=0xaaaaac214ab0, string=0xaaaaac11b4c0 "on", name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1561
#7 0x0000aaaaab54ee8c in object_apply_global_props (obj=0xaaaaac214ab0, props=0xaaaaac018e20, errp=0xaaaaabd6fd88 <error_fatal>) at qom/object.c:407
#8 0x0000aaaaab1dd5a4 in qdev_prop_set_globals (dev=0xaaaaac214ab0) at hw/core/qdev-properties.c:1218
#9 0x0000aaaaab1d9fac in device_post_init (obj=0xaaaaac214ab0) at hw/core/qdev.c:1050
...
#15 0x0000aaaaab54f310 in object_initialize_with_type (obj=0xaaaaac214ab0, size=52208, type=0xaaaaabe237f0) at qom/object.c:512
#16 0x0000aaaaab54fa24 in object_new_with_type (type=0xaaaaabe237f0) at qom/object.c:687
#17 0x0000aaaaab54fa80 in object_new (typename=0xaaaaabe23970 "host-arm-cpu") at qom/object.c:702
#18 0x0000aaaaaaf04a74 in machvirt_init (machine=0xaaaaac0a8550) at hw/arm/virt.c:1770
#19 0x0000aaaaab1e8720 in machine_run_board_init (machine=0xaaaaac0a8550) at hw/core/machine.c:1138
#20 0x0000aaaaaaf95394 in qemu_init (argc=5, argv=0xffffffffea58, envp=0xffffffffea88) at softmmu/vl.c:4348
#21 0x0000aaaaaada3f74 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:48
This is because in frame #2, cpu->kvm_state is still NULL
(the vCPU is not yet realized).
KVM has a hard requirement of all cores supporting the same
feature set. We only need to check if the accelerator supports
a feature, not each vCPU individually.
Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState
argument (already realized/valid at this point) instead of a
CPUState argument.
Reported-by: Haibo Xu <haibo.xu@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
target/arm/kvm_arm.h | 25 +++++++++++++------------
target/arm/cpu.c | 2 +-
target/arm/cpu64.c | 10 +++++-----
target/arm/kvm.c | 4 ++--
target/arm/kvm64.c | 14 +++++---------
5 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 48bf5e16d5..8209525f20 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -12,6 +12,7 @@
#define QEMU_KVM_ARM_H
#include "sysemu/kvm.h"
+#include "sysemu/accel.h"
#include "exec/memory.h"
#include "qemu/error-report.h"
@@ -269,29 +270,29 @@ void kvm_arm_add_vcpu_properties(Object *obj);
/**
* kvm_arm_aarch32_supported:
- * @cs: CPUState
+ * @as: AccelState
*
- * Returns: true if the KVM VCPU can enable AArch32 mode
+ * Returns: true if the KVM accelerator can enable AArch32 mode
* and false otherwise.
*/
-bool kvm_arm_aarch32_supported(CPUState *cs);
+bool kvm_arm_aarch32_supported(AccelState *as);
/**
* kvm_arm_pmu_supported:
- * @cs: CPUState
+ * @as: AccelState
*
- * Returns: true if the KVM VCPU can enable its PMU
+ * Returns: true if the KVM accelerator can enable its PMU
* and false otherwise.
*/
-bool kvm_arm_pmu_supported(CPUState *cs);
+bool kvm_arm_pmu_supported(AccelState *as);
/**
* kvm_arm_sve_supported:
- * @cs: CPUState
+ * @as: AccelState
*
- * Returns true if the KVM VCPU can enable SVE and false otherwise.
+ * Returns true if the KVM accelerator can enable SVE and false otherwise.
*/
-bool kvm_arm_sve_supported(CPUState *cs);
+bool kvm_arm_sve_supported(AccelState *as);
/**
* kvm_arm_get_max_vm_ipa_size:
@@ -359,17 +360,17 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
static inline void kvm_arm_add_vcpu_properties(Object *obj) {}
-static inline bool kvm_arm_aarch32_supported(CPUState *cs)
+static inline bool kvm_arm_aarch32_supported(AccelState *as)
{
return false;
}
-static inline bool kvm_arm_pmu_supported(CPUState *cs)
+static inline bool kvm_arm_pmu_supported(AccelState *as)
{
return false;
}
-static inline bool kvm_arm_sve_supported(CPUState *cs)
+static inline bool kvm_arm_sve_supported(AccelState *as)
{
return false;
}
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5b7a36b5d7..29b314427c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1108,7 +1108,7 @@ static void arm_set_pmu(Object *obj, bool value, Error **errp)
ARMCPU *cpu = ARM_CPU(obj);
if (value) {
- if (kvm_enabled() && !kvm_arm_pmu_supported(CPU(cpu))) {
+ if (kvm_enabled() && !kvm_arm_pmu_supported(current_accel())) {
error_setg(errp, "'pmu' feature not supported by KVM on this host");
return;
}
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 778cecc2e6..13835768ab 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -266,7 +266,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
/* Collect the set of vector lengths supported by KVM. */
bitmap_zero(kvm_supported, ARM_MAX_VQ);
- if (kvm_enabled() && kvm_arm_sve_supported(CPU(cpu))) {
+ if (kvm_enabled() && kvm_arm_sve_supported(current_accel())) {
kvm_arm_sve_get_vls(CPU(cpu), kvm_supported);
} else if (kvm_enabled()) {
assert(!cpu_isar_feature(aa64_sve, cpu));
@@ -473,7 +473,7 @@ static void cpu_max_set_sve_max_vq(Object *obj, Visitor *v, const char *name,
return;
}
- if (kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) {
+ if (kvm_enabled() && !kvm_arm_sve_supported(current_accel())) {
error_setg(errp, "cannot set sve-max-vq");
error_append_hint(errp, "SVE not supported by KVM on this host\n");
return;
@@ -519,7 +519,7 @@ static void cpu_arm_set_sve_vq(Object *obj, Visitor *v, const char *name,
return;
}
- if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) {
+ if (value && kvm_enabled() && !kvm_arm_sve_supported(current_accel())) {
error_setg(errp, "cannot enable %s", name);
error_append_hint(errp, "SVE not supported by KVM on this host\n");
return;
@@ -556,7 +556,7 @@ static void cpu_arm_set_sve(Object *obj, Visitor *v, const char *name,
return;
}
- if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) {
+ if (value && kvm_enabled() && !kvm_arm_sve_supported(current_accel())) {
error_setg(errp, "'sve' feature not supported by KVM on this host");
return;
}
@@ -751,7 +751,7 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
* uniform execution state like do_interrupt.
*/
if (value == false) {
- if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) {
+ if (!kvm_enabled() || !kvm_arm_aarch32_supported(current_accel())) {
error_setg(errp, "'aarch64' feature cannot be disabled "
"unless KVM is enabled and 32-bit EL1 "
"is supported");
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index eef3bbd1cc..2247a96757 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -208,9 +208,9 @@ void kvm_arm_add_vcpu_properties(Object *obj)
}
}
-bool kvm_arm_pmu_supported(CPUState *cpu)
+bool kvm_arm_pmu_supported(AccelState *as)
{
- return kvm_check_extension(cpu->kvm_state, KVM_CAP_ARM_PMU_V3);
+ return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_PMU_V3);
}
int kvm_arm_get_max_vm_ipa_size(MachineState *ms)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index f09ed9f4df..ae4e37ce78 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -652,18 +652,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
return true;
}
-bool kvm_arm_aarch32_supported(CPUState *cpu)
+bool kvm_arm_aarch32_supported(AccelState *as)
{
- KVMState *s = KVM_STATE(current_accel());
-
- return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT);
+ return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_EL1_32BIT);
}
-bool kvm_arm_sve_supported(CPUState *cpu)
+bool kvm_arm_sve_supported(AccelState *as)
{
- KVMState *s = KVM_STATE(current_accel());
-
- return kvm_check_extension(s, KVM_CAP_ARM_SVE);
+ return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_SVE);
}
QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
@@ -798,7 +794,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
env->features &= ~(1ULL << ARM_FEATURE_PMU);
}
if (cpu_isar_feature(aa64_sve, cpu)) {
- assert(kvm_arm_sve_supported(cs));
+ assert(kvm_arm_sve_supported(ACCEL(cs->kvm_state)));
cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
}
--
2.21.3
Patchew URL: https://patchew.org/QEMU/20200617130800.26355-1-philmd@redhat.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === CC qga/guest-agent-command-state.o CC qga/main.o CC qga/commands-posix.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) CC qga/channel-posix.o CC qga/qapi-generated/qga-qapi-types.o CC qga/qapi-generated/qga-qapi-visit.o --- GEN docs/interop/qemu-ga-ref.html GEN docs/interop/qemu-ga-ref.txt GEN docs/interop/qemu-ga-ref.7 /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK qemu-keymap LINK ivshmem-client /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK ivshmem-server /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK qemu-nbd /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK qemu-storage-daemon /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) AS pc-bios/optionrom/multiboot.o AS pc-bios/optionrom/linuxboot.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK qemu-img CC pc-bios/optionrom/linuxboot_dma.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) AS pc-bios/optionrom/kvmvapic.o LINK qemu-io /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK qemu-edid AS pc-bios/optionrom/pvh.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINK fsdev/virtfs-proxy-helper CC pc-bios/optionrom/pvh_main.o LINK scsi/qemu-pr-helper /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) BUILD pc-bios/optionrom/multiboot.img BUILD pc-bios/optionrom/linuxboot.img LINK qemu-bridge-helper --- BUILD pc-bios/optionrom/linuxboot.raw BUILD pc-bios/optionrom/linuxboot_dma.img LINK virtiofsd /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) SIGN pc-bios/optionrom/multiboot.bin SIGN pc-bios/optionrom/linuxboot.bin BUILD pc-bios/optionrom/linuxboot_dma.raw --- BUILD pc-bios/optionrom/pvh.img BUILD pc-bios/optionrom/pvh.raw SIGN pc-bios/optionrom/pvh.bin /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) BUILD pc-bios/optionrom/kvmvapic.img BUILD pc-bios/optionrom/kvmvapic.raw SIGN pc-bios/optionrom/kvmvapic.bin LINK qemu-ga /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) GEN x86_64-softmmu/config-target.h GEN x86_64-softmmu/hmp-commands.h GEN x86_64-softmmu/hmp-commands-info.h --- CC x86_64-softmmu/hw/virtio/virtio.o CC x86_64-softmmu/hw/virtio/vhost.o CC x86_64-softmmu/hw/virtio/vhost-backend.o /tmp/qemu-test/src/fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! /tmp/qemu-test/src/fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven ); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ! 8 errors generated. make[1]: *** [/tmp/qemu-test/src/rules.mak:69: fpu/softfloat.o] Error 1 make[1]: *** Waiting for unfinished jobs.... /tmp/qemu-test/src/migration/ram.c:919:45: error: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion] xbzrle_counters.encoding_rate = UINT64_MAX; ~ ^~~~~~~~~~ /usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX' --- 18446744073709551615UL ^~~~~~~~~~~~~~~~~~~~~~ 1 error generated. make[1]: *** [/tmp/qemu-test/src/rules.mak:69: migration/ram.o] Error 1 make: *** [Makefile:527: x86_64-softmmu/all] Error 2 Traceback (most recent call last): File "./tests/docker/docker.py", line 669, in <module> sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=adc833a3dd9f4aaa9d9d130b75d0b794', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-hngiejzd/src/docker-src.2020-06-17-09.25.40.24507:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=adc833a3dd9f4aaa9d9d130b75d0b794 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-hngiejzd/src' make: *** [docker-run-test-debug@fedora] Error 2 real 4m1.214s user 0m8.553s The full log is available at http://patchew.org/logs/20200617130800.26355-1-philmd@redhat.com/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Wed, Jun 17, 2020 at 03:08:00PM +0200, Philippe Mathieu-Daudé wrote: > Since commit d70c996df23f, when enabling the PMU we get: > > $ qemu-system-aarch64 -cpu host,pmu=on -M virt,accel=kvm,gic-version=3 > Segmentation fault (core dumped) > > Thread 1 "qemu-system-aar" received signal SIGSEGV, Segmentation fault. > 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588 > 2588 ret = ioctl(s->fd, type, arg); > (gdb) bt > #0 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588 > #1 0x0000aaaaaae31568 in kvm_check_extension (s=0x0, extension=126) at accel/kvm/kvm-all.c:916 > #2 0x0000aaaaaafce254 in kvm_arm_pmu_supported (cpu=0xaaaaac214ab0) at target/arm/kvm.c:213 > #3 0x0000aaaaaafc0f94 in arm_set_pmu (obj=0xaaaaac214ab0, value=true, errp=0xffffffffe438) at target/arm/cpu.c:1111 > #4 0x0000aaaaab5533ac in property_set_bool (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", opaque=0xaaaaac222730, errp=0xffffffffe438) at qom/object.c:2170 > #5 0x0000aaaaab5512f0 in object_property_set (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1328 > #6 0x0000aaaaab551e10 in object_property_parse (obj=0xaaaaac214ab0, string=0xaaaaac11b4c0 "on", name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1561 > #7 0x0000aaaaab54ee8c in object_apply_global_props (obj=0xaaaaac214ab0, props=0xaaaaac018e20, errp=0xaaaaabd6fd88 <error_fatal>) at qom/object.c:407 > #8 0x0000aaaaab1dd5a4 in qdev_prop_set_globals (dev=0xaaaaac214ab0) at hw/core/qdev-properties.c:1218 > #9 0x0000aaaaab1d9fac in device_post_init (obj=0xaaaaac214ab0) at hw/core/qdev.c:1050 > ... > #15 0x0000aaaaab54f310 in object_initialize_with_type (obj=0xaaaaac214ab0, size=52208, type=0xaaaaabe237f0) at qom/object.c:512 > #16 0x0000aaaaab54fa24 in object_new_with_type (type=0xaaaaabe237f0) at qom/object.c:687 > #17 0x0000aaaaab54fa80 in object_new (typename=0xaaaaabe23970 "host-arm-cpu") at qom/object.c:702 > #18 0x0000aaaaaaf04a74 in machvirt_init (machine=0xaaaaac0a8550) at hw/arm/virt.c:1770 > #19 0x0000aaaaab1e8720 in machine_run_board_init (machine=0xaaaaac0a8550) at hw/core/machine.c:1138 > #20 0x0000aaaaaaf95394 in qemu_init (argc=5, argv=0xffffffffea58, envp=0xffffffffea88) at softmmu/vl.c:4348 > #21 0x0000aaaaaada3f74 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:48 > > This is because in frame #2, cpu->kvm_state is still NULL > (the vCPU is not yet realized). > > KVM has a hard requirement of all cores supporting the same > feature set. We only need to check if the accelerator supports > a feature, not each vCPU individually. > > Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState > argument (already realized/valid at this point) instead of a > CPUState argument. I'd rather not do that. IMO, a CPU feature test should operate on CPU, not an "accelerator". How that test is implemented is another story. If the CPUState isn't interesting, but it points to something that is, or there's another function that uses globals to get the job done, then fine, but the callers of a CPU feature test shouldn't need to know that. I think we should just revert d70c996df23f and then apply the same change to kvm_arm_pmu_supported() that other similar functions got with 4f7f589381d5. Thanks, drew
On 17/06/20 17:23, Andrew Jones wrote: >> >> Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState >> argument (already realized/valid at this point) instead of a >> CPUState argument. > I'd rather not do that. IMO, a CPU feature test should operate on CPU, > not an "accelerator". If it's a test that the feature is enabled (e.g. via -cpu) then I agree. For something that ends up as a KVM_CHECK_EXTENSION or KVM_ENABLE_CAP on the KVM fd, however, I think passing an AccelState is better. kvm_arm_pmu_supported case is clearly the latter, even the error message hints at that: + if (kvm_enabled() && !kvm_arm_pmu_supported(current_accel())) { error_setg(errp, "'pmu' feature not supported by KVM on this host"); return; } but the same is true of kvm_arm_aarch32_supported and kvm_arm_sve_supported. Applying the change to kvm_arm_pmu_supported as you suggest below would be a bit of a bandaid because it would not have consistent prototypes. Sp for Philippe's patch Acked-by: Paolo Bonzini <pbonzini@redhat.com> Thanks, Paolo > How that test is implemented is another story. > If the CPUState isn't interesting, but it points to something that is, > or there's another function that uses globals to get the job done, then > fine, but the callers of a CPU feature test shouldn't need to know that. > > I think we should just revert d70c996df23f and then apply the same > change to kvm_arm_pmu_supported() that other similar functions got > with 4f7f589381d5.
On Wed, Jun 17, 2020 at 07:37:42PM +0200, Paolo Bonzini wrote: > On 17/06/20 17:23, Andrew Jones wrote: > >> > >> Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState > >> argument (already realized/valid at this point) instead of a > >> CPUState argument. > > I'd rather not do that. IMO, a CPU feature test should operate on CPU, > > not an "accelerator". > > If it's a test that the feature is enabled (e.g. via -cpu) then I agree. > For something that ends up as a KVM_CHECK_EXTENSION or KVM_ENABLE_CAP on > the KVM fd, however, I think passing an AccelState is better. I can live with that justification as long as we don't support heterogeneous VCPU configurations. And, if that ever happens, then I guess we'll be reworking a lot more than just the interface of these cpu feature probes. Thanks, drew > kvm_arm_pmu_supported case is clearly the latter, even the error message > hints at that: > > + if (kvm_enabled() && !kvm_arm_pmu_supported(current_accel())) { > error_setg(errp, "'pmu' feature not supported by KVM on this host"); > return; > } > > but the same is true of kvm_arm_aarch32_supported and kvm_arm_sve_supported. > > Applying the change to kvm_arm_pmu_supported as you suggest below would be > a bit of a bandaid because it would not have consistent prototypes. Sp > for Philippe's patch > > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > > Thanks, > > Paolo > > > How that test is implemented is another story. > > If the CPUState isn't interesting, but it points to something that is, > > or there's another function that uses globals to get the job done, then > > fine, but the callers of a CPU feature test shouldn't need to know that. > > > > I think we should just revert d70c996df23f and then apply the same > > change to kvm_arm_pmu_supported() that other similar functions got > > with 4f7f589381d5. > >
On 18/06/20 10:57, Andrew Jones wrote: >> If it's a test that the feature is enabled (e.g. via -cpu) then I agree. >> For something that ends up as a KVM_CHECK_EXTENSION or KVM_ENABLE_CAP on >> the KVM fd, however, I think passing an AccelState is better. > I can live with that justification as long as we don't support > heterogeneous VCPU configurations. And, if that ever happens, then I > guess we'll be reworking a lot more than just the interface of these > cpu feature probes. Yes, and anyway configuring "what is allowed" would be separate from checking "what is supported". Thanks,
On Wed, Jun 17, 2020 at 03:08:00PM +0200, Philippe Mathieu-Daudé wrote: > Since commit d70c996df23f, when enabling the PMU we get: > > $ qemu-system-aarch64 -cpu host,pmu=on -M virt,accel=kvm,gic-version=3 > Segmentation fault (core dumped) > > Thread 1 "qemu-system-aar" received signal SIGSEGV, Segmentation fault. > 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588 > 2588 ret = ioctl(s->fd, type, arg); > (gdb) bt > #0 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588 > #1 0x0000aaaaaae31568 in kvm_check_extension (s=0x0, extension=126) at accel/kvm/kvm-all.c:916 > #2 0x0000aaaaaafce254 in kvm_arm_pmu_supported (cpu=0xaaaaac214ab0) at target/arm/kvm.c:213 > #3 0x0000aaaaaafc0f94 in arm_set_pmu (obj=0xaaaaac214ab0, value=true, errp=0xffffffffe438) at target/arm/cpu.c:1111 > #4 0x0000aaaaab5533ac in property_set_bool (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", opaque=0xaaaaac222730, errp=0xffffffffe438) at qom/object.c:2170 > #5 0x0000aaaaab5512f0 in object_property_set (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1328 > #6 0x0000aaaaab551e10 in object_property_parse (obj=0xaaaaac214ab0, string=0xaaaaac11b4c0 "on", name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1561 > #7 0x0000aaaaab54ee8c in object_apply_global_props (obj=0xaaaaac214ab0, props=0xaaaaac018e20, errp=0xaaaaabd6fd88 <error_fatal>) at qom/object.c:407 > #8 0x0000aaaaab1dd5a4 in qdev_prop_set_globals (dev=0xaaaaac214ab0) at hw/core/qdev-properties.c:1218 > #9 0x0000aaaaab1d9fac in device_post_init (obj=0xaaaaac214ab0) at hw/core/qdev.c:1050 > ... > #15 0x0000aaaaab54f310 in object_initialize_with_type (obj=0xaaaaac214ab0, size=52208, type=0xaaaaabe237f0) at qom/object.c:512 > #16 0x0000aaaaab54fa24 in object_new_with_type (type=0xaaaaabe237f0) at qom/object.c:687 > #17 0x0000aaaaab54fa80 in object_new (typename=0xaaaaabe23970 "host-arm-cpu") at qom/object.c:702 > #18 0x0000aaaaaaf04a74 in machvirt_init (machine=0xaaaaac0a8550) at hw/arm/virt.c:1770 > #19 0x0000aaaaab1e8720 in machine_run_board_init (machine=0xaaaaac0a8550) at hw/core/machine.c:1138 > #20 0x0000aaaaaaf95394 in qemu_init (argc=5, argv=0xffffffffea58, envp=0xffffffffea88) at softmmu/vl.c:4348 > #21 0x0000aaaaaada3f74 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:48 > > This is because in frame #2, cpu->kvm_state is still NULL > (the vCPU is not yet realized). > > KVM has a hard requirement of all cores supporting the same > feature set. We only need to check if the accelerator supports > a feature, not each vCPU individually. > > Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState > argument (already realized/valid at this point) instead of a > CPUState argument. > > Reported-by: Haibo Xu <haibo.xu@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > target/arm/kvm_arm.h | 25 +++++++++++++------------ > target/arm/cpu.c | 2 +- > target/arm/cpu64.c | 10 +++++----- > target/arm/kvm.c | 4 ++-- > target/arm/kvm64.c | 14 +++++--------- > 5 files changed, 26 insertions(+), 29 deletions(-) > > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > index 48bf5e16d5..8209525f20 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -12,6 +12,7 @@ > #define QEMU_KVM_ARM_H > > #include "sysemu/kvm.h" > +#include "sysemu/accel.h" > #include "exec/memory.h" > #include "qemu/error-report.h" > > @@ -269,29 +270,29 @@ void kvm_arm_add_vcpu_properties(Object *obj); > > /** > * kvm_arm_aarch32_supported: > - * @cs: CPUState > + * @as: AccelState > * > - * Returns: true if the KVM VCPU can enable AArch32 mode > + * Returns: true if the KVM accelerator can enable AArch32 mode > * and false otherwise. > */ > -bool kvm_arm_aarch32_supported(CPUState *cs); > +bool kvm_arm_aarch32_supported(AccelState *as); > > /** > * kvm_arm_pmu_supported: > - * @cs: CPUState > + * @as: AccelState > * > - * Returns: true if the KVM VCPU can enable its PMU > + * Returns: true if the KVM accelerator can enable its PMU > * and false otherwise. > */ > -bool kvm_arm_pmu_supported(CPUState *cs); > +bool kvm_arm_pmu_supported(AccelState *as); > > /** > * kvm_arm_sve_supported: > - * @cs: CPUState > + * @as: AccelState > * > - * Returns true if the KVM VCPU can enable SVE and false otherwise. > + * Returns true if the KVM accelerator can enable SVE and false otherwise. > */ > -bool kvm_arm_sve_supported(CPUState *cs); > +bool kvm_arm_sve_supported(AccelState *as); > > /** > * kvm_arm_get_max_vm_ipa_size: > @@ -359,17 +360,17 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) > > static inline void kvm_arm_add_vcpu_properties(Object *obj) {} > > -static inline bool kvm_arm_aarch32_supported(CPUState *cs) > +static inline bool kvm_arm_aarch32_supported(AccelState *as) > { > return false; > } > > -static inline bool kvm_arm_pmu_supported(CPUState *cs) > +static inline bool kvm_arm_pmu_supported(AccelState *as) > { > return false; > } > > -static inline bool kvm_arm_sve_supported(CPUState *cs) > +static inline bool kvm_arm_sve_supported(AccelState *as) > { > return false; > } > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 5b7a36b5d7..29b314427c 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1108,7 +1108,7 @@ static void arm_set_pmu(Object *obj, bool value, Error **errp) > ARMCPU *cpu = ARM_CPU(obj); > > if (value) { > - if (kvm_enabled() && !kvm_arm_pmu_supported(CPU(cpu))) { > + if (kvm_enabled() && !kvm_arm_pmu_supported(current_accel())) { > error_setg(errp, "'pmu' feature not supported by KVM on this host"); > return; > } > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 778cecc2e6..13835768ab 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -266,7 +266,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp) > > /* Collect the set of vector lengths supported by KVM. */ > bitmap_zero(kvm_supported, ARM_MAX_VQ); > - if (kvm_enabled() && kvm_arm_sve_supported(CPU(cpu))) { > + if (kvm_enabled() && kvm_arm_sve_supported(current_accel())) { > kvm_arm_sve_get_vls(CPU(cpu), kvm_supported); > } else if (kvm_enabled()) { > assert(!cpu_isar_feature(aa64_sve, cpu)); > @@ -473,7 +473,7 @@ static void cpu_max_set_sve_max_vq(Object *obj, Visitor *v, const char *name, > return; > } > > - if (kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) { > + if (kvm_enabled() && !kvm_arm_sve_supported(current_accel())) { > error_setg(errp, "cannot set sve-max-vq"); > error_append_hint(errp, "SVE not supported by KVM on this host\n"); > return; > @@ -519,7 +519,7 @@ static void cpu_arm_set_sve_vq(Object *obj, Visitor *v, const char *name, > return; > } > > - if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) { > + if (value && kvm_enabled() && !kvm_arm_sve_supported(current_accel())) { > error_setg(errp, "cannot enable %s", name); > error_append_hint(errp, "SVE not supported by KVM on this host\n"); > return; > @@ -556,7 +556,7 @@ static void cpu_arm_set_sve(Object *obj, Visitor *v, const char *name, > return; > } > > - if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) { > + if (value && kvm_enabled() && !kvm_arm_sve_supported(current_accel())) { > error_setg(errp, "'sve' feature not supported by KVM on this host"); > return; > } > @@ -751,7 +751,7 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) > * uniform execution state like do_interrupt. > */ > if (value == false) { > - if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { > + if (!kvm_enabled() || !kvm_arm_aarch32_supported(current_accel())) { > error_setg(errp, "'aarch64' feature cannot be disabled " > "unless KVM is enabled and 32-bit EL1 " > "is supported"); > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index eef3bbd1cc..2247a96757 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -208,9 +208,9 @@ void kvm_arm_add_vcpu_properties(Object *obj) > } > } > > -bool kvm_arm_pmu_supported(CPUState *cpu) > +bool kvm_arm_pmu_supported(AccelState *as) > { > - return kvm_check_extension(cpu->kvm_state, KVM_CAP_ARM_PMU_V3); > + return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_PMU_V3); > } > > int kvm_arm_get_max_vm_ipa_size(MachineState *ms) > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index f09ed9f4df..ae4e37ce78 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -652,18 +652,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > return true; > } > > -bool kvm_arm_aarch32_supported(CPUState *cpu) > +bool kvm_arm_aarch32_supported(AccelState *as) > { > - KVMState *s = KVM_STATE(current_accel()); > - > - return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); > + return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_EL1_32BIT); > } > > -bool kvm_arm_sve_supported(CPUState *cpu) > +bool kvm_arm_sve_supported(AccelState *as) > { > - KVMState *s = KVM_STATE(current_accel()); > - > - return kvm_check_extension(s, KVM_CAP_ARM_SVE); > + return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_SVE); > } > > QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1); > @@ -798,7 +794,7 @@ int kvm_arch_init_vcpu(CPUState *cs) > env->features &= ~(1ULL << ARM_FEATURE_PMU); > } > if (cpu_isar_feature(aa64_sve, cpu)) { > - assert(kvm_arm_sve_supported(cs)); > + assert(kvm_arm_sve_supported(ACCEL(cs->kvm_state))); Might as well use current_accel() here too, right? > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; > } > > -- > 2.21.3 > > At all callsites we pass current_accel() to the kvm_arm_<feat>_supported() functions. Is there any reason not to drop their input parameter and just use current_accel() internally? Thanks, drew
On 6/18/20 11:22 AM, Andrew Jones wrote: > On Wed, Jun 17, 2020 at 03:08:00PM +0200, Philippe Mathieu-Daudé wrote: >> Since commit d70c996df23f, when enabling the PMU we get: >> >> $ qemu-system-aarch64 -cpu host,pmu=on -M virt,accel=kvm,gic-version=3 >> Segmentation fault (core dumped) >> >> Thread 1 "qemu-system-aar" received signal SIGSEGV, Segmentation fault. >> 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588 >> 2588 ret = ioctl(s->fd, type, arg); >> (gdb) bt >> #0 0x0000aaaaaae356d0 in kvm_ioctl (s=0x0, type=44547) at accel/kvm/kvm-all.c:2588 >> #1 0x0000aaaaaae31568 in kvm_check_extension (s=0x0, extension=126) at accel/kvm/kvm-all.c:916 >> #2 0x0000aaaaaafce254 in kvm_arm_pmu_supported (cpu=0xaaaaac214ab0) at target/arm/kvm.c:213 >> #3 0x0000aaaaaafc0f94 in arm_set_pmu (obj=0xaaaaac214ab0, value=true, errp=0xffffffffe438) at target/arm/cpu.c:1111 >> #4 0x0000aaaaab5533ac in property_set_bool (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", opaque=0xaaaaac222730, errp=0xffffffffe438) at qom/object.c:2170 >> #5 0x0000aaaaab5512f0 in object_property_set (obj=0xaaaaac214ab0, v=0xaaaaac223a80, name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1328 >> #6 0x0000aaaaab551e10 in object_property_parse (obj=0xaaaaac214ab0, string=0xaaaaac11b4c0 "on", name=0xaaaaac11a970 "pmu", errp=0xffffffffe438) at qom/object.c:1561 >> #7 0x0000aaaaab54ee8c in object_apply_global_props (obj=0xaaaaac214ab0, props=0xaaaaac018e20, errp=0xaaaaabd6fd88 <error_fatal>) at qom/object.c:407 >> #8 0x0000aaaaab1dd5a4 in qdev_prop_set_globals (dev=0xaaaaac214ab0) at hw/core/qdev-properties.c:1218 >> #9 0x0000aaaaab1d9fac in device_post_init (obj=0xaaaaac214ab0) at hw/core/qdev.c:1050 >> ... >> #15 0x0000aaaaab54f310 in object_initialize_with_type (obj=0xaaaaac214ab0, size=52208, type=0xaaaaabe237f0) at qom/object.c:512 >> #16 0x0000aaaaab54fa24 in object_new_with_type (type=0xaaaaabe237f0) at qom/object.c:687 >> #17 0x0000aaaaab54fa80 in object_new (typename=0xaaaaabe23970 "host-arm-cpu") at qom/object.c:702 >> #18 0x0000aaaaaaf04a74 in machvirt_init (machine=0xaaaaac0a8550) at hw/arm/virt.c:1770 >> #19 0x0000aaaaab1e8720 in machine_run_board_init (machine=0xaaaaac0a8550) at hw/core/machine.c:1138 >> #20 0x0000aaaaaaf95394 in qemu_init (argc=5, argv=0xffffffffea58, envp=0xffffffffea88) at softmmu/vl.c:4348 >> #21 0x0000aaaaaada3f74 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at softmmu/main.c:48 >> >> This is because in frame #2, cpu->kvm_state is still NULL >> (the vCPU is not yet realized). >> >> KVM has a hard requirement of all cores supporting the same >> feature set. We only need to check if the accelerator supports >> a feature, not each vCPU individually. >> >> Fix by kvm_arm_<FEATURE>_supported() functions take a AccelState >> argument (already realized/valid at this point) instead of a >> CPUState argument. >> >> Reported-by: Haibo Xu <haibo.xu@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> target/arm/kvm_arm.h | 25 +++++++++++++------------ >> target/arm/cpu.c | 2 +- >> target/arm/cpu64.c | 10 +++++----- >> target/arm/kvm.c | 4 ++-- >> target/arm/kvm64.c | 14 +++++--------- >> 5 files changed, 26 insertions(+), 29 deletions(-) >> >> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h >> index 48bf5e16d5..8209525f20 100644 >> --- a/target/arm/kvm_arm.h >> +++ b/target/arm/kvm_arm.h >> @@ -12,6 +12,7 @@ >> #define QEMU_KVM_ARM_H >> >> #include "sysemu/kvm.h" >> +#include "sysemu/accel.h" >> #include "exec/memory.h" >> #include "qemu/error-report.h" >> >> @@ -269,29 +270,29 @@ void kvm_arm_add_vcpu_properties(Object *obj); >> >> /** >> * kvm_arm_aarch32_supported: >> - * @cs: CPUState >> + * @as: AccelState >> * >> - * Returns: true if the KVM VCPU can enable AArch32 mode >> + * Returns: true if the KVM accelerator can enable AArch32 mode >> * and false otherwise. >> */ >> -bool kvm_arm_aarch32_supported(CPUState *cs); >> +bool kvm_arm_aarch32_supported(AccelState *as); >> >> /** >> * kvm_arm_pmu_supported: >> - * @cs: CPUState >> + * @as: AccelState >> * >> - * Returns: true if the KVM VCPU can enable its PMU >> + * Returns: true if the KVM accelerator can enable its PMU >> * and false otherwise. >> */ >> -bool kvm_arm_pmu_supported(CPUState *cs); >> +bool kvm_arm_pmu_supported(AccelState *as); >> >> /** >> * kvm_arm_sve_supported: >> - * @cs: CPUState >> + * @as: AccelState >> * >> - * Returns true if the KVM VCPU can enable SVE and false otherwise. >> + * Returns true if the KVM accelerator can enable SVE and false otherwise. >> */ >> -bool kvm_arm_sve_supported(CPUState *cs); >> +bool kvm_arm_sve_supported(AccelState *as); >> >> /** >> * kvm_arm_get_max_vm_ipa_size: >> @@ -359,17 +360,17 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) >> >> static inline void kvm_arm_add_vcpu_properties(Object *obj) {} >> >> -static inline bool kvm_arm_aarch32_supported(CPUState *cs) >> +static inline bool kvm_arm_aarch32_supported(AccelState *as) >> { >> return false; >> } >> >> -static inline bool kvm_arm_pmu_supported(CPUState *cs) >> +static inline bool kvm_arm_pmu_supported(AccelState *as) >> { >> return false; >> } >> >> -static inline bool kvm_arm_sve_supported(CPUState *cs) >> +static inline bool kvm_arm_sve_supported(AccelState *as) >> { >> return false; >> } >> diff --git a/target/arm/cpu.c b/target/arm/cpu.c >> index 5b7a36b5d7..29b314427c 100644 >> --- a/target/arm/cpu.c >> +++ b/target/arm/cpu.c >> @@ -1108,7 +1108,7 @@ static void arm_set_pmu(Object *obj, bool value, Error **errp) >> ARMCPU *cpu = ARM_CPU(obj); >> >> if (value) { >> - if (kvm_enabled() && !kvm_arm_pmu_supported(CPU(cpu))) { >> + if (kvm_enabled() && !kvm_arm_pmu_supported(current_accel())) { >> error_setg(errp, "'pmu' feature not supported by KVM on this host"); >> return; >> } >> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c >> index 778cecc2e6..13835768ab 100644 >> --- a/target/arm/cpu64.c >> +++ b/target/arm/cpu64.c >> @@ -266,7 +266,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp) >> >> /* Collect the set of vector lengths supported by KVM. */ >> bitmap_zero(kvm_supported, ARM_MAX_VQ); >> - if (kvm_enabled() && kvm_arm_sve_supported(CPU(cpu))) { >> + if (kvm_enabled() && kvm_arm_sve_supported(current_accel())) { >> kvm_arm_sve_get_vls(CPU(cpu), kvm_supported); >> } else if (kvm_enabled()) { >> assert(!cpu_isar_feature(aa64_sve, cpu)); >> @@ -473,7 +473,7 @@ static void cpu_max_set_sve_max_vq(Object *obj, Visitor *v, const char *name, >> return; >> } >> >> - if (kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) { >> + if (kvm_enabled() && !kvm_arm_sve_supported(current_accel())) { >> error_setg(errp, "cannot set sve-max-vq"); >> error_append_hint(errp, "SVE not supported by KVM on this host\n"); >> return; >> @@ -519,7 +519,7 @@ static void cpu_arm_set_sve_vq(Object *obj, Visitor *v, const char *name, >> return; >> } >> >> - if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) { >> + if (value && kvm_enabled() && !kvm_arm_sve_supported(current_accel())) { >> error_setg(errp, "cannot enable %s", name); >> error_append_hint(errp, "SVE not supported by KVM on this host\n"); >> return; >> @@ -556,7 +556,7 @@ static void cpu_arm_set_sve(Object *obj, Visitor *v, const char *name, >> return; >> } >> >> - if (value && kvm_enabled() && !kvm_arm_sve_supported(CPU(cpu))) { >> + if (value && kvm_enabled() && !kvm_arm_sve_supported(current_accel())) { >> error_setg(errp, "'sve' feature not supported by KVM on this host"); >> return; >> } >> @@ -751,7 +751,7 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) >> * uniform execution state like do_interrupt. >> */ >> if (value == false) { >> - if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { >> + if (!kvm_enabled() || !kvm_arm_aarch32_supported(current_accel())) { >> error_setg(errp, "'aarch64' feature cannot be disabled " >> "unless KVM is enabled and 32-bit EL1 " >> "is supported"); >> diff --git a/target/arm/kvm.c b/target/arm/kvm.c >> index eef3bbd1cc..2247a96757 100644 >> --- a/target/arm/kvm.c >> +++ b/target/arm/kvm.c >> @@ -208,9 +208,9 @@ void kvm_arm_add_vcpu_properties(Object *obj) >> } >> } >> >> -bool kvm_arm_pmu_supported(CPUState *cpu) >> +bool kvm_arm_pmu_supported(AccelState *as) >> { >> - return kvm_check_extension(cpu->kvm_state, KVM_CAP_ARM_PMU_V3); >> + return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_PMU_V3); >> } >> >> int kvm_arm_get_max_vm_ipa_size(MachineState *ms) >> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c >> index f09ed9f4df..ae4e37ce78 100644 >> --- a/target/arm/kvm64.c >> +++ b/target/arm/kvm64.c >> @@ -652,18 +652,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >> return true; >> } >> >> -bool kvm_arm_aarch32_supported(CPUState *cpu) >> +bool kvm_arm_aarch32_supported(AccelState *as) >> { >> - KVMState *s = KVM_STATE(current_accel()); >> - >> - return kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); >> + return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_EL1_32BIT); >> } >> >> -bool kvm_arm_sve_supported(CPUState *cpu) >> +bool kvm_arm_sve_supported(AccelState *as) >> { >> - KVMState *s = KVM_STATE(current_accel()); >> - >> - return kvm_check_extension(s, KVM_CAP_ARM_SVE); >> + return kvm_check_extension(KVM_STATE(as), KVM_CAP_ARM_SVE); >> } >> >> QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1); >> @@ -798,7 +794,7 @@ int kvm_arch_init_vcpu(CPUState *cs) >> env->features &= ~(1ULL << ARM_FEATURE_PMU); >> } >> if (cpu_isar_feature(aa64_sve, cpu)) { >> - assert(kvm_arm_sve_supported(cs)); >> + assert(kvm_arm_sve_supported(ACCEL(cs->kvm_state))); > > Might as well use current_accel() here too, right? I was not sure, I used kvm_state since at this point we are sure it is initialized. > >> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; >> } >> >> -- >> 2.21.3 >> >> > > At all callsites we pass current_accel() to the kvm_arm_<feat>_supported() > functions. Is there any reason not to drop their input parameter and just > use current_accel() internally? Clever idea :) > > Thanks, > drew >
On 18/06/20 12:17, Philippe Mathieu-Daudé wrote: >>> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; >>> } >>> >>> -- >>> 2.21.3 >>> >>> >> At all callsites we pass current_accel() to the kvm_arm_<feat>_supported() >> functions. Is there any reason not to drop their input parameter and just >> use current_accel() internally? > Clever idea :) Or just the kvm_state global. Paolo
© 2016 - 2024 Red Hat, Inc.