[Qemu-devel] [PATCH v1 0/2] Xilinx ZCU102 fixes for 2.11

[Qemu-devel] [PATCH v1 0/2] Xilinx ZCU102 fixes for 2.11

[Alistair Francis]

These are two small fixes for 2.11.

Alistair Francis (2):
  xlnx-zynqmp: Properly support the smp command line option
  xlnx-zcu102: Add an info message deprecating the EP108

 hw/arm/xlnx-zcu102.c |  6 +++++-
 hw/arm/xlnx-zynqmp.c | 26 ++++++++++++++++----------
 2 files changed, 21 insertions(+), 11 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH v1 2/2] xlnx-zcu102: Add an info message deprecating the EP108

[Alistair Francis]

The EP108 was an early access development board that is no longer used.
Add an info message to convert any users to the ZCU102 instead. On QEMU
they are both identical.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zcu102.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 7ec03dad42..adddd234a1 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -164,6 +164,9 @@ static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxZCU102 *s = EP108_MACHINE(machine);
 
+    info_report("The Xilinx EP108 machine is deprecated, please use the "
+                "ZCU102 machine instead. It has the same features supported.");
+
     xlnx_zynqmp_init(s, machine);
 }
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH v1 1/2] xlnx-zynqmp: Properly support the smp command line option

[Eduardo Habkost]

On Tue, Nov 07, 2017 at 10:59:39AM -0800, Alistair Francis wrote:
> Allow the -smp command line option to control the number of CPUs we
> create.
> 
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> 
>  hw/arm/xlnx-zcu102.c |  3 ++-
>  hw/arm/xlnx-zynqmp.c | 26 ++++++++++++++++----------
>  2 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index e2d15a1c9d..7ec03dad42 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -235,7 +235,8 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->desc = "Xilinx ZynqMP ZCU102 board";
> +    mc->desc = "Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5s based on " \
> +               "the value of smp";
>      mc->init = xlnx_zcu102_init;
>      mc->block_default_type = IF_IDE;
>      mc->units_per_default_bus = 1;
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index d4b6560194..c707c66322 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -98,8 +98,9 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>  {
>      Error *err = NULL;
>      int i;
> +    int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
>  
> -    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
> +    for (i = 0; i < num_rpus; i++) {
>          char *name;
>  
>          object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
> @@ -132,8 +133,9 @@ static void xlnx_zynqmp_init(Object *obj)
>  {
>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>      int i;
> +    int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>  
> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
> +    for (i = 0; i < num_apus; i++) {
>          object_initialize(&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
>                            "cortex-a53-" TYPE_ARM_CPU);
>          object_property_add_child(obj, "apu-cpu[*]", OBJECT(&s->apu_cpu[i]),
> @@ -182,6 +184,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>      MemoryRegion *system_memory = get_system_memory();
>      uint8_t i;
>      uint64_t ram_size;
> +    int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>      ram_addr_t ddr_low_size, ddr_high_size;
>      qemu_irq gic_spi[GIC_NUM_SPI_INTR];
> @@ -233,10 +236,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>  
>      qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
>      qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
> -    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
> +    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
>  
>      /* Realize APUs before realizing the GIC. KVM requires this.  */
> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
> +    for (i = 0; i < num_apus; i++) {
>          char *name;
>  
>          object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
> @@ -292,7 +295,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>          }
>      }
>  
> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
> +    for (i = 0; i < num_apus; i++) {
>          qemu_irq irq;
>  
>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
> @@ -307,11 +310,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>      }
>  
>      if (s->has_rpu) {
> -        xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
> -        if (err) {
> -            error_propagate(errp, err);
> -            return;
> -        }
> +        info_report("The 'has_rpu' property is no longer required, to use the "
> +                    "RPUs just use -smp 6.");
> +    }

Is "-global driver=xlnx,,zynqmp,property=has_rpu,value=on"
without an explicit -smp option supposed to be a supported
configuration?

0) On current master, we have this:

  $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on
  **
  ERROR:/home/ehabkost/rh/proj/virt/qemu/tcg/tcg.c:538:tcg_register_thread: assertion failed: (n < max_cpus)
  Aborted (core dumped)

1) With your patch we have this:

  $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
  QEMU 2.10.50 monitor - type 'help' for more information
  (qemu) qemu-system-aarch64: info: The 'has_rpu' property is no longer required, to use the RPUs just use -smp 6.
  (qemu) info cpus
  * CPU #0: thread_id=1662
  (qemu) 

2) With your patch plus Emilio's original min_cpus/default_cpus
proposal[1], we have this:

  $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
  QEMU 2.10.50 monitor - type 'help' for more information
  (qemu) qemu-system-aarch64: info: The 'has_rpu' property is no longer required, to use the RPUs just use -smp 6.
  (qemu) info cpus
  * CPU #0: thread_id=7112
    CPU #1: (halted) thread_id=7113
    CPU #2: (halted) thread_id=7114
    CPU #3: (halted) thread_id=7115
  (qemu)

3) With Emilio's max_additional_cpus proposal[2], we have this:

  $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
  QEMU 2.10.50 monitor - type 'help' for more information
  (qemu) info cpus
  * CPU #0: thread_id=4045
    CPU #1: (halted) thread_id=4046
    CPU #2: (halted) thread_id=4047
    CPU #3: (halted) thread_id=4048
    CPU #4: (halted) thread_id=4049
    CPU #5: (halted) thread_id=4050
  (qemu) 


Which option is preferred?  I like option #2 because it's
simpler, but I would like to confirm this is really the intended
behavior.


[1] https://mid.mail-archive.com/1509734853-3014-1-git-send-email-cota@braap.org
[2] https://mid.mail-archive.com/20171106215454.GB2152@flamenco


> +
> +    xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
>      }
>  
>      if (!s->boot_cpu_ptr) {
> -- 
> 2.11.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v1 1/2] xlnx-zynqmp: Properly support the smp command line option

[Alistair Francis]

On Tue, Nov 7, 2017 at 12:00 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Nov 07, 2017 at 10:59:39AM -0800, Alistair Francis wrote:
>> Allow the -smp command line option to control the number of CPUs we
>> create.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  hw/arm/xlnx-zcu102.c |  3 ++-
>>  hw/arm/xlnx-zynqmp.c | 26 ++++++++++++++++----------
>>  2 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
>> index e2d15a1c9d..7ec03dad42 100644
>> --- a/hw/arm/xlnx-zcu102.c
>> +++ b/hw/arm/xlnx-zcu102.c
>> @@ -235,7 +235,8 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
>>  {
>>      MachineClass *mc = MACHINE_CLASS(oc);
>>
>> -    mc->desc = "Xilinx ZynqMP ZCU102 board";
>> +    mc->desc = "Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5s based on " \
>> +               "the value of smp";
>>      mc->init = xlnx_zcu102_init;
>>      mc->block_default_type = IF_IDE;
>>      mc->units_per_default_bus = 1;
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index d4b6560194..c707c66322 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -98,8 +98,9 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>>  {
>>      Error *err = NULL;
>>      int i;
>> +    int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
>>
>> -    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
>> +    for (i = 0; i < num_rpus; i++) {
>>          char *name;
>>
>>          object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
>> @@ -132,8 +133,9 @@ static void xlnx_zynqmp_init(Object *obj)
>>  {
>>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>>      int i;
>> +    int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>>
>> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
>> +    for (i = 0; i < num_apus; i++) {
>>          object_initialize(&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
>>                            "cortex-a53-" TYPE_ARM_CPU);
>>          object_property_add_child(obj, "apu-cpu[*]", OBJECT(&s->apu_cpu[i]),
>> @@ -182,6 +184,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>>      MemoryRegion *system_memory = get_system_memory();
>>      uint8_t i;
>>      uint64_t ram_size;
>> +    int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>>      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>>      ram_addr_t ddr_low_size, ddr_high_size;
>>      qemu_irq gic_spi[GIC_NUM_SPI_INTR];
>> @@ -233,10 +236,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>>
>>      qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
>>      qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
>> -    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
>> +    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
>>
>>      /* Realize APUs before realizing the GIC. KVM requires this.  */
>> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
>> +    for (i = 0; i < num_apus; i++) {
>>          char *name;
>>
>>          object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
>> @@ -292,7 +295,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>>          }
>>      }
>>
>> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
>> +    for (i = 0; i < num_apus; i++) {
>>          qemu_irq irq;
>>
>>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
>> @@ -307,11 +310,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>>      }
>>
>>      if (s->has_rpu) {
>> -        xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
>> -        if (err) {
>> -            error_propagate(errp, err);
>> -            return;
>> -        }
>> +        info_report("The 'has_rpu' property is no longer required, to use the "
>> +                    "RPUs just use -smp 6.");
>> +    }
>
> Is "-global driver=xlnx,,zynqmp,property=has_rpu,value=on"
> without an explicit -smp option supposed to be a supported
> configuration?
>
> 0) On current master, we have this:
>
>   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on
>   **
>   ERROR:/home/ehabkost/rh/proj/virt/qemu/tcg/tcg.c:538:tcg_register_thread: assertion failed: (n < max_cpus)
>   Aborted (core dumped)
>
> 1) With your patch we have this:
>
>   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
>   QEMU 2.10.50 monitor - type 'help' for more information
>   (qemu) qemu-system-aarch64: info: The 'has_rpu' property is no longer required, to use the RPUs just use -smp 6.
>   (qemu) info cpus
>   * CPU #0: thread_id=1662
>   (qemu)
>
> 2) With your patch plus Emilio's original min_cpus/default_cpus
> proposal[1], we have this:
>
>   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
>   QEMU 2.10.50 monitor - type 'help' for more information
>   (qemu) qemu-system-aarch64: info: The 'has_rpu' property is no longer required, to use the RPUs just use -smp 6.
>   (qemu) info cpus
>   * CPU #0: thread_id=7112
>     CPU #1: (halted) thread_id=7113
>     CPU #2: (halted) thread_id=7114
>     CPU #3: (halted) thread_id=7115
>   (qemu)
>
> 3) With Emilio's max_additional_cpus proposal[2], we have this:
>
>   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
>   QEMU 2.10.50 monitor - type 'help' for more information
>   (qemu) info cpus
>   * CPU #0: thread_id=4045
>     CPU #1: (halted) thread_id=4046
>     CPU #2: (halted) thread_id=4047
>     CPU #3: (halted) thread_id=4048
>     CPU #4: (halted) thread_id=4049
>     CPU #5: (halted) thread_id=4050
>   (qemu)
>
>
> Which option is preferred?  I like option #2 because it's
> simpler, but I would like to confirm this is really the intended
> behavior.

After this patch (and the fix to TCG otherwise it seg faults) no one
should use the has_rpu property. It is ignored and will print a
message saying that it is ignored.

Users should only use the -smp option now. So option 2 is the way to go.

Thanks,
Alistair

>
>
> [1] https://mid.mail-archive.com/1509734853-3014-1-git-send-email-cota@braap.org
> [2] https://mid.mail-archive.com/20171106215454.GB2152@flamenco
>
>
>> +
>> +    xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
>> +    if (err) {
>> +        error_propagate(errp, err);
>> +        return;
>>      }
>>
>>      if (!s->boot_cpu_ptr) {
>> --
>> 2.11.0
>>
>
> --
> Eduardo

Re: [Qemu-devel] [PATCH v1 1/2] xlnx-zynqmp: Properly support the smp command line option

[Eduardo Habkost]

On Tue, Nov 07, 2017 at 12:04:18PM -0800, Alistair Francis wrote:
> On Tue, Nov 7, 2017 at 12:00 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > On Tue, Nov 07, 2017 at 10:59:39AM -0800, Alistair Francis wrote:
> >> Allow the -smp command line option to control the number of CPUs we
> >> create.
> >>
> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> >> ---
> >>
> >>  hw/arm/xlnx-zcu102.c |  3 ++-
> >>  hw/arm/xlnx-zynqmp.c | 26 ++++++++++++++++----------
> >>  2 files changed, 18 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> >> index e2d15a1c9d..7ec03dad42 100644
> >> --- a/hw/arm/xlnx-zcu102.c
> >> +++ b/hw/arm/xlnx-zcu102.c
> >> @@ -235,7 +235,8 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
> >>  {
> >>      MachineClass *mc = MACHINE_CLASS(oc);
> >>
> >> -    mc->desc = "Xilinx ZynqMP ZCU102 board";
> >> +    mc->desc = "Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5s based on " \
> >> +               "the value of smp";
> >>      mc->init = xlnx_zcu102_init;
> >>      mc->block_default_type = IF_IDE;
> >>      mc->units_per_default_bus = 1;
> >> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> >> index d4b6560194..c707c66322 100644
> >> --- a/hw/arm/xlnx-zynqmp.c
> >> +++ b/hw/arm/xlnx-zynqmp.c
> >> @@ -98,8 +98,9 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
> >>  {
> >>      Error *err = NULL;
> >>      int i;
> >> +    int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
> >>
> >> -    for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
> >> +    for (i = 0; i < num_rpus; i++) {
> >>          char *name;
> >>
> >>          object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
> >> @@ -132,8 +133,9 @@ static void xlnx_zynqmp_init(Object *obj)
> >>  {
> >>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
> >>      int i;
> >> +    int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
> >>
> >> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
> >> +    for (i = 0; i < num_apus; i++) {
> >>          object_initialize(&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
> >>                            "cortex-a53-" TYPE_ARM_CPU);
> >>          object_property_add_child(obj, "apu-cpu[*]", OBJECT(&s->apu_cpu[i]),
> >> @@ -182,6 +184,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> >>      MemoryRegion *system_memory = get_system_memory();
> >>      uint8_t i;
> >>      uint64_t ram_size;
> >> +    int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
> >>      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
> >>      ram_addr_t ddr_low_size, ddr_high_size;
> >>      qemu_irq gic_spi[GIC_NUM_SPI_INTR];
> >> @@ -233,10 +236,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> >>
> >>      qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
> >>      qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
> >> -    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
> >> +    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
> >>
> >>      /* Realize APUs before realizing the GIC. KVM requires this.  */
> >> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
> >> +    for (i = 0; i < num_apus; i++) {
> >>          char *name;
> >>
> >>          object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
> >> @@ -292,7 +295,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> >>          }
> >>      }
> >>
> >> -    for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
> >> +    for (i = 0; i < num_apus; i++) {
> >>          qemu_irq irq;
> >>
> >>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
> >> @@ -307,11 +310,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> >>      }
> >>
> >>      if (s->has_rpu) {
> >> -        xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
> >> -        if (err) {
> >> -            error_propagate(errp, err);
> >> -            return;
> >> -        }
> >> +        info_report("The 'has_rpu' property is no longer required, to use the "
> >> +                    "RPUs just use -smp 6.");
> >> +    }
> >
> > Is "-global driver=xlnx,,zynqmp,property=has_rpu,value=on"
> > without an explicit -smp option supposed to be a supported
> > configuration?
> >
> > 0) On current master, we have this:
> >
> >   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on
> >   **
> >   ERROR:/home/ehabkost/rh/proj/virt/qemu/tcg/tcg.c:538:tcg_register_thread: assertion failed: (n < max_cpus)
> >   Aborted (core dumped)
> >
> > 1) With your patch we have this:
> >
> >   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
> >   QEMU 2.10.50 monitor - type 'help' for more information
> >   (qemu) qemu-system-aarch64: info: The 'has_rpu' property is no longer required, to use the RPUs just use -smp 6.
> >   (qemu) info cpus
> >   * CPU #0: thread_id=1662
> >   (qemu)
> >
> > 2) With your patch plus Emilio's original min_cpus/default_cpus
> > proposal[1], we have this:
> >
> >   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
> >   QEMU 2.10.50 monitor - type 'help' for more information
> >   (qemu) qemu-system-aarch64: info: The 'has_rpu' property is no longer required, to use the RPUs just use -smp 6.
> >   (qemu) info cpus
> >   * CPU #0: thread_id=7112
> >     CPU #1: (halted) thread_id=7113
> >     CPU #2: (halted) thread_id=7114
> >     CPU #3: (halted) thread_id=7115
> >   (qemu)
> >
> > 3) With Emilio's max_additional_cpus proposal[2], we have this:
> >
> >   $ ./aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -global driver=xlnx,,zynqmp,property=has_rpu,value=on -monitor stdio
> >   QEMU 2.10.50 monitor - type 'help' for more information
> >   (qemu) info cpus
> >   * CPU #0: thread_id=4045
> >     CPU #1: (halted) thread_id=4046
> >     CPU #2: (halted) thread_id=4047
> >     CPU #3: (halted) thread_id=4048
> >     CPU #4: (halted) thread_id=4049
> >     CPU #5: (halted) thread_id=4050
> >   (qemu)
> >
> >
> > Which option is preferred?  I like option #2 because it's
> > simpler, but I would like to confirm this is really the intended
> > behavior.
> 
> After this patch (and the fix to TCG otherwise it seg faults) no one
> should use the has_rpu property. It is ignored and will print a
> message saying that it is ignored.
> 
> Users should only use the -smp option now. So option 2 is the way to go.

Sounds reasonable to me.

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

I just sent one small comment to the min_cpus/default_cpus patch
at [1], and I'm waiting for Richard's input on Emilio's TCG
fix[3].

[3] https://mid.mail-archive.com/20171106201332.GA2152@flamenco


> 
> Thanks,
> Alistair
> 
> >
> >
> > [1] https://mid.mail-archive.com/1509734853-3014-1-git-send-email-cota@braap.org
> > [2] https://mid.mail-archive.com/20171106215454.GB2152@flamenco
> >
> >
> >> +
> >> +    xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
> >> +    if (err) {
> >> +        error_propagate(errp, err);
> >> +        return;
> >>      }
> >>
> >>      if (!s->boot_cpu_ptr) {
> >> --
> >> 2.11.0
> >>
> >
> > --
> > Eduardo

-- 
Eduardo