[Qemu-devel] [PATCHv2 0/2] Further VSMT fixes

[Qemu-devel] [PATCHv2 0/2] Further VSMT fixes

[David Gibson]

Here are some follow on fixes to Ziviani's proposed changes to VSMT
handling.  This should fix migration of POWER8 compat mode guests
between POWER8 and POWER9 hosts.

The changes are simple, the rationale's rather more complex.

Changes since v2:
  * Dropped one patch, already merged
  * Discovered that the previous version broken running KVM PR in
    almost all cases, even ones that should still be ok
      - Added a new 1/2 patch to address that

David Gibson (2):
  spapr: Allow some cases where we can't set VSMT mode in the kernel
  spapr: Adjust default VSMT value for better migration compatibility

 hw/ppc/spapr.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCHv2 1/2] spapr: Allow some cases where we can't set VSMT mode in the kernel

[David Gibson]

At present if we require a vsmt mode that's not equal to the kernel's
default, and the kernel doesn't let us change it (e.g. because it's an old
kernel without support) then we always fail.

But in fact we can cope with the kernel having a different vsmt as long as
  a) it's >= the actual number of vthreads/vcore (so that guest threads
     that are supposed to be on the same core act like it)
  b) it's a submultiple of the requested vsmt mode (so that guest threads
     spaced by the vsmt value will act like they're on different cores)

Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
without breaking existing cases.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e35214bfc3..6d3613d934 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
     if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
         ret = kvmppc_set_smt_threads(spapr->vsmt);
         if (ret) {
+            /* Looks like KVM isn't able to change VSMT mode */
             error_setg(&local_err,
                        "Failed to set KVM's VSMT mode to %d (errno %d)",
                        spapr->vsmt, ret);
-            if (!vsmt_user) {
-                error_append_hint(&local_err, "On PPC, a VM with %d threads/"
-                             "core on a host with %d threads/core requires "
-                             " the use of VSMT mode %d.\n",
-                             smp_threads, kvm_smt, spapr->vsmt);
+            /* We can live with that if the default one is big enough
+             * for the number of threads, and a submultiple of the one
+             * we want.  In this case we'll waste some vcpu ids, but
+             * behaviour will be correct */
+            if ((kvm_smt >= smp_threads) && (spapr->vsmt % kvm_smt) == 0) {
+                warn_report_err(local_err);
+                local_err = NULL;
+                goto out;
+            } else {
+                if (!vsmt_user) {
+                    error_append_hint(&local_err,
+                                      "On PPC, a VM with %d threads/core"
+                                      " on a host with %d threads/core"
+                                      " requires the use of VSMT mode %d.\n",
+                                      smp_threads, kvm_smt, spapr->vsmt);
+                }
+                kvmppc_hint_smt_possible(&local_err);
+                goto out;
             }
-            kvmppc_hint_smt_possible(&local_err);
-            goto out;
         }
     }
     /* else TCG: nothing to do currently */
-- 
2.14.3

[Qemu-devel] [PATCHv2 2/2] spapr: Adjust default VSMT value for better migration compatibility

[David Gibson]

fa98fbfc "PC: KVM: Support machine option to set VSMT mode" introduced the
"vsmt" parameter for the pseries machine type, which controls the spacing
of the vcpu ids of thread 0 for each virtual core.  This was done to bring
some consistency and stability to how that was done, while still allowing
backwards compatibility for migration and otherwise.

The default value we used for vsmt was set to the max of the host's
advertised default number of threads and the number of vthreads per vcore
in the guest.  This was done to continue running without extra parameters
on older KVM versions which don't allow the VSMT value to be changed.

Unfortunately, even that smaller than before leakage of host configuration
into guest visible configuration still breaks things.  Specifically a guest
with 4 (or less) vthread/vcore will get a different vsmt value when
running on a POWER8 (vsmt==8) and POWER9 (vsmt==4) host.  That means the
vcpu ids don't line up so you can't migrate between them, though you should
be able to.

Long term we really want to make vsmt == smp_threads for sufficiently
new machine types.  However, that means that qemu will then require a
sufficiently recent KVM (one which supports changing VSMT) - that's still
not widely enough deployed to be really comfortable to do.

In the meantime we need some default that will work as often as
possible.  This patch changes that default to 8 in all circumstances.
This does change guest visible behaviour (including for existing
machine versions) for many cases - just not the most common/important
case.

Following is case by case justification for why this is still the least
worst option.  Note that any of the old behaviours can still be duplicated
after this patch, it's just that it requires manual intervention by
setting the vsmt property on the command line.

KVM HV on POWER8 host:
   This is the overwhelmingly common case in production setups, and is
   unchanged by design.  POWER8 hosts will advertise a default VSMT mode
   of 8, and > 8 vthreads/vcore isn't permitted

KVM HV on POWER7 host:
   Will break, but POWER7s allowing KVM were never released to the public.

KVM HV on POWER9 host:
   Not yet released to the public, breaking this now will reduce other
   breakage later.

KVM HV on PowerPC 970:
   Will theoretically break it, but it was barely supported to begin with
   and already required various user visible hacks to work.  Also so old
   that I just don't care.

TCG:
   This is the nastiest one; it means migration of TCG guests (without
   manual vsmt setting) will break.  Since TCG is rarely used in production
   I think this is worth it for the other benefits.  It does also remove
   one more barrier to TCG<->KVM migration which could be interesting for
   debugging applications.

KVM PR:
   As with TCG, this will break migration of existing configurations,
   without adding extra manual vsmt options.  As with TCG, it is rare in
   production so I think the benefits outweigh breakages.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6d3613d934..a216ceada8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2305,9 +2305,14 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
         }
         /* In this case, spapr->vsmt has been set by the command line */
     } else {
-        /* Choose a VSMT mode that may be higher than necessary but is
-         * likely to be compatible with hosts that don't have VSMT. */
-        spapr->vsmt = MAX(kvm_smt, smp_threads);
+        /*
+         * Default VSMT value is tricky, because we need it to be as
+         * consistent as possible (for migration), but this requires
+         * changing it for at least some existing cases.  We pick 8 as
+         * the value that we'd get with KVM on POWER8, the
+         * overwhelmingly common case in production systems.
+         */
+        spapr->vsmt = 8;
     }
 
     /* KVM: If necessary, set the SMT mode: */
-- 
2.14.3

Re: [Qemu-devel] [PATCHv2 1/2] spapr: Allow some cases where we can't set VSMT mode in the kernel

[Greg Kurz]

On Tue, 16 Jan 2018 15:47:13 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> At present if we require a vsmt mode that's not equal to the kernel's
> default, and the kernel doesn't let us change it (e.g. because it's an old
> kernel without support) then we always fail.
> 
> But in fact we can cope with the kernel having a different vsmt as long as
>   a) it's >= the actual number of vthreads/vcore (so that guest threads
>      that are supposed to be on the same core act like it)
>   b) it's a submultiple of the requested vsmt mode (so that guest threads
>      spaced by the vsmt value will act like they're on different cores)
> 
> Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
> without breaking existing cases.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

I could check the following on a POWER9 host:

$ ./ppc64-softmmu/qemu-system-ppc64 -accel kvm -smp threads=1
qemu-system-ppc64: warning: Failed to set KVM's VSMT mode to 8 (errno -22)

and the guest boots.

$ ./ppc64-softmmu/qemu-system-ppc64 -accel kvm -smp threads=2
qemu-system-ppc64: Failed to set KVM's VSMT mode to 8 (errno -22)
On PPC, a VM with 2 threads/core on a host with 1 threads/core requires the
 use of VSMT mode 8.
This KVM seems to be too old to support VSMT.

and QEMU exits.

Tested-by: Greg Kurz <groug@kaod.org>

Just one minor remark below but anyway:

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e35214bfc3..6d3613d934 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
>      if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
>          ret = kvmppc_set_smt_threads(spapr->vsmt);
>          if (ret) {
> +            /* Looks like KVM isn't able to change VSMT mode */
>              error_setg(&local_err,
>                         "Failed to set KVM's VSMT mode to %d (errno %d)",
>                         spapr->vsmt, ret);
> -            if (!vsmt_user) {
> -                error_append_hint(&local_err, "On PPC, a VM with %d threads/"
> -                             "core on a host with %d threads/core requires "
> -                             " the use of VSMT mode %d.\n",
> -                             smp_threads, kvm_smt, spapr->vsmt);
> +            /* We can live with that if the default one is big enough
> +             * for the number of threads, and a submultiple of the one
> +             * we want.  In this case we'll waste some vcpu ids, but
> +             * behaviour will be correct */
> +            if ((kvm_smt >= smp_threads) && (spapr->vsmt % kvm_smt) == 0) {

Inconsistent use of parens in the left and right operands of &&

> +                warn_report_err(local_err);
> +                local_err = NULL;
> +                goto out;
> +            } else {
> +                if (!vsmt_user) {
> +                    error_append_hint(&local_err,
> +                                      "On PPC, a VM with %d threads/core"
> +                                      " on a host with %d threads/core"
> +                                      " requires the use of VSMT mode %d.\n",
> +                                      smp_threads, kvm_smt, spapr->vsmt);
> +                }
> +                kvmppc_hint_smt_possible(&local_err);
> +                goto out;
>              }
> -            kvmppc_hint_smt_possible(&local_err);
> -            goto out;
>          }
>      }
>      /* else TCG: nothing to do currently */

Re: [Qemu-devel] [PATCHv2 1/2] spapr: Allow some cases where we can't set VSMT mode in the kernel

[Laurent Vivier]

On 16/01/2018 05:47, David Gibson wrote:
> At present if we require a vsmt mode that's not equal to the kernel's
> default, and the kernel doesn't let us change it (e.g. because it's an old
> kernel without support) then we always fail.
> 
> But in fact we can cope with the kernel having a different vsmt as long as
>   a) it's >= the actual number of vthreads/vcore (so that guest threads
>      that are supposed to be on the same core act like it)
>   b) it's a submultiple of the requested vsmt mode (so that guest threads
>      spaced by the vsmt value will act like they're on different cores)
> 
> Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
> without breaking existing cases.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e35214bfc3..6d3613d934 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
>      if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
>          ret = kvmppc_set_smt_threads(spapr->vsmt);
>          if (ret) {
> +            /* Looks like KVM isn't able to change VSMT mode */
>              error_setg(&local_err,
>                         "Failed to set KVM's VSMT mode to %d (errno %d)",
>                         spapr->vsmt, ret);
> -            if (!vsmt_user) {
> -                error_append_hint(&local_err, "On PPC, a VM with %d threads/"
> -                             "core on a host with %d threads/core requires "
> -                             " the use of VSMT mode %d.\n",
> -                             smp_threads, kvm_smt, spapr->vsmt);
> +            /* We can live with that if the default one is big enough
> +             * for the number of threads, and a submultiple of the one
> +             * we want.  In this case we'll waste some vcpu ids, but
> +             * behaviour will be correct */
> +            if ((kvm_smt >= smp_threads) && (spapr->vsmt % kvm_smt) == 0) {

I agree with Greg: inconsistent use of parenthesis, should be

    if (kvm_smt >= smp_threads && (spapr->vsmt % kvm_smt) == 0) {

Anyway:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Thanks,
Laurent

Re: [Qemu-devel] [PATCHv2 1/2] spapr: Allow some cases where we can't set VSMT mode in the kernel

[David Gibson]

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

On Tue, Jan 16, 2018 at 10:20:18AM +0100, Greg Kurz wrote:
> On Tue, 16 Jan 2018 15:47:13 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > At present if we require a vsmt mode that's not equal to the kernel's
> > default, and the kernel doesn't let us change it (e.g. because it's an old
> > kernel without support) then we always fail.
> > 
> > But in fact we can cope with the kernel having a different vsmt as long as
> >   a) it's >= the actual number of vthreads/vcore (so that guest threads
> >      that are supposed to be on the same core act like it)
> >   b) it's a submultiple of the requested vsmt mode (so that guest threads
> >      spaced by the vsmt value will act like they're on different cores)
> > 
> > Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
> > without breaking existing cases.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> 
> I could check the following on a POWER9 host:
> 
> $ ./ppc64-softmmu/qemu-system-ppc64 -accel kvm -smp threads=1
> qemu-system-ppc64: warning: Failed to set KVM's VSMT mode to 8 (errno -22)
> 
> and the guest boots.
> 
> $ ./ppc64-softmmu/qemu-system-ppc64 -accel kvm -smp threads=2
> qemu-system-ppc64: Failed to set KVM's VSMT mode to 8 (errno -22)
> On PPC, a VM with 2 threads/core on a host with 1 threads/core requires the
>  use of VSMT mode 8.
> This KVM seems to be too old to support VSMT.
> 
> and QEMU exits.

I assume the above is with an old kernel that doesn't have the ability
to set the SMT cap?

> 
> Tested-by: Greg Kurz <groug@kaod.org>
> 
> Just one minor remark below but anyway:
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> >  hw/ppc/spapr.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e35214bfc3..6d3613d934 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
> >      if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
> >          ret = kvmppc_set_smt_threads(spapr->vsmt);
> >          if (ret) {
> > +            /* Looks like KVM isn't able to change VSMT mode */
> >              error_setg(&local_err,
> >                         "Failed to set KVM's VSMT mode to %d (errno %d)",
> >                         spapr->vsmt, ret);
> > -            if (!vsmt_user) {
> > -                error_append_hint(&local_err, "On PPC, a VM with %d threads/"
> > -                             "core on a host with %d threads/core requires "
> > -                             " the use of VSMT mode %d.\n",
> > -                             smp_threads, kvm_smt, spapr->vsmt);
> > +            /* We can live with that if the default one is big enough
> > +             * for the number of threads, and a submultiple of the one
> > +             * we want.  In this case we'll waste some vcpu ids, but
> > +             * behaviour will be correct */
> > +            if ((kvm_smt >= smp_threads) && (spapr->vsmt % kvm_smt) == 0) {
> 
> Inconsistent use of parens in the left and right operands of &&
> 
> > +                warn_report_err(local_err);
> > +                local_err = NULL;
> > +                goto out;
> > +            } else {
> > +                if (!vsmt_user) {
> > +                    error_append_hint(&local_err,
> > +                                      "On PPC, a VM with %d threads/core"
> > +                                      " on a host with %d threads/core"
> > +                                      " requires the use of VSMT mode %d.\n",
> > +                                      smp_threads, kvm_smt, spapr->vsmt);
> > +                }
> > +                kvmppc_hint_smt_possible(&local_err);
> > +                goto out;
> >              }
> > -            kvmppc_hint_smt_possible(&local_err);
> > -            goto out;
> >          }
> >      }
> >      /* else TCG: nothing to do currently */
> 

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

Re: [Qemu-devel] [PATCHv2 1/2] spapr: Allow some cases where we can't set VSMT mode in the kernel

[David Gibson]

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

On Tue, Jan 16, 2018 at 11:34:52AM +0100, Laurent Vivier wrote:
> On 16/01/2018 05:47, David Gibson wrote:
> > At present if we require a vsmt mode that's not equal to the kernel's
> > default, and the kernel doesn't let us change it (e.g. because it's an old
> > kernel without support) then we always fail.
> > 
> > But in fact we can cope with the kernel having a different vsmt as long as
> >   a) it's >= the actual number of vthreads/vcore (so that guest threads
> >      that are supposed to be on the same core act like it)
> >   b) it's a submultiple of the requested vsmt mode (so that guest threads
> >      spaced by the vsmt value will act like they're on different cores)
> > 
> > Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
> > without breaking existing cases.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e35214bfc3..6d3613d934 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
> >      if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
> >          ret = kvmppc_set_smt_threads(spapr->vsmt);
> >          if (ret) {
> > +            /* Looks like KVM isn't able to change VSMT mode */
> >              error_setg(&local_err,
> >                         "Failed to set KVM's VSMT mode to %d (errno %d)",
> >                         spapr->vsmt, ret);
> > -            if (!vsmt_user) {
> > -                error_append_hint(&local_err, "On PPC, a VM with %d threads/"
> > -                             "core on a host with %d threads/core requires "
> > -                             " the use of VSMT mode %d.\n",
> > -                             smp_threads, kvm_smt, spapr->vsmt);
> > +            /* We can live with that if the default one is big enough
> > +             * for the number of threads, and a submultiple of the one
> > +             * we want.  In this case we'll waste some vcpu ids, but
> > +             * behaviour will be correct */
> > +            if ((kvm_smt >= smp_threads) && (spapr->vsmt % kvm_smt) == 0) {
> 
> I agree with Greg: inconsistent use of parenthesis, should be
> 
>     if (kvm_smt >= smp_threads && (spapr->vsmt % kvm_smt) == 0) {

Corrected in my tree.

> Anyway:
> 
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> 
> Thanks,
> Laurent
> 

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

Re: [Qemu-devel] [PATCHv2 1/2] spapr: Allow some cases where we can't set VSMT mode in the kernel

[Greg Kurz]

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

On Tue, 16 Jan 2018 23:24:32 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Jan 16, 2018 at 10:20:18AM +0100, Greg Kurz wrote:
> > On Tue, 16 Jan 2018 15:47:13 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > At present if we require a vsmt mode that's not equal to the kernel's
> > > default, and the kernel doesn't let us change it (e.g. because it's an old
> > > kernel without support) then we always fail.
> > > 
> > > But in fact we can cope with the kernel having a different vsmt as long as
> > >   a) it's >= the actual number of vthreads/vcore (so that guest threads
> > >      that are supposed to be on the same core act like it)
> > >   b) it's a submultiple of the requested vsmt mode (so that guest threads
> > >      spaced by the vsmt value will act like they're on different cores)
> > > 
> > > Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
> > > without breaking existing cases.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---  
> > 
> > I could check the following on a POWER9 host:
> > 
> > $ ./ppc64-softmmu/qemu-system-ppc64 -accel kvm -smp threads=1
> > qemu-system-ppc64: warning: Failed to set KVM's VSMT mode to 8 (errno -22)
> > 
> > and the guest boots.
> > 
> > $ ./ppc64-softmmu/qemu-system-ppc64 -accel kvm -smp threads=2
> > qemu-system-ppc64: Failed to set KVM's VSMT mode to 8 (errno -22)
> > On PPC, a VM with 2 threads/core on a host with 1 threads/core requires the
> >  use of VSMT mode 8.
> > This KVM seems to be too old to support VSMT.
> > 
> > and QEMU exits.  
> 
> I assume the above is with an old kernel that doesn't have the ability
> to set the SMT cap?
> 

Yes this was tested with a 4.11 kernel (setting of SMT came with 4.12).

> > 
> > Tested-by: Greg Kurz <groug@kaod.org>
> > 
> > Just one minor remark below but anyway:
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> >   
> > >  hw/ppc/spapr.c | 26 +++++++++++++++++++-------
> > >  1 file changed, 19 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index e35214bfc3..6d3613d934 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
> > >      if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
> > >          ret = kvmppc_set_smt_threads(spapr->vsmt);
> > >          if (ret) {
> > > +            /* Looks like KVM isn't able to change VSMT mode */
> > >              error_setg(&local_err,
> > >                         "Failed to set KVM's VSMT mode to %d (errno %d)",
> > >                         spapr->vsmt, ret);
> > > -            if (!vsmt_user) {
> > > -                error_append_hint(&local_err, "On PPC, a VM with %d threads/"
> > > -                             "core on a host with %d threads/core requires "
> > > -                             " the use of VSMT mode %d.\n",
> > > -                             smp_threads, kvm_smt, spapr->vsmt);
> > > +            /* We can live with that if the default one is big enough
> > > +             * for the number of threads, and a submultiple of the one
> > > +             * we want.  In this case we'll waste some vcpu ids, but
> > > +             * behaviour will be correct */
> > > +            if ((kvm_smt >= smp_threads) && (spapr->vsmt % kvm_smt) == 0) {  
> > 
> > Inconsistent use of parens in the left and right operands of &&
> >   
> > > +                warn_report_err(local_err);
> > > +                local_err = NULL;
> > > +                goto out;
> > > +            } else {
> > > +                if (!vsmt_user) {
> > > +                    error_append_hint(&local_err,
> > > +                                      "On PPC, a VM with %d threads/core"
> > > +                                      " on a host with %d threads/core"
> > > +                                      " requires the use of VSMT mode %d.\n",
> > > +                                      smp_threads, kvm_smt, spapr->vsmt);
> > > +                }
> > > +                kvmppc_hint_smt_possible(&local_err);
> > > +                goto out;
> > >              }
> > > -            kvmppc_hint_smt_possible(&local_err);
> > > -            goto out;
> > >          }
> > >      }
> > >      /* else TCG: nothing to do currently */  
> >   
> 


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