Re: [Qemu-devel] [PATCH 03/11] hw/arm/virt: use VirtMachineState.smp_cpus

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:14 +0100
Andrew Jones <address@hidden> wrote:

> Most places we need smp_cpus we use vms->smp_cpus already. This
> cleanup makes sure we do everywhere, preparing for the removal
> of the global smp_cpus someday.
I'm not sure it's good idea to make/have smp_cpus as ARM specific
machine field.
It should be MachineState.smp_cpus since it's applicable to all machines.
I also fail to see how this patch helps at all as in the end
VirtMachineState.smp_cpus should be converted to MachineState.smp_cpus
and we'd end up changing the same places twice with this patch.
I'd rather see conversion to MachineState.smp_cpus from the beginning.
So suggesting to drop this patch if from this series if it doesn't get
in the way of removing VirtGuestInfo.


> Signed-off-by: Andrew Jones <address@hidden>
> ---
>  hw/arm/virt.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 67c0abb30b5b..18aa3672739d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -583,7 +583,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq 
> *pic)
>  
>      gicdev = qdev_create(NULL, gictype);
>      qdev_prop_set_uint32(gicdev, "revision", type);
> -    qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
> +    qdev_prop_set_uint32(gicdev, "num-cpu", vms->smp_cpus);
>      /* Note that the num-irq property counts both internal and external
>       * interrupts; there are always 32 of the former (mandated by GIC spec).
>       */
> @@ -604,7 +604,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq 
> *pic)
>       * maintenance interrupt signal to the appropriate GIC PPI inputs,
>       * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
>       */
> -    for (i = 0; i < smp_cpus; i++) {
> +    for (i = 0; i < vms->smp_cpus; i++) {
>          DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
>          int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
>          int irq;
> @@ -629,7 +629,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq 
> *pic)
>                                                       + 
> ARCH_GICV3_MAINT_IRQ));
>  
>          sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, 
> ARM_CPU_IRQ));
> -        sysbus_connect_irq(gicbusdev, i + smp_cpus,
> +        sysbus_connect_irq(gicbusdev, i + vms->smp_cpus,
>                             qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
>          sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
                                                 ^^^^^^^^
and other places in this file.

>                             qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> @@ -958,7 +958,7 @@ static void create_fw_cfg(const VirtMachineState *vms, 
> AddressSpace *as)
>      char *nodename;
>  
>      fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> +    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)vms->smp_cpus);
>  
>      nodename = g_strdup_printf("/address@hidden" PRIx64, base);
>      qemu_fdt_add_subnode(vms->fdt, nodename);
> @@ -1362,7 +1362,7 @@ static void machvirt_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    for (n = 0; n < smp_cpus; n++) {
> +    for (n = 0; n < vms->smp_cpus; n++) {
>          Object *cpuobj = object_new(typename);
>          if (!vmc->disallow_affinity_adjustment) {
>              /* Adjust MPIDR like 64-bit KVM hosts, which incorporate the
> @@ -1452,7 +1452,7 @@ static void machvirt_init(MachineState *machine)
>      create_fw_cfg(vms, &address_space_memory);
>      rom_set_fw(fw_cfg_find());
>  
> -    guest_info->smp_cpus = smp_cpus;
> +    guest_info->smp_cpus = vms->smp_cpus;
>      guest_info->fw_cfg = fw_cfg_find();
>      guest_info->memmap = vms->memmap;
>      guest_info->irqmap = vms->irqmap;
> @@ -1466,7 +1466,7 @@ static void machvirt_init(MachineState *machine)
>      vms->bootinfo.kernel_filename = machine->kernel_filename;
>      vms->bootinfo.kernel_cmdline = machine->kernel_cmdline;
>      vms->bootinfo.initrd_filename = machine->initrd_filename;
> -    vms->bootinfo.nb_cpus = smp_cpus;
> +    vms->bootinfo.nb_cpus = vms->smp_cpus;
>      vms->bootinfo.board_id = -1;
>      vms->bootinfo.loader_start = vms->memmap[VIRT_MEM].base;
>      vms->bootinfo.get_dtb = machvirt_dtb;