Re: [Qemu-devel] [PATCH] hw/arm/virt: fix max-cpus check

[Andrew Jones <drjones@redhat.com>]

On Thu, Feb 04, 2016 at 09:54:45AM +0800, Shannon Zhao wrote:
> 
> 
> On 2016/2/3 22:59, Andrew Jones wrote:
> > mach-virt doesn't yet support hotplug, but command lines specifying
> > -smp <num>,maxcpus=<bigger-num> don't fail. Of course specifying
> > bigger-num as something bigger than the machine supports, e.g. > 8
> > on a gicv2 machine, should fail though. This fix also makes mach-
> > virt's max-cpus check truly consistent with the one in vl.c:main,
> > as the one there was already correctly checking max-cpus instead
> > of smp-cpus.
> > 
> > Reported-by: Shannon Zhao <shannon.zhao@linaro.org>
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  hw/arm/virt.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 15658f49c4e06..44bbbea92b1cf 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1013,7 +1013,7 @@ static void machvirt_init(MachineState *machine)
> >      MemoryRegion *sysmem = get_system_memory();
> >      MemoryRegion *secure_sysmem = NULL;
> >      int gic_version = vms->gic_version;
> > -    int n, max_cpus;
> > +    int n, virt_max_cpus;
> >      MemoryRegion *ram = g_new(MemoryRegion, 1);
> >      const char *cpu_model = machine->cpu_model;
> >      VirtBoardInfo *vbi;
> > @@ -1051,15 +1051,15 @@ static void machvirt_init(MachineState *machine)
> >       * many redistributors we can fit into the memory map.
> >       */
> >      if (gic_version == 3) {
> > -        max_cpus = vbi->memmap[VIRT_GIC_REDIST].size / 0x20000;
> > +        virt_max_cpus = vbi->memmap[VIRT_GIC_REDIST].size / 0x20000;
> >      } else {
> > -        max_cpus = GIC_NCPU;
> > +        virt_max_cpus = GIC_NCPU;
> >      }
> >  
> > -    if (smp_cpus > max_cpus) {
> > +    if (max_cpus > virt_max_cpus) {
> >          error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> Does it need to change the SMP?    ^~~~

If we consider max_cpus to be [the maximum] "SMP CPUs" after hotplug, then
I think it's OK. When I changed smp_cpus to max_cpus in vl.c (c00cd995),
either that was my reasoning for leaving it (or it was just overlooked :-),
but now that the messages are consistent, I should change them both if that's
the consensus.

Thanks,
drew