Re: [PATCH 1/2] spapr.c: do not use MachineClass::max_cpus to limit CPUs

[David Gibson]

On Thu, Apr 08, 2021 at 05:40:48PM -0300, Daniel Henrique Barboza wrote:
> Up to this patch, 'max_cpus' value is hardcoded to 1024 (commit
> 6244bb7e5811). In theory this patch would simply bump it to 2048, since
> it's the default NR_CPUS kernel setting for ppc64 servers nowadays, but
> the whole mechanic of MachineClass:max_cpus is flawed for the pSeries
> machine. The two supported accelerators, KVM and TCG, can live without
> it.
> 
> TCG guests don't have a theoretical limit. The user must be free to
> emulate as many CPUs as the hardware is capable of. And even if there
> were a limit, max_cpus is not the proper way to report it since it's a
> common value checked by SMP code in machine_smp_parse() for KVM as well.
> 
> For KVM guests, the proper way to limit KVM CPUs is by host
> configuration via NR_CPUS, not a QEMU hardcoded value. There is no
> technical reason for a pSeries QEMU guest to forcefully stay below
> NR_CPUS.
> 
> This hardcoded value also disregard hosts that might have a lower
> NR_CPUS limit, say 512. In this case, machine.c:machine_smp_parse() will
> allow a 1024 value to pass, but then kvm_init() will complain about it
> because it will exceed NR_CPUS:
> 
> Number of SMP cpus requested (1024) exceeds the maximum cpus supported
> by KVM (512)
> 
> A better 'max_cpus' value would consider host settings, but
> MachineClass::max_cpus is defined well before machine_init() and
> kvm_init(). We can't check for KVM limits because it's too soon, so we
> end up making a guess.

Well.. it's not so much that we're guessing KVM limits.  I think
max_cpus in the generic code is more about hard CPU limits which are
part of the machine architecture itself.  You're right that that
doesn't really make sense for the paravirtual PAPR machine though.

> This patch makes MachineClass:max_cpus settings innocuous by setting it
> to INT32_MAX. machine.c:machine_smp_parse() will not fail the
> verification based on max_cpus, letting kvm_init() do the checking with
> actual host settings. And TCG guests get to do whatever the hardware is
> capable of emulating.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Applied to ppc-for-6.1.

> ---
>  hw/ppc/spapr.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 73a06df3b1..d6a67da21f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4482,7 +4482,16 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>      mc->init = spapr_machine_init;
>      mc->reset = spapr_machine_reset;
>      mc->block_default_type = IF_SCSI;
> -    mc->max_cpus = 1024;
> +
> +    /*
> +     * Setting max_cpus to INT32_MAX. Both KVM and TCG max_cpus values
> +     * should be limited by the host capability instead of hardcoded.
> +     * max_cpus for KVM guests will be checked in kvm_init(), and TCG
> +     * guests are welcome to have as many CPUs as the host are capable
> +     * of emulate.
> +     */
> +    mc->max_cpus = INT32_MAX;
> +
>      mc->no_parallel = 1;
>      mc->default_boot_order = "";
>      mc->default_ram_size = 512 * MiB;

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

signature.asc
Description: PGP signature