[PATCH v2 00/18] Complete the implementation of -accel

[PATCH v2 00/18] Complete the implementation of -accel

[Paolo Bonzini]

This series completes the implementation of -accel:

- Alternative accelerators can be specified with multiple -accel options,
  and each -accel option will configure the respective accelerator.
  This is implemented in patches 1 to 6, with a small addendum in patch 7.

- Accelerators can now support arbitrary options, which are implemented
  in terms of QOM properties.  This is implemented in patches 8 to 11,
  together with the conversion of the sole existing "-accel" option to QOM.

- Accelerator-related machine options, and legacy options such as -tb-size,
  forward to accelerator objects for backwards compatibility, but they
  do not exist anymore as QOM properties.  This is implemented in patch
  6 for "-machine accel" and patches 12 to 16 for everything else.

Paolo

v1->v2: improve bisectability by moving patch "vl: move icount configuration
        earlier" towards the beginning of the series

        new patch to compile accel/accel.c just once

Paolo Bonzini (18):
  memory: do not look at current_machine->accel
  vl: move icount configuration earlier
  tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c
  vl: extract accelerator option processing to a separate function
  vl: merge -accel processing into configure_accelerators
  accel: compile accel/accel.c just once
  vl: introduce object_parse_property_opt
  vl: configure accelerators from -accel options
  vl: warn for unavailable accelerators, clarify messages
  qom: introduce object_register_sugar_prop
  qom: add object_new_with_class
  accel: pass object to accel_init_machine
  tcg: convert "-accel threads" to a QOM property
  tcg: add "-accel tcg,tb-size" and deprecate "-tb-size"
  xen: convert "-machine igd-passthru" to an accelerator property
  kvm: convert "-machine kvm_shadow_mem" to an accelerator property
  kvm: introduce kvm_kernel_irqchip_* functions
  kvm: convert "-machine kernel_irqchip" to an accelerator property

 Makefile.objs             |   1 +
 accel/Makefile.objs       |   2 +-
 accel/accel.c             |  73 +--------------
 accel/kvm/kvm-all.c       | 117 ++++++++++++++++++++++--
 accel/tcg/tcg-all.c       | 149 +++++++++++++++++++++++++++++-
 cpus.c                    |  72 ---------------
 hw/core/machine.c         | 141 -----------------------------
 hw/ppc/e500.c             |   4 +-
 hw/ppc/spapr_irq.c        |  10 +--
 hw/xen/xen-common.c       |  16 ++++
 include/hw/boards.h       |   7 --
 include/qom/object.h      |  13 +++
 include/sysemu/accel.h    |   4 +-
 include/sysemu/cpus.h     |   2 -
 include/sysemu/kvm.h      |   7 +-
 memory.c                  |   5 +-
 qemu-deprecated.texi      |   6 ++
 qemu-options.hx           |  30 ++++---
 qom/object.c              |  28 +++++-
 target/arm/kvm.c          |   8 +-
 target/i386/cpu.c         |   8 +-
 target/i386/kvm.c         |   6 +-
 target/mips/kvm.c         |   2 +-
 target/ppc/kvm.c          |   2 +-
 target/s390x/cpu_models.c |   4 +-
 target/s390x/kvm.c        |   2 +-
 vl.c                      | 225 ++++++++++++++++++++++++++++++++--------------
 27 files changed, 532 insertions(+), 412 deletions(-)

-- 
1.8.3.1

[PATCH v2 01/18] memory: do not look at current_machine->accel

[Paolo Bonzini]

"info mtree -f" prints the wrong accelerator name if used with for example
"-machine accel=kvm:tcg".  The right thing to do is to fetch the name
from the AccelClass, which will also work nicely once
current_machine->accel stops existing.

Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/memory.c b/memory.c
index 06484c2..6d79cf1 100644
--- a/memory.c
+++ b/memory.c
@@ -2979,7 +2979,6 @@ struct FlatViewInfo {
     bool dispatch_tree;
     bool owner;
     AccelClass *ac;
-    const char *ac_name;
 };
 
 static void mtree_print_flatview(gpointer key, gpointer value,
@@ -3049,7 +3048,7 @@ static void mtree_print_flatview(gpointer key, gpointer value,
                 if (fvi->ac->has_memory(current_machine, as,
                                         int128_get64(range->addr.start),
                                         MR_SIZE(range->addr.size) + 1)) {
-                    qemu_printf(" %s", fvi->ac_name);
+                    qemu_printf(" %s", fvi->ac->name);
                 }
             }
         }
@@ -3097,8 +3096,6 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner)
 
         if (ac->has_memory) {
             fvi.ac = ac;
-            fvi.ac_name = current_machine->accel ? current_machine->accel :
-                object_class_get_name(OBJECT_CLASS(ac));
         }
 
         /* Gather all FVs in one table */
-- 
1.8.3.1

[PATCH v2 02/18] vl: move icount configuration earlier

[Paolo Bonzini]

Once qemu_tcg_configure is turned into a QOM property setter, it will not
be able to set a default value for mttcg_enabled.  Setting the default will
move to the TCG instance_init function, which currently runs before "-icount"
is processed.

However, it is harmless to do configure_icount for all accelerators; we will
just fail later if a non-TCG accelerator is selected.  So do that.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/vl.c b/vl.c
index 6a65a64..f69fea1 100644
--- a/vl.c
+++ b/vl.c
@@ -2825,6 +2825,12 @@ static void user_register_global_props(void)
                       global_init_func, NULL, NULL);
 }
 
+static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
+{
+    configure_icount(opts, errp);
+    return 0;
+}
+
 int main(int argc, char **argv, char **envp)
 {
     int i;
@@ -4165,6 +4171,8 @@ int main(int argc, char **argv, char **envp)
      * Note: uses machine properties such as kernel-irqchip, must run
      * after machine_set_property().
      */
+    qemu_opts_foreach(qemu_find_opts("icount"),
+                      do_configure_icount, NULL, &error_fatal);
     configure_accelerator(current_machine, argv[0]);
 
     /*
@@ -4250,13 +4258,9 @@ int main(int argc, char **argv, char **envp)
     qemu_spice_init();
 
     cpu_ticks_init();
-    if (icount_opts) {
-        if (!tcg_enabled()) {
-            error_report("-icount is not allowed with hardware virtualization");
-            exit(1);
-        }
-        configure_icount(icount_opts, &error_abort);
-        qemu_opts_del(icount_opts);
+    if (!tcg_enabled() && use_icount) {
+        error_report("-icount is not allowed with hardware virtualization");
+        exit(1);
     }
 
     if (tcg_enabled()) {
-- 
1.8.3.1

[PATCH v2 03/18] tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c

[Paolo Bonzini]

Move everything related to mttcg_enabled in accel/tcg/tcg-all.c,
which will make even more sense when "thread" becomes a QOM property.

For now, initializing mttcg_enabled in the instance_init function
prepares for the next patch, which will only invoke qemu_tcg_configure
when the command line includes a -accel option.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/tcg-all.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 cpus.c              | 72 ---------------------------------------------
 2 files changed, 85 insertions(+), 72 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index c59d5b0..78a0ab5 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -30,6 +30,11 @@
 #include "cpu.h"
 #include "sysemu/cpus.h"
 #include "qemu/main-loop.h"
+#include "tcg/tcg.h"
+#include "include/qapi/error.h"
+#include "include/qemu/error-report.h"
+#include "include/hw/boards.h"
+#include "qemu/option.h"
 
 unsigned long tcg_tb_size;
 
@@ -58,6 +63,55 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
     }
 }
 
+/*
+ * We default to false if we know other options have been enabled
+ * which are currently incompatible with MTTCG. Otherwise when each
+ * guest (target) has been updated to support:
+ *   - atomic instructions
+ *   - memory ordering primitives (barriers)
+ * they can set the appropriate CONFIG flags in ${target}-softmmu.mak
+ *
+ * Once a guest architecture has been converted to the new primitives
+ * there are two remaining limitations to check.
+ *
+ * - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
+ * - The host must have a stronger memory order than the guest
+ *
+ * It may be possible in future to support strong guests on weak hosts
+ * but that will require tagging all load/stores in a guest with their
+ * implicit memory order requirements which would likely slow things
+ * down a lot.
+ */
+
+static bool check_tcg_memory_orders_compatible(void)
+{
+#if defined(TCG_GUEST_DEFAULT_MO) && defined(TCG_TARGET_DEFAULT_MO)
+    return (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO) == 0;
+#else
+    return false;
+#endif
+}
+
+static bool default_mttcg_enabled(void)
+{
+    if (use_icount || TCG_OVERSIZED_GUEST) {
+        return false;
+    } else {
+#ifdef TARGET_SUPPORTS_MTTCG
+        return check_tcg_memory_orders_compatible();
+#else
+        return false;
+#endif
+    }
+}
+
+static void tcg_accel_instance_init(Object *obj)
+{
+    TCGState *s = TCG_STATE(obj);
+
+    mttcg_enabled = default_mttcg_enabled();
+}
+
 static int tcg_init(MachineState *ms)
 {
     tcg_exec_init(tcg_tb_size * 1024 * 1024);
@@ -65,6 +119,36 @@ static int tcg_init(MachineState *ms)
     return 0;
 }
 
+void qemu_tcg_configure(QemuOpts *opts, Error **errp)
+{
+    const char *t = qemu_opt_get(opts, "thread");
+    if (!t) {
+        return;
+    }
+    if (strcmp(t, "multi") == 0) {
+        if (TCG_OVERSIZED_GUEST) {
+            error_setg(errp, "No MTTCG when guest word size > hosts");
+        } else if (use_icount) {
+            error_setg(errp, "No MTTCG when icount is enabled");
+        } else {
+#ifndef TARGET_SUPPORTS_MTTCG
+            warn_report("Guest not yet converted to MTTCG - "
+                        "you may get unexpected results");
+#endif
+            if (!check_tcg_memory_orders_compatible()) {
+                warn_report("Guest expects a stronger memory ordering "
+                            "than the host provides");
+                error_printf("This may cause strange/hard to debug errors\n");
+            }
+            mttcg_enabled = true;
+        }
+    } else if (strcmp(t, "single") == 0) {
+        mttcg_enabled = false;
+    } else {
+        error_setg(errp, "Invalid 'thread' setting %s", t);
+    }
+}
+
 static void tcg_accel_class_init(ObjectClass *oc, void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
@@ -78,6 +162,7 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
 static const TypeInfo tcg_accel_type = {
     .name = TYPE_TCG_ACCEL,
     .parent = TYPE_ACCEL,
+    .instance_init = tcg_accel_instance_init,
     .class_init = tcg_accel_class_init,
 };
 
diff --git a/cpus.c b/cpus.c
index 63bda15..b472378 100644
--- a/cpus.c
+++ b/cpus.c
@@ -166,78 +166,6 @@ typedef struct TimersState {
 static TimersState timers_state;
 bool mttcg_enabled;
 
-/*
- * We default to false if we know other options have been enabled
- * which are currently incompatible with MTTCG. Otherwise when each
- * guest (target) has been updated to support:
- *   - atomic instructions
- *   - memory ordering primitives (barriers)
- * they can set the appropriate CONFIG flags in ${target}-softmmu.mak
- *
- * Once a guest architecture has been converted to the new primitives
- * there are two remaining limitations to check.
- *
- * - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
- * - The host must have a stronger memory order than the guest
- *
- * It may be possible in future to support strong guests on weak hosts
- * but that will require tagging all load/stores in a guest with their
- * implicit memory order requirements which would likely slow things
- * down a lot.
- */
-
-static bool check_tcg_memory_orders_compatible(void)
-{
-#if defined(TCG_GUEST_DEFAULT_MO) && defined(TCG_TARGET_DEFAULT_MO)
-    return (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO) == 0;
-#else
-    return false;
-#endif
-}
-
-static bool default_mttcg_enabled(void)
-{
-    if (use_icount || TCG_OVERSIZED_GUEST) {
-        return false;
-    } else {
-#ifdef TARGET_SUPPORTS_MTTCG
-        return check_tcg_memory_orders_compatible();
-#else
-        return false;
-#endif
-    }
-}
-
-void qemu_tcg_configure(QemuOpts *opts, Error **errp)
-{
-    const char *t = qemu_opt_get(opts, "thread");
-    if (t) {
-        if (strcmp(t, "multi") == 0) {
-            if (TCG_OVERSIZED_GUEST) {
-                error_setg(errp, "No MTTCG when guest word size > hosts");
-            } else if (use_icount) {
-                error_setg(errp, "No MTTCG when icount is enabled");
-            } else {
-#ifndef TARGET_SUPPORTS_MTTCG
-                warn_report("Guest not yet converted to MTTCG - "
-                            "you may get unexpected results");
-#endif
-                if (!check_tcg_memory_orders_compatible()) {
-                    warn_report("Guest expects a stronger memory ordering "
-                                "than the host provides");
-                    error_printf("This may cause strange/hard to debug errors\n");
-                }
-                mttcg_enabled = true;
-            }
-        } else if (strcmp(t, "single") == 0) {
-            mttcg_enabled = false;
-        } else {
-            error_setg(errp, "Invalid 'thread' setting %s", t);
-        }
-    } else {
-        mttcg_enabled = default_mttcg_enabled();
-    }
-}
 
 /* The current number of executed instructions is based on what we
  * originally budgeted minus the current state of the decrementing
-- 
1.8.3.1

[PATCH v2 04/18] vl: extract accelerator option processing to a separate function

[Paolo Bonzini]

As a first step towards supporting multiple "-accel" options, push the
late processing of -icount and -accel into a new function, and use
qemu_opts_foreach to retrieve -accel options instead of stashing
them into globals.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/vl.c b/vl.c
index f69fea1..1ad6dfb 100644
--- a/vl.c
+++ b/vl.c
@@ -2831,6 +2831,25 @@ static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
+{
+    if (tcg_enabled()) {
+        qemu_tcg_configure(opts, &error_fatal);
+    }
+    return 0;
+}
+
+static void configure_accelerators(void)
+{
+    qemu_opts_foreach(qemu_find_opts("accel"),
+                      do_configure_accelerator, NULL, &error_fatal);
+
+    if (!tcg_enabled() && use_icount) {
+        error_report("-icount is not allowed with hardware virtualization");
+        exit(1);
+    }
+}
+
 int main(int argc, char **argv, char **envp)
 {
     int i;
@@ -4258,14 +4277,7 @@ int main(int argc, char **argv, char **envp)
     qemu_spice_init();
 
     cpu_ticks_init();
-    if (!tcg_enabled() && use_icount) {
-        error_report("-icount is not allowed with hardware virtualization");
-        exit(1);
-    }
-
-    if (tcg_enabled()) {
-        qemu_tcg_configure(accel_opts, &error_fatal);
-    }
+    configure_accelerators();
 
     if (default_net) {
         QemuOptsList *net = qemu_find_opts("net");
-- 
1.8.3.1

[PATCH v2 05/18] vl: merge -accel processing into configure_accelerators

[Paolo Bonzini]

The next step is to move the parsing of "-machine accel=..." into vl.c,
unifying it with the configure_accelerators() function that has just
been introduced.  This way, we will be able to desugar it into multiple
"-accel" options, without polluting accel/accel.c.

The CONFIG_TCG and CONFIG_KVM symbols are not available in vl.c, but
we can use accel_find instead to find their value at runtime.  Once we
know that the binary has one of TCG or KVM, the default accelerator
can be expressed simply as "tcg:kvm", because TCG never fails to initialize.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/accel.c          | 69 ++------------------------------------------------
 include/sysemu/accel.h |  4 ++-
 vl.c                   | 64 ++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 64 insertions(+), 73 deletions(-)

diff --git a/accel/accel.c b/accel/accel.c
index 5fa3171..60c3827 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -28,13 +28,7 @@
 #include "hw/boards.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/kvm.h"
-#include "sysemu/qtest.h"
-#include "hw/xen/xen.h"
 #include "qom/object.h"
-#include "qemu/error-report.h"
-#include "qemu/option.h"
-#include "qapi/error.h"
 
 static const TypeInfo accel_type = {
     .name = TYPE_ACCEL,
@@ -44,7 +38,7 @@ static const TypeInfo accel_type = {
 };
 
 /* Lookup AccelClass from opt_name. Returns NULL if not found */
-static AccelClass *accel_find(const char *opt_name)
+AccelClass *accel_find(const char *opt_name)
 {
     char *class_name = g_strdup_printf(ACCEL_CLASS_NAME("%s"), opt_name);
     AccelClass *ac = ACCEL_CLASS(object_class_by_name(class_name));
@@ -52,7 +46,7 @@ static AccelClass *accel_find(const char *opt_name)
     return ac;
 }
 
-static int accel_init_machine(AccelClass *acc, MachineState *ms)
+int accel_init_machine(AccelClass *acc, MachineState *ms)
 {
     ObjectClass *oc = OBJECT_CLASS(acc);
     const char *cname = object_class_get_name(oc);
@@ -71,65 +65,6 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
     return ret;
 }
 
-void configure_accelerator(MachineState *ms, const char *progname)
-{
-    const char *accel;
-    char **accel_list, **tmp;
-    int ret;
-    bool accel_initialised = false;
-    bool init_failed = false;
-    AccelClass *acc = NULL;
-
-    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
-    if (accel == NULL) {
-        /* Select the default accelerator */
-        int pnlen = strlen(progname);
-        if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
-            /* If the program name ends with "kvm", we prefer KVM */
-            accel = "kvm:tcg";
-        } else {
-#if defined(CONFIG_TCG)
-            accel = "tcg";
-#elif defined(CONFIG_KVM)
-            accel = "kvm";
-#else
-            error_report("No accelerator selected and"
-                         " no default accelerator available");
-            exit(1);
-#endif
-        }
-    }
-
-    accel_list = g_strsplit(accel, ":", 0);
-
-    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
-        acc = accel_find(*tmp);
-        if (!acc) {
-            continue;
-        }
-        ret = accel_init_machine(acc, ms);
-        if (ret < 0) {
-            init_failed = true;
-            error_report("failed to initialize %s: %s",
-                         acc->name, strerror(-ret));
-        } else {
-            accel_initialised = true;
-        }
-    }
-    g_strfreev(accel_list);
-
-    if (!accel_initialised) {
-        if (!init_failed) {
-            error_report("-machine accel=%s: No accelerator found", accel);
-        }
-        exit(1);
-    }
-
-    if (init_failed) {
-        error_report("Back to %s accelerator", acc->name);
-    }
-}
-
 void accel_setup_post(MachineState *ms)
 {
     AccelState *accel = ms->accelerator;
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 8eb60b8..90b6213 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -66,7 +66,9 @@ typedef struct AccelClass {
 
 extern unsigned long tcg_tb_size;
 
-void configure_accelerator(MachineState *ms, const char *progname);
+AccelClass *accel_find(const char *opt_name);
+int accel_init_machine(AccelClass *acc, MachineState *ms);
+
 /* Called just before os_setup_post (ie just before drop OS privs) */
 void accel_setup_post(MachineState *ms);
 
diff --git a/vl.c b/vl.c
index 1ad6dfb..19c77b4 100644
--- a/vl.c
+++ b/vl.c
@@ -2839,8 +2839,65 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
-static void configure_accelerators(void)
+static void configure_accelerators(const char *progname)
 {
+    const char *accel;
+    char **accel_list, **tmp;
+    int ret;
+    bool accel_initialised = false;
+    bool init_failed = false;
+    AccelClass *acc = NULL;
+
+    qemu_opts_foreach(qemu_find_opts("icount"),
+                      do_configure_icount, NULL, &error_fatal);
+
+    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
+    if (accel == NULL) {
+        /* Select the default accelerator */
+        if (!accel_find("tcg") && !accel_find("kvm")) {
+            error_report("No accelerator selected and"
+                         " no default accelerator available");
+            exit(1);
+        } else {
+            int pnlen = strlen(progname);
+            if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
+                /* If the program name ends with "kvm", we prefer KVM */
+                accel = "kvm:tcg";
+            } else {
+                accel = "tcg:kvm";
+            }
+        }
+    }
+
+    accel_list = g_strsplit(accel, ":", 0);
+
+    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
+        acc = accel_find(*tmp);
+        if (!acc) {
+            continue;
+        }
+        ret = accel_init_machine(acc, current_machine);
+        if (ret < 0) {
+            init_failed = true;
+            error_report("failed to initialize %s: %s",
+                         acc->name, strerror(-ret));
+        } else {
+            accel_initialised = true;
+        }
+    }
+    g_strfreev(accel_list);
+
+    if (!accel_initialised) {
+        if (!init_failed) {
+            error_report("-machine accel=%s: No accelerator found", accel);
+        }
+        exit(1);
+    }
+
+    if (init_failed) {
+        error_report("Back to %s accelerator", acc->name);
+    }
+
     qemu_opts_foreach(qemu_find_opts("accel"),
                       do_configure_accelerator, NULL, &error_fatal);
 
@@ -4190,9 +4247,7 @@ int main(int argc, char **argv, char **envp)
      * Note: uses machine properties such as kernel-irqchip, must run
      * after machine_set_property().
      */
-    qemu_opts_foreach(qemu_find_opts("icount"),
-                      do_configure_icount, NULL, &error_fatal);
-    configure_accelerator(current_machine, argv[0]);
+    configure_accelerators(argv[0]);
 
     /*
      * Beware, QOM objects created before this point miss global and
@@ -4277,7 +4332,6 @@ int main(int argc, char **argv, char **envp)
     qemu_spice_init();
 
     cpu_ticks_init();
-    configure_accelerators();
 
     if (default_net) {
         QemuOptsList *net = qemu_find_opts("net");
-- 
1.8.3.1

[PATCH v2 06/18] accel: compile accel/accel.c just once

[Paolo Bonzini]

Now that accel/accel.c does not use CONFIG_TCG or CONFIG_KVM anymore,
it need not be compiled once for every softmmu target.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs       | 1 +
 accel/Makefile.objs | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.objs b/Makefile.objs
index 11ba1a3..b6fcbac 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -55,6 +55,7 @@ common-obj-$(CONFIG_POSIX) += os-posix.o
 
 common-obj-$(CONFIG_LINUX) += fsdev/
 
+common-obj-y += accel/
 common-obj-y += migration/
 
 common-obj-y += audio/
diff --git a/accel/Makefile.objs b/accel/Makefile.objs
index 8b498d3..17e5ac6 100644
--- a/accel/Makefile.objs
+++ b/accel/Makefile.objs
@@ -1,4 +1,4 @@
-obj-$(CONFIG_SOFTMMU) += accel.o
+common-obj-$(CONFIG_SOFTMMU) += accel.o
 obj-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_POSIX)) += qtest.o
 obj-$(CONFIG_KVM) += kvm/
 obj-$(CONFIG_TCG) += tcg/
-- 
1.8.3.1

[PATCH v2 07/18] vl: introduce object_parse_property_opt

[Paolo Bonzini]

We will reuse the parsing loop of machine_set_property soon for "-accel",
but we do not want the "_" -> "-" conversion since "-accel" can just
standardize on dashes.  We will also add a bunch of legacy option handling
to keep the QOM machine object clean.  Extract the loop into a separate
function, and keep the legacy handling in machine_set_property.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/vl.c b/vl.c
index 19c77b4..66dff1b 100644
--- a/vl.c
+++ b/vl.c
@@ -2615,27 +2615,17 @@ static MachineClass *select_machine(void)
     return machine_class;
 }
 
-static int machine_set_property(void *opaque,
-                                const char *name, const char *value,
-                                Error **errp)
+static int object_parse_property_opt(Object *obj,
+                                     const char *name, const char *value,
+                                     const char *skip, Error **errp)
 {
-    Object *obj = OBJECT(opaque);
     Error *local_err = NULL;
-    char *p, *qom_name;
 
-    if (strcmp(name, "type") == 0) {
+    if (g_str_equal(name, skip)) {
         return 0;
     }
 
-    qom_name = g_strdup(name);
-    for (p = qom_name; *p; p++) {
-        if (*p == '_') {
-            *p = '-';
-        }
-    }
-
-    object_property_parse(obj, value, qom_name, &local_err);
-    g_free(qom_name);
+    object_property_parse(obj, value, name, &local_err);
 
     if (local_err) {
         error_propagate(errp, local_err);
@@ -2645,6 +2635,21 @@ static int machine_set_property(void *opaque,
     return 0;
 }
 
+static int machine_set_property(void *opaque,
+                                const char *name, const char *value,
+                                Error **errp)
+{
+    g_autofree char *qom_name = g_strdup(name);
+    char *p;
+
+    for (p = qom_name; *p; p++) {
+        if (*p == '_') {
+            *p = '-';
+        }
+    }
+
+    return object_parse_property_opt(opaque, name, value, "type", errp);
+}
 
 /*
  * Initial object creation happens before all other
-- 
1.8.3.1

[PATCH v2 08/18] vl: configure accelerators from -accel options

[Paolo Bonzini]

Drop the "accel" property from MachineState, and instead desugar
"-machine accel=" to a list of "-accel" options.

This has a semantic change due to removing merge_lists from -accel.
For example:

- "-accel kvm -accel tcg" all but ignored "-accel kvm".  This is a bugfix.

- "-accel kvm -accel thread=single" ignored "thread=single", since it
  applied the option to KVM.  Now it fails due to not specifying the
  accelerator on "-accel thread=single".

- "-accel tcg -accel thread=single" chose single-threaded TCG, while now
  it will fail due to not specifying the accelerator on "-accel
  thread=single".

Also, "-machine accel" and "-accel" become incompatible.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c   | 21 ------------
 include/hw/boards.h |  1 -
 vl.c                | 93 +++++++++++++++++++++++++++++++----------------------
 3 files changed, 54 insertions(+), 61 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1689ad3..45ddfb6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -173,21 +173,6 @@ GlobalProperty hw_compat_2_1[] = {
 };
 const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
 
-static char *machine_get_accel(Object *obj, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    return g_strdup(ms->accel);
-}
-
-static void machine_set_accel(Object *obj, const char *value, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    g_free(ms->accel);
-    ms->accel = g_strdup(value);
-}
-
 static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
                                        const char *name, void *opaque,
                                        Error **errp)
@@ -808,11 +793,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
     mc->numa_mem_align_shift = 23;
     mc->numa_auto_assign_ram = numa_default_auto_assign_ram;
 
-    object_class_property_add_str(oc, "accel",
-        machine_get_accel, machine_set_accel, &error_abort);
-    object_class_property_set_description(oc, "accel",
-        "Accelerator list", &error_abort);
-
     object_class_property_add(oc, "kernel-irqchip", "on|off|split",
         NULL, machine_set_kernel_irqchip,
         NULL, NULL, &error_abort);
@@ -971,7 +951,6 @@ static void machine_finalize(Object *obj)
 {
     MachineState *ms = MACHINE(obj);
 
-    g_free(ms->accel);
     g_free(ms->kernel_filename);
     g_free(ms->initrd_filename);
     g_free(ms->kernel_cmdline);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index de45087..36fcbda 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -275,7 +275,6 @@ struct MachineState {
 
     /*< public >*/
 
-    char *accel;
     bool kernel_irqchip_allowed;
     bool kernel_irqchip_required;
     bool kernel_irqchip_split;
diff --git a/vl.c b/vl.c
index 66dff1b..f0cb438 100644
--- a/vl.c
+++ b/vl.c
@@ -294,7 +294,6 @@ static QemuOptsList qemu_accel_opts = {
     .name = "accel",
     .implied_opt_name = "accel",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_accel_opts.head),
-    .merge_lists = true,
     .desc = {
         {
             .name = "accel",
@@ -2648,6 +2647,11 @@ static int machine_set_property(void *opaque,
         }
     }
 
+    /* Legacy options do not correspond to MachineState properties.  */
+    if (g_str_equal(qom_name, "accel")) {
+        return 0;
+    }
+
     return object_parse_property_opt(opaque, name, value, "type", errp);
 }
 
@@ -2838,74 +2842,88 @@ static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
 
 static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
 {
+    bool *p_init_failed = opaque;
+    const char *acc = qemu_opt_get(opts, "accel");
+    AccelClass *ac = accel_find(acc);
+    int ret;
+
+    if (!ac) {
+        return 0;
+    }
+    ret = accel_init_machine(ac, current_machine);
+    if (ret < 0) {
+        *p_init_failed = true;
+        error_report("failed to initialize %s: %s",
+                     acc, strerror(-ret));
+        return 0;
+    }
+
     if (tcg_enabled()) {
         qemu_tcg_configure(opts, &error_fatal);
     }
-    return 0;
+    return 1;
 }
 
 static void configure_accelerators(const char *progname)
 {
     const char *accel;
     char **accel_list, **tmp;
-    int ret;
     bool accel_initialised = false;
     bool init_failed = false;
-    AccelClass *acc = NULL;
 
     qemu_opts_foreach(qemu_find_opts("icount"),
                       do_configure_icount, NULL, &error_fatal);
 
     accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
-    if (accel == NULL) {
-        /* Select the default accelerator */
-        if (!accel_find("tcg") && !accel_find("kvm")) {
-            error_report("No accelerator selected and"
-                         " no default accelerator available");
-            exit(1);
-        } else {
-            int pnlen = strlen(progname);
-            if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
-                /* If the program name ends with "kvm", we prefer KVM */
-                accel = "kvm:tcg";
+    if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
+        if (accel == NULL) {
+            /* Select the default accelerator */
+            if (!accel_find("tcg") && !accel_find("kvm")) {
+                error_report("No accelerator selected and"
+                             " no default accelerator available");
+                exit(1);
             } else {
-                accel = "tcg:kvm";
+                int pnlen = strlen(progname);
+                if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
+                    /* If the program name ends with "kvm", we prefer KVM */
+                    accel = "kvm:tcg";
+                } else {
+                    accel = "tcg:kvm";
+                }
             }
         }
-    }
 
-    accel_list = g_strsplit(accel, ":", 0);
+        accel_list = g_strsplit(accel, ":", 0);
 
-    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
-        acc = accel_find(*tmp);
-        if (!acc) {
-            continue;
+        for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
+            /*
+             * Filter invalid accelerators here, to prevent obscenities
+             * such as "-machine accel=tcg,,thread=single".
+             */
+            if (accel_find(*tmp)) {
+                qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
+            }
         }
-        ret = accel_init_machine(acc, current_machine);
-        if (ret < 0) {
-            init_failed = true;
-            error_report("failed to initialize %s: %s",
-                         acc->name, strerror(-ret));
-        } else {
-            accel_initialised = true;
+    } else {
+        if (accel != NULL) {
+            error_report("The -accel and \"-machine accel=\" options are incompatible");
+            exit(1);
         }
     }
-    g_strfreev(accel_list);
 
-    if (!accel_initialised) {
+    if (!qemu_opts_foreach(qemu_find_opts("accel"),
+                           do_configure_accelerator, &init_failed, &error_fatal)) {
         if (!init_failed) {
-            error_report("-machine accel=%s: No accelerator found", accel);
+            error_report("no accelerator found");
         }
         exit(1);
     }
 
     if (init_failed) {
-        error_report("Back to %s accelerator", acc->name);
+        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+        error_report("Back to %s accelerator", ac->name);
     }
 
-    qemu_opts_foreach(qemu_find_opts("accel"),
-                      do_configure_accelerator, NULL, &error_fatal);
-
     if (!tcg_enabled() && use_icount) {
         error_report("-icount is not allowed with hardware virtualization");
         exit(1);
@@ -3616,9 +3634,6 @@ int main(int argc, char **argv, char **envp)
                                  "use -M accel=... for now instead");
                     exit(1);
                 }
-                opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
-                                        false, &error_abort);
-                qemu_opt_set(opts, "accel", optarg, &error_abort);
                 break;
             case QEMU_OPTION_usb:
                 olist = qemu_find_opts("machine");
-- 
1.8.3.1

[PATCH v2 09/18] vl: warn for unavailable accelerators, clarify messages

[Paolo Bonzini]

So far, specifying an accelerator that was not compiled in did not result
in an error; fix that.

While at it, clarify the mysterious "Back to TCG" message.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index f0cb438..58aad4f 100644
--- a/vl.c
+++ b/vl.c
@@ -2848,6 +2848,8 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
     int ret;
 
     if (!ac) {
+        *p_init_failed = true;
+        error_report("invalid accelerator %s", acc);
         return 0;
     }
     ret = accel_init_machine(ac, current_machine);
@@ -2902,6 +2904,9 @@ static void configure_accelerators(const char *progname)
              */
             if (accel_find(*tmp)) {
                 qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
+            } else {
+                init_failed = true;
+                error_report("invalid accelerator %s", *tmp);
             }
         }
     } else {
@@ -2921,7 +2926,7 @@ static void configure_accelerators(const char *progname)
 
     if (init_failed) {
         AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
-        error_report("Back to %s accelerator", ac->name);
+        error_report("falling back to %s", ac->name);
     }
 
     if (!tcg_enabled() && use_icount) {
-- 
1.8.3.1

[PATCH v2 10/18] qom: introduce object_register_sugar_prop

[Paolo Bonzini]

Similar to the existing "-rtc driftfix" option, we will convert some
legacy "-machine" command line options to global properties on accelerators.
Because accelerators are not devices, we cannot use qdev_prop_register_global.
Instead, provide a slot in the generic object_compat_props arrays for
command line syntactic sugar.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qom/object.h |  1 +
 qom/object.c         | 23 +++++++++++++++++++++--
 vl.c                 | 10 +++-------
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 128d00c..230b18f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -679,6 +679,7 @@ void object_apply_global_props(Object *obj, const GPtrArray *props,
                                Error **errp);
 void object_set_machine_compat_props(GPtrArray *compat_props);
 void object_set_accelerator_compat_props(GPtrArray *compat_props);
+void object_register_sugar_prop(const char *driver, const char *prop, const char *value);
 void object_apply_compat_props(Object *obj);
 
 /**
diff --git a/qom/object.c b/qom/object.c
index d51b57f..bfb4413 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -414,10 +414,29 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp
  * Global property defaults
  * Slot 0: accelerator's global property defaults
  * Slot 1: machine's global property defaults
+ * Slot 2: global properties from legacy command line option
  * Each is a GPtrArray of of GlobalProperty.
  * Applied in order, later entries override earlier ones.
  */
-static GPtrArray *object_compat_props[2];
+static GPtrArray *object_compat_props[3];
+
+/*
+ * Retrieve @GPtrArray for global property defined with options
+ * other than "-global".  These are generally used for syntactic
+ * sugar and legacy command line options.
+ */
+void object_register_sugar_prop(const char *driver, const char *prop, const char *value)
+{
+    GlobalProperty *g;
+    if (!object_compat_props[2]) {
+        object_compat_props[2] = g_ptr_array_new();
+    }
+    g = g_new(GlobalProperty, 1);
+    g->driver = g_strdup(driver);
+    g->property = g_strdup(prop);
+    g->value = g_strdup(value);
+    g_ptr_array_add(object_compat_props[2], g);
+}
 
 /*
  * Set machine's global property defaults to @compat_props.
@@ -445,7 +464,7 @@ void object_apply_compat_props(Object *obj)
 
     for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
         object_apply_global_props(obj, object_compat_props[i],
-                                  &error_abort);
+                                  i == 2 ? &error_fatal : &error_abort);
     }
 }
 
diff --git a/vl.c b/vl.c
index 58aad4f..d6c77bc 100644
--- a/vl.c
+++ b/vl.c
@@ -897,13 +897,9 @@ static void configure_rtc(QemuOpts *opts)
     value = qemu_opt_get(opts, "driftfix");
     if (value) {
         if (!strcmp(value, "slew")) {
-            static GlobalProperty slew_lost_ticks = {
-                .driver   = "mc146818rtc",
-                .property = "lost_tick_policy",
-                .value    = "slew",
-            };
-
-            qdev_prop_register_global(&slew_lost_ticks);
+            object_register_sugar_prop("mc146818rtc",
+                                       "lost_tick_policy",
+                                       "slew");
         } else if (!strcmp(value, "none")) {
             /* discard is default */
         } else {
-- 
1.8.3.1

[PATCH v2 11/18] qom: add object_new_with_class

[Paolo Bonzini]

Similar to CPU and machine classes, "-accel" class names are mangled,
so we have to first get a class via accel_find and then instantiate it.
Provide a new function to instantiate a class without going through
object_class_get_name, and use it for CPUs and machines already.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/accel.c             |  4 +---
 include/qom/object.h      | 12 ++++++++++++
 qom/object.c              |  5 +++++
 target/i386/cpu.c         |  8 ++++----
 target/s390x/cpu_models.c |  4 ++--
 vl.c                      |  3 +--
 6 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/accel/accel.c b/accel/accel.c
index 60c3827..dd38a46 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -48,9 +48,7 @@ AccelClass *accel_find(const char *opt_name)
 
 int accel_init_machine(AccelClass *acc, MachineState *ms)
 {
-    ObjectClass *oc = OBJECT_CLASS(acc);
-    const char *cname = object_class_get_name(oc);
-    AccelState *accel = ACCEL(object_new(cname));
+    AccelState *accel = ACCEL(object_new_with_class(OBJECT_CLASS(acc)));
     int ret;
     ms->accelerator = accel;
     *(acc->allowed) = true;
diff --git a/include/qom/object.h b/include/qom/object.h
index 230b18f..f9ad692 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -593,6 +593,18 @@ struct InterfaceClass
                                              __FILE__, __LINE__, __func__))
 
 /**
+ * object_new_with_class:
+ * @klass: The class to instantiate.
+ *
+ * This function will initialize a new object using heap allocated memory.
+ * The returned object has a reference count of 1, and will be freed when
+ * the last reference is dropped.
+ *
+ * Returns: The newly allocated and instantiated object.
+ */
+Object *object_new_with_class(ObjectClass *klass);
+
+/**
  * object_new:
  * @typename: The name of the type of the object to instantiate.
  *
diff --git a/qom/object.c b/qom/object.c
index bfb4413..bc444d3 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -658,6 +658,11 @@ static Object *object_new_with_type(Type type)
     return obj;
 }
 
+Object *object_new_with_class(ObjectClass *klass)
+{
+    return object_new_with_type(klass->type);
+}
+
 Object *object_new(const char *typename)
 {
     TypeImpl *ti = type_get_by_name(typename);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 69f518a..a044078 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4640,7 +4640,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
         return;
     }
 
-    xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
+    xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc)));
 
     x86_cpu_expand_features(xc, &err);
     if (err) {
@@ -4711,7 +4711,7 @@ static GSList *get_sorted_cpu_model_list(void)
 
 static char *x86_cpu_class_get_model_id(X86CPUClass *xc)
 {
-    Object *obj = object_new(object_class_get_name(OBJECT_CLASS(xc)));
+    Object *obj = object_new_with_class(OBJECT_CLASS(xc));
     char *r = object_property_get_str(obj, "model-id", &error_abort);
     object_unref(obj);
     return r;
@@ -5092,7 +5092,7 @@ static X86CPU *x86_cpu_from_model(const char *model, QDict *props, Error **errp)
         goto out;
     }
 
-    xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
+    xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc)));
     if (props) {
         object_apply_props(OBJECT(xc), props, &err);
         if (err) {
@@ -5936,7 +5936,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
     APICCommonState *apic;
     ObjectClass *apic_class = OBJECT_CLASS(apic_get_class());
 
-    cpu->apic_state = DEVICE(object_new(object_class_get_name(apic_class)));
+    cpu->apic_state = DEVICE(object_new_with_class(apic_class));
 
     object_property_add_child(OBJECT(cpu), "lapic",
                               OBJECT(cpu->apic_state), &error_abort);
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7e92fb2..72cf48b 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -440,7 +440,7 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
     if (cpu_list_data->model) {
         Object *obj;
         S390CPU *sc;
-        obj = object_new(object_class_get_name(klass));
+        obj = object_new_with_class(klass);
         sc = S390_CPU(obj);
         if (sc->model) {
             info->has_unavailable_features = true;
@@ -501,7 +501,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
         error_setg(errp, "The CPU definition '%s' requires KVM", info->name);
         return;
     }
-    obj = object_new(object_class_get_name(oc));
+    obj = object_new_with_class(oc);
     cpu = S390_CPU(obj);
 
     if (!cpu->model) {
diff --git a/vl.c b/vl.c
index d6c77bc..f18b26b 100644
--- a/vl.c
+++ b/vl.c
@@ -3989,8 +3989,7 @@ int main(int argc, char **argv, char **envp)
                       cleanup_add_fd, NULL, &error_fatal);
 #endif
 
-    current_machine = MACHINE(object_new(object_class_get_name(
-                          OBJECT_CLASS(machine_class))));
+    current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
     if (machine_help_func(qemu_get_machine_opts(), current_machine)) {
         exit(0);
     }
-- 
1.8.3.1

[PATCH v2 12/18] accel: pass object to accel_init_machine

[Paolo Bonzini]

We will have to set QOM properties before accel_init_machine, based on the
options provided to -accel.  Construct the object outside it so that it
will be possible to iterate on properties between object_new_with_class
and accel_init_machine.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/accel.c          | 4 ++--
 include/sysemu/accel.h | 2 +-
 vl.c                   | 4 +++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/accel/accel.c b/accel/accel.c
index dd38a46..1c5c3a6 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -46,9 +46,9 @@ AccelClass *accel_find(const char *opt_name)
     return ac;
 }
 
-int accel_init_machine(AccelClass *acc, MachineState *ms)
+int accel_init_machine(AccelState *accel, MachineState *ms)
 {
-    AccelState *accel = ACCEL(object_new_with_class(OBJECT_CLASS(acc)));
+    AccelClass *acc = ACCEL_GET_CLASS(accel);
     int ret;
     ms->accelerator = accel;
     *(acc->allowed) = true;
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 90b6213..22cac0f 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -67,7 +67,7 @@ typedef struct AccelClass {
 extern unsigned long tcg_tb_size;
 
 AccelClass *accel_find(const char *opt_name);
-int accel_init_machine(AccelClass *acc, MachineState *ms);
+int accel_init_machine(AccelState *accel, MachineState *ms);
 
 /* Called just before os_setup_post (ie just before drop OS privs) */
 void accel_setup_post(MachineState *ms);
diff --git a/vl.c b/vl.c
index f18b26b..078ab7a 100644
--- a/vl.c
+++ b/vl.c
@@ -2841,6 +2841,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
     bool *p_init_failed = opaque;
     const char *acc = qemu_opt_get(opts, "accel");
     AccelClass *ac = accel_find(acc);
+    AccelState *accel;
     int ret;
 
     if (!ac) {
@@ -2848,7 +2849,8 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
         error_report("invalid accelerator %s", acc);
         return 0;
     }
-    ret = accel_init_machine(ac, current_machine);
+    accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
+    ret = accel_init_machine(accel, current_machine);
     if (ret < 0) {
         *p_init_failed = true;
         error_report("failed to initialize %s: %s",
-- 
1.8.3.1

[PATCH v2 13/18] tcg: convert "-accel threads" to a QOM property

[Paolo Bonzini]

Replace the ad-hoc qemu_tcg_configure with generic code invoking QOM
property getters and setters.  More properties (and thus more valid
-accel suboptions) will be added in the next patches, which will move
accelerator-related "-machine" options to accelerators.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/tcg-all.c   | 50 ++++++++++++++++++++++++++++++++++++--------------
 include/sysemu/cpus.h |  2 --
 vl.c                  | 32 +++++++++++++++++---------------
 3 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 78a0ab5..7829f02 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -34,7 +34,17 @@
 #include "include/qapi/error.h"
 #include "include/qemu/error-report.h"
 #include "include/hw/boards.h"
-#include "qemu/option.h"
+
+typedef struct TCGState {
+    AccelState parent_obj;
+
+    bool mttcg_enabled;
+} TCGState;
+
+#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg")
+
+#define TCG_STATE(obj) \
+        OBJECT_CHECK(TCGState, (obj), TYPE_TCG_ACCEL)
 
 unsigned long tcg_tb_size;
 
@@ -109,23 +119,31 @@ static void tcg_accel_instance_init(Object *obj)
 {
     TCGState *s = TCG_STATE(obj);
 
-    mttcg_enabled = default_mttcg_enabled();
+    s->mttcg_enabled = default_mttcg_enabled();
 }
 
 static int tcg_init(MachineState *ms)
 {
+    TCGState *s = TCG_STATE(current_machine->accelerator);
+
     tcg_exec_init(tcg_tb_size * 1024 * 1024);
     cpu_interrupt_handler = tcg_handle_interrupt;
+    mttcg_enabled = s->mttcg_enabled;
     return 0;
 }
 
-void qemu_tcg_configure(QemuOpts *opts, Error **errp)
+static char *tcg_get_thread(Object *obj, Error **errp)
 {
-    const char *t = qemu_opt_get(opts, "thread");
-    if (!t) {
-        return;
-    }
-    if (strcmp(t, "multi") == 0) {
+    TCGState *s = TCG_STATE(obj);
+
+    return g_strdup(s->mttcg_enabled ? "multi" : "single");
+}
+
+static void tcg_set_thread(Object *obj, const char *value, Error **errp)
+{
+    TCGState *s = TCG_STATE(obj);
+
+    if (strcmp(value, "multi") == 0) {
         if (TCG_OVERSIZED_GUEST) {
             error_setg(errp, "No MTTCG when guest word size > hosts");
         } else if (use_icount) {
@@ -140,12 +158,12 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
                             "than the host provides");
                 error_printf("This may cause strange/hard to debug errors\n");
             }
-            mttcg_enabled = true;
+            s->mttcg_enabled = true;
         }
-    } else if (strcmp(t, "single") == 0) {
-        mttcg_enabled = false;
+    } else if (strcmp(value, "single") == 0) {
+        s->mttcg_enabled = false;
     } else {
-        error_setg(errp, "Invalid 'thread' setting %s", t);
+        error_setg(errp, "Invalid 'thread' setting %s", value);
     }
 }
 
@@ -155,15 +173,19 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
     ac->name = "tcg";
     ac->init_machine = tcg_init;
     ac->allowed = &tcg_allowed;
-}
 
-#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg")
+    object_class_property_add_str(oc, "thread",
+                                  tcg_get_thread,
+                                  tcg_set_thread,
+                                  NULL);
+}
 
 static const TypeInfo tcg_accel_type = {
     .name = TYPE_TCG_ACCEL,
     .parent = TYPE_ACCEL,
     .instance_init = tcg_accel_instance_init,
     .class_init = tcg_accel_class_init,
+    .instance_size = sizeof(TCGState),
 };
 
 static void register_accel_types(void)
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 32c05f2..3c1da6a 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -40,6 +40,4 @@ extern int smp_threads;
 
 void list_cpus(const char *optarg);
 
-void qemu_tcg_configure(QemuOpts *opts, Error **errp);
-
 #endif
diff --git a/vl.c b/vl.c
index 078ab7a..b6c23d1 100644
--- a/vl.c
+++ b/vl.c
@@ -295,17 +295,12 @@ static QemuOptsList qemu_accel_opts = {
     .implied_opt_name = "accel",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_accel_opts.head),
     .desc = {
-        {
-            .name = "accel",
-            .type = QEMU_OPT_STRING,
-            .help = "Select the type of accelerator",
-        },
-        {
-            .name = "thread",
-            .type = QEMU_OPT_STRING,
-            .help = "Enable/disable multi-threaded TCG",
-        },
-        { /* end of list */ }
+        /*
+         * no elements => accept any
+         * sanity checking will happen later
+         * when setting accelerator properties
+         */
+        { }
     },
 };
 
@@ -2836,6 +2831,13 @@ static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static int accelerator_set_property(void *opaque,
+                                const char *name, const char *value,
+                                Error **errp)
+{
+    return object_parse_property_opt(opaque, name, value, "accel", errp);
+}
+
 static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
 {
     bool *p_init_failed = opaque;
@@ -2850,6 +2852,10 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
         return 0;
     }
     accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
+    qemu_opt_foreach(opts, accelerator_set_property,
+                     accel,
+                     &error_fatal);
+
     ret = accel_init_machine(accel, current_machine);
     if (ret < 0) {
         *p_init_failed = true;
@@ -2857,10 +2863,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
                      acc, strerror(-ret));
         return 0;
     }
-
-    if (tcg_enabled()) {
-        qemu_tcg_configure(opts, &error_fatal);
-    }
     return 1;
 }
 
-- 
1.8.3.1

[PATCH v2 14/18] tcg: add "-accel tcg, tb-size" and deprecate "-tb-size"

[Paolo Bonzini]

-tb-size fits nicely in the new framework for accelerator-specific options.  It
is a very niche option, so insta-deprecate it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/tcg-all.c    | 40 +++++++++++++++++++++++++++++++++++++---
 include/sysemu/accel.h |  2 --
 qemu-deprecated.texi   |  6 ++++++
 qemu-options.hx        |  8 ++++++--
 vl.c                   |  8 ++++----
 5 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 7829f02..1dc384c 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -34,11 +34,13 @@
 #include "include/qapi/error.h"
 #include "include/qemu/error-report.h"
 #include "include/hw/boards.h"
+#include "qapi/qapi-builtin-visit.h"
 
 typedef struct TCGState {
     AccelState parent_obj;
 
     bool mttcg_enabled;
+    unsigned long tb_size;
 } TCGState;
 
 #define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg")
@@ -46,8 +48,6 @@ typedef struct TCGState {
 #define TCG_STATE(obj) \
         OBJECT_CHECK(TCGState, (obj), TYPE_TCG_ACCEL)
 
-unsigned long tcg_tb_size;
-
 /* mask must never be zero, except for A20 change call */
 static void tcg_handle_interrupt(CPUState *cpu, int mask)
 {
@@ -126,7 +126,7 @@ static int tcg_init(MachineState *ms)
 {
     TCGState *s = TCG_STATE(current_machine->accelerator);
 
-    tcg_exec_init(tcg_tb_size * 1024 * 1024);
+    tcg_exec_init(s->tb_size * 1024 * 1024);
     cpu_interrupt_handler = tcg_handle_interrupt;
     mttcg_enabled = s->mttcg_enabled;
     return 0;
@@ -167,6 +167,33 @@ static void tcg_set_thread(Object *obj, const char *value, Error **errp)
     }
 }
 
+static void tcg_get_tb_size(Object *obj, Visitor *v,
+                            const char *name, void *opaque,
+                            Error **errp)
+{
+    TCGState *s = TCG_STATE(obj);
+    uint32_t value = s->tb_size;
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void tcg_set_tb_size(Object *obj, Visitor *v,
+                            const char *name, void *opaque,
+                            Error **errp)
+{
+    TCGState *s = TCG_STATE(obj);
+    Error *error = NULL;
+    uint32_t value;
+
+    visit_type_uint32(v, name, &value, &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+
+    s->tb_size = value;
+}
+
 static void tcg_accel_class_init(ObjectClass *oc, void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
@@ -178,6 +205,13 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
                                   tcg_get_thread,
                                   tcg_set_thread,
                                   NULL);
+
+    object_class_property_add(oc, "tb-size", "int",
+        tcg_get_tb_size, tcg_set_tb_size,
+        NULL, NULL, &error_abort);
+    object_class_property_set_description(oc, "tb-size",
+        "TCG translation block cache size", &error_abort);
+
 }
 
 static const TypeInfo tcg_accel_type = {
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 22cac0f..d4c1429 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -64,8 +64,6 @@ typedef struct AccelClass {
 #define ACCEL_GET_CLASS(obj) \
     OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
 
-extern unsigned long tcg_tb_size;
-
 AccelClass *accel_find(const char *opt_name);
 int accel_init_machine(AccelState *accel, MachineState *ms);
 
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 4b4b742..487d2b4 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -147,6 +147,12 @@ QEMU 4.1 has three options, please migrate to one of these three:
       to do is specify the kernel they want to boot with the -kernel option
  3. ``-bios <file>`` - Tells QEMU to load the specified file as the firmwrae.
 
+@subsection -tb-size option (since 5.0)
+
+QEMU 5.0 introduced an alternative syntax to specify the size of the translation
+block cache, @option{-accel tcg,tb-size=}.  The new syntax deprecates the
+previously available @option{-tb-size} option.
+
 @section QEMU Machine Protocol (QMP) commands
 
 @subsection change (since 2.5.0)
diff --git a/qemu-options.hx b/qemu-options.hx
index 65c9473..9775258 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,8 +118,9 @@ Select CPU model (@code{-cpu help} for list and additional feature selection)
 ETEXI
 
 DEF("accel", HAS_ARG, QEMU_OPTION_accel,
-    "-accel [accel=]accelerator[,thread=single|multi]\n"
+    "-accel [accel=]accelerator[,prop[=value][,...]]\n"
     "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
+    "                tb-size=n (TCG translation block cache size)\n"
     "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
 STEXI
 @item -accel @var{name}[,prop=@var{value}[,...]]
@@ -129,6 +130,8 @@ kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If th
 more than one accelerator specified, the next one is used if the previous one
 fails to initialize.
 @table @option
+@item tb-size=@var{n}
+Controls the size (in MiB) of the TCG translation block cache.
 @item thread=single|multi
 Controls number of TCG threads. When the TCG is multi-threaded there will be one
 thread per vCPU therefor taking advantage of additional host cores. The default
@@ -4012,7 +4015,8 @@ DEF("tb-size", HAS_ARG, QEMU_OPTION_tb_size, \
 STEXI
 @item -tb-size @var{n}
 @findex -tb-size
-Set TB size.
+Set TCG translation block cache size.  Deprecated, use @samp{-accel tcg,tb-size=@var{n}}
+instead.
 ETEXI
 
 DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
diff --git a/vl.c b/vl.c
index b6c23d1..e6ff56b 100644
--- a/vl.c
+++ b/vl.c
@@ -2852,6 +2852,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
         return 0;
     }
     accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
+    object_apply_compat_props(OBJECT(accel));
     qemu_opt_foreach(opts, accelerator_set_property,
                      accel,
                      &error_fatal);
@@ -2863,6 +2864,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
                      acc, strerror(-ret));
         return 0;
     }
+
     return 1;
 }
 
@@ -3745,10 +3747,8 @@ int main(int argc, char **argv, char **envp)
                 error_report("TCG is disabled");
                 exit(1);
 #endif
-                if (qemu_strtoul(optarg, NULL, 0, &tcg_tb_size) < 0) {
-                    error_report("Invalid argument to -tb-size");
-                    exit(1);
-                }
+                warn_report("The -tb-size option is deprecated, use -accel tcg,tb-size instead");
+                object_register_sugar_prop(ACCEL_CLASS_NAME("tcg"), "tb-size", optarg);
                 break;
             case QEMU_OPTION_icount:
                 icount_opts = qemu_opts_parse_noisily(qemu_find_opts("icount"),
-- 
1.8.3.1

[PATCH v2 15/18] xen: convert "-machine igd-passthru" to an accelerator property

[Paolo Bonzini]

The first machine property to fall is Xen's Intel integrated graphics
passthrough.  The "-machine igd-passthru" option does not set anymore
a property on the machine object, but desugars to a GlobalProperty on
accelerator objects.

The setter is very simple, since the value ends up in a
global variable, so this patch also provides an example before the more
complicated cases that follow it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c   | 20 --------------------
 hw/xen/xen-common.c | 16 ++++++++++++++++
 include/hw/boards.h |  1 -
 qemu-options.hx     |  7 ++++---
 vl.c                | 14 ++++----------
 5 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 45ddfb6..d7a0356 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -412,20 +412,6 @@ static void machine_set_graphics(Object *obj, bool value, Error **errp)
     ms->enable_graphics = value;
 }
 
-static bool machine_get_igd_gfx_passthru(Object *obj, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    return ms->igd_gfx_passthru;
-}
-
-static void machine_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    ms->igd_gfx_passthru = value;
-}
-
 static char *machine_get_firmware(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -862,12 +848,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
     object_class_property_set_description(oc, "graphics",
         "Set on/off to enable/disable graphics emulation", &error_abort);
 
-    object_class_property_add_bool(oc, "igd-passthru",
-        machine_get_igd_gfx_passthru, machine_set_igd_gfx_passthru,
-        &error_abort);
-    object_class_property_set_description(oc, "igd-passthru",
-        "Set on/off to enable/disable igd passthrou", &error_abort);
-
     object_class_property_add_str(oc, "firmware",
         machine_get_firmware, machine_set_firmware,
         &error_abort);
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 5284b0d..6cba30c 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -124,6 +124,16 @@ static void xen_change_state_handler(void *opaque, int running,
     }
 }
 
+static bool xen_get_igd_gfx_passthru(Object *obj, Error **errp)
+{
+    return has_igd_gfx_passthru;
+}
+
+static void xen_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
+{
+    has_igd_gfx_passthru = value;
+}
+
 static void xen_setup_post(MachineState *ms, AccelState *accel)
 {
     int rc;
@@ -177,6 +187,12 @@ static void xen_accel_class_init(ObjectClass *oc, void *data)
     ac->compat_props = g_ptr_array_new();
 
     compat_props_add(ac->compat_props, compat, G_N_ELEMENTS(compat));
+
+    object_class_property_add_bool(oc, "igd-passthru",
+        xen_get_igd_gfx_passthru, xen_set_igd_gfx_passthru,
+        &error_abort);
+    object_class_property_set_description(oc, "igd-passthru",
+        "Set on/off to enable/disable igd passthrou", &error_abort);
 }
 
 #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 36fcbda..cdcf481 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -287,7 +287,6 @@ struct MachineState {
     bool mem_merge;
     bool usb;
     bool usb_disabled;
-    bool igd_gfx_passthru;
     char *firmware;
     bool iommu;
     bool suppress_vmdesc;
diff --git a/qemu-options.hx b/qemu-options.hx
index 9775258..6f12b31 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -37,7 +37,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                kvm_shadow_mem=size of KVM shadow MMU in bytes\n"
     "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
     "                mem-merge=on|off controls memory merge support (default: on)\n"
-    "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
     "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
     "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
     "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
@@ -71,8 +70,6 @@ more than one accelerator specified, the next one is used if the previous one
 fails to initialize.
 @item kernel_irqchip=on|off
 Controls in-kernel irqchip support for the chosen accelerator when available.
-@item gfx_passthru=on|off
-Enables IGD GFX passthrough support for the chosen machine when available.
 @item vmport=on|off|auto
 Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
 value based on accel. For accel=xen the default is off otherwise the default
@@ -120,6 +117,7 @@ ETEXI
 DEF("accel", HAS_ARG, QEMU_OPTION_accel,
     "-accel [accel=]accelerator[,prop[=value][,...]]\n"
     "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
+    "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"
     "                tb-size=n (TCG translation block cache size)\n"
     "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
 STEXI
@@ -130,6 +128,9 @@ kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If th
 more than one accelerator specified, the next one is used if the previous one
 fails to initialize.
 @table @option
+@item igd-passthru=on|off
+When Xen is in use, this option controls whether Intel integrated graphics
+devices can be passed through to the guest (default=off)
 @item tb-size=@var{n}
 Controls the size (in MiB) of the TCG translation block cache.
 @item thread=single|multi
diff --git a/vl.c b/vl.c
index e6ff56b..ee872f2 100644
--- a/vl.c
+++ b/vl.c
@@ -1257,13 +1257,6 @@ static void configure_msg(QemuOpts *opts)
 }
 
 
-/* Now we still need this for compatibility with XEN. */
-bool has_igd_gfx_passthru;
-static void igd_gfx_passthru(void)
-{
-    has_igd_gfx_passthru = current_machine->igd_gfx_passthru;
-}
-
 /***********************************************************/
 /* USB devices */
 
@@ -2642,6 +2635,10 @@ static int machine_set_property(void *opaque,
     if (g_str_equal(qom_name, "accel")) {
         return 0;
     }
+    if (g_str_equal(qom_name, "igd-passthru")) {
+        object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
+        return 0;
+    }
 
     return object_parse_property_opt(opaque, name, value, "type", errp);
 }
@@ -4456,9 +4453,6 @@ int main(int argc, char **argv, char **envp)
             exit(1);
     }
 
-    /* Check if IGD GFX passthrough. */
-    igd_gfx_passthru();
-
     /* init generic devices */
     rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
     qemu_opts_foreach(qemu_find_opts("device"),
-- 
1.8.3.1

[PATCH v2 16/18] kvm: convert "-machine kvm_shadow_mem" to an accelerator property

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 hw/core/machine.c   | 39 ---------------------------------------
 include/hw/boards.h |  2 --
 qemu-options.hx     |  6 +++---
 target/i386/kvm.c   |  2 +-
 vl.c                |  4 ++++
 6 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index ca00daa..f0b9294 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -41,6 +41,7 @@
 #include "hw/irq.h"
 #include "sysemu/sev.h"
 #include "sysemu/balloon.h"
+#include "qapi/visitor.h"
 
 #include "hw/boards.h"
 
@@ -92,6 +93,7 @@ struct KVMState
     int max_nested_state_len;
     int many_ioeventfds;
     int intx_set_mask;
+    int kvm_shadow_mem;
     bool sync_mmu;
     bool manual_dirty_log_protect;
     /* The man page (and posix) say ioctl numbers are signed int, but
@@ -2940,6 +2942,40 @@ static bool kvm_accel_has_memory(MachineState *ms, AddressSpace *as,
     return false;
 }
 
+static void kvm_get_kvm_shadow_mem(Object *obj, Visitor *v,
+                                   const char *name, void *opaque,
+                                   Error **errp)
+{
+    KVMState *s = KVM_STATE(obj);
+    int64_t value = s->kvm_shadow_mem;
+
+    visit_type_int(v, name, &value, errp);
+}
+
+static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
+                                   const char *name, void *opaque,
+                                   Error **errp)
+{
+    KVMState *s = KVM_STATE(obj);
+    Error *error = NULL;
+    int64_t value;
+
+    visit_type_int(v, name, &value, &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+
+    s->kvm_shadow_mem = value;
+}
+
+static void kvm_accel_instance_init(Object *obj)
+{
+    KVMState *s = KVM_STATE(obj);
+
+    s->kvm_shadow_mem = -1;
+}
+
 static void kvm_accel_class_init(ObjectClass *oc, void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
@@ -2947,11 +2983,18 @@ static void kvm_accel_class_init(ObjectClass *oc, void *data)
     ac->init_machine = kvm_init;
     ac->has_memory = kvm_accel_has_memory;
     ac->allowed = &kvm_allowed;
+
+    object_class_property_add(oc, "kvm-shadow-mem", "int",
+        kvm_get_kvm_shadow_mem, kvm_set_kvm_shadow_mem,
+        NULL, NULL, &error_abort);
+    object_class_property_set_description(oc, "kvm-shadow-mem",
+        "KVM shadow MMU size", &error_abort);
 }
 
 static const TypeInfo kvm_accel_type = {
     .name = TYPE_KVM_ACCEL,
     .parent = TYPE_ACCEL,
+    .instance_init = kvm_accel_instance_init,
     .class_init = kvm_accel_class_init,
     .instance_size = sizeof(KVMState),
 };
diff --git a/hw/core/machine.c b/hw/core/machine.c
index d7a0356..bfb5f6f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -211,33 +211,6 @@ static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
     }
 }
 
-static void machine_get_kvm_shadow_mem(Object *obj, Visitor *v,
-                                       const char *name, void *opaque,
-                                       Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-    int64_t value = ms->kvm_shadow_mem;
-
-    visit_type_int(v, name, &value, errp);
-}
-
-static void machine_set_kvm_shadow_mem(Object *obj, Visitor *v,
-                                       const char *name, void *opaque,
-                                       Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-    Error *error = NULL;
-    int64_t value;
-
-    visit_type_int(v, name, &value, &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-
-    ms->kvm_shadow_mem = value;
-}
-
 static char *machine_get_kernel(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -785,12 +758,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
     object_class_property_set_description(oc, "kernel-irqchip",
         "Configure KVM in-kernel irqchip", &error_abort);
 
-    object_class_property_add(oc, "kvm-shadow-mem", "int",
-        machine_get_kvm_shadow_mem, machine_set_kvm_shadow_mem,
-        NULL, NULL, &error_abort);
-    object_class_property_set_description(oc, "kvm-shadow-mem",
-        "KVM shadow MMU size", &error_abort);
-
     object_class_property_add_str(oc, "kernel",
         machine_get_kernel, machine_set_kernel, &error_abort);
     object_class_property_set_description(oc, "kernel",
@@ -892,7 +859,6 @@ static void machine_initfn(Object *obj)
 
     ms->kernel_irqchip_allowed = true;
     ms->kernel_irqchip_split = mc->default_kernel_irqchip_split;
-    ms->kvm_shadow_mem = -1;
     ms->dump_guest_core = true;
     ms->mem_merge = true;
     ms->enable_graphics = true;
@@ -963,11 +929,6 @@ bool machine_kernel_irqchip_split(MachineState *machine)
     return machine->kernel_irqchip_split;
 }
 
-int machine_kvm_shadow_mem(MachineState *machine)
-{
-    return machine->kvm_shadow_mem;
-}
-
 int machine_phandle_start(MachineState *machine)
 {
     return machine->phandle_start;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index cdcf481..46119eb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -66,7 +66,6 @@ bool machine_usb(MachineState *machine);
 bool machine_kernel_irqchip_allowed(MachineState *machine);
 bool machine_kernel_irqchip_required(MachineState *machine);
 bool machine_kernel_irqchip_split(MachineState *machine);
-int machine_kvm_shadow_mem(MachineState *machine);
 int machine_phandle_start(MachineState *machine);
 bool machine_dump_guest_core(MachineState *machine);
 bool machine_mem_merge(MachineState *machine);
@@ -278,7 +277,6 @@ struct MachineState {
     bool kernel_irqchip_allowed;
     bool kernel_irqchip_required;
     bool kernel_irqchip_split;
-    int kvm_shadow_mem;
     char *dtb;
     char *dumpdtb;
     int phandle_start;
diff --git a/qemu-options.hx b/qemu-options.hx
index 6f12b31..779c8af 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -34,7 +34,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                supported accelerators are kvm, xen, hax, hvf, whpx or tcg (default: tcg)\n"
     "                kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n"
     "                vmport=on|off|auto controls emulation of vmport (default: auto)\n"
-    "                kvm_shadow_mem=size of KVM shadow MMU in bytes\n"
     "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
     "                mem-merge=on|off controls memory merge support (default: on)\n"
     "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
@@ -74,8 +73,6 @@ Controls in-kernel irqchip support for the chosen accelerator when available.
 Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
 value based on accel. For accel=xen the default is off otherwise the default
 is on.
-@item kvm_shadow_mem=size
-Defines the size of the KVM shadow MMU.
 @item dump-guest-core=on|off
 Include guest memory in a core dump. The default is on.
 @item mem-merge=on|off
@@ -118,6 +115,7 @@ DEF("accel", HAS_ARG, QEMU_OPTION_accel,
     "-accel [accel=]accelerator[,prop[=value][,...]]\n"
     "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
     "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"
+    "                kvm-shadow-mem=size of KVM shadow MMU in bytes\n"
     "                tb-size=n (TCG translation block cache size)\n"
     "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
 STEXI
@@ -131,6 +129,8 @@ fails to initialize.
 @item igd-passthru=on|off
 When Xen is in use, this option controls whether Intel integrated graphics
 devices can be passed through to the guest (default=off)
+@item kvm-shadow-mem=size
+Defines the size of the KVM shadow MMU.
 @item tb-size=@var{n}
 Controls the size (in MiB) of the TCG translation block cache.
 @item thread=single|multi
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 1d10046..62ce681 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2163,7 +2163,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     }
     qemu_register_reset(kvm_unpoison_all, NULL);
 
-    shadow_mem = machine_kvm_shadow_mem(ms);
+    shadow_mem = object_property_get_int(OBJECT(s), "kvm-shadow-mem", &error_abort);
     if (shadow_mem != -1) {
         shadow_mem /= 4096;
         ret = kvm_vm_ioctl(s, KVM_SET_NR_MMU_PAGES, shadow_mem);
diff --git a/vl.c b/vl.c
index ee872f2..3678522 100644
--- a/vl.c
+++ b/vl.c
@@ -2639,6 +2639,10 @@ static int machine_set_property(void *opaque,
         object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
         return 0;
     }
+    if (g_str_equal(qom_name, "kvm-shadow-mem")) {
+        object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value);
+        return 0;
+    }
 
     return object_parse_property_opt(opaque, name, value, "type", errp);
 }
-- 
1.8.3.1

[PATCH v2 17/18] kvm: introduce kvm_kernel_irqchip_* functions

[Paolo Bonzini]

The KVMState struct is opaque, so provide accessors for the fields
that will be moved from current_machine to the accelerator.  For now
they just forward to the machine object, but this will change.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c  | 23 +++++++++++++++++++----
 hw/ppc/e500.c        |  4 ++--
 hw/ppc/spapr_irq.c   | 10 +++++-----
 include/sysemu/kvm.h |  7 +++++--
 target/arm/kvm.c     |  8 ++++----
 target/i386/kvm.c    |  4 ++--
 target/mips/kvm.c    |  2 +-
 target/ppc/kvm.c     |  2 +-
 target/s390x/kvm.c   |  2 +-
 9 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f0b9294..c0a6351 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1760,7 +1760,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi)
     g_hash_table_insert(s->gsimap, irq, GINT_TO_POINTER(gsi));
 }
 
-static void kvm_irqchip_create(MachineState *machine, KVMState *s)
+static void kvm_irqchip_create(KVMState *s)
 {
     int ret;
 
@@ -1778,9 +1778,9 @@ static void kvm_irqchip_create(MachineState *machine, KVMState *s)
 
     /* First probe and see if there's a arch-specific hook to create the
      * in-kernel irqchip for us */
-    ret = kvm_arch_irqchip_create(machine, s);
+    ret = kvm_arch_irqchip_create(s);
     if (ret == 0) {
-        if (machine_kernel_irqchip_split(machine)) {
+        if (kvm_kernel_irqchip_split()) {
             perror("Split IRQ chip mode not supported.");
             exit(1);
         } else {
@@ -2052,7 +2052,7 @@ static int kvm_init(MachineState *ms)
     }
 
     if (machine_kernel_irqchip_allowed(ms)) {
-        kvm_irqchip_create(ms, s);
+        kvm_irqchip_create(s);
     }
 
     if (kvm_eventfds_allowed) {
@@ -2969,6 +2969,21 @@ static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
     s->kvm_shadow_mem = value;
 }
 
+bool kvm_kernel_irqchip_allowed(void)
+{
+    return machine_kernel_irqchip_allowed(current_machine);
+}
+
+bool kvm_kernel_irqchip_required(void)
+{
+    return machine_kernel_irqchip_required(current_machine);
+}
+
+bool kvm_kernel_irqchip_split(void)
+{
+    return machine_kernel_irqchip_split(current_machine);
+}
+
 static void kvm_accel_instance_init(Object *obj)
 {
     KVMState *s = KVM_STATE(obj);
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 91cd4c2..928efaa 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -801,10 +801,10 @@ static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms,
     if (kvm_enabled()) {
         Error *err = NULL;
 
-        if (machine_kernel_irqchip_allowed(machine)) {
+        if (kvm_kernel_irqchip_allowed()) {
             dev = ppce500_init_mpic_kvm(pmc, irqs, &err);
         }
-        if (machine_kernel_irqchip_required(machine) && !dev) {
+        if (kvm_kernel_irqchip_required() && !dev) {
             error_reportf_err(err,
                               "kernel_irqchip requested but unavailable: ");
             exit(1);
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index d6bb7fd..c3f8870 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -77,9 +77,9 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **),
     MachineState *machine = MACHINE(qdev_get_machine());
     Error *local_err = NULL;
 
-    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
+    if (kvm_enabled() && kvm_kernel_irqchip_allowed()) {
         if (fn(intc, &local_err) < 0) {
-            if (machine_kernel_irqchip_required(machine)) {
+            if (kvm_kernel_irqchip_required()) {
                 error_prepend(&local_err,
                               "kernel_irqchip requested but unavailable: ");
                 error_propagate(errp, local_err);
@@ -184,7 +184,7 @@ static int spapr_irq_check(SpaprMachineState *spapr, Error **errp)
      */
     if (kvm_enabled() &&
         spapr->irq == &spapr_irq_dual &&
-        machine_kernel_irqchip_required(machine) &&
+        kvm_kernel_irqchip_required() &&
         xics_kvm_has_broken_disconnect(spapr)) {
         error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on");
         return -1;
@@ -290,12 +290,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
     MachineState *machine = MACHINE(spapr);
     SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
 
-    if (machine_kernel_irqchip_split(machine)) {
+    if (kvm_enabled() && kvm_kernel_irqchip_split()) {
         error_setg(errp, "kernel_irqchip split mode not supported on pseries");
         return;
     }
 
-    if (!kvm_enabled() && machine_kernel_irqchip_required(machine)) {
+    if (!kvm_enabled() && kvm_kernel_irqchip_required()) {
         error_setg(errp,
                    "kernel_irqchip requested but only available with KVM");
         return;
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 9fe233b..aaf2a50 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -519,10 +519,13 @@ void kvm_pc_gsi_handler(void *opaque, int n, int level);
 void kvm_pc_setup_irq_routing(bool pci_enabled);
 void kvm_init_irq_routing(KVMState *s);
 
+bool kvm_kernel_irqchip_allowed(void);
+bool kvm_kernel_irqchip_required(void);
+bool kvm_kernel_irqchip_split(void);
+
 /**
  * kvm_arch_irqchip_create:
  * @KVMState: The KVMState pointer
- * @MachineState: The MachineState pointer
  *
  * Allow architectures to create an in-kernel irq chip themselves.
  *
@@ -530,7 +533,7 @@ void kvm_init_irq_routing(KVMState *s);
  *            0: irq chip was not created
  *          > 0: irq chip was created
  */
-int kvm_arch_irqchip_create(MachineState *ms, KVMState *s);
+int kvm_arch_irqchip_create(KVMState *s);
 
 /**
  * kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 5b82cef..b87b59a 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -741,11 +741,11 @@ void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s)
 {
-     if (machine_kernel_irqchip_split(ms)) {
-         perror("-machine kernel_irqchip=split is not supported on ARM.");
-         exit(1);
+    if (kvm_kernel_irqchip_split()) {
+        perror("-machine kernel_irqchip=split is not supported on ARM.");
+        exit(1);
     }
 
     /* If we can create the VGIC using the newer device control API, we
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 62ce681..ef63f3a 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -4494,10 +4494,10 @@ void kvm_arch_init_irq_routing(KVMState *s)
     }
 }
 
-int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s)
 {
     int ret;
-    if (machine_kernel_irqchip_split(ms)) {
+    if (kvm_kernel_irqchip_split()) {
         ret = kvm_vm_enable_cap(s, KVM_CAP_SPLIT_IRQCHIP, 0, 24);
         if (ret) {
             error_report("Could not enable split irqchip mode: %s",
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 578bc14..de3e26e 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -57,7 +57,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     return 0;
 }
 
-int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s)
 {
     return 0;
 }
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index c77f984..461dc6d 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -152,7 +152,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     return 0;
 }
 
-int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s)
 {
     return 0;
 }
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 0c9d14b..f3a742b 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -374,7 +374,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     return 0;
 }
 
-int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
+int kvm_arch_irqchip_create(KVMState *s)
 {
     return 0;
 }
-- 
1.8.3.1

[PATCH v2 18/18] kvm: convert "-machine kernel_irqchip" to an accelerator property

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/core/machine.c   | 61 -----------------------------------------------------
 include/hw/boards.h |  3 ---
 qemu-options.hx     |  9 +++++---
 vl.c                |  3 ++-
 5 files changed, 62 insertions(+), 73 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c0a6351..b0adbb2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -42,6 +42,8 @@
 #include "sysemu/sev.h"
 #include "sysemu/balloon.h"
 #include "qapi/visitor.h"
+#include "qapi/qapi-types-common.h"
+#include "qapi/qapi-visit-common.h"
 
 #include "hw/boards.h"
 
@@ -94,6 +96,9 @@ struct KVMState
     int many_ioeventfds;
     int intx_set_mask;
     int kvm_shadow_mem;
+    bool kernel_irqchip_allowed;
+    bool kernel_irqchip_required;
+    bool kernel_irqchip_split;
     bool sync_mmu;
     bool manual_dirty_log_protect;
     /* The man page (and posix) say ioctl numbers are signed int, but
@@ -1780,7 +1785,7 @@ static void kvm_irqchip_create(KVMState *s)
      * in-kernel irqchip for us */
     ret = kvm_arch_irqchip_create(s);
     if (ret == 0) {
-        if (kvm_kernel_irqchip_split()) {
+        if (s->kernel_irqchip_split) {
             perror("Split IRQ chip mode not supported.");
             exit(1);
         } else {
@@ -2051,7 +2056,7 @@ static int kvm_init(MachineState *ms)
         goto err;
     }
 
-    if (machine_kernel_irqchip_allowed(ms)) {
+    if (s->kernel_irqchip_allowed) {
         kvm_irqchip_create(s);
     }
 
@@ -2969,19 +2974,57 @@ static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
     s->kvm_shadow_mem = value;
 }
 
+static void kvm_set_kernel_irqchip(Object *obj, Visitor *v,
+                                   const char *name, void *opaque,
+                                   Error **errp)
+{
+    Error *err = NULL;
+    KVMState *s = KVM_STATE(obj);
+    OnOffSplit mode;
+
+    visit_type_OnOffSplit(v, name, &mode, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    } else {
+        switch (mode) {
+        case ON_OFF_SPLIT_ON:
+            s->kernel_irqchip_allowed = true;
+            s->kernel_irqchip_required = true;
+            s->kernel_irqchip_split = false;
+            break;
+        case ON_OFF_SPLIT_OFF:
+            s->kernel_irqchip_allowed = false;
+            s->kernel_irqchip_required = false;
+            s->kernel_irqchip_split = false;
+            break;
+        case ON_OFF_SPLIT_SPLIT:
+            s->kernel_irqchip_allowed = true;
+            s->kernel_irqchip_required = true;
+            s->kernel_irqchip_split = true;
+            break;
+        default:
+            /* The value was checked in visit_type_OnOffSplit() above. If
+             * we get here, then something is wrong in QEMU.
+             */
+            abort();
+        }
+    }
+}
+
 bool kvm_kernel_irqchip_allowed(void)
 {
-    return machine_kernel_irqchip_allowed(current_machine);
+    return kvm_state->kernel_irqchip_allowed;
 }
 
 bool kvm_kernel_irqchip_required(void)
 {
-    return machine_kernel_irqchip_required(current_machine);
+    return kvm_state->kernel_irqchip_required;
 }
 
 bool kvm_kernel_irqchip_split(void)
 {
-    return machine_kernel_irqchip_split(current_machine);
+    return kvm_state->kernel_irqchip_split;
 }
 
 static void kvm_accel_instance_init(Object *obj)
@@ -2999,6 +3042,12 @@ static void kvm_accel_class_init(ObjectClass *oc, void *data)
     ac->has_memory = kvm_accel_has_memory;
     ac->allowed = &kvm_allowed;
 
+    object_class_property_add(oc, "kernel-irqchip", "on|off|split",
+        NULL, kvm_set_kernel_irqchip,
+        NULL, NULL, &error_abort);
+    object_class_property_set_description(oc, "kernel-irqchip",
+        "Configure KVM in-kernel irqchip", &error_abort);
+
     object_class_property_add(oc, "kvm-shadow-mem", "int",
         kvm_get_kvm_shadow_mem, kvm_set_kvm_shadow_mem,
         NULL, NULL, &error_abort);
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bfb5f6f..c1cad2c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -173,44 +173,6 @@ GlobalProperty hw_compat_2_1[] = {
 };
 const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
 
-static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
-                                       const char *name, void *opaque,
-                                       Error **errp)
-{
-    Error *err = NULL;
-    MachineState *ms = MACHINE(obj);
-    OnOffSplit mode;
-
-    visit_type_OnOffSplit(v, name, &mode, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    } else {
-        switch (mode) {
-        case ON_OFF_SPLIT_ON:
-            ms->kernel_irqchip_allowed = true;
-            ms->kernel_irqchip_required = true;
-            ms->kernel_irqchip_split = false;
-            break;
-        case ON_OFF_SPLIT_OFF:
-            ms->kernel_irqchip_allowed = false;
-            ms->kernel_irqchip_required = false;
-            ms->kernel_irqchip_split = false;
-            break;
-        case ON_OFF_SPLIT_SPLIT:
-            ms->kernel_irqchip_allowed = true;
-            ms->kernel_irqchip_required = true;
-            ms->kernel_irqchip_split = true;
-            break;
-        default:
-            /* The value was checked in visit_type_OnOffSplit() above. If
-             * we get here, then something is wrong in QEMU.
-             */
-            abort();
-        }
-    }
-}
-
 static char *machine_get_kernel(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -752,12 +714,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
     mc->numa_mem_align_shift = 23;
     mc->numa_auto_assign_ram = numa_default_auto_assign_ram;
 
-    object_class_property_add(oc, "kernel-irqchip", "on|off|split",
-        NULL, machine_set_kernel_irqchip,
-        NULL, NULL, &error_abort);
-    object_class_property_set_description(oc, "kernel-irqchip",
-        "Configure KVM in-kernel irqchip", &error_abort);
-
     object_class_property_add_str(oc, "kernel",
         machine_get_kernel, machine_set_kernel, &error_abort);
     object_class_property_set_description(oc, "kernel",
@@ -857,8 +813,6 @@ static void machine_initfn(Object *obj)
     MachineState *ms = MACHINE(obj);
     MachineClass *mc = MACHINE_GET_CLASS(obj);
 
-    ms->kernel_irqchip_allowed = true;
-    ms->kernel_irqchip_split = mc->default_kernel_irqchip_split;
     ms->dump_guest_core = true;
     ms->mem_merge = true;
     ms->enable_graphics = true;
@@ -914,21 +868,6 @@ bool machine_usb(MachineState *machine)
     return machine->usb;
 }
 
-bool machine_kernel_irqchip_allowed(MachineState *machine)
-{
-    return machine->kernel_irqchip_allowed;
-}
-
-bool machine_kernel_irqchip_required(MachineState *machine)
-{
-    return machine->kernel_irqchip_required;
-}
-
-bool machine_kernel_irqchip_split(MachineState *machine)
-{
-    return machine->kernel_irqchip_split;
-}
-
 int machine_phandle_start(MachineState *machine)
 {
     return machine->phandle_start;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 46119eb..b42fedd 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -63,9 +63,6 @@ extern MachineState *current_machine;
 
 void machine_run_board_init(MachineState *machine);
 bool machine_usb(MachineState *machine);
-bool machine_kernel_irqchip_allowed(MachineState *machine);
-bool machine_kernel_irqchip_required(MachineState *machine);
-bool machine_kernel_irqchip_split(MachineState *machine);
 int machine_phandle_start(MachineState *machine);
 bool machine_dump_guest_core(MachineState *machine);
 bool machine_mem_merge(MachineState *machine);
diff --git a/qemu-options.hx b/qemu-options.hx
index 779c8af..945fc4a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -32,7 +32,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                selects emulated machine ('-machine help' for list)\n"
     "                property accel=accel1[:accel2[:...]] selects accelerator\n"
     "                supported accelerators are kvm, xen, hax, hvf, whpx or tcg (default: tcg)\n"
-    "                kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n"
     "                vmport=on|off|auto controls emulation of vmport (default: auto)\n"
     "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
     "                mem-merge=on|off controls memory merge support (default: on)\n"
@@ -67,8 +66,6 @@ This is used to enable an accelerator. Depending on the target architecture,
 kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If there is
 more than one accelerator specified, the next one is used if the previous one
 fails to initialize.
-@item kernel_irqchip=on|off
-Controls in-kernel irqchip support for the chosen accelerator when available.
 @item vmport=on|off|auto
 Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
 value based on accel. For accel=xen the default is off otherwise the default
@@ -115,6 +112,7 @@ DEF("accel", HAS_ARG, QEMU_OPTION_accel,
     "-accel [accel=]accelerator[,prop[=value][,...]]\n"
     "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
     "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"
+    "                kernel-irqchip=on|off|split controls accelerated irqchip support (default=on)\n"
     "                kvm-shadow-mem=size of KVM shadow MMU in bytes\n"
     "                tb-size=n (TCG translation block cache size)\n"
     "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
@@ -129,6 +127,11 @@ fails to initialize.
 @item igd-passthru=on|off
 When Xen is in use, this option controls whether Intel integrated graphics
 devices can be passed through to the guest (default=off)
+@item kernel-irqchip=on|off|split
+Controls KVM in-kernel irqchip support.  The default is full acceleration of the
+interrupt controllers.  On x86, split irqchip reduces the kernel attack
+surface, at a performance cost for non-MSI interrupts.  Disabling the in-kernel
+irqchip completely is not recommended except for debugging purposes.
 @item kvm-shadow-mem=size
 Defines the size of the KVM shadow MMU.
 @item tb-size=@var{n}
diff --git a/vl.c b/vl.c
index 3678522..53e4ff6 100644
--- a/vl.c
+++ b/vl.c
@@ -2639,7 +2639,8 @@ static int machine_set_property(void *opaque,
         object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
         return 0;
     }
-    if (g_str_equal(qom_name, "kvm-shadow-mem")) {
+    if (g_str_equal(qom_name, "kvm-shadow-mem") ||
+        g_str_equal(qom_name, "kernel-irqchip")) {
         object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value);
         return 0;
     }
-- 
1.8.3.1

Re: [PATCH v2 13/18] tcg: convert "-accel threads" to a QOM property

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> Replace the ad-hoc qemu_tcg_configure with generic code invoking QOM
> property getters and setters.  More properties (and thus more valid
> -accel suboptions) will be added in the next patches, which will move
> accelerator-related "-machine" options to accelerators.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/tcg-all.c   | 50 ++++++++++++++++++++++++++++++++++++--------------
>   include/sysemu/cpus.h |  2 --
>   vl.c                  | 32 +++++++++++++++++---------------
>   3 files changed, 53 insertions(+), 31 deletions(-)
> 
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 78a0ab5..7829f02 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -34,7 +34,17 @@
>   #include "include/qapi/error.h"
>   #include "include/qemu/error-report.h"
>   #include "include/hw/boards.h"
> -#include "qemu/option.h"
> +
> +typedef struct TCGState {
> +    AccelState parent_obj;
> +
> +    bool mttcg_enabled;
> +} TCGState;
> +
> +#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg")
> +
> +#define TCG_STATE(obj) \
> +        OBJECT_CHECK(TCGState, (obj), TYPE_TCG_ACCEL)
>   
>   unsigned long tcg_tb_size;
>   
> @@ -109,23 +119,31 @@ static void tcg_accel_instance_init(Object *obj)
>   {
>       TCGState *s = TCG_STATE(obj);
>   
> -    mttcg_enabled = default_mttcg_enabled();
> +    s->mttcg_enabled = default_mttcg_enabled();
>   }
>   
>   static int tcg_init(MachineState *ms)
>   {
> +    TCGState *s = TCG_STATE(current_machine->accelerator);
> +
>       tcg_exec_init(tcg_tb_size * 1024 * 1024);
>       cpu_interrupt_handler = tcg_handle_interrupt;
> +    mttcg_enabled = s->mttcg_enabled;
>       return 0;
>   }
>   
> -void qemu_tcg_configure(QemuOpts *opts, Error **errp)
> +static char *tcg_get_thread(Object *obj, Error **errp)
>   {
> -    const char *t = qemu_opt_get(opts, "thread");
> -    if (!t) {
> -        return;
> -    }
> -    if (strcmp(t, "multi") == 0) {
> +    TCGState *s = TCG_STATE(obj);
> +
> +    return g_strdup(s->mttcg_enabled ? "multi" : "single");
> +}
> +
> +static void tcg_set_thread(Object *obj, const char *value, Error **errp)
> +{
> +    TCGState *s = TCG_STATE(obj);
> +
> +    if (strcmp(value, "multi") == 0) {
>           if (TCG_OVERSIZED_GUEST) {
>               error_setg(errp, "No MTTCG when guest word size > hosts");
>           } else if (use_icount) {
> @@ -140,12 +158,12 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
>                               "than the host provides");
>                   error_printf("This may cause strange/hard to debug errors\n");
>               }
> -            mttcg_enabled = true;
> +            s->mttcg_enabled = true;
>           }
> -    } else if (strcmp(t, "single") == 0) {
> -        mttcg_enabled = false;
> +    } else if (strcmp(value, "single") == 0) {
> +        s->mttcg_enabled = false;
>       } else {
> -        error_setg(errp, "Invalid 'thread' setting %s", t);
> +        error_setg(errp, "Invalid 'thread' setting %s", value);
>       }
>   }
>   
> @@ -155,15 +173,19 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
>       ac->name = "tcg";
>       ac->init_machine = tcg_init;
>       ac->allowed = &tcg_allowed;
> -}
>   
> -#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg")
> +    object_class_property_add_str(oc, "thread",
> +                                  tcg_get_thread,
> +                                  tcg_set_thread,
> +                                  NULL);
> +}
>   
>   static const TypeInfo tcg_accel_type = {
>       .name = TYPE_TCG_ACCEL,
>       .parent = TYPE_ACCEL,
>       .instance_init = tcg_accel_instance_init,
>       .class_init = tcg_accel_class_init,
> +    .instance_size = sizeof(TCGState),
>   };
>   
>   static void register_accel_types(void)
> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
> index 32c05f2..3c1da6a 100644
> --- a/include/sysemu/cpus.h
> +++ b/include/sysemu/cpus.h
> @@ -40,6 +40,4 @@ extern int smp_threads;
>   
>   void list_cpus(const char *optarg);
>   
> -void qemu_tcg_configure(QemuOpts *opts, Error **errp);
> -
>   #endif
> diff --git a/vl.c b/vl.c
> index 078ab7a..b6c23d1 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -295,17 +295,12 @@ static QemuOptsList qemu_accel_opts = {
>       .implied_opt_name = "accel",
>       .head = QTAILQ_HEAD_INITIALIZER(qemu_accel_opts.head),
>       .desc = {
> -        {
> -            .name = "accel",
> -            .type = QEMU_OPT_STRING,
> -            .help = "Select the type of accelerator",
> -        },
> -        {
> -            .name = "thread",
> -            .type = QEMU_OPT_STRING,
> -            .help = "Enable/disable multi-threaded TCG",
> -        },
> -        { /* end of list */ }
> +        /*
> +         * no elements => accept any
> +         * sanity checking will happen later
> +         * when setting accelerator properties
> +         */
> +        { }
>       },
>   };
>   
> @@ -2836,6 +2831,13 @@ static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
>       return 0;
>   }
>   
> +static int accelerator_set_property(void *opaque,
> +                                const char *name, const char *value,
> +                                Error **errp)
> +{
> +    return object_parse_property_opt(opaque, name, value, "accel", errp);
> +}
> +
>   static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>   {
>       bool *p_init_failed = opaque;
> @@ -2850,6 +2852,10 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>           return 0;
>       }
>       accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
> +    qemu_opt_foreach(opts, accelerator_set_property,
> +                     accel,
> +                     &error_fatal);
> +
>       ret = accel_init_machine(accel, current_machine);
>       if (ret < 0) {
>           *p_init_failed = true;
> @@ -2857,10 +2863,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>                        acc, strerror(-ret));
>           return 0;
>       }
> -
> -    if (tcg_enabled()) {
> -        qemu_tcg_configure(opts, &error_fatal);
> -    }
>       return 1;
>   }
>   
> 

Re: [PATCH v2 11/18] qom: add object_new_with_class

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> Similar to CPU and machine classes, "-accel" class names are mangled,
> so we have to first get a class via accel_find and then instantiate it.
> Provide a new function to instantiate a class without going through
> object_class_get_name, and use it for CPUs and machines already.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   accel/accel.c             |  4 +---
>   include/qom/object.h      | 12 ++++++++++++
>   qom/object.c              |  5 +++++
>   target/i386/cpu.c         |  8 ++++----
>   target/s390x/cpu_models.c |  4 ++--
>   vl.c                      |  3 +--
>   6 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/accel/accel.c b/accel/accel.c
> index 60c3827..dd38a46 100644
> --- a/accel/accel.c
> +++ b/accel/accel.c
> @@ -48,9 +48,7 @@ AccelClass *accel_find(const char *opt_name)
>   
>   int accel_init_machine(AccelClass *acc, MachineState *ms)
>   {
> -    ObjectClass *oc = OBJECT_CLASS(acc);
> -    const char *cname = object_class_get_name(oc);
> -    AccelState *accel = ACCEL(object_new(cname));
> +    AccelState *accel = ACCEL(object_new_with_class(OBJECT_CLASS(acc)));
>       int ret;
>       ms->accelerator = accel;
>       *(acc->allowed) = true;
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 230b18f..f9ad692 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -593,6 +593,18 @@ struct InterfaceClass
>                                                __FILE__, __LINE__, __func__))
>   
>   /**
> + * object_new_with_class:
> + * @klass: The class to instantiate.
> + *
> + * This function will initialize a new object using heap allocated memory.
> + * The returned object has a reference count of 1, and will be freed when
> + * the last reference is dropped.
> + *
> + * Returns: The newly allocated and instantiated object.
> + */
> +Object *object_new_with_class(ObjectClass *klass);

The function name bugs me... Pick your poison?

   object_new_by_class
   object_new_of_class
   object_new_for_class
   object_new_from_class
   object_new_with_class

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +
> +/**
>    * object_new:
>    * @typename: The name of the type of the object to instantiate.
>    *
> diff --git a/qom/object.c b/qom/object.c
> index bfb4413..bc444d3 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -658,6 +658,11 @@ static Object *object_new_with_type(Type type)
>       return obj;
>   }
>   
> +Object *object_new_with_class(ObjectClass *klass)
> +{
> +    return object_new_with_type(klass->type);
> +}
> +
>   Object *object_new(const char *typename)
>   {
>       TypeImpl *ti = type_get_by_name(typename);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 69f518a..a044078 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4640,7 +4640,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>           return;
>       }
>   
> -    xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
> +    xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc)));
>   
>       x86_cpu_expand_features(xc, &err);
>       if (err) {
> @@ -4711,7 +4711,7 @@ static GSList *get_sorted_cpu_model_list(void)
>   
>   static char *x86_cpu_class_get_model_id(X86CPUClass *xc)
>   {
> -    Object *obj = object_new(object_class_get_name(OBJECT_CLASS(xc)));
> +    Object *obj = object_new_with_class(OBJECT_CLASS(xc));
>       char *r = object_property_get_str(obj, "model-id", &error_abort);
>       object_unref(obj);
>       return r;
> @@ -5092,7 +5092,7 @@ static X86CPU *x86_cpu_from_model(const char *model, QDict *props, Error **errp)
>           goto out;
>       }
>   
> -    xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
> +    xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc)));
>       if (props) {
>           object_apply_props(OBJECT(xc), props, &err);
>           if (err) {
> @@ -5936,7 +5936,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>       APICCommonState *apic;
>       ObjectClass *apic_class = OBJECT_CLASS(apic_get_class());
>   
> -    cpu->apic_state = DEVICE(object_new(object_class_get_name(apic_class)));
> +    cpu->apic_state = DEVICE(object_new_with_class(apic_class));
>   
>       object_property_add_child(OBJECT(cpu), "lapic",
>                                 OBJECT(cpu->apic_state), &error_abort);
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 7e92fb2..72cf48b 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -440,7 +440,7 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
>       if (cpu_list_data->model) {
>           Object *obj;
>           S390CPU *sc;
> -        obj = object_new(object_class_get_name(klass));
> +        obj = object_new_with_class(klass);
>           sc = S390_CPU(obj);
>           if (sc->model) {
>               info->has_unavailable_features = true;
> @@ -501,7 +501,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
>           error_setg(errp, "The CPU definition '%s' requires KVM", info->name);
>           return;
>       }
> -    obj = object_new(object_class_get_name(oc));
> +    obj = object_new_with_class(oc);
>       cpu = S390_CPU(obj);
>   
>       if (!cpu->model) {
> diff --git a/vl.c b/vl.c
> index d6c77bc..f18b26b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3989,8 +3989,7 @@ int main(int argc, char **argv, char **envp)
>                         cleanup_add_fd, NULL, &error_fatal);
>   #endif
>   
> -    current_machine = MACHINE(object_new(object_class_get_name(
> -                          OBJECT_CLASS(machine_class))));
> +    current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
>       if (machine_help_func(qemu_get_machine_opts(), current_machine)) {
>           exit(0);
>       }
> 

Re: [PATCH v2 15/18] xen: convert "-machine igd-passthru" to an accelerator property

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> The first machine property to fall is Xen's Intel integrated graphics
> passthrough.  The "-machine igd-passthru" option does not set anymore
> a property on the machine object, but desugars to a GlobalProperty on
> accelerator objects.
> 
> The setter is very simple, since the value ends up in a
> global variable, so this patch also provides an example before the more
> complicated cases that follow it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/core/machine.c   | 20 --------------------
>   hw/xen/xen-common.c | 16 ++++++++++++++++
>   include/hw/boards.h |  1 -
>   qemu-options.hx     |  7 ++++---
>   vl.c                | 14 ++++----------
>   5 files changed, 24 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 45ddfb6..d7a0356 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -412,20 +412,6 @@ static void machine_set_graphics(Object *obj, bool value, Error **errp)
>       ms->enable_graphics = value;
>   }
>   
> -static bool machine_get_igd_gfx_passthru(Object *obj, Error **errp)
> -{
> -    MachineState *ms = MACHINE(obj);
> -
> -    return ms->igd_gfx_passthru;
> -}
> -
> -static void machine_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
> -{
> -    MachineState *ms = MACHINE(obj);
> -
> -    ms->igd_gfx_passthru = value;
> -}
> -
>   static char *machine_get_firmware(Object *obj, Error **errp)
>   {
>       MachineState *ms = MACHINE(obj);
> @@ -862,12 +848,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
>       object_class_property_set_description(oc, "graphics",
>           "Set on/off to enable/disable graphics emulation", &error_abort);
>   
> -    object_class_property_add_bool(oc, "igd-passthru",
> -        machine_get_igd_gfx_passthru, machine_set_igd_gfx_passthru,
> -        &error_abort);
> -    object_class_property_set_description(oc, "igd-passthru",
> -        "Set on/off to enable/disable igd passthrou", &error_abort);

So before this option was available in builds/targets ...

> -
>       object_class_property_add_str(oc, "firmware",
>           machine_get_firmware, machine_set_firmware,
>           &error_abort);
> diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
> index 5284b0d..6cba30c 100644
> --- a/hw/xen/xen-common.c
> +++ b/hw/xen/xen-common.c
> @@ -124,6 +124,16 @@ static void xen_change_state_handler(void *opaque, int running,
>       }
>   }
>   
> +static bool xen_get_igd_gfx_passthru(Object *obj, Error **errp)
> +{
> +    return has_igd_gfx_passthru;
> +}
> +
> +static void xen_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
> +{
> +    has_igd_gfx_passthru = value;
> +}
> +
>   static void xen_setup_post(MachineState *ms, AccelState *accel)
>   {
>       int rc;
> @@ -177,6 +187,12 @@ static void xen_accel_class_init(ObjectClass *oc, void *data)
>       ac->compat_props = g_ptr_array_new();
>   
>       compat_props_add(ac->compat_props, compat, G_N_ELEMENTS(compat));
> +
> +    object_class_property_add_bool(oc, "igd-passthru",
> +        xen_get_igd_gfx_passthru, xen_set_igd_gfx_passthru,
> +        &error_abort);
> +    object_class_property_set_description(oc, "igd-passthru",
> +        "Set on/off to enable/disable igd passthrou", &error_abort);

... and now it is only available when building with --enable-xen.

This is a good cleanup.

I wonder if ppl uses 'igd-passthru=off' without Xen, won't it break 
their command line?

>   }
>   
>   #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 36fcbda..cdcf481 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -287,7 +287,6 @@ struct MachineState {
>       bool mem_merge;
>       bool usb;
>       bool usb_disabled;
> -    bool igd_gfx_passthru;
>       char *firmware;
>       bool iommu;
>       bool suppress_vmdesc;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9775258..6f12b31 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -37,7 +37,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>       "                kvm_shadow_mem=size of KVM shadow MMU in bytes\n"
>       "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>       "                mem-merge=on|off controls memory merge support (default: on)\n"
> -    "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
>       "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>       "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
>       "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
> @@ -71,8 +70,6 @@ more than one accelerator specified, the next one is used if the previous one
>   fails to initialize.
>   @item kernel_irqchip=on|off
>   Controls in-kernel irqchip support for the chosen accelerator when available.
> -@item gfx_passthru=on|off
> -Enables IGD GFX passthrough support for the chosen machine when available.
>   @item vmport=on|off|auto
>   Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
>   value based on accel. For accel=xen the default is off otherwise the default
> @@ -120,6 +117,7 @@ ETEXI
>   DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>       "-accel [accel=]accelerator[,prop[=value][,...]]\n"
>       "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"

Here we could use:

#ifdef CONFIG_XEN_BACKEND

(not sure we want such #ifdefry although)

> +    "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"

#endif

>       "                tb-size=n (TCG translation block cache size)\n"
>       "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
>   STEXI
> @@ -130,6 +128,9 @@ kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If th
>   more than one accelerator specified, the next one is used if the previous one
>   fails to initialize.
>   @table @option
> +@item igd-passthru=on|off
> +When Xen is in use, this option controls whether Intel integrated graphics
> +devices can be passed through to the guest (default=off)
>   @item tb-size=@var{n}
>   Controls the size (in MiB) of the TCG translation block cache.
>   @item thread=single|multi
> diff --git a/vl.c b/vl.c
> index e6ff56b..ee872f2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1257,13 +1257,6 @@ static void configure_msg(QemuOpts *opts)
>   }
>   
>   
> -/* Now we still need this for compatibility with XEN. */
> -bool has_igd_gfx_passthru;
> -static void igd_gfx_passthru(void)
> -{
> -    has_igd_gfx_passthru = current_machine->igd_gfx_passthru;
> -}
> -
>   /***********************************************************/
>   /* USB devices */
>   
> @@ -2642,6 +2635,10 @@ static int machine_set_property(void *opaque,
>       if (g_str_equal(qom_name, "accel")) {
>           return 0;
>       }

Similarly:

#ifdef CONFIG_XEN_BACKEND

> +    if (g_str_equal(qom_name, "igd-passthru")) {
> +        object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
> +        return 0;
> +    }

#endif

>   
>       return object_parse_property_opt(opaque, name, value, "type", errp);
>   }
> @@ -4456,9 +4453,6 @@ int main(int argc, char **argv, char **envp)
>               exit(1);
>       }
>   
> -    /* Check if IGD GFX passthrough. */
> -    igd_gfx_passthru();
> -
>       /* init generic devices */
>       rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
>       qemu_opts_foreach(qemu_find_opts("device"),
> 

Re: [PATCH v2 14/18] tcg: add "-accel tcg, tb-size" and deprecate "-tb-size"

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> -tb-size fits nicely in the new framework for accelerator-specific options.  It
> is a very niche option, so insta-deprecate it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
[...]
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 65c9473..9775258 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -118,8 +118,9 @@ Select CPU model (@code{-cpu help} for list and additional feature selection)
>   ETEXI
>   
>   DEF("accel", HAS_ARG, QEMU_OPTION_accel,
> -    "-accel [accel=]accelerator[,thread=single|multi]\n"
> +    "-accel [accel=]accelerator[,prop[=value][,...]]\n"

^ This seems from the previous patch, 'convert "-accel threads"'.

>       "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
> +    "                tb-size=n (TCG translation block cache size)\n"
>       "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
>   STEXI
>   @item -accel @var{name}[,prop=@var{value}[,...]]
> @@ -129,6 +130,8 @@ kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If th
>   more than one accelerator specified, the next one is used if the previous one
>   fails to initialize.
>   @table @option
> +@item tb-size=@var{n}
> +Controls the size (in MiB) of the TCG translation block cache.
>   @item thread=single|multi
>   Controls number of TCG threads. When the TCG is multi-threaded there will be one
>   thread per vCPU therefor taking advantage of additional host cores. The default
[...]

Re: [PATCH v2 11/18] qom: add object_new_with_class

[Paolo Bonzini]

On 09/12/19 16:31, Philippe Mathieu-Daudé wrote:
> On 12/9/19 4:01 PM, Paolo Bonzini wrote:
>>     /**
>> + * object_new_with_class:
>> + * @klass: The class to instantiate.
>> + *
>> + * This function will initialize a new object using heap allocated
>> memory.
>> + * The returned object has a reference count of 1, and will be freed
>> when
>> + * the last reference is dropped.
>> + *
>> + * Returns: The newly allocated and instantiated object.
>> + */
>> +Object *object_new_with_class(ObjectClass *klass);
> 
> The function name bugs me... Pick your poison?
> 
>   object_new_by_class
>   object_new_of_class
>   object_new_for_class
>   object_new_from_class
>   object_new_with_class
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

From and with both make sense, I just picked the one consistent with
object_new_with_type.

Paolo

Re: [PATCH v2 14/18] tcg: add "-accel tcg, tb-size" and deprecate "-tb-size"

[Paolo Bonzini]

On 09/12/19 16:52, Philippe Mathieu-Daudé wrote:
>>
>>     DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>> -    "-accel [accel=]accelerator[,thread=single|multi]\n"
>> +    "-accel [accel=]accelerator[,prop[=value][,...]]\n"
> 
> ^ This seems from the previous patch, 'convert "-accel threads"'.

I left it here because until now it was accurate (thread became a QOM
property, but it was still the only -accel suboption).

Paolo

Re: [PATCH v2 00/18] Complete the implementation of -accel

[no-reply]

Patchew URL: https://patchew.org/QEMU/1575903705-12925-1-git-send-email-pbonzini@redhat.com/



Hi,

This series failed the docker-quick@centos7 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
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      migration/fd.o
  CC      migration/exec.o
/tmp/qemu-test/src/hw/xen/xen-common.c: In function 'xen_get_igd_gfx_passthru':
/tmp/qemu-test/src/hw/xen/xen-common.c:129:12: error: 'has_igd_gfx_passthru' undeclared (first use in this function)
     return has_igd_gfx_passthru;
            ^
/tmp/qemu-test/src/hw/xen/xen-common.c:129:12: note: each undeclared identifier is reported only once for each function it appears in
/tmp/qemu-test/src/hw/xen/xen-common.c: In function 'xen_set_igd_gfx_passthru':
/tmp/qemu-test/src/hw/xen/xen-common.c:134:5: error: 'has_igd_gfx_passthru' undeclared (first use in this function)
     has_igd_gfx_passthru = value;
     ^
/tmp/qemu-test/src/hw/xen/xen-common.c: In function 'xen_accel_class_init':
/tmp/qemu-test/src/hw/xen/xen-common.c:193:10: error: 'error_abort' undeclared (first use in this function)
         &error_abort);
          ^
make: *** [hw/xen/xen-common.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 662, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=4ff14485a3e34faa8e0fe7af9be44271', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-vvzmvig_/src/docker-src.2019-12-09-17.39.39.16043:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=4ff14485a3e34faa8e0fe7af9be44271
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-vvzmvig_/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m51.895s
user    0m7.919s


The full log is available at
http://patchew.org/logs/1575903705-12925-1-git-send-email-pbonzini@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v2 00/18] Complete the implementation of -accel

[no-reply]

Patchew URL: https://patchew.org/QEMU/1575903705-12925-1-git-send-email-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v2 00/18] Complete the implementation of -accel
Type: series
Message-id: 1575903705-12925-1-git-send-email-pbonzini@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
fabb8b3 kvm: convert "-machine kernel_irqchip" to an accelerator property
6a1dbee kvm: introduce kvm_kernel_irqchip_* functions
800c958 kvm: convert "-machine kvm_shadow_mem" to an accelerator property
d72d3ea xen: convert "-machine igd-passthru" to an accelerator property
24067b2 tcg: add "-accel tcg, tb-size" and deprecate "-tb-size"
8b42a7e tcg: convert "-accel threads" to a QOM property
3ebd51d accel: pass object to accel_init_machine
4351594 qom: add object_new_with_class
314fb80 qom: introduce object_register_sugar_prop
0995654 vl: warn for unavailable accelerators, clarify messages
5c7b904 vl: configure accelerators from -accel options
6a4da93 vl: introduce object_parse_property_opt
a4b9880 accel: compile accel/accel.c just once
62cdc79 vl: merge -accel processing into configure_accelerators
6246357 vl: extract accelerator option processing to a separate function
63877b9 tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c
1bd01e6 vl: move icount configuration earlier
86d414d memory: do not look at current_machine->accel

=== OUTPUT BEGIN ===
1/18 Checking commit 86d414d86513 (memory: do not look at current_machine->accel)
2/18 Checking commit 1bd01e62ccaf (vl: move icount configuration earlier)
3/18 Checking commit 63877b94ad15 (tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c)
4/18 Checking commit 62463579c28f (vl: extract accelerator option processing to a separate function)
5/18 Checking commit 62cdc7986f89 (vl: merge -accel processing into configure_accelerators)
6/18 Checking commit a4b98800e6a2 (accel: compile accel/accel.c just once)
7/18 Checking commit 6a4da93b15b0 (vl: introduce object_parse_property_opt)
8/18 Checking commit 5c7b904bf344 (vl: configure accelerators from -accel options)
WARNING: line over 80 characters
#206: FILE: vl.c:2909:
+            error_report("The -accel and \"-machine accel=\" options are incompatible");

WARNING: line over 80 characters
#214: FILE: vl.c:2915:
+                           do_configure_accelerator, &init_failed, &error_fatal)) {

total: 0 errors, 2 warnings, 196 lines checked

Patch 8/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/18 Checking commit 099565450487 (vl: warn for unavailable accelerators, clarify messages)
10/18 Checking commit 314fb80d9a2c (qom: introduce object_register_sugar_prop)
WARNING: line over 80 characters
#24: FILE: include/qom/object.h:682:
+void object_register_sugar_prop(const char *driver, const char *prop, const char *value);

WARNING: line over 80 characters
#48: FILE: qom/object.c:428:
+void object_register_sugar_prop(const char *driver, const char *prop, const char *value)

total: 0 errors, 2 warnings, 61 lines checked

Patch 10/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/18 Checking commit 4351594192c0 (qom: add object_new_with_class)
WARNING: line over 80 characters
#142: FILE: vl.c:3992:
+    current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));

total: 0 errors, 1 warnings, 96 lines checked

Patch 11/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/18 Checking commit 3ebd51dc3893 (accel: pass object to accel_init_machine)
13/18 Checking commit 8b42a7e25f3f (tcg: convert "-accel threads" to a QOM property)
14/18 Checking commit 24067b2d77d6 (tcg: add "-accel tcg, tb-size" and deprecate "-tb-size")
ERROR: line over 90 characters
#189: FILE: vl.c:3750:
+                warn_report("The -tb-size option is deprecated, use -accel tcg,tb-size instead");

WARNING: line over 80 characters
#190: FILE: vl.c:3751:
+                object_register_sugar_prop(ACCEL_CLASS_NAME("tcg"), "tb-size", optarg);

total: 1 errors, 1 warnings, 148 lines checked

Patch 14/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

15/18 Checking commit d72d3ea5e7e4 (xen: convert "-machine igd-passthru" to an accelerator property)
16/18 Checking commit 800c95819ba7 (kvm: convert "-machine kvm_shadow_mem" to an accelerator property)
WARNING: line over 80 characters
#228: FILE: target/i386/kvm.c:2166:
+    shadow_mem = object_property_get_int(OBJECT(s), "kvm-shadow-mem", &error_abort);

total: 0 errors, 1 warnings, 197 lines checked

Patch 16/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/18 Checking commit 6a1dbee7f610 (kvm: introduce kvm_kernel_irqchip_* functions)
18/18 Checking commit fabb8b30a711 (kvm: convert "-machine kernel_irqchip" to an accelerator property)
WARNING: Block comments use a leading /* on a separate line
#85: FILE: accel/kvm/kvm-all.c:3007:
+            /* The value was checked in visit_type_OnOffSplit() above. If

total: 0 errors, 1 warnings, 241 lines checked

Patch 18/18 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1575903705-12925-1-git-send-email-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v2 07/18] vl: introduce object_parse_property_opt

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> We will reuse the parsing loop of machine_set_property soon for "-accel",
> but we do not want the "_" -> "-" conversion since "-accel" can just
> standardize on dashes.  We will also add a bunch of legacy option handling
> to keep the QOM machine object clean.  Extract the loop into a separate
> function, and keep the legacy handling in machine_set_property.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   vl.c | 35 ++++++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 19c77b4..66dff1b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2615,27 +2615,17 @@ static MachineClass *select_machine(void)
>       return machine_class;
>   }
>   
> -static int machine_set_property(void *opaque,
> -                                const char *name, const char *value,
> -                                Error **errp)
> +static int object_parse_property_opt(Object *obj,
> +                                     const char *name, const char *value,
> +                                     const char *skip, Error **errp)

Can you document the 'skip' argument?

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>   {
> -    Object *obj = OBJECT(opaque);
>       Error *local_err = NULL;
> -    char *p, *qom_name;
>   
> -    if (strcmp(name, "type") == 0) {
> +    if (g_str_equal(name, skip)) {
>           return 0;
>       }
>   
> -    qom_name = g_strdup(name);
> -    for (p = qom_name; *p; p++) {
> -        if (*p == '_') {
> -            *p = '-';
> -        }
> -    }
> -
> -    object_property_parse(obj, value, qom_name, &local_err);
> -    g_free(qom_name);
> +    object_property_parse(obj, value, name, &local_err);
>   
>       if (local_err) {
>           error_propagate(errp, local_err);
> @@ -2645,6 +2635,21 @@ static int machine_set_property(void *opaque,
>       return 0;
>   }
>   
> +static int machine_set_property(void *opaque,
> +                                const char *name, const char *value,
> +                                Error **errp)
> +{
> +    g_autofree char *qom_name = g_strdup(name);
> +    char *p;
> +
> +    for (p = qom_name; *p; p++) {
> +        if (*p == '_') {
> +            *p = '-';
> +        }
> +    }
> +
> +    return object_parse_property_opt(opaque, name, value, "type", errp);
> +}
>   
>   /*
>    * Initial object creation happens before all other
> 

Re: [PATCH v2 03/18] tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> Move everything related to mttcg_enabled in accel/tcg/tcg-all.c,
> which will make even more sense when "thread" becomes a QOM property.
> 
> For now, initializing mttcg_enabled in the instance_init function
> prepares for the next patch, which will only invoke qemu_tcg_configure
> when the command line includes a -accel option.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/tcg-all.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   cpus.c              | 72 ---------------------------------------------
>   2 files changed, 85 insertions(+), 72 deletions(-)
> 
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index c59d5b0..78a0ab5 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -30,6 +30,11 @@
>   #include "cpu.h"
>   #include "sysemu/cpus.h"
>   #include "qemu/main-loop.h"
> +#include "tcg/tcg.h"
> +#include "include/qapi/error.h"
> +#include "include/qemu/error-report.h"
> +#include "include/hw/boards.h"
> +#include "qemu/option.h"
>   
>   unsigned long tcg_tb_size;
>   
> @@ -58,6 +63,55 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
>       }
>   }
>   
> +/*
> + * We default to false if we know other options have been enabled
> + * which are currently incompatible with MTTCG. Otherwise when each
> + * guest (target) has been updated to support:
> + *   - atomic instructions
> + *   - memory ordering primitives (barriers)
> + * they can set the appropriate CONFIG flags in ${target}-softmmu.mak
> + *
> + * Once a guest architecture has been converted to the new primitives
> + * there are two remaining limitations to check.
> + *
> + * - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
> + * - The host must have a stronger memory order than the guest
> + *
> + * It may be possible in future to support strong guests on weak hosts
> + * but that will require tagging all load/stores in a guest with their
> + * implicit memory order requirements which would likely slow things
> + * down a lot.
> + */
> +
> +static bool check_tcg_memory_orders_compatible(void)
> +{
> +#if defined(TCG_GUEST_DEFAULT_MO) && defined(TCG_TARGET_DEFAULT_MO)
> +    return (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO) == 0;
> +#else
> +    return false;
> +#endif
> +}
> +
> +static bool default_mttcg_enabled(void)
> +{
> +    if (use_icount || TCG_OVERSIZED_GUEST) {
> +        return false;
> +    } else {
> +#ifdef TARGET_SUPPORTS_MTTCG
> +        return check_tcg_memory_orders_compatible();
> +#else
> +        return false;
> +#endif
> +    }
> +}
> +
> +static void tcg_accel_instance_init(Object *obj)
> +{
> +    TCGState *s = TCG_STATE(obj);
> +
> +    mttcg_enabled = default_mttcg_enabled();
> +}
> +
>   static int tcg_init(MachineState *ms)
>   {
>       tcg_exec_init(tcg_tb_size * 1024 * 1024);
> @@ -65,6 +119,36 @@ static int tcg_init(MachineState *ms)
>       return 0;
>   }
>   
> +void qemu_tcg_configure(QemuOpts *opts, Error **errp)
> +{
> +    const char *t = qemu_opt_get(opts, "thread");
> +    if (!t) {
> +        return;
> +    }
> +    if (strcmp(t, "multi") == 0) {
> +        if (TCG_OVERSIZED_GUEST) {
> +            error_setg(errp, "No MTTCG when guest word size > hosts");
> +        } else if (use_icount) {
> +            error_setg(errp, "No MTTCG when icount is enabled");
> +        } else {
> +#ifndef TARGET_SUPPORTS_MTTCG
> +            warn_report("Guest not yet converted to MTTCG - "
> +                        "you may get unexpected results");
> +#endif
> +            if (!check_tcg_memory_orders_compatible()) {
> +                warn_report("Guest expects a stronger memory ordering "
> +                            "than the host provides");
> +                error_printf("This may cause strange/hard to debug errors\n");
> +            }
> +            mttcg_enabled = true;
> +        }
> +    } else if (strcmp(t, "single") == 0) {
> +        mttcg_enabled = false;
> +    } else {
> +        error_setg(errp, "Invalid 'thread' setting %s", t);
> +    }
> +}
> +
>   static void tcg_accel_class_init(ObjectClass *oc, void *data)
>   {
>       AccelClass *ac = ACCEL_CLASS(oc);
> @@ -78,6 +162,7 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
>   static const TypeInfo tcg_accel_type = {
>       .name = TYPE_TCG_ACCEL,
>       .parent = TYPE_ACCEL,
> +    .instance_init = tcg_accel_instance_init,
>       .class_init = tcg_accel_class_init,
>   };
>   
> diff --git a/cpus.c b/cpus.c
> index 63bda15..b472378 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -166,78 +166,6 @@ typedef struct TimersState {
>   static TimersState timers_state;
>   bool mttcg_enabled;
>   
> -/*
> - * We default to false if we know other options have been enabled
> - * which are currently incompatible with MTTCG. Otherwise when each
> - * guest (target) has been updated to support:
> - *   - atomic instructions
> - *   - memory ordering primitives (barriers)
> - * they can set the appropriate CONFIG flags in ${target}-softmmu.mak
> - *
> - * Once a guest architecture has been converted to the new primitives
> - * there are two remaining limitations to check.
> - *
> - * - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
> - * - The host must have a stronger memory order than the guest
> - *
> - * It may be possible in future to support strong guests on weak hosts
> - * but that will require tagging all load/stores in a guest with their
> - * implicit memory order requirements which would likely slow things
> - * down a lot.
> - */
> -
> -static bool check_tcg_memory_orders_compatible(void)
> -{
> -#if defined(TCG_GUEST_DEFAULT_MO) && defined(TCG_TARGET_DEFAULT_MO)
> -    return (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO) == 0;
> -#else
> -    return false;
> -#endif
> -}
> -
> -static bool default_mttcg_enabled(void)
> -{
> -    if (use_icount || TCG_OVERSIZED_GUEST) {
> -        return false;
> -    } else {
> -#ifdef TARGET_SUPPORTS_MTTCG
> -        return check_tcg_memory_orders_compatible();
> -#else
> -        return false;
> -#endif
> -    }
> -}
> -
> -void qemu_tcg_configure(QemuOpts *opts, Error **errp)
> -{
> -    const char *t = qemu_opt_get(opts, "thread");
> -    if (t) {
> -        if (strcmp(t, "multi") == 0) {
> -            if (TCG_OVERSIZED_GUEST) {
> -                error_setg(errp, "No MTTCG when guest word size > hosts");
> -            } else if (use_icount) {
> -                error_setg(errp, "No MTTCG when icount is enabled");
> -            } else {
> -#ifndef TARGET_SUPPORTS_MTTCG
> -                warn_report("Guest not yet converted to MTTCG - "
> -                            "you may get unexpected results");
> -#endif
> -                if (!check_tcg_memory_orders_compatible()) {
> -                    warn_report("Guest expects a stronger memory ordering "
> -                                "than the host provides");
> -                    error_printf("This may cause strange/hard to debug errors\n");
> -                }
> -                mttcg_enabled = true;
> -            }
> -        } else if (strcmp(t, "single") == 0) {
> -            mttcg_enabled = false;
> -        } else {
> -            error_setg(errp, "Invalid 'thread' setting %s", t);
> -        }
> -    } else {
> -        mttcg_enabled = default_mttcg_enabled();
> -    }
> -}
>   
>   /* The current number of executed instructions is based on what we
>    * originally budgeted minus the current state of the decrementing
> 

Re: [PATCH v2 04/18] vl: extract accelerator option processing to a separate function

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> As a first step towards supporting multiple "-accel" options, push the
> late processing of -icount and -accel into a new function, and use
> qemu_opts_foreach to retrieve -accel options instead of stashing
> them into globals.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   vl.c | 28 ++++++++++++++++++++--------
>   1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index f69fea1..1ad6dfb 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2831,6 +2831,25 @@ static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
>       return 0;
>   }
>   
> +static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
> +{
> +    if (tcg_enabled()) {
> +        qemu_tcg_configure(opts, &error_fatal);
> +    }
> +    return 0;
> +}
> +
> +static void configure_accelerators(void)
> +{
> +    qemu_opts_foreach(qemu_find_opts("accel"),
> +                      do_configure_accelerator, NULL, &error_fatal);
> +
> +    if (!tcg_enabled() && use_icount) {
> +        error_report("-icount is not allowed with hardware virtualization");
> +        exit(1);
> +    }
> +}
> +
>   int main(int argc, char **argv, char **envp)
>   {
>       int i;
> @@ -4258,14 +4277,7 @@ int main(int argc, char **argv, char **envp)
>       qemu_spice_init();
>   
>       cpu_ticks_init();
> -    if (!tcg_enabled() && use_icount) {
> -        error_report("-icount is not allowed with hardware virtualization");
> -        exit(1);
> -    }
> -
> -    if (tcg_enabled()) {
> -        qemu_tcg_configure(accel_opts, &error_fatal);
> -    }
> +    configure_accelerators();
>   
>       if (default_net) {
>           QemuOptsList *net = qemu_find_opts("net");
> 

Re: [PATCH v2 05/18] vl: merge -accel processing into configure_accelerators

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> The next step is to move the parsing of "-machine accel=..." into vl.c,
> unifying it with the configure_accelerators() function that has just
> been introduced.  This way, we will be able to desugar it into multiple
> "-accel" options, without polluting accel/accel.c.
> 
> The CONFIG_TCG and CONFIG_KVM symbols are not available in vl.c, but
> we can use accel_find instead to find their value at runtime.  Once we
> know that the binary has one of TCG or KVM, the default accelerator
> can be expressed simply as "tcg:kvm", because TCG never fails to initialize.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/accel.c          | 69 ++------------------------------------------------
>   include/sysemu/accel.h |  4 ++-
>   vl.c                   | 64 ++++++++++++++++++++++++++++++++++++++++++----
>   3 files changed, 64 insertions(+), 73 deletions(-)
> 
> diff --git a/accel/accel.c b/accel/accel.c
> index 5fa3171..60c3827 100644
> --- a/accel/accel.c
> +++ b/accel/accel.c
> @@ -28,13 +28,7 @@
>   #include "hw/boards.h"
>   #include "sysemu/arch_init.h"
>   #include "sysemu/sysemu.h"
> -#include "sysemu/kvm.h"
> -#include "sysemu/qtest.h"
> -#include "hw/xen/xen.h"
>   #include "qom/object.h"
> -#include "qemu/error-report.h"
> -#include "qemu/option.h"
> -#include "qapi/error.h"
>   
>   static const TypeInfo accel_type = {
>       .name = TYPE_ACCEL,
> @@ -44,7 +38,7 @@ static const TypeInfo accel_type = {
>   };
>   
>   /* Lookup AccelClass from opt_name. Returns NULL if not found */
> -static AccelClass *accel_find(const char *opt_name)
> +AccelClass *accel_find(const char *opt_name)
>   {
>       char *class_name = g_strdup_printf(ACCEL_CLASS_NAME("%s"), opt_name);
>       AccelClass *ac = ACCEL_CLASS(object_class_by_name(class_name));
> @@ -52,7 +46,7 @@ static AccelClass *accel_find(const char *opt_name)
>       return ac;
>   }
>   
> -static int accel_init_machine(AccelClass *acc, MachineState *ms)
> +int accel_init_machine(AccelClass *acc, MachineState *ms)
>   {
>       ObjectClass *oc = OBJECT_CLASS(acc);
>       const char *cname = object_class_get_name(oc);
> @@ -71,65 +65,6 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
>       return ret;
>   }
>   
> -void configure_accelerator(MachineState *ms, const char *progname)
> -{
> -    const char *accel;
> -    char **accel_list, **tmp;
> -    int ret;
> -    bool accel_initialised = false;
> -    bool init_failed = false;
> -    AccelClass *acc = NULL;
> -
> -    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> -    if (accel == NULL) {
> -        /* Select the default accelerator */
> -        int pnlen = strlen(progname);
> -        if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
> -            /* If the program name ends with "kvm", we prefer KVM */
> -            accel = "kvm:tcg";
> -        } else {
> -#if defined(CONFIG_TCG)
> -            accel = "tcg";
> -#elif defined(CONFIG_KVM)
> -            accel = "kvm";
> -#else
> -            error_report("No accelerator selected and"
> -                         " no default accelerator available");
> -            exit(1);
> -#endif
> -        }
> -    }
> -
> -    accel_list = g_strsplit(accel, ":", 0);
> -
> -    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> -        acc = accel_find(*tmp);
> -        if (!acc) {
> -            continue;
> -        }
> -        ret = accel_init_machine(acc, ms);
> -        if (ret < 0) {
> -            init_failed = true;
> -            error_report("failed to initialize %s: %s",
> -                         acc->name, strerror(-ret));
> -        } else {
> -            accel_initialised = true;
> -        }
> -    }
> -    g_strfreev(accel_list);
> -
> -    if (!accel_initialised) {
> -        if (!init_failed) {
> -            error_report("-machine accel=%s: No accelerator found", accel);
> -        }
> -        exit(1);
> -    }
> -
> -    if (init_failed) {
> -        error_report("Back to %s accelerator", acc->name);
> -    }
> -}
> -
>   void accel_setup_post(MachineState *ms)
>   {
>       AccelState *accel = ms->accelerator;
> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
> index 8eb60b8..90b6213 100644
> --- a/include/sysemu/accel.h
> +++ b/include/sysemu/accel.h
> @@ -66,7 +66,9 @@ typedef struct AccelClass {
>   
>   extern unsigned long tcg_tb_size;
>   
> -void configure_accelerator(MachineState *ms, const char *progname);
> +AccelClass *accel_find(const char *opt_name);
> +int accel_init_machine(AccelClass *acc, MachineState *ms);
> +
>   /* Called just before os_setup_post (ie just before drop OS privs) */
>   void accel_setup_post(MachineState *ms);
>   
> diff --git a/vl.c b/vl.c
> index 1ad6dfb..19c77b4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2839,8 +2839,65 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>       return 0;
>   }
>   
> -static void configure_accelerators(void)
> +static void configure_accelerators(const char *progname)
>   {
> +    const char *accel;
> +    char **accel_list, **tmp;
> +    int ret;
> +    bool accel_initialised = false;
> +    bool init_failed = false;
> +    AccelClass *acc = NULL;
> +
> +    qemu_opts_foreach(qemu_find_opts("icount"),
> +                      do_configure_icount, NULL, &error_fatal);
> +
> +    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> +    if (accel == NULL) {
> +        /* Select the default accelerator */
> +        if (!accel_find("tcg") && !accel_find("kvm")) {
> +            error_report("No accelerator selected and"
> +                         " no default accelerator available");
> +            exit(1);
> +        } else {
> +            int pnlen = strlen(progname);
> +            if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
> +                /* If the program name ends with "kvm", we prefer KVM */
> +                accel = "kvm:tcg";
> +            } else {
> +                accel = "tcg:kvm";
> +            }
> +        }
> +    }
> +
> +    accel_list = g_strsplit(accel, ":", 0);
> +
> +    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> +        acc = accel_find(*tmp);
> +        if (!acc) {
> +            continue;
> +        }
> +        ret = accel_init_machine(acc, current_machine);
> +        if (ret < 0) {
> +            init_failed = true;
> +            error_report("failed to initialize %s: %s",
> +                         acc->name, strerror(-ret));
> +        } else {
> +            accel_initialised = true;
> +        }
> +    }
> +    g_strfreev(accel_list);
> +
> +    if (!accel_initialised) {
> +        if (!init_failed) {
> +            error_report("-machine accel=%s: No accelerator found", accel);
> +        }
> +        exit(1);
> +    }
> +
> +    if (init_failed) {
> +        error_report("Back to %s accelerator", acc->name);
> +    }
> +
>       qemu_opts_foreach(qemu_find_opts("accel"),
>                         do_configure_accelerator, NULL, &error_fatal);
>   
> @@ -4190,9 +4247,7 @@ int main(int argc, char **argv, char **envp)
>        * Note: uses machine properties such as kernel-irqchip, must run
>        * after machine_set_property().
>        */
> -    qemu_opts_foreach(qemu_find_opts("icount"),
> -                      do_configure_icount, NULL, &error_fatal);
> -    configure_accelerator(current_machine, argv[0]);
> +    configure_accelerators(argv[0]);
>   
>       /*
>        * Beware, QOM objects created before this point miss global and
> @@ -4277,7 +4332,6 @@ int main(int argc, char **argv, char **envp)
>       qemu_spice_init();
>   
>       cpu_ticks_init();
> -    configure_accelerators();
>   
>       if (default_net) {
>           QemuOptsList *net = qemu_find_opts("net");
> 

Re: [PATCH v2 06/18] accel: compile accel/accel.c just once

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> Now that accel/accel.c does not use CONFIG_TCG or CONFIG_KVM anymore,
> it need not be compiled once for every softmmu target.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   Makefile.objs       | 1 +
>   accel/Makefile.objs | 2 +-
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 11ba1a3..b6fcbac 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -55,6 +55,7 @@ common-obj-$(CONFIG_POSIX) += os-posix.o
>   
>   common-obj-$(CONFIG_LINUX) += fsdev/
>   
> +common-obj-y += accel/
>   common-obj-y += migration/
>   
>   common-obj-y += audio/
> diff --git a/accel/Makefile.objs b/accel/Makefile.objs
> index 8b498d3..17e5ac6 100644
> --- a/accel/Makefile.objs
> +++ b/accel/Makefile.objs
> @@ -1,4 +1,4 @@
> -obj-$(CONFIG_SOFTMMU) += accel.o
> +common-obj-$(CONFIG_SOFTMMU) += accel.o
>   obj-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_POSIX)) += qtest.o
>   obj-$(CONFIG_KVM) += kvm/
>   obj-$(CONFIG_TCG) += tcg/
> 

Re: [PATCH v2 16/18] kvm: convert "-machine kvm_shadow_mem" to an accelerator property

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   accel/kvm/kvm-all.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>   hw/core/machine.c   | 39 ---------------------------------------
>   include/hw/boards.h |  2 --
>   qemu-options.hx     |  6 +++---
>   target/i386/kvm.c   |  2 +-
>   vl.c                |  4 ++++
>   6 files changed, 51 insertions(+), 45 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index ca00daa..f0b9294 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -41,6 +41,7 @@
>   #include "hw/irq.h"
>   #include "sysemu/sev.h"
>   #include "sysemu/balloon.h"
> +#include "qapi/visitor.h"
>   
>   #include "hw/boards.h"
>   
> @@ -92,6 +93,7 @@ struct KVMState
>       int max_nested_state_len;
>       int many_ioeventfds;
>       int intx_set_mask;
> +    int kvm_shadow_mem;
>       bool sync_mmu;
>       bool manual_dirty_log_protect;
>       /* The man page (and posix) say ioctl numbers are signed int, but
> @@ -2940,6 +2942,40 @@ static bool kvm_accel_has_memory(MachineState *ms, AddressSpace *as,
>       return false;
>   }
>   
> +static void kvm_get_kvm_shadow_mem(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +    KVMState *s = KVM_STATE(obj);
> +    int64_t value = s->kvm_shadow_mem;
> +
> +    visit_type_int(v, name, &value, errp);
> +}
> +
> +static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +    KVMState *s = KVM_STATE(obj);
> +    Error *error = NULL;
> +    int64_t value;
> +
> +    visit_type_int(v, name, &value, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    s->kvm_shadow_mem = value;
> +}
> +
> +static void kvm_accel_instance_init(Object *obj)
> +{
> +    KVMState *s = KVM_STATE(obj);
> +
> +    s->kvm_shadow_mem = -1;
> +}
> +
>   static void kvm_accel_class_init(ObjectClass *oc, void *data)
>   {
>       AccelClass *ac = ACCEL_CLASS(oc);
> @@ -2947,11 +2983,18 @@ static void kvm_accel_class_init(ObjectClass *oc, void *data)
>       ac->init_machine = kvm_init;
>       ac->has_memory = kvm_accel_has_memory;
>       ac->allowed = &kvm_allowed;
> +
> +    object_class_property_add(oc, "kvm-shadow-mem", "int",
> +        kvm_get_kvm_shadow_mem, kvm_set_kvm_shadow_mem,
> +        NULL, NULL, &error_abort);
> +    object_class_property_set_description(oc, "kvm-shadow-mem",
> +        "KVM shadow MMU size", &error_abort);
>   }
>   
>   static const TypeInfo kvm_accel_type = {
>       .name = TYPE_KVM_ACCEL,
>       .parent = TYPE_ACCEL,
> +    .instance_init = kvm_accel_instance_init,
>       .class_init = kvm_accel_class_init,
>       .instance_size = sizeof(KVMState),
>   };
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index d7a0356..bfb5f6f 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -211,33 +211,6 @@ static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
>       }
>   }
>   
> -static void machine_get_kvm_shadow_mem(Object *obj, Visitor *v,
> -                                       const char *name, void *opaque,
> -                                       Error **errp)
> -{
> -    MachineState *ms = MACHINE(obj);
> -    int64_t value = ms->kvm_shadow_mem;
> -
> -    visit_type_int(v, name, &value, errp);
> -}
> -
> -static void machine_set_kvm_shadow_mem(Object *obj, Visitor *v,
> -                                       const char *name, void *opaque,
> -                                       Error **errp)
> -{
> -    MachineState *ms = MACHINE(obj);
> -    Error *error = NULL;
> -    int64_t value;
> -
> -    visit_type_int(v, name, &value, &error);
> -    if (error) {
> -        error_propagate(errp, error);
> -        return;
> -    }
> -
> -    ms->kvm_shadow_mem = value;
> -}
> -
>   static char *machine_get_kernel(Object *obj, Error **errp)
>   {
>       MachineState *ms = MACHINE(obj);
> @@ -785,12 +758,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
>       object_class_property_set_description(oc, "kernel-irqchip",
>           "Configure KVM in-kernel irqchip", &error_abort);
>   
> -    object_class_property_add(oc, "kvm-shadow-mem", "int",
> -        machine_get_kvm_shadow_mem, machine_set_kvm_shadow_mem,
> -        NULL, NULL, &error_abort);
> -    object_class_property_set_description(oc, "kvm-shadow-mem",
> -        "KVM shadow MMU size", &error_abort);
> -
>       object_class_property_add_str(oc, "kernel",
>           machine_get_kernel, machine_set_kernel, &error_abort);
>       object_class_property_set_description(oc, "kernel",
> @@ -892,7 +859,6 @@ static void machine_initfn(Object *obj)
>   
>       ms->kernel_irqchip_allowed = true;
>       ms->kernel_irqchip_split = mc->default_kernel_irqchip_split;
> -    ms->kvm_shadow_mem = -1;
>       ms->dump_guest_core = true;
>       ms->mem_merge = true;
>       ms->enable_graphics = true;
> @@ -963,11 +929,6 @@ bool machine_kernel_irqchip_split(MachineState *machine)
>       return machine->kernel_irqchip_split;
>   }
>   
> -int machine_kvm_shadow_mem(MachineState *machine)
> -{
> -    return machine->kvm_shadow_mem;
> -}
> -
>   int machine_phandle_start(MachineState *machine)
>   {
>       return machine->phandle_start;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index cdcf481..46119eb 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -66,7 +66,6 @@ bool machine_usb(MachineState *machine);
>   bool machine_kernel_irqchip_allowed(MachineState *machine);
>   bool machine_kernel_irqchip_required(MachineState *machine);
>   bool machine_kernel_irqchip_split(MachineState *machine);
> -int machine_kvm_shadow_mem(MachineState *machine);
>   int machine_phandle_start(MachineState *machine);
>   bool machine_dump_guest_core(MachineState *machine);
>   bool machine_mem_merge(MachineState *machine);
> @@ -278,7 +277,6 @@ struct MachineState {
>       bool kernel_irqchip_allowed;
>       bool kernel_irqchip_required;
>       bool kernel_irqchip_split;
> -    int kvm_shadow_mem;
>       char *dtb;
>       char *dumpdtb;
>       int phandle_start;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 6f12b31..779c8af 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -34,7 +34,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>       "                supported accelerators are kvm, xen, hax, hvf, whpx or tcg (default: tcg)\n"
>       "                kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n"
>       "                vmport=on|off|auto controls emulation of vmport (default: auto)\n"
> -    "                kvm_shadow_mem=size of KVM shadow MMU in bytes\n"
>       "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>       "                mem-merge=on|off controls memory merge support (default: on)\n"
>       "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
> @@ -74,8 +73,6 @@ Controls in-kernel irqchip support for the chosen accelerator when available.
>   Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
>   value based on accel. For accel=xen the default is off otherwise the default
>   is on.
> -@item kvm_shadow_mem=size
> -Defines the size of the KVM shadow MMU.
>   @item dump-guest-core=on|off
>   Include guest memory in a core dump. The default is on.
>   @item mem-merge=on|off
> @@ -118,6 +115,7 @@ DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>       "-accel [accel=]accelerator[,prop[=value][,...]]\n"
>       "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
>       "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"
> +    "                kvm-shadow-mem=size of KVM shadow MMU in bytes\n"
>       "                tb-size=n (TCG translation block cache size)\n"
>       "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
>   STEXI
> @@ -131,6 +129,8 @@ fails to initialize.
>   @item igd-passthru=on|off
>   When Xen is in use, this option controls whether Intel integrated graphics
>   devices can be passed through to the guest (default=off)
> +@item kvm-shadow-mem=size
> +Defines the size of the KVM shadow MMU.
>   @item tb-size=@var{n}
>   Controls the size (in MiB) of the TCG translation block cache.
>   @item thread=single|multi
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 1d10046..62ce681 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -2163,7 +2163,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       }
>       qemu_register_reset(kvm_unpoison_all, NULL);
>   
> -    shadow_mem = machine_kvm_shadow_mem(ms);
> +    shadow_mem = object_property_get_int(OBJECT(s), "kvm-shadow-mem", &error_abort);
>       if (shadow_mem != -1) {
>           shadow_mem /= 4096;
>           ret = kvm_vm_ioctl(s, KVM_SET_NR_MMU_PAGES, shadow_mem);
> diff --git a/vl.c b/vl.c
> index ee872f2..3678522 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2639,6 +2639,10 @@ static int machine_set_property(void *opaque,
>           object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
>           return 0;
>       }
> +    if (g_str_equal(qom_name, "kvm-shadow-mem")) {
> +        object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value);
> +        return 0;
> +    }
>   
>       return object_parse_property_opt(opaque, name, value, "type", errp);
>   }
> 

Nice.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 17/18] kvm: introduce kvm_kernel_irqchip_* functions

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> The KVMState struct is opaque, so provide accessors for the fields
> that will be moved from current_machine to the accelerator.  For now
> they just forward to the machine object, but this will change.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/kvm/kvm-all.c  | 23 +++++++++++++++++++----
>   hw/ppc/e500.c        |  4 ++--
>   hw/ppc/spapr_irq.c   | 10 +++++-----
>   include/sysemu/kvm.h |  7 +++++--
>   target/arm/kvm.c     |  8 ++++----
>   target/i386/kvm.c    |  4 ++--
>   target/mips/kvm.c    |  2 +-
>   target/ppc/kvm.c     |  2 +-
>   target/s390x/kvm.c   |  2 +-
>   9 files changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index f0b9294..c0a6351 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1760,7 +1760,7 @@ void kvm_irqchip_set_qemuirq_gsi(KVMState *s, qemu_irq irq, int gsi)
>       g_hash_table_insert(s->gsimap, irq, GINT_TO_POINTER(gsi));
>   }
>   
> -static void kvm_irqchip_create(MachineState *machine, KVMState *s)
> +static void kvm_irqchip_create(KVMState *s)
>   {
>       int ret;
>   
> @@ -1778,9 +1778,9 @@ static void kvm_irqchip_create(MachineState *machine, KVMState *s)
>   
>       /* First probe and see if there's a arch-specific hook to create the
>        * in-kernel irqchip for us */
> -    ret = kvm_arch_irqchip_create(machine, s);
> +    ret = kvm_arch_irqchip_create(s);
>       if (ret == 0) {
> -        if (machine_kernel_irqchip_split(machine)) {
> +        if (kvm_kernel_irqchip_split()) {
>               perror("Split IRQ chip mode not supported.");
>               exit(1);
>           } else {
> @@ -2052,7 +2052,7 @@ static int kvm_init(MachineState *ms)
>       }
>   
>       if (machine_kernel_irqchip_allowed(ms)) {
> -        kvm_irqchip_create(ms, s);
> +        kvm_irqchip_create(s);
>       }
>   
>       if (kvm_eventfds_allowed) {
> @@ -2969,6 +2969,21 @@ static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
>       s->kvm_shadow_mem = value;
>   }
>   
> +bool kvm_kernel_irqchip_allowed(void)
> +{
> +    return machine_kernel_irqchip_allowed(current_machine);
> +}
> +
> +bool kvm_kernel_irqchip_required(void)
> +{
> +    return machine_kernel_irqchip_required(current_machine);
> +}
> +
> +bool kvm_kernel_irqchip_split(void)
> +{
> +    return machine_kernel_irqchip_split(current_machine);
> +}
> +
>   static void kvm_accel_instance_init(Object *obj)
>   {
>       KVMState *s = KVM_STATE(obj);
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 91cd4c2..928efaa 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -801,10 +801,10 @@ static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms,
>       if (kvm_enabled()) {
>           Error *err = NULL;
>   
> -        if (machine_kernel_irqchip_allowed(machine)) {
> +        if (kvm_kernel_irqchip_allowed()) {
>               dev = ppce500_init_mpic_kvm(pmc, irqs, &err);
>           }
> -        if (machine_kernel_irqchip_required(machine) && !dev) {
> +        if (kvm_kernel_irqchip_required() && !dev) {
>               error_reportf_err(err,
>                                 "kernel_irqchip requested but unavailable: ");
>               exit(1);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d6bb7fd..c3f8870 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -77,9 +77,9 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **),
>       MachineState *machine = MACHINE(qdev_get_machine());
>       Error *local_err = NULL;
>   
> -    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> +    if (kvm_enabled() && kvm_kernel_irqchip_allowed()) {
>           if (fn(intc, &local_err) < 0) {
> -            if (machine_kernel_irqchip_required(machine)) {
> +            if (kvm_kernel_irqchip_required()) {
>                   error_prepend(&local_err,
>                                 "kernel_irqchip requested but unavailable: ");
>                   error_propagate(errp, local_err);
> @@ -184,7 +184,7 @@ static int spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>        */
>       if (kvm_enabled() &&
>           spapr->irq == &spapr_irq_dual &&
> -        machine_kernel_irqchip_required(machine) &&
> +        kvm_kernel_irqchip_required() &&
>           xics_kvm_has_broken_disconnect(spapr)) {
>           error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on");
>           return -1;
> @@ -290,12 +290,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>       MachineState *machine = MACHINE(spapr);
>       SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>   
> -    if (machine_kernel_irqchip_split(machine)) {
> +    if (kvm_enabled() && kvm_kernel_irqchip_split()) {
>           error_setg(errp, "kernel_irqchip split mode not supported on pseries");
>           return;
>       }
>   
> -    if (!kvm_enabled() && machine_kernel_irqchip_required(machine)) {
> +    if (!kvm_enabled() && kvm_kernel_irqchip_required()) {
>           error_setg(errp,
>                      "kernel_irqchip requested but only available with KVM");
>           return;
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 9fe233b..aaf2a50 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -519,10 +519,13 @@ void kvm_pc_gsi_handler(void *opaque, int n, int level);
>   void kvm_pc_setup_irq_routing(bool pci_enabled);
>   void kvm_init_irq_routing(KVMState *s);
>   
> +bool kvm_kernel_irqchip_allowed(void);
> +bool kvm_kernel_irqchip_required(void);
> +bool kvm_kernel_irqchip_split(void);
> +
>   /**
>    * kvm_arch_irqchip_create:
>    * @KVMState: The KVMState pointer
> - * @MachineState: The MachineState pointer
>    *
>    * Allow architectures to create an in-kernel irq chip themselves.
>    *
> @@ -530,7 +533,7 @@ void kvm_init_irq_routing(KVMState *s);
>    *            0: irq chip was not created
>    *          > 0: irq chip was created
>    */
> -int kvm_arch_irqchip_create(MachineState *ms, KVMState *s);
> +int kvm_arch_irqchip_create(KVMState *s);
>   
>   /**
>    * kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 5b82cef..b87b59a 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -741,11 +741,11 @@ void kvm_arch_init_irq_routing(KVMState *s)
>   {
>   }
>   
> -int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
> +int kvm_arch_irqchip_create(KVMState *s)
>   {
> -     if (machine_kernel_irqchip_split(ms)) {
> -         perror("-machine kernel_irqchip=split is not supported on ARM.");
> -         exit(1);
> +    if (kvm_kernel_irqchip_split()) {
> +        perror("-machine kernel_irqchip=split is not supported on ARM.");
> +        exit(1);
>       }
>   
>       /* If we can create the VGIC using the newer device control API, we
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 62ce681..ef63f3a 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -4494,10 +4494,10 @@ void kvm_arch_init_irq_routing(KVMState *s)
>       }
>   }
>   
> -int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
> +int kvm_arch_irqchip_create(KVMState *s)
>   {
>       int ret;
> -    if (machine_kernel_irqchip_split(ms)) {
> +    if (kvm_kernel_irqchip_split()) {
>           ret = kvm_vm_enable_cap(s, KVM_CAP_SPLIT_IRQCHIP, 0, 24);
>           if (ret) {
>               error_report("Could not enable split irqchip mode: %s",
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index 578bc14..de3e26e 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -57,7 +57,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       return 0;
>   }
>   
> -int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
> +int kvm_arch_irqchip_create(KVMState *s)
>   {
>       return 0;
>   }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index c77f984..461dc6d 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -152,7 +152,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       return 0;
>   }
>   
> -int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
> +int kvm_arch_irqchip_create(KVMState *s)
>   {
>       return 0;
>   }
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 0c9d14b..f3a742b 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -374,7 +374,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       return 0;
>   }
>   
> -int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
> +int kvm_arch_irqchip_create(KVMState *s)
>   {
>       return 0;
>   }
> 

Re: [PATCH v2 03/18] tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c

[Marc-André Lureau]

Hi

On Mon, Dec 9, 2019 at 7:07 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Move everything related to mttcg_enabled in accel/tcg/tcg-all.c,
> which will make even more sense when "thread" becomes a QOM property.
>
> For now, initializing mttcg_enabled in the instance_init function
> prepares for the next patch, which will only invoke qemu_tcg_configure
> when the command line includes a -accel option.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/tcg/tcg-all.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  cpus.c              | 72 ---------------------------------------------
>  2 files changed, 85 insertions(+), 72 deletions(-)
>
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index c59d5b0..78a0ab5 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -30,6 +30,11 @@
>  #include "cpu.h"
>  #include "sysemu/cpus.h"
>  #include "qemu/main-loop.h"
> +#include "tcg/tcg.h"
> +#include "include/qapi/error.h"
> +#include "include/qemu/error-report.h"
> +#include "include/hw/boards.h"
> +#include "qemu/option.h"
>
>  unsigned long tcg_tb_size;
>
> @@ -58,6 +63,55 @@ static void tcg_handle_interrupt(CPUState *cpu, int mask)
>      }
>  }
>
> +/*
> + * We default to false if we know other options have been enabled
> + * which are currently incompatible with MTTCG. Otherwise when each
> + * guest (target) has been updated to support:
> + *   - atomic instructions
> + *   - memory ordering primitives (barriers)
> + * they can set the appropriate CONFIG flags in ${target}-softmmu.mak
> + *
> + * Once a guest architecture has been converted to the new primitives
> + * there are two remaining limitations to check.
> + *
> + * - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
> + * - The host must have a stronger memory order than the guest
> + *
> + * It may be possible in future to support strong guests on weak hosts
> + * but that will require tagging all load/stores in a guest with their
> + * implicit memory order requirements which would likely slow things
> + * down a lot.
> + */
> +
> +static bool check_tcg_memory_orders_compatible(void)
> +{
> +#if defined(TCG_GUEST_DEFAULT_MO) && defined(TCG_TARGET_DEFAULT_MO)
> +    return (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO) == 0;
> +#else
> +    return false;
> +#endif
> +}
> +
> +static bool default_mttcg_enabled(void)
> +{
> +    if (use_icount || TCG_OVERSIZED_GUEST) {
> +        return false;
> +    } else {
> +#ifdef TARGET_SUPPORTS_MTTCG
> +        return check_tcg_memory_orders_compatible();
> +#else
> +        return false;
> +#endif
> +    }
> +}
> +
> +static void tcg_accel_instance_init(Object *obj)
> +{
> +    TCGState *s = TCG_STATE(obj);

It breaks compilation:

accel/tcg/tcg-all.c:110:19: error: implicit declaration of function
‘TCG_STATE’ [-Werror=implicit-function-declaration]
  110 |     TCGState *s = TCG_STATE(obj);

> +
> +    mttcg_enabled = default_mttcg_enabled();
> +}
> +
>  static int tcg_init(MachineState *ms)
>  {
>      tcg_exec_init(tcg_tb_size * 1024 * 1024);
> @@ -65,6 +119,36 @@ static int tcg_init(MachineState *ms)
>      return 0;
>  }
>
> +void qemu_tcg_configure(QemuOpts *opts, Error **errp)
> +{
> +    const char *t = qemu_opt_get(opts, "thread");
> +    if (!t) {
> +        return;
> +    }
> +    if (strcmp(t, "multi") == 0) {
> +        if (TCG_OVERSIZED_GUEST) {
> +            error_setg(errp, "No MTTCG when guest word size > hosts");
> +        } else if (use_icount) {
> +            error_setg(errp, "No MTTCG when icount is enabled");
> +        } else {
> +#ifndef TARGET_SUPPORTS_MTTCG
> +            warn_report("Guest not yet converted to MTTCG - "
> +                        "you may get unexpected results");
> +#endif
> +            if (!check_tcg_memory_orders_compatible()) {
> +                warn_report("Guest expects a stronger memory ordering "
> +                            "than the host provides");
> +                error_printf("This may cause strange/hard to debug errors\n");
> +            }
> +            mttcg_enabled = true;
> +        }
> +    } else if (strcmp(t, "single") == 0) {
> +        mttcg_enabled = false;
> +    } else {
> +        error_setg(errp, "Invalid 'thread' setting %s", t);
> +    }
> +}
> +
>  static void tcg_accel_class_init(ObjectClass *oc, void *data)
>  {
>      AccelClass *ac = ACCEL_CLASS(oc);
> @@ -78,6 +162,7 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
>  static const TypeInfo tcg_accel_type = {
>      .name = TYPE_TCG_ACCEL,
>      .parent = TYPE_ACCEL,
> +    .instance_init = tcg_accel_instance_init,
>      .class_init = tcg_accel_class_init,
>  };
>
> diff --git a/cpus.c b/cpus.c
> index 63bda15..b472378 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -166,78 +166,6 @@ typedef struct TimersState {
>  static TimersState timers_state;
>  bool mttcg_enabled;
>
> -/*
> - * We default to false if we know other options have been enabled
> - * which are currently incompatible with MTTCG. Otherwise when each
> - * guest (target) has been updated to support:
> - *   - atomic instructions
> - *   - memory ordering primitives (barriers)
> - * they can set the appropriate CONFIG flags in ${target}-softmmu.mak
> - *
> - * Once a guest architecture has been converted to the new primitives
> - * there are two remaining limitations to check.
> - *
> - * - The guest can't be oversized (e.g. 64 bit guest on 32 bit host)
> - * - The host must have a stronger memory order than the guest
> - *
> - * It may be possible in future to support strong guests on weak hosts
> - * but that will require tagging all load/stores in a guest with their
> - * implicit memory order requirements which would likely slow things
> - * down a lot.
> - */
> -
> -static bool check_tcg_memory_orders_compatible(void)
> -{
> -#if defined(TCG_GUEST_DEFAULT_MO) && defined(TCG_TARGET_DEFAULT_MO)
> -    return (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO) == 0;
> -#else
> -    return false;
> -#endif
> -}
> -
> -static bool default_mttcg_enabled(void)
> -{
> -    if (use_icount || TCG_OVERSIZED_GUEST) {
> -        return false;
> -    } else {
> -#ifdef TARGET_SUPPORTS_MTTCG
> -        return check_tcg_memory_orders_compatible();
> -#else
> -        return false;
> -#endif
> -    }
> -}
> -
> -void qemu_tcg_configure(QemuOpts *opts, Error **errp)
> -{
> -    const char *t = qemu_opt_get(opts, "thread");
> -    if (t) {
> -        if (strcmp(t, "multi") == 0) {
> -            if (TCG_OVERSIZED_GUEST) {
> -                error_setg(errp, "No MTTCG when guest word size > hosts");
> -            } else if (use_icount) {
> -                error_setg(errp, "No MTTCG when icount is enabled");
> -            } else {
> -#ifndef TARGET_SUPPORTS_MTTCG
> -                warn_report("Guest not yet converted to MTTCG - "
> -                            "you may get unexpected results");
> -#endif
> -                if (!check_tcg_memory_orders_compatible()) {
> -                    warn_report("Guest expects a stronger memory ordering "
> -                                "than the host provides");
> -                    error_printf("This may cause strange/hard to debug errors\n");
> -                }
> -                mttcg_enabled = true;
> -            }
> -        } else if (strcmp(t, "single") == 0) {
> -            mttcg_enabled = false;
> -        } else {
> -            error_setg(errp, "Invalid 'thread' setting %s", t);
> -        }
> -    } else {
> -        mttcg_enabled = default_mttcg_enabled();
> -    }
> -}
>
>  /* The current number of executed instructions is based on what we
>   * originally budgeted minus the current state of the decrementing
> --
> 1.8.3.1
>
>
>


-- 
Marc-André Lureau

Re: [PATCH v2 18/18] kvm: convert "-machine kernel_irqchip" to an accelerator property

[Philippe Mathieu-Daudé]

On 12/9/19 4:01 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   accel/kvm/kvm-all.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-----
>   hw/core/machine.c   | 61 -----------------------------------------------------
>   include/hw/boards.h |  3 ---
>   qemu-options.hx     |  9 +++++---
>   vl.c                |  3 ++-
>   5 files changed, 62 insertions(+), 73 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index c0a6351..b0adbb2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -42,6 +42,8 @@
>   #include "sysemu/sev.h"
>   #include "sysemu/balloon.h"
>   #include "qapi/visitor.h"
> +#include "qapi/qapi-types-common.h"
> +#include "qapi/qapi-visit-common.h"
>   
>   #include "hw/boards.h"
>   
> @@ -94,6 +96,9 @@ struct KVMState
>       int many_ioeventfds;
>       int intx_set_mask;
>       int kvm_shadow_mem;
> +    bool kernel_irqchip_allowed;
> +    bool kernel_irqchip_required;
> +    bool kernel_irqchip_split;
>       bool sync_mmu;
>       bool manual_dirty_log_protect;
>       /* The man page (and posix) say ioctl numbers are signed int, but
> @@ -1780,7 +1785,7 @@ static void kvm_irqchip_create(KVMState *s)
>        * in-kernel irqchip for us */
>       ret = kvm_arch_irqchip_create(s);
>       if (ret == 0) {
> -        if (kvm_kernel_irqchip_split()) {
> +        if (s->kernel_irqchip_split) {
>               perror("Split IRQ chip mode not supported.");
>               exit(1);
>           } else {
> @@ -2051,7 +2056,7 @@ static int kvm_init(MachineState *ms)
>           goto err;
>       }
>   
> -    if (machine_kernel_irqchip_allowed(ms)) {
> +    if (s->kernel_irqchip_allowed) {
>           kvm_irqchip_create(s);
>       }
>   
> @@ -2969,19 +2974,57 @@ static void kvm_set_kvm_shadow_mem(Object *obj, Visitor *v,
>       s->kvm_shadow_mem = value;
>   }
>   
> +static void kvm_set_kernel_irqchip(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +    Error *err = NULL;
> +    KVMState *s = KVM_STATE(obj);
> +    OnOffSplit mode;
> +
> +    visit_type_OnOffSplit(v, name, &mode, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    } else {
> +        switch (mode) {
> +        case ON_OFF_SPLIT_ON:
> +            s->kernel_irqchip_allowed = true;
> +            s->kernel_irqchip_required = true;
> +            s->kernel_irqchip_split = false;
> +            break;
> +        case ON_OFF_SPLIT_OFF:
> +            s->kernel_irqchip_allowed = false;
> +            s->kernel_irqchip_required = false;
> +            s->kernel_irqchip_split = false;
> +            break;
> +        case ON_OFF_SPLIT_SPLIT:
> +            s->kernel_irqchip_allowed = true;
> +            s->kernel_irqchip_required = true;
> +            s->kernel_irqchip_split = true;
> +            break;
> +        default:
> +            /* The value was checked in visit_type_OnOffSplit() above. If

With recent style enforcement you now need to split this comment in 2 lines.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +             * we get here, then something is wrong in QEMU.
> +             */
> +            abort();
> +        }
> +    }
> +}
> +
>   bool kvm_kernel_irqchip_allowed(void)
>   {
> -    return machine_kernel_irqchip_allowed(current_machine);
> +    return kvm_state->kernel_irqchip_allowed;
>   }
>   
>   bool kvm_kernel_irqchip_required(void)
>   {
> -    return machine_kernel_irqchip_required(current_machine);
> +    return kvm_state->kernel_irqchip_required;
>   }
>   
>   bool kvm_kernel_irqchip_split(void)
>   {
> -    return machine_kernel_irqchip_split(current_machine);
> +    return kvm_state->kernel_irqchip_split;
>   }
>   
>   static void kvm_accel_instance_init(Object *obj)
> @@ -2999,6 +3042,12 @@ static void kvm_accel_class_init(ObjectClass *oc, void *data)
>       ac->has_memory = kvm_accel_has_memory;
>       ac->allowed = &kvm_allowed;
>   
> +    object_class_property_add(oc, "kernel-irqchip", "on|off|split",
> +        NULL, kvm_set_kernel_irqchip,
> +        NULL, NULL, &error_abort);
> +    object_class_property_set_description(oc, "kernel-irqchip",
> +        "Configure KVM in-kernel irqchip", &error_abort);
> +
>       object_class_property_add(oc, "kvm-shadow-mem", "int",
>           kvm_get_kvm_shadow_mem, kvm_set_kvm_shadow_mem,
>           NULL, NULL, &error_abort);
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index bfb5f6f..c1cad2c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -173,44 +173,6 @@ GlobalProperty hw_compat_2_1[] = {
>   };
>   const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
>   
> -static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
> -                                       const char *name, void *opaque,
> -                                       Error **errp)
> -{
> -    Error *err = NULL;
> -    MachineState *ms = MACHINE(obj);
> -    OnOffSplit mode;
> -
> -    visit_type_OnOffSplit(v, name, &mode, &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    } else {
> -        switch (mode) {
> -        case ON_OFF_SPLIT_ON:
> -            ms->kernel_irqchip_allowed = true;
> -            ms->kernel_irqchip_required = true;
> -            ms->kernel_irqchip_split = false;
> -            break;
> -        case ON_OFF_SPLIT_OFF:
> -            ms->kernel_irqchip_allowed = false;
> -            ms->kernel_irqchip_required = false;
> -            ms->kernel_irqchip_split = false;
> -            break;
> -        case ON_OFF_SPLIT_SPLIT:
> -            ms->kernel_irqchip_allowed = true;
> -            ms->kernel_irqchip_required = true;
> -            ms->kernel_irqchip_split = true;
> -            break;
> -        default:
> -            /* The value was checked in visit_type_OnOffSplit() above. If
> -             * we get here, then something is wrong in QEMU.
> -             */
> -            abort();
> -        }
> -    }
> -}
> -
>   static char *machine_get_kernel(Object *obj, Error **errp)
>   {
>       MachineState *ms = MACHINE(obj);
> @@ -752,12 +714,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
>       mc->numa_mem_align_shift = 23;
>       mc->numa_auto_assign_ram = numa_default_auto_assign_ram;
>   
> -    object_class_property_add(oc, "kernel-irqchip", "on|off|split",
> -        NULL, machine_set_kernel_irqchip,
> -        NULL, NULL, &error_abort);
> -    object_class_property_set_description(oc, "kernel-irqchip",
> -        "Configure KVM in-kernel irqchip", &error_abort);
> -
>       object_class_property_add_str(oc, "kernel",
>           machine_get_kernel, machine_set_kernel, &error_abort);
>       object_class_property_set_description(oc, "kernel",
> @@ -857,8 +813,6 @@ static void machine_initfn(Object *obj)
>       MachineState *ms = MACHINE(obj);
>       MachineClass *mc = MACHINE_GET_CLASS(obj);
>   
> -    ms->kernel_irqchip_allowed = true;
> -    ms->kernel_irqchip_split = mc->default_kernel_irqchip_split;
>       ms->dump_guest_core = true;
>       ms->mem_merge = true;
>       ms->enable_graphics = true;
> @@ -914,21 +868,6 @@ bool machine_usb(MachineState *machine)
>       return machine->usb;
>   }
>   
> -bool machine_kernel_irqchip_allowed(MachineState *machine)
> -{
> -    return machine->kernel_irqchip_allowed;
> -}
> -
> -bool machine_kernel_irqchip_required(MachineState *machine)
> -{
> -    return machine->kernel_irqchip_required;
> -}
> -
> -bool machine_kernel_irqchip_split(MachineState *machine)
> -{
> -    return machine->kernel_irqchip_split;
> -}
> -
>   int machine_phandle_start(MachineState *machine)
>   {
>       return machine->phandle_start;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 46119eb..b42fedd 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -63,9 +63,6 @@ extern MachineState *current_machine;
>   
>   void machine_run_board_init(MachineState *machine);
>   bool machine_usb(MachineState *machine);
> -bool machine_kernel_irqchip_allowed(MachineState *machine);
> -bool machine_kernel_irqchip_required(MachineState *machine);
> -bool machine_kernel_irqchip_split(MachineState *machine);
>   int machine_phandle_start(MachineState *machine);
>   bool machine_dump_guest_core(MachineState *machine);
>   bool machine_mem_merge(MachineState *machine);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 779c8af..945fc4a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -32,7 +32,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>       "                selects emulated machine ('-machine help' for list)\n"
>       "                property accel=accel1[:accel2[:...]] selects accelerator\n"
>       "                supported accelerators are kvm, xen, hax, hvf, whpx or tcg (default: tcg)\n"
> -    "                kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n"
>       "                vmport=on|off|auto controls emulation of vmport (default: auto)\n"
>       "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>       "                mem-merge=on|off controls memory merge support (default: on)\n"
> @@ -67,8 +66,6 @@ This is used to enable an accelerator. Depending on the target architecture,
>   kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If there is
>   more than one accelerator specified, the next one is used if the previous one
>   fails to initialize.
> -@item kernel_irqchip=on|off
> -Controls in-kernel irqchip support for the chosen accelerator when available.
>   @item vmport=on|off|auto
>   Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
>   value based on accel. For accel=xen the default is off otherwise the default
> @@ -115,6 +112,7 @@ DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>       "-accel [accel=]accelerator[,prop[=value][,...]]\n"
>       "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
>       "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"
> +    "                kernel-irqchip=on|off|split controls accelerated irqchip support (default=on)\n"
>       "                kvm-shadow-mem=size of KVM shadow MMU in bytes\n"
>       "                tb-size=n (TCG translation block cache size)\n"
>       "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
> @@ -129,6 +127,11 @@ fails to initialize.
>   @item igd-passthru=on|off
>   When Xen is in use, this option controls whether Intel integrated graphics
>   devices can be passed through to the guest (default=off)
> +@item kernel-irqchip=on|off|split
> +Controls KVM in-kernel irqchip support.  The default is full acceleration of the
> +interrupt controllers.  On x86, split irqchip reduces the kernel attack
> +surface, at a performance cost for non-MSI interrupts.  Disabling the in-kernel
> +irqchip completely is not recommended except for debugging purposes.
>   @item kvm-shadow-mem=size
>   Defines the size of the KVM shadow MMU.
>   @item tb-size=@var{n}
> diff --git a/vl.c b/vl.c
> index 3678522..53e4ff6 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2639,7 +2639,8 @@ static int machine_set_property(void *opaque,
>           object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
>           return 0;
>       }
> -    if (g_str_equal(qom_name, "kvm-shadow-mem")) {
> +    if (g_str_equal(qom_name, "kvm-shadow-mem") ||
> +        g_str_equal(qom_name, "kernel-irqchip")) {
>           object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value);
>           return 0;
>       }
> 

Re: [PATCH v2 10/18] qom: introduce object_register_sugar_prop

[Marc-André Lureau]

Hi

On Mon, Dec 9, 2019 at 7:10 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Similar to the existing "-rtc driftfix" option, we will convert some
> legacy "-machine" command line options to global properties on accelerators.
> Because accelerators are not devices, we cannot use qdev_prop_register_global.
> Instead, provide a slot in the generic object_compat_props arrays for
> command line syntactic sugar.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qom/object.h |  1 +
>  qom/object.c         | 23 +++++++++++++++++++++--
>  vl.c                 | 10 +++-------
>  3 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 128d00c..230b18f 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -679,6 +679,7 @@ void object_apply_global_props(Object *obj, const GPtrArray *props,
>                                 Error **errp);
>  void object_set_machine_compat_props(GPtrArray *compat_props);
>  void object_set_accelerator_compat_props(GPtrArray *compat_props);
> +void object_register_sugar_prop(const char *driver, const char *prop, const char *value);
>  void object_apply_compat_props(Object *obj);
>
>  /**
> diff --git a/qom/object.c b/qom/object.c
> index d51b57f..bfb4413 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -414,10 +414,29 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp
>   * Global property defaults
>   * Slot 0: accelerator's global property defaults
>   * Slot 1: machine's global property defaults
> + * Slot 2: global properties from legacy command line option
>   * Each is a GPtrArray of of GlobalProperty.
>   * Applied in order, later entries override earlier ones.
>   */
> -static GPtrArray *object_compat_props[2];
> +static GPtrArray *object_compat_props[3];
> +
> +/*
> + * Retrieve @GPtrArray for global property defined with options
> + * other than "-global".  These are generally used for syntactic
> + * sugar and legacy command line options.
> + */
> +void object_register_sugar_prop(const char *driver, const char *prop, const char *value)
> +{
> +    GlobalProperty *g;
> +    if (!object_compat_props[2]) {
> +        object_compat_props[2] = g_ptr_array_new();
> +    }
> +    g = g_new(GlobalProperty, 1);
> +    g->driver = g_strdup(driver);
> +    g->property = g_strdup(prop);
> +    g->value = g_strdup(value);
> +    g_ptr_array_add(object_compat_props[2], g);
> +}
>
>  /*
>   * Set machine's global property defaults to @compat_props.
> @@ -445,7 +464,7 @@ void object_apply_compat_props(Object *obj)
>
>      for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
>          object_apply_global_props(obj, object_compat_props[i],
> -                                  &error_abort);
> +                                  i == 2 ? &error_fatal : &error_abort);
>      }
>  }
>
> diff --git a/vl.c b/vl.c
> index 58aad4f..d6c77bc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -897,13 +897,9 @@ static void configure_rtc(QemuOpts *opts)
>      value = qemu_opt_get(opts, "driftfix");
>      if (value) {
>          if (!strcmp(value, "slew")) {
> -            static GlobalProperty slew_lost_ticks = {
> -                .driver   = "mc146818rtc",
> -                .property = "lost_tick_policy",
> -                .value    = "slew",
> -            };
> -
> -            qdev_prop_register_global(&slew_lost_ticks);
> +            object_register_sugar_prop("mc146818rtc",
> +                                       "lost_tick_policy",
> +                                       "slew");
>          } else if (!strcmp(value, "none")) {
>              /* discard is default */
>          } else {
> --
> 1.8.3.1
>
>
>


I am not too happy about the change, but the "global properties"
handling is an area that can be improved later.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


-- 
Marc-André Lureau

Re: [PATCH v2 14/18] tcg: add "-accel tcg, tb-size" and deprecate "-tb-size"

[Marc-André Lureau]

Hi

On Mon, Dec 9, 2019 at 7:17 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> -tb-size fits nicely in the new framework for accelerator-specific options.  It
> is a very niche option, so insta-deprecate it.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/tcg/tcg-all.c    | 40 +++++++++++++++++++++++++++++++++++++---
>  include/sysemu/accel.h |  2 --
>  qemu-deprecated.texi   |  6 ++++++
>  qemu-options.hx        |  8 ++++++--
>  vl.c                   |  8 ++++----
>  5 files changed, 53 insertions(+), 11 deletions(-)
>
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 7829f02..1dc384c 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -34,11 +34,13 @@
>  #include "include/qapi/error.h"
>  #include "include/qemu/error-report.h"
>  #include "include/hw/boards.h"
> +#include "qapi/qapi-builtin-visit.h"
>
>  typedef struct TCGState {
>      AccelState parent_obj;
>
>      bool mttcg_enabled;
> +    unsigned long tb_size;
>  } TCGState;
>
>  #define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg")
> @@ -46,8 +48,6 @@ typedef struct TCGState {
>  #define TCG_STATE(obj) \
>          OBJECT_CHECK(TCGState, (obj), TYPE_TCG_ACCEL)
>
> -unsigned long tcg_tb_size;
> -
>  /* mask must never be zero, except for A20 change call */
>  static void tcg_handle_interrupt(CPUState *cpu, int mask)
>  {
> @@ -126,7 +126,7 @@ static int tcg_init(MachineState *ms)
>  {
>      TCGState *s = TCG_STATE(current_machine->accelerator);
>
> -    tcg_exec_init(tcg_tb_size * 1024 * 1024);
> +    tcg_exec_init(s->tb_size * 1024 * 1024);
>      cpu_interrupt_handler = tcg_handle_interrupt;
>      mttcg_enabled = s->mttcg_enabled;
>      return 0;
> @@ -167,6 +167,33 @@ static void tcg_set_thread(Object *obj, const char *value, Error **errp)
>      }
>  }
>
> +static void tcg_get_tb_size(Object *obj, Visitor *v,
> +                            const char *name, void *opaque,
> +                            Error **errp)
> +{
> +    TCGState *s = TCG_STATE(obj);
> +    uint32_t value = s->tb_size;
> +
> +    visit_type_uint32(v, name, &value, errp);
> +}
> +
> +static void tcg_set_tb_size(Object *obj, Visitor *v,
> +                            const char *name, void *opaque,
> +                            Error **errp)
> +{
> +    TCGState *s = TCG_STATE(obj);
> +    Error *error = NULL;
> +    uint32_t value;
> +
> +    visit_type_uint32(v, name, &value, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    s->tb_size = value;
> +}
> +
>  static void tcg_accel_class_init(ObjectClass *oc, void *data)
>  {
>      AccelClass *ac = ACCEL_CLASS(oc);
> @@ -178,6 +205,13 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
>                                    tcg_get_thread,
>                                    tcg_set_thread,
>                                    NULL);
> +
> +    object_class_property_add(oc, "tb-size", "int",
> +        tcg_get_tb_size, tcg_set_tb_size,
> +        NULL, NULL, &error_abort);

After Felipe properties series, we should switch this to
object_class_property_add_uint32_ptr(). Would you mind adding a FIXME
if you agree?


> +    object_class_property_set_description(oc, "tb-size",
> +        "TCG translation block cache size", &error_abort);
> +
>  }
>
>  static const TypeInfo tcg_accel_type = {
> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
> index 22cac0f..d4c1429 100644
> --- a/include/sysemu/accel.h
> +++ b/include/sysemu/accel.h
> @@ -64,8 +64,6 @@ typedef struct AccelClass {
>  #define ACCEL_GET_CLASS(obj) \
>      OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
>
> -extern unsigned long tcg_tb_size;
> -
>  AccelClass *accel_find(const char *opt_name);
>  int accel_init_machine(AccelState *accel, MachineState *ms);
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 4b4b742..487d2b4 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -147,6 +147,12 @@ QEMU 4.1 has three options, please migrate to one of these three:
>        to do is specify the kernel they want to boot with the -kernel option
>   3. ``-bios <file>`` - Tells QEMU to load the specified file as the firmwrae.
>
> +@subsection -tb-size option (since 5.0)
> +
> +QEMU 5.0 introduced an alternative syntax to specify the size of the translation
> +block cache, @option{-accel tcg,tb-size=}.  The new syntax deprecates the
> +previously available @option{-tb-size} option.
> +
>  @section QEMU Machine Protocol (QMP) commands
>
>  @subsection change (since 2.5.0)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 65c9473..9775258 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -118,8 +118,9 @@ Select CPU model (@code{-cpu help} for list and additional feature selection)
>  ETEXI
>
>  DEF("accel", HAS_ARG, QEMU_OPTION_accel,
> -    "-accel [accel=]accelerator[,thread=single|multi]\n"
> +    "-accel [accel=]accelerator[,prop[=value][,...]]\n"
>      "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
> +    "                tb-size=n (TCG translation block cache size)\n"
>      "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
>  STEXI
>  @item -accel @var{name}[,prop=@var{value}[,...]]
> @@ -129,6 +130,8 @@ kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If th
>  more than one accelerator specified, the next one is used if the previous one
>  fails to initialize.
>  @table @option
> +@item tb-size=@var{n}
> +Controls the size (in MiB) of the TCG translation block cache.
>  @item thread=single|multi
>  Controls number of TCG threads. When the TCG is multi-threaded there will be one
>  thread per vCPU therefor taking advantage of additional host cores. The default
> @@ -4012,7 +4015,8 @@ DEF("tb-size", HAS_ARG, QEMU_OPTION_tb_size, \
>  STEXI
>  @item -tb-size @var{n}
>  @findex -tb-size
> -Set TB size.
> +Set TCG translation block cache size.  Deprecated, use @samp{-accel tcg,tb-size=@var{n}}
> +instead.
>  ETEXI
>
>  DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
> diff --git a/vl.c b/vl.c
> index b6c23d1..e6ff56b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2852,6 +2852,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>          return 0;
>      }
>      accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
> +    object_apply_compat_props(OBJECT(accel));
>      qemu_opt_foreach(opts, accelerator_set_property,
>                       accel,
>                       &error_fatal);
> @@ -2863,6 +2864,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>                       acc, strerror(-ret));
>          return 0;
>      }
> +
>      return 1;
>  }
>
> @@ -3745,10 +3747,8 @@ int main(int argc, char **argv, char **envp)
>                  error_report("TCG is disabled");
>                  exit(1);
>  #endif
> -                if (qemu_strtoul(optarg, NULL, 0, &tcg_tb_size) < 0) {
> -                    error_report("Invalid argument to -tb-size");
> -                    exit(1);
> -                }
> +                warn_report("The -tb-size option is deprecated, use -accel tcg,tb-size instead");
> +                object_register_sugar_prop(ACCEL_CLASS_NAME("tcg"), "tb-size", optarg);
>                  break;
>              case QEMU_OPTION_icount:
>                  icount_opts = qemu_opts_parse_noisily(qemu_find_opts("icount"),
> --
> 1.8.3.1
>
>
>

otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



-- 
Marc-André Lureau

Re: [PATCH v2 15/18] xen: convert "-machine igd-passthru" to an accelerator property

[Marc-André Lureau]

Hi

On Mon, Dec 9, 2019 at 7:12 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The first machine property to fall is Xen's Intel integrated graphics
> passthrough.  The "-machine igd-passthru" option does not set anymore
> a property on the machine object, but desugars to a GlobalProperty on
> accelerator objects.
>
> The setter is very simple, since the value ends up in a
> global variable, so this patch also provides an example before the more
> complicated cases that follow it.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/machine.c   | 20 --------------------
>  hw/xen/xen-common.c | 16 ++++++++++++++++
>  include/hw/boards.h |  1 -
>  qemu-options.hx     |  7 ++++---
>  vl.c                | 14 ++++----------
>  5 files changed, 24 insertions(+), 34 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 45ddfb6..d7a0356 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -412,20 +412,6 @@ static void machine_set_graphics(Object *obj, bool value, Error **errp)
>      ms->enable_graphics = value;
>  }
>
> -static bool machine_get_igd_gfx_passthru(Object *obj, Error **errp)
> -{
> -    MachineState *ms = MACHINE(obj);
> -
> -    return ms->igd_gfx_passthru;
> -}
> -
> -static void machine_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
> -{
> -    MachineState *ms = MACHINE(obj);
> -
> -    ms->igd_gfx_passthru = value;
> -}
> -
>  static char *machine_get_firmware(Object *obj, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -862,12 +848,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
>      object_class_property_set_description(oc, "graphics",
>          "Set on/off to enable/disable graphics emulation", &error_abort);
>
> -    object_class_property_add_bool(oc, "igd-passthru",
> -        machine_get_igd_gfx_passthru, machine_set_igd_gfx_passthru,
> -        &error_abort);
> -    object_class_property_set_description(oc, "igd-passthru",
> -        "Set on/off to enable/disable igd passthrou", &error_abort);
> -
>      object_class_property_add_str(oc, "firmware",
>          machine_get_firmware, machine_set_firmware,
>          &error_abort);
> diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
> index 5284b0d..6cba30c 100644
> --- a/hw/xen/xen-common.c
> +++ b/hw/xen/xen-common.c
> @@ -124,6 +124,16 @@ static void xen_change_state_handler(void *opaque, int running,
>      }
>  }
>
> +static bool xen_get_igd_gfx_passthru(Object *obj, Error **errp)
> +{
> +    return has_igd_gfx_passthru;
> +}
> +
> +static void xen_set_igd_gfx_passthru(Object *obj, bool value, Error **errp)
> +{
> +    has_igd_gfx_passthru = value;
> +}
> +
>  static void xen_setup_post(MachineState *ms, AccelState *accel)
>  {
>      int rc;
> @@ -177,6 +187,12 @@ static void xen_accel_class_init(ObjectClass *oc, void *data)
>      ac->compat_props = g_ptr_array_new();
>
>      compat_props_add(ac->compat_props, compat, G_N_ELEMENTS(compat));
> +
> +    object_class_property_add_bool(oc, "igd-passthru",
> +        xen_get_igd_gfx_passthru, xen_set_igd_gfx_passthru,
> +        &error_abort);
> +    object_class_property_set_description(oc, "igd-passthru",
> +        "Set on/off to enable/disable igd passthrou", &error_abort);
>  }
>
>  #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 36fcbda..cdcf481 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -287,7 +287,6 @@ struct MachineState {
>      bool mem_merge;
>      bool usb;
>      bool usb_disabled;
> -    bool igd_gfx_passthru;
>      char *firmware;
>      bool iommu;
>      bool suppress_vmdesc;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9775258..6f12b31 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -37,7 +37,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                kvm_shadow_mem=size of KVM shadow MMU in bytes\n"
>      "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>      "                mem-merge=on|off controls memory merge support (default: on)\n"
> -    "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
> @@ -71,8 +70,6 @@ more than one accelerator specified, the next one is used if the previous one
>  fails to initialize.
>  @item kernel_irqchip=on|off
>  Controls in-kernel irqchip support for the chosen accelerator when available.
> -@item gfx_passthru=on|off
> -Enables IGD GFX passthrough support for the chosen machine when available.
>  @item vmport=on|off|auto
>  Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the
>  value based on accel. For accel=xen the default is off otherwise the default
> @@ -120,6 +117,7 @@ ETEXI
>  DEF("accel", HAS_ARG, QEMU_OPTION_accel,
>      "-accel [accel=]accelerator[,prop[=value][,...]]\n"
>      "                select accelerator (kvm, xen, hax, hvf, whpx or tcg; use 'help' for a list)\n"
> +    "                igd-passthru=on|off (enable Xen integrated Intel graphics passthrough, default=off)\n"
>      "                tb-size=n (TCG translation block cache size)\n"
>      "                thread=single|multi (enable multi-threaded TCG)\n", QEMU_ARCH_ALL)
>  STEXI
> @@ -130,6 +128,9 @@ kvm, xen, hax, hvf, whpx or tcg can be available. By default, tcg is used. If th
>  more than one accelerator specified, the next one is used if the previous one
>  fails to initialize.
>  @table @option
> +@item igd-passthru=on|off
> +When Xen is in use, this option controls whether Intel integrated graphics
> +devices can be passed through to the guest (default=off)
>  @item tb-size=@var{n}
>  Controls the size (in MiB) of the TCG translation block cache.
>  @item thread=single|multi
> diff --git a/vl.c b/vl.c
> index e6ff56b..ee872f2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1257,13 +1257,6 @@ static void configure_msg(QemuOpts *opts)
>  }
>
>
> -/* Now we still need this for compatibility with XEN. */
> -bool has_igd_gfx_passthru;
> -static void igd_gfx_passthru(void)
> -{
> -    has_igd_gfx_passthru = current_machine->igd_gfx_passthru;
> -}
> -
>  /***********************************************************/
>  /* USB devices */
>
> @@ -2642,6 +2635,10 @@ static int machine_set_property(void *opaque,
>      if (g_str_equal(qom_name, "accel")) {
>          return 0;
>      }
> +    if (g_str_equal(qom_name, "igd-passthru")) {
> +        object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);

shouldn't it warn about deprecation?

> +        return 0;
> +    }
>
>      return object_parse_property_opt(opaque, name, value, "type", errp);
>  }
> @@ -4456,9 +4453,6 @@ int main(int argc, char **argv, char **envp)
>              exit(1);
>      }
>
> -    /* Check if IGD GFX passthrough. */
> -    igd_gfx_passthru();
> -
>      /* init generic devices */
>      rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
>      qemu_opts_foreach(qemu_find_opts("device"),
> --
> 1.8.3.1
>
>
>


-- 
Marc-André Lureau

Re: [PATCH v2 15/18] xen: convert "-machine igd-passthru" to an accelerator property

[Paolo Bonzini]

On 10/12/19 13:56, Marc-André Lureau wrote:
>> +    if (g_str_equal(qom_name, "igd-passthru")) {
>> +        object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value);
> 
> shouldn't it warn about deprecation?

The old version is not deprecated (I'm only deprecating -tb-size because
it's a toplevel option, but not the -machine versions; they have been
there forever and probably have too many users).

Paolo

Re: [PATCH v2 01/18] memory: do not look at current_machine->accel

[Richard Henderson]

On 12/9/19 7:01 AM, Paolo Bonzini wrote:
> "info mtree -f" prints the wrong accelerator name if used with for example
> "-machine accel=kvm:tcg".  The right thing to do is to fetch the name
> from the AccelClass, which will also work nicely once
> current_machine->accel stops existing.
> 
> Tested-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  memory.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 02/18] vl: move icount configuration earlier

[Richard Henderson]

On 12/9/19 7:01 AM, Paolo Bonzini wrote:
> Once qemu_tcg_configure is turned into a QOM property setter, it will not
> be able to set a default value for mttcg_enabled.  Setting the default will
> move to the TCG instance_init function, which currently runs before "-icount"
> is processed.
> 
> However, it is harmless to do configure_icount for all accelerators; we will
> just fail later if a non-TCG accelerator is selected.  So do that.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  vl.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 03/18] tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c

[Richard Henderson]

On 12/9/19 7:01 AM, Paolo Bonzini wrote:
> Move everything related to mttcg_enabled in accel/tcg/tcg-all.c,
> which will make even more sense when "thread" becomes a QOM property.
> 
> For now, initializing mttcg_enabled in the instance_init function
> prepares for the next patch, which will only invoke qemu_tcg_configure
> when the command line includes a -accel option.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/tcg/tcg-all.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  cpus.c              | 72 ---------------------------------------------
>  2 files changed, 85 insertions(+), 72 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~