[Qemu-devel] [PATCH v2 0/2] hw/arm/virt: max-cpus init/check fixup

[Qemu-devel] [PATCH v2 0/2] hw/arm/virt: max-cpus init/check fixup

[Andrew Jones]

Changes from v1
. drop the RFC patch "arm_gic_common.h: add gicv2 aliases for defines" [Peter]
. add a patch to cleanup vl.c's max-cpu error message a little [Drew]
. cleanup mach-virt's new error message [Peter]
. make the error message exactly like vl's cleaned up message [Drew]
. add a comment to virt_class_init explaining that we'll further restrict
  max_cpus later [Peter]

---

Sorry to be so late getting this simple v2 out. I was trying to
get a new feature (qmp-dump-guest-memory support for arm) posted
before the 2.5 soft freeze (but missed that anyway...)

Andrew Jones (2):
  vl: trivial: minor tweaks to a max-cpu error msg
  hw/arm/virt: don't use a15memmap directly

 hw/arm/virt.c | 25 +++++++++++++++++++++----
 vl.c          |  4 ++--
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH 1/2] vl: trivial: minor tweaks to a max-cpu error msg

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 7c806a2428399..9a64b75ebbd24 100644
--- a/vl.c
+++ b/vl.c
@@ -4080,8 +4080,8 @@ int main(int argc, char **argv, char **envp)
 
     machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
     if (max_cpus > machine_class->max_cpus) {
-        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
-                "supported by machine `%s' (%d)\n", max_cpus,
+        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
+                "supported by machine '%s' (%d)\n", max_cpus,
                 machine_class->name, machine_class->max_cpus);
         exit(1);
     }
-- 
2.4.3

Re: [Qemu-devel] [PATCH 1/2] vl: trivial: minor tweaks to a max-cpu error msg

[Eduardo Habkost]

On Sun, Oct 18, 2015 at 07:35:27PM +0200, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>

If this can go through my tree, please let me know.

> ---
>  vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 7c806a2428399..9a64b75ebbd24 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4080,8 +4080,8 @@ int main(int argc, char **argv, char **envp)
>  
>      machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
>      if (max_cpus > machine_class->max_cpus) {
> -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
> -                "supported by machine `%s' (%d)\n", max_cpus,
> +        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
> +                "supported by machine '%s' (%d)\n", max_cpus,
>                  machine_class->name, machine_class->max_cpus);
>          exit(1);
>      }
> -- 
> 2.4.3
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/2] vl: trivial: minor tweaks to a max-cpu error msg

[Paolo Bonzini]

On 19/10/2015 20:51, Eduardo Habkost wrote:
> On Sun, Oct 18, 2015 at 07:35:27PM +0200, Andrew Jones wrote:
>> Signed-off-by: Andrew Jones <drjones@redhat.com>
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Acked-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> If this can go through my tree, please let me know.

Sure, why not.

Paolo

>> ---
>>  vl.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 7c806a2428399..9a64b75ebbd24 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4080,8 +4080,8 @@ int main(int argc, char **argv, char **envp)
>>  
>>      machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
>>      if (max_cpus > machine_class->max_cpus) {
>> -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
>> -                "supported by machine `%s' (%d)\n", max_cpus,
>> +        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
>> +                "supported by machine '%s' (%d)\n", max_cpus,
>>                  machine_class->name, machine_class->max_cpus);
>>          exit(1);
>>      }
>> -- 
>> 2.4.3
>>
> 

Re: [Qemu-devel] [PATCH 1/2] vl: trivial: minor tweaks to a max-cpu error msg

[Markus Armbruster]

Andrew Jones <drjones@redhat.com> writes:

> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 7c806a2428399..9a64b75ebbd24 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4080,8 +4080,8 @@ int main(int argc, char **argv, char **envp)
>  
>      machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
>      if (max_cpus > machine_class->max_cpus) {
> -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
> -                "supported by machine `%s' (%d)\n", max_cpus,
> +        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
> +                "supported by machine '%s' (%d)\n", max_cpus,
>                  machine_class->name, machine_class->max_cpus);
>          exit(1);
>      }

While you're touching this, error_report(), please.

Re: [Qemu-devel] [PATCH 1/2] vl: trivial: minor tweaks to a max-cpu error msg

[Andrew Jones]

On Tue, Oct 20, 2015 at 09:12:18AM +0200, Markus Armbruster wrote:
> Andrew Jones <drjones@redhat.com> writes:
> 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  vl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 7c806a2428399..9a64b75ebbd24 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4080,8 +4080,8 @@ int main(int argc, char **argv, char **envp)
> >  
> >      machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
> >      if (max_cpus > machine_class->max_cpus) {
> > -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
> > -                "supported by machine `%s' (%d)\n", max_cpus,
> > +        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
> > +                "supported by machine '%s' (%d)\n", max_cpus,
> >                  machine_class->name, machine_class->max_cpus);
> >          exit(1);
> >      }
> 
> While you're touching this, error_report(), please.
>

Good call. Eduardo, I'll send you a v2 now.

drew 

Re: [Qemu-devel] [PATCH 1/2] vl: trivial: minor tweaks to a max-cpu error msg

[Eduardo Habkost]

On Mon, Oct 26, 2015 at 02:00:49PM +0100, Andrew Jones wrote:
> On Tue, Oct 20, 2015 at 09:12:18AM +0200, Markus Armbruster wrote:
> > Andrew Jones <drjones@redhat.com> writes:
> > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > ---
> > >  vl.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/vl.c b/vl.c
> > > index 7c806a2428399..9a64b75ebbd24 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -4080,8 +4080,8 @@ int main(int argc, char **argv, char **envp)
> > >  
> > >      machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
> > >      if (max_cpus > machine_class->max_cpus) {
> > > -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
> > > -                "supported by machine `%s' (%d)\n", max_cpus,
> > > +        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
> > > +                "supported by machine '%s' (%d)\n", max_cpus,
> > >                  machine_class->name, machine_class->max_cpus);
> > >          exit(1);
> > >      }
> > 
> > While you're touching this, error_report(), please.
> >
> 
> Good call. Eduardo, I'll send you a v2 now.

v1 was already merged, as it was an improvement anyway. A separate patch
to use error_report() will be welcome, though.

-- 
Eduardo

[Qemu-devel] [PATCH v2 2/2] hw/arm/virt: don't use a15memmap directly

[Andrew Jones]

We should always go through VirtBoardInfo when we need the memmap.
To avoid using a15memmap directly, in this case, we need to defer
the max-cpus check from class init time to instance init time. In
class init we now use MAX_CPUMASK_BITS for max_cpus initialization,
which is the maximum QEMU supports, and also, incidentally, the
maximum KVM/gicv3 currently supports. Also, a nice side-effect of
delaying the max-cpus check is that we now get more appropriate
error messages for gicv2 machines that try to configure more than
123 cpus. Before this patch it would complain that the requested
number of cpus was greater than 123, but for gicv2 configs, it
should complain that the number is greater than 8.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4e7160ce96997..8ec346f8fda3e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -923,7 +923,7 @@ static void machvirt_init(MachineState *machine)
     qemu_irq pic[NUM_IRQS];
     MemoryRegion *sysmem = get_system_memory();
     int gic_version = vms->gic_version;
-    int n;
+    int n, max_cpus;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     const char *cpu_model = machine->cpu_model;
     VirtBoardInfo *vbi;
@@ -957,6 +957,22 @@ static void machvirt_init(MachineState *machine)
         exit(1);
     }
 
+    /* The maximum number of CPUs depends on the GIC version, or on how
+     * many redistributors we can fit into the memory map.
+     */
+    if (gic_version == 3) {
+        max_cpus = vbi->memmap[VIRT_GIC_REDIST].size / 0x20000;
+    } else {
+        max_cpus = GIC_NCPU;
+    }
+
+    if (smp_cpus > max_cpus) {
+        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+                     "supported by machine 'mach-virt' (%d)",
+                     smp_cpus, max_cpus);
+        exit(1);
+    }
+
     vbi->smp_cpus = smp_cpus;
 
     if (machine->ram_size > vbi->memmap[VIRT_MEM].size) {
@@ -1155,10 +1171,11 @@ static void virt_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM Virtual Machine",
     mc->init = machvirt_init;
-    /* Our maximum number of CPUs depends on how many redistributors
-     * we can fit into memory map
+    /* Start max_cpus at the maximum QEMU supports. We'll further restrict
+     * it later in machvirt_init, where we have more information about the
+     * configuration of the particular instance.
      */
-    mc->max_cpus = a15memmap[VIRT_GIC_REDIST].size / 0x20000;
+    mc->max_cpus = MAX_CPUMASK_BITS;
     mc->has_dynamic_sysbus = true;
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 0/2] hw/arm/virt: max-cpus init/check fixup

[Laszlo Ersek]

On 10/18/15 19:35, Andrew Jones wrote:
> Changes from v1
> . drop the RFC patch "arm_gic_common.h: add gicv2 aliases for defines" [Peter]
> . add a patch to cleanup vl.c's max-cpu error message a little [Drew]
> . cleanup mach-virt's new error message [Peter]
> . make the error message exactly like vl's cleaned up message [Drew]
> . add a comment to virt_class_init explaining that we'll further restrict
>   max_cpus later [Peter]
> 
> ---
> 
> Sorry to be so late getting this simple v2 out. I was trying to
> get a new feature (qmp-dump-guest-memory support for arm) posted
> before the 2.5 soft freeze (but missed that anyway...)

(off-topic)

When you do that, please consider extending the
"scripts/dump-guest-memory.py" script as well.

(This is not even a "nice to have" request -- the script already doesn't
support all the target arches on which the QMP command is available (it
only supports x86_64), so nothing is lost if you don't extend the
script. My only goal is to put the thought in your mind; you decide :))

Thanks!
Laszlo

> 
> Andrew Jones (2):
>   vl: trivial: minor tweaks to a max-cpu error msg
>   hw/arm/virt: don't use a15memmap directly
> 
>  hw/arm/virt.c | 25 +++++++++++++++++++++----
>  vl.c          |  4 ++--
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 0/2] hw/arm/virt: max-cpus init/check fixup

[Peter Maydell]

On 18 October 2015 at 18:35, Andrew Jones <drjones@redhat.com> wrote:
> Changes from v1
> . drop the RFC patch "arm_gic_common.h: add gicv2 aliases for defines" [Peter]
> . add a patch to cleanup vl.c's max-cpu error message a little [Drew]
> . cleanup mach-virt's new error message [Peter]
> . make the error message exactly like vl's cleaned up message [Drew]
> . add a comment to virt_class_init explaining that we'll further restrict
>   max_cpus later [Peter]
>
> ---
>
> Sorry to be so late getting this simple v2 out. I was trying to
> get a new feature (qmp-dump-guest-memory support for arm) posted
> before the 2.5 soft freeze (but missed that anyway...)
>
> Andrew Jones (2):
>   vl: trivial: minor tweaks to a max-cpu error msg
>   hw/arm/virt: don't use a15memmap directly

Thanks; applied patch 2/2 to target-arm.next, since Eduardo said
he would take 1/2 through his tree.

-- PMM