Re: [Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 7916 bytes --]

On Wed, Jun 20, 2018 at 02:54:15PM +0200, Greg Kurz wrote:
> The spapr capability framework was introduced in QEMU 2.12. It allows
> to have an explicit control on how host features are exposed to the
> guest. This is especially needed to handle migration between hetero-
> geneous hosts (eg, POWER8 to POWER9). It is also used to expose fixes/
> workarounds against speculative execution vulnerabilities to guests.
> The framework was hence backported to QEMU 2.11.1, especially these
> commits:
> 
> 0fac4aa93074 spapr: Add pseries-2.12 machine type
> 9070f408f491 spapr: Treat Hardware Transactional Memory (HTM) as an
>  optional capability
> 
> 0fac4aa93074 has the confusing effect of making pseries-2.12 the default
> machine type for QEMU 2.11.1, instead of the expected pseries-2.11. This
> patch changes the default machine back to pseries-2.11.
> 
> Unfortunately, 9070f408f491 enforces the HTM capability for pseries-2.11
> to be enabled by default, ie, when not passing cap-htm on the command
> line. This breaks several 'make check' testcases that run qemu-system-ppc64
> with TCG.
> 
> The only sane way to fix this is to adapt the impacted testcases so that
> they all pass cap-htm=off in this case. This patch does that as well.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v2: - have the testcases to pass cap-htm=off instead of violating the
>       capabilities logic.
> 
> Upstream doesn't need anything like that since newer pseries machine types
> start with HTM disabled by default. This is really a oneshot fix for 2.11.2,
> and I've tried to make it as small as possible.
> 
> This is a full replacement of the previous version. It is based on Mike's
> staging tree for 2.11:

Thanks for fixing this up

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Btw, 2.11.z should probably have the 2.12 machine type removed
entirely, as well as (obviously) not being the default.  Not within
scope for this patch, though.

> 
> https://github.com/mdroth/qemu/commits/stable-2.11-staging 72cc467aabd1a2
> ---
>  hw/ppc/spapr.c           |    4 ++--
>  tests/boot-serial-test.c |    8 ++++++--
>  tests/migration-test.c   |    4 ++--
>  tests/prom-env-test.c    |    6 ++++--
>  tests/pxe-test.c         |   10 +++++++---
>  5 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1a2dd1f597d9..6499a867520f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3820,7 +3820,7 @@ static void spapr_machine_2_12_class_options(MachineClass *mc)
>      /* Defaults for the latest behaviour inherited from the base class */
>  }
>  
> -DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> +DEFINE_SPAPR_MACHINE(2_12, "2.12", false);
>  
>  /*
>   * pseries-2.11
> @@ -3842,7 +3842,7 @@ static void spapr_machine_2_11_class_options(MachineClass *mc)
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
>  }
>  
> -DEFINE_SPAPR_MACHINE(2_11, "2.11", false);
> +DEFINE_SPAPR_MACHINE(2_11, "2.11", true);
>  
>  /*
>   * pseries-2.10
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index c935d69824bd..98c5462377f8 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -73,18 +73,22 @@ static void test_machine(const void *data)
>      const testdef_t *test = data;
>      char tmpname[] = "/tmp/qtest-boot-serial-XXXXXX";
>      int fd;
> +    const char *machine_props;
>  
>      fd = mkstemp(tmpname);
>      g_assert(fd != -1);
>  
> +    machine_props = strcmp(test->machine, "pseries") == 0 ? ",cap-htm=off" : "";
> +
>      /*
>       * Make sure that this test uses tcg if available: It is used as a
>       * fast-enough smoketest for that.
>       */
> -    global_qtest = qtest_startf("-M %s,accel=tcg:kvm "
> +    global_qtest = qtest_startf("-M %s%s,accel=tcg:kvm "
>                                  "-chardev file,id=serial0,path=%s "
>                                  "-no-shutdown -serial chardev:serial0 %s",
> -                                test->machine, tmpname, test->extra);
> +                                test->machine, machine_props, tmpname,
> +                                test->extra);
>      unlink(tmpname);
>  
>      check_guest_output(test, fd);
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index be598d3257ba..906d29b38241 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -460,12 +460,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>          /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */
>          accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg";
>          init_bootfile_ppc(bootpath);
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> +        cmd_src = g_strdup_printf("-machine accel=%s,cap-htm=off -m 256M"
>                                    " -name pcsource,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,if=pflash,format=raw",
>                                    accel, tmpfs, bootpath);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +        cmd_dst = g_strdup_printf("-machine accel=%s,cap-htm=off -m 256M"
>                                    " -name pcdest,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -incoming %s",
> diff --git a/tests/prom-env-test.c b/tests/prom-env-test.c
> index 8c867e631ab6..f6946090d183 100644
> --- a/tests/prom-env-test.c
> +++ b/tests/prom-env-test.c
> @@ -45,14 +45,16 @@ static void check_guest_memory(void)
>  static void test_machine(const void *machine)
>  {
>      const char *extra_args;
> +    const char *extra_props;
>  
>      /* The pseries firmware boots much faster without the default devices */
>      extra_args = strcmp(machine, "pseries") == 0 ? "-nodefaults" : "";
> +    extra_props = strcmp(machine, "pseries") == 0 ? ",cap-htm=off" : "";
>  
> -    global_qtest = qtest_startf("-M %s,accel=tcg %s "
> +    global_qtest = qtest_startf("-M %s%s,accel=tcg %s "
>                                  "-prom-env 'use-nvramrc?=true' "
>                                  "-prom-env 'nvramrc=%x %x l!' ",
> -                                (const char *)machine, extra_args,
> +                                (const char *)machine, extra_props, extra_args,
>                                  MAGIC, ADDRESS);
>      check_guest_memory();
>      qtest_quit(global_qtest);
> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> index 937f29e63193..f3a65bd80cb1 100644
> --- a/tests/pxe-test.c
> +++ b/tests/pxe-test.c
> @@ -25,11 +25,15 @@ static char disk[] = "tests/pxe-test-disk-XXXXXX";
>  static void test_pxe_one(const char *params, bool ipv6)
>  {
>      char *args;
> +    const char *machine_props;
>  
> -    args = g_strdup_printf("-machine accel=kvm:tcg -nodefaults -boot order=n "
> +    machine_props =
> +        strcmp(qtest_get_arch(), "ppc64") == 0 ? ",cap-htm=off" : "";
> +
> +    args = g_strdup_printf("-machine accel=kvm:tcg%s -nodefaults -boot order=n "
>                             "-netdev user,id=" NETNAME ",tftp=./,bootfile=%s,"
> -                           "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : "on",
> -                           ipv6 ? "on" : "off", params);
> +                           "ipv4=%s,ipv6=%s %s", machine_props, disk,
> +                           ipv6 ? "off" : "on", ipv6 ? "on" : "off", params);
>  
>      qtest_start(args);
>      boot_sector_test();
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]