Re: [RFC PATCH v2 5/6] hw/arm/virt-acpi-build: Add PPTT table

[Andrew Jones]

On Tue, May 18, 2021 at 06:34:08PM +0000, Salil Mehta wrote:
>  Those benefits, when vcpu pinning is used, are the same benefits
> > as for the host, which already use PPTT tables to describe topology, even
> > though hot plug isn't supported.
> 
> yes sure, you mean pinning vcpus according to the cpu topology for 
> performance?

Yup

> 
> > 
> > Now, if you're saying we should only generate tables for smp.cpus, not
> 
> Correct. This is what I thought we must be doing even now
> 
> > smp.maxcpus, because hot plug isn't supported anyway, then I see your
> > point. But, it'd be better to require smp.cpus == smp.maxcpus in our
> > smp_parse function to do that, which we've never done before, so we may
> > have trouble supporting existing command lines.
> 
> I am trying to recall, if the vcpu Hotplug is not supported then can they
> ever be different?
> 
> cpus =  (threads * cores * sockets)
> 
> static void smp_parse(MachineState *ms, QemuOpts *opts)
> {
>      [...]
> 
>         if (sockets * cores * threads != ms->smp.max_cpus) {
>             warn_report("Invalid CPU topology deprecated: "
>                         "sockets (%u) * cores (%u) * threads (%u) "
>                         "!= maxcpus (%u)",
>                         sockets, cores, threads,
>                         ms->smp.max_cpus);
>         }
>      [...]
> }
>   
> Although, above check does not exit(1) and just warns on detecting invalid
> CPU topology. Not sure why?

Hmm, not sure what code you have there. I see this in
hw/core/machine.c:smp_parse

        if (ms->smp.max_cpus < cpus) {
            error_report("maxcpus must be equal to or greater than smp");
            exit(1);
        }

        if (sockets * cores * threads != ms->smp.max_cpus) {
            error_report("Invalid CPU topology: "
                         "sockets (%u) * cores (%u) * threads (%u) "
                         "!= maxcpus (%u)",
                         sockets, cores, threads,
                         ms->smp.max_cpus);
            exit(1);
        }

> 
> Well if you think there are subtleties to support above implementation and
> we cannot do it now then sure it is your call. :)

The problem is that -smp 4,maxcpus=8 doesn't error out today, even though
it doesn't do anything. OTOH, -smp 4,cores=2 doesn't error out either, but
we're proposing that it should. Maybe we can start erroring out when
cpus != maxcpus until hot plug is supported?

Thanks,
drew