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

Greg Kurz posted 1 patch 5 years, 10 months ago
Failed in applying to current master (apply log)
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(-)
[Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type
Posted by Greg Kurz 5 years, 10 months ago
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:

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();


Re: [Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type
Posted by David Gibson 5 years, 10 months ago
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
Re: [Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type
Posted by Greg Kurz 5 years, 10 months ago
On Thu, 21 Jun 2018 11:18:09 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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.
> 

Well... it is indeed weird but I don't think it hurts. Also, people
may have started using pseries-2.12 with QEMU 2.11.1 (I seem to
remember Mike told me about something like this the other day)...

> > 
> > 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();
> >   
> 

Re: [Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type
Posted by David Gibson 5 years, 9 months ago
On Thu, Jun 21, 2018 at 03:23:21PM +0200, Greg Kurz wrote:
> On Thu, 21 Jun 2018 11:18:09 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > 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.
> > 
> 
> Well... it is indeed weird but I don't think it hurts. Also, people
> may have started using pseries-2.12 with QEMU 2.11.1 (I seem to
> remember Mike told me about something like this the other day)...

I came back to thinking about this and realized this is a terrible
idea.  The problem is that because of the way we define the latest
machine type, then backwards compat props for the earlier ones, it's
very likely that the "pseries-2.12" in 2.11 won't be the same as
pseries-2.12 in 2.12, simply because 2.11 won't have the necessary
features to implement pseries-2.12 as in 2.12.


-- 
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
Re: [Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type
Posted by Michael Roth 5 years, 9 months ago
Quoting David Gibson (2018-07-17 05:50:06)
> On Thu, Jun 21, 2018 at 03:23:21PM +0200, Greg Kurz wrote:
> > On Thu, 21 Jun 2018 11:18:09 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > 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.
> > > 
> > 
> > Well... it is indeed weird but I don't think it hurts. Also, people
> > may have started using pseries-2.12 with QEMU 2.11.1 (I seem to
> > remember Mike told me about something like this the other day)...
> 
> I came back to thinking about this and realized this is a terrible
> idea.  The problem is that because of the way we define the latest
> machine type, then backwards compat props for the earlier ones, it's
> very likely that the "pseries-2.12" in 2.11 won't be the same as
> pseries-2.12 in 2.12, simply because 2.11 won't have the necessary
> features to implement pseries-2.12 as in 2.12.

What I saw was likely a holdover from early Ubuntu 18.04 p9 testing
before HTM emulation had made it's way into their kernel. In that
particular case Ubuntu doesn't support it at all now, but it remains
the only option for anyone else who happens to find themselves in
that situation and doesn't have a libvirt/Openstack with appropriate
machine capabilities.

Given that it was at least possible to run that config (however
broken) with QEMU 2.11.0, it seemed fair to leave some sort of
"out". But really it just seems like we don't have much to gain
by removing it at this point.

I agree in general though and probably would've handled this
differently if I'd thought through it a bit more.

> 
> 
> -- 
> 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


Re: [Qemu-devel] [PATCH v2 for-2.11.2] spapr: make pseries-2.11 the default machine type
Posted by Dr. David Alan Gilbert 5 years, 9 months ago
* Michael Roth (mdroth@linux.vnet.ibm.com) wrote:
> Quoting David Gibson (2018-07-17 05:50:06)
> > On Thu, Jun 21, 2018 at 03:23:21PM +0200, Greg Kurz wrote:
> > > On Thu, 21 Jun 2018 11:18:09 +1000
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > 
> > > > 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.
> > > > 
> > > 
> > > Well... it is indeed weird but I don't think it hurts. Also, people
> > > may have started using pseries-2.12 with QEMU 2.11.1 (I seem to
> > > remember Mike told me about something like this the other day)...
> > 
> > I came back to thinking about this and realized this is a terrible
> > idea.  The problem is that because of the way we define the latest
> > machine type, then backwards compat props for the earlier ones, it's
> > very likely that the "pseries-2.12" in 2.11 won't be the same as
> > pseries-2.12 in 2.12, simply because 2.11 won't have the necessary
> > features to implement pseries-2.12 as in 2.12.
> 
> What I saw was likely a holdover from early Ubuntu 18.04 p9 testing
> before HTM emulation had made it's way into their kernel. In that
> particular case Ubuntu doesn't support it at all now, but it remains
> the only option for anyone else who happens to find themselves in
> that situation and doesn't have a libvirt/Openstack with appropriate
> machine capabilities.
> 
> Given that it was at least possible to run that config (however
> broken) with QEMU 2.11.0, it seemed fair to leave some sort of
> "out". But really it just seems like we don't have much to gain
> by removing it at this point.

It may confuse people trying to migrate from 2.11.1 to 2.12

Dave

> I agree in general though and probably would've handled this
> differently if I'd thought through it a bit more.
> 
> > 
> > 
> > -- 
> > 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
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK