diff mbox series

[v3,01/10] make one-insn-per-tb an accel option

Message ID 20230417164041.684562-2-peter.maydell@linaro.org
State New
Headers show
Series Deprecate/rename singlestep command line option, monitor interfaces | expand

Commit Message

Peter Maydell April 17, 2023, 4:40 p.m. UTC 
This commit adds 'one-insn-per-tb' as a property on the TCG
accelerator object, so you can enable it with
   -accel tcg,one-insn-per-tb=on

It has the same behaviour as the existing '-singlestep' command line
option.  We use a different name because 'singlestep' has always been
a confusing choice, because it doesn't have anything to do with
single-stepping the CPU.  What it does do is force TCG emulation to
put one guest instruction in each TB, which can be useful in some
situations (such as analysing debug logs).

The existing '-singlestep' commandline options are decoupled from the
global 'singlestep' variable and instead now are syntactic sugar for
setting the accel property.  (These can then go away after a
deprecation period.)

The global variable remains for the moment as:
 * what the TCG code looks at to change its behaviour
 * what HMP and QMP use to query and set the behaviour

In the following commits we'll clean those up to not directly
look at the global variable.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tcg-all.c | 21 +++++++++++++++++++++
 bsd-user/main.c     |  8 ++++++--
 linux-user/main.c   |  8 ++++++--
 softmmu/vl.c        | 17 +++++++++++++++--
 qemu-options.hx     |  7 +++++++
 5 files changed, 55 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 5dab1ae9dd3..fcf361c8db6 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -42,6 +42,7 @@  struct TCGState {
     AccelState parent_obj;
 
     bool mttcg_enabled;
+    bool one_insn_per_tb;
     int splitwx_enabled;
     unsigned long tb_size;
 };
@@ -208,6 +209,20 @@  static void tcg_set_splitwx(Object *obj, bool value, Error **errp)
     s->splitwx_enabled = value;
 }
 
+static bool tcg_get_one_insn_per_tb(Object *obj, Error **errp)
+{
+    TCGState *s = TCG_STATE(obj);
+    return s->one_insn_per_tb;
+}
+
+static void tcg_set_one_insn_per_tb(Object *obj, bool value, Error **errp)
+{
+    TCGState *s = TCG_STATE(obj);
+    s->one_insn_per_tb = value;
+    /* For the moment, set the global also: this changes the behaviour */
+    singlestep = value;
+}
+
 static int tcg_gdbstub_supported_sstep_flags(void)
 {
     /*
@@ -245,6 +260,12 @@  static void tcg_accel_class_init(ObjectClass *oc, void *data)
         tcg_get_splitwx, tcg_set_splitwx);
     object_class_property_set_description(oc, "split-wx",
         "Map jit pages into separate RW and RX regions");
+
+    object_class_property_add_bool(oc, "one-insn-per-tb",
+                                   tcg_get_one_insn_per_tb,
+                                   tcg_set_one_insn_per_tb);
+    object_class_property_set_description(oc, "one-insn-per-tb",
+        "Only put one guest insn in each translation block");
 }
 
 static const TypeInfo tcg_accel_type = {
diff --git a/bsd-user/main.c b/bsd-user/main.c
index babc3b009b6..09b84da190c 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -50,6 +50,7 @@ 
 #include "target_arch_cpu.h"
 
 int singlestep;
+static bool opt_one_insn_per_tb;
 uintptr_t guest_base;
 bool have_guest_base;
 /*
@@ -386,7 +387,7 @@  int main(int argc, char **argv)
         } else if (!strcmp(r, "seed")) {
             seed_optarg = optarg;
         } else if (!strcmp(r, "singlestep")) {
-            singlestep = 1;
+            opt_one_insn_per_tb = true;
         } else if (!strcmp(r, "strace")) {
             do_strace = 1;
         } else if (!strcmp(r, "trace")) {
@@ -444,9 +445,12 @@  int main(int argc, char **argv)
 
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     {
-        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
+        AccelState *accel = current_accel();
+        AccelClass *ac = ACCEL_GET_CLASS(accel);
 
         accel_init_interfaces(ac);
+        object_property_set_bool(OBJECT(accel), "one-insn-per-tb",
+                                 opt_one_insn_per_tb, &error_abort);
         ac->init_machine(NULL);
     }
     cpu = cpu_create(cpu_type);
diff --git a/linux-user/main.c b/linux-user/main.c
index fe03293516a..489694ad654 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -69,6 +69,7 @@  char *exec_path;
 char real_exec_path[PATH_MAX];
 
 int singlestep;
+static bool opt_one_insn_per_tb;
 static const char *argv0;
 static const char *gdbstub;
 static envlist_t *envlist;
@@ -411,7 +412,7 @@  static void handle_arg_reserved_va(const char *arg)
 
 static void handle_arg_singlestep(const char *arg)
 {
-    singlestep = 1;
+    opt_one_insn_per_tb = true;
 }
 
 static void handle_arg_strace(const char *arg)
@@ -777,9 +778,12 @@  int main(int argc, char **argv, char **envp)
 
     /* init tcg before creating CPUs and to get qemu_host_page_size */
     {
-        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
+        AccelState *accel = current_accel();
+        AccelClass *ac = ACCEL_GET_CLASS(accel);
 
         accel_init_interfaces(ac);
+        object_property_set_bool(OBJECT(accel), "one-insn-per-tb",
+                                 opt_one_insn_per_tb, &error_abort);
         ac->init_machine(NULL);
     }
     cpu = cpu_create(cpu_type);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ea20b23e4c8..492b5fe65e6 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -182,6 +182,7 @@  static const char *log_file;
 static bool list_data_dirs;
 static const char *qtest_chrdev;
 static const char *qtest_log;
+static bool opt_one_insn_per_tb;
 
 static int has_defaults = 1;
 static int default_serial = 1;
@@ -2220,7 +2221,19 @@  static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
     qemu_opt_foreach(opts, accelerator_set_property,
                      accel,
                      &error_fatal);
-
+    /*
+     * If legacy -singlestep option is set, honour it for TCG and
+     * silently ignore for any other accelerator (which is how this
+     * option has always behaved).
+     */
+    if (opt_one_insn_per_tb) {
+        /*
+         * This will always succeed for TCG, and we want to ignore
+         * the error from trying to set a nonexistent property
+         * on any other accelerator.
+         */
+        object_property_set_bool(OBJECT(accel), "one-insn-per-tb", true, NULL);
+    }
     ret = accel_init_machine(accel, current_machine);
     if (ret < 0) {
         if (!qtest_with_kvm || ret != -ENOENT) {
@@ -2955,7 +2968,7 @@  void qemu_init(int argc, char **argv)
                 qdict_put_str(machine_opts_dict, "firmware", optarg);
                 break;
             case QEMU_OPTION_singlestep:
-                singlestep = 1;
+                opt_one_insn_per_tb = true;
                 break;
             case QEMU_OPTION_S:
                 autostart = 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 59bdf67a2c5..1dffd36884e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -182,6 +182,7 @@  DEF("accel", HAS_ARG, QEMU_OPTION_accel,
     "                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"
+    "                one-insn-per-tb=on|off (one guest instruction per TCG translation block)\n"
     "                split-wx=on|off (enable TCG split w^x mapping)\n"
     "                tb-size=n (TCG translation block cache size)\n"
     "                dirty-ring-size=n (KVM dirty ring GFN count, default 0)\n"
@@ -210,6 +211,12 @@  SRST
     ``kvm-shadow-mem=size``
         Defines the size of the KVM shadow MMU.
 
+    ``one-insn-per-tb=on|off``
+        Makes the TCG accelerator put only one guest instruction into
+        each translation block. This slows down emulation a lot, but
+        can be useful in some situations, such as when trying to analyse
+        the logs produced by the ``-d`` option.
+
     ``split-wx=on|off``
         Controls the use of split w^x mapping for the TCG code generation
         buffer. Some operating systems require this to be enabled, and in