[Qemu-devel] [PATCH 00/11] Remove VirtGuestInfo

[Qemu-devel] [PATCH 00/11] Remove VirtGuestInfo

[Andrew Jones]

This series is based on Peter's qemu-arm gicv3-virt branch. It's
main goal (patches 04-10), which was suggested by Eduardo, is to
remove an unnecessary structure, VirtGuestInfo, which is a
maintenance burden, as it requires duplicating Virt machine state.
Additionally patches 01-03 do some mach-virt cleanups and patch 11
adds the ACPI equivalent to one of the DT patches in Peter's
gicv3-virt branch.

The last patch isn't really tested as the guest kernel versions I
used seem to ignore the GTDT irqflags and either just choose edge,
4.5-based kernel, or level, 4.9-based kernel.

Patches available here
https://github.com/rhdrjones/qemu/commits/virt/remove-guest-info


Andrew Jones (11):
  hw/arm/virt: parameter passing cleanups
  hw/arm/virt: use VirtMachineState.gic_version
  hw/arm/virt: use VirtMachineState.smp_cpus
  hw/arm/virt: eliminate struct VirtGuestInfoState
  hw/arm/virt: remove include/hw/arm/virt-acpi-build.h
  hw/arm/virt: move VirtMachineState/Class to virt.h
  hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo
  hw/arm/virt-acpi-build: remove redundant members from VirtGuestInfo
  hw/arm/virt-acpi-build: don't save VirtGuestInfo on AcpiBuildState
  hw/arm/virt: remove VirtGuestInfo
  hw/arm/virt-acpi-build: Don't incorrectly claim architectural timer to
    be edge-triggered

 MAINTAINERS                      |   2 -
 hw/arm/virt-acpi-build.c         | 103 +++++++++++++-------------
 hw/arm/virt.c                    | 152 ++++++++++++++-------------------------
 include/hw/arm/virt-acpi-build.h |  47 ------------
 include/hw/arm/virt.h            |  45 +++++++++++-
 5 files changed, 151 insertions(+), 198 deletions(-)
 delete mode 100644 include/hw/arm/virt-acpi-build.h

-- 
2.9.3

[Qemu-devel] [PATCH 01/11] hw/arm/virt: parameter passing cleanups

[Andrew Jones]

Some simple cleanups made possible by "hw/arm/virt: Merge
VirtBoardInfo and VirtMachineState"

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7eec50a82494..d451bc4f6b9b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -570,10 +570,10 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
     fdt_add_v2m_gic_node(vms);
 }
 
-static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type,
-                       bool secure, bool no_its)
+static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type)
 {
     /* We create a standalone GIC */
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     DeviceState *gicdev;
     SysBusDevice *gicbusdev;
     const char *gictype;
@@ -589,7 +589,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type,
      */
     qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
     if (!kvm_irqchip_in_kernel()) {
-        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
+        qdev_prop_set_bit(gicdev, "has-security-extensions", vms->secure);
     }
     qdev_init_nofail(gicdev);
     gicbusdev = SYS_BUS_DEVICE(gicdev);
@@ -643,7 +643,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type,
 
     fdt_add_gic_node(vms, type);
 
-    if (type == 3 && !no_its) {
+    if (type == 3 && !vmc->no_its) {
         create_its(vms, gicdev);
     } else if (type == 2) {
         create_v2m(vms, pic);
@@ -1005,8 +1005,7 @@ static void create_pcie_irq_map(const VirtMachineState *vms,
                            0x7           /* PCI irq */);
 }
 
-static void create_pcie(const VirtMachineState *vms, qemu_irq *pic,
-                        bool use_highmem)
+static void create_pcie(const VirtMachineState *vms, qemu_irq *pic)
 {
     hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base;
     hwaddr size_mmio = vms->memmap[VIRT_PCIE_MMIO].size;
@@ -1049,7 +1048,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic,
                              mmio_reg, base_mmio, size_mmio);
     memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
 
-    if (use_highmem) {
+    if (vms->highmem) {
         /* Map high MMIO space */
         MemoryRegion *high_mmio_alias = g_new0(MemoryRegion, 1);
 
@@ -1098,7 +1097,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic,
     qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
                                  2, base_ecam, 2, size_ecam);
 
-    if (use_highmem) {
+    if (vms->highmem) {
         qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "ranges",
                                      1, FDT_PCI_RANGE_IOPORT, 2, 0,
                                      2, base_pio, 2, size_pio,
@@ -1428,7 +1427,7 @@ static void machvirt_init(MachineState *machine)
 
     create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
 
-    create_gic(vms, pic, gic_version, vms->secure, vmc->no_its);
+    create_gic(vms, pic, gic_version);
 
     fdt_add_pmu_nodes(vms, gic_version);
 
@@ -1441,7 +1440,7 @@ static void machvirt_init(MachineState *machine)
 
     create_rtc(vms, pic);
 
-    create_pcie(vms, pic, vms->highmem);
+    create_pcie(vms, pic);
 
     create_gpio(vms, pic);
 
-- 
2.9.3

[Qemu-devel] [PATCH 02/11] hw/arm/virt: use VirtMachineState.gic_version

[Andrew Jones]

machvirt_init may need to probe for the gic version. If so, then
make sure the result is written to VirtMachineState. With the
state up to date, use it instead of a local variable. This is a
cleanup that prepares for VirtMachineState to be passed to functions
even outside hw/arm/virt.c

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d451bc4f6b9b..67c0abb30b5b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -319,7 +319,7 @@ static void fdt_add_psci_node(const VirtMachineState *vms)
     qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
 }
 
-static void fdt_add_timer_nodes(const VirtMachineState *vms, int gictype)
+static void fdt_add_timer_nodes(const VirtMachineState *vms)
 {
     /* On real hardware these interrupts are level-triggered.
      * On KVM they were edge-triggered before host kernel version 4.4,
@@ -347,7 +347,7 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms, int gictype)
         irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
     }
 
-    if (gictype == 2) {
+    if (vms->gic_version == 2) {
         irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
                              GIC_FDT_IRQ_PPI_CPU_WIDTH,
                              (1 << vms->smp_cpus) - 1);
@@ -462,7 +462,7 @@ static void fdt_add_v2m_gic_node(VirtMachineState *vms)
     qemu_fdt_setprop_cell(vms->fdt, "/intc/v2m", "phandle", vms->msi_phandle);
 }
 
-static void fdt_add_gic_node(VirtMachineState *vms, int type)
+static void fdt_add_gic_node(VirtMachineState *vms)
 {
     vms->gic_phandle = qemu_fdt_alloc_phandle(vms->fdt);
     qemu_fdt_setprop_cell(vms->fdt, "/", "interrupt-parent", vms->gic_phandle);
@@ -473,7 +473,7 @@ static void fdt_add_gic_node(VirtMachineState *vms, int type)
     qemu_fdt_setprop_cell(vms->fdt, "/intc", "#address-cells", 0x2);
     qemu_fdt_setprop_cell(vms->fdt, "/intc", "#size-cells", 0x2);
     qemu_fdt_setprop(vms->fdt, "/intc", "ranges", NULL, 0);
-    if (type == 3) {
+    if (vms->gic_version == 3) {
         qemu_fdt_setprop_string(vms->fdt, "/intc", "compatible",
                                 "arm,gic-v3");
         qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg",
@@ -500,7 +500,7 @@ static void fdt_add_gic_node(VirtMachineState *vms, int type)
     qemu_fdt_setprop_cell(vms->fdt, "/intc", "phandle", vms->gic_phandle);
 }
 
-static void fdt_add_pmu_nodes(const VirtMachineState *vms, int gictype)
+static void fdt_add_pmu_nodes(const VirtMachineState *vms)
 {
     CPUState *cpu;
     ARMCPU *armcpu;
@@ -514,7 +514,7 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms, int gictype)
         }
     }
 
-    if (gictype == 2) {
+    if (vms->gic_version == 2) {
         irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
                              GIC_FDT_IRQ_PPI_CPU_WIDTH,
                              (1 << vms->smp_cpus) - 1);
@@ -570,14 +570,14 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
     fdt_add_v2m_gic_node(vms);
 }
 
-static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type)
+static void create_gic(VirtMachineState *vms, qemu_irq *pic)
 {
     /* We create a standalone GIC */
     VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     DeviceState *gicdev;
     SysBusDevice *gicbusdev;
     const char *gictype;
-    int i;
+    int type = vms->gic_version, i;
 
     gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
 
@@ -641,7 +641,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type)
         pic[i] = qdev_get_gpio_in(gicdev, i);
     }
 
-    fdt_add_gic_node(vms, type);
+    fdt_add_gic_node(vms);
 
     if (type == 3 && !vmc->no_its) {
         create_its(vms, gicdev);
@@ -1237,7 +1237,6 @@ static void machvirt_init(MachineState *machine)
     qemu_irq pic[NUM_IRQS];
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *secure_sysmem = NULL;
-    int gic_version = vms->gic_version;
     int n, virt_max_cpus;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     const char *cpu_model = machine->cpu_model;
@@ -1258,14 +1257,14 @@ static void machvirt_init(MachineState *machine)
     /* We can probe only here because during property set
      * KVM is not available yet
      */
-    if (!gic_version) {
+    if (!vms->gic_version) {
         if (!kvm_enabled()) {
             error_report("gic-version=host requires KVM");
             exit(1);
         }
 
-        gic_version = kvm_arm_vgic_probe();
-        if (!gic_version) {
+        vms->gic_version = kvm_arm_vgic_probe();
+        if (!vms->gic_version) {
             error_report("Unable to determine GIC version supported by host");
             exit(1);
         }
@@ -1300,7 +1299,7 @@ static void machvirt_init(MachineState *machine)
     /* 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) {
+    if (vms->gic_version == 3) {
         virt_max_cpus = vms->memmap[VIRT_GIC_REDIST].size / 0x20000;
         clustersz = GICV3_TARGETLIST_BITS;
     } else {
@@ -1417,7 +1416,7 @@ static void machvirt_init(MachineState *machine)
 
         object_property_set_bool(cpuobj, true, "realized", NULL);
     }
-    fdt_add_timer_nodes(vms, gic_version);
+    fdt_add_timer_nodes(vms);
     fdt_add_cpu_nodes(vms);
     fdt_add_psci_node(vms);
 
@@ -1427,9 +1426,9 @@ static void machvirt_init(MachineState *machine)
 
     create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
 
-    create_gic(vms, pic, gic_version);
+    create_gic(vms, pic);
 
-    fdt_add_pmu_nodes(vms, gic_version);
+    fdt_add_pmu_nodes(vms);
 
     create_uart(vms, pic, VIRT_UART, sysmem, serial_hds[0]);
 
@@ -1458,7 +1457,7 @@ static void machvirt_init(MachineState *machine)
     guest_info->memmap = vms->memmap;
     guest_info->irqmap = vms->irqmap;
     guest_info->use_highmem = vms->highmem;
-    guest_info->gic_version = gic_version;
+    guest_info->gic_version = vms->gic_version;
     guest_info->no_its = vmc->no_its;
     guest_info_state->machine_done.notify = virt_guest_info_machine_done;
     qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
-- 
2.9.3

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

[Andrew Jones]

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.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 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,
                            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("/fw-cfg@%" 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;
-- 
2.9.3

[Qemu-devel] [PATCH 04/11] hw/arm/virt: eliminate struct VirtGuestInfoState

[Andrew Jones]

Instead of allocating a new struct just for VirtGuestInfo and the
machine_done Notifier, place them inside VirtMachineState. This
is the mach-virt equivalent of "pc: Eliminate struct
PcGuestInfoState"

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c                    | 20 +++++++++++---------
 include/hw/arm/virt-acpi-build.h |  6 ------
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 18aa3672739d..0657f9ae8722 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -76,6 +76,8 @@ typedef struct {
 
 typedef struct {
     MachineState parent;
+    VirtGuestInfo acpi_guest_info;
+    Notifier machine_done;
     bool secure;
     bool highmem;
     bool virt;
@@ -1222,12 +1224,13 @@ static void virt_build_smbios(VirtGuestInfo *guest_info)
 }
 
 static
-void virt_guest_info_machine_done(Notifier *notifier, void *data)
+void virt_machine_done(Notifier *notifier, void *data)
 {
-    VirtGuestInfoState *guest_info_state = container_of(notifier,
-                                              VirtGuestInfoState, machine_done);
-    virt_acpi_setup(&guest_info_state->info);
-    virt_build_smbios(&guest_info_state->info);
+    VirtMachineState *vms = container_of(notifier, VirtMachineState,
+                                         machine_done);
+
+    virt_acpi_setup(&vms->acpi_guest_info);
+    virt_build_smbios(&vms->acpi_guest_info);
 }
 
 static void machvirt_init(MachineState *machine)
@@ -1240,8 +1243,7 @@ static void machvirt_init(MachineState *machine)
     int n, virt_max_cpus;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     const char *cpu_model = machine->cpu_model;
-    VirtGuestInfoState *guest_info_state = g_malloc0(sizeof *guest_info_state);
-    VirtGuestInfo *guest_info = &guest_info_state->info;
+    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
     char **cpustr;
     ObjectClass *oc;
     const char *typename;
@@ -1459,8 +1461,8 @@ static void machvirt_init(MachineState *machine)
     guest_info->use_highmem = vms->highmem;
     guest_info->gic_version = vms->gic_version;
     guest_info->no_its = vmc->no_its;
-    guest_info_state->machine_done.notify = virt_guest_info_machine_done;
-    qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
+    vms->machine_done.notify = virt_machine_done;
+    qemu_add_machine_init_done_notifier(&vms->machine_done);
 
     vms->bootinfo.ram_size = machine->ram_size;
     vms->bootinfo.kernel_filename = machine->kernel_filename;
diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
index f5ec749b8fea..d74d7a1af650 100644
--- a/include/hw/arm/virt-acpi-build.h
+++ b/include/hw/arm/virt-acpi-build.h
@@ -36,12 +36,6 @@ typedef struct VirtGuestInfo {
     bool no_its;
 } VirtGuestInfo;
 
-
-typedef struct VirtGuestInfoState {
-    VirtGuestInfo info;
-    Notifier machine_done;
-} VirtGuestInfoState;
-
 void virt_acpi_setup(VirtGuestInfo *guest_info);
 
 #endif
-- 
2.9.3

[Qemu-devel] [PATCH 05/11] hw/arm/virt: remove include/hw/arm/virt-acpi-build.h

[Andrew Jones]

include/hw/arm/virt-acpi-build.h is only used for VirtGuestInfo,
which doesn't even necessarily have to be ACPI specific. Move
VirtGuestInfo to include/hw/arm/virt.h, allowing us to remove
include/hw/arm/virt-acpi-build.h, and to prepare for even more
code motion.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 MAINTAINERS                      |  2 --
 hw/arm/virt-acpi-build.c         |  2 +-
 hw/arm/virt.c                    |  1 -
 include/hw/arm/virt-acpi-build.h | 41 ----------------------------------------
 include/hw/arm/virt.h            | 16 +++++++++++++++-
 5 files changed, 16 insertions(+), 46 deletions(-)
 delete mode 100644 include/hw/arm/virt-acpi-build.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 4a605791fc98..465b9f0f8440 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -508,7 +508,6 @@ M: Shannon Zhao <shannon.zhao@linaro.org>
 L: qemu-arm@nongnu.org
 S: Maintained
 F: hw/arm/virt-acpi-build.c
-F: include/hw/arm/virt-acpi-build.h
 
 STM32F205
 M: Alistair Francis <alistair@alistair23.me>
@@ -885,7 +884,6 @@ F: hw/acpi/*
 F: hw/smbios/*
 F: hw/i386/acpi-build.[hc]
 F: hw/arm/virt-acpi-build.c
-F: include/hw/arm/virt-acpi-build.h
 
 ppc4xx
 M: Alexander Graf <agraf@suse.de>
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index d4160dfa7d34..db44e2dd5f12 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -29,7 +29,6 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
-#include "hw/arm/virt-acpi-build.h"
 #include "qemu/bitmap.h"
 #include "trace.h"
 #include "qom/cpu.h"
@@ -43,6 +42,7 @@
 #include "hw/acpi/aml-build.h"
 #include "hw/pci/pcie_host.h"
 #include "hw/pci/pci.h"
+#include "hw/arm/virt.h"
 #include "sysemu/numa.h"
 #include "kvm_arm.h"
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0657f9ae8722..b137d9bbb689 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -48,7 +48,6 @@
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "hw/pci-host/gpex.h"
-#include "hw/arm/virt-acpi-build.h"
 #include "hw/arm/sysbus-fdt.h"
 #include "hw/platform-bus.h"
 #include "hw/arm/fdt.h"
diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
deleted file mode 100644
index d74d7a1af650..000000000000
--- a/include/hw/arm/virt-acpi-build.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- *
- * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
- *
- * Author: Shannon Zhao <zhaoshenglong@huawei.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2 or later, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef QEMU_VIRT_ACPI_BUILD_H
-#define QEMU_VIRT_ACPI_BUILD_H
-
-#include "qemu-common.h"
-#include "hw/arm/virt.h"
-#include "qemu/notify.h"
-
-#define ACPI_GICC_ENABLED 1
-
-typedef struct VirtGuestInfo {
-    int smp_cpus;
-    FWCfgState *fw_cfg;
-    const MemMapEntry *memmap;
-    const int *irqmap;
-    bool use_highmem;
-    int gic_version;
-    bool no_its;
-} VirtGuestInfo;
-
-void virt_acpi_setup(VirtGuestInfo *guest_info);
-
-#endif
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index b805b7622834..c2c1f6837442 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -32,6 +32,7 @@
 
 #include "qemu-common.h"
 #include "exec/hwaddr.h"
+#include "qemu/notify.h"
 
 #define NUM_GICV2M_SPIS       64
 #define NUM_VIRTIO_TRANSPORTS 32
@@ -47,6 +48,8 @@
 
 #define PPI(irq) ((irq) + 16)
 
+#define ACPI_GICC_ENABLED 1
+
 enum {
     VIRT_FLASH,
     VIRT_MEM,
@@ -76,5 +79,16 @@ typedef struct MemMapEntry {
     hwaddr size;
 } MemMapEntry;
 
+typedef struct VirtGuestInfo {
+    int smp_cpus;
+    FWCfgState *fw_cfg;
+    const MemMapEntry *memmap;
+    const int *irqmap;
+    bool use_highmem;
+    int gic_version;
+    bool no_its;
+} VirtGuestInfo;
+
+void virt_acpi_setup(VirtGuestInfo *guest_info);
 
-#endif
+#endif /* QEMU_ARM_VIRT_H */
-- 
2.9.3

[Qemu-devel] [PATCH 06/11] hw/arm/virt: move VirtMachineState/Class to virt.h

[Andrew Jones]

In preparation to share more Virt machine state than just guest-info
with other mach-virt source files, move the State and Class structures
to virt.h

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c         | 51 +++++++--------------------------------------------
 include/hw/arm/virt.h | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b137d9bbb689..7860fade52f1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -58,50 +58,6 @@
 #include "qapi/visitor.h"
 #include "standard-headers/linux/input.h"
 
-/* Number of external interrupt lines to configure the GIC with */
-#define NUM_IRQS 256
-
-#define PLATFORM_BUS_NUM_IRQS 64
-
-static ARMPlatformBusSystemParams platform_bus_params;
-
-typedef struct {
-    MachineClass parent;
-    bool disallow_affinity_adjustment;
-    bool no_its;
-    bool no_pmu;
-    bool claim_edge_triggered_timers;
-} VirtMachineClass;
-
-typedef struct {
-    MachineState parent;
-    VirtGuestInfo acpi_guest_info;
-    Notifier machine_done;
-    bool secure;
-    bool highmem;
-    bool virt;
-    int32_t gic_version;
-    struct arm_boot_info bootinfo;
-    const MemMapEntry *memmap;
-    const int *irqmap;
-    int smp_cpus;
-    void *fdt;
-    int fdt_size;
-    uint32_t clock_phandle;
-    uint32_t gic_phandle;
-    uint32_t msi_phandle;
-    int psci_conduit;
-} VirtMachineState;
-
-#define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
-#define VIRT_MACHINE(obj) \
-    OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
-#define VIRT_MACHINE_GET_CLASS(obj) \
-    OBJECT_GET_CLASS(VirtMachineClass, obj, TYPE_VIRT_MACHINE)
-#define VIRT_MACHINE_CLASS(klass) \
-    OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
-
-
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
                                                     void *data) \
@@ -131,6 +87,13 @@ typedef struct {
     DEFINE_VIRT_MACHINE_LATEST(major, minor, false)
 
 
+/* Number of external interrupt lines to configure the GIC with */
+#define NUM_IRQS 256
+
+#define PLATFORM_BUS_NUM_IRQS 64
+
+static ARMPlatformBusSystemParams platform_bus_params;
+
 /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
  * RAM can go up to the 256GB mark, leaving 256GB of the physical
  * address space unallocated and free for future use between 256G and 512G.
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index c2c1f6837442..77d25dbddb62 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -33,6 +33,8 @@
 #include "qemu-common.h"
 #include "exec/hwaddr.h"
 #include "qemu/notify.h"
+#include "hw/boards.h"
+#include "hw/arm/arm.h"
 
 #define NUM_GICV2M_SPIS       64
 #define NUM_VIRTIO_TRANSPORTS 32
@@ -89,6 +91,43 @@ typedef struct VirtGuestInfo {
     bool no_its;
 } VirtGuestInfo;
 
+typedef struct {
+    MachineClass parent;
+    bool disallow_affinity_adjustment;
+    bool no_its;
+    bool no_pmu;
+    bool claim_edge_triggered_timers;
+} VirtMachineClass;
+
+typedef struct {
+    MachineState parent;
+    VirtGuestInfo acpi_guest_info;
+    Notifier machine_done;
+    bool secure;
+    bool highmem;
+    bool virt;
+    int32_t gic_version;
+    struct arm_boot_info bootinfo;
+    const MemMapEntry *memmap;
+    const int *irqmap;
+    int smp_cpus;
+    void *fdt;
+    int fdt_size;
+    uint32_t clock_phandle;
+    uint32_t gic_phandle;
+    uint32_t msi_phandle;
+    int psci_conduit;
+} VirtMachineState;
+
+#define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
+#define VIRT_MACHINE(obj) \
+    OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
+#define VIRT_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(VirtMachineClass, obj, TYPE_VIRT_MACHINE)
+#define VIRT_MACHINE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
+
+
 void virt_acpi_setup(VirtGuestInfo *guest_info);
 
 #endif /* QEMU_ARM_VIRT_H */
-- 
2.9.3

[Qemu-devel] [PATCH 07/11] hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo

[Andrew Jones]

Only two functions take VirtGuestInfo parameters. Now that guest-info
is part of VirtMachineState, and VirtMachineState is defined in the
virt header, pass that instead.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt-acpi-build.c | 3 ++-
 hw/arm/virt.c            | 8 ++++----
 include/hw/arm/virt.h    | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index db44e2dd5f12..f7ce6aeed307 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -822,8 +822,9 @@ static const VMStateDescription vmstate_virt_acpi_build = {
     },
 };
 
-void virt_acpi_setup(VirtGuestInfo *guest_info)
+void virt_acpi_setup(VirtMachineState *vms)
 {
+    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
     AcpiBuildTables tables;
     AcpiBuildState *build_state;
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7860fade52f1..bcf8a2954c16 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1156,9 +1156,9 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
     return board->fdt;
 }
 
-static void virt_build_smbios(VirtGuestInfo *guest_info)
+static void virt_build_smbios(VirtMachineState *vms)
 {
-    FWCfgState *fw_cfg = guest_info->fw_cfg;
+    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";
@@ -1191,8 +1191,8 @@ void virt_machine_done(Notifier *notifier, void *data)
     VirtMachineState *vms = container_of(notifier, VirtMachineState,
                                          machine_done);
 
-    virt_acpi_setup(&vms->acpi_guest_info);
-    virt_build_smbios(&vms->acpi_guest_info);
+    virt_acpi_setup(vms);
+    virt_build_smbios(vms);
 }
 
 static void machvirt_init(MachineState *machine)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 77d25dbddb62..cbedc682ae99 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -128,6 +128,6 @@ typedef struct {
     OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
 
 
-void virt_acpi_setup(VirtGuestInfo *guest_info);
+void virt_acpi_setup(VirtMachineState *vms);
 
 #endif /* QEMU_ARM_VIRT_H */
-- 
2.9.3

[Qemu-devel] [PATCH 08/11] hw/arm/virt-acpi-build: remove redundant members from VirtGuestInfo

[Andrew Jones]

Now that we pass VirtMachineState, and guest-info is just part of
that state, we can remove all the redundant members and access
the VirtMachineState directly.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt-acpi-build.c | 72 +++++++++++++++++++++++++-----------------------
 hw/arm/virt.c            |  6 ----
 include/hw/arm/virt.h    |  6 ----
 3 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f7ce6aeed307..7a1efd0dc538 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -384,7 +384,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
 }
 
 static void
-build_iort(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
+build_iort(GArray *table_data, BIOSLinker *linker)
 {
     int iort_start = table_data->len;
     AcpiIortIdMapping *idmap;
@@ -439,11 +439,11 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 }
 
 static void
-build_spcr(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
+build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
     AcpiSerialPortConsoleRedirection *spcr;
-    const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
-    int irq = guest_info->irqmap[VIRT_UART] + ARM_SPI_BASE;
+    const MemMapEntry *uart_memmap = &vms->memmap[VIRT_UART];
+    int irq = vms->irqmap[VIRT_UART] + ARM_SPI_BASE;
 
     spcr = acpi_data_push(table_data, sizeof(*spcr));
 
@@ -472,16 +472,16 @@ build_spcr(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 }
 
 static void
-build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
+build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
     AcpiSystemResourceAffinityTable *srat;
     AcpiSratProcessorGiccAffinity *core;
     AcpiSratMemoryAffinity *numamem;
     int i, j, srat_start;
     uint64_t mem_base;
-    uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof(uint32_t));
+    uint32_t *cpu_node = g_malloc0(vms->smp_cpus * sizeof(uint32_t));
 
-    for (i = 0; i < guest_info->smp_cpus; i++) {
+    for (i = 0; i < vms->smp_cpus; i++) {
         j = numa_get_node_for_cpu(i);
         if (j < nb_numa_nodes) {
                 cpu_node[i] = j;
@@ -492,7 +492,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
     srat = acpi_data_push(table_data, sizeof(*srat));
     srat->reserved1 = cpu_to_le32(1);
 
-    for (i = 0; i < guest_info->smp_cpus; ++i) {
+    for (i = 0; i < vms->smp_cpus; ++i) {
         core = acpi_data_push(table_data, sizeof(*core));
         core->type = ACPI_SRAT_PROCESSOR_GICC;
         core->length = sizeof(*core);
@@ -502,7 +502,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
     }
     g_free(cpu_node);
 
-    mem_base = guest_info->memmap[VIRT_MEM].base;
+    mem_base = vms->memmap[VIRT_MEM].base;
     for (i = 0; i < nb_numa_nodes; ++i) {
         numamem = acpi_data_push(table_data, sizeof(*numamem));
         build_srat_memory(numamem, mem_base, numa_info[i].node_mem, i,
@@ -515,10 +515,10 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
 }
 
 static void
-build_mcfg(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
+build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
     AcpiTableMcfg *mcfg;
-    const MemMapEntry *memmap = guest_info->memmap;
+    const MemMapEntry *memmap = vms->memmap;
     int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
 
     mcfg = acpi_data_push(table_data, len);
@@ -561,11 +561,12 @@ build_gtdt(GArray *table_data, BIOSLinker *linker)
 
 /* MADT */
 static void
-build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
+build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     int madt_start = table_data->len;
-    const MemMapEntry *memmap = guest_info->memmap;
-    const int *irqmap = guest_info->irqmap;
+    const MemMapEntry *memmap = vms->memmap;
+    const int *irqmap = vms->irqmap;
     AcpiMultipleApicTable *madt;
     AcpiMadtGenericDistributor *gicd;
     AcpiMadtGenericMsiFrame *gic_msi;
@@ -577,16 +578,16 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
     gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR;
     gicd->length = sizeof(*gicd);
     gicd->base_address = memmap[VIRT_GIC_DIST].base;
-    gicd->version = guest_info->gic_version;
+    gicd->version = vms->gic_version;
 
-    for (i = 0; i < guest_info->smp_cpus; i++) {
+    for (i = 0; i < vms->smp_cpus; i++) {
         AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,
                                                      sizeof *gicc);
         ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
 
         gicc->type = ACPI_APIC_GENERIC_INTERRUPT;
         gicc->length = sizeof(*gicc);
-        if (guest_info->gic_version == 2) {
+        if (vms->gic_version == 2) {
             gicc->base_address = memmap[VIRT_GIC_CPU].base;
         }
         gicc->cpu_interface_number = i;
@@ -599,7 +600,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
         }
     }
 
-    if (guest_info->gic_version == 3) {
+    if (vms->gic_version == 3) {
         AcpiMadtGenericTranslator *gic_its;
         AcpiMadtGenericRedistributor *gicr = acpi_data_push(table_data,
                                                          sizeof *gicr);
@@ -609,7 +610,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
         gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
         gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
 
-        if (its_class_name() && !guest_info->no_its) {
+        if (its_class_name() && !vmc->no_its) {
             gic_its = acpi_data_push(table_data, sizeof *gic_its);
             gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
             gic_its->length = sizeof(*gic_its);
@@ -658,11 +659,11 @@ build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt_tbl_offset)
 
 /* DSDT */
 static void
-build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
+build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
     Aml *scope, *dsdt;
-    const MemMapEntry *memmap = guest_info->memmap;
-    const int *irqmap = guest_info->irqmap;
+    const MemMapEntry *memmap = vms->memmap;
+    const int *irqmap = vms->irqmap;
 
     dsdt = init_aml_allocator();
     /* Reserve space for header */
@@ -674,7 +675,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
      * the RTC ACPI device at all when using UEFI.
      */
     scope = aml_scope("\\_SB");
-    acpi_dsdt_add_cpus(scope, guest_info->smp_cpus);
+    acpi_dsdt_add_cpus(scope, vms->smp_cpus);
     acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
     acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
@@ -682,7 +683,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
     acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
-                      guest_info->use_highmem);
+                      vms->highmem);
     acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
                        (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
     acpi_dsdt_add_power_button(scope);
@@ -709,8 +710,9 @@ struct AcpiBuildState {
 } AcpiBuildState;
 
 static
-void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
+void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     GArray *table_offsets;
     unsigned dsdt, rsdt;
     GArray *tables_blob = tables->table_data;
@@ -724,32 +726,32 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
 
     /* DSDT is pointed to by FADT */
     dsdt = tables_blob->len;
-    build_dsdt(tables_blob, tables->linker, guest_info);
+    build_dsdt(tables_blob, tables->linker, vms);
 
     /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, dsdt);
 
     acpi_add_table(table_offsets, tables_blob);
-    build_madt(tables_blob, tables->linker, guest_info);
+    build_madt(tables_blob, tables->linker, vms);
 
     acpi_add_table(table_offsets, tables_blob);
     build_gtdt(tables_blob, tables->linker);
 
     acpi_add_table(table_offsets, tables_blob);
-    build_mcfg(tables_blob, tables->linker, guest_info);
+    build_mcfg(tables_blob, tables->linker, vms);
 
     acpi_add_table(table_offsets, tables_blob);
-    build_spcr(tables_blob, tables->linker, guest_info);
+    build_spcr(tables_blob, tables->linker, vms);
 
     if (nb_numa_nodes > 0) {
         acpi_add_table(table_offsets, tables_blob);
-        build_srat(tables_blob, tables->linker, guest_info);
+        build_srat(tables_blob, tables->linker, vms);
     }
 
-    if (its_class_name() && !guest_info->no_its) {
+    if (its_class_name() && !vmc->no_its) {
         acpi_add_table(table_offsets, tables_blob);
-        build_iort(tables_blob, tables->linker, guest_info);
+        build_iort(tables_blob, tables->linker);
     }
 
     /* RSDT is pointed to by RSDP */
@@ -788,13 +790,13 @@ static void virt_acpi_build_update(void *build_opaque)
 
     acpi_build_tables_init(&tables);
 
-    virt_acpi_build(build_state->guest_info, &tables);
+    virt_acpi_build(container_of(build_state->guest_info,
+                                 VirtMachineState, acpi_guest_info), &tables);
 
     acpi_ram_update(build_state->table_mr, tables.table_data);
     acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
     acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
 
-
     acpi_build_tables_cleanup(&tables, true);
 }
 
@@ -842,7 +844,7 @@ void virt_acpi_setup(VirtMachineState *vms)
     build_state->guest_info = guest_info;
 
     acpi_build_tables_init(&tables);
-    virt_acpi_build(build_state->guest_info, &tables);
+    virt_acpi_build(vms, &tables);
 
     /* Now expose it all to Guest */
     build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index bcf8a2954c16..2cb2e5b4f52a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1416,13 +1416,7 @@ static void machvirt_init(MachineState *machine)
     create_fw_cfg(vms, &address_space_memory);
     rom_set_fw(fw_cfg_find());
 
-    guest_info->smp_cpus = vms->smp_cpus;
     guest_info->fw_cfg = fw_cfg_find();
-    guest_info->memmap = vms->memmap;
-    guest_info->irqmap = vms->irqmap;
-    guest_info->use_highmem = vms->highmem;
-    guest_info->gic_version = vms->gic_version;
-    guest_info->no_its = vmc->no_its;
     vms->machine_done.notify = virt_machine_done;
     qemu_add_machine_init_done_notifier(&vms->machine_done);
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index cbedc682ae99..52b35478223e 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -82,13 +82,7 @@ typedef struct MemMapEntry {
 } MemMapEntry;
 
 typedef struct VirtGuestInfo {
-    int smp_cpus;
     FWCfgState *fw_cfg;
-    const MemMapEntry *memmap;
-    const int *irqmap;
-    bool use_highmem;
-    int gic_version;
-    bool no_its;
 } VirtGuestInfo;
 
 typedef struct {
-- 
2.9.3

[Qemu-devel] [PATCH 09/11] hw/arm/virt-acpi-build: don't save VirtGuestInfo on AcpiBuildState

[Andrew Jones]

We can get to VirtMachineState without the need for saving a pointer
on AcpiBuildState. This is the mach-virt equivalent to "acpi: Don't save
PcGuestInfo on AcpiBuildState"

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

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7a1efd0dc538..137781646bec 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -706,7 +706,6 @@ struct AcpiBuildState {
     MemoryRegion *linker_mr;
     /* Is table patched? */
     bool patched;
-    VirtGuestInfo *guest_info;
 } AcpiBuildState;
 
 static
@@ -779,6 +778,8 @@ static void acpi_ram_update(MemoryRegion *mr, GArray *data)
 
 static void virt_acpi_build_update(void *build_opaque)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
+    VirtMachineState *vms = VIRT_MACHINE(machine);
     AcpiBuildState *build_state = build_opaque;
     AcpiBuildTables tables;
 
@@ -790,8 +791,7 @@ static void virt_acpi_build_update(void *build_opaque)
 
     acpi_build_tables_init(&tables);
 
-    virt_acpi_build(container_of(build_state->guest_info,
-                                 VirtMachineState, acpi_guest_info), &tables);
+    virt_acpi_build(vms, &tables);
 
     acpi_ram_update(build_state->table_mr, tables.table_data);
     acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
@@ -841,7 +841,6 @@ void virt_acpi_setup(VirtMachineState *vms)
     }
 
     build_state = g_malloc0(sizeof *build_state);
-    build_state->guest_info = guest_info;
 
     acpi_build_tables_init(&tables);
     virt_acpi_build(vms, &tables);
-- 
2.9.3

[Qemu-devel] [PATCH 10/11] hw/arm/virt: remove VirtGuestInfo

[Andrew Jones]

by moving VirtGuestInfo.fw_cfg to VirtMachineState. This is the
mach-virt equivalent of "pc: Move PcGuestInfo.fw_cfg to
PCMachineState" and "pc: Eliminate PcGuestInfo struct" combined.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt-acpi-build.c |  7 +++----
 hw/arm/virt.c            | 18 +++++++-----------
 include/hw/arm/virt.h    |  6 +-----
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 137781646bec..0e6caf5f1083 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -826,11 +826,10 @@ static const VMStateDescription vmstate_virt_acpi_build = {
 
 void virt_acpi_setup(VirtMachineState *vms)
 {
-    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
     AcpiBuildTables tables;
     AcpiBuildState *build_state;
 
-    if (!guest_info->fw_cfg) {
+    if (!vms->fw_cfg) {
         trace_virt_acpi_setup();
         return;
     }
@@ -855,8 +854,8 @@ void virt_acpi_setup(VirtMachineState *vms)
         acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
                           "etc/table-loader", 0);
 
-    fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
-                    tables.tcpalog->data, acpi_data_len(tables.tcpalog));
+    fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
+                    acpi_data_len(tables.tcpalog));
 
     build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
                                               ACPI_BUILD_RSDP_FILE, 0);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2cb2e5b4f52a..348e23ff663c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -914,15 +914,14 @@ static void create_flash(const VirtMachineState *vms,
     }
 }
 
-static void create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
+static void create_fw_cfg(VirtMachineState *vms, AddressSpace *as)
 {
     hwaddr base = vms->memmap[VIRT_FW_CFG].base;
     hwaddr size = vms->memmap[VIRT_FW_CFG].size;
-    FWCfgState *fw_cfg;
     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)vms->smp_cpus);
+    vms->fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
+    fw_cfg_add_i16(vms->fw_cfg, FW_CFG_NB_CPUS, (uint16_t)vms->smp_cpus);
 
     nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
     qemu_fdt_add_subnode(vms->fdt, nodename);
@@ -1158,12 +1157,11 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
-    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     const char *product = "QEMU Virtual Machine";
 
-    if (!fw_cfg) {
+    if (!vms->fw_cfg) {
         return;
     }
 
@@ -1178,9 +1176,9 @@ static void virt_build_smbios(VirtMachineState *vms)
                       &smbios_anchor, &smbios_anchor_len);
 
     if (smbios_anchor) {
-        fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
+        fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-tables",
                         smbios_tables, smbios_tables_len);
-        fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
+        fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-anchor",
                         smbios_anchor, smbios_anchor_len);
     }
 }
@@ -1205,7 +1203,6 @@ static void machvirt_init(MachineState *machine)
     int n, virt_max_cpus;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     const char *cpu_model = machine->cpu_model;
-    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
     char **cpustr;
     ObjectClass *oc;
     const char *typename;
@@ -1414,9 +1411,8 @@ static void machvirt_init(MachineState *machine)
     create_virtio_devices(vms, pic);
 
     create_fw_cfg(vms, &address_space_memory);
-    rom_set_fw(fw_cfg_find());
+    rom_set_fw(vms->fw_cfg);
 
-    guest_info->fw_cfg = fw_cfg_find();
     vms->machine_done.notify = virt_machine_done;
     qemu_add_machine_init_done_notifier(&vms->machine_done);
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 52b35478223e..ece3260bfeb5 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -81,10 +81,6 @@ typedef struct MemMapEntry {
     hwaddr size;
 } MemMapEntry;
 
-typedef struct VirtGuestInfo {
-    FWCfgState *fw_cfg;
-} VirtGuestInfo;
-
 typedef struct {
     MachineClass parent;
     bool disallow_affinity_adjustment;
@@ -95,8 +91,8 @@ typedef struct {
 
 typedef struct {
     MachineState parent;
-    VirtGuestInfo acpi_guest_info;
     Notifier machine_done;
+    FWCfgState *fw_cfg;
     bool secure;
     bool highmem;
     bool virt;
-- 
2.9.3

[Qemu-devel] [PATCH 11/11] hw/arm/virt-acpi-build: Don't incorrectly claim architectural timer to be edge-triggered

[Andrew Jones]

This is the ACPI equivalent to "hw/arm/virt: Don't incorrectly claim
architectural timer to be edge-triggered" which fixes the DT for
machine types 2.9 and later.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt-acpi-build.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 0e6caf5f1083..62cf4e7f6615 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -535,24 +535,30 @@ build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 
 /* GTDT */
 static void
-build_gtdt(GArray *table_data, BIOSLinker *linker)
+build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     int gtdt_start = table_data->len;
     AcpiGenericTimerTable *gtdt;
+    uint8_t irqflags = ACPI_LEVEL_SENSITIVE;
+
+    if (vmc->claim_edge_triggered_timers) {
+        irqflags = ACPI_EDGE_SENSITIVE;
+    }
 
     gtdt = acpi_data_push(table_data, sizeof *gtdt);
     /* The interrupt values are the same with the device tree when adding 16 */
     gtdt->secure_el1_interrupt = ARCH_TIMER_S_EL1_IRQ + 16;
-    gtdt->secure_el1_flags = ACPI_EDGE_SENSITIVE;
+    gtdt->secure_el1_flags = irqflags;
 
     gtdt->non_secure_el1_interrupt = ARCH_TIMER_NS_EL1_IRQ + 16;
-    gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;
+    gtdt->non_secure_el1_flags = irqflags | ACPI_GTDT_ALWAYS_ON;
 
     gtdt->virtual_timer_interrupt = ARCH_TIMER_VIRT_IRQ + 16;
-    gtdt->virtual_timer_flags = ACPI_EDGE_SENSITIVE;
+    gtdt->virtual_timer_flags = irqflags;
 
     gtdt->non_secure_el2_interrupt = ARCH_TIMER_NS_EL2_IRQ + 16;
-    gtdt->non_secure_el2_flags = ACPI_EDGE_SENSITIVE;
+    gtdt->non_secure_el2_flags = irqflags;
 
     build_header(linker, table_data,
                  (void *)(table_data->data + gtdt_start), "GTDT",
@@ -735,7 +741,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
     build_madt(tables_blob, tables->linker, vms);
 
     acpi_add_table(table_offsets, tables_blob);
-    build_gtdt(tables_blob, tables->linker);
+    build_gtdt(tables_blob, tables->linker, vms);
 
     acpi_add_table(table_offsets, tables_blob);
     build_mcfg(tables_blob, tables->linker, vms);
-- 
2.9.3

Re: [Qemu-devel] [PATCH 01/11] hw/arm/virt: parameter passing cleanups

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:12 +0100
Andrew Jones <drjones@redhat.com> wrote:

> Some simple cleanups made possible by "hw/arm/virt: Merge
> VirtBoardInfo and VirtMachineState"
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7eec50a82494..d451bc4f6b9b 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -570,10 +570,10 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
>      fdt_add_v2m_gic_node(vms);
>  }
>  
> -static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type,
> -                       bool secure, bool no_its)
> +static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type)
>  {
>      /* We create a standalone GIC */
> +    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      DeviceState *gicdev;
>      SysBusDevice *gicbusdev;
>      const char *gictype;
> @@ -589,7 +589,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type,
>       */
>      qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
>      if (!kvm_irqchip_in_kernel()) {
> -        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
> +        qdev_prop_set_bit(gicdev, "has-security-extensions", vms->secure);
>      }
>      qdev_init_nofail(gicdev);
>      gicbusdev = SYS_BUS_DEVICE(gicdev);
> @@ -643,7 +643,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type,
>  
>      fdt_add_gic_node(vms, type);
>  
> -    if (type == 3 && !no_its) {
> +    if (type == 3 && !vmc->no_its) {
>          create_its(vms, gicdev);
>      } else if (type == 2) {
>          create_v2m(vms, pic);
> @@ -1005,8 +1005,7 @@ static void create_pcie_irq_map(const VirtMachineState *vms,
>                             0x7           /* PCI irq */);
>  }
>  
> -static void create_pcie(const VirtMachineState *vms, qemu_irq *pic,
> -                        bool use_highmem)
> +static void create_pcie(const VirtMachineState *vms, qemu_irq *pic)
>  {
>      hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base;
>      hwaddr size_mmio = vms->memmap[VIRT_PCIE_MMIO].size;
> @@ -1049,7 +1048,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic,
>                               mmio_reg, base_mmio, size_mmio);
>      memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
>  
> -    if (use_highmem) {
> +    if (vms->highmem) {
>          /* Map high MMIO space */
>          MemoryRegion *high_mmio_alias = g_new0(MemoryRegion, 1);
>  
> @@ -1098,7 +1097,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic,
>      qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
>                                   2, base_ecam, 2, size_ecam);
>  
> -    if (use_highmem) {
> +    if (vms->highmem) {
>          qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "ranges",
>                                       1, FDT_PCI_RANGE_IOPORT, 2, 0,
>                                       2, base_pio, 2, size_pio,
> @@ -1428,7 +1427,7 @@ static void machvirt_init(MachineState *machine)
>  
>      create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
>  
> -    create_gic(vms, pic, gic_version, vms->secure, vmc->no_its);
> +    create_gic(vms, pic, gic_version);
>  
>      fdt_add_pmu_nodes(vms, gic_version);
>  
> @@ -1441,7 +1440,7 @@ static void machvirt_init(MachineState *machine)
>  
>      create_rtc(vms, pic);
>  
> -    create_pcie(vms, pic, vms->highmem);
> +    create_pcie(vms, pic);
>  
>      create_gpio(vms, pic);
>  

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

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:13 +0100
Andrew Jones <drjones@redhat.com> wrote:

> machvirt_init may need to probe for the gic version. If so, then
> make sure the result is written to VirtMachineState. With the
> state up to date, use it instead of a local variable. This is a
> cleanup that prepares for VirtMachineState to be passed to functions
> even outside hw/arm/virt.c
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d451bc4f6b9b..67c0abb30b5b 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -319,7 +319,7 @@ static void fdt_add_psci_node(const VirtMachineState *vms)
>      qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
>  }
>  
> -static void fdt_add_timer_nodes(const VirtMachineState *vms, int gictype)
> +static void fdt_add_timer_nodes(const VirtMachineState *vms)
>  {
>      /* On real hardware these interrupts are level-triggered.
>       * On KVM they were edge-triggered before host kernel version 4.4,
> @@ -347,7 +347,7 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms, int gictype)
>          irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
>      }
>  
> -    if (gictype == 2) {
> +    if (vms->gic_version == 2) {
>          irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
>                               GIC_FDT_IRQ_PPI_CPU_WIDTH,
>                               (1 << vms->smp_cpus) - 1);
> @@ -462,7 +462,7 @@ static void fdt_add_v2m_gic_node(VirtMachineState *vms)
>      qemu_fdt_setprop_cell(vms->fdt, "/intc/v2m", "phandle", vms->msi_phandle);
>  }
>  
> -static void fdt_add_gic_node(VirtMachineState *vms, int type)
> +static void fdt_add_gic_node(VirtMachineState *vms)
>  {
>      vms->gic_phandle = qemu_fdt_alloc_phandle(vms->fdt);
>      qemu_fdt_setprop_cell(vms->fdt, "/", "interrupt-parent", vms->gic_phandle);
> @@ -473,7 +473,7 @@ static void fdt_add_gic_node(VirtMachineState *vms, int type)
>      qemu_fdt_setprop_cell(vms->fdt, "/intc", "#address-cells", 0x2);
>      qemu_fdt_setprop_cell(vms->fdt, "/intc", "#size-cells", 0x2);
>      qemu_fdt_setprop(vms->fdt, "/intc", "ranges", NULL, 0);
> -    if (type == 3) {
> +    if (vms->gic_version == 3) {
>          qemu_fdt_setprop_string(vms->fdt, "/intc", "compatible",
>                                  "arm,gic-v3");
>          qemu_fdt_setprop_sized_cells(vms->fdt, "/intc", "reg",
> @@ -500,7 +500,7 @@ static void fdt_add_gic_node(VirtMachineState *vms, int type)
>      qemu_fdt_setprop_cell(vms->fdt, "/intc", "phandle", vms->gic_phandle);
>  }
>  
> -static void fdt_add_pmu_nodes(const VirtMachineState *vms, int gictype)
> +static void fdt_add_pmu_nodes(const VirtMachineState *vms)
>  {
>      CPUState *cpu;
>      ARMCPU *armcpu;
> @@ -514,7 +514,7 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms, int gictype)
>          }
>      }
>  
> -    if (gictype == 2) {
> +    if (vms->gic_version == 2) {
>          irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
>                               GIC_FDT_IRQ_PPI_CPU_WIDTH,
>                               (1 << vms->smp_cpus) - 1);
> @@ -570,14 +570,14 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
>      fdt_add_v2m_gic_node(vms);
>  }
>  
> -static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type)
> +static void create_gic(VirtMachineState *vms, qemu_irq *pic)
>  {
>      /* We create a standalone GIC */
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      DeviceState *gicdev;
>      SysBusDevice *gicbusdev;
>      const char *gictype;
> -    int i;
> +    int type = vms->gic_version, i;
>  
>      gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
>  
> @@ -641,7 +641,7 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic, int type)
>          pic[i] = qdev_get_gpio_in(gicdev, i);
>      }
>  
> -    fdt_add_gic_node(vms, type);
> +    fdt_add_gic_node(vms);
>  
>      if (type == 3 && !vmc->no_its) {
>          create_its(vms, gicdev);
> @@ -1237,7 +1237,6 @@ static void machvirt_init(MachineState *machine)
>      qemu_irq pic[NUM_IRQS];
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *secure_sysmem = NULL;
> -    int gic_version = vms->gic_version;
>      int n, virt_max_cpus;
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      const char *cpu_model = machine->cpu_model;
> @@ -1258,14 +1257,14 @@ static void machvirt_init(MachineState *machine)
>      /* We can probe only here because during property set
>       * KVM is not available yet
>       */
> -    if (!gic_version) {
> +    if (!vms->gic_version) {
>          if (!kvm_enabled()) {
>              error_report("gic-version=host requires KVM");
>              exit(1);
>          }
>  
> -        gic_version = kvm_arm_vgic_probe();
> -        if (!gic_version) {
> +        vms->gic_version = kvm_arm_vgic_probe();
> +        if (!vms->gic_version) {
>              error_report("Unable to determine GIC version supported by host");
>              exit(1);
>          }
> @@ -1300,7 +1299,7 @@ static void machvirt_init(MachineState *machine)
>      /* 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) {
> +    if (vms->gic_version == 3) {
>          virt_max_cpus = vms->memmap[VIRT_GIC_REDIST].size / 0x20000;
>          clustersz = GICV3_TARGETLIST_BITS;
>      } else {
> @@ -1417,7 +1416,7 @@ static void machvirt_init(MachineState *machine)
>  
>          object_property_set_bool(cpuobj, true, "realized", NULL);
>      }
> -    fdt_add_timer_nodes(vms, gic_version);
> +    fdt_add_timer_nodes(vms);
>      fdt_add_cpu_nodes(vms);
>      fdt_add_psci_node(vms);
>  
> @@ -1427,9 +1426,9 @@ static void machvirt_init(MachineState *machine)
>  
>      create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
>  
> -    create_gic(vms, pic, gic_version);
> +    create_gic(vms, pic);
>  
> -    fdt_add_pmu_nodes(vms, gic_version);
> +    fdt_add_pmu_nodes(vms);
>  
>      create_uart(vms, pic, VIRT_UART, sysmem, serial_hds[0]);
>  
> @@ -1458,7 +1457,7 @@ static void machvirt_init(MachineState *machine)
>      guest_info->memmap = vms->memmap;
>      guest_info->irqmap = vms->irqmap;
>      guest_info->use_highmem = vms->highmem;
> -    guest_info->gic_version = gic_version;
> +    guest_info->gic_version = vms->gic_version;
>      guest_info->no_its = vmc->no_its;
>      guest_info_state->machine_done.notify = virt_guest_info_machine_done;
>      qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);

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 <drjones@redhat.com> 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 <drjones@redhat.com>
> ---
>  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("/fw-cfg@%" 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;

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 <drjones@redhat.com> 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 <drjones@redhat.com>
> > ---
> >  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("/fw-cfg@%" 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;
> 
> 

Re: [Qemu-devel] [PATCH 04/11] hw/arm/virt: eliminate struct VirtGuestInfoState

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:15 +0100
Andrew Jones <drjones@redhat.com> wrote:

> Instead of allocating a new struct just for VirtGuestInfo and the
> machine_done Notifier, place them inside VirtMachineState. This
> is the mach-virt equivalent of "pc: Eliminate struct
> PcGuestInfoState"
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt.c                    | 20 +++++++++++---------
>  include/hw/arm/virt-acpi-build.h |  6 ------
>  2 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 18aa3672739d..0657f9ae8722 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -76,6 +76,8 @@ typedef struct {
>  
>  typedef struct {
>      MachineState parent;
> +    VirtGuestInfo acpi_guest_info;
> +    Notifier machine_done;
>      bool secure;
>      bool highmem;
>      bool virt;
> @@ -1222,12 +1224,13 @@ static void virt_build_smbios(VirtGuestInfo *guest_info)
>  }
>  
>  static
> -void virt_guest_info_machine_done(Notifier *notifier, void *data)
> +void virt_machine_done(Notifier *notifier, void *data)
>  {
> -    VirtGuestInfoState *guest_info_state = container_of(notifier,
> -                                              VirtGuestInfoState, machine_done);
> -    virt_acpi_setup(&guest_info_state->info);
> -    virt_build_smbios(&guest_info_state->info);
> +    VirtMachineState *vms = container_of(notifier, VirtMachineState,
> +                                         machine_done);
> +
> +    virt_acpi_setup(&vms->acpi_guest_info);
> +    virt_build_smbios(&vms->acpi_guest_info);
>  }
>  
>  static void machvirt_init(MachineState *machine)
> @@ -1240,8 +1243,7 @@ static void machvirt_init(MachineState *machine)
>      int n, virt_max_cpus;
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      const char *cpu_model = machine->cpu_model;
> -    VirtGuestInfoState *guest_info_state = g_malloc0(sizeof *guest_info_state);
> -    VirtGuestInfo *guest_info = &guest_info_state->info;
> +    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
>      char **cpustr;
>      ObjectClass *oc;
>      const char *typename;
> @@ -1459,8 +1461,8 @@ static void machvirt_init(MachineState *machine)
>      guest_info->use_highmem = vms->highmem;
>      guest_info->gic_version = vms->gic_version;
>      guest_info->no_its = vmc->no_its;
> -    guest_info_state->machine_done.notify = virt_guest_info_machine_done;
> -    qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
> +    vms->machine_done.notify = virt_machine_done;
> +    qemu_add_machine_init_done_notifier(&vms->machine_done);
>  
>      vms->bootinfo.ram_size = machine->ram_size;
>      vms->bootinfo.kernel_filename = machine->kernel_filename;
> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
> index f5ec749b8fea..d74d7a1af650 100644
> --- a/include/hw/arm/virt-acpi-build.h
> +++ b/include/hw/arm/virt-acpi-build.h
> @@ -36,12 +36,6 @@ typedef struct VirtGuestInfo {
>      bool no_its;
>  } VirtGuestInfo;
>  
> -
> -typedef struct VirtGuestInfoState {
> -    VirtGuestInfo info;
> -    Notifier machine_done;
> -} VirtGuestInfoState;
> -
>  void virt_acpi_setup(VirtGuestInfo *guest_info);
>  
>  #endif

Re: [Qemu-devel] [PATCH 05/11] hw/arm/virt: remove include/hw/arm/virt-acpi-build.h

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:16 +0100
Andrew Jones <drjones@redhat.com> wrote:

> include/hw/arm/virt-acpi-build.h is only used for VirtGuestInfo,
> which doesn't even necessarily have to be ACPI specific. Move
> VirtGuestInfo to include/hw/arm/virt.h, allowing us to remove
> include/hw/arm/virt-acpi-build.h, and to prepare for even more
> code motion.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  MAINTAINERS                      |  2 --
>  hw/arm/virt-acpi-build.c         |  2 +-
>  hw/arm/virt.c                    |  1 -
>  include/hw/arm/virt-acpi-build.h | 41 ----------------------------------------
>  include/hw/arm/virt.h            | 16 +++++++++++++++-
>  5 files changed, 16 insertions(+), 46 deletions(-)
>  delete mode 100644 include/hw/arm/virt-acpi-build.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4a605791fc98..465b9f0f8440 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -508,7 +508,6 @@ M: Shannon Zhao <shannon.zhao@linaro.org>
>  L: qemu-arm@nongnu.org
>  S: Maintained
>  F: hw/arm/virt-acpi-build.c
> -F: include/hw/arm/virt-acpi-build.h
>  
>  STM32F205
>  M: Alistair Francis <alistair@alistair23.me>
> @@ -885,7 +884,6 @@ F: hw/acpi/*
>  F: hw/smbios/*
>  F: hw/i386/acpi-build.[hc]
>  F: hw/arm/virt-acpi-build.c
> -F: include/hw/arm/virt-acpi-build.h
>  
>  ppc4xx
>  M: Alexander Graf <agraf@suse.de>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index d4160dfa7d34..db44e2dd5f12 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -29,7 +29,6 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu-common.h"
> -#include "hw/arm/virt-acpi-build.h"
>  #include "qemu/bitmap.h"
>  #include "trace.h"
>  #include "qom/cpu.h"
> @@ -43,6 +42,7 @@
>  #include "hw/acpi/aml-build.h"
>  #include "hw/pci/pcie_host.h"
>  #include "hw/pci/pci.h"
> +#include "hw/arm/virt.h"
>  #include "sysemu/numa.h"
>  #include "kvm_arm.h"
>  
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 0657f9ae8722..b137d9bbb689 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -48,7 +48,6 @@
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
>  #include "hw/pci-host/gpex.h"
> -#include "hw/arm/virt-acpi-build.h"
>  #include "hw/arm/sysbus-fdt.h"
>  #include "hw/platform-bus.h"
>  #include "hw/arm/fdt.h"
> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
> deleted file mode 100644
> index d74d7a1af650..000000000000
> --- a/include/hw/arm/virt-acpi-build.h
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/*
> - *
> - * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
> - *
> - * Author: Shannon Zhao <zhaoshenglong@huawei.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms and conditions of the GNU General Public License,
> - * version 2 or later, as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> - * more details.
> - *
> - * You should have received a copy of the GNU General Public License along with
> - * this program.  If not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#ifndef QEMU_VIRT_ACPI_BUILD_H
> -#define QEMU_VIRT_ACPI_BUILD_H
> -
> -#include "qemu-common.h"
> -#include "hw/arm/virt.h"
> -#include "qemu/notify.h"
> -
> -#define ACPI_GICC_ENABLED 1
> -
> -typedef struct VirtGuestInfo {
> -    int smp_cpus;
> -    FWCfgState *fw_cfg;
> -    const MemMapEntry *memmap;
> -    const int *irqmap;
> -    bool use_highmem;
> -    int gic_version;
> -    bool no_its;
> -} VirtGuestInfo;
> -
> -void virt_acpi_setup(VirtGuestInfo *guest_info);
> -
> -#endif
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index b805b7622834..c2c1f6837442 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -32,6 +32,7 @@
>  
>  #include "qemu-common.h"
>  #include "exec/hwaddr.h"
> +#include "qemu/notify.h"
>  
>  #define NUM_GICV2M_SPIS       64
>  #define NUM_VIRTIO_TRANSPORTS 32
> @@ -47,6 +48,8 @@
>  
>  #define PPI(irq) ((irq) + 16)
>  
> +#define ACPI_GICC_ENABLED 1
> +
>  enum {
>      VIRT_FLASH,
>      VIRT_MEM,
> @@ -76,5 +79,16 @@ typedef struct MemMapEntry {
>      hwaddr size;
>  } MemMapEntry;
>  
> +typedef struct VirtGuestInfo {
> +    int smp_cpus;
> +    FWCfgState *fw_cfg;
> +    const MemMapEntry *memmap;
> +    const int *irqmap;
> +    bool use_highmem;
> +    int gic_version;
> +    bool no_its;
> +} VirtGuestInfo;
> +
> +void virt_acpi_setup(VirtGuestInfo *guest_info);
>  
> -#endif
> +#endif /* QEMU_ARM_VIRT_H */

Re: [Qemu-devel] [PATCH 06/11] hw/arm/virt: move VirtMachineState/Class to virt.h

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:17 +0100
Andrew Jones <drjones@redhat.com> wrote:

> In preparation to share more Virt machine state than just guest-info
> with other mach-virt source files, move the State and Class structures
> to virt.h
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
[...]
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index c2c1f6837442..77d25dbddb62 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -33,6 +33,8 @@
>  #include "qemu-common.h"
>  #include "exec/hwaddr.h"
>  #include "qemu/notify.h"
> +#include "hw/boards.h"
nit:
one won't need this header be included directly in virt.c anymore

> +#include "hw/arm/arm.h"
>  
>  #define NUM_GICV2M_SPIS       64
>  #define NUM_VIRTIO_TRANSPORTS 32
> @@ -89,6 +91,43 @@ typedef struct VirtGuestInfo {
>      bool no_its;
>  } VirtGuestInfo;
>  
> +typedef struct {
> +    MachineClass parent;
> +    bool disallow_affinity_adjustment;
> +    bool no_its;
> +    bool no_pmu;
> +    bool claim_edge_triggered_timers;
> +} VirtMachineClass;
> +
> +typedef struct {
> +    MachineState parent;
> +    VirtGuestInfo acpi_guest_info;
> +    Notifier machine_done;
> +    bool secure;
> +    bool highmem;
> +    bool virt;
> +    int32_t gic_version;
> +    struct arm_boot_info bootinfo;
> +    const MemMapEntry *memmap;
> +    const int *irqmap;
> +    int smp_cpus;
> +    void *fdt;
> +    int fdt_size;
> +    uint32_t clock_phandle;
> +    uint32_t gic_phandle;
> +    uint32_t msi_phandle;
> +    int psci_conduit;
> +} VirtMachineState;
> +
> +#define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
> +#define VIRT_MACHINE(obj) \
> +    OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
> +#define VIRT_MACHINE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(VirtMachineClass, obj, TYPE_VIRT_MACHINE)
> +#define VIRT_MACHINE_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
> +
> +
>  void virt_acpi_setup(VirtGuestInfo *guest_info);
>  
>  #endif /* QEMU_ARM_VIRT_H */

Re: [Qemu-devel] [PATCH 07/11] hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:18 +0100
Andrew Jones <drjones@redhat.com> wrote:

> Only two functions take VirtGuestInfo parameters. Now that guest-info
> is part of VirtMachineState, and VirtMachineState is defined in the
> virt header, pass that instead.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c | 3 ++-
>  hw/arm/virt.c            | 8 ++++----
>  include/hw/arm/virt.h    | 2 +-
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index db44e2dd5f12..f7ce6aeed307 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -822,8 +822,9 @@ static const VMStateDescription vmstate_virt_acpi_build = {
>      },
>  };
>  
> -void virt_acpi_setup(VirtGuestInfo *guest_info)
> +void virt_acpi_setup(VirtMachineState *vms)
>  {
> +    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
>      AcpiBuildTables tables;
>      AcpiBuildState *build_state;
>  
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7860fade52f1..bcf8a2954c16 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1156,9 +1156,9 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>      return board->fdt;
>  }
>  
> -static void virt_build_smbios(VirtGuestInfo *guest_info)
> +static void virt_build_smbios(VirtMachineState *vms)
Looks like all it needs is 'fw_cfg'
If there aren't plans to use more data from VirtMachineState,
just passing fw_cfg should be enough.

>  {
> -    FWCfgState *fw_cfg = guest_info->fw_cfg;
> +    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
>      uint8_t *smbios_tables, *smbios_anchor;
>      size_t smbios_tables_len, smbios_anchor_len;
>      const char *product = "QEMU Virtual Machine";
> @@ -1191,8 +1191,8 @@ void virt_machine_done(Notifier *notifier, void *data)
>      VirtMachineState *vms = container_of(notifier, VirtMachineState,
>                                           machine_done);
>  
> -    virt_acpi_setup(&vms->acpi_guest_info);
> -    virt_build_smbios(&vms->acpi_guest_info);
> +    virt_acpi_setup(vms);
> +    virt_build_smbios(vms);
>  }
>  
>  static void machvirt_init(MachineState *machine)
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 77d25dbddb62..cbedc682ae99 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -128,6 +128,6 @@ typedef struct {
>      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
>  
>  
> -void virt_acpi_setup(VirtGuestInfo *guest_info);
> +void virt_acpi_setup(VirtMachineState *vms);
>  
>  #endif /* QEMU_ARM_VIRT_H */

Re: [Qemu-devel] [PATCH 10/11] hw/arm/virt: remove VirtGuestInfo

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:21 +0100
Andrew Jones <drjones@redhat.com> wrote:

> by moving VirtGuestInfo.fw_cfg to VirtMachineState. This is the
> mach-virt equivalent of "pc: Move PcGuestInfo.fw_cfg to
> PCMachineState" and "pc: Eliminate PcGuestInfo struct" combined.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c |  7 +++----
>  hw/arm/virt.c            | 18 +++++++-----------
>  include/hw/arm/virt.h    |  6 +-----
>  3 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 137781646bec..0e6caf5f1083 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -826,11 +826,10 @@ static const VMStateDescription vmstate_virt_acpi_build = {
>  
>  void virt_acpi_setup(VirtMachineState *vms)
>  {
> -    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
>      AcpiBuildTables tables;
>      AcpiBuildState *build_state;
>  
> -    if (!guest_info->fw_cfg) {
> +    if (!vms->fw_cfg) {
>          trace_virt_acpi_setup();
>          return;
>      }
> @@ -855,8 +854,8 @@ void virt_acpi_setup(VirtMachineState *vms)
>          acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
>                            "etc/table-loader", 0);
>  
> -    fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
> -                    tables.tcpalog->data, acpi_data_len(tables.tcpalog));
> +    fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
> +                    acpi_data_len(tables.tcpalog));
>  
>      build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
>                                                ACPI_BUILD_RSDP_FILE, 0);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2cb2e5b4f52a..348e23ff663c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -914,15 +914,14 @@ static void create_flash(const VirtMachineState *vms,
>      }
>  }
>  
> -static void create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
> +static void create_fw_cfg(VirtMachineState *vms, AddressSpace *as)
>  {
>      hwaddr base = vms->memmap[VIRT_FW_CFG].base;
>      hwaddr size = vms->memmap[VIRT_FW_CFG].size;
> -    FWCfgState *fw_cfg;
>      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)vms->smp_cpus);
> +    vms->fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
> +    fw_cfg_add_i16(vms->fw_cfg, FW_CFG_NB_CPUS, (uint16_t)vms->smp_cpus);
>  
>      nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
>      qemu_fdt_add_subnode(vms->fdt, nodename);
> @@ -1158,12 +1157,11 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>  
>  static void virt_build_smbios(VirtMachineState *vms)
>  {
> -    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
>      uint8_t *smbios_tables, *smbios_anchor;
>      size_t smbios_tables_len, smbios_anchor_len;
>      const char *product = "QEMU Virtual Machine";
>  
> -    if (!fw_cfg) {
> +    if (!vms->fw_cfg) {
>          return;
>      }
>  
> @@ -1178,9 +1176,9 @@ static void virt_build_smbios(VirtMachineState *vms)
>                        &smbios_anchor, &smbios_anchor_len);
>  
>      if (smbios_anchor) {
> -        fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
> +        fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-tables",
>                          smbios_tables, smbios_tables_len);
> -        fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
> +        fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-anchor",
>                          smbios_anchor, smbios_anchor_len);
>      }
>  }
> @@ -1205,7 +1203,6 @@ static void machvirt_init(MachineState *machine)
>      int n, virt_max_cpus;
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      const char *cpu_model = machine->cpu_model;
> -    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
>      char **cpustr;
>      ObjectClass *oc;
>      const char *typename;
> @@ -1414,9 +1411,8 @@ static void machvirt_init(MachineState *machine)
>      create_virtio_devices(vms, pic);
>  
>      create_fw_cfg(vms, &address_space_memory);

above would look clearer as:
vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

> -    rom_set_fw(fw_cfg_find());
> +    rom_set_fw(vms->fw_cfg);
>  
> -    guest_info->fw_cfg = fw_cfg_find();
>      vms->machine_done.notify = virt_machine_done;
>      qemu_add_machine_init_done_notifier(&vms->machine_done);
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 52b35478223e..ece3260bfeb5 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -81,10 +81,6 @@ typedef struct MemMapEntry {
>      hwaddr size;
>  } MemMapEntry;
>  
> -typedef struct VirtGuestInfo {
> -    FWCfgState *fw_cfg;
> -} VirtGuestInfo;
> -
>  typedef struct {
>      MachineClass parent;
>      bool disallow_affinity_adjustment;
> @@ -95,8 +91,8 @@ typedef struct {
>  
>  typedef struct {
>      MachineState parent;
> -    VirtGuestInfo acpi_guest_info;
>      Notifier machine_done;
> +    FWCfgState *fw_cfg;
>      bool secure;
>      bool highmem;
>      bool virt;

Re: [Qemu-devel] [PATCH 08/11] hw/arm/virt-acpi-build: remove redundant members from VirtGuestInfo

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:19 +0100
Andrew Jones <drjones@redhat.com> wrote:

> Now that we pass VirtMachineState, and guest-info is just part of
> that state, we can remove all the redundant members and access
> the VirtMachineState directly.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt-acpi-build.c | 72 +++++++++++++++++++++++++-----------------------
>  hw/arm/virt.c            |  6 ----
>  include/hw/arm/virt.h    |  6 ----
>  3 files changed, 37 insertions(+), 47 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f7ce6aeed307..7a1efd0dc538 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -384,7 +384,7 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
>  }
>  
>  static void
> -build_iort(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> +build_iort(GArray *table_data, BIOSLinker *linker)
>  {
>      int iort_start = table_data->len;
>      AcpiIortIdMapping *idmap;
> @@ -439,11 +439,11 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>  }
>  
>  static void
> -build_spcr(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> +build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
>      AcpiSerialPortConsoleRedirection *spcr;
> -    const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
> -    int irq = guest_info->irqmap[VIRT_UART] + ARM_SPI_BASE;
> +    const MemMapEntry *uart_memmap = &vms->memmap[VIRT_UART];
> +    int irq = vms->irqmap[VIRT_UART] + ARM_SPI_BASE;
>  
>      spcr = acpi_data_push(table_data, sizeof(*spcr));
>  
> @@ -472,16 +472,16 @@ build_spcr(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>  }
>  
>  static void
> -build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> +build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
>      AcpiSystemResourceAffinityTable *srat;
>      AcpiSratProcessorGiccAffinity *core;
>      AcpiSratMemoryAffinity *numamem;
>      int i, j, srat_start;
>      uint64_t mem_base;
> -    uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof(uint32_t));
> +    uint32_t *cpu_node = g_malloc0(vms->smp_cpus * sizeof(uint32_t));
>  
> -    for (i = 0; i < guest_info->smp_cpus; i++) {
> +    for (i = 0; i < vms->smp_cpus; i++) {
>          j = numa_get_node_for_cpu(i);
>          if (j < nb_numa_nodes) {
>                  cpu_node[i] = j;
> @@ -492,7 +492,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>      srat = acpi_data_push(table_data, sizeof(*srat));
>      srat->reserved1 = cpu_to_le32(1);
>  
> -    for (i = 0; i < guest_info->smp_cpus; ++i) {
> +    for (i = 0; i < vms->smp_cpus; ++i) {
>          core = acpi_data_push(table_data, sizeof(*core));
>          core->type = ACPI_SRAT_PROCESSOR_GICC;
>          core->length = sizeof(*core);
> @@ -502,7 +502,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>      }
>      g_free(cpu_node);
>  
> -    mem_base = guest_info->memmap[VIRT_MEM].base;
> +    mem_base = vms->memmap[VIRT_MEM].base;
>      for (i = 0; i < nb_numa_nodes; ++i) {
>          numamem = acpi_data_push(table_data, sizeof(*numamem));
>          build_srat_memory(numamem, mem_base, numa_info[i].node_mem, i,
> @@ -515,10 +515,10 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>  }
>  
>  static void
> -build_mcfg(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> +build_mcfg(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
>      AcpiTableMcfg *mcfg;
> -    const MemMapEntry *memmap = guest_info->memmap;
> +    const MemMapEntry *memmap = vms->memmap;
>      int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>  
>      mcfg = acpi_data_push(table_data, len);
> @@ -561,11 +561,12 @@ build_gtdt(GArray *table_data, BIOSLinker *linker)
>  
>  /* MADT */
>  static void
> -build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> +build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      int madt_start = table_data->len;
> -    const MemMapEntry *memmap = guest_info->memmap;
> -    const int *irqmap = guest_info->irqmap;
> +    const MemMapEntry *memmap = vms->memmap;
> +    const int *irqmap = vms->irqmap;
>      AcpiMultipleApicTable *madt;
>      AcpiMadtGenericDistributor *gicd;
>      AcpiMadtGenericMsiFrame *gic_msi;
> @@ -577,16 +578,16 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>      gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR;
>      gicd->length = sizeof(*gicd);
>      gicd->base_address = memmap[VIRT_GIC_DIST].base;
> -    gicd->version = guest_info->gic_version;
> +    gicd->version = vms->gic_version;
>  
> -    for (i = 0; i < guest_info->smp_cpus; i++) {
> +    for (i = 0; i < vms->smp_cpus; i++) {
>          AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,
>                                                       sizeof *gicc);
>          ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
>  
>          gicc->type = ACPI_APIC_GENERIC_INTERRUPT;
>          gicc->length = sizeof(*gicc);
> -        if (guest_info->gic_version == 2) {
> +        if (vms->gic_version == 2) {
>              gicc->base_address = memmap[VIRT_GIC_CPU].base;
>          }
>          gicc->cpu_interface_number = i;
> @@ -599,7 +600,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>          }
>      }
>  
> -    if (guest_info->gic_version == 3) {
> +    if (vms->gic_version == 3) {
>          AcpiMadtGenericTranslator *gic_its;
>          AcpiMadtGenericRedistributor *gicr = acpi_data_push(table_data,
>                                                           sizeof *gicr);
> @@ -609,7 +610,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>          gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
>          gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
>  
> -        if (its_class_name() && !guest_info->no_its) {
> +        if (its_class_name() && !vmc->no_its) {
>              gic_its = acpi_data_push(table_data, sizeof *gic_its);
>              gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
>              gic_its->length = sizeof(*gic_its);
> @@ -658,11 +659,11 @@ build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt_tbl_offset)
>  
>  /* DSDT */
>  static void
> -build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> +build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
>      Aml *scope, *dsdt;
> -    const MemMapEntry *memmap = guest_info->memmap;
> -    const int *irqmap = guest_info->irqmap;
> +    const MemMapEntry *memmap = vms->memmap;
> +    const int *irqmap = vms->irqmap;
>  
>      dsdt = init_aml_allocator();
>      /* Reserve space for header */
> @@ -674,7 +675,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>       * the RTC ACPI device at all when using UEFI.
>       */
>      scope = aml_scope("\\_SB");
> -    acpi_dsdt_add_cpus(scope, guest_info->smp_cpus);
> +    acpi_dsdt_add_cpus(scope, vms->smp_cpus);
>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
> @@ -682,7 +683,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>                      (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
>      acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
> -                      guest_info->use_highmem);
> +                      vms->highmem);
>      acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>                         (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>      acpi_dsdt_add_power_button(scope);
> @@ -709,8 +710,9 @@ struct AcpiBuildState {
>  } AcpiBuildState;
>  
>  static
> -void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
> +void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      GArray *table_offsets;
>      unsigned dsdt, rsdt;
>      GArray *tables_blob = tables->table_data;
> @@ -724,32 +726,32 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>  
>      /* DSDT is pointed to by FADT */
>      dsdt = tables_blob->len;
> -    build_dsdt(tables_blob, tables->linker, guest_info);
> +    build_dsdt(tables_blob, tables->linker, vms);
>  
>      /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
>      acpi_add_table(table_offsets, tables_blob);
>      build_fadt(tables_blob, tables->linker, dsdt);
>  
>      acpi_add_table(table_offsets, tables_blob);
> -    build_madt(tables_blob, tables->linker, guest_info);
> +    build_madt(tables_blob, tables->linker, vms);
>  
>      acpi_add_table(table_offsets, tables_blob);
>      build_gtdt(tables_blob, tables->linker);
>  
>      acpi_add_table(table_offsets, tables_blob);
> -    build_mcfg(tables_blob, tables->linker, guest_info);
> +    build_mcfg(tables_blob, tables->linker, vms);
>  
>      acpi_add_table(table_offsets, tables_blob);
> -    build_spcr(tables_blob, tables->linker, guest_info);
> +    build_spcr(tables_blob, tables->linker, vms);
>  
>      if (nb_numa_nodes > 0) {
>          acpi_add_table(table_offsets, tables_blob);
> -        build_srat(tables_blob, tables->linker, guest_info);
> +        build_srat(tables_blob, tables->linker, vms);
>      }
>  
> -    if (its_class_name() && !guest_info->no_its) {
> +    if (its_class_name() && !vmc->no_its) {
>          acpi_add_table(table_offsets, tables_blob);
> -        build_iort(tables_blob, tables->linker, guest_info);
> +        build_iort(tables_blob, tables->linker);
>      }
>  
>      /* RSDT is pointed to by RSDP */
> @@ -788,13 +790,13 @@ static void virt_acpi_build_update(void *build_opaque)
>  
>      acpi_build_tables_init(&tables);
>  
> -    virt_acpi_build(build_state->guest_info, &tables);
> +    virt_acpi_build(container_of(build_state->guest_info,
> +                                 VirtMachineState, acpi_guest_info), &tables);
>  
>      acpi_ram_update(build_state->table_mr, tables.table_data);
>      acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
>      acpi_ram_update(build_state->linker_mr, tables.linker->cmd_blob);
>  
> -
>      acpi_build_tables_cleanup(&tables, true);
>  }
>  
> @@ -842,7 +844,7 @@ void virt_acpi_setup(VirtMachineState *vms)
>      build_state->guest_info = guest_info;
>  
>      acpi_build_tables_init(&tables);
> -    virt_acpi_build(build_state->guest_info, &tables);
> +    virt_acpi_build(vms, &tables);
>  
>      /* Now expose it all to Guest */
>      build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index bcf8a2954c16..2cb2e5b4f52a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1416,13 +1416,7 @@ static void machvirt_init(MachineState *machine)
>      create_fw_cfg(vms, &address_space_memory);
>      rom_set_fw(fw_cfg_find());
>  
> -    guest_info->smp_cpus = vms->smp_cpus;
>      guest_info->fw_cfg = fw_cfg_find();
> -    guest_info->memmap = vms->memmap;
> -    guest_info->irqmap = vms->irqmap;
> -    guest_info->use_highmem = vms->highmem;
> -    guest_info->gic_version = vms->gic_version;
> -    guest_info->no_its = vmc->no_its;
>      vms->machine_done.notify = virt_machine_done;
>      qemu_add_machine_init_done_notifier(&vms->machine_done);
>  
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index cbedc682ae99..52b35478223e 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -82,13 +82,7 @@ typedef struct MemMapEntry {
>  } MemMapEntry;
>  
>  typedef struct VirtGuestInfo {
> -    int smp_cpus;
>      FWCfgState *fw_cfg;
> -    const MemMapEntry *memmap;
> -    const int *irqmap;
> -    bool use_highmem;
> -    int gic_version;
> -    bool no_its;
>  } VirtGuestInfo;
>  
>  typedef struct {

Re: [Qemu-devel] [PATCH 09/11] hw/arm/virt-acpi-build: don't save VirtGuestInfo on AcpiBuildState

[Igor Mammedov]

On Tue, 13 Dec 2016 22:45:20 +0100
Andrew Jones <drjones@redhat.com> wrote:

> We can get to VirtMachineState without the need for saving a pointer
> on AcpiBuildState. This is the mach-virt equivalent to "acpi: Don't save
> PcGuestInfo on AcpiBuildState"
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7a1efd0dc538..137781646bec 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -706,7 +706,6 @@ struct AcpiBuildState {
>      MemoryRegion *linker_mr;
>      /* Is table patched? */
>      bool patched;
> -    VirtGuestInfo *guest_info;
>  } AcpiBuildState;
>  
>  static
> @@ -779,6 +778,8 @@ static void acpi_ram_update(MemoryRegion *mr, GArray *data)
>  
>  static void virt_acpi_build_update(void *build_opaque)
>  {
> +    MachineState *machine = MACHINE(qdev_get_machine());
> +    VirtMachineState *vms = VIRT_MACHINE(machine);
>      AcpiBuildState *build_state = build_opaque;
>      AcpiBuildTables tables;
>  
> @@ -790,8 +791,7 @@ static void virt_acpi_build_update(void *build_opaque)
>  
>      acpi_build_tables_init(&tables);
>  
> -    virt_acpi_build(container_of(build_state->guest_info,
> -                                 VirtMachineState, acpi_guest_info), &tables);
> +    virt_acpi_build(vms, &tables);

virt_acpi_build(VIRT_MACHINE(qdev_get_machine()), &tables);

or even better do it inside of virt_acpi_build(&tables);


>  
>      acpi_ram_update(build_state->table_mr, tables.table_data);
>      acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
> @@ -841,7 +841,6 @@ void virt_acpi_setup(VirtMachineState *vms)
>      }
>  
>      build_state = g_malloc0(sizeof *build_state);
> -    build_state->guest_info = guest_info;
>  
>      acpi_build_tables_init(&tables);
>      virt_acpi_build(vms, &tables);

Re: [Qemu-devel] [PATCH 07/11] hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo

[Andrew Jones]

On Thu, Dec 15, 2016 at 03:45:55PM +0100, Igor Mammedov wrote:
> On Tue, 13 Dec 2016 22:45:18 +0100
> Andrew Jones <drjones@redhat.com> wrote:
> 
> > Only two functions take VirtGuestInfo parameters. Now that guest-info
> > is part of VirtMachineState, and VirtMachineState is defined in the
> > virt header, pass that instead.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  hw/arm/virt-acpi-build.c | 3 ++-
> >  hw/arm/virt.c            | 8 ++++----
> >  include/hw/arm/virt.h    | 2 +-
> >  3 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index db44e2dd5f12..f7ce6aeed307 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -822,8 +822,9 @@ static const VMStateDescription vmstate_virt_acpi_build = {
> >      },
> >  };
> >  
> > -void virt_acpi_setup(VirtGuestInfo *guest_info)
> > +void virt_acpi_setup(VirtMachineState *vms)
> >  {
> > +    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
> >      AcpiBuildTables tables;
> >      AcpiBuildState *build_state;
> >  
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 7860fade52f1..bcf8a2954c16 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1156,9 +1156,9 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> >      return board->fdt;
> >  }
> >  
> > -static void virt_build_smbios(VirtGuestInfo *guest_info)
> > +static void virt_build_smbios(VirtMachineState *vms)
> Looks like all it needs is 'fw_cfg'
> If there aren't plans to use more data from VirtMachineState,
> just passing fw_cfg should be enough.

I prefer leaving it vms, as that's consistent with pretty much all
the other functions in this file, and if we do need more state then
it'll already be there. But I won't insist on this, it can always be
changed later.

Thanks,
drew

> 
> >  {
> > -    FWCfgState *fw_cfg = guest_info->fw_cfg;
> > +    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
> >      uint8_t *smbios_tables, *smbios_anchor;
> >      size_t smbios_tables_len, smbios_anchor_len;
> >      const char *product = "QEMU Virtual Machine";
> > @@ -1191,8 +1191,8 @@ void virt_machine_done(Notifier *notifier, void *data)
> >      VirtMachineState *vms = container_of(notifier, VirtMachineState,
> >                                           machine_done);
> >  
> > -    virt_acpi_setup(&vms->acpi_guest_info);
> > -    virt_build_smbios(&vms->acpi_guest_info);
> > +    virt_acpi_setup(vms);
> > +    virt_build_smbios(vms);
> >  }
> >  
> >  static void machvirt_init(MachineState *machine)
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index 77d25dbddb62..cbedc682ae99 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -128,6 +128,6 @@ typedef struct {
> >      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
> >  
> >  
> > -void virt_acpi_setup(VirtGuestInfo *guest_info);
> > +void virt_acpi_setup(VirtMachineState *vms);
> >  
> >  #endif /* QEMU_ARM_VIRT_H */
> 
> 

Re: [Qemu-devel] [PATCH 09/11] hw/arm/virt-acpi-build: don't save VirtGuestInfo on AcpiBuildState

[Andrew Jones]

On Thu, Dec 15, 2016 at 04:08:22PM +0100, Igor Mammedov wrote:
> On Tue, 13 Dec 2016 22:45:20 +0100
> Andrew Jones <drjones@redhat.com> wrote:
> 
> > We can get to VirtMachineState without the need for saving a pointer
> > on AcpiBuildState. This is the mach-virt equivalent to "acpi: Don't save
> > PcGuestInfo on AcpiBuildState"
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  hw/arm/virt-acpi-build.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 7a1efd0dc538..137781646bec 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -706,7 +706,6 @@ struct AcpiBuildState {
> >      MemoryRegion *linker_mr;
> >      /* Is table patched? */
> >      bool patched;
> > -    VirtGuestInfo *guest_info;
> >  } AcpiBuildState;
> >  
> >  static
> > @@ -779,6 +778,8 @@ static void acpi_ram_update(MemoryRegion *mr, GArray *data)
> >  
> >  static void virt_acpi_build_update(void *build_opaque)
> >  {
> > +    MachineState *machine = MACHINE(qdev_get_machine());
> > +    VirtMachineState *vms = VIRT_MACHINE(machine);
> >      AcpiBuildState *build_state = build_opaque;
> >      AcpiBuildTables tables;
> >  
> > @@ -790,8 +791,7 @@ static void virt_acpi_build_update(void *build_opaque)
> >  
> >      acpi_build_tables_init(&tables);
> >  
> > -    virt_acpi_build(container_of(build_state->guest_info,
> > -                                 VirtMachineState, acpi_guest_info), &tables);
> > +    virt_acpi_build(vms, &tables);
> 
> virt_acpi_build(VIRT_MACHINE(qdev_get_machine()), &tables);
> 
> or even better do it inside of virt_acpi_build(&tables);

Sounds good.

Thanks,
drew

> 
> 
> >  
> >      acpi_ram_update(build_state->table_mr, tables.table_data);
> >      acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
> > @@ -841,7 +841,6 @@ void virt_acpi_setup(VirtMachineState *vms)
> >      }
> >  
> >      build_state = g_malloc0(sizeof *build_state);
> > -    build_state->guest_info = guest_info;
> >  
> >      acpi_build_tables_init(&tables);
> >      virt_acpi_build(vms, &tables);
> 

Re: [Qemu-devel] [PATCH 10/11] hw/arm/virt: remove VirtGuestInfo

[Andrew Jones]

On Thu, Dec 15, 2016 at 03:59:35PM +0100, Igor Mammedov wrote:
> On Tue, 13 Dec 2016 22:45:21 +0100
> Andrew Jones <drjones@redhat.com> wrote:
> 
> > by moving VirtGuestInfo.fw_cfg to VirtMachineState. This is the
> > mach-virt equivalent of "pc: Move PcGuestInfo.fw_cfg to
> > PCMachineState" and "pc: Eliminate PcGuestInfo struct" combined.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  hw/arm/virt-acpi-build.c |  7 +++----
> >  hw/arm/virt.c            | 18 +++++++-----------
> >  include/hw/arm/virt.h    |  6 +-----
> >  3 files changed, 11 insertions(+), 20 deletions(-)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 137781646bec..0e6caf5f1083 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -826,11 +826,10 @@ static const VMStateDescription vmstate_virt_acpi_build = {
> >  
> >  void virt_acpi_setup(VirtMachineState *vms)
> >  {
> > -    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
> >      AcpiBuildTables tables;
> >      AcpiBuildState *build_state;
> >  
> > -    if (!guest_info->fw_cfg) {
> > +    if (!vms->fw_cfg) {
> >          trace_virt_acpi_setup();
> >          return;
> >      }
> > @@ -855,8 +854,8 @@ void virt_acpi_setup(VirtMachineState *vms)
> >          acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
> >                            "etc/table-loader", 0);
> >  
> > -    fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
> > -                    tables.tcpalog->data, acpi_data_len(tables.tcpalog));
> > +    fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
> > +                    acpi_data_len(tables.tcpalog));
> >  
> >      build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
> >                                                ACPI_BUILD_RSDP_FILE, 0);
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 2cb2e5b4f52a..348e23ff663c 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -914,15 +914,14 @@ static void create_flash(const VirtMachineState *vms,
> >      }
> >  }
> >  
> > -static void create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
> > +static void create_fw_cfg(VirtMachineState *vms, AddressSpace *as)
> >  {
> >      hwaddr base = vms->memmap[VIRT_FW_CFG].base;
> >      hwaddr size = vms->memmap[VIRT_FW_CFG].size;
> > -    FWCfgState *fw_cfg;
> >      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)vms->smp_cpus);
> > +    vms->fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
> > +    fw_cfg_add_i16(vms->fw_cfg, FW_CFG_NB_CPUS, (uint16_t)vms->smp_cpus);
> >  
> >      nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
> >      qemu_fdt_add_subnode(vms->fdt, nodename);
> > @@ -1158,12 +1157,11 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> >  
> >  static void virt_build_smbios(VirtMachineState *vms)
> >  {
> > -    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
> >      uint8_t *smbios_tables, *smbios_anchor;
> >      size_t smbios_tables_len, smbios_anchor_len;
> >      const char *product = "QEMU Virtual Machine";
> >  
> > -    if (!fw_cfg) {
> > +    if (!vms->fw_cfg) {
> >          return;
> >      }
> >  
> > @@ -1178,9 +1176,9 @@ static void virt_build_smbios(VirtMachineState *vms)
> >                        &smbios_anchor, &smbios_anchor_len);
> >  
> >      if (smbios_anchor) {
> > -        fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
> > +        fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-tables",
> >                          smbios_tables, smbios_tables_len);
> > -        fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
> > +        fw_cfg_add_file(vms->fw_cfg, "etc/smbios/smbios-anchor",
> >                          smbios_anchor, smbios_anchor_len);
> >      }
> >  }
> > @@ -1205,7 +1203,6 @@ static void machvirt_init(MachineState *machine)
> >      int n, virt_max_cpus;
> >      MemoryRegion *ram = g_new(MemoryRegion, 1);
> >      const char *cpu_model = machine->cpu_model;
> > -    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
> >      char **cpustr;
> >      ObjectClass *oc;
> >      const char *typename;
> > @@ -1414,9 +1411,8 @@ static void machvirt_init(MachineState *machine)
> >      create_virtio_devices(vms, pic);
> >  
> >      create_fw_cfg(vms, &address_space_memory);
> 
> above would look clearer as:
> vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

Agreed, only none of the other create_* functions in this
file have return values... Anyway, I'm fine either way.

Thanks,
drew

> 
> > -    rom_set_fw(fw_cfg_find());
> > +    rom_set_fw(vms->fw_cfg);
> >  
> > -    guest_info->fw_cfg = fw_cfg_find();
> >      vms->machine_done.notify = virt_machine_done;
> >      qemu_add_machine_init_done_notifier(&vms->machine_done);
> >  
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index 52b35478223e..ece3260bfeb5 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -81,10 +81,6 @@ typedef struct MemMapEntry {
> >      hwaddr size;
> >  } MemMapEntry;
> >  
> > -typedef struct VirtGuestInfo {
> > -    FWCfgState *fw_cfg;
> > -} VirtGuestInfo;
> > -
> >  typedef struct {
> >      MachineClass parent;
> >      bool disallow_affinity_adjustment;
> > @@ -95,8 +91,8 @@ typedef struct {
> >  
> >  typedef struct {
> >      MachineState parent;
> > -    VirtGuestInfo acpi_guest_info;
> >      Notifier machine_done;
> > +    FWCfgState *fw_cfg;
> >      bool secure;
> >      bool highmem;
> >      bool virt;
> 

Re: [Qemu-devel] [PATCH 00/11] Remove VirtGuestInfo

[Michael S. Tsirkin]

On Tue, Dec 13, 2016 at 10:45:11PM +0100, Andrew Jones wrote:
> This series is based on Peter's qemu-arm gicv3-virt branch. It's
> main goal (patches 04-10), which was suggested by Eduardo, is to
> remove an unnecessary structure, VirtGuestInfo, which is a
> maintenance burden, as it requires duplicating Virt machine state.
> Additionally patches 01-03 do some mach-virt cleanups and patch 11
> adds the ACPI equivalent to one of the DT patches in Peter's
> gicv3-virt branch.
> 
> The last patch isn't really tested as the guest kernel versions I
> used seem to ignore the GTDT irqflags and either just choose edge,
> 4.5-based kernel, or level, 4.9-based kernel.
> 
> Patches available here
> https://github.com/rhdrjones/qemu/commits/virt/remove-guest-info

I like this in general.
Acked-by: Michael S. Tsirkin <mst@redhat.com>

Pls merge through the ARM tree.

> 
> Andrew Jones (11):
>   hw/arm/virt: parameter passing cleanups
>   hw/arm/virt: use VirtMachineState.gic_version
>   hw/arm/virt: use VirtMachineState.smp_cpus
>   hw/arm/virt: eliminate struct VirtGuestInfoState
>   hw/arm/virt: remove include/hw/arm/virt-acpi-build.h
>   hw/arm/virt: move VirtMachineState/Class to virt.h
>   hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo
>   hw/arm/virt-acpi-build: remove redundant members from VirtGuestInfo
>   hw/arm/virt-acpi-build: don't save VirtGuestInfo on AcpiBuildState
>   hw/arm/virt: remove VirtGuestInfo
>   hw/arm/virt-acpi-build: Don't incorrectly claim architectural timer to
>     be edge-triggered
> 
>  MAINTAINERS                      |   2 -
>  hw/arm/virt-acpi-build.c         | 103 +++++++++++++-------------
>  hw/arm/virt.c                    | 152 ++++++++++++++-------------------------
>  include/hw/arm/virt-acpi-build.h |  47 ------------
>  include/hw/arm/virt.h            |  45 +++++++++++-
>  5 files changed, 151 insertions(+), 198 deletions(-)
>  delete mode 100644 include/hw/arm/virt-acpi-build.h
> 
> -- 
> 2.9.3

Re: [Qemu-devel] [PATCH 07/11] hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo

[Igor Mammedov]

On Thu, 15 Dec 2016 17:24:05 +0100
Andrew Jones <drjones@redhat.com> wrote:

> On Thu, Dec 15, 2016 at 03:45:55PM +0100, Igor Mammedov wrote:
> > On Tue, 13 Dec 2016 22:45:18 +0100
> > Andrew Jones <drjones@redhat.com> wrote:
> >   
> > > Only two functions take VirtGuestInfo parameters. Now that guest-info
> > > is part of VirtMachineState, and VirtMachineState is defined in the
> > > virt header, pass that instead.
> > > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> > > ---
> > >  hw/arm/virt-acpi-build.c | 3 ++-
> > >  hw/arm/virt.c            | 8 ++++----
> > >  include/hw/arm/virt.h    | 2 +-
> > >  3 files changed, 7 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > index db44e2dd5f12..f7ce6aeed307 100644
> > > --- a/hw/arm/virt-acpi-build.c
> > > +++ b/hw/arm/virt-acpi-build.c
> > > @@ -822,8 +822,9 @@ static const VMStateDescription vmstate_virt_acpi_build = {
> > >      },
> > >  };
> > >  
> > > -void virt_acpi_setup(VirtGuestInfo *guest_info)
> > > +void virt_acpi_setup(VirtMachineState *vms)
> > >  {
> > > +    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
> > >      AcpiBuildTables tables;
> > >      AcpiBuildState *build_state;
> > >  
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 7860fade52f1..bcf8a2954c16 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -1156,9 +1156,9 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> > >      return board->fdt;
> > >  }
> > >  
> > > -static void virt_build_smbios(VirtGuestInfo *guest_info)
> > > +static void virt_build_smbios(VirtMachineState *vms)  
> > Looks like all it needs is 'fw_cfg'
> > If there aren't plans to use more data from VirtMachineState,
> > just passing fw_cfg should be enough.  
> 
> I prefer leaving it vms, as that's consistent with pretty much all
> the other functions in this file, and if we do need more state then
> it'll already be there. But I won't insist on this, it can always be
> changed later.
fw_Cfg seems cleaner but I don't mind it as it's now.


> 
> Thanks,
> drew
> 
> >   
> > >  {
> > > -    FWCfgState *fw_cfg = guest_info->fw_cfg;
> > > +    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
> > >      uint8_t *smbios_tables, *smbios_anchor;
> > >      size_t smbios_tables_len, smbios_anchor_len;
> > >      const char *product = "QEMU Virtual Machine";
> > > @@ -1191,8 +1191,8 @@ void virt_machine_done(Notifier *notifier, void *data)
> > >      VirtMachineState *vms = container_of(notifier, VirtMachineState,
> > >                                           machine_done);
> > >  
> > > -    virt_acpi_setup(&vms->acpi_guest_info);
> > > -    virt_build_smbios(&vms->acpi_guest_info);
> > > +    virt_acpi_setup(vms);
> > > +    virt_build_smbios(vms);
> > >  }
> > >  
> > >  static void machvirt_init(MachineState *machine)
> > > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > > index 77d25dbddb62..cbedc682ae99 100644
> > > --- a/include/hw/arm/virt.h
> > > +++ b/include/hw/arm/virt.h
> > > @@ -128,6 +128,6 @@ typedef struct {
> > >      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
> > >  
> > >  
> > > -void virt_acpi_setup(VirtGuestInfo *guest_info);
> > > +void virt_acpi_setup(VirtMachineState *vms);
> > >  
> > >  #endif /* QEMU_ARM_VIRT_H */  
> > 
> >   
>