Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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,
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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__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
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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. >> 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at >> accel/kvm/kvm-all.c:2588 >> 2588ret = ioctl(s->fd, type, arg); >> (gdb) bt >> #0 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at >> accel/kvm/kvm-all.c:2588 >> #1 0xaae31568 in kvm_check_extension (s=0x0, extension=126) at >> accel/kvm/kvm-all.c:916 >> #2 0xaafce254 in kvm_arm_pmu_supported (cpu=0xac214ab0) at >> target/arm/kvm.c:213 >> #3 0xaafc0f94 in arm_set_pmu (obj=0xac214ab0, value=true, >> errp=0xe438) at target/arm/cpu.c: >> #4 0xab5533ac in property_set_bool (obj=0xac214ab0, >> v=0xac223a80, name=0xac11a970 "pmu", opaque=0xac222730, >> errp=0xe438) at qom/object.c:2170 >> #5 0xab5512f0 in object_property_set (obj=0xac214ab0, >> v=0xac223a80, name=0xac11a970 "pmu", errp=0xe438) at >> qom/object.c:1328 >> #6 0xab551e10 in object_property_parse (obj=0xac214ab0, >> string=0xac11b4c0 "on", name=0xac11a970 "pmu", errp=0xe438) >> at qom/object.c:1561 >> #7 0xab54ee8c in object_apply_global_props (obj=0xac214ab0, >> props=0xac018e20, errp=0xabd6fd88 ) at qom/object.c:407 >> #8 0xab1dd5a4 in qdev_prop_set_globals (dev=0xac214ab0) at >> hw/core/qdev-properties.c:1218 >> #9 0xab1d9fac in device_post_init (obj=0xac214ab0) at >> hw/core/qdev.c:1050 >> ... >> #15 0xab54f310 in object_initialize_with_type (obj=0xac214ab0, >> size=52208, type=0xabe237f0) at qom/object.c:512 >> #16 0xab54fa24 in object_new_with_type (type=0xabe237f0) at >> qom/object.c:687 >> #17 0xab54fa80 in object_new (typename=0xabe23970 >> "host-arm-cpu") at qom/object.c:702 >> #18 0xaaf04a74 in machvirt_init (machine=0xac0a8550) at >> hw/arm/virt.c:1770 >> #19 0xab1e8720 in machine_run_board_init (machine=0xac0a8550) >> at hw/core/machine.c:1138 >> #20 0xaaf95394 in qemu_init (argc=5, argv=0xea58, >> envp=0xea88) at softmmu/vl.c:4348 >> #21 0xaada3f74 in main (argc=, argv=> out>, envp=) 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__supported() functions take a AccelState >> argument (already realized/valid at this point) instead of a >> CPUState argument. >> >> Reported-by: Haibo Xu >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> 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
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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.
> 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at
> accel/kvm/kvm-all.c:2588
> 2588ret = ioctl(s->fd, type, arg);
> (gdb) bt
> #0 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at
> accel/kvm/kvm-all.c:2588
> #1 0xaae31568 in kvm_check_extension (s=0x0, extension=126) at
> accel/kvm/kvm-all.c:916
> #2 0xaafce254 in kvm_arm_pmu_supported (cpu=0xac214ab0) at
> target/arm/kvm.c:213
> #3 0xaafc0f94 in arm_set_pmu (obj=0xac214ab0, value=true,
> errp=0xe438) at target/arm/cpu.c:
> #4 0xab5533ac in property_set_bool (obj=0xac214ab0,
> v=0xac223a80, name=0xac11a970 "pmu", opaque=0xac222730,
> errp=0xe438) at qom/object.c:2170
> #5 0xab5512f0 in object_property_set (obj=0xac214ab0,
> v=0xac223a80, name=0xac11a970 "pmu", errp=0xe438) at
> qom/object.c:1328
> #6 0xab551e10 in object_property_parse (obj=0xac214ab0,
> string=0xac11b4c0 "on", name=0xac11a970 "pmu", errp=0xe438)
> at qom/object.c:1561
> #7 0xab54ee8c in object_apply_global_props (obj=0xac214ab0,
> props=0xac018e20, errp=0xabd6fd88 ) at qom/object.c:407
> #8 0xab1dd5a4 in qdev_prop_set_globals (dev=0xac214ab0) at
> hw/core/qdev-properties.c:1218
> #9 0xab1d9fac in device_post_init (obj=0xac214ab0) at
> hw/core/qdev.c:1050
> ...
> #15 0xab54f310 in object_initialize_with_type (obj=0xac214ab0,
> size=52208, type=0xabe237f0) at qom/object.c:512
> #16 0xab54fa24 in object_new_with_type (type=0xabe237f0) at
> qom/object.c:687
> #17 0xab54fa80 in object_new (typename=0xabe23970
> "host-arm-cpu") at qom/object.c:702
> #18 0xaaf04a74 in machvirt_init (machine=0xac0a8550) at
> hw/arm/virt.c:1770
> #19 0xab1e8720 in machine_run_board_init (machine=0xac0a8550)
> at hw/core/machine.c:1138
> #20 0xaaf95394 in qemu_init (argc=5, argv=0xea58,
> envp=0xea88) at softmmu/vl.c:4348
> #21 0xaada3f74 in main (argc=, argv=,
> envp=) 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__supported() functions take a AccelState
> argument (already realized/valid at this point) instead of a
> CPUState argument.
>
> Reported-by: Haibo Xu
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> 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
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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__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
>
> 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.
>
>
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
On 17/06/20 17:23, Andrew Jones wrote:
>>
>> Fix by kvm_arm__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
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.
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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. > 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at > accel/kvm/kvm-all.c:2588 > 2588ret = ioctl(s->fd, type, arg); > (gdb) bt > #0 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at > accel/kvm/kvm-all.c:2588 > #1 0xaae31568 in kvm_check_extension (s=0x0, extension=126) at > accel/kvm/kvm-all.c:916 > #2 0xaafce254 in kvm_arm_pmu_supported (cpu=0xac214ab0) at > target/arm/kvm.c:213 > #3 0xaafc0f94 in arm_set_pmu (obj=0xac214ab0, value=true, > errp=0xe438) at target/arm/cpu.c: > #4 0xab5533ac in property_set_bool (obj=0xac214ab0, > v=0xac223a80, name=0xac11a970 "pmu", opaque=0xac222730, > errp=0xe438) at qom/object.c:2170 > #5 0xab5512f0 in object_property_set (obj=0xac214ab0, > v=0xac223a80, name=0xac11a970 "pmu", errp=0xe438) at > qom/object.c:1328 > #6 0xab551e10 in object_property_parse (obj=0xac214ab0, > string=0xac11b4c0 "on", name=0xac11a970 "pmu", errp=0xe438) > at qom/object.c:1561 > #7 0xab54ee8c in object_apply_global_props (obj=0xac214ab0, > props=0xac018e20, errp=0xabd6fd88 ) at qom/object.c:407 > #8 0xab1dd5a4 in qdev_prop_set_globals (dev=0xac214ab0) at > hw/core/qdev-properties.c:1218 > #9 0xab1d9fac in device_post_init (obj=0xac214ab0) at > hw/core/qdev.c:1050 > ... > #15 0xab54f310 in object_initialize_with_type (obj=0xac214ab0, > size=52208, type=0xabe237f0) at qom/object.c:512 > #16 0xab54fa24 in object_new_with_type (type=0xabe237f0) at > qom/object.c:687 > #17 0xab54fa80 in object_new (typename=0xabe23970 > "host-arm-cpu") at qom/object.c:702 > #18 0xaaf04a74 in machvirt_init (machine=0xac0a8550) at > hw/arm/virt.c:1770 > #19 0xab1e8720 in machine_run_board_init (machine=0xac0a8550) > at hw/core/machine.c:1138 > #20 0xaaf95394 in qemu_init (argc=5, argv=0xea58, > envp=0xea88) at softmmu/vl.c:4348 > #21 0xaada3f74 in main (argc=, argv=, > envp=) 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__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
Re: [PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
Patchew URL: https://patchew.org/QEMU/20200617130800.26355-1-phi...@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) LINKqemu-keymap LINKivshmem-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) LINKivshmem-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) LINKqemu-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) LINKqemu-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) LINKqemu-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 LINKqemu-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) LINKqemu-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) LINKfsdev/virtfs-proxy-helper CC pc-bios/optionrom/pvh_main.o LINKscsi/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 LINKqemu-bridge-helper --- BUILD pc-bios/optionrom/linuxboot.raw BUILD pc-bios/optionrom/linuxboot_dma.img LINKvirtiofsd /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) SIGNpc-bios/optionrom/multiboot.bin SIGNpc-bios/optionrom/linuxboot.bin BUILD pc-bios/optionrom/linuxboot_dma.raw --- BUILD pc-bios/optionrom/pvh.img BUILD pc-bios/optionrom/pvh.raw SIGNpc-bios/optionrom/pvh.bin /usr/bin/ld:
[PATCH] target/arm/kvm: Check supported feature per accelerator (not per vCPU)
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.
0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at
accel/kvm/kvm-all.c:2588
2588ret = ioctl(s->fd, type, arg);
(gdb) bt
#0 0xaae356d0 in kvm_ioctl (s=0x0, type=44547) at
accel/kvm/kvm-all.c:2588
#1 0xaae31568 in kvm_check_extension (s=0x0, extension=126) at
accel/kvm/kvm-all.c:916
#2 0xaafce254 in kvm_arm_pmu_supported (cpu=0xac214ab0) at
target/arm/kvm.c:213
#3 0xaafc0f94 in arm_set_pmu (obj=0xac214ab0, value=true,
errp=0xe438) at target/arm/cpu.c:
#4 0xab5533ac in property_set_bool (obj=0xac214ab0,
v=0xac223a80, name=0xac11a970 "pmu", opaque=0xac222730,
errp=0xe438) at qom/object.c:2170
#5 0xab5512f0 in object_property_set (obj=0xac214ab0,
v=0xac223a80, name=0xac11a970 "pmu", errp=0xe438) at
qom/object.c:1328
#6 0xab551e10 in object_property_parse (obj=0xac214ab0,
string=0xac11b4c0 "on", name=0xac11a970 "pmu", errp=0xe438) at
qom/object.c:1561
#7 0xab54ee8c in object_apply_global_props (obj=0xac214ab0,
props=0xac018e20, errp=0xabd6fd88 ) at qom/object.c:407
#8 0xab1dd5a4 in qdev_prop_set_globals (dev=0xac214ab0) at
hw/core/qdev-properties.c:1218
#9 0xab1d9fac in device_post_init (obj=0xac214ab0) at
hw/core/qdev.c:1050
...
#15 0xab54f310 in object_initialize_with_type (obj=0xac214ab0,
size=52208, type=0xabe237f0) at qom/object.c:512
#16 0xab54fa24 in object_new_with_type (type=0xabe237f0) at
qom/object.c:687
#17 0xab54fa80 in object_new (typename=0xabe23970 "host-arm-cpu")
at qom/object.c:702
#18 0xaaf04a74 in machvirt_init (machine=0xac0a8550) at
hw/arm/virt.c:1770
#19 0xab1e8720 in machine_run_board_init (machine=0xac0a8550) at
hw/core/machine.c:1138
#20 0xaaf95394 in qemu_init (argc=5, argv=0xea58,
envp=0xea88) at softmmu/vl.c:4348
#21 0xaada3f74 in main (argc=, argv=,
envp=) 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__supported() functions take a AccelState
argument (already realized/valid at this point) instead of a
CPUState argument.
Reported-by: Haibo Xu
Signed-off-by: Philippe Mathieu-Daudé
---
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
