From: Greg Kurz <groug@kaod.org>
Support for setting VSMT is available in KVM since linux-4.13. Most distros
that support KVM on POWER already have it. It thus seem reasonable enough
to have the default machine to set VSMT to smp_threads.
This brings contiguous VCPU ids and thus brings their upper bound down to
the machine's max_cpus. This is especially useful for XIVE KVM devices,
which may thus allocate only one VP descriptor per VCPU.
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <157010411885.246126.12610015369068227139.stgit@bahia.lan>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 7 ++++++-
include/hw/ppc/spapr.h | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4eb97d3a9b..428b834f30 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2496,6 +2496,7 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
{
MachineState *ms = MACHINE(spapr);
+ SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
Error *local_err = NULL;
bool vsmt_user = !!spapr->vsmt;
int kvm_smt = kvmppc_smt_threads();
@@ -2522,7 +2523,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
goto out;
}
/* In this case, spapr->vsmt has been set by the command line */
- } else {
+ } else if (!smc->smp_threads_vsmt) {
/*
* Default VSMT value is tricky, because we need it to be as
* consistent as possible (for migration), but this requires
@@ -2531,6 +2532,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
* overwhelmingly common case in production systems.
*/
spapr->vsmt = MAX(8, smp_threads);
+ } else {
+ spapr->vsmt = smp_threads;
}
/* KVM: If necessary, set the SMT mode: */
@@ -4438,6 +4441,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
smc->irq = &spapr_irq_dual;
smc->dr_phb_enabled = true;
smc->linux_pci_probe = true;
+ smc->smp_threads_vsmt = true;
}
static const TypeInfo spapr_machine_info = {
@@ -4505,6 +4509,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
spapr_machine_4_2_class_options(mc);
smc->linux_pci_probe = false;
+ smc->smp_threads_vsmt = false;
compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index cbd1a4c9f3..2009eb64f9 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -122,6 +122,7 @@ struct SpaprMachineClass {
bool broken_host_serial_model; /* present real host info to the guest */
bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
bool linux_pci_probe;
+ bool smp_threads_vsmt; /* set VSMT to smp_threads by default */
void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
uint64_t *buid, hwaddr *pio,
--
2.21.0
On 24/10/2019 10:17, David Gibson wrote:
> From: Greg Kurz <groug@kaod.org>
>
> Support for setting VSMT is available in KVM since linux-4.13. Most distros
> that support KVM on POWER already have it. It thus seem reasonable enough
> to have the default machine to set VSMT to smp_threads.
>
> This brings contiguous VCPU ids and thus brings their upper bound down to
> the machine's max_cpus. This is especially useful for XIVE KVM devices,
> which may thus allocate only one VP descriptor per VCPU.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Message-Id: <157010411885.246126.12610015369068227139.stgit@bahia.lan>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr.c | 7 ++++++-
> include/hw/ppc/spapr.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4eb97d3a9b..428b834f30 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2496,6 +2496,7 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
> static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
> {
> MachineState *ms = MACHINE(spapr);
> + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> Error *local_err = NULL;
> bool vsmt_user = !!spapr->vsmt;
> int kvm_smt = kvmppc_smt_threads();
> @@ -2522,7 +2523,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
> goto out;
> }
> /* In this case, spapr->vsmt has been set by the command line */
> - } else {
> + } else if (!smc->smp_threads_vsmt) {
> /*
> * Default VSMT value is tricky, because we need it to be as
> * consistent as possible (for migration), but this requires
> @@ -2531,6 +2532,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
> * overwhelmingly common case in production systems.
> */
> spapr->vsmt = MAX(8, smp_threads);
> + } else {
> + spapr->vsmt = smp_threads;
> }
>
> /* KVM: If necessary, set the SMT mode: */
> @@ -4438,6 +4441,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> smc->irq = &spapr_irq_dual;
> smc->dr_phb_enabled = true;
> smc->linux_pci_probe = true;
> + smc->smp_threads_vsmt = true;
> }
>
> static const TypeInfo spapr_machine_info = {
> @@ -4505,6 +4509,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
>
> spapr_machine_4_2_class_options(mc);
> smc->linux_pci_probe = false;
> + smc->smp_threads_vsmt = false;
> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
> }
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index cbd1a4c9f3..2009eb64f9 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -122,6 +122,7 @@ struct SpaprMachineClass {
> bool broken_host_serial_model; /* present real host info to the guest */
> bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
> bool linux_pci_probe;
> + bool smp_threads_vsmt; /* set VSMT to smp_threads by default */
>
> void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
> uint64_t *buid, hwaddr *pio,
>
This patch breaks tests/migration-test on P8 host with kernel older than
4.3 because it tries by default to set the VSMT to 1.
qemu-system-ppc64: Failed to set KVM's VSMT mode to 1 (errno -22)
On PPC, a VM with 1 threads/core on a host with 8 threads/core requires
the use of VSMT mode 1.
This KVM seems to be too old to support VSMT.
As this is clearly intentional, is there a way to fix migration-test?
Thanks,
Laurent
On Fri, Nov 08, 2019 at 02:11:03PM +0100, Laurent Vivier wrote:
> On 24/10/2019 10:17, David Gibson wrote:
> > From: Greg Kurz <groug@kaod.org>
> >
> > Support for setting VSMT is available in KVM since linux-4.13. Most distros
> > that support KVM on POWER already have it. It thus seem reasonable enough
> > to have the default machine to set VSMT to smp_threads.
> >
> > This brings contiguous VCPU ids and thus brings their upper bound down to
> > the machine's max_cpus. This is especially useful for XIVE KVM devices,
> > which may thus allocate only one VP descriptor per VCPU.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > Message-Id: <157010411885.246126.12610015369068227139.stgit@bahia.lan>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr.c | 7 ++++++-
> > include/hw/ppc/spapr.h | 1 +
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4eb97d3a9b..428b834f30 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2496,6 +2496,7 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
> > static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
> > {
> > MachineState *ms = MACHINE(spapr);
> > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> > Error *local_err = NULL;
> > bool vsmt_user = !!spapr->vsmt;
> > int kvm_smt = kvmppc_smt_threads();
> > @@ -2522,7 +2523,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
> > goto out;
> > }
> > /* In this case, spapr->vsmt has been set by the command line */
> > - } else {
> > + } else if (!smc->smp_threads_vsmt) {
> > /*
> > * Default VSMT value is tricky, because we need it to be as
> > * consistent as possible (for migration), but this requires
> > @@ -2531,6 +2532,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
> > * overwhelmingly common case in production systems.
> > */
> > spapr->vsmt = MAX(8, smp_threads);
> > + } else {
> > + spapr->vsmt = smp_threads;
> > }
> >
> > /* KVM: If necessary, set the SMT mode: */
> > @@ -4438,6 +4441,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > smc->irq = &spapr_irq_dual;
> > smc->dr_phb_enabled = true;
> > smc->linux_pci_probe = true;
> > + smc->smp_threads_vsmt = true;
> > }
> >
> > static const TypeInfo spapr_machine_info = {
> > @@ -4505,6 +4509,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
> >
> > spapr_machine_4_2_class_options(mc);
> > smc->linux_pci_probe = false;
> > + smc->smp_threads_vsmt = false;
> > compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
> > compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
> > }
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index cbd1a4c9f3..2009eb64f9 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -122,6 +122,7 @@ struct SpaprMachineClass {
> > bool broken_host_serial_model; /* present real host info to the guest */
> > bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
> > bool linux_pci_probe;
> > + bool smp_threads_vsmt; /* set VSMT to smp_threads by default */
> >
> > void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
> > uint64_t *buid, hwaddr *pio,
> >
>
> This patch breaks tests/migration-test on P8 host with kernel older than
> 4.3 because it tries by default to set the VSMT to 1.
>
> qemu-system-ppc64: Failed to set KVM's VSMT mode to 1 (errno -22)
> On PPC, a VM with 1 threads/core on a host with 8 threads/core requires
> the use of VSMT mode 1.
> This KVM seems to be too old to support VSMT.
>
> As this is clearly intentional, is there a way to fix migration-test?
Hrm. I believe the argument for this was that the broken kernels were
old enough we didn't care. What platform are you testing on where
you're hitting this?
--
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
On 08/11/2019 15:26, David Gibson wrote:
> On Fri, Nov 08, 2019 at 02:11:03PM +0100, Laurent Vivier wrote:
>> On 24/10/2019 10:17, David Gibson wrote:
>>> From: Greg Kurz <groug@kaod.org>
>>>
>>> Support for setting VSMT is available in KVM since linux-4.13. Most distros
>>> that support KVM on POWER already have it. It thus seem reasonable enough
>>> to have the default machine to set VSMT to smp_threads.
>>>
>>> This brings contiguous VCPU ids and thus brings their upper bound down to
>>> the machine's max_cpus. This is especially useful for XIVE KVM devices,
>>> which may thus allocate only one VP descriptor per VCPU.
>>>
>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>> Message-Id: <157010411885.246126.12610015369068227139.stgit@bahia.lan>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>> hw/ppc/spapr.c | 7 ++++++-
>>> include/hw/ppc/spapr.h | 1 +
>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 4eb97d3a9b..428b834f30 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -2496,6 +2496,7 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
>>> static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
>>> {
>>> MachineState *ms = MACHINE(spapr);
>>> + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>>> Error *local_err = NULL;
>>> bool vsmt_user = !!spapr->vsmt;
>>> int kvm_smt = kvmppc_smt_threads();
>>> @@ -2522,7 +2523,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
>>> goto out;
>>> }
>>> /* In this case, spapr->vsmt has been set by the command line */
>>> - } else {
>>> + } else if (!smc->smp_threads_vsmt) {
>>> /*
>>> * Default VSMT value is tricky, because we need it to be as
>>> * consistent as possible (for migration), but this requires
>>> @@ -2531,6 +2532,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
>>> * overwhelmingly common case in production systems.
>>> */
>>> spapr->vsmt = MAX(8, smp_threads);
>>> + } else {
>>> + spapr->vsmt = smp_threads;
>>> }
>>>
>>> /* KVM: If necessary, set the SMT mode: */
>>> @@ -4438,6 +4441,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>> smc->irq = &spapr_irq_dual;
>>> smc->dr_phb_enabled = true;
>>> smc->linux_pci_probe = true;
>>> + smc->smp_threads_vsmt = true;
>>> }
>>>
>>> static const TypeInfo spapr_machine_info = {
>>> @@ -4505,6 +4509,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
>>>
>>> spapr_machine_4_2_class_options(mc);
>>> smc->linux_pci_probe = false;
>>> + smc->smp_threads_vsmt = false;
>>> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
>>> compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>>> }
>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>> index cbd1a4c9f3..2009eb64f9 100644
>>> --- a/include/hw/ppc/spapr.h
>>> +++ b/include/hw/ppc/spapr.h
>>> @@ -122,6 +122,7 @@ struct SpaprMachineClass {
>>> bool broken_host_serial_model; /* present real host info to the guest */
>>> bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
>>> bool linux_pci_probe;
>>> + bool smp_threads_vsmt; /* set VSMT to smp_threads by default */
>>>
>>> void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
>>> uint64_t *buid, hwaddr *pio,
>>>
>>
>> This patch breaks tests/migration-test on P8 host with kernel older than
>> 4.3 because it tries by default to set the VSMT to 1.
>>
>> qemu-system-ppc64: Failed to set KVM's VSMT mode to 1 (errno -22)
>> On PPC, a VM with 1 threads/core on a host with 8 threads/core requires
>> the use of VSMT mode 1.
>> This KVM seems to be too old to support VSMT.
>>
>> As this is clearly intentional, is there a way to fix migration-test?
>
> Hrm. I believe the argument for this was that the broken kernels were
> old enough we didn't care. What platform are you testing on where
> you're hitting this?
>
I'm going to propose a patch to fix this problem.
(it was on RHEL7)
Thanks,
Laurent
On 08/11/2019 16:34, Laurent Vivier wrote:
> On 08/11/2019 15:26, David Gibson wrote:
>> On Fri, Nov 08, 2019 at 02:11:03PM +0100, Laurent Vivier wrote:
>>> On 24/10/2019 10:17, David Gibson wrote:
>>>> From: Greg Kurz <groug@kaod.org>
>>>>
>>>> Support for setting VSMT is available in KVM since linux-4.13. Most distros
>>>> that support KVM on POWER already have it. It thus seem reasonable enough
>>>> to have the default machine to set VSMT to smp_threads.
>>>>
>>>> This brings contiguous VCPU ids and thus brings their upper bound down to
>>>> the machine's max_cpus. This is especially useful for XIVE KVM devices,
>>>> which may thus allocate only one VP descriptor per VCPU.
>>>>
>>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>>> Message-Id: <157010411885.246126.12610015369068227139.stgit@bahia.lan>
>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>> ---
>>>> hw/ppc/spapr.c | 7 ++++++-
>>>> include/hw/ppc/spapr.h | 1 +
>>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index 4eb97d3a9b..428b834f30 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -2496,6 +2496,7 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
>>>> static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
>>>> {
>>>> MachineState *ms = MACHINE(spapr);
>>>> + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>>>> Error *local_err = NULL;
>>>> bool vsmt_user = !!spapr->vsmt;
>>>> int kvm_smt = kvmppc_smt_threads();
>>>> @@ -2522,7 +2523,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
>>>> goto out;
>>>> }
>>>> /* In this case, spapr->vsmt has been set by the command line */
>>>> - } else {
>>>> + } else if (!smc->smp_threads_vsmt) {
>>>> /*
>>>> * Default VSMT value is tricky, because we need it to be as
>>>> * consistent as possible (for migration), but this requires
>>>> @@ -2531,6 +2532,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
>>>> * overwhelmingly common case in production systems.
>>>> */
>>>> spapr->vsmt = MAX(8, smp_threads);
>>>> + } else {
>>>> + spapr->vsmt = smp_threads;
>>>> }
>>>>
>>>> /* KVM: If necessary, set the SMT mode: */
>>>> @@ -4438,6 +4441,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>>> smc->irq = &spapr_irq_dual;
>>>> smc->dr_phb_enabled = true;
>>>> smc->linux_pci_probe = true;
>>>> + smc->smp_threads_vsmt = true;
>>>> }
>>>>
>>>> static const TypeInfo spapr_machine_info = {
>>>> @@ -4505,6 +4509,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc)
>>>>
>>>> spapr_machine_4_2_class_options(mc);
>>>> smc->linux_pci_probe = false;
>>>> + smc->smp_threads_vsmt = false;
>>>> compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
>>>> compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>>>> }
>>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>> index cbd1a4c9f3..2009eb64f9 100644
>>>> --- a/include/hw/ppc/spapr.h
>>>> +++ b/include/hw/ppc/spapr.h
>>>> @@ -122,6 +122,7 @@ struct SpaprMachineClass {
>>>> bool broken_host_serial_model; /* present real host info to the guest */
>>>> bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
>>>> bool linux_pci_probe;
>>>> + bool smp_threads_vsmt; /* set VSMT to smp_threads by default */
>>>>
>>>> void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
>>>> uint64_t *buid, hwaddr *pio,
>>>>
>>>
>>> This patch breaks tests/migration-test on P8 host with kernel older than
>>> 4.3 because it tries by default to set the VSMT to 1.
>>>
>>> qemu-system-ppc64: Failed to set KVM's VSMT mode to 1 (errno -22)
>>> On PPC, a VM with 1 threads/core on a host with 8 threads/core requires
>>> the use of VSMT mode 1.
>>> This KVM seems to be too old to support VSMT.
>>>
>>> As this is clearly intentional, is there a way to fix migration-test?
>>
>> Hrm. I believe the argument for this was that the broken kernels were
>> old enough we didn't care. What platform are you testing on where
>> you're hitting this?
>>
>
> I'm going to propose a patch to fix this problem.
> (it was on RHEL7)
Patch sent:
[PATCH] spapr: Fix VSMT mode when it is not supported by the kernel
Message-Id: <20191108154035.12913-1-lvivier@redhat.com>
Thanks,
Laurent
© 2016 - 2026 Red Hat, Inc.