Re: Core dump happened when starting a VM on arm64 server

[Andrew Jones]

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 "