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

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

[Andrew Jones]

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 "
                      "supported by machine 'mach-virt' (%d)",
-                     smp_cpus, max_cpus);
+                     max_cpus, virt_max_cpus);
         exit(1);
     }
 
-- 
2.4.3

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

[Shannon Zhao]

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?    ^~~~

>                       "supported by machine 'mach-virt' (%d)",
> -                     smp_cpus, max_cpus);
> +                     max_cpus, virt_max_cpus);
>          exit(1);
>      }

-- 
Shannon

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

[Andrew Jones]

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

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

[Shannon Zhao]

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

2016年2月3日星期三，Andrew Jones <drjones@redhat.com> 写道：

> 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 <javascript:;>>
> Signed-off-by: Andrew Jones <drjones@redhat.com <javascript:;>>

Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---
>  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 "
>                       "supported by machine 'mach-virt' (%d)",
> -                     smp_cpus, max_cpus);
> +                     max_cpus, virt_max_cpus);
>          exit(1);
>      }
>
> --
> 2.4.3
>
>

[-- Attachment #2: Type: text/html, Size: 2937 bytes --]

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

[Peter Maydell]

On 5 February 2016 at 12:45, Shannon Zhao <shannon.zhao@linaro.org> wrote:
>
>
> 2016年2月3日星期三，Andrew Jones <drjones@redhat.com> 写道：
>>
>> 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>
>
> Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>
>>



Applied to target-arm.next, thanks.

-- PMM