On Wed, Jun 17, 2020 at 12:32:09PM +0200, Philippe Mathieu-Daudé wrote: > On 6/17/20 10:23 AM, Philippe Mathieu-Daudé wrote: > > On 6/11/20 11:14 AM, Andrew Jones wrote: > >> On Thu, Jun 11, 2020 at 04:46:45PM +0800, Haibo Xu wrote: > >>> Hi, > >>> > >>> I met a qemu core dump issue when starting a VM with cpu feature > >>> "pmu=on" on an arm server. > >>> The commands to start the machine is: > >>> > >>> ./qemu-system-aarch64 \ > >>> -cpu host,pmu=on -M virt,accel=kvm,gic-version=3 -nographic > >>> -m 2048M \ > >>> -kernel ./Image \ > >>> -initrd /boot/initrd.img-5.6.0-rc2+ \ > >>> -append "root=/dev/vda rw console=ttyAMA0" -nodefaults -serial > >>> stdio\ > >>> -drive if=none,file=./xenial.rootfs.ext4,id=hd0,format=raw \ > >>> -device virtio-blk-device,drive=hd0 > >>> > >>> > >>> And here is the stack dump: > >>> > >>> Core was generated by `./qemu-system-aarch64 -cpu host,pmu=on -M > >>> virt,accel=kvm,gic-version=3 -nograph'. > >>> Program terminated with signal SIGSEGV, Segmentation fault. > >>> #0 kvm_ioctl (s=0x0, type=type@entry=44547) at > >> > >> s=0x0 means cpu->kvm_state is NULL > >> > >>> The root cause is in the arm_get_pmu() operation which was introduced > >>> in ae502508f83. > >> > >> Actually the root cause is d70c996df23f ("target/arm/kvm: Use > >> CPUState::kvm_state in kvm_arm_pmu_supported()"). ae502508f83 used > >> the machine kvm_state, not the cpu kvm_state, and that allows pmu=on > >> to work. d70c996df23f changed that saying that "KVMState is already > >> accessible via CPUState::kvm_state, use it.", but I'm not sure why, > >> since kvm_init_vcpu() doesn't run until the vcpu thread is created. > >> > >> Philippe? > > > > Sorry for some reason I missed this email. I'll look at this today. > > Quick reproducer: > > $ qemu-system-aarch64 -cpu host,pmu=on -M virt,accel=kvm,gic-version=3 > Segmentation fault (core dumped) > > Eduardo, I thought we had a 'machine' qtest testing for various > combination of properties, but I can't find it, do you remember? > Or maybe it was Thomas? Or Markus? =)
For arm cpu properties we have tests/qtest/arm-cpu-features. We can add a regression test for this and other properties there. I just tried it. I needed to add a new macro (see below), but otherwise it worked to reproduce the problem. Thanks, drew diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c index 469217367661..d6bdbc171893 100644 --- a/tests/qtest/arm-cpu-features.c +++ b/tests/qtest/arm-cpu-features.c @@ -159,16 +159,35 @@ static bool resp_get_feature(QDict *resp, const char *feature) qobject_unref(_resp); \ }) -#define assert_feature(qts, cpu_type, feature, expected_value) \ +#define resp_assert_feature(resp, feature, expected_value) \ ({ \ - QDict *_resp, *_props; \ + QDict *_props; \ \ - _resp = do_query_no_props(qts, cpu_type); \ g_assert(_resp); \ g_assert(resp_has_props(_resp)); \ _props = resp_get_props(_resp); \ g_assert(qdict_get(_props, feature)); \ g_assert(qdict_get_bool(_props, feature) == (expected_value)); \ +}) + +#define assert_feature(qts, cpu_type, feature, expected_value) \ +({ \ + QDict *_resp; \ + \ + _resp = do_query_no_props(qts, cpu_type); \ + g_assert(_resp); \ + resp_assert_feature(_resp, feature, expected_value); \ + qobject_unref(_resp); \ +}) + +#define assert_set_feature(qts, cpu_type, feature, value) \ +({ \ + const char *_fmt = (value) ? "{ %s: true }" : "{ %s: false }"; \ + QDict *_resp; \ + \ + _resp = do_query(qts, cpu_type, _fmt, feature); \ + g_assert(_resp); \ + resp_assert_feature(_resp, feature, value); \ qobject_unref(_resp); \ }) @@ -464,7 +483,10 @@ static void test_query_cpu_model_expansion_kvm(const void *data) return; } + /* Enabling and disabling kvm-no-adjvtime should always work. */ assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime"); + assert_set_feature(qts, "host", "kvm-no-adjvtime", true); + assert_set_feature(qts, "host", "kvm-no-adjvtime", false); if (g_str_equal(qtest_get_arch(), "aarch64")) { bool kvm_supports_sve; @@ -475,7 +497,11 @@ static void test_query_cpu_model_expansion_kvm(const void *data) char *error; assert_has_feature_enabled(qts, "host", "aarch64"); + + /* Enabling and disabling pmu should always work. */ assert_has_feature_enabled(qts, "host", "pmu"); + assert_set_feature(qts, "host", "pmu", true); + assert_set_feature(qts, "host", "pmu", false); assert_error(qts, "cortex-a15", "We cannot guarantee the CPU type 'cortex-a15' works "