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