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

[Andrew Jones]

On Thu, Dec 15, 2016 at 11:37:06AM +0100, Igor Mammedov wrote:
> 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.

All good points. Dropping this patch.

Thanks,
drew

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