Re: [Qemu-devel] [PATCH v10 05/23] tcg: add options for enabling MTTCG

[Pranith Kumar]

Hi Alex,

I just spotted this:

On Mon, Feb 6, 2017 at 10:30 AM, Alex Bennée <address@hidden> wrote:

> From: KONRAD Frederic <address@hidden>
>
> We know there will be cases where MTTCG won't work until additional work
> is done in the front/back ends to support. It will however be useful to
> be able to turn it on.
>
> As a result MTTCG will default to off unless the combination is
> supported. However the user can turn it on for the sake of testing.
>
> Signed-off-by: KONRAD Frederic <address@hidden>
> [AJB: move to -accel tcg,thread=multi|single, defaults]
> Signed-off-by: Alex Bennée <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
> ---
> v1:
>   - merge with add mttcg option.
>   - update commit message
> v2:
>   - machine_init->opts_init
> v3:
>   - moved from -tcg to -accel tcg,thread=single|multi
>   - fix checkpatch warnings
> v4:
>   - make mttcg_enabled extern, qemu_tcg_mttcg_enabled() now just macro
>   - qemu_tcg_configure now propagates Error instead of exiting
>   - better error checking of thread=foo
>   - use CONFIG flags for default_mttcg_enabled()
>   - disable mttcg with icount, error if both forced on
> v7
>   - explicitly disable MTTCG for TCG_OVERSIZED_GUEST
>   - use check_tcg_memory_orders_compatible() instead of CONFIG_MTTCG_HOST
>   - change CONFIG_MTTCG_TARGET to TARGET_SUPPORTS_MTTCG
> v8
>   - fix missing include tcg.h
>   - change mismatched MOs to a warning instead of error
> v10
>   - TCG_DEFAULT_MO -> TCG_GUEST_DEFAULT_MO
> ---
>  cpus.c                | 72 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++
>  include/qom/cpu.h     |  9 +++++++
>  include/sysemu/cpus.h |  2 ++
>  qemu-options.hx       | 20 ++++++++++++++
>  tcg/tcg.h             |  9 +++++++
>  vl.c                  | 49 ++++++++++++++++++++++++++++++++++-
>  6 files changed, 160 insertions(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 71a82e5004..6a7e5a50d8 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -25,6 +25,7 @@
>  /* Needed early for CONFIG_BSD etc. */
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
> +#include "qemu/config-file.h"
>  #include "cpu.h"
>  #include "monitor/monitor.h"
>  #include "qapi/qmp/qerror.h"
> @@ -45,6 +46,7 @@
>  #include "qemu/main-loop.h"
>  #include "qemu/bitmap.h"
>  #include "qemu/seqlock.h"
> +#include "tcg.h"
>  #include "qapi-event.h"
>  #include "hw/nmi.h"
>  #include "sysemu/replay.h"
> @@ -150,6 +152,76 @@ typedef struct TimersState {
>  } 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)
> +{
> +    QemuOpts *icount_opts = qemu_find_opts_singleton("icount");
> +    const char *rr = qemu_opt_get(icount_opts, "rr");
> +
> +    if (rr || 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 (!check_tcg_memory_orders_compatible()) {
> +                    error_report("Guest requires stronger MO that host");
>

s/MO that host/MO than host/?

Also, this message is a bit cryptic for end users. How about saying "Guest
and host architectures are incompatible, you may encounter errors!" or
something similar?

Thanks,
-- 
Pranith