On Mon, Jul 19, 2021 at 11:20:37AM +0800, Yanan Wang wrote:
> We totally have two requirements for a valid SMP configuration:
s/totally//
> the sum of "sockets * dies * cores * threads" must represent all
the product
> the possible cpus, i.e., max_cpus, and must include the initial
> present cpus, i.e., smp_cpus.
>
> We only need to ensure "sockets * dies * cores * threads == maxcpus"
> at first and then ensure "sockets * dies * cores * threads >= cpus".
Or, "maxcpus >= cpus"
> With a reasonable order of the sanity-check, we can simplify the
> error reporting code.
>
> Signed-off-by: Yanan Wang <wangyana...@huawei.com>
> ---
> hw/core/machine.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 668f0a1553..8b4d07d3fc 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -788,21 +788,6 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> cpus = cpus > 0 ? cpus : sockets * dies * cores * threads;
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - if (sockets * dies * cores * threads < cpus) {
> - g_autofree char *dies_msg = g_strdup_printf(
> - mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> - error_setg(errp, "cpu topology: "
> - "sockets (%u)%s * cores (%u) * threads (%u) < "
> - "smp_cpus (%u)",
> - sockets, dies_msg, cores, threads, cpus);
> - return;
> - }
> -
> - if (maxcpus < cpus) {
> - error_setg(errp, "maxcpus must be equal to or greater than smp");
> - return;
> - }
This may be redundant when determining a valid config, but by checking it
separately we can provide a more useful error message.
> -
> if (sockets * dies * cores * threads != maxcpus) {
> g_autofree char *dies_msg = g_strdup_printf(
> mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> @@ -814,6 +799,16 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> return;
> }
>
> + if (sockets * dies * cores * threads < cpus) {
> + g_autofree char *dies_msg = g_strdup_printf(
> + mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> + error_setg(errp, "Invalid CPU topology: "
> + "sockets (%u)%s * cores (%u) * threads (%u) < "
> + "smp_cpus (%u)",
> + sockets, dies_msg, cores, threads, cpus);
> + return;
> + }
> +
> ms->smp.cpus = cpus;
> ms->smp.sockets = sockets;
> ms->smp.dies = dies;
> --
> 2.19.1
>
I'm not sure we need this patch.
Thanks,
drew
