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

Message ID 1575903705-12925-15-git-send-email-pbonzini@redhat.com
State New
Headers show
Series
  • Complete the implementation of -accel
Related show

Commit Message

Paolo Bonzini Dec. 9, 2019, 3:01 p.m. UTC
-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(-)

Comments

Philippe Mathieu-Daudé Dec. 9, 2019, 3:52 p.m. UTC | #1
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
[...]
Paolo Bonzini Dec. 9, 2019, 5:18 p.m. UTC | #2
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
Marc-André Lureau Dec. 10, 2019, 12:53 p.m. UTC | #3
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>

Patch
diff mbox series

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"),