[Qemu-devel] [PATCH v3] accel: forbid early use of kvm_enabled() and friends

[Greg Kurz]

It is unsafe to rely on *_enabled() helpers before the accelerator has
been initialized, ie, accel_init_machine() has succeeded, because they
always return false. But it is still possible to end up calling them
indirectly by inadvertance, and cause QEMU to misbehave.

This patch causes QEMU to abort if we try to check for any accelerator
before accel_init_machine() was called. This will help to catch bugs
earlier.

Signed-off-by: Greg Kurz <address@hidden>
---

Current master (SHA1 2a018f6e9878) has such a kvm_enabled() misuse that
triggers the assert:

https://travis-ci.org/gkurz/qemu/jobs/400059821#L8709

The fix for the misuse is available, waiting to be merged:

https://patchwork.ozlabs.org/patch/938077/

v3: - use an inline function
    - convert all accelerators
    - dropped David's R-b

v2: - dropped change in qom/cpu.c (useless header inclusion)
    - only #include "sysemu/kvm.h" if we actually need it
    - added David's R-b from v1 because changes in v2 are minor
---
 accel/accel.c             |    4 ++++
 include/hw/xen/xen.h      |    3 ++-
 include/qemu-common.h     |    3 ++-
 include/sysemu/accel.h    |    8 ++++++++
 include/sysemu/hvf.h      |    3 ++-
 include/sysemu/kvm.h      |    3 ++-
 include/sysemu/qtest.h    |    3 ++-
 stubs/Makefile.objs       |    1 +
 stubs/accel.c             |   12 ++++++++++++
 target/i386/hax-all.c     |    2 +-
 target/i386/whpx-all.c    |    2 +-
 tests/ptimer-test-stubs.c |    2 ++
 12 files changed, 39 insertions(+), 7 deletions(-)
 create mode 100644 stubs/accel.c

diff --git a/accel/accel.c b/accel/accel.c
index 966b2d8f536c..bbd890abd7ae 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -68,6 +68,8 @@ static int accel_init_machine(AccelClass *acc, MachineState 
*ms)
     return ret;
 }
 
+AccelState *current_accelerator;
+
 void configure_accelerator(MachineState *ms)
 {
     const char *accel;
@@ -116,6 +118,8 @@ void configure_accelerator(MachineState *ms)
     if (init_failed) {
         error_report("Back to %s accelerator", acc->name);
     }
+
+    current_accelerator = ms->accelerator;
 }
 
 void accel_register_compat_props(AccelState *accel)
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 7efcdaa8fe92..5a1664f4948f 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -25,9 +25,10 @@ extern bool xen_domid_restrict;
 
 extern bool xen_allowed;
 
+#include "sysemu/accel.h"
 static inline bool xen_enabled(void)
 {
-    return xen_allowed;
+    return assert_accelerator_initialized(xen_allowed);
 }
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 85f4749aefb7..01d5e4d97dbf 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -82,7 +82,8 @@ int qemu_openpty_raw(int *aslave, char *pty_name);
 extern bool tcg_allowed;
 void tcg_exec_init(unsigned long tb_size);
 #ifdef CONFIG_TCG
-#define tcg_enabled() (tcg_allowed)
+#include "sysemu/accel.h"
+#define tcg_enabled() (assert_accelerator_initialized(tcg_allowed))
 #else
 #define tcg_enabled() 0
 #endif
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 637358f43014..843334ae5229 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -72,4 +72,12 @@ void accel_register_compat_props(AccelState *accel);
 /* Called just before os_setup_post (ie just before drop OS privs) */
 void accel_setup_post(MachineState *ms);
 
+extern AccelState *current_accelerator;
+
+static inline bool assert_accelerator_initialized(bool allowed)
+{
+    assert(current_accelerator != NULL);
+    return allowed;
+}
+
 #endif
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 241118845c52..62bdc9ff9b3d 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -26,7 +26,8 @@ extern int hvf_disabled;
 #include "hw/hw.h"
 uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
                                  int reg);
-#define hvf_enabled() !hvf_disabled
+#include "sysemu/accel.h"
+#define hvf_enabled() assert_accelerator_initialized(!hvf_disabled)
 #else
 #define hvf_enabled() 0
 #define hvf_get_supported_cpuid(func, idx, reg) 0
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0b64b8e06786..5a2e59e99128 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -46,7 +46,8 @@ extern bool kvm_direct_msi_allowed;
 extern bool kvm_ioeventfd_any_length_allowed;
 extern bool kvm_msi_use_devid;
 
-#define kvm_enabled()           (kvm_allowed)
+#include "sysemu/accel.h"
+#define kvm_enabled()           (assert_accelerator_initialized(kvm_allowed))
 /**
  * kvm_irqchip_in_kernel:
  *
diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 70aa40aa72e9..12ef4f9d86e7 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -18,9 +18,10 @@
 
 extern bool qtest_allowed;
 
+#include "sysemu/accel.h"
 static inline bool qtest_enabled(void)
 {
-    return qtest_allowed;
+    return assert_accelerator_initialized(qtest_allowed);
 }
 
 bool qtest_driver(void);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 53d3f32cb258..2d5142287525 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -43,3 +43,4 @@ stub-obj-y += xen-common.o
 stub-obj-y += xen-hvm.o
 stub-obj-y += pci-host-piix.o
 stub-obj-y += ram-block.o
+stub-obj-y += accel.o
diff --git a/stubs/accel.c b/stubs/accel.c
new file mode 100644
index 000000000000..22821743ff85
--- /dev/null
+++ b/stubs/accel.c
@@ -0,0 +1,12 @@
+/*
+ * accel stubs
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/accel.h"
+
+/* assert_accelerator_initialized() requires a non-null value */
+AccelState *current_accelerator = (AccelState *) 0xdeadbeef;
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index d2e512856bb8..7c78bd7d094d 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -57,7 +57,7 @@ static int hax_arch_get_registers(CPUArchState *env);
 
 int hax_enabled(void)
 {
-    return hax_allowed;
+    return assert_accelerator_initialized(hax_allowed);
 }
 
 int valid_hax_tunnel_size(uint16_t size)
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 57e53e1f1f40..9e3dd922c2e3 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -1468,7 +1468,7 @@ static int whpx_accel_init(MachineState *ms)
 
 int whpx_enabled(void)
 {
-    return whpx_allowed;
+    return assert_accelerator_initialized(whpx_allowed);
 }
 
 static void whpx_accel_class_init(ObjectClass *oc, void *data)
diff --git a/tests/ptimer-test-stubs.c b/tests/ptimer-test-stubs.c
index ca5cc3b13bb3..95510d85c04f 100644
--- a/tests/ptimer-test-stubs.c
+++ b/tests/ptimer-test-stubs.c
@@ -33,6 +33,8 @@ int64_t ptimer_test_time_ns;
 /* Do not artificially limit period - see hw/core/ptimer.c.  */
 int use_icount = 1;
 bool qtest_allowed;
+/* assert_accelerator_initialized() requires a non-null value */
+AccelState *current_accelerator = (AccelState *) 0xdeadbeef;
 
 void timer_init_tl(QEMUTimer *ts,
                    QEMUTimerList *timer_list, int scale,