From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>, QEMU <qemu-devel@nongnu.org>,
elmarco@redhat.com
Subject: Re: [PATCH v2 14/18] tcg: add "-accel tcg, tb-size" and deprecate "-tb-size"
Date: Tue, 10 Dec 2019 16:53:36 +0400 [thread overview]
Message-ID: <CAJ+F1CK2CPwjt39i9Jd+Uf9h9tRjU3dyf7B1dnwHAy0MFhVYnA@mail.gmail.com> (raw)
In-Reply-To: <1575903705-12925-15-git-send-email-pbonzini@redhat.com>
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
next prev parent reply other threads:[~2019-12-10 12:54 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-09 15:01 [PATCH v2 00/18] Complete the implementation of -accel Paolo Bonzini
2019-12-09 15:01 ` [PATCH v2 01/18] memory: do not look at current_machine->accel Paolo Bonzini
2019-12-12 4:39 ` Richard Henderson
2019-12-09 15:01 ` [PATCH v2 02/18] vl: move icount configuration earlier Paolo Bonzini
2019-12-12 4:41 ` Richard Henderson
2019-12-09 15:01 ` [PATCH v2 03/18] tcg: move qemu_tcg_configure to accel/tcg/tcg-all.c Paolo Bonzini
2019-12-10 11:55 ` Philippe Mathieu-Daudé
2019-12-10 12:05 ` Marc-André Lureau
2019-12-12 4:47 ` Richard Henderson
2019-12-09 15:01 ` [PATCH v2 04/18] vl: extract accelerator option processing to a separate function Paolo Bonzini
2019-12-10 11:56 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 05/18] vl: merge -accel processing into configure_accelerators Paolo Bonzini
2019-12-10 11:57 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 06/18] accel: compile accel/accel.c just once Paolo Bonzini
2019-12-10 11:58 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 07/18] vl: introduce object_parse_property_opt Paolo Bonzini
2019-12-10 11:53 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 08/18] vl: configure accelerators from -accel options Paolo Bonzini
2019-12-09 15:01 ` [PATCH v2 09/18] vl: warn for unavailable accelerators, clarify messages Paolo Bonzini
2019-12-09 15:01 ` [PATCH v2 10/18] qom: introduce object_register_sugar_prop Paolo Bonzini
2019-12-10 12:36 ` Marc-André Lureau
2019-12-09 15:01 ` [PATCH v2 11/18] qom: add object_new_with_class Paolo Bonzini
2019-12-09 15:31 ` Philippe Mathieu-Daudé
2019-12-09 17:14 ` Paolo Bonzini
2019-12-09 15:01 ` [PATCH v2 12/18] accel: pass object to accel_init_machine Paolo Bonzini
2019-12-09 15:01 ` [PATCH v2 13/18] tcg: convert "-accel threads" to a QOM property Paolo Bonzini
2019-12-09 15:28 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 14/18] tcg: add "-accel tcg, tb-size" and deprecate "-tb-size" Paolo Bonzini
2019-12-09 15:52 ` Philippe Mathieu-Daudé
2019-12-09 17:18 ` Paolo Bonzini
2019-12-10 12:53 ` Marc-André Lureau [this message]
2019-12-09 15:01 ` [PATCH v2 15/18] xen: convert "-machine igd-passthru" to an accelerator property Paolo Bonzini
2019-12-09 15:48 ` Philippe Mathieu-Daudé
2019-12-10 12:56 ` Marc-André Lureau
2019-12-10 17:44 ` Paolo Bonzini
2019-12-09 15:01 ` [PATCH v2 16/18] kvm: convert "-machine kvm_shadow_mem" " Paolo Bonzini
2019-12-10 12:01 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 17/18] kvm: introduce kvm_kernel_irqchip_* functions Paolo Bonzini
2019-12-10 12:02 ` Philippe Mathieu-Daudé
2019-12-09 15:01 ` [PATCH v2 18/18] kvm: convert "-machine kernel_irqchip" to an accelerator property Paolo Bonzini
2019-12-10 12:05 ` Philippe Mathieu-Daudé
2019-12-09 22:42 ` [PATCH v2 00/18] Complete the implementation of -accel no-reply
2019-12-09 22:58 ` no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJ+F1CK2CPwjt39i9Jd+Uf9h9tRjU3dyf7B1dnwHAy0MFhVYnA@mail.gmail.com \
--to=marcandre.lureau@gmail.com \
--cc=elmarco@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).