diff mbox

hw/arm/virt: fix max-cpus check

Message ID 1454511578-24863-1-git-send-email-drjones@redhat.com
State Superseded
Headers show

Commit Message

Andrew Jones Feb. 3, 2016, 2:59 p.m. UTC
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(-)

-- 
2.4.3

Comments

Andrew Jones Feb. 4, 2016, 9:18 a.m. UTC | #1
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
Shannon Zhao Feb. 5, 2016, 12:45 p.m. UTC | #2
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

>

>
Peter Maydell Feb. 9, 2016, 5:07 p.m. UTC | #3
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
diff mbox

Patch

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);
     }