[PATCH v3 00/14] hw/i386: separate isapc out from pc_piix

[PATCH v3 00/14] hw/i386: separate isapc out from pc_piix

[Mark Cave-Ayland]

For various historical reasons the initialisation of the isapc machine is closely
intertwined with the initialisation of the pc machine, which is preventing some
future improvements to the pc machine initialisation logic.

Since the consensus [1] was that the isapc is still useful for testing and running
older OSs, this series splits the isapc machine and its main initialisation
routine pc_init_isa() into a separate isapc.c file to reduce the maintenance
burden on pc machine developers.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>

[1] https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg06137.html

(Patches still needing review: 1, 5, 7, 9, 13 and 14)


v3:
- Rebase onto master
- Add patch 1 to restrict isapc machine to 32-bit x86 CPUs as suggested by
  Philippe
- Include logic in patch 1 to handle the case if an isapc machine is
  launched with -cpu max as suggested by Daniel
- Add patch 13 to tidy-up pc_init1() for the i440fx-pc machine in the same
  way as patch 11 does for the isapc machine as suggested by Bernhard

v2:
- Rebase onto master to account for the fix in commit 0b006153b7
  ("hw/i386/pc_piix: Fix RTC ISA IRQ wiring of isapc machine")
- Replace verbatim MIT licence text with SPDX identifier as discussed
  with Daniel


Mark Cave-Ayland (14):
  hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
  hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init()
  hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation
    from pc_init_isa()
  hw/i386/pc_piix.c: remove SMI and piix4_pm initialisation from
    pc_init_isa()
  hw/i386/pc_piix.c: remove SGX initialisation from pc_init_isa()
  hw/i386/pc_piix.c: remove nvdimm initialisation from pc_init_isa()
  hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()
  hw/i386/pc_piix.c: hardcode hole64_size to 0 in pc_init_isa()
  hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from
    pc_init_isa()
  hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa()
  hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false in
    pc_init_isa()
  hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL in pc_init_isa()
  hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in
    pc_init1()
  hw/i386: move isapc machine to separate isapc.c file

 hw/i386/Kconfig     |   3 -
 hw/i386/isapc.c     | 169 ++++++++++++++++++++++++++++++++
 hw/i386/meson.build |   1 +
 hw/i386/pc_piix.c   | 231 +++++++++++++++-----------------------------
 4 files changed, 246 insertions(+), 158 deletions(-)
 create mode 100644 hw/i386/isapc.c

-- 
2.43.0

[PATCH v3 01/14] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs

[Mark Cave-Ayland]

The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
possible to specify any CPU via -cpu on the command line, it makes no
sense to allow modern 64-bit CPUs to be used.

Restrict the isapc machine to the available 32-bit CPUs, taking care to
handle the case where if a user inadvertently uses -cpu max then the "best"
32-bit CPU is used (in this case the pentium3).

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ea7572e783..67c52d79b2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
+    /*
+     * There is a small chance that someone unintentionally passes "-cpu max"
+     * for the isapc machine, which will provide a much more modern 32-bit
+     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
+     * been specified, choose the "best" 32-bit cpu possible which we consider
+     * be the pentium3 (deliberately choosing an Intel CPU given that the
+     * default 486 CPU for the isapc machine is also an Intel CPU).
+     */
+    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
+        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
+    }
+
     pc_init1(machine, NULL);
 }
 #endif
@@ -786,7 +798,19 @@ DEFINE_I440FX_MACHINE(2, 6);
 #ifdef CONFIG_ISAPC
 static void isapc_machine_options(MachineClass *m)
 {
+    static const char * const valid_cpu_types[] = {
+        X86_CPU_TYPE_NAME("486"),
+        X86_CPU_TYPE_NAME("athlon"),
+        X86_CPU_TYPE_NAME("kvm32"),
+        X86_CPU_TYPE_NAME("pentium"),
+        X86_CPU_TYPE_NAME("pentium2"),
+        X86_CPU_TYPE_NAME("pentium3"),
+        X86_CPU_TYPE_NAME("qemu32"),
+        X86_CPU_TYPE_NAME("max"),
+        NULL
+    };
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     m->desc = "ISA-only PC";
     m->max_cpus = 1;
     m->option_rom_has_mr = true;
@@ -799,6 +823,7 @@ static void isapc_machine_options(MachineClass *m)
     pcmc->has_reserved_memory = false;
     m->default_nic = "ne2k_isa";
     m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+    m->valid_cpu_types = valid_cpu_types;
     m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
 }
-- 
2.43.0

[PATCH v3 02/14] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init()

[Mark Cave-Ayland]

This is to prepare for splitting the isapc machine into its own separate file.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 261 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 260 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 67c52d79b2..fe53beb39b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -418,6 +418,87 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
+    const char *pci_type = NULL;
+    PCMachineState *pcms = PC_MACHINE(machine);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    X86MachineState *x86ms = X86_MACHINE(machine);
+    MemoryRegion *system_memory = get_system_memory();
+    MemoryRegion *system_io = get_system_io();
+    Object *phb = NULL;
+    ISABus *isa_bus;
+    Object *piix4_pm = NULL;
+    qemu_irq smi_irq;
+    GSIState *gsi_state;
+    MemoryRegion *ram_memory;
+    MemoryRegion *pci_memory = NULL;
+    MemoryRegion *rom_memory = system_memory;
+    ram_addr_t lowmem;
+    uint64_t hole64_size = 0;
+
+    /*
+     * Calculate ram split, for memory below and above 4G.  It's a bit
+     * complicated for backward compatibility reasons ...
+     *
+     *  - Traditional split is 3.5G (lowmem = 0xe0000000).  This is the
+     *    default value for max_ram_below_4g now.
+     *
+     *  - Then, to gigabyte align the memory, we move the split to 3G
+     *    (lowmem = 0xc0000000).  But only in case we have to split in
+     *    the first place, i.e. ram_size is larger than (traditional)
+     *    lowmem.  And for new machine types (gigabyte_align = true)
+     *    only, for live migration compatibility reasons.
+     *
+     *  - Next the max-ram-below-4g option was added, which allowed to
+     *    reduce lowmem to a smaller value, to allow a larger PCI I/O
+     *    window below 4G.  qemu doesn't enforce gigabyte alignment here,
+     *    but prints a warning.
+     *
+     *  - Finally max-ram-below-4g got updated to also allow raising lowmem,
+     *    so legacy non-PAE guests can get as much memory as possible in
+     *    the 32bit address space below 4G.
+     *
+     *  - Note that Xen has its own ram setup code in xen_ram_init(),
+     *    called via xen_hvm_init_pc().
+     *
+     * Examples:
+     *    qemu -M pc-1.7 -m 4G    (old default)    -> 3584M low,  512M high
+     *    qemu -M pc -m 4G        (new default)    -> 3072M low, 1024M high
+     *    qemu -M pc,max-ram-below-4g=2G -m 4G     -> 2048M low, 2048M high
+     *    qemu -M pc,max-ram-below-4g=4G -m 3968M  -> 3968M low (=4G-128M)
+     */
+    if (xen_enabled()) {
+        xen_hvm_init_pc(pcms, &ram_memory);
+    } else {
+        ram_memory = machine->ram;
+        if (!pcms->max_ram_below_4g) {
+            pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+        }
+        lowmem = pcms->max_ram_below_4g;
+        if (machine->ram_size >= pcms->max_ram_below_4g) {
+            if (pcmc->gigabyte_align) {
+                if (lowmem > 0xc0000000) {
+                    lowmem = 0xc0000000;
+                }
+                if (lowmem & (1 * GiB - 1)) {
+                    warn_report("Large machine and max_ram_below_4g "
+                                "(%" PRIu64 ") not a multiple of 1G; "
+                                "possible bad performance.",
+                                pcms->max_ram_below_4g);
+                }
+            }
+        }
+
+        if (machine->ram_size >= lowmem) {
+            x86ms->above_4g_mem_size = machine->ram_size - lowmem;
+            x86ms->below_4g_mem_size = lowmem;
+        } else {
+            x86ms->above_4g_mem_size = 0;
+            x86ms->below_4g_mem_size = machine->ram_size;
+        }
+    }
+
+    pc_machine_init_sgx_epc(pcms);
+
     /*
      * There is a small chance that someone unintentionally passes "-cpu max"
      * for the isapc machine, which will provide a much more modern 32-bit
@@ -430,7 +511,185 @@ static void pc_init_isa(MachineState *machine)
         machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
     }
 
-    pc_init1(machine, NULL);
+    x86_cpus_init(x86ms, pcmc->default_cpu_version);
+
+    if (kvm_enabled()) {
+        kvmclock_create(pcmc->kvmclock_create_always);
+    }
+
+    if (pcmc->pci_enabled) {
+        pci_memory = g_new(MemoryRegion, 1);
+        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
+        rom_memory = pci_memory;
+
+        phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
+        object_property_add_child(OBJECT(machine), "i440fx", phb);
+        object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
+                                 OBJECT(ram_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
+                                 OBJECT(pci_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
+                                 OBJECT(system_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
+                                 OBJECT(system_io), &error_fatal);
+        object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
+                                 x86ms->below_4g_mem_size, &error_fatal);
+        object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
+                                 x86ms->above_4g_mem_size, &error_fatal);
+        object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
+                                &error_fatal);
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+
+        pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
+        pci_bus_map_irqs(pcms->pcibus,
+                         xen_enabled() ? xen_pci_slot_get_pirq
+                                       : pc_pci_slot_get_pirq);
+
+        hole64_size = object_property_get_uint(phb,
+                                               PCI_HOST_PROP_PCI_HOLE64_SIZE,
+                                               &error_abort);
+    }
+
+    /* allocate ram and load rom/bios */
+    if (!xen_enabled()) {
+        pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
+    } else {
+        assert(machine->ram_size == x86ms->below_4g_mem_size +
+                                    x86ms->above_4g_mem_size);
+
+        pc_system_flash_cleanup_unused(pcms);
+        if (machine->kernel_filename != NULL) {
+            /* For xen HVM direct kernel boot, load linux here */
+            xen_load_linux(pcms);
+        }
+    }
+
+    gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
+
+    if (pcmc->pci_enabled) {
+        PCIDevice *pci_dev;
+        DeviceState *dev;
+        size_t i;
+
+        pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
+        object_property_set_bool(OBJECT(pci_dev), "has-usb",
+                                 machine_usb(machine), &error_abort);
+        object_property_set_bool(OBJECT(pci_dev), "has-acpi",
+                                 x86_machine_is_acpi_enabled(x86ms),
+                                 &error_abort);
+        object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
+                                 &error_abort);
+        object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
+                                 &error_abort);
+        qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
+        object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
+                                 x86_machine_is_smm_enabled(x86ms),
+                                 &error_abort);
+        dev = DEVICE(pci_dev);
+        for (i = 0; i < ISA_NUM_IRQS; i++) {
+            qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
+        }
+        pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
+
+        if (xen_enabled()) {
+            pci_device_set_intx_routing_notifier(
+                        pci_dev, piix_intx_routing_notifier_xen);
+
+            /*
+             * Xen supports additional interrupt routes from the PCI devices to
+             * the IOAPIC: the four pins of each PCI device on the bus are also
+             * connected to the IOAPIC directly.
+             * These additional routes can be discovered through ACPI.
+             */
+            pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
+                         XEN_IOAPIC_NUM_PIRQS);
+        }
+
+        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
+        x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+                                                              "rtc"));
+        piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+        pci_ide_create_devs(PCI_DEVICE(dev));
+        pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
+        pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
+    } else {
+        isa_bus = isa_bus_new(NULL, system_memory, system_io,
+                              &error_abort);
+        isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+
+        x86ms->rtc = isa_new(TYPE_MC146818_RTC);
+        qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
+        isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
+
+        i8257_dma_init(OBJECT(machine), isa_bus, 0);
+        pcms->hpet_enabled = false;
+    }
+
+    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+    }
+
+    if (phb) {
+        ioapic_init_gsi(gsi_state, phb);
+    }
+
+    if (tcg_enabled()) {
+        x86_register_ferr_irq(x86ms->gsi[13]);
+    }
+
+    pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
+
+    /* init basic PC hardware */
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
+                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
+
+    pc_nic_init(pcmc, isa_bus, pcms->pcibus);
+
+#ifdef CONFIG_IDE_ISA
+    if (!pcmc->pci_enabled) {
+        DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+        int i;
+
+        ide_drive_get(hd, ARRAY_SIZE(hd));
+        for (i = 0; i < MAX_IDE_BUS; i++) {
+            ISADevice *dev;
+            char busname[] = "ide.0";
+            dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+                               ide_irq[i],
+                               hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+            /*
+             * The ide bus name is ide.0 for the first bus and ide.1 for the
+             * second one.
+             */
+            busname[4] = '0' + i;
+            pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
+        }
+    }
+#endif
+
+    if (piix4_pm) {
+        smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
+
+        qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
+        pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
+        /* TODO: Populate SPD eeprom data.  */
+        smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
+
+        object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
+                                 TYPE_HOTPLUG_HANDLER,
+                                 (Object **)&x86ms->acpi_dev,
+                                 object_property_allow_set_link,
+                                 OBJ_PROP_LINK_STRONG);
+        object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
+                                 piix4_pm, &error_abort);
+    }
+
+    if (machine->nvdimms_state->is_enabled) {
+        nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
+                               x86_nvdimm_acpi_dsmio,
+                               x86ms->fw_cfg, OBJECT(pcms));
+    }
 }
 #endif
 
-- 
2.43.0

[PATCH v3 03/14] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa()

[Mark Cave-Ayland]

PCI code will never be used for an isapc machine.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 105 ++++------------------------------------------
 1 file changed, 8 insertions(+), 97 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fe53beb39b..5d59cbde09 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -418,19 +418,16 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
-    const char *pci_type = NULL;
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     X86MachineState *x86ms = X86_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *system_io = get_system_io();
-    Object *phb = NULL;
     ISABus *isa_bus;
     Object *piix4_pm = NULL;
     qemu_irq smi_irq;
     GSIState *gsi_state;
     MemoryRegion *ram_memory;
-    MemoryRegion *pci_memory = NULL;
     MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
@@ -517,39 +514,6 @@ static void pc_init_isa(MachineState *machine)
         kvmclock_create(pcmc->kvmclock_create_always);
     }
 
-    if (pcmc->pci_enabled) {
-        pci_memory = g_new(MemoryRegion, 1);
-        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
-        rom_memory = pci_memory;
-
-        phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
-        object_property_add_child(OBJECT(machine), "i440fx", phb);
-        object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
-                                 OBJECT(ram_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
-                                 OBJECT(pci_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
-                                 OBJECT(system_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
-                                 OBJECT(system_io), &error_fatal);
-        object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
-                                 x86ms->below_4g_mem_size, &error_fatal);
-        object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
-                                 x86ms->above_4g_mem_size, &error_fatal);
-        object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
-                                &error_fatal);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
-
-        pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
-        pci_bus_map_irqs(pcms->pcibus,
-                         xen_enabled() ? xen_pci_slot_get_pirq
-                                       : pc_pci_slot_get_pirq);
-
-        hole64_size = object_property_get_uint(phb,
-                                               PCI_HOST_PROP_PCI_HOLE64_SIZE,
-                                               &error_abort);
-    }
-
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
         pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
@@ -566,74 +530,21 @@ static void pc_init_isa(MachineState *machine)
 
     gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
 
-    if (pcmc->pci_enabled) {
-        PCIDevice *pci_dev;
-        DeviceState *dev;
-        size_t i;
-
-        pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
-        object_property_set_bool(OBJECT(pci_dev), "has-usb",
-                                 machine_usb(machine), &error_abort);
-        object_property_set_bool(OBJECT(pci_dev), "has-acpi",
-                                 x86_machine_is_acpi_enabled(x86ms),
-                                 &error_abort);
-        object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
-                                 &error_abort);
-        object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
-                                 &error_abort);
-        qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
-        object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
-                                 x86_machine_is_smm_enabled(x86ms),
-                                 &error_abort);
-        dev = DEVICE(pci_dev);
-        for (i = 0; i < ISA_NUM_IRQS; i++) {
-            qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
-        }
-        pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
-
-        if (xen_enabled()) {
-            pci_device_set_intx_routing_notifier(
-                        pci_dev, piix_intx_routing_notifier_xen);
-
-            /*
-             * Xen supports additional interrupt routes from the PCI devices to
-             * the IOAPIC: the four pins of each PCI device on the bus are also
-             * connected to the IOAPIC directly.
-             * These additional routes can be discovered through ACPI.
-             */
-            pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
-                         XEN_IOAPIC_NUM_PIRQS);
-        }
-
-        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
-        x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
-                                                              "rtc"));
-        piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
-        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
-        pci_ide_create_devs(PCI_DEVICE(dev));
-        pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
-        pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
-    } else {
-        isa_bus = isa_bus_new(NULL, system_memory, system_io,
-                              &error_abort);
-        isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+    isa_bus = isa_bus_new(NULL, system_memory, system_io,
+                            &error_abort);
+    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
 
-        x86ms->rtc = isa_new(TYPE_MC146818_RTC);
-        qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
-        isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
+    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
+    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
+    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
 
-        i8257_dma_init(OBJECT(machine), isa_bus, 0);
-        pcms->hpet_enabled = false;
-    }
+    i8257_dma_init(OBJECT(machine), isa_bus, 0);
+    pcms->hpet_enabled = false;
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
         pc_i8259_create(isa_bus, gsi_state->i8259_irq);
     }
 
-    if (phb) {
-        ioapic_init_gsi(gsi_state, phb);
-    }
-
     if (tcg_enabled()) {
         x86_register_ferr_irq(x86ms->gsi[13]);
     }
-- 
2.43.0

[PATCH v3 04/14] hw/i386/pc_piix.c: remove SMI and piix4_pm initialisation from pc_init_isa()

[Mark Cave-Ayland]

These are based upon the PIIX4 PCI chipset and so can never be used on an isapc machine.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5d59cbde09..baaf744edb 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -424,8 +424,6 @@ static void pc_init_isa(MachineState *machine)
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *system_io = get_system_io();
     ISABus *isa_bus;
-    Object *piix4_pm = NULL;
-    qemu_irq smi_irq;
     GSIState *gsi_state;
     MemoryRegion *ram_memory;
     MemoryRegion *rom_memory = system_memory;
@@ -579,23 +577,6 @@ static void pc_init_isa(MachineState *machine)
     }
 #endif
 
-    if (piix4_pm) {
-        smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
-
-        qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
-        pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
-        /* TODO: Populate SPD eeprom data.  */
-        smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
-
-        object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
-                                 TYPE_HOTPLUG_HANDLER,
-                                 (Object **)&x86ms->acpi_dev,
-                                 object_property_allow_set_link,
-                                 OBJ_PROP_LINK_STRONG);
-        object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
-                                 piix4_pm, &error_abort);
-    }
-
     if (machine->nvdimms_state->is_enabled) {
         nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
                                x86_nvdimm_acpi_dsmio,
-- 
2.43.0

[PATCH v3 05/14] hw/i386/pc_piix.c: remove SGX initialisation from pc_init_isa()

[Mark Cave-Ayland]

The Intel SGX instructions only exist on recent CPUs and so would never be available
on a CPU from the pre-PCI era.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/pc_piix.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index baaf744edb..75226aa775 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -492,8 +492,6 @@ static void pc_init_isa(MachineState *machine)
         }
     }
 
-    pc_machine_init_sgx_epc(pcms);
-
     /*
      * There is a small chance that someone unintentionally passes "-cpu max"
      * for the isapc machine, which will provide a much more modern 32-bit
-- 
2.43.0

[PATCH v3 06/14] hw/i386/pc_piix.c: remove nvdimm initialisation from pc_init_isa()

[Mark Cave-Ayland]

NVDIMMs cannot be used by PCs from a pre-PCI era.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 75226aa775..a6a35405ba 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -574,12 +574,6 @@ static void pc_init_isa(MachineState *machine)
         }
     }
 #endif
-
-    if (machine->nvdimms_state->is_enabled) {
-        nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
-                               x86_nvdimm_acpi_dsmio,
-                               x86ms->fw_cfg, OBJECT(pcms));
-    }
 }
 #endif
 
-- 
2.43.0

[PATCH v3 07/14] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()

[Mark Cave-Ayland]

All isapc machines must have 32-bit CPUs and so the RAM split logic can be hardcoded
accordingly.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/pc_piix.c | 58 ++++-------------------------------------------
 1 file changed, 4 insertions(+), 54 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a6a35405ba..838ab9e15e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -427,69 +427,19 @@ static void pc_init_isa(MachineState *machine)
     GSIState *gsi_state;
     MemoryRegion *ram_memory;
     MemoryRegion *rom_memory = system_memory;
-    ram_addr_t lowmem;
     uint64_t hole64_size = 0;
 
     /*
-     * Calculate ram split, for memory below and above 4G.  It's a bit
-     * complicated for backward compatibility reasons ...
-     *
-     *  - Traditional split is 3.5G (lowmem = 0xe0000000).  This is the
-     *    default value for max_ram_below_4g now.
-     *
-     *  - Then, to gigabyte align the memory, we move the split to 3G
-     *    (lowmem = 0xc0000000).  But only in case we have to split in
-     *    the first place, i.e. ram_size is larger than (traditional)
-     *    lowmem.  And for new machine types (gigabyte_align = true)
-     *    only, for live migration compatibility reasons.
-     *
-     *  - Next the max-ram-below-4g option was added, which allowed to
-     *    reduce lowmem to a smaller value, to allow a larger PCI I/O
-     *    window below 4G.  qemu doesn't enforce gigabyte alignment here,
-     *    but prints a warning.
-     *
-     *  - Finally max-ram-below-4g got updated to also allow raising lowmem,
-     *    so legacy non-PAE guests can get as much memory as possible in
-     *    the 32bit address space below 4G.
-     *
-     *  - Note that Xen has its own ram setup code in xen_ram_init(),
-     *    called via xen_hvm_init_pc().
-     *
-     * Examples:
-     *    qemu -M pc-1.7 -m 4G    (old default)    -> 3584M low,  512M high
-     *    qemu -M pc -m 4G        (new default)    -> 3072M low, 1024M high
-     *    qemu -M pc,max-ram-below-4g=2G -m 4G     -> 2048M low, 2048M high
-     *    qemu -M pc,max-ram-below-4g=4G -m 3968M  -> 3968M low (=4G-128M)
+     * There is no RAM split for the isapc machine
      */
     if (xen_enabled()) {
         xen_hvm_init_pc(pcms, &ram_memory);
     } else {
         ram_memory = machine->ram;
-        if (!pcms->max_ram_below_4g) {
-            pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
-        }
-        lowmem = pcms->max_ram_below_4g;
-        if (machine->ram_size >= pcms->max_ram_below_4g) {
-            if (pcmc->gigabyte_align) {
-                if (lowmem > 0xc0000000) {
-                    lowmem = 0xc0000000;
-                }
-                if (lowmem & (1 * GiB - 1)) {
-                    warn_report("Large machine and max_ram_below_4g "
-                                "(%" PRIu64 ") not a multiple of 1G; "
-                                "possible bad performance.",
-                                pcms->max_ram_below_4g);
-                }
-            }
-        }
 
-        if (machine->ram_size >= lowmem) {
-            x86ms->above_4g_mem_size = machine->ram_size - lowmem;
-            x86ms->below_4g_mem_size = lowmem;
-        } else {
-            x86ms->above_4g_mem_size = 0;
-            x86ms->below_4g_mem_size = machine->ram_size;
-        }
+        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+        x86ms->above_4g_mem_size = 0;
+        x86ms->below_4g_mem_size = machine->ram_size;
     }
 
     /*
-- 
2.43.0

[PATCH v3 08/14] hw/i386/pc_piix.c: hardcode hole64_size to 0 in pc_init_isa()

[Mark Cave-Ayland]

All isapc machines must have 32-bit CPUs and have no PCI 64-bit hole so it can be
hardcoded to 0.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 838ab9e15e..f07ae6da26 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -427,7 +427,6 @@ static void pc_init_isa(MachineState *machine)
     GSIState *gsi_state;
     MemoryRegion *ram_memory;
     MemoryRegion *rom_memory = system_memory;
-    uint64_t hole64_size = 0;
 
     /*
      * There is no RAM split for the isapc machine
@@ -462,7 +461,7 @@ static void pc_init_isa(MachineState *machine)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
+        pc_memory_init(pcms, system_memory, rom_memory, 0);
     } else {
         assert(machine->ram_size == x86ms->below_4g_mem_size +
                                     x86ms->above_4g_mem_size);
-- 
2.43.0

[PATCH v3 09/14] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa()

[Mark Cave-Ayland]

This function contains 'assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled)' and so we can
safely assume that it should never be used for the isapc machine.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/pc_piix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f07ae6da26..c3de1d830c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -466,7 +466,6 @@ static void pc_init_isa(MachineState *machine)
         assert(machine->ram_size == x86ms->below_4g_mem_size +
                                     x86ms->above_4g_mem_size);
 
-        pc_system_flash_cleanup_unused(pcms);
         if (machine->kernel_filename != NULL) {
             /* For xen HVM direct kernel boot, load linux here */
             xen_load_linux(pcms);
-- 
2.43.0

[PATCH v3 10/14] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa()

[Mark Cave-Ayland]

By definition an isapc machine must always use ISA IDE drives so ensure that they
are always enabled. At the same time also remove the surrounding CONFIG_IDE_ISA
define since it will be enabled via the ISAPC Kconfig.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c3de1d830c..571a8d1ca6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -427,6 +427,8 @@ static void pc_init_isa(MachineState *machine)
     GSIState *gsi_state;
     MemoryRegion *ram_memory;
     MemoryRegion *rom_memory = system_memory;
+    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+    int i;
 
     /*
      * There is no RAM split for the isapc machine
@@ -501,27 +503,20 @@ static void pc_init_isa(MachineState *machine)
 
     pc_nic_init(pcmc, isa_bus, pcms->pcibus);
 
-#ifdef CONFIG_IDE_ISA
-    if (!pcmc->pci_enabled) {
-        DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-        int i;
-
-        ide_drive_get(hd, ARRAY_SIZE(hd));
-        for (i = 0; i < MAX_IDE_BUS; i++) {
-            ISADevice *dev;
-            char busname[] = "ide.0";
-            dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
-                               ide_irq[i],
-                               hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
-            /*
-             * The ide bus name is ide.0 for the first bus and ide.1 for the
-             * second one.
-             */
-            busname[4] = '0' + i;
-            pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
-        }
+    ide_drive_get(hd, ARRAY_SIZE(hd));
+    for (i = 0; i < MAX_IDE_BUS; i++) {
+        ISADevice *dev;
+        char busname[] = "ide.0";
+        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+                            ide_irq[i],
+                            hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+        /*
+         * The ide bus name is ide.0 for the first bus and ide.1 for the
+         * second one.
+         */
+        busname[4] = '0' + i;
+        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
     }
-#endif
 }
 #endif
 
-- 
2.43.0

[PATCH v3 11/14] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false in pc_init_isa()

[Mark Cave-Ayland]

By definition PCI can never be enabled on an isapc machine so hardcode the relevant values
set via pcmc->pci_enabled.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 571a8d1ca6..4e3fb17506 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -474,7 +474,7 @@ static void pc_init_isa(MachineState *machine)
         }
     }
 
-    gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
+    gsi_state = pc_gsi_create(&x86ms->gsi, false);
 
     isa_bus = isa_bus_new(NULL, system_memory, system_io,
                             &error_abort);
@@ -495,7 +495,7 @@ static void pc_init_isa(MachineState *machine)
         x86_register_ferr_irq(x86ms->gsi[13]);
     }
 
-    pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
+    pc_vga_init(isa_bus, NULL);
 
     /* init basic PC hardware */
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
-- 
2.43.0

[PATCH v3 12/14] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL in pc_init_isa()

[Mark Cave-Ayland]

By definition PCI can never be enabled on an isapc machine so hardcode the PCIBus argument
of pc_nic_init() to NULL.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4e3fb17506..76b8543bfe 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -501,7 +501,7 @@ static void pc_init_isa(MachineState *machine)
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
                          !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
 
-    pc_nic_init(pcmc, isa_bus, pcms->pcibus);
+    pc_nic_init(pcmc, isa_bus, NULL);
 
     ide_drive_get(hd, ARRAY_SIZE(hd));
     for (i = 0; i < MAX_IDE_BUS; i++) {
-- 
2.43.0

[PATCH v3 13/14] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1()

[Mark Cave-Ayland]

PCI is always enabled on the pc-i440fx machine so hardcode the relevant logic
in pc_init1().

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/pc_piix.c | 194 ++++++++++++++++++----------------------------
 1 file changed, 76 insertions(+), 118 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 76b8543bfe..c9d8a1cdf7 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -71,7 +71,7 @@
 
 #define XEN_IOAPIC_NUM_PIRQS 128ULL
 
-#ifdef CONFIG_IDE_ISA
+#ifdef CONFIG_ISAPC
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
@@ -118,6 +118,9 @@ static void pc_init1(MachineState *machine, const char *pci_type)
     MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
+    PCIDevice *pci_dev;
+    DeviceState *dev;
+    size_t i;
 
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
@@ -188,38 +191,36 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         kvmclock_create(pcmc->kvmclock_create_always);
     }
 
-    if (pcmc->pci_enabled) {
-        pci_memory = g_new(MemoryRegion, 1);
-        memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
-        rom_memory = pci_memory;
-
-        phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
-        object_property_add_child(OBJECT(machine), "i440fx", phb);
-        object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
-                                 OBJECT(ram_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
-                                 OBJECT(pci_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
-                                 OBJECT(system_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
-                                 OBJECT(system_io), &error_fatal);
-        object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
-                                 x86ms->below_4g_mem_size, &error_fatal);
-        object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
-                                 x86ms->above_4g_mem_size, &error_fatal);
-        object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
-                                &error_fatal);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
-
-        pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
-        pci_bus_map_irqs(pcms->pcibus,
-                         xen_enabled() ? xen_pci_slot_get_pirq
-                                       : pc_pci_slot_get_pirq);
-
-        hole64_size = object_property_get_uint(phb,
-                                               PCI_HOST_PROP_PCI_HOLE64_SIZE,
-                                               &error_abort);
-    }
+    pci_memory = g_new(MemoryRegion, 1);
+    memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
+    rom_memory = pci_memory;
+
+    phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
+    object_property_add_child(OBJECT(machine), "i440fx", phb);
+    object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
+                                OBJECT(ram_memory), &error_fatal);
+    object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
+                                OBJECT(pci_memory), &error_fatal);
+    object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
+                                OBJECT(system_memory), &error_fatal);
+    object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
+                                OBJECT(system_io), &error_fatal);
+    object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
+                                x86ms->below_4g_mem_size, &error_fatal);
+    object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
+                                x86ms->above_4g_mem_size, &error_fatal);
+    object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
+                            &error_fatal);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+
+    pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
+    pci_bus_map_irqs(pcms->pcibus,
+                        xen_enabled() ? xen_pci_slot_get_pirq
+                                    : pc_pci_slot_get_pirq);
+
+    hole64_size = object_property_get_uint(phb,
+                                            PCI_HOST_PROP_PCI_HOLE64_SIZE,
+                                            &error_abort);
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
@@ -235,72 +236,51 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         }
     }
 
-    gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
-
-    if (pcmc->pci_enabled) {
-        PCIDevice *pci_dev;
-        DeviceState *dev;
-        size_t i;
-
-        pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
-        object_property_set_bool(OBJECT(pci_dev), "has-usb",
-                                 machine_usb(machine), &error_abort);
-        object_property_set_bool(OBJECT(pci_dev), "has-acpi",
-                                 x86_machine_is_acpi_enabled(x86ms),
-                                 &error_abort);
-        object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
-                                 &error_abort);
-        object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
-                                 &error_abort);
-        qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
-        object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
-                                 x86_machine_is_smm_enabled(x86ms),
-                                 &error_abort);
-        dev = DEVICE(pci_dev);
-        for (i = 0; i < ISA_NUM_IRQS; i++) {
-            qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
-        }
-        pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
-
-        if (xen_enabled()) {
-            pci_device_set_intx_routing_notifier(
-                        pci_dev, piix_intx_routing_notifier_xen);
-
-            /*
-             * Xen supports additional interrupt routes from the PCI devices to
-             * the IOAPIC: the four pins of each PCI device on the bus are also
-             * connected to the IOAPIC directly.
-             * These additional routes can be discovered through ACPI.
-             */
-            pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
-                         XEN_IOAPIC_NUM_PIRQS);
-        }
+    gsi_state = pc_gsi_create(&x86ms->gsi, true);
+
+    pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
+    object_property_set_bool(OBJECT(pci_dev), "has-usb",
+                                machine_usb(machine), &error_abort);
+    object_property_set_bool(OBJECT(pci_dev), "has-acpi",
+                                x86_machine_is_acpi_enabled(x86ms),
+                                &error_abort);
+    object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
+                                &error_abort);
+    object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
+                                &error_abort);
+    qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
+    object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
+                                x86_machine_is_smm_enabled(x86ms),
+                                &error_abort);
+    dev = DEVICE(pci_dev);
+    for (i = 0; i < ISA_NUM_IRQS; i++) {
+        qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
+    }
+    pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
 
-        isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
-        x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
-                                                              "rtc"));
-        piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
-        dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
-        pci_ide_create_devs(PCI_DEVICE(dev));
-        pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
-        pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
-    } else {
-        uint32_t irq;
+    if (xen_enabled()) {
+        pci_device_set_intx_routing_notifier(
+                    pci_dev, piix_intx_routing_notifier_xen);
 
-        isa_bus = isa_bus_new(NULL, system_memory, system_io,
-                              &error_abort);
-        isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+        /*
+         * Xen supports additional interrupt routes from the PCI devices to
+         * the IOAPIC: the four pins of each PCI device on the bus are also
+         * connected to the IOAPIC directly.
+         * These additional routes can be discovered through ACPI.
+         */
+        pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
+                        XEN_IOAPIC_NUM_PIRQS);
+    }
 
-        x86ms->rtc = isa_new(TYPE_MC146818_RTC);
-        qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
-        isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
-        irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
-                                       &error_fatal);
-        isa_connect_gpio_out(ISA_DEVICE(x86ms->rtc), 0, irq);
+    isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
+    x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+                                                            "rtc"));
+    piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+    dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+    pci_ide_create_devs(PCI_DEVICE(dev));
+    pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
+    pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
 
-        i8257_dma_init(OBJECT(machine), isa_bus, 0);
-        pcms->hpet_enabled = false;
-    }
 
     if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
         pc_i8259_create(isa_bus, gsi_state->i8259_irq);
@@ -314,7 +294,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         x86_register_ferr_irq(x86ms->gsi[13]);
     }
 
-    pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
+    pc_vga_init(isa_bus, pcms->pcibus);
 
     /* init basic PC hardware */
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
@@ -322,28 +302,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
 
     pc_nic_init(pcmc, isa_bus, pcms->pcibus);
 
-#ifdef CONFIG_IDE_ISA
-    if (!pcmc->pci_enabled) {
-        DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-        int i;
-
-        ide_drive_get(hd, ARRAY_SIZE(hd));
-        for (i = 0; i < MAX_IDE_BUS; i++) {
-            ISADevice *dev;
-            char busname[] = "ide.0";
-            dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
-                               ide_irq[i],
-                               hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
-            /*
-             * The ide bus name is ide.0 for the first bus and ide.1 for the
-             * second one.
-             */
-            busname[4] = '0' + i;
-            pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
-        }
-    }
-#endif
-
     if (piix4_pm) {
         smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
 
-- 
2.43.0

[PATCH v3 14/14] hw/i386: move isapc machine to separate isapc.c file

[Mark Cave-Ayland]

Now that pc_init_isa() is independent of any PCI initialisation, move it into a
separate isapc.c file. This enables us to finally fix the dependency of ISAPC on
I440FX in hw/i386/Kconfig.

Note that as part of the move to a separate file we can see that the licence text
is a verbatim copy of the MIT licence. The text originates from commit 1df912cf9e
("VL license of the day is MIT/BSD") so we can be sure that this was the original
intent. As a consequence we can update the file header to use a SPDX tag as per
the current project contribution guidelines.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/i386/Kconfig     |   3 -
 hw/i386/isapc.c     | 169 ++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/meson.build |   1 +
 hw/i386/pc_piix.c   | 148 --------------------------------------
 4 files changed, 170 insertions(+), 151 deletions(-)
 create mode 100644 hw/i386/isapc.c

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index eb65bda6e0..a7c746fe9e 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -96,9 +96,6 @@ config ISAPC
     select ISA_BUS
     select PC
     select IDE_ISA
-    # FIXME: it is in the same file as i440fx, and does not compile
-    # if separated
-    depends on I440FX
 
 config Q35
     bool
diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
new file mode 100644
index 0000000000..5ac077a860
--- /dev/null
+++ b/hw/i386/isapc.c
@@ -0,0 +1,169 @@
+/*
+ * QEMU PC System Emulator
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/char/parallel-isa.h"
+#include "hw/dma/i8257.h"
+#include "hw/loader.h"
+#include "hw/i386/pc.h"
+#include "hw/ide/isa.h"
+#include "hw/ide/ide-bus.h"
+#include "system/kvm.h"
+#include "hw/i386/kvm/clock.h"
+#include "hw/xen/xen-x86.h"
+#include "system/xen.h"
+#include "hw/rtc/mc146818rtc.h"
+#include "target/i386/cpu.h"
+
+static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
+static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
+static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
+
+
+static void pc_init_isa(MachineState *machine)
+{
+    PCMachineState *pcms = PC_MACHINE(machine);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    X86MachineState *x86ms = X86_MACHINE(machine);
+    MemoryRegion *system_memory = get_system_memory();
+    MemoryRegion *system_io = get_system_io();
+    ISABus *isa_bus;
+    GSIState *gsi_state;
+    MemoryRegion *ram_memory;
+    MemoryRegion *rom_memory = system_memory;
+    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+    uint32_t irq;
+    int i;
+
+    /*
+     * There is no RAM split for the isapc machine
+     */
+    if (xen_enabled()) {
+        xen_hvm_init_pc(pcms, &ram_memory);
+    } else {
+        ram_memory = machine->ram;
+
+        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+        x86ms->above_4g_mem_size = 0;
+        x86ms->below_4g_mem_size = machine->ram_size;
+    }
+
+    /*
+     * There is a small chance that someone unintentionally passes "-cpu max"
+     * for the isapc machine, which will provide a much more modern 32-bit
+     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
+     * been specified, choose the "best" 32-bit cpu possible which we consider
+     * be the pentium3 (deliberately choosing an Intel CPU given that the
+     * default 486 CPU for the isapc machine is also an Intel CPU).
+     */
+    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
+        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
+    }
+
+    x86_cpus_init(x86ms, pcmc->default_cpu_version);
+
+    if (kvm_enabled()) {
+        kvmclock_create(pcmc->kvmclock_create_always);
+    }
+
+    /* allocate ram and load rom/bios */
+    if (!xen_enabled()) {
+        pc_memory_init(pcms, system_memory, rom_memory, 0);
+    } else {
+        assert(machine->ram_size == x86ms->below_4g_mem_size +
+                                    x86ms->above_4g_mem_size);
+
+        if (machine->kernel_filename != NULL) {
+            /* For xen HVM direct kernel boot, load linux here */
+            xen_load_linux(pcms);
+        }
+    }
+
+    gsi_state = pc_gsi_create(&x86ms->gsi, false);
+
+    isa_bus = isa_bus_new(NULL, system_memory, system_io,
+                            &error_abort);
+    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+
+    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
+    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
+    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
+    irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
+                                   &error_fatal);
+    isa_connect_gpio_out(ISA_DEVICE(x86ms->rtc), 0, irq);
+
+    i8257_dma_init(OBJECT(machine), isa_bus, 0);
+    pcms->hpet_enabled = false;
+
+    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+    }
+
+    if (tcg_enabled()) {
+        x86_register_ferr_irq(x86ms->gsi[13]);
+    }
+
+    pc_vga_init(isa_bus, NULL);
+
+    /* init basic PC hardware */
+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
+                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
+
+    pc_nic_init(pcmc, isa_bus, NULL);
+
+    ide_drive_get(hd, ARRAY_SIZE(hd));
+    for (i = 0; i < MAX_IDE_BUS; i++) {
+        ISADevice *dev;
+        char busname[] = "ide.0";
+        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+                           ide_irq[i],
+                           hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+        /*
+         * The ide bus name is ide.0 for the first bus and ide.1 for the
+         * second one.
+         */
+        busname[4] = '0' + i;
+        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
+    }
+}
+
+static void isapc_machine_options(MachineClass *m)
+{
+    static const char * const valid_cpu_types[] = {
+        X86_CPU_TYPE_NAME("486"),
+        X86_CPU_TYPE_NAME("athlon"),
+        X86_CPU_TYPE_NAME("kvm32"),
+        X86_CPU_TYPE_NAME("pentium"),
+        X86_CPU_TYPE_NAME("pentium2"),
+        X86_CPU_TYPE_NAME("pentium3"),
+        X86_CPU_TYPE_NAME("qemu32"),
+        X86_CPU_TYPE_NAME("max"),
+        NULL
+    };
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
+    m->desc = "ISA-only PC";
+    m->max_cpus = 1;
+    m->option_rom_has_mr = true;
+    m->rom_file_has_mr = false;
+    pcmc->pci_enabled = false;
+    pcmc->has_acpi_build = false;
+    pcmc->smbios_defaults = false;
+    pcmc->gigabyte_align = false;
+    pcmc->smbios_legacy_mode = true;
+    pcmc->has_reserved_memory = false;
+    m->default_nic = "ne2k_isa";
+    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+    m->valid_cpu_types = valid_cpu_types;
+    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
+    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
+}
+
+DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
+                  isapc_machine_options);
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 7896f348cf..436b3ce52d 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -14,6 +14,7 @@ i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
 i386_ss.add(when: 'CONFIG_AMD_IOMMU', if_true: files('amd_iommu.c'),
                                       if_false: files('amd_iommu-stub.c'))
 i386_ss.add(when: 'CONFIG_I440FX', if_true: files('pc_piix.c'))
+i386_ss.add(when: 'CONFIG_ISAPC', if_true: files('isapc.c'))
 i386_ss.add(when: 'CONFIG_MICROVM', if_true: files('x86-common.c', 'microvm.c', 'acpi-microvm.c', 'microvm-dt.c'))
 i386_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: files('nitro_enclave.c'))
 i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c9d8a1cdf7..8d0dfd881d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -71,12 +71,6 @@
 
 #define XEN_IOAPIC_NUM_PIRQS 128ULL
 
-#ifdef CONFIG_ISAPC
-static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
-static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
-static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
-#endif
-
 /*
  * Return the global irq number corresponding to a given device irq
  * pin. We could also use the bus number to have a more precise mapping.
@@ -373,111 +367,6 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
     pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
 }
 
-#ifdef CONFIG_ISAPC
-static void pc_init_isa(MachineState *machine)
-{
-    PCMachineState *pcms = PC_MACHINE(machine);
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-    X86MachineState *x86ms = X86_MACHINE(machine);
-    MemoryRegion *system_memory = get_system_memory();
-    MemoryRegion *system_io = get_system_io();
-    ISABus *isa_bus;
-    GSIState *gsi_state;
-    MemoryRegion *ram_memory;
-    MemoryRegion *rom_memory = system_memory;
-    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    int i;
-
-    /*
-     * There is no RAM split for the isapc machine
-     */
-    if (xen_enabled()) {
-        xen_hvm_init_pc(pcms, &ram_memory);
-    } else {
-        ram_memory = machine->ram;
-
-        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
-        x86ms->above_4g_mem_size = 0;
-        x86ms->below_4g_mem_size = machine->ram_size;
-    }
-
-    /*
-     * There is a small chance that someone unintentionally passes "-cpu max"
-     * for the isapc machine, which will provide a much more modern 32-bit
-     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
-     * been specified, choose the "best" 32-bit cpu possible which we consider
-     * be the pentium3 (deliberately choosing an Intel CPU given that the
-     * default 486 CPU for the isapc machine is also an Intel CPU).
-     */
-    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
-        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
-    }
-
-    x86_cpus_init(x86ms, pcmc->default_cpu_version);
-
-    if (kvm_enabled()) {
-        kvmclock_create(pcmc->kvmclock_create_always);
-    }
-
-    /* allocate ram and load rom/bios */
-    if (!xen_enabled()) {
-        pc_memory_init(pcms, system_memory, rom_memory, 0);
-    } else {
-        assert(machine->ram_size == x86ms->below_4g_mem_size +
-                                    x86ms->above_4g_mem_size);
-
-        if (machine->kernel_filename != NULL) {
-            /* For xen HVM direct kernel boot, load linux here */
-            xen_load_linux(pcms);
-        }
-    }
-
-    gsi_state = pc_gsi_create(&x86ms->gsi, false);
-
-    isa_bus = isa_bus_new(NULL, system_memory, system_io,
-                            &error_abort);
-    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
-
-    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
-    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
-    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
-
-    i8257_dma_init(OBJECT(machine), isa_bus, 0);
-    pcms->hpet_enabled = false;
-
-    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
-        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
-    }
-
-    if (tcg_enabled()) {
-        x86_register_ferr_irq(x86ms->gsi[13]);
-    }
-
-    pc_vga_init(isa_bus, NULL);
-
-    /* init basic PC hardware */
-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
-                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
-
-    pc_nic_init(pcmc, isa_bus, NULL);
-
-    ide_drive_get(hd, ARRAY_SIZE(hd));
-    for (i = 0; i < MAX_IDE_BUS; i++) {
-        ISADevice *dev;
-        char busname[] = "ide.0";
-        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
-                            ide_irq[i],
-                            hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
-        /*
-         * The ide bus name is ide.0 for the first bus and ide.1 for the
-         * second one.
-         */
-        busname[4] = '0' + i;
-        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
-    }
-}
-#endif
-
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init_pci(MachineState *machine)
 {
@@ -839,43 +728,6 @@ static void pc_i440fx_machine_2_6_options(MachineClass *m)
 
 DEFINE_I440FX_MACHINE(2, 6);
 
-#ifdef CONFIG_ISAPC
-static void isapc_machine_options(MachineClass *m)
-{
-    static const char * const valid_cpu_types[] = {
-        X86_CPU_TYPE_NAME("486"),
-        X86_CPU_TYPE_NAME("athlon"),
-        X86_CPU_TYPE_NAME("kvm32"),
-        X86_CPU_TYPE_NAME("pentium"),
-        X86_CPU_TYPE_NAME("pentium2"),
-        X86_CPU_TYPE_NAME("pentium3"),
-        X86_CPU_TYPE_NAME("qemu32"),
-        X86_CPU_TYPE_NAME("max"),
-        NULL
-    };
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-
-    m->desc = "ISA-only PC";
-    m->max_cpus = 1;
-    m->option_rom_has_mr = true;
-    m->rom_file_has_mr = false;
-    pcmc->pci_enabled = false;
-    pcmc->has_acpi_build = false;
-    pcmc->smbios_defaults = false;
-    pcmc->gigabyte_align = false;
-    pcmc->smbios_legacy_mode = true;
-    pcmc->has_reserved_memory = false;
-    m->default_nic = "ne2k_isa";
-    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
-    m->valid_cpu_types = valid_cpu_types;
-    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
-    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
-}
-
-DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
-                  isapc_machine_options);
-#endif
-
 #ifdef CONFIG_XEN
 static void xenfv_machine_4_2_options(MachineClass *m)
 {
-- 
2.43.0

Re: [PATCH v3 14/14] hw/i386: move isapc machine to separate isapc.c file

[Bernhard Beschow]

Am 4. Juli 2025 14:09:41 UTC schrieb Mark Cave-Ayland <mark.caveayland@nutanix.com>:
>Now that pc_init_isa() is independent of any PCI initialisation, move it into a
>separate isapc.c file. This enables us to finally fix the dependency of ISAPC on
>I440FX in hw/i386/Kconfig.
>
>Note that as part of the move to a separate file we can see that the licence text
>is a verbatim copy of the MIT licence. The text originates from commit 1df912cf9e
>("VL license of the day is MIT/BSD") so we can be sure that this was the original
>intent. As a consequence we can update the file header to use a SPDX tag as per
>the current project contribution guidelines.
>
>Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>---
> hw/i386/Kconfig     |   3 -
> hw/i386/isapc.c     | 169 ++++++++++++++++++++++++++++++++++++++++++++
> hw/i386/meson.build |   1 +
> hw/i386/pc_piix.c   | 148 --------------------------------------
> 4 files changed, 170 insertions(+), 151 deletions(-)
> create mode 100644 hw/i386/isapc.c
>
>diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
>index eb65bda6e0..a7c746fe9e 100644
>--- a/hw/i386/Kconfig
>+++ b/hw/i386/Kconfig
>@@ -96,9 +96,6 @@ config ISAPC
>     select ISA_BUS
>     select PC
>     select IDE_ISA
>-    # FIXME: it is in the same file as i440fx, and does not compile
>-    # if separated
>-    depends on I440FX

Yay!

> 
> config Q35
>     bool
>diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>new file mode 100644
>index 0000000000..5ac077a860
>--- /dev/null
>+++ b/hw/i386/isapc.c
>@@ -0,0 +1,169 @@
>+/*
>+ * QEMU PC System Emulator
>+ *
>+ * Copyright (c) 2003-2004 Fabrice Bellard
>+ *
>+ * SPDX-License-Identifier: MIT
>+ */
>+
>+#include "qemu/osdep.h"
>+
>+#include "hw/char/parallel-isa.h"
>+#include "hw/dma/i8257.h"
>+#include "hw/loader.h"

Is loader.h used here? It seems to be unneeded in pc_piix already, so no need for copying it here.

>+#include "hw/i386/pc.h"
>+#include "hw/ide/isa.h"
>+#include "hw/ide/ide-bus.h"
>+#include "system/kvm.h"
>+#include "hw/i386/kvm/clock.h"
>+#include "hw/xen/xen-x86.h"
>+#include "system/xen.h"
>+#include "hw/rtc/mc146818rtc.h"
>+#include "target/i386/cpu.h"

i8257.h, mc146818rtc.h, and probably ide/isa.h can now be removed from pc_piix since these are either instantiated in the southbridge or not used there.

>+
>+static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>+static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>+static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>+
>+
>+static void pc_init_isa(MachineState *machine)
>+{
>+    PCMachineState *pcms = PC_MACHINE(machine);
>+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>+    X86MachineState *x86ms = X86_MACHINE(machine);
>+    MemoryRegion *system_memory = get_system_memory();
>+    MemoryRegion *system_io = get_system_io();
>+    ISABus *isa_bus;
>+    GSIState *gsi_state;
>+    MemoryRegion *ram_memory;
>+    MemoryRegion *rom_memory = system_memory;

rom_memory isn't needed any more since system_memory can be used directly. Same for pc_piix where pci_memory can be used directly (see pc_q35).

>+    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>+    uint32_t irq;
>+    int i;
>+
>+    /*
>+     * There is no RAM split for the isapc machine
>+     */
>+    if (xen_enabled()) {
>+        xen_hvm_init_pc(pcms, &ram_memory);
>+    } else {
>+        ram_memory = machine->ram;
>+
>+        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
>+        x86ms->above_4g_mem_size = 0;
>+        x86ms->below_4g_mem_size = machine->ram_size;
>+    }
>+
>+    /*
>+     * There is a small chance that someone unintentionally passes "-cpu max"
>+     * for the isapc machine, which will provide a much more modern 32-bit
>+     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
>+     * been specified, choose the "best" 32-bit cpu possible which we consider
>+     * be the pentium3 (deliberately choosing an Intel CPU given that the
>+     * default 486 CPU for the isapc machine is also an Intel CPU).
>+     */
>+    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
>+        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
>+    }
>+
>+    x86_cpus_init(x86ms, pcmc->default_cpu_version);
>+
>+    if (kvm_enabled()) {
>+        kvmclock_create(pcmc->kvmclock_create_always);
>+    }
>+
>+    /* allocate ram and load rom/bios */
>+    if (!xen_enabled()) {
>+        pc_memory_init(pcms, system_memory, rom_memory, 0);
>+    } else {
>+        assert(machine->ram_size == x86ms->below_4g_mem_size +
>+                                    x86ms->above_4g_mem_size);
>+
>+        if (machine->kernel_filename != NULL) {
>+            /* For xen HVM direct kernel boot, load linux here */
>+            xen_load_linux(pcms);
>+        }
>+    }
>+
>+    gsi_state = pc_gsi_create(&x86ms->gsi, false);
>+
>+    isa_bus = isa_bus_new(NULL, system_memory, system_io,
>+                            &error_abort);
>+    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
>+
>+    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
>+    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
>+    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
>+    irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
>+                                   &error_fatal);
>+    isa_connect_gpio_out(ISA_DEVICE(x86ms->rtc), 0, irq);
>+
>+    i8257_dma_init(OBJECT(machine), isa_bus, 0);
>+    pcms->hpet_enabled = false;
>+
>+    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
>+        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
>+    }
>+
>+    if (tcg_enabled()) {
>+        x86_register_ferr_irq(x86ms->gsi[13]);
>+    }
>+
>+    pc_vga_init(isa_bus, NULL);
>+
>+    /* init basic PC hardware */
>+    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
>+                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
>+
>+    pc_nic_init(pcmc, isa_bus, NULL);
>+
>+    ide_drive_get(hd, ARRAY_SIZE(hd));
>+    for (i = 0; i < MAX_IDE_BUS; i++) {
>+        ISADevice *dev;
>+        char busname[] = "ide.0";
>+        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>+                           ide_irq[i],
>+                           hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
>+        /*
>+         * The ide bus name is ide.0 for the first bus and ide.1 for the
>+         * second one.
>+         */
>+        busname[4] = '0' + i;
>+        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
>+    }
>+}
>+
>+static void isapc_machine_options(MachineClass *m)
>+{
>+    static const char * const valid_cpu_types[] = {
>+        X86_CPU_TYPE_NAME("486"),
>+        X86_CPU_TYPE_NAME("athlon"),
>+        X86_CPU_TYPE_NAME("kvm32"),
>+        X86_CPU_TYPE_NAME("pentium"),
>+        X86_CPU_TYPE_NAME("pentium2"),
>+        X86_CPU_TYPE_NAME("pentium3"),
>+        X86_CPU_TYPE_NAME("qemu32"),
>+        X86_CPU_TYPE_NAME("max"),
>+        NULL
>+    };
>+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>+
>+    m->desc = "ISA-only PC";
>+    m->max_cpus = 1;
>+    m->option_rom_has_mr = true;
>+    m->rom_file_has_mr = false;
>+    pcmc->pci_enabled = false;
>+    pcmc->has_acpi_build = false;
>+    pcmc->smbios_defaults = false;
>+    pcmc->gigabyte_align = false;
>+    pcmc->smbios_legacy_mode = true;
>+    pcmc->has_reserved_memory = false;
>+    m->default_nic = "ne2k_isa";
>+    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
>+    m->valid_cpu_types = valid_cpu_types;
>+    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
>+    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
>+}
>+
>+DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
>+                  isapc_machine_options);
>diff --git a/hw/i386/meson.build b/hw/i386/meson.build
>index 7896f348cf..436b3ce52d 100644
>--- a/hw/i386/meson.build
>+++ b/hw/i386/meson.build
>@@ -14,6 +14,7 @@ i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
> i386_ss.add(when: 'CONFIG_AMD_IOMMU', if_true: files('amd_iommu.c'),
>                                       if_false: files('amd_iommu-stub.c'))
> i386_ss.add(when: 'CONFIG_I440FX', if_true: files('pc_piix.c'))
>+i386_ss.add(when: 'CONFIG_ISAPC', if_true: files('isapc.c'))
> i386_ss.add(when: 'CONFIG_MICROVM', if_true: files('x86-common.c', 'microvm.c', 'acpi-microvm.c', 'microvm-dt.c'))
> i386_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: files('nitro_enclave.c'))
> i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index c9d8a1cdf7..8d0dfd881d 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -71,12 +71,6 @@
> 
> #define XEN_IOAPIC_NUM_PIRQS 128ULL
> 
>-#ifdef CONFIG_ISAPC
>-static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>-static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>-static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>-#endif
>-
> /*
>  * Return the global irq number corresponding to a given device irq
>  * pin. We could also use the bus number to have a more precise mapping.
>@@ -373,111 +367,6 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
>     pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
> }
> 
>-#ifdef CONFIG_ISAPC
>-static void pc_init_isa(MachineState *machine)
>-{
>-    PCMachineState *pcms = PC_MACHINE(machine);
>-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>-    X86MachineState *x86ms = X86_MACHINE(machine);
>-    MemoryRegion *system_memory = get_system_memory();
>-    MemoryRegion *system_io = get_system_io();
>-    ISABus *isa_bus;
>-    GSIState *gsi_state;
>-    MemoryRegion *ram_memory;
>-    MemoryRegion *rom_memory = system_memory;
>-    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>-    int i;
>-
>-    /*
>-     * There is no RAM split for the isapc machine
>-     */
>-    if (xen_enabled()) {
>-        xen_hvm_init_pc(pcms, &ram_memory);
>-    } else {
>-        ram_memory = machine->ram;
>-
>-        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
>-        x86ms->above_4g_mem_size = 0;
>-        x86ms->below_4g_mem_size = machine->ram_size;
>-    }
>-
>-    /*
>-     * There is a small chance that someone unintentionally passes "-cpu max"
>-     * for the isapc machine, which will provide a much more modern 32-bit
>-     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
>-     * been specified, choose the "best" 32-bit cpu possible which we consider
>-     * be the pentium3 (deliberately choosing an Intel CPU given that the
>-     * default 486 CPU for the isapc machine is also an Intel CPU).
>-     */
>-    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
>-        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
>-    }
>-
>-    x86_cpus_init(x86ms, pcmc->default_cpu_version);
>-
>-    if (kvm_enabled()) {
>-        kvmclock_create(pcmc->kvmclock_create_always);
>-    }
>-
>-    /* allocate ram and load rom/bios */
>-    if (!xen_enabled()) {
>-        pc_memory_init(pcms, system_memory, rom_memory, 0);
>-    } else {
>-        assert(machine->ram_size == x86ms->below_4g_mem_size +
>-                                    x86ms->above_4g_mem_size);
>-
>-        if (machine->kernel_filename != NULL) {
>-            /* For xen HVM direct kernel boot, load linux here */
>-            xen_load_linux(pcms);
>-        }
>-    }
>-
>-    gsi_state = pc_gsi_create(&x86ms->gsi, false);
>-
>-    isa_bus = isa_bus_new(NULL, system_memory, system_io,
>-                            &error_abort);
>-    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
>-
>-    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
>-    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
>-    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
>-
>-    i8257_dma_init(OBJECT(machine), isa_bus, 0);
>-    pcms->hpet_enabled = false;
>-
>-    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
>-        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
>-    }
>-
>-    if (tcg_enabled()) {
>-        x86_register_ferr_irq(x86ms->gsi[13]);
>-    }
>-
>-    pc_vga_init(isa_bus, NULL);
>-
>-    /* init basic PC hardware */
>-    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
>-                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
>-
>-    pc_nic_init(pcmc, isa_bus, NULL);
>-
>-    ide_drive_get(hd, ARRAY_SIZE(hd));
>-    for (i = 0; i < MAX_IDE_BUS; i++) {
>-        ISADevice *dev;
>-        char busname[] = "ide.0";
>-        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>-                            ide_irq[i],
>-                            hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
>-        /*
>-         * The ide bus name is ide.0 for the first bus and ide.1 for the
>-         * second one.
>-         */
>-        busname[4] = '0' + i;
>-        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
>-    }
>-}
>-#endif
>-
> #ifdef CONFIG_XEN
> static void pc_xen_hvm_init_pci(MachineState *machine)
> {
>@@ -839,43 +728,6 @@ static void pc_i440fx_machine_2_6_options(MachineClass *m)
> 
> DEFINE_I440FX_MACHINE(2, 6);
> 
>-#ifdef CONFIG_ISAPC
>-static void isapc_machine_options(MachineClass *m)
>-{
>-    static const char * const valid_cpu_types[] = {
>-        X86_CPU_TYPE_NAME("486"),
>-        X86_CPU_TYPE_NAME("athlon"),
>-        X86_CPU_TYPE_NAME("kvm32"),
>-        X86_CPU_TYPE_NAME("pentium"),
>-        X86_CPU_TYPE_NAME("pentium2"),
>-        X86_CPU_TYPE_NAME("pentium3"),
>-        X86_CPU_TYPE_NAME("qemu32"),
>-        X86_CPU_TYPE_NAME("max"),
>-        NULL
>-    };
>-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>-
>-    m->desc = "ISA-only PC";
>-    m->max_cpus = 1;
>-    m->option_rom_has_mr = true;
>-    m->rom_file_has_mr = false;
>-    pcmc->pci_enabled = false;
>-    pcmc->has_acpi_build = false;
>-    pcmc->smbios_defaults = false;
>-    pcmc->gigabyte_align = false;
>-    pcmc->smbios_legacy_mode = true;
>-    pcmc->has_reserved_memory = false;
>-    m->default_nic = "ne2k_isa";
>-    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
>-    m->valid_cpu_types = valid_cpu_types;
>-    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
>-    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
>-}
>-
>-DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
>-                  isapc_machine_options);
>-#endif
>-
> #ifdef CONFIG_XEN
> static void xenfv_machine_4_2_options(MachineClass *m)
> {

With above comments addressed:

Reviewed-by: Bernhard Beschow shentey@gmail.com>

Re: [PATCH v3 14/14] hw/i386: move isapc machine to separate isapc.c file

[Mark Cave-Ayland]

On 08/07/2025 18:36, Bernhard Beschow wrote:

> Am 4. Juli 2025 14:09:41 UTC schrieb Mark Cave-Ayland <mark.caveayland@nutanix.com>:
>> Now that pc_init_isa() is independent of any PCI initialisation, move it into a
>> separate isapc.c file. This enables us to finally fix the dependency of ISAPC on
>> I440FX in hw/i386/Kconfig.
>>
>> Note that as part of the move to a separate file we can see that the licence text
>> is a verbatim copy of the MIT licence. The text originates from commit 1df912cf9e
>> ("VL license of the day is MIT/BSD") so we can be sure that this was the original
>> intent. As a consequence we can update the file header to use a SPDX tag as per
>> the current project contribution guidelines.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>> hw/i386/Kconfig     |   3 -
>> hw/i386/isapc.c     | 169 ++++++++++++++++++++++++++++++++++++++++++++
>> hw/i386/meson.build |   1 +
>> hw/i386/pc_piix.c   | 148 --------------------------------------
>> 4 files changed, 170 insertions(+), 151 deletions(-)
>> create mode 100644 hw/i386/isapc.c
>>
>> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
>> index eb65bda6e0..a7c746fe9e 100644
>> --- a/hw/i386/Kconfig
>> +++ b/hw/i386/Kconfig
>> @@ -96,9 +96,6 @@ config ISAPC
>>      select ISA_BUS
>>      select PC
>>      select IDE_ISA
>> -    # FIXME: it is in the same file as i440fx, and does not compile
>> -    # if separated
>> -    depends on I440FX
> 
> Yay!
> 
>>
>> config Q35
>>      bool
>> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>> new file mode 100644
>> index 0000000000..5ac077a860
>> --- /dev/null
>> +++ b/hw/i386/isapc.c
>> @@ -0,0 +1,169 @@
>> +/*
>> + * QEMU PC System Emulator
>> + *
>> + * Copyright (c) 2003-2004 Fabrice Bellard
>> + *
>> + * SPDX-License-Identifier: MIT
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "hw/char/parallel-isa.h"
>> +#include "hw/dma/i8257.h"
>> +#include "hw/loader.h"
> 
> Is loader.h used here? It seems to be unneeded in pc_piix already, so no need for copying it here.
> 
>> +#include "hw/i386/pc.h"
>> +#include "hw/ide/isa.h"
>> +#include "hw/ide/ide-bus.h"
>> +#include "system/kvm.h"
>> +#include "hw/i386/kvm/clock.h"
>> +#include "hw/xen/xen-x86.h"
>> +#include "system/xen.h"
>> +#include "hw/rtc/mc146818rtc.h"
>> +#include "target/i386/cpu.h"
> 
> i8257.h, mc146818rtc.h, and probably ide/isa.h can now be removed from pc_piix since these are either instantiated in the southbridge or not used there.
> 
>> +
>> +static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>> +static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>> +static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>> +
>> +
>> +static void pc_init_isa(MachineState *machine)
>> +{
>> +    PCMachineState *pcms = PC_MACHINE(machine);
>> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>> +    X86MachineState *x86ms = X86_MACHINE(machine);
>> +    MemoryRegion *system_memory = get_system_memory();
>> +    MemoryRegion *system_io = get_system_io();
>> +    ISABus *isa_bus;
>> +    GSIState *gsi_state;
>> +    MemoryRegion *ram_memory;
>> +    MemoryRegion *rom_memory = system_memory;
> 
> rom_memory isn't needed any more since system_memory can be used directly. Same for pc_piix where pci_memory can be used directly (see pc_q35).
> 
>> +    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> +    uint32_t irq;
>> +    int i;
>> +
>> +    /*
>> +     * There is no RAM split for the isapc machine
>> +     */
>> +    if (xen_enabled()) {
>> +        xen_hvm_init_pc(pcms, &ram_memory);
>> +    } else {
>> +        ram_memory = machine->ram;
>> +
>> +        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
>> +        x86ms->above_4g_mem_size = 0;
>> +        x86ms->below_4g_mem_size = machine->ram_size;
>> +    }
>> +
>> +    /*
>> +     * There is a small chance that someone unintentionally passes "-cpu max"
>> +     * for the isapc machine, which will provide a much more modern 32-bit
>> +     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
>> +     * been specified, choose the "best" 32-bit cpu possible which we consider
>> +     * be the pentium3 (deliberately choosing an Intel CPU given that the
>> +     * default 486 CPU for the isapc machine is also an Intel CPU).
>> +     */
>> +    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
>> +        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
>> +    }
>> +
>> +    x86_cpus_init(x86ms, pcmc->default_cpu_version);
>> +
>> +    if (kvm_enabled()) {
>> +        kvmclock_create(pcmc->kvmclock_create_always);
>> +    }
>> +
>> +    /* allocate ram and load rom/bios */
>> +    if (!xen_enabled()) {
>> +        pc_memory_init(pcms, system_memory, rom_memory, 0);
>> +    } else {
>> +        assert(machine->ram_size == x86ms->below_4g_mem_size +
>> +                                    x86ms->above_4g_mem_size);
>> +
>> +        if (machine->kernel_filename != NULL) {
>> +            /* For xen HVM direct kernel boot, load linux here */
>> +            xen_load_linux(pcms);
>> +        }
>> +    }
>> +
>> +    gsi_state = pc_gsi_create(&x86ms->gsi, false);
>> +
>> +    isa_bus = isa_bus_new(NULL, system_memory, system_io,
>> +                            &error_abort);
>> +    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
>> +
>> +    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
>> +    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
>> +    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
>> +    irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
>> +                                   &error_fatal);
>> +    isa_connect_gpio_out(ISA_DEVICE(x86ms->rtc), 0, irq);
>> +
>> +    i8257_dma_init(OBJECT(machine), isa_bus, 0);
>> +    pcms->hpet_enabled = false;
>> +
>> +    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
>> +        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
>> +    }
>> +
>> +    if (tcg_enabled()) {
>> +        x86_register_ferr_irq(x86ms->gsi[13]);
>> +    }
>> +
>> +    pc_vga_init(isa_bus, NULL);
>> +
>> +    /* init basic PC hardware */
>> +    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
>> +                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
>> +
>> +    pc_nic_init(pcmc, isa_bus, NULL);
>> +
>> +    ide_drive_get(hd, ARRAY_SIZE(hd));
>> +    for (i = 0; i < MAX_IDE_BUS; i++) {
>> +        ISADevice *dev;
>> +        char busname[] = "ide.0";
>> +        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>> +                           ide_irq[i],
>> +                           hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
>> +        /*
>> +         * The ide bus name is ide.0 for the first bus and ide.1 for the
>> +         * second one.
>> +         */
>> +        busname[4] = '0' + i;
>> +        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
>> +    }
>> +}
>> +
>> +static void isapc_machine_options(MachineClass *m)
>> +{
>> +    static const char * const valid_cpu_types[] = {
>> +        X86_CPU_TYPE_NAME("486"),
>> +        X86_CPU_TYPE_NAME("athlon"),
>> +        X86_CPU_TYPE_NAME("kvm32"),
>> +        X86_CPU_TYPE_NAME("pentium"),
>> +        X86_CPU_TYPE_NAME("pentium2"),
>> +        X86_CPU_TYPE_NAME("pentium3"),
>> +        X86_CPU_TYPE_NAME("qemu32"),
>> +        X86_CPU_TYPE_NAME("max"),
>> +        NULL
>> +    };
>> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>> +
>> +    m->desc = "ISA-only PC";
>> +    m->max_cpus = 1;
>> +    m->option_rom_has_mr = true;
>> +    m->rom_file_has_mr = false;
>> +    pcmc->pci_enabled = false;
>> +    pcmc->has_acpi_build = false;
>> +    pcmc->smbios_defaults = false;
>> +    pcmc->gigabyte_align = false;
>> +    pcmc->smbios_legacy_mode = true;
>> +    pcmc->has_reserved_memory = false;
>> +    m->default_nic = "ne2k_isa";
>> +    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
>> +    m->valid_cpu_types = valid_cpu_types;
>> +    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
>> +    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
>> +}
>> +
>> +DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
>> +                  isapc_machine_options);
>> diff --git a/hw/i386/meson.build b/hw/i386/meson.build
>> index 7896f348cf..436b3ce52d 100644
>> --- a/hw/i386/meson.build
>> +++ b/hw/i386/meson.build
>> @@ -14,6 +14,7 @@ i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
>> i386_ss.add(when: 'CONFIG_AMD_IOMMU', if_true: files('amd_iommu.c'),
>>                                        if_false: files('amd_iommu-stub.c'))
>> i386_ss.add(when: 'CONFIG_I440FX', if_true: files('pc_piix.c'))
>> +i386_ss.add(when: 'CONFIG_ISAPC', if_true: files('isapc.c'))
>> i386_ss.add(when: 'CONFIG_MICROVM', if_true: files('x86-common.c', 'microvm.c', 'acpi-microvm.c', 'microvm-dt.c'))
>> i386_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: files('nitro_enclave.c'))
>> i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index c9d8a1cdf7..8d0dfd881d 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -71,12 +71,6 @@
>>
>> #define XEN_IOAPIC_NUM_PIRQS 128ULL
>>
>> -#ifdef CONFIG_ISAPC
>> -static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>> -static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>> -static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>> -#endif
>> -
>> /*
>>   * Return the global irq number corresponding to a given device irq
>>   * pin. We could also use the bus number to have a more precise mapping.
>> @@ -373,111 +367,6 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
>>      pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
>> }
>>
>> -#ifdef CONFIG_ISAPC
>> -static void pc_init_isa(MachineState *machine)
>> -{
>> -    PCMachineState *pcms = PC_MACHINE(machine);
>> -    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>> -    X86MachineState *x86ms = X86_MACHINE(machine);
>> -    MemoryRegion *system_memory = get_system_memory();
>> -    MemoryRegion *system_io = get_system_io();
>> -    ISABus *isa_bus;
>> -    GSIState *gsi_state;
>> -    MemoryRegion *ram_memory;
>> -    MemoryRegion *rom_memory = system_memory;
>> -    DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> -    int i;
>> -
>> -    /*
>> -     * There is no RAM split for the isapc machine
>> -     */
>> -    if (xen_enabled()) {
>> -        xen_hvm_init_pc(pcms, &ram_memory);
>> -    } else {
>> -        ram_memory = machine->ram;
>> -
>> -        pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
>> -        x86ms->above_4g_mem_size = 0;
>> -        x86ms->below_4g_mem_size = machine->ram_size;
>> -    }
>> -
>> -    /*
>> -     * There is a small chance that someone unintentionally passes "-cpu max"
>> -     * for the isapc machine, which will provide a much more modern 32-bit
>> -     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
>> -     * been specified, choose the "best" 32-bit cpu possible which we consider
>> -     * be the pentium3 (deliberately choosing an Intel CPU given that the
>> -     * default 486 CPU for the isapc machine is also an Intel CPU).
>> -     */
>> -    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
>> -        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
>> -    }
>> -
>> -    x86_cpus_init(x86ms, pcmc->default_cpu_version);
>> -
>> -    if (kvm_enabled()) {
>> -        kvmclock_create(pcmc->kvmclock_create_always);
>> -    }
>> -
>> -    /* allocate ram and load rom/bios */
>> -    if (!xen_enabled()) {
>> -        pc_memory_init(pcms, system_memory, rom_memory, 0);
>> -    } else {
>> -        assert(machine->ram_size == x86ms->below_4g_mem_size +
>> -                                    x86ms->above_4g_mem_size);
>> -
>> -        if (machine->kernel_filename != NULL) {
>> -            /* For xen HVM direct kernel boot, load linux here */
>> -            xen_load_linux(pcms);
>> -        }
>> -    }
>> -
>> -    gsi_state = pc_gsi_create(&x86ms->gsi, false);
>> -
>> -    isa_bus = isa_bus_new(NULL, system_memory, system_io,
>> -                            &error_abort);
>> -    isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
>> -
>> -    x86ms->rtc = isa_new(TYPE_MC146818_RTC);
>> -    qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
>> -    isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
>> -
>> -    i8257_dma_init(OBJECT(machine), isa_bus, 0);
>> -    pcms->hpet_enabled = false;
>> -
>> -    if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
>> -        pc_i8259_create(isa_bus, gsi_state->i8259_irq);
>> -    }
>> -
>> -    if (tcg_enabled()) {
>> -        x86_register_ferr_irq(x86ms->gsi[13]);
>> -    }
>> -
>> -    pc_vga_init(isa_bus, NULL);
>> -
>> -    /* init basic PC hardware */
>> -    pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
>> -                         !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
>> -
>> -    pc_nic_init(pcmc, isa_bus, NULL);
>> -
>> -    ide_drive_get(hd, ARRAY_SIZE(hd));
>> -    for (i = 0; i < MAX_IDE_BUS; i++) {
>> -        ISADevice *dev;
>> -        char busname[] = "ide.0";
>> -        dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
>> -                            ide_irq[i],
>> -                            hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
>> -        /*
>> -         * The ide bus name is ide.0 for the first bus and ide.1 for the
>> -         * second one.
>> -         */
>> -        busname[4] = '0' + i;
>> -        pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
>> -    }
>> -}
>> -#endif
>> -
>> #ifdef CONFIG_XEN
>> static void pc_xen_hvm_init_pci(MachineState *machine)
>> {
>> @@ -839,43 +728,6 @@ static void pc_i440fx_machine_2_6_options(MachineClass *m)
>>
>> DEFINE_I440FX_MACHINE(2, 6);
>>
>> -#ifdef CONFIG_ISAPC
>> -static void isapc_machine_options(MachineClass *m)
>> -{
>> -    static const char * const valid_cpu_types[] = {
>> -        X86_CPU_TYPE_NAME("486"),
>> -        X86_CPU_TYPE_NAME("athlon"),
>> -        X86_CPU_TYPE_NAME("kvm32"),
>> -        X86_CPU_TYPE_NAME("pentium"),
>> -        X86_CPU_TYPE_NAME("pentium2"),
>> -        X86_CPU_TYPE_NAME("pentium3"),
>> -        X86_CPU_TYPE_NAME("qemu32"),
>> -        X86_CPU_TYPE_NAME("max"),
>> -        NULL
>> -    };
>> -    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>> -
>> -    m->desc = "ISA-only PC";
>> -    m->max_cpus = 1;
>> -    m->option_rom_has_mr = true;
>> -    m->rom_file_has_mr = false;
>> -    pcmc->pci_enabled = false;
>> -    pcmc->has_acpi_build = false;
>> -    pcmc->smbios_defaults = false;
>> -    pcmc->gigabyte_align = false;
>> -    pcmc->smbios_legacy_mode = true;
>> -    pcmc->has_reserved_memory = false;
>> -    m->default_nic = "ne2k_isa";
>> -    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
>> -    m->valid_cpu_types = valid_cpu_types;
>> -    m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
>> -    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
>> -}
>> -
>> -DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
>> -                  isapc_machine_options);
>> -#endif
>> -
>> #ifdef CONFIG_XEN
>> static void xenfv_machine_4_2_options(MachineClass *m)
>> {
> 
> With above comments addressed:
> 
> Reviewed-by: Bernhard Beschow shentey@gmail.com>

Thanks for the review! All the suggestions look good, so I've included 
them in the next version of the series.


ATB,

Mark.

Re: [PATCH v3 01/14] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs

[Philippe Mathieu-Daudé]

On 4/7/25 16:09, Mark Cave-Ayland wrote:
> The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
> possible to specify any CPU via -cpu on the command line, it makes no
> sense to allow modern 64-bit CPUs to be used.
> 
> Restrict the isapc machine to the available 32-bit CPUs, taking care to
> handle the case where if a user inadvertently uses -cpu max then the "best"
> 32-bit CPU is used (in this case the pentium3).
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>   hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index ea7572e783..67c52d79b2 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
>   #ifdef CONFIG_ISAPC
>   static void pc_init_isa(MachineState *machine)
>   {
> +    /*
> +     * There is a small chance that someone unintentionally passes "-cpu max"
> +     * for the isapc machine, which will provide a much more modern 32-bit
> +     * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
> +     * been specified, choose the "best" 32-bit cpu possible which we consider
> +     * be the pentium3 (deliberately choosing an Intel CPU given that the
> +     * default 486 CPU for the isapc machine is also an Intel CPU).
> +     */
> +    if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
> +        machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");

We should warn() the user about using "pentium3" for "max".

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +    }
> +
>       pc_init1(machine, NULL);
>   }

Re: [PATCH v3 09/14] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa()

[Philippe Mathieu-Daudé]

On 4/7/25 16:09, Mark Cave-Ayland wrote:
> This function contains 'assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled)' and so we can
> safely assume that it should never be used for the isapc machine.
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>   hw/i386/pc_piix.c | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 02/14] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init()

[Philippe Mathieu-Daudé]

On 4/7/25 16:09, Mark Cave-Ayland wrote:
> This is to prepare for splitting the isapc machine into its own separate file.
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/i386/pc_piix.c | 261 +++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 260 insertions(+), 1 deletion(-)

Further possible cleanup:

-- >8 --
     hw/i386/pc_piix: Inline pc_xen_hvm_init_pci()

     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ede7b68946b..4cec4f44764 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -362,14 +362,6 @@ static void pc_set_south_bridge(Object *obj, int 
value, Error **errp)
  }

  #ifdef CONFIG_XEN
-static void pc_xen_hvm_init_pci(MachineState *machine)
-{
-    const char *pci_type = xen_igd_gfx_pt_enabled() ?
-                TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : 
TYPE_I440FX_PCI_DEVICE;
-
-    pc_init1(machine, pci_type);
-}
-
  static void pc_xen_hvm_init(MachineState *machine)
  {
      PCMachineState *pcms = PC_MACHINE(machine);
@@ -379,7 +371,9 @@ static void pc_xen_hvm_init(MachineState *machine)
          exit(1);
      }

-    pc_xen_hvm_init_pci(machine);
+    pc_init1(machine, xen_igd_gfx_pt_enabled()
+                      ? TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
+                      : TYPE_I440FX_PCI_DEVICE);
      xen_igd_reserve_slot(pcms->pcibus);
      pci_create_simple(pcms->pcibus, -1, "xen-platform");
  }

---

Re: [PATCH v3 13/14] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1()

[Philippe Mathieu-Daudé]

On 4/7/25 16:09, Mark Cave-Ayland wrote:
> PCI is always enabled on the pc-i440fx machine so hardcode the relevant logic
> in pc_init1().
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>   hw/i386/pc_piix.c | 194 ++++++++++++++++++----------------------------
>   1 file changed, 76 insertions(+), 118 deletions(-)

We are not far from removing PCMachineClass::pci_enabled.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 02/14] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init()

[Mark Cave-Ayland]

On 10/07/2025 11:38, Philippe Mathieu-Daudé wrote:

> On 4/7/25 16:09, Mark Cave-Ayland wrote:
>> This is to prepare for splitting the isapc machine into its own 
>> separate file.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/i386/pc_piix.c | 261 +++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 260 insertions(+), 1 deletion(-)
> 
> Further possible cleanup:
> 
> -- >8 --
>      hw/i386/pc_piix: Inline pc_xen_hvm_init_pci()
> 
>      Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index ede7b68946b..4cec4f44764 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -362,14 +362,6 @@ static void pc_set_south_bridge(Object *obj, int 
> value, Error **errp)
>   }
> 
>   #ifdef CONFIG_XEN
> -static void pc_xen_hvm_init_pci(MachineState *machine)
> -{
> -    const char *pci_type = xen_igd_gfx_pt_enabled() ?
> -                TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : 
> TYPE_I440FX_PCI_DEVICE;
> -
> -    pc_init1(machine, pci_type);
> -}
> -
>   static void pc_xen_hvm_init(MachineState *machine)
>   {
>       PCMachineState *pcms = PC_MACHINE(machine);
> @@ -379,7 +371,9 @@ static void pc_xen_hvm_init(MachineState *machine)
>           exit(1);
>       }
> 
> -    pc_xen_hvm_init_pci(machine);
> +    pc_init1(machine, xen_igd_gfx_pt_enabled()
> +                      ? TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
> +                      : TYPE_I440FX_PCI_DEVICE);
>       xen_igd_reserve_slot(pcms->pcibus);
>       pci_create_simple(pcms->pcibus, -1, "xen-platform");
>   }

Looks reasonable to me: I'll add this into v5.


ATB,

Mark.