qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix
@ 2025-07-11  9:57 Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 01/19] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
                   ` (18 more replies)
  0 siblings, 19 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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: 2-3, 7, 9)

v5:
- Add warn_report() to patch 1 as requested by Phil
- Add patch 3 to inline pc_xen_hvm_init_pci() as suggested by Phil
- Add R-B tags from Phil

v4:
- Rebase onto master
- Add patch 2 to remove unused loader.h header as suggested by Bernhard
- Add patches 16-18 to tidy up the pc_piix headers and the pc_piix/isapc
  rom_memory variable after the isapc split also suggested by Bernhard

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 (19):
  hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
  hw/i386/pc_piix.c: remove include for loader.h
  hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init()
  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/pc_piix.c: remove unused headers after isapc machine split
  hw/i386/pc_piix.c: replace rom_memory with pci_memory
  hw/i386/isapc.c: replace rom_memory with system_memory

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

-- 
2.43.0



^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v5 01/19] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 02/19] hw/i386/pc_piix.c: remove include for loader.h Mark Cave-Ayland
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ea7572e783..34beea993e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -418,6 +418,19 @@ 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");
+        warn_report("-cpu max is invalid for isapc machine, using pentium3");
+    }
+
     pc_init1(machine, NULL);
 }
 #endif
@@ -786,7 +799,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 +824,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 02/19] hw/i386/pc_piix.c: remove include for loader.h
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 01/19] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-15  6:42   ` Philippe Mathieu-Daudé
  2025-07-11  9:57 ` [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init() Mark Cave-Ayland
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

This header is not required since the loader functionality is handled separately
by pc_memory_init() in pc.c.

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 34beea993e..4b0eeb4497 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -28,7 +28,6 @@
 #include "qemu/units.h"
 #include "hw/char/parallel-isa.h"
 #include "hw/dma/i8257.h"
-#include "hw/loader.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 01/19] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 02/19] hw/i386/pc_piix.c: remove include for loader.h Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11 10:31   ` Philippe Mathieu-Daudé
  2025-07-11  9:57 ` [PATCH v5 04/19] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init() Mark Cave-Ayland
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

This helps to simplify the initialisation of the Xen hvm machine.

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

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4b0eeb4497..b33480e203 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -435,14 +435,6 @@ static void pc_init_isa(MachineState *machine)
 #endif
 
 #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);
@@ -452,7 +444,10 @@ 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");
 }
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 04/19] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (2 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 05/19] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa() Mark Cave-Ayland
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 b33480e203..f576e98727 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -417,6 +417,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)
         warn_report("-cpu max is invalid for isapc machine, using 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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 05/19] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (3 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 04/19] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 06/19] hw/i386/pc_piix.c: remove SMI and piix4_pm " Mark Cave-Ayland
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 f576e98727..1a4b14ba0b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -417,19 +417,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 06/19] hw/i386/pc_piix.c: remove SMI and piix4_pm initialisation from pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (4 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 05/19] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 07/19] hw/i386/pc_piix.c: remove SGX " Mark Cave-Ayland
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 1a4b14ba0b..16d666f97e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -423,8 +423,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 07/19] hw/i386/pc_piix.c: remove SGX initialisation from pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (5 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 06/19] hw/i386/pc_piix.c: remove SMI and piix4_pm " Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 08/19] hw/i386/pc_piix.c: remove nvdimm " Mark Cave-Ayland
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 16d666f97e..c49be1d799 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -491,8 +491,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 08/19] hw/i386/pc_piix.c: remove nvdimm initialisation from pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (6 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 07/19] hw/i386/pc_piix.c: remove SGX " Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa() Mark Cave-Ayland
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 c49be1d799..78f50a7893 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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (7 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 08/19] hw/i386/pc_piix.c: remove nvdimm " Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11 10:41   ` Philippe Mathieu-Daudé
  2025-07-11  9:57 ` [PATCH v5 10/19] hw/i386/pc_piix.c: hardcode hole64_size to 0 " Mark Cave-Ayland
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 78f50a7893..fbd9500061 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,69 +426,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 10/19] hw/i386/pc_piix.c: hardcode hole64_size to 0 in pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (8 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 11/19] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa() Mark Cave-Ayland
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 fbd9500061..f64588f01c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,7 +426,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 11/19] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (9 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 10/19] hw/i386/pc_piix.c: hardcode hole64_size to 0 " Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 12/19] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa() Mark Cave-Ayland
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 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 f64588f01c..27dfdd5403 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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 12/19] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (10 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 11/19] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 13/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false " Mark Cave-Ayland
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 27dfdd5403..59c2949379 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,6 +426,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 13/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false in pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (11 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 12/19] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 14/19] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL " Mark Cave-Ayland
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 59c2949379..8277aaef1c 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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 14/19] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL in pc_init_isa()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (12 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 13/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false " Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 15/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1() Mark Cave-Ayland
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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 8277aaef1c..f6cd1e728d 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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 15/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1()
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (13 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 14/19] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL " Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 16/19] hw/i386: move isapc machine to separate isapc.c file Mark Cave-Ayland
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 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 f6cd1e728d..fa63659113 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,7 +70,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 };
@@ -117,6 +117,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
@@ -187,38 +190,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()) {
@@ -234,72 +235,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);
@@ -313,7 +293,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,
@@ -321,28 +301,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 16/19] hw/i386: move isapc machine to separate isapc.c file
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (14 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 15/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1() Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 17/19] hw/i386/pc_piix.c: remove unused headers after isapc machine split Mark Cave-Ayland
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

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>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/Kconfig     |   3 -
 hw/i386/isapc.c     | 170 ++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/meson.build |   1 +
 hw/i386/pc_piix.c   | 149 --------------------------------------
 4 files changed, 171 insertions(+), 152 deletions(-)
 create mode 100644 hw/i386/isapc.c

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 14d23e27b5..8ffcc9f7aa 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..b24cbf41ea
--- /dev/null
+++ b/hw/i386/isapc.c
@@ -0,0 +1,170 @@
+/*
+ * QEMU PC System Emulator
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+
+#include "qemu/error-report.h"
+#include "hw/char/parallel-isa.h"
+#include "hw/dma/i8257.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");
+        warn_report("-cpu max is invalid for isapc machine, using 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 fa63659113..4c7da53943 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,12 +70,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.
@@ -372,112 +366,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");
-        warn_report("-cpu max is invalid for isapc machine, using 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(MachineState *machine)
 {
@@ -834,43 +722,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



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 17/19] hw/i386/pc_piix.c: remove unused headers after isapc machine split
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (15 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 16/19] hw/i386: move isapc machine to separate isapc.c file Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 18/19] hw/i386/pc_piix.c: replace rom_memory with pci_memory Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 19/19] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

The headers for isapc-only devices can be removed from pc_piix.c since they are
no longer used by the i440fx-pc machine.

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, 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4c7da53943..17975ba0c4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -27,19 +27,16 @@
 
 #include "qemu/units.h"
 #include "hw/char/parallel-isa.h"
-#include "hw/dma/i8257.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
 #include "hw/pci-host/i440fx.h"
-#include "hw/rtc/mc146818rtc.h"
 #include "hw/southbridge/piix.h"
 #include "hw/display/ramfb.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_ids.h"
 #include "hw/usb.h"
 #include "net/net.h"
-#include "hw/ide/isa.h"
 #include "hw/ide/pci.h"
 #include "hw/irq.h"
 #include "system/kvm.h"
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 18/19] hw/i386/pc_piix.c: replace rom_memory with pci_memory
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (16 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 17/19] hw/i386/pc_piix.c: remove unused headers after isapc machine split Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11  9:57 ` [PATCH v5 19/19] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
  18 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

Now that we can guarantee the i440fx-pc machine will always have a PCI bus, any
instances of rom_memory can be replaced by pci_memory and rom_memory removed
completely.

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, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 17975ba0c4..7d2f6abef3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -105,7 +105,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
     GSIState *gsi_state;
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory = NULL;
-    MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
     PCIDevice *pci_dev;
@@ -183,7 +182,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
 
     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);
@@ -214,7 +212,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
 
     /* 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, pci_memory, hole64_size);
     } else {
         assert(machine->ram_size == x86ms->below_4g_mem_size +
                                     x86ms->above_4g_mem_size);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v5 19/19] hw/i386/isapc.c: replace rom_memory with system_memory
  2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
                   ` (17 preceding siblings ...)
  2025-07-11  9:57 ` [PATCH v5 18/19] hw/i386/pc_piix.c: replace rom_memory with pci_memory Mark Cave-Ayland
@ 2025-07-11  9:57 ` Mark Cave-Ayland
  2025-07-11 10:31   ` Philippe Mathieu-Daudé
  18 siblings, 1 reply; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-07-11  9:57 UTC (permalink / raw)
  To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel

Now that we can guarantee the isapc machine will never have a PCI bus, any
instances of rom_memory can be replaced by system_memory and rom_memory
removed completely.

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

diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
index b24cbf41ea..abf648706e 100644
--- a/hw/i386/isapc.c
+++ b/hw/i386/isapc.c
@@ -36,7 +36,6 @@ static void pc_init_isa(MachineState *machine)
     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;
@@ -75,7 +74,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, 0);
+        pc_memory_init(pcms, system_memory, system_memory, 0);
     } else {
         assert(machine->ram_size == x86ms->below_4g_mem_size +
                                     x86ms->above_4g_mem_size);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init()
  2025-07-11  9:57 ` [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init() Mark Cave-Ayland
@ 2025-07-11 10:31   ` Philippe Mathieu-Daudé
  2025-08-22 10:31     ` Mark Cave-Ayland
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-11 10:31 UTC (permalink / raw)
  To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
	imammedo, qemu-devel

On 11/7/25 11:57, Mark Cave-Ayland wrote:
> This helps to simplify the initialisation of the Xen hvm machine.
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>

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

or S-o-b per 
https://lore.kernel.org/qemu-devel/3ebf1793-6d55-4e07-a2b6-cb738d8634fb@linaro.org/ 
;) I don't mind.

> ---
>   hw/i386/pc_piix.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 19/19] hw/i386/isapc.c: replace rom_memory with system_memory
  2025-07-11  9:57 ` [PATCH v5 19/19] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
@ 2025-07-11 10:31   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-11 10:31 UTC (permalink / raw)
  To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
	imammedo, qemu-devel

On 11/7/25 11:57, Mark Cave-Ayland wrote:
> Now that we can guarantee the isapc machine will never have a PCI bus, any
> instances of rom_memory can be replaced by system_memory and rom_memory
> removed completely.
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>   hw/i386/isapc.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

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



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()
  2025-07-11  9:57 ` [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa() Mark Cave-Ayland
@ 2025-07-11 10:41   ` Philippe Mathieu-Daudé
  2025-08-22 10:40     ` Mark Cave-Ayland
  0 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-11 10:41 UTC (permalink / raw)
  To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
	imammedo, qemu-devel

On 11/7/25 11:57, Mark Cave-Ayland wrote:
> 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 78f50a7893..fbd9500061 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -426,69 +426,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 */

Since nothing to split, I'd expect the whole 4*GiB space here.

> +        x86ms->above_4g_mem_size = 0;
> +        x86ms->below_4g_mem_size = machine->ram_size;
>       }
>   
>       /*



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 02/19] hw/i386/pc_piix.c: remove include for loader.h
  2025-07-11  9:57 ` [PATCH v5 02/19] hw/i386/pc_piix.c: remove include for loader.h Mark Cave-Ayland
@ 2025-07-15  6:42   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-15  6:42 UTC (permalink / raw)
  To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
	imammedo, qemu-devel

On 11/7/25 11:57, Mark Cave-Ayland wrote:
> This header is not required since the loader functionality is handled separately
> by pc_memory_init() in pc.c.
> 
> 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>



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init()
  2025-07-11 10:31   ` Philippe Mathieu-Daudé
@ 2025-08-22 10:31     ` Mark Cave-Ayland
  0 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-08-22 10:31 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, pbonzini, mst, marcel.apfelbaum,
	eduardo, imammedo, qemu-devel

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

> On 11/7/25 11:57, Mark Cave-Ayland wrote:
>> This helps to simplify the initialisation of the Xen hvm machine.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> or S-o-b per https://urldefense.proofpoint.com/v2/url? 
> u=https-3A__lore.kernel.org_qemu-2Ddevel_3ebf1793-2D6d55-2D4e07-2Da2b6-2Dcb738d8634fb-40linaro.org_&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=Pl3V4I4Al1u6DFkjXsQnXZESBoBZ7B0YGifdAZ4_WBJ4MTAzniEcysfiwfJtiEbM&s=ZGdmwJU07OvV-JmnkU7z7i2D6SaAeQ7Wo1n1wIefjOQ&e= ;) I don't mind.
> 
>> ---
>>   hw/i386/pc_piix.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)

Thanks! I'll use a S-o-b for v6 in recognition of your efforts :)


ATB,

Mark.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()
  2025-07-11 10:41   ` Philippe Mathieu-Daudé
@ 2025-08-22 10:40     ` Mark Cave-Ayland
  0 siblings, 0 replies; 26+ messages in thread
From: Mark Cave-Ayland @ 2025-08-22 10:40 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, pbonzini, mst, marcel.apfelbaum,
	eduardo, imammedo, qemu-devel

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

> On 11/7/25 11:57, Mark Cave-Ayland wrote:
>> 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 78f50a7893..fbd9500061 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -426,69 +426,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 */
> 
> Since nothing to split, I'd expect the whole 4*GiB space here.

That's true: there is some logic in pc_q35_init() that sets it to 4G if 
the property is unset, so I think that makes sense.

>> +        x86ms->above_4g_mem_size = 0;
>> +        x86ms->below_4g_mem_size = machine->ram_size;
>>       }
>>       /*


ATB,

Mark.



^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2025-08-22 10:42 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11  9:57 [PATCH v5 00/19] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 01/19] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 02/19] hw/i386/pc_piix.c: remove include for loader.h Mark Cave-Ayland
2025-07-15  6:42   ` Philippe Mathieu-Daudé
2025-07-11  9:57 ` [PATCH v5 03/19] hw/i386/pc_piix.c: inline pc_xen_hvm_init_pci() into pc_xen_hvm_init() Mark Cave-Ayland
2025-07-11 10:31   ` Philippe Mathieu-Daudé
2025-08-22 10:31     ` Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 04/19] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init() Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 05/19] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa() Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 06/19] hw/i386/pc_piix.c: remove SMI and piix4_pm " Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 07/19] hw/i386/pc_piix.c: remove SGX " Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 08/19] hw/i386/pc_piix.c: remove nvdimm " Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 09/19] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa() Mark Cave-Ayland
2025-07-11 10:41   ` Philippe Mathieu-Daudé
2025-08-22 10:40     ` Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 10/19] hw/i386/pc_piix.c: hardcode hole64_size to 0 " Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 11/19] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa() Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 12/19] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa() Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 13/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false " Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 14/19] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL " Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 15/19] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1() Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 16/19] hw/i386: move isapc machine to separate isapc.c file Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 17/19] hw/i386/pc_piix.c: remove unused headers after isapc machine split Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 18/19] hw/i386/pc_piix.c: replace rom_memory with pci_memory Mark Cave-Ayland
2025-07-11  9:57 ` [PATCH v5 19/19] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
2025-07-11 10:31   ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).