On Tue, 16 Jan 2018 23:24:32 +1100 David Gibson <da...@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 <da...@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 <da...@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 <gr...@kaod.org>
> >
> > Just one minor remark below but anyway:
> >
> > Reviewed-by: Greg Kurz <gr...@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 */
> >
>
pgp2wypl64yPk.pgp
Description: OpenPGP digital signature
