Re: [Qemu-devel] [Qemu-trivial] [PATCH] vl: remove (max_cpus > 255) check from smp_parse

[Eduardo Habkost <ehabkost@redhat.com>]

On Tue, Dec 03, 2013 at 02:30:48PM +0100, Andreas Färber wrote:
> Am 03.12.2013 00:03, schrieb Alexey Kardashevskiy:
> > On 12/03/2013 09:09 AM, Andreas Färber wrote:
> >> Am 02.12.2013 18:06, schrieb Michael Tokarev:
> >>> 25.11.2013 07:39, Alexey Kardashevskiy wrote:
> >>>> Since modern POWER7/POWER8 chips can have more that 256 CPU threads
> >>>> (>2000 actually), remove this check from smp_parse.
> >>>>
> >>>> The CPUs number is still checked against machine->max_cpus and this check
> >>>> should be enough not to break other archs.
> >>
> >> "should be" is not exactly the highest level of confidence for a
> >> "trivial" patch... :/
> [...]
> >> Alexey, did you actually check that, e.g., x86 machines don't break with
> >> 256 or 257 CPUs now?
> > 
> > PC_DEFAULT_MACHINE_OPTIONS sets it to 255. And I cannot find any machine
> > which would not define max_cpus, have I missed any?
> 
> If you've actually *checked* the other machines' code then fine with me,
> just say so in the commit message. :)

I just grepped for "max_cpus" and checked every match. The largest
values I found were:

hw/ppc/spapr.c: 256
s390: 255
pc: 255

All the rest had values <= 32.

Machines with missing max_cpus value shouldn't be a problem, as
max_cpus==0 is interpreted as 1 by the vl.c code.

But we still need to add a check for max_cpus > machine->max_cpus to
vl.c, before we eliminate the smp_parse() check.

There's also this, at main():

        if (i == nb_numa_nodes) {
            for (i = 0; i < max_cpus; i++) {
                set_bit(i, node_cpumask[i % nb_numa_nodes]);
            }
        }

node_cpumask[] is initialized using bitmap_new(MAX_CPUMASK_BITS), and
MAX_CPUMASK_BITS is 255. To fix this, we can initialize node_cpumask[] using
max_cpus instead, if we initialize it after smp_parse().

-- 
Eduardo