[RFC PATCH-for-9.1 00/29] hw/i386/pc: Decouple ISA vs PCI-based machines

[RFC PATCH-for-9.1 00/29] hw/i386/pc: Decouple ISA vs PCI-based machines

[Philippe Mathieu-Daudé]

Hi Igor,

This is the first steps to decouple the isapc VS q35/i440fx
machines. A new TYPE_PC_PCI_MACHINE is introduced to help
differentiating. Fields unrelated to the legacy isapc are
moved to the new PcPciMachineState structure.

More work remain in hw/i386/pc_piix.c so we can build a
binary with only CONFIG_ISAPC enabled.

Based-on: <20240327095124.73639-1-philmd@linaro.org>
"hw/i386: Remove deprecated pc-i440fx-2.0 -> 2.3 machines"

Philippe Mathieu-Daudé (29):
  hw/i386/pc: Declare CPU QOM types using DEFINE_TYPES() macro
  hw/i386/pc: Extract pc_machine_is_pci_enabled() helper
  hw/i386/pc: Pass base machine type as argument to DEFINE_PC_MACHINE()
  hw/i386/pc: Introduce PC_PCI_MACHINE QOM type
  hw/i386/pc: Remove PCMachineClass::pci_enabled field
  hw/i386/pc: Move pci_root_uid field to PcPciMachineClass
  hw/i386/pc: Call fw_cfg_add_extra_pci_roots() in pc_pci_machine_done()
  hw/i386/pc: Move CXLState to PcPciMachineState
  hw/i386/pc: Pass PCMachineState argument to acpi_setup()
  hw/i386/pc: Remove PCMachineClass::has_acpi_build field
  hw/i386/pc: Move acpi_setup() call to pc_pci_machine_done()
  hw/i386/pc: Move acpi_build_enabled to PcPciMachineState
  hw/i386/pc: Remove non-PCI code from pc_system_firmware_init()
  hw/i386/pc: Move pc_system_flash_create() to pc_pci_machine_initfn()
  hw/i386/pc: Move FW/pflash related fields to PcPciMachineState
  hw/i386/pc: Move south-bridge related fields to PcPciMachine
  hw/i386/pc: Inline gigabyte_align()
  hw/i386/pc: Inline has_reserved_memory()
  hw/i386/pc: Pass PcPciMachineState argument to CXL helpers
  hw/i386/pc: Pass PcPciMachineState argument to pc_pci_hole64_start()
  hw/i386/fw_cfg: Include missing 'qapi-types-machine.h' header
  hw/i386/fw_cfg: Define fw_cfg_build_smbios() stub
  hw/i386/fw_cfg: Inline smbios_defaults()
  hw/i386/fw_cfg: Inline smbios_legacy_mode()
  hw/i386/fw_cfg: Replace smbios_defaults() by !smbios_legacy_mode()
  hw/i386/fw_cfg: Factor fw_cfg_build_smbios_legacy() out
  hw/i386/pc: Call fw_cfg_build_smbios_legacy() in pc_machine_done()
  hw/i386/pc: Rename pc_init1() -> pc_piix_init()
  hw/i386/pc: Move ISA-only PC machine to pc_isa.c

 MAINTAINERS                  |   1 +
 hw/i386/acpi-build.h         |   3 +-
 hw/i386/fw_cfg.h             |   2 +
 include/hw/i386/pc.h         |  78 +++++++-------
 hw/i386/acpi-build.c         |  34 ++++--
 hw/i386/fw_cfg-smbios-stub.c |  19 ++++
 hw/i386/fw_cfg.c             |  35 +++---
 hw/i386/pc.c                 | 199 ++++++++++++++++++++++-------------
 hw/i386/pc_isa.c             |  33 ++++++
 hw/i386/pc_piix.c            |  91 +++++++---------
 hw/i386/pc_q35.c             |  12 ++-
 hw/i386/pc_sysfw.c           |  68 +++++-------
 hw/i386/xen/xen-hvm.c        |   3 +-
 hw/isa/piix.c                |   2 +-
 hw/pci-host/i440fx.c         |   4 +-
 hw/pci-host/q35.c            |   2 +-
 hw/i386/meson.build          |   2 +
 17 files changed, 342 insertions(+), 246 deletions(-)
 create mode 100644 hw/i386/fw_cfg-smbios-stub.c
 create mode 100644 hw/i386/pc_isa.c

-- 
2.41.0

[RFC PATCH-for-9.1 01/29] hw/i386/pc: Declare CPU QOM types using DEFINE_TYPES() macro

[Philippe Mathieu-Daudé]

When multiple QOM types are registered in the same file,
it is simpler to use the the DEFINE_TYPES() macro. In
particular because type array declared with such macro
are easier to review.

In few commits we are going to add more types, so replace
the type_register_static() to ease further reviews.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0be8f08c47..2c41b08478 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1810,23 +1810,20 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
         pc_machine_set_fd_bootchk);
 }
 
-static const TypeInfo pc_machine_info = {
-    .name = TYPE_PC_MACHINE,
-    .parent = TYPE_X86_MACHINE,
-    .abstract = true,
-    .instance_size = sizeof(PCMachineState),
-    .instance_init = pc_machine_initfn,
-    .class_size = sizeof(PCMachineClass),
-    .class_init = pc_machine_class_init,
-    .interfaces = (InterfaceInfo[]) {
-         { TYPE_HOTPLUG_HANDLER },
-         { }
+static const TypeInfo pc_machine_types[] = {
+    {
+        .name           = TYPE_PC_MACHINE,
+        .parent         = TYPE_X86_MACHINE,
+        .abstract       = true,
+        .instance_size  = sizeof(PCMachineState),
+        .instance_init  = pc_machine_initfn,
+        .class_size     = sizeof(PCMachineClass),
+        .class_init     = pc_machine_class_init,
+        .interfaces     = (InterfaceInfo[]) {
+             { TYPE_HOTPLUG_HANDLER },
+             { }
+        },
     },
 };
 
-static void pc_machine_register_types(void)
-{
-    type_register_static(&pc_machine_info);
-}
-
-type_init(pc_machine_register_types)
+DEFINE_TYPES(pc_machine_types)
-- 
2.41.0

[RFC PATCH-for-9.1 02/29] hw/i386/pc: Extract pc_machine_is_pci_enabled() helper

[Philippe Mathieu-Daudé]

Introduce the pc_machine_is_pci_enabled() helper to be
able to alter PCMachineClass fields later.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h |  2 ++
 hw/i386/pc.c         | 11 +++++++++--
 hw/i386/pc_piix.c    | 11 ++++++-----
 hw/i386/pc_q35.c     |  2 +-
 hw/i386/pc_sysfw.c   | 11 ++++-------
 5 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 0ad971782c..6b885424bb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -129,6 +129,8 @@ struct PCMachineClass {
 #define TYPE_PC_MACHINE "generic-pc-machine"
 OBJECT_DECLARE_TYPE(PCMachineState, PCMachineClass, PC_MACHINE)
 
+bool pc_machine_is_pci_enabled(PCMachineState *pcms);
+
 /* ioapic.c */
 
 GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2c41b08478..7065f11e97 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -814,6 +814,7 @@ void pc_memory_init(PCMachineState *pcms,
     hwaddr maxphysaddr, maxusedaddr;
     hwaddr cxl_base, cxl_resv_end = 0;
     X86CPU *cpu = X86_CPU(first_cpu);
+    bool pci_enabled = pc_machine_is_pci_enabled(pcms);
 
     assert(machine->ram_size == x86ms->below_4g_mem_size +
                                 x86ms->above_4g_mem_size);
@@ -949,7 +950,7 @@ void pc_memory_init(PCMachineState *pcms,
     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
     memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
                            &error_fatal);
-    if (pcmc->pci_enabled) {
+    if (pci_enabled) {
         memory_region_set_readonly(option_rom_mr, true);
     }
     memory_region_add_subregion_overlap(rom_memory,
@@ -1642,6 +1643,7 @@ static void pc_machine_initfn(Object *obj)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    bool pci_enabled = pc_machine_is_pci_enabled(pcms);
 
 #ifdef CONFIG_VMPORT
     pcms->vmport = ON_OFF_AUTO_AUTO;
@@ -1668,7 +1670,7 @@ static void pc_machine_initfn(Object *obj)
     pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
     object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
                               OBJECT(pcms->pcspk), "audiodev");
-    if (pcmc->pci_enabled) {
+    if (pci_enabled) {
         cxl_machine_init(obj, &pcms->cxl_devices_state);
     }
 
@@ -1810,6 +1812,11 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
         pc_machine_set_fd_bootchk);
 }
 
+bool pc_machine_is_pci_enabled(PCMachineState *pcms)
+{
+    return PC_MACHINE_GET_CLASS(pcms)->pci_enabled;
+}
+
 static const TypeInfo pc_machine_types[] = {
     {
         .name           = TYPE_PC_MACHINE,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c00d2a66a6..1be1e050c7 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -117,6 +117,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
     MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
+    bool pci_enabled = pc_machine_is_pci_enabled(pcms);
 
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
@@ -187,7 +188,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         kvmclock_create(pcmc->kvmclock_create_always);
     }
 
-    if (pcmc->pci_enabled) {
+    if (pci_enabled) {
         pci_memory = g_new(MemoryRegion, 1);
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
@@ -234,9 +235,9 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         }
     }
 
-    gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
+    gsi_state = pc_gsi_create(&x86ms->gsi, pci_enabled);
 
-    if (pcmc->pci_enabled) {
+    if (pci_enabled) {
         PCIDevice *pci_dev;
         DeviceState *dev;
         size_t i;
@@ -308,7 +309,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, pci_enabled ? pcms->pcibus : NULL);
 
     assert(pcms->vmport != ON_OFF_AUTO__MAX);
     if (pcms->vmport == ON_OFF_AUTO_AUTO) {
@@ -322,7 +323,7 @@ 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) {
+    if (!pci_enabled) {
         DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
         int i;
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b5922b44af..43ee1e595c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -145,7 +145,7 @@ static void pc_q35_init(MachineState *machine)
     bool keep_pci_slot_hpc;
     uint64_t pci_hole64_size = 0;
 
-    assert(pcmc->pci_enabled);
+    assert(pc_machine_is_pci_enabled(pcms));
 
     /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
      * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 3efabbbab2..862a082b0a 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -93,9 +93,7 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
 
 void pc_system_flash_create(PCMachineState *pcms)
 {
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-
-    if (pcmc->pci_enabled) {
+    if (pc_machine_is_pci_enabled(pcms)) {
         pcms->flash[0] = pc_pflash_create(pcms, "system.flash0",
                                           "pflash0");
         pcms->flash[1] = pc_pflash_create(pcms, "system.flash1",
@@ -108,7 +106,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
     char *prop_name;
     int i;
 
-    assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled);
+    assert(pc_machine_is_pci_enabled(pcms));
 
     for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
         if (!qdev_is_realized(DEVICE(pcms->flash[i]))) {
@@ -146,7 +144,7 @@ static void pc_system_flash_map(PCMachineState *pcms,
     void *flash_ptr;
     int flash_size;
 
-    assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled);
+    assert(pc_machine_is_pci_enabled(pcms));
 
     for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
         system_flash = pcms->flash[i];
@@ -201,11 +199,10 @@ static void pc_system_flash_map(PCMachineState *pcms,
 void pc_system_firmware_init(PCMachineState *pcms,
                              MemoryRegion *rom_memory)
 {
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     int i;
     BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];
 
-    if (!pcmc->pci_enabled) {
+    if (!pc_machine_is_pci_enabled(pcms)) {
         x86_bios_rom_init(MACHINE(pcms), "bios.bin", rom_memory, true);
         return;
     }
-- 
2.41.0

[RFC PATCH-for-9.1 03/29] hw/i386/pc: Pass base machine type as argument to DEFINE_PC_MACHINE()

[Philippe Mathieu-Daudé]

Currently PC machines are based on TYPE_PC_MACHINE.
In preparation of being based on different types,
pass the current type as argument.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 4 ++--
 hw/i386/pc_piix.c    | 9 +++++----
 hw/i386/pc_q35.c     | 3 ++-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6b885424bb..33023ebbbe 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -268,7 +268,7 @@ extern const size_t pc_compat_2_4_len;
 extern GlobalProperty pc_compat_2_3[];
 extern const size_t pc_compat_2_3_len;
 
-#define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
+#define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn, parent_class) \
     static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
     { \
         MachineClass *mc = MACHINE_CLASS(oc); \
@@ -277,7 +277,7 @@ extern const size_t pc_compat_2_3_len;
     } \
     static const TypeInfo pc_machine_type_##suffix = { \
         .name       = namestr TYPE_MACHINE_SUFFIX, \
-        .parent     = TYPE_PC_MACHINE, \
+        .parent     = parent_class, \
         .class_init = pc_machine_##suffix##_class_init, \
     }; \
     static void pc_machine_init_##suffix(void) \
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 1be1e050c7..b9f85148e3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -462,7 +462,8 @@ static void pc_xen_hvm_init(MachineState *machine)
         } \
         pc_init1(machine, TYPE_I440FX_PCI_DEVICE); \
     } \
-    DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
+    DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn, \
+                      TYPE_PC_MACHINE)
 
 static void pc_i440fx_machine_options(MachineClass *m)
 {
@@ -824,7 +825,7 @@ static void isapc_machine_options(MachineClass *m)
 }
 
 DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
-                  isapc_machine_options);
+                  isapc_machine_options, TYPE_PC_MACHINE);
 #endif
 
 #ifdef CONFIG_XEN
@@ -837,7 +838,7 @@ static void xenfv_4_2_machine_options(MachineClass *m)
 }
 
 DEFINE_PC_MACHINE(xenfv_4_2, "xenfv-4.2", pc_xen_hvm_init,
-                  xenfv_4_2_machine_options);
+                  xenfv_4_2_machine_options, TYPE_PC_MACHINE);
 
 static void xenfv_3_1_machine_options(MachineClass *m)
 {
@@ -849,5 +850,5 @@ static void xenfv_3_1_machine_options(MachineClass *m)
 }
 
 DEFINE_PC_MACHINE(xenfv, "xenfv-3.1", pc_xen_hvm_init,
-                  xenfv_3_1_machine_options);
+                  xenfv_3_1_machine_options, TYPE_PC_MACHINE);
 #endif
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 43ee1e595c..7dbee38f03 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -338,7 +338,8 @@ static void pc_q35_init(MachineState *machine)
         } \
         pc_q35_init(machine); \
     } \
-    DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
+    DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn, \
+                      TYPE_PC_MACHINE)
 
 
 static void pc_q35_machine_options(MachineClass *m)
-- 
2.41.0

[RFC PATCH-for-9.1 04/29] hw/i386/pc: Introduce PC_PCI_MACHINE QOM type

[Philippe Mathieu-Daudé]

Introduce TYPE_PC_PCI_MACHINE for machines where PCI
is expected (as opposition to the ISA-only PC machine).

This type inherits from the well known TYPE_PC_MACHINE.

Convert I440FX/PIIX and Q35 machines to use it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 25 ++++++++++++++++---------
 hw/i386/pc.c         | 25 +++++++++++++++++++++++++
 hw/i386/pc_piix.c    |  6 +++---
 hw/i386/pc_q35.c     |  2 +-
 4 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 33023ebbbe..1a4a61148a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -22,11 +22,8 @@
  * @boot_cpus: number of present VCPUs
  */
 typedef struct PCMachineState {
-    /*< private >*/
     X86MachineState parent_obj;
 
-    /* <public> */
-
     /* State for other subsystems/APIs: */
     Notifier machine_done;
 
@@ -60,6 +57,12 @@ typedef struct PCMachineState {
     CXLState cxl_devices_state;
 } PCMachineState;
 
+typedef struct PcPciMachineState {
+    PCMachineState parent_obj;
+
+    Notifier machine_done;
+} PcPciMachineState;
+
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
 #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
 #define PC_MACHINE_VMPORT           "vmport"
@@ -80,12 +83,9 @@ typedef struct PCMachineState {
  *                  way we can use 1GByte pages in the host.
  *
  */
-struct PCMachineClass {
-    /*< private >*/
+typedef struct PCMachineClass {
     X86MachineClass parent_class;
 
-    /*< public >*/
-
     /* Device configuration: */
     bool pci_enabled;
     const char *default_south_bridge;
@@ -124,13 +124,20 @@ struct PCMachineClass {
      * check for memory.
      */
     bool broken_32bit_mem_addr_check;
-};
+} PCMachineClass;
 
-#define TYPE_PC_MACHINE "generic-pc-machine"
+typedef struct PcPciMachineClass {
+    PCMachineClass parent_class;
+} PcPciMachineClass;
+
+#define TYPE_PC_MACHINE "common-pc-machine"
 OBJECT_DECLARE_TYPE(PCMachineState, PCMachineClass, PC_MACHINE)
 
 bool pc_machine_is_pci_enabled(PCMachineState *pcms);
 
+#define TYPE_PC_PCI_MACHINE "pci-pc-machine"
+OBJECT_DECLARE_TYPE(PcPciMachineState, PcPciMachineClass, PC_PCI_MACHINE)
+
 /* ioapic.c */
 
 GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7065f11e97..eafd521489 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -621,6 +621,10 @@ void pc_machine_done(Notifier *notifier, void *data)
     pc_cmos_init_late(pcms);
 }
 
+static void pc_pci_machine_done(Notifier *notifier, void *data)
+{
+}
+
 /* setup pci memory address space mapping into system address space */
 void pc_pci_as_mapping_init(MemoryRegion *system_memory,
                             MemoryRegion *pci_address_space)
@@ -1678,6 +1682,14 @@ static void pc_machine_initfn(Object *obj)
     qemu_add_machine_init_done_notifier(&pcms->machine_done);
 }
 
+static void pc_pci_machine_initfn(Object *obj)
+{
+    PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
+
+    ppms->machine_done.notify = pc_pci_machine_done;
+    qemu_add_machine_init_done_notifier(&ppms->machine_done);
+}
+
 static void pc_machine_reset(MachineState *machine, ShutdownCause reason)
 {
     CPUState *cs;
@@ -1812,6 +1824,10 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
         pc_machine_set_fd_bootchk);
 }
 
+static void pc_pci_machine_class_init(ObjectClass *oc, void *data)
+{
+}
+
 bool pc_machine_is_pci_enabled(PCMachineState *pcms)
 {
     return PC_MACHINE_GET_CLASS(pcms)->pci_enabled;
@@ -1831,6 +1847,15 @@ static const TypeInfo pc_machine_types[] = {
              { }
         },
     },
+    {
+        .name           = TYPE_PC_PCI_MACHINE,
+        .parent         = TYPE_PC_MACHINE,
+        .abstract       = true,
+        .instance_size  = sizeof(PcPciMachineState),
+        .instance_init  = pc_pci_machine_initfn,
+        .class_size     = sizeof(PcPciMachineClass),
+        .class_init     = pc_pci_machine_class_init,
+    },
 };
 
 DEFINE_TYPES(pc_machine_types)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b9f85148e3..7ada452f91 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -463,7 +463,7 @@ static void pc_xen_hvm_init(MachineState *machine)
         pc_init1(machine, TYPE_I440FX_PCI_DEVICE); \
     } \
     DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn, \
-                      TYPE_PC_MACHINE)
+                      TYPE_PC_PCI_MACHINE)
 
 static void pc_i440fx_machine_options(MachineClass *m)
 {
@@ -838,7 +838,7 @@ static void xenfv_4_2_machine_options(MachineClass *m)
 }
 
 DEFINE_PC_MACHINE(xenfv_4_2, "xenfv-4.2", pc_xen_hvm_init,
-                  xenfv_4_2_machine_options, TYPE_PC_MACHINE);
+                  xenfv_4_2_machine_options, TYPE_PC_PCI_MACHINE);
 
 static void xenfv_3_1_machine_options(MachineClass *m)
 {
@@ -850,5 +850,5 @@ static void xenfv_3_1_machine_options(MachineClass *m)
 }
 
 DEFINE_PC_MACHINE(xenfv, "xenfv-3.1", pc_xen_hvm_init,
-                  xenfv_3_1_machine_options, TYPE_PC_MACHINE);
+                  xenfv_3_1_machine_options, TYPE_PC_PCI_MACHINE);
 #endif
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 7dbee38f03..c3b0467ef3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -339,7 +339,7 @@ static void pc_q35_init(MachineState *machine)
         pc_q35_init(machine); \
     } \
     DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn, \
-                      TYPE_PC_MACHINE)
+                      TYPE_PC_PCI_MACHINE)
 
 
 static void pc_q35_machine_options(MachineClass *m)
-- 
2.41.0

[RFC PATCH-for-9.1 05/29] hw/i386/pc: Remove PCMachineClass::pci_enabled field

[Philippe Mathieu-Daudé]

All TYPE_PC_PCI_MACHINE-based machines have pci_enabled
set to %true. By checking a TYPE_PC_MACHINE inherits the
TYPE_PC_PCI_MACHINE base class, we don't need this field
anymore.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 1 -
 hw/i386/pc.c         | 3 +--
 hw/i386/pc_piix.c    | 1 -
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1a4a61148a..0b23e5ec7b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -87,7 +87,6 @@ typedef struct PCMachineClass {
     X86MachineClass parent_class;
 
     /* Device configuration: */
-    bool pci_enabled;
     const char *default_south_bridge;
 
     /* Compat options: */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index eafd521489..a16bb1554c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1740,7 +1740,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
-    pcmc->pci_enabled = true;
     pcmc->has_acpi_build = true;
     pcmc->smbios_defaults = true;
     pcmc->gigabyte_align = true;
@@ -1830,7 +1829,7 @@ static void pc_pci_machine_class_init(ObjectClass *oc, void *data)
 
 bool pc_machine_is_pci_enabled(PCMachineState *pcms)
 {
-    return PC_MACHINE_GET_CLASS(pcms)->pci_enabled;
+    return !!object_dynamic_cast(OBJECT(pcms), TYPE_PC_PCI_MACHINE);
 }
 
 static const TypeInfo pc_machine_types[] = {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7ada452f91..776d02db73 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -813,7 +813,6 @@ static void isapc_machine_options(MachineClass *m)
     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;
-- 
2.41.0

[RFC PATCH-for-9.1 06/29] hw/i386/pc: Move pci_root_uid field to PcPciMachineClass

[Philippe Mathieu-Daudé]

The 'pci_root_uid' field is irrelevant for non-PCI
machines, restrict it to the PcPciMachineClass.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 4 +++-
 hw/i386/acpi-build.c | 9 +++++++--
 hw/i386/pc_piix.c    | 7 +++++--
 hw/i386/pc_q35.c     | 7 +++++--
 4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 0b23e5ec7b..24c8e17e62 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -96,7 +96,6 @@ typedef struct PCMachineClass {
 
     /* ACPI compat: */
     bool has_acpi_build;
-    int pci_root_uid;
 
     /* SMBIOS compat: */
     bool smbios_defaults;
@@ -127,6 +126,9 @@ typedef struct PCMachineClass {
 
 typedef struct PcPciMachineClass {
     PCMachineClass parent_class;
+
+    /* ACPI compat: */
+    int pci_root_uid;
 } PcPciMachineClass;
 
 #define TYPE_PC_MACHINE "common-pc-machine"
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6f9925d176..b9890886f6 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1426,6 +1426,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     CrsRangeSet crs_range_set;
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
+    PcPciMachineClass *ppmc;
     X86MachineState *x86ms = X86_MACHINE(machine);
     AcpiMcfgInfo mcfg;
     bool mcfg_valid = !!acpi_get_mcfg(&mcfg);
@@ -1448,10 +1449,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 
     build_dbg_aml(dsdt);
     if (i440fx) {
+        ppmc = PC_PCI_MACHINE_GET_CLASS(machine);
+
         sb_scope = aml_scope("_SB");
         dev = aml_device("PCI0");
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
-        aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
+        aml_append(dev, aml_name_decl("_UID", aml_int(ppmc->pci_root_uid)));
         aml_append(dev, aml_pci_edsm());
         aml_append(sb_scope, dev);
         aml_append(dsdt, sb_scope);
@@ -1461,11 +1464,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         }
         build_piix4_pci0_int(dsdt);
     } else if (q35) {
+        ppmc = PC_PCI_MACHINE_GET_CLASS(machine);
+
         sb_scope = aml_scope("_SB");
         dev = aml_device("PCI0");
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
         aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
-        aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
+        aml_append(dev, aml_name_decl("_UID", aml_int(ppmc->pci_root_uid)));
         aml_append(dev, build_q35_osc_method(!pm->pcihp_bridge_en));
         aml_append(dev, aml_pci_edsm());
         aml_append(sb_scope, dev);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 776d02db73..c42dd46e59 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -468,9 +468,11 @@ static void pc_xen_hvm_init(MachineState *machine)
 static void pc_i440fx_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+    PcPciMachineClass *ppmc = PC_PCI_MACHINE_CLASS(m);
     ObjectClass *oc = OBJECT_CLASS(m);
+
     pcmc->default_south_bridge = TYPE_PIIX3_DEVICE;
-    pcmc->pci_root_uid = 0;
+    ppmc->pci_root_uid = 0;
     pcmc->default_cpu_version = 1;
 
     m->family = "pc_piix";
@@ -622,12 +624,13 @@ DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL,
 static void pc_i440fx_5_1_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+    PcPciMachineClass *ppmc = PC_PCI_MACHINE_CLASS(m);
 
     pc_i440fx_5_2_machine_options(m);
     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
     pcmc->kvmclock_create_always = false;
-    pcmc->pci_root_uid = 1;
+    ppmc->pci_root_uid = 1;
 }
 
 DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c3b0467ef3..dc0bf85464 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -345,7 +345,9 @@ static void pc_q35_init(MachineState *machine)
 static void pc_q35_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-    pcmc->pci_root_uid = 0;
+    PcPciMachineClass *ppmc = PC_PCI_MACHINE_CLASS(m);
+
+    ppmc->pci_root_uid = 0;
     pcmc->default_cpu_version = 1;
 
     m->family = "pc_q35";
@@ -495,12 +497,13 @@ DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL,
 static void pc_q35_5_1_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+    PcPciMachineClass *ppmc = PC_PCI_MACHINE_CLASS(m);
 
     pc_q35_5_2_machine_options(m);
     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
     pcmc->kvmclock_create_always = false;
-    pcmc->pci_root_uid = 1;
+    ppmc->pci_root_uid = 1;
 }
 
 DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL,
-- 
2.41.0

[RFC PATCH-for-9.1 07/29] hw/i386/pc: Call fw_cfg_add_extra_pci_roots() in pc_pci_machine_done()

[Philippe Mathieu-Daudé]

fw_cfg_add_extra_pci_roots() expects a PCI bus, which only
PCI-based machines have. No need to call it on the ISA-only
machine. Move it to the PCI-specific machine_done handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a16bb1554c..f9226f7115 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -608,8 +608,6 @@ void pc_machine_done(Notifier *notifier, void *data)
     /* set the number of CPUs */
     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
-    fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
-
     acpi_setup();
     if (x86ms->fw_cfg) {
         fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type);
@@ -623,6 +621,12 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 static void pc_pci_machine_done(Notifier *notifier, void *data)
 {
+    PcPciMachineState *ppms = container_of(notifier,
+                                           PcPciMachineState, machine_done);
+    PCMachineState *pcms = PC_MACHINE(ppms);
+    X86MachineState *x86ms = X86_MACHINE(pcms);
+
+    fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
 }
 
 /* setup pci memory address space mapping into system address space */
-- 
2.41.0

[RFC PATCH-for-9.1 08/29] hw/i386/pc: Move CXLState to PcPciMachineState

[Philippe Mathieu-Daudé]

CXL depends on PCIe, which isn't available on non-PCI
machines such the ISA-only PC one.
Move CXLState to PcPciMachineState, and move the CXL
specific calls to pc_pci_machine_initfn() and
pc_pci_machine_done().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h |  3 ++-
 hw/i386/acpi-build.c | 14 +++++++++++---
 hw/i386/pc.c         | 39 ++++++++++++++++++++-------------------
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 24c8e17e62..a97493d29d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -54,13 +54,14 @@ typedef struct PCMachineState {
     hwaddr memhp_io_base;
 
     SGXEPCState sgx_epc;
-    CXLState cxl_devices_state;
 } PCMachineState;
 
 typedef struct PcPciMachineState {
     PCMachineState parent_obj;
 
     Notifier machine_done;
+
+    CXLState cxl_devices_state;
 } PcPciMachineState;
 
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b9890886f6..6e8e32e5d2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1554,6 +1554,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     crs_range_set_init(&crs_range_set);
     bus = PC_MACHINE(machine)->pcibus;
     if (bus) {
+        PcPciMachineState *ppms;
+
+        assert(pc_machine_is_pci_enabled(pcms));
+        ppms = PC_PCI_MACHINE(machine);
+
         QLIST_FOREACH(bus, &bus->child, sibling) {
             uint8_t bus_num = pci_bus_num(bus);
             uint8_t numa_node = pci_bus_numa_node(bus);
@@ -1607,7 +1612,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 
             /* Handle the ranges for the PXB expanders */
             if (pci_bus_is_cxl(bus)) {
-                MemoryRegion *mr = &pcms->cxl_devices_state.host_mr;
+                MemoryRegion *mr = &ppms->cxl_devices_state.host_mr;
                 uint64_t base = mr->addr;
 
                 cxl_present = true;
@@ -2513,6 +2518,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     Object *vmgenid_dev;
     char *oem_id;
     char *oem_table_id;
+    bool pci_enabled = pc_machine_is_pci_enabled(pcms);
+    PcPciMachineState *ppms = pci_enabled ? PC_PCI_MACHINE(pcms) : NULL;
 
     acpi_get_pm_info(machine, &pm);
     acpi_get_misc_info(&misc);
@@ -2640,9 +2647,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
                           machine->nvdimms_state, machine->ram_slots,
                           x86ms->oem_id, x86ms->oem_table_id);
     }
-    if (pcms->cxl_devices_state.is_enabled) {
+    if (ppms && ppms->cxl_devices_state.is_enabled) {
         cxl_build_cedt(table_offsets, tables_blob, tables->linker,
-                       x86ms->oem_id, x86ms->oem_table_id, &pcms->cxl_devices_state);
+                       x86ms->oem_id, x86ms->oem_table_id,
+                       &ppms->cxl_devices_state);
     }
 
     acpi_add_table(table_offsets, tables_blob);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f9226f7115..6d87d1d4c2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -598,13 +598,6 @@ void pc_machine_done(Notifier *notifier, void *data)
                                         PCMachineState, machine_done);
     X86MachineState *x86ms = X86_MACHINE(pcms);
 
-    cxl_hook_up_pxb_registers(pcms->pcibus, &pcms->cxl_devices_state,
-                              &error_fatal);
-
-    if (pcms->cxl_devices_state.is_enabled) {
-        cxl_fmws_link_targets(&pcms->cxl_devices_state, &error_fatal);
-    }
-
     /* set the number of CPUs */
     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
@@ -626,6 +619,13 @@ static void pc_pci_machine_done(Notifier *notifier, void *data)
     PCMachineState *pcms = PC_MACHINE(ppms);
     X86MachineState *x86ms = X86_MACHINE(pcms);
 
+    cxl_hook_up_pxb_registers(pcms->pcibus, &ppms->cxl_devices_state,
+                              &error_fatal);
+
+    if (ppms->cxl_devices_state.is_enabled) {
+        cxl_fmws_link_targets(&ppms->cxl_devices_state, &error_fatal);
+    }
+
     fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
 }
 
@@ -719,13 +719,14 @@ static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
 
 static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
 {
+    PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
     uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
 
-    if (pcms->cxl_devices_state.fixed_windows) {
+    if (ppms->cxl_devices_state.fixed_windows) {
         GList *it;
 
         start = ROUND_UP(start, 256 * MiB);
-        for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
+        for (it = ppms->cxl_devices_state.fixed_windows; it; it = it->next) {
             CXLFixedWindow *fw = it->data;
             start += fw->size;
         }
@@ -823,6 +824,7 @@ void pc_memory_init(PCMachineState *pcms,
     hwaddr cxl_base, cxl_resv_end = 0;
     X86CPU *cpu = X86_CPU(first_cpu);
     bool pci_enabled = pc_machine_is_pci_enabled(pcms);
+    PcPciMachineState *ppms = pci_enabled ? PC_PCI_MACHINE(pcms) : NULL;
 
     assert(machine->ram_size == x86ms->below_4g_mem_size +
                                 x86ms->above_4g_mem_size);
@@ -926,20 +928,20 @@ void pc_memory_init(PCMachineState *pcms,
         machine_memory_devices_init(machine, device_mem_base, device_mem_size);
     }
 
-    if (pcms->cxl_devices_state.is_enabled) {
-        MemoryRegion *mr = &pcms->cxl_devices_state.host_mr;
+    if (ppms && ppms->cxl_devices_state.is_enabled) {
+        MemoryRegion *mr = &ppms->cxl_devices_state.host_mr;
         hwaddr cxl_size = MiB;
 
         cxl_base = pc_get_cxl_range_start(pcms);
         memory_region_init(mr, OBJECT(machine), "cxl_host_reg", cxl_size);
         memory_region_add_subregion(system_memory, cxl_base, mr);
         cxl_resv_end = cxl_base + cxl_size;
-        if (pcms->cxl_devices_state.fixed_windows) {
+        if (ppms->cxl_devices_state.fixed_windows) {
             hwaddr cxl_fmw_base;
             GList *it;
 
             cxl_fmw_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
-            for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
+            for (it = ppms->cxl_devices_state.fixed_windows; it; it = it->next) {
                 CXLFixedWindow *fw = it->data;
 
                 fw->base = cxl_fmw_base;
@@ -979,7 +981,7 @@ void pc_memory_init(PCMachineState *pcms,
             res_mem_end += memory_region_size(&machine->device_memory->mr);
         }
 
-        if (pcms->cxl_devices_state.is_enabled) {
+        if (ppms->cxl_devices_state.is_enabled) {
             res_mem_end = cxl_resv_end;
         }
         *val = cpu_to_le64(ROUND_UP(res_mem_end, 1 * GiB));
@@ -1010,11 +1012,12 @@ uint64_t pc_pci_hole64_start(void)
 {
     PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
     MachineState *ms = MACHINE(pcms);
     uint64_t hole64_start = 0;
     ram_addr_t size = 0;
 
-    if (pcms->cxl_devices_state.is_enabled) {
+    if (ppms->cxl_devices_state.is_enabled) {
         hole64_start = pc_get_cxl_range_end(pcms);
     } else if (pcmc->has_reserved_memory && (ms->ram_size < ms->maxram_size)) {
         pc_get_device_memory_range(pcms, &hole64_start, &size);
@@ -1651,7 +1654,6 @@ static void pc_machine_initfn(Object *obj)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-    bool pci_enabled = pc_machine_is_pci_enabled(pcms);
 
 #ifdef CONFIG_VMPORT
     pcms->vmport = ON_OFF_AUTO_AUTO;
@@ -1678,9 +1680,6 @@ static void pc_machine_initfn(Object *obj)
     pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
     object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
                               OBJECT(pcms->pcspk), "audiodev");
-    if (pci_enabled) {
-        cxl_machine_init(obj, &pcms->cxl_devices_state);
-    }
 
     pcms->machine_done.notify = pc_machine_done;
     qemu_add_machine_init_done_notifier(&pcms->machine_done);
@@ -1690,6 +1689,8 @@ static void pc_pci_machine_initfn(Object *obj)
 {
     PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
 
+    cxl_machine_init(obj, &ppms->cxl_devices_state);
+
     ppms->machine_done.notify = pc_pci_machine_done;
     qemu_add_machine_init_done_notifier(&ppms->machine_done);
 }
-- 
2.41.0

Re: [RFC PATCH-for-9.1 08/29] hw/i386/pc: Move CXLState to PcPciMachineState

[Jonathan Cameron]

On Thu, 28 Mar 2024 16:54:16 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> CXL depends on PCIe, which isn't available on non-PCI
> machines such the ISA-only PC one.
> Move CXLState to PcPciMachineState, and move the CXL
> specific calls to pc_pci_machine_initfn() and
> pc_pci_machine_done().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

LGTM as a change on it's own - I've not reviewed the series
in general though, hence just an ack as an rb feels too strong.

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [RFC PATCH-for-9.1 08/29] hw/i386/pc: Move CXLState to PcPciMachineState

[Jonathan Cameron via]

On Thu, 28 Mar 2024 16:54:16 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> CXL depends on PCIe, which isn't available on non-PCI
> machines such the ISA-only PC one.
> Move CXLState to PcPciMachineState, and move the CXL
> specific calls to pc_pci_machine_initfn() and
> pc_pci_machine_done().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

LGTM as a change on it's own - I've not reviewed the series
in general though, hence just an ack as an rb feels too strong.

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

[RFC PATCH-for-9.1 09/29] hw/i386/pc: Pass PCMachineState argument to acpi_setup()

[Philippe Mathieu-Daudé]

acpi_setup() caller knows about the machine state, so pass
it as argument to avoid a qdev_get_machine() call.

We already resolved X86_MACHINE(pcms) as 'x86ms' so use the
latter.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/acpi-build.h | 3 ++-
 hw/i386/acpi-build.c | 5 ++---
 hw/i386/pc.c         | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 0dce155c8c..31de5bddbd 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -2,6 +2,7 @@
 #ifndef HW_I386_ACPI_BUILD_H
 #define HW_I386_ACPI_BUILD_H
 #include "hw/acpi/acpi-defs.h"
+#include "hw/i386/pc.h"
 
 extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
 
@@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
 #define ACPI_PCIHP_SEJ_BASE 0x8
 #define ACPI_PCIHP_BNMR_BASE 0x10
 
-void acpi_setup(void);
+void acpi_setup(PCMachineState *pcms);
 Object *acpi_get_i386_pci_host(void);
 
 #endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6e8e32e5d2..e702d5e9d2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2749,9 +2749,8 @@ static const VMStateDescription vmstate_acpi_build = {
     },
 };
 
-void acpi_setup(void)
+void acpi_setup(PCMachineState *pcms)
 {
-    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
     X86MachineState *x86ms = X86_MACHINE(pcms);
     AcpiBuildTables tables;
     AcpiBuildState *build_state;
@@ -2771,7 +2770,7 @@ void acpi_setup(void)
         return;
     }
 
-    if (!x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
+    if (!x86_machine_is_acpi_enabled(x86ms)) {
         ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
         return;
     }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6d87d1d4c2..dfc0247bb6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -601,7 +601,7 @@ void pc_machine_done(Notifier *notifier, void *data)
     /* set the number of CPUs */
     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
-    acpi_setup();
+    acpi_setup(pcms);
     if (x86ms->fw_cfg) {
         fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type);
         fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
-- 
2.41.0

Re: [RFC PATCH-for-9.1 09/29] hw/i386/pc: Pass PCMachineState argument to acpi_setup()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Thu, 28 Mar 2024, Philippe Mathieu-Daudé wrote:
> acpi_setup() caller knows about the machine state, so pass
> it as argument to avoid a qdev_get_machine() call.
>
> We already resolved X86_MACHINE(pcms) as 'x86ms' so use the
> latter.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/i386/acpi-build.h | 3 ++-
> hw/i386/acpi-build.c | 5 ++---
> hw/i386/pc.c         | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
> index 0dce155c8c..31de5bddbd 100644
> --- a/hw/i386/acpi-build.h
> +++ b/hw/i386/acpi-build.h
> @@ -2,6 +2,7 @@
> #ifndef HW_I386_ACPI_BUILD_H
> #define HW_I386_ACPI_BUILD_H
> #include "hw/acpi/acpi-defs.h"
> +#include "hw/i386/pc.h"
>
> extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
>
> @@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
> #define ACPI_PCIHP_SEJ_BASE 0x8
> #define ACPI_PCIHP_BNMR_BASE 0x10
>
> -void acpi_setup(void);
> +void acpi_setup(PCMachineState *pcms);

This is changed to PcPciMachineState * in a following patch so can't you 
already introduce it here to avoid some churn?

Regards,
BALATON Zoltan

> Object *acpi_get_i386_pci_host(void);
>
> #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 6e8e32e5d2..e702d5e9d2 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2749,9 +2749,8 @@ static const VMStateDescription vmstate_acpi_build = {
>     },
> };
>
> -void acpi_setup(void)
> +void acpi_setup(PCMachineState *pcms)
> {
> -    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>     X86MachineState *x86ms = X86_MACHINE(pcms);
>     AcpiBuildTables tables;
>     AcpiBuildState *build_state;
> @@ -2771,7 +2770,7 @@ void acpi_setup(void)
>         return;
>     }
>
> -    if (!x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
> +    if (!x86_machine_is_acpi_enabled(x86ms)) {
>         ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
>         return;
>     }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6d87d1d4c2..dfc0247bb6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -601,7 +601,7 @@ void pc_machine_done(Notifier *notifier, void *data)
>     /* set the number of CPUs */
>     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
>
> -    acpi_setup();
> +    acpi_setup(pcms);
>     if (x86ms->fw_cfg) {
>         fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type);
>         fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
>

Re: [RFC PATCH-for-9.1 09/29] hw/i386/pc: Pass PCMachineState argument to acpi_setup()

[Philippe Mathieu-Daudé]

On 28/3/24 19:45, BALATON Zoltan wrote:
> On Thu, 28 Mar 2024, Philippe Mathieu-Daudé wrote:
>> acpi_setup() caller knows about the machine state, so pass
>> it as argument to avoid a qdev_get_machine() call.
>>
>> We already resolved X86_MACHINE(pcms) as 'x86ms' so use the
>> latter.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/i386/acpi-build.h | 3 ++-
>> hw/i386/acpi-build.c | 5 ++---
>> hw/i386/pc.c         | 2 +-
>> 3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
>> index 0dce155c8c..31de5bddbd 100644
>> --- a/hw/i386/acpi-build.h
>> +++ b/hw/i386/acpi-build.h
>> @@ -2,6 +2,7 @@
>> #ifndef HW_I386_ACPI_BUILD_H
>> #define HW_I386_ACPI_BUILD_H
>> #include "hw/acpi/acpi-defs.h"
>> +#include "hw/i386/pc.h"
>>
>> extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
>>
>> @@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress 
>> x86_nvdimm_acpi_dsmio;
>> #define ACPI_PCIHP_SEJ_BASE 0x8
>> #define ACPI_PCIHP_BNMR_BASE 0x10
>>
>> -void acpi_setup(void);
>> +void acpi_setup(PCMachineState *pcms);
> 
> This is changed to PcPciMachineState * in a following patch so can't you 
> already introduce it here to avoid some churn?

Unfortunately not, because we'd need to use:

   PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);

which would trigger an assertion at this point.

> 
> Regards,
> BALATON Zoltan

Re: [RFC PATCH-for-9.1 09/29] hw/i386/pc: Pass PCMachineState argument to acpi_setup()

[Bernhard Beschow]

Am 28. März 2024 15:54:17 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>acpi_setup() caller knows about the machine state, so pass
>it as argument to avoid a qdev_get_machine() call.
>
>We already resolved X86_MACHINE(pcms) as 'x86ms' so use the
>latter.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

This patch looks like good material on its own.

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

>---
> hw/i386/acpi-build.h | 3 ++-
> hw/i386/acpi-build.c | 5 ++---
> hw/i386/pc.c         | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
>index 0dce155c8c..31de5bddbd 100644
>--- a/hw/i386/acpi-build.h
>+++ b/hw/i386/acpi-build.h
>@@ -2,6 +2,7 @@
> #ifndef HW_I386_ACPI_BUILD_H
> #define HW_I386_ACPI_BUILD_H
> #include "hw/acpi/acpi-defs.h"
>+#include "hw/i386/pc.h"
> 
> extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
> 
>@@ -9,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
> #define ACPI_PCIHP_SEJ_BASE 0x8
> #define ACPI_PCIHP_BNMR_BASE 0x10
> 
>-void acpi_setup(void);
>+void acpi_setup(PCMachineState *pcms);
> Object *acpi_get_i386_pci_host(void);
> 
> #endif
>diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>index 6e8e32e5d2..e702d5e9d2 100644
>--- a/hw/i386/acpi-build.c
>+++ b/hw/i386/acpi-build.c
>@@ -2749,9 +2749,8 @@ static const VMStateDescription vmstate_acpi_build = {
>     },
> };
> 
>-void acpi_setup(void)
>+void acpi_setup(PCMachineState *pcms)
> {
>-    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>     X86MachineState *x86ms = X86_MACHINE(pcms);
>     AcpiBuildTables tables;
>     AcpiBuildState *build_state;
>@@ -2771,7 +2770,7 @@ void acpi_setup(void)
>         return;
>     }
> 
>-    if (!x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
>+    if (!x86_machine_is_acpi_enabled(x86ms)) {
>         ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
>         return;
>     }
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index 6d87d1d4c2..dfc0247bb6 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -601,7 +601,7 @@ void pc_machine_done(Notifier *notifier, void *data)
>     /* set the number of CPUs */
>     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
> 
>-    acpi_setup();
>+    acpi_setup(pcms);
>     if (x86ms->fw_cfg) {
>         fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type);
>         fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);

[RFC PATCH-for-9.1 10/29] hw/i386/pc: Remove PCMachineClass::has_acpi_build field

[Philippe Mathieu-Daudé]

PCMachineClass::has_acpi_build is always %true for PCI
based machines. Remove it, setting the 'acpi_build_enabled'
field once in pc_pci_machine_initfn().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 3 ---
 hw/i386/pc.c         | 6 +++---
 hw/i386/pc_piix.c    | 1 -
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a97493d29d..dd5ee448ef 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -95,9 +95,6 @@ typedef struct PCMachineClass {
     /* Default CPU model version.  See x86_cpu_set_default_version(). */
     int default_cpu_version;
 
-    /* ACPI compat: */
-    bool has_acpi_build;
-
     /* SMBIOS compat: */
     bool smbios_defaults;
     bool smbios_legacy_mode;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index dfc0247bb6..f0dc04e2fc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1664,8 +1664,6 @@ static void pc_machine_initfn(Object *obj)
     pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
     pcms->south_bridge = pcmc->default_south_bridge;
 
-    /* acpi build is enabled by default if machine supports it */
-    pcms->acpi_build_enabled = pcmc->has_acpi_build;
     pcms->smbus_enabled = true;
     pcms->sata_enabled = true;
     pcms->i8042_enabled = true;
@@ -1688,6 +1686,9 @@ static void pc_machine_initfn(Object *obj)
 static void pc_pci_machine_initfn(Object *obj)
 {
     PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    pcms->acpi_build_enabled = true;
 
     cxl_machine_init(obj, &ppms->cxl_devices_state);
 
@@ -1745,7 +1746,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
-    pcmc->has_acpi_build = true;
     pcmc->smbios_defaults = true;
     pcmc->gigabyte_align = true;
     pcmc->has_reserved_memory = true;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c42dd46e59..7aa2598e10 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -816,7 +816,6 @@ static void isapc_machine_options(MachineClass *m)
     m->max_cpus = 1;
     m->option_rom_has_mr = true;
     m->rom_file_has_mr = false;
-    pcmc->has_acpi_build = false;
     pcmc->smbios_defaults = false;
     pcmc->gigabyte_align = false;
     pcmc->smbios_legacy_mode = true;
-- 
2.41.0

[RFC PATCH-for-9.1 11/29] hw/i386/pc: Move acpi_setup() call to pc_pci_machine_done()

[Philippe Mathieu-Daudé]

acpi_setup() returns early if acpi_build_enabled is not set:

  2752 void acpi_setup(PCMachineState *pcms)
  2753 {
   ...
  2768     if (!pcms->acpi_build_enabled) {
  2769         ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
  2770         return;
  2771     }

acpi_build_enabled is only set on PCI-based machines, so it
is pointless to call acpi_setup() from non-PCI like the ISA-only
machine, move the call to pc_pci_machine_done().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f0dc04e2fc..47fe3a7c02 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -601,7 +601,6 @@ void pc_machine_done(Notifier *notifier, void *data)
     /* set the number of CPUs */
     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
-    acpi_setup(pcms);
     if (x86ms->fw_cfg) {
         fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type);
         fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
@@ -627,6 +626,8 @@ static void pc_pci_machine_done(Notifier *notifier, void *data)
     }
 
     fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
+
+    acpi_setup(pcms);
 }
 
 /* setup pci memory address space mapping into system address space */
-- 
2.41.0

[RFC PATCH-for-9.1 12/29] hw/i386/pc: Move acpi_build_enabled to PcPciMachineState

[Philippe Mathieu-Daudé]

Since only PCI-based machines use the 'acpi_build_enabled',
move it to PcPciMachineState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/acpi-build.h  | 2 +-
 include/hw/i386/pc.h  | 3 ++-
 hw/i386/acpi-build.c  | 8 ++++----
 hw/i386/pc.c          | 5 ++---
 hw/i386/xen/xen-hvm.c | 3 ++-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 31de5bddbd..4c1511c432 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -10,7 +10,7 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
 #define ACPI_PCIHP_SEJ_BASE 0x8
 #define ACPI_PCIHP_BNMR_BASE 0x10
 
-void acpi_setup(PCMachineState *pcms);
+void acpi_setup(PcPciMachineState *ppms);
 Object *acpi_get_i386_pci_host(void);
 
 #endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index dd5ee448ef..67f8f4730b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -41,7 +41,6 @@ typedef struct PCMachineState {
     SmbiosEntryPointType smbios_entry_point_type;
     const char *south_bridge;
 
-    bool acpi_build_enabled;
     bool smbus_enabled;
     bool sata_enabled;
     bool hpet_enabled;
@@ -61,6 +60,8 @@ typedef struct PcPciMachineState {
 
     Notifier machine_done;
 
+    bool acpi_build_enabled;
+
     CXLState cxl_devices_state;
 } PcPciMachineState;
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e702d5e9d2..ee0e99a2fa 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2749,9 +2749,9 @@ static const VMStateDescription vmstate_acpi_build = {
     },
 };
 
-void acpi_setup(PCMachineState *pcms)
+void acpi_setup(PcPciMachineState *ppms)
 {
-    X86MachineState *x86ms = X86_MACHINE(pcms);
+    X86MachineState *x86ms = X86_MACHINE(ppms);
     AcpiBuildTables tables;
     AcpiBuildState *build_state;
     Object *vmgenid_dev;
@@ -2765,7 +2765,7 @@ void acpi_setup(PCMachineState *pcms)
         return;
     }
 
-    if (!pcms->acpi_build_enabled) {
+    if (!ppms->acpi_build_enabled) {
         ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
         return;
     }
@@ -2778,7 +2778,7 @@ void acpi_setup(PCMachineState *pcms)
     build_state = g_malloc0(sizeof *build_state);
 
     acpi_build_tables_init(&tables);
-    acpi_build(&tables, MACHINE(pcms));
+    acpi_build(&tables, MACHINE(ppms));
 
     /* Now expose it all to Guest */
     build_state->table_mr = acpi_add_rom_blob(acpi_build_update,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 47fe3a7c02..f184808e3e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -627,7 +627,7 @@ static void pc_pci_machine_done(Notifier *notifier, void *data)
 
     fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
 
-    acpi_setup(pcms);
+    acpi_setup(ppms);
 }
 
 /* setup pci memory address space mapping into system address space */
@@ -1687,9 +1687,8 @@ static void pc_machine_initfn(Object *obj)
 static void pc_pci_machine_initfn(Object *obj)
 {
     PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
-    PCMachineState *pcms = PC_MACHINE(obj);
 
-    pcms->acpi_build_enabled = true;
+    ppms->acpi_build_enabled = true;
 
     cxl_machine_init(obj, &ppms->cxl_devices_state);
 
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7745cb3963..ce48d51842 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -586,6 +586,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
 void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory)
 {
     MachineState *ms = MACHINE(pcms);
+    PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
     unsigned int max_cpus = ms->smp.max_cpus;
     int rc;
     xen_pfn_t ioreq_pfn;
@@ -624,7 +625,7 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory)
     xen_ram_init(pcms, ms->ram_size, ram_memory);
 
     /* Disable ACPI build because Xen handles it */
-    pcms->acpi_build_enabled = false;
+    ppms->acpi_build_enabled = false;
 
     return;
 
-- 
2.41.0

[RFC PATCH-for-9.1 13/29] hw/i386/pc: Remove non-PCI code from pc_system_firmware_init()

[Philippe Mathieu-Daudé]

x86_bios_rom_init() is the single non-PCI-machine call
from pc_system_firmware_init(). Extract it to the caller.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c       | 6 +++++-
 hw/i386/pc_sysfw.c | 5 +----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f184808e3e..5b96daa414 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -956,7 +956,11 @@ void pc_memory_init(PCMachineState *pcms,
     }
 
     /* Initialize PC system firmware */
-    pc_system_firmware_init(pcms, rom_memory);
+    if (pci_enabled) {
+        pc_system_firmware_init(pcms, rom_memory);
+    } else {
+        x86_bios_rom_init(machine, "bios.bin", rom_memory, true);
+    }
 
     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
     memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 862a082b0a..541dcaef71 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -202,10 +202,7 @@ void pc_system_firmware_init(PCMachineState *pcms,
     int i;
     BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];
 
-    if (!pc_machine_is_pci_enabled(pcms)) {
-        x86_bios_rom_init(MACHINE(pcms), "bios.bin", rom_memory, true);
-        return;
-    }
+    assert(pc_machine_is_pci_enabled(pcms));
 
     /* Map legacy -drive if=pflash to machine properties */
     for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-- 
2.41.0

Re: [RFC PATCH-for-9.1 13/29] hw/i386/pc: Remove non-PCI code from pc_system_firmware_init()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]

On Thu, 28 Mar 2024, Philippe Mathieu-Daudé wrote:
> x86_bios_rom_init() is the single non-PCI-machine call
> from pc_system_firmware_init(). Extract it to the caller.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/i386/pc.c       | 6 +++++-
> hw/i386/pc_sysfw.c | 5 +----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f184808e3e..5b96daa414 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -956,7 +956,11 @@ void pc_memory_init(PCMachineState *pcms,
>     }
>
>     /* Initialize PC system firmware */
> -    pc_system_firmware_init(pcms, rom_memory);
> +    if (pci_enabled) {
> +        pc_system_firmware_init(pcms, rom_memory);
> +    } else {
> +        x86_bios_rom_init(machine, "bios.bin", rom_memory, true);
> +    }
>
>     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>     memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index 862a082b0a..541dcaef71 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -202,10 +202,7 @@ void pc_system_firmware_init(PCMachineState *pcms,

Maybe also rename to pc_pci_firmware_init() to make  it clear this is only 
for PCI PC machine now?

Regards,
BALATON Zoltan

>     int i;
>     BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];
>
> -    if (!pc_machine_is_pci_enabled(pcms)) {
> -        x86_bios_rom_init(MACHINE(pcms), "bios.bin", rom_memory, true);
> -        return;
> -    }
> +    assert(pc_machine_is_pci_enabled(pcms));
>
>     /* Map legacy -drive if=pflash to machine properties */
>     for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
>

Re: [RFC PATCH-for-9.1 13/29] hw/i386/pc: Remove non-PCI code from pc_system_firmware_init()

[Bernhard Beschow]

Am 28. März 2024 15:54:21 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>x86_bios_rom_init() is the single non-PCI-machine call
>from pc_system_firmware_init(). Extract it to the caller.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/pc.c       | 6 +++++-
> hw/i386/pc_sysfw.c | 5 +----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index f184808e3e..5b96daa414 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -956,7 +956,11 @@ void pc_memory_init(PCMachineState *pcms,
>     }
> 
>     /* Initialize PC system firmware */
>-    pc_system_firmware_init(pcms, rom_memory);
>+    if (pci_enabled) {
>+        pc_system_firmware_init(pcms, rom_memory);
>+    } else {
>+        x86_bios_rom_init(machine, "bios.bin", rom_memory, true);
>+    }
> 
>     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>     memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
>diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>index 862a082b0a..541dcaef71 100644
>--- a/hw/i386/pc_sysfw.c
>+++ b/hw/i386/pc_sysfw.c
>@@ -202,10 +202,7 @@ void pc_system_firmware_init(PCMachineState *pcms,
>     int i;
>     BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];
> 
>-    if (!pc_machine_is_pci_enabled(pcms)) {
>-        x86_bios_rom_init(MACHINE(pcms), "bios.bin", rom_memory, true);
>-        return;
>-    }
>+    assert(pc_machine_is_pci_enabled(pcms));

AFAICS nothing refers to pci in the whole file any longer. The only reason for checking pci_enabled before seems for filtering out the x86_bios_rom_init() case. This has been moved to the caller. Can we thus drop the assert? This allows for further removal of code in this patch and avoids superficial barriers for reusing this code. Or do I miss something?

Anyway, this patch looks like good material on its own and could be tagged independently.

With dropping the assert considered:
Reviewed-by: Bernhard Beschow <shentey@gmail.com>

> 
>     /* Map legacy -drive if=pflash to machine properties */
>     for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {

[RFC PATCH-for-9.1 14/29] hw/i386/pc: Move pc_system_flash_create() to pc_pci_machine_initfn()

[Philippe Mathieu-Daudé]

pc_system_flash_create() is only useful for PCI-based machines.
Move the call to the PCI-based init() handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c       |  2 +-
 hw/i386/pc_sysfw.c | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5b96daa414..33724791fd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1679,7 +1679,6 @@ static void pc_machine_initfn(Object *obj)
     pcms->fd_bootchk = true;
     pcms->default_bus_bypass_iommu = false;
 
-    pc_system_flash_create(pcms);
     pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
     object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
                               OBJECT(pcms->pcspk), "audiodev");
@@ -1694,6 +1693,7 @@ static void pc_pci_machine_initfn(Object *obj)
 
     ppms->acpi_build_enabled = true;
 
+    pc_system_flash_create(PC_MACHINE(obj));
     cxl_machine_init(obj, &ppms->cxl_devices_state);
 
     ppms->machine_done.notify = pc_pci_machine_done;
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 541dcaef71..167ff24fcb 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -93,12 +93,10 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
 
 void pc_system_flash_create(PCMachineState *pcms)
 {
-    if (pc_machine_is_pci_enabled(pcms)) {
-        pcms->flash[0] = pc_pflash_create(pcms, "system.flash0",
-                                          "pflash0");
-        pcms->flash[1] = pc_pflash_create(pcms, "system.flash1",
-                                          "pflash1");
-    }
+    assert(pc_machine_is_pci_enabled(pcms));
+
+    pcms->flash[0] = pc_pflash_create(pcms, "system.flash0", "pflash0");
+    pcms->flash[1] = pc_pflash_create(pcms, "system.flash1", "pflash1");
 }
 
 void pc_system_flash_cleanup_unused(PCMachineState *pcms)
-- 
2.41.0

Re: [RFC PATCH-for-9.1 14/29] hw/i386/pc: Move pc_system_flash_create() to pc_pci_machine_initfn()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]

On Thu, 28 Mar 2024, Philippe Mathieu-Daudé wrote:
> pc_system_flash_create() is only useful for PCI-based machines.
> Move the call to the PCI-based init() handler.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/i386/pc.c       |  2 +-
> hw/i386/pc_sysfw.c | 10 ++++------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5b96daa414..33724791fd 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1679,7 +1679,6 @@ static void pc_machine_initfn(Object *obj)
>     pcms->fd_bootchk = true;
>     pcms->default_bus_bypass_iommu = false;
>
> -    pc_system_flash_create(pcms);
>     pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
>     object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
>                               OBJECT(pcms->pcspk), "audiodev");
> @@ -1694,6 +1693,7 @@ static void pc_pci_machine_initfn(Object *obj)
>
>     ppms->acpi_build_enabled = true;
>
> +    pc_system_flash_create(PC_MACHINE(obj));
>     cxl_machine_init(obj, &ppms->cxl_devices_state);
>
>     ppms->machine_done.notify = pc_pci_machine_done;
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index 541dcaef71..167ff24fcb 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -93,12 +93,10 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
>
> void pc_system_flash_create(PCMachineState *pcms)
> {
> -    if (pc_machine_is_pci_enabled(pcms)) {
> -        pcms->flash[0] = pc_pflash_create(pcms, "system.flash0",
> -                                          "pflash0");
> -        pcms->flash[1] = pc_pflash_create(pcms, "system.flash1",
> -                                          "pflash1");
> -    }
> +    assert(pc_machine_is_pci_enabled(pcms));
> +
> +    pcms->flash[0] = pc_pflash_create(pcms, "system.flash0", "pflash0");
> +    pcms->flash[1] = pc_pflash_create(pcms, "system.flash1", "pflash1");
> }

This could just be inlined as it's called once, then no need for assert 
and a separate function.

Regards,
BALATON Zoltan

>
> void pc_system_flash_cleanup_unused(PCMachineState *pcms)
>

[RFC PATCH-for-9.1 15/29] hw/i386/pc: Move FW/pflash related fields to PcPciMachineState

[Philippe Mathieu-Daudé]

Only PCI-based machines use the set of parallel flash devices.
Move the fields from PCMachineState to PcPciMachineState.
Directly pass a PcPciMachineState argument to the
pc_system_flash/fw methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 10 ++++----
 hw/i386/pc.c         | 25 +++++++++---------
 hw/i386/pc_piix.c    |  3 ++-
 hw/i386/pc_sysfw.c   | 60 +++++++++++++++++++-------------------------
 4 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 67f8f4730b..668347c248 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -30,7 +30,6 @@ typedef struct PCMachineState {
     /* Pointers to devices and objects: */
     PCIBus *pcibus;
     I2CBus *smbus;
-    PFlashCFI01 *flash[2];
     ISADevice *pcspk;
     DeviceState *iommu;
     BusState *idebus[MAX_IDE_BUS];
@@ -47,7 +46,6 @@ typedef struct PCMachineState {
     bool i8042_enabled;
     bool default_bus_bypass_iommu;
     bool fd_bootchk;
-    uint64_t max_fw_size;
 
     /* ACPI Memory hotplug IO base address */
     hwaddr memhp_io_base;
@@ -61,7 +59,9 @@ typedef struct PcPciMachineState {
     Notifier machine_done;
 
     bool acpi_build_enabled;
+    uint64_t max_fw_size;
 
+    PFlashCFI01 *flash[2];
     CXLState cxl_devices_state;
 } PcPciMachineState;
 
@@ -184,9 +184,9 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
 #define TYPE_PORT92 "port92"
 
 /* pc_sysfw.c */
-void pc_system_flash_create(PCMachineState *pcms);
-void pc_system_flash_cleanup_unused(PCMachineState *pcms);
-void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
+void pc_system_flash_create(PcPciMachineState *ppms);
+void pc_system_flash_cleanup_unused(PcPciMachineState *ppms);
+void pc_system_firmware_init(PcPciMachineState *ppms, MemoryRegion *rom_memory);
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len);
 void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 33724791fd..5753a3ff0b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -957,7 +957,7 @@ void pc_memory_init(PCMachineState *pcms,
 
     /* Initialize PC system firmware */
     if (pci_enabled) {
-        pc_system_firmware_init(pcms, rom_memory);
+        pc_system_firmware_init(ppms, rom_memory);
     } else {
         x86_bios_rom_init(machine, "bios.bin", rom_memory, true);
     }
@@ -1617,8 +1617,8 @@ static void pc_machine_get_max_fw_size(Object *obj, Visitor *v,
                                        const char *name, void *opaque,
                                        Error **errp)
 {
-    PCMachineState *pcms = PC_MACHINE(obj);
-    uint64_t value = pcms->max_fw_size;
+    PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
+    uint64_t value = ppms->max_fw_size;
 
     visit_type_size(v, name, &value, errp);
 }
@@ -1627,7 +1627,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
                                        const char *name, void *opaque,
                                        Error **errp)
 {
-    PCMachineState *pcms = PC_MACHINE(obj);
+    PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
     uint64_t value;
 
     if (!visit_type_size(v, name, &value, errp)) {
@@ -1651,7 +1651,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
         return;
     }
 
-    pcms->max_fw_size = value;
+    ppms->max_fw_size = value;
 }
 
 
@@ -1672,7 +1672,6 @@ static void pc_machine_initfn(Object *obj)
     pcms->smbus_enabled = true;
     pcms->sata_enabled = true;
     pcms->i8042_enabled = true;
-    pcms->max_fw_size = 8 * MiB;
 #ifdef CONFIG_HPET
     pcms->hpet_enabled = true;
 #endif
@@ -1692,8 +1691,9 @@ static void pc_pci_machine_initfn(Object *obj)
     PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
 
     ppms->acpi_build_enabled = true;
+    ppms->max_fw_size = 8 * MiB;
 
-    pc_system_flash_create(PC_MACHINE(obj));
+    pc_system_flash_create(ppms);
     cxl_machine_init(obj, &ppms->cxl_devices_state);
 
     ppms->machine_done.notify = pc_pci_machine_done;
@@ -1815,12 +1815,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
         pc_machine_get_default_bus_bypass_iommu,
         pc_machine_set_default_bus_bypass_iommu);
 
-    object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size",
-        pc_machine_get_max_fw_size, pc_machine_set_max_fw_size,
-        NULL, NULL);
-    object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
-        "Maximum combined firmware size");
-
     object_class_property_add(oc, PC_MACHINE_SMBIOS_EP, "str",
         pc_machine_get_smbios_ep, pc_machine_set_smbios_ep,
         NULL, NULL);
@@ -1834,6 +1828,11 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
 
 static void pc_pci_machine_class_init(ObjectClass *oc, void *data)
 {
+    object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size",
+                              pc_machine_get_max_fw_size,
+                              pc_machine_set_max_fw_size, NULL, NULL);
+    object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
+                                          "Maximum combined firmware size");
 }
 
 bool pc_machine_is_pci_enabled(PCMachineState *pcms)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7aa2598e10..6b3403d0bd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -118,6 +118,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
     bool pci_enabled = pc_machine_is_pci_enabled(pcms);
+    PcPciMachineState *ppms = pci_enabled ? PC_PCI_MACHINE(pcms) : NULL;
 
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
@@ -228,7 +229,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         assert(machine->ram_size == x86ms->below_4g_mem_size +
                                     x86ms->above_4g_mem_size);
 
-        pc_system_flash_cleanup_unused(pcms);
+        pc_system_flash_cleanup_unused(ppms);
         if (machine->kernel_filename != NULL) {
             /* For xen HVM direct kernel boot, load linux here */
             xen_load_linux(pcms);
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 167ff24fcb..54d15afa49 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -71,7 +71,7 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
     memory_region_set_readonly(isa_bios, true);
 }
 
-static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
+static PFlashCFI01 *pc_pflash_create(PcPciMachineState *ppms,
                                      const char *name,
                                      const char *alias_prop_name)
 {
@@ -80,8 +80,8 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
     qdev_prop_set_uint64(dev, "sector-length", FLASH_SECTOR_SIZE);
     qdev_prop_set_uint8(dev, "width", 1);
     qdev_prop_set_string(dev, "name", name);
-    object_property_add_child(OBJECT(pcms), name, OBJECT(dev));
-    object_property_add_alias(OBJECT(pcms), alias_prop_name,
+    object_property_add_child(OBJECT(ppms), name, OBJECT(dev));
+    object_property_add_alias(OBJECT(ppms), alias_prop_name,
                               OBJECT(dev), "drive");
     /*
      * The returned reference is tied to the child property and
@@ -91,28 +91,24 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
     return PFLASH_CFI01(dev);
 }
 
-void pc_system_flash_create(PCMachineState *pcms)
+void pc_system_flash_create(PcPciMachineState *ppms)
 {
-    assert(pc_machine_is_pci_enabled(pcms));
-
-    pcms->flash[0] = pc_pflash_create(pcms, "system.flash0", "pflash0");
-    pcms->flash[1] = pc_pflash_create(pcms, "system.flash1", "pflash1");
+    ppms->flash[0] = pc_pflash_create(ppms, "system.flash0", "pflash0");
+    ppms->flash[1] = pc_pflash_create(ppms, "system.flash1", "pflash1");
 }
 
-void pc_system_flash_cleanup_unused(PCMachineState *pcms)
+void pc_system_flash_cleanup_unused(PcPciMachineState *ppms)
 {
     char *prop_name;
     int i;
 
-    assert(pc_machine_is_pci_enabled(pcms));
-
-    for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-        if (!qdev_is_realized(DEVICE(pcms->flash[i]))) {
+    for (i = 0; i < ARRAY_SIZE(ppms->flash); i++) {
+        if (!qdev_is_realized(DEVICE(ppms->flash[i]))) {
             prop_name = g_strdup_printf("pflash%d", i);
-            object_property_del(OBJECT(pcms), prop_name);
+            object_property_del(OBJECT(ppms), prop_name);
             g_free(prop_name);
-            object_unparent(OBJECT(pcms->flash[i]));
-            pcms->flash[i] = NULL;
+            object_unparent(OBJECT(ppms->flash[i]));
+            ppms->flash[i] = NULL;
         }
     }
 }
@@ -130,7 +126,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
  * pc_isa_bios_init().  Merging several flash devices for isa-bios is
  * not supported.
  */
-static void pc_system_flash_map(PCMachineState *pcms,
+static void pc_system_flash_map(PcPciMachineState *ppms,
                                 MemoryRegion *rom_memory)
 {
     hwaddr total_size = 0;
@@ -142,10 +138,8 @@ static void pc_system_flash_map(PCMachineState *pcms,
     void *flash_ptr;
     int flash_size;
 
-    assert(pc_machine_is_pci_enabled(pcms));
-
-    for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-        system_flash = pcms->flash[i];
+    for (i = 0; i < ARRAY_SIZE(ppms->flash); i++) {
+        system_flash = ppms->flash[i];
         blk = pflash_cfi01_get_blk(system_flash);
         if (!blk) {
             break;
@@ -166,10 +160,10 @@ static void pc_system_flash_map(PCMachineState *pcms,
         }
         if ((hwaddr)size != size
             || total_size > HWADDR_MAX - size
-            || total_size + size > pcms->max_fw_size) {
+            || total_size + size > ppms->max_fw_size) {
             error_report("combined size of system firmware exceeds "
                          "%" PRIu64 " bytes",
-                         pcms->max_fw_size);
+                         ppms->max_fw_size);
             exit(1);
         }
 
@@ -194,23 +188,21 @@ static void pc_system_flash_map(PCMachineState *pcms,
     }
 }
 
-void pc_system_firmware_init(PCMachineState *pcms,
+void pc_system_firmware_init(PcPciMachineState *ppms,
                              MemoryRegion *rom_memory)
 {
     int i;
-    BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];
-
-    assert(pc_machine_is_pci_enabled(pcms));
+    BlockBackend *pflash_blk[ARRAY_SIZE(ppms->flash)];
 
     /* Map legacy -drive if=pflash to machine properties */
-    for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-        pflash_cfi01_legacy_drive(pcms->flash[i],
+    for (i = 0; i < ARRAY_SIZE(ppms->flash); i++) {
+        pflash_cfi01_legacy_drive(ppms->flash[i],
                                   drive_get(IF_PFLASH, 0, i));
-        pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]);
+        pflash_blk[i] = pflash_cfi01_get_blk(ppms->flash[i]);
     }
 
     /* Reject gaps */
-    for (i = 1; i < ARRAY_SIZE(pcms->flash); i++) {
+    for (i = 1; i < ARRAY_SIZE(ppms->flash); i++) {
         if (pflash_blk[i] && !pflash_blk[i - 1]) {
             error_report("pflash%d requires pflash%d", i, i - 1);
             exit(1);
@@ -219,7 +211,7 @@ void pc_system_firmware_init(PCMachineState *pcms,
 
     if (!pflash_blk[0]) {
         /* Machine property pflash0 not set, use ROM mode */
-        x86_bios_rom_init(MACHINE(pcms), "bios.bin", rom_memory, false);
+        x86_bios_rom_init(MACHINE(ppms), "bios.bin", rom_memory, false);
     } else {
         if (kvm_enabled() && !kvm_readonly_mem_enabled()) {
             /*
@@ -231,10 +223,10 @@ void pc_system_firmware_init(PCMachineState *pcms,
             exit(1);
         }
 
-        pc_system_flash_map(pcms, rom_memory);
+        pc_system_flash_map(ppms, rom_memory);
     }
 
-    pc_system_flash_cleanup_unused(pcms);
+    pc_system_flash_cleanup_unused(ppms);
 }
 
 void x86_firmware_configure(void *ptr, int size)
-- 
2.41.0

[RFC PATCH-for-9.1 16/29] hw/i386/pc: Move south-bridge related fields to PcPciMachine

[Philippe Mathieu-Daudé]

South bridge type is only relevant for the i440fx/piix
machine, which is PCI-based.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h |  8 ++++----
 hw/i386/pc.c         |  3 ++-
 hw/i386/pc_piix.c    | 12 ++++++------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 668347c248..2db2aa03d3 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -38,7 +38,6 @@ typedef struct PCMachineState {
     uint64_t max_ram_below_4g;
     OnOffAuto vmport;
     SmbiosEntryPointType smbios_entry_point_type;
-    const char *south_bridge;
 
     bool smbus_enabled;
     bool sata_enabled;
@@ -59,6 +58,7 @@ typedef struct PcPciMachineState {
     Notifier machine_done;
 
     bool acpi_build_enabled;
+    const char *southbridge_typename;
     uint64_t max_fw_size;
 
     PFlashCFI01 *flash[2];
@@ -88,9 +88,6 @@ typedef struct PcPciMachineState {
 typedef struct PCMachineClass {
     X86MachineClass parent_class;
 
-    /* Device configuration: */
-    const char *default_south_bridge;
-
     /* Compat options: */
 
     /* Default CPU model version.  See x86_cpu_set_default_version(). */
@@ -126,6 +123,9 @@ typedef struct PCMachineClass {
 typedef struct PcPciMachineClass {
     PCMachineClass parent_class;
 
+    /* Device configuration: */
+    const char *default_southbridge_typename;
+
     /* ACPI compat: */
     int pci_root_uid;
 } PcPciMachineClass;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5753a3ff0b..dd44df0470 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1667,7 +1667,6 @@ static void pc_machine_initfn(Object *obj)
 #endif /* CONFIG_VMPORT */
     pcms->max_ram_below_4g = 0; /* use default */
     pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
-    pcms->south_bridge = pcmc->default_south_bridge;
 
     pcms->smbus_enabled = true;
     pcms->sata_enabled = true;
@@ -1689,9 +1688,11 @@ static void pc_machine_initfn(Object *obj)
 static void pc_pci_machine_initfn(Object *obj)
 {
     PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
+    PcPciMachineClass *ppmc = PC_PCI_MACHINE_GET_CLASS(ppms);
 
     ppms->acpi_build_enabled = true;
     ppms->max_fw_size = 8 * MiB;
+    ppms->southbridge_typename = ppmc->default_southbridge_typename;
 
     pc_system_flash_create(ppms);
     cxl_machine_init(obj, &ppms->cxl_devices_state);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6b3403d0bd..2043a7022a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -243,7 +243,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         DeviceState *dev;
         size_t i;
 
-        pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
+        pci_dev = pci_new_multifunction(-1, ppms->southbridge_typename);
         object_property_set_bool(OBJECT(pci_dev), "has-usb",
                                  machine_usb(machine), &error_abort);
         object_property_set_bool(OBJECT(pci_dev), "has-acpi",
@@ -385,12 +385,12 @@ static const QEnumLookup PCSouthBridgeOption_lookup = {
 
 static int pc_get_south_bridge(Object *obj, Error **errp)
 {
-    PCMachineState *pcms = PC_MACHINE(obj);
+    PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
     int i;
 
     for (i = 0; i < PCSouthBridgeOption_lookup.size; i++) {
         if (g_strcmp0(PCSouthBridgeOption_lookup.array[i],
-                      pcms->south_bridge) == 0) {
+                      ppms->southbridge_typename) == 0) {
             return i;
         }
     }
@@ -401,7 +401,7 @@ static int pc_get_south_bridge(Object *obj, Error **errp)
 
 static void pc_set_south_bridge(Object *obj, int value, Error **errp)
 {
-    PCMachineState *pcms = PC_MACHINE(obj);
+    PcPciMachineState *ppms = PC_PCI_MACHINE(obj);
 
     if (value < 0) {
         error_setg(errp, "Value can't be negative");
@@ -413,7 +413,7 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
         return;
     }
 
-    pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
+    ppms->southbridge_typename = PCSouthBridgeOption_lookup.array[value];
 }
 
 /* Looking for a pc_compat_2_4() function? It doesn't exist.
@@ -472,7 +472,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
     PcPciMachineClass *ppmc = PC_PCI_MACHINE_CLASS(m);
     ObjectClass *oc = OBJECT_CLASS(m);
 
-    pcmc->default_south_bridge = TYPE_PIIX3_DEVICE;
+    ppmc->default_southbridge_typename = TYPE_PIIX3_DEVICE;
     ppmc->pci_root_uid = 0;
     pcmc->default_cpu_version = 1;
 
-- 
2.41.0

[RFC PATCH-for-9.1 17/29] hw/i386/pc: Inline gigabyte_align()

[Philippe Mathieu-Daudé]

All PCI-based machines have the gigabyte_align field
set to %true. Simplify by using an inlined helper
checking whether the machine is PCI-based or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h |  9 ---------
 hw/i386/pc.c         |  1 -
 hw/i386/pc_piix.c    | 16 +++++++++++++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 2db2aa03d3..758dd5f29b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -76,14 +76,6 @@ typedef struct PcPciMachineState {
 
 /**
  * PCMachineClass:
- *
- * Compat fields:
- *
- * @gigabyte_align: Make sure that guest addresses aligned at
- *                  1Gbyte boundaries get mapped to host
- *                  addresses aligned at 1Gbyte boundaries. This
- *                  way we can use 1GByte pages in the host.
- *
  */
 typedef struct PCMachineClass {
     X86MachineClass parent_class;
@@ -99,7 +91,6 @@ typedef struct PCMachineClass {
     SmbiosEntryPointType default_smbios_ep_type;
 
     /* RAM / address space compat: */
-    bool gigabyte_align;
     bool has_reserved_memory;
     bool broken_reserved_end;
     bool enforce_amd_1tb_hole;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index dd44df0470..093a7c35f7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1752,7 +1752,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     pcmc->smbios_defaults = true;
-    pcmc->gigabyte_align = true;
     pcmc->has_reserved_memory = true;
     pcmc->enforce_amd_1tb_hole = true;
     pcmc->pvh_enabled = true;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2043a7022a..0bc14da768 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -99,6 +99,17 @@ static void piix_intx_routing_notifier_xen(PCIDevice *dev)
     }
 }
 
+/*
+ * gigabyte_align: Make sure that guest addresses aligned at
+ *                 1Gbyte boundaries get mapped to host
+ *                 addresses aligned at 1Gbyte boundaries.
+ *                 This way we can use 1GByte pages in the host.
+ */
+static bool gigabyte_align(PCMachineState *pcms)
+{
+    return pc_machine_is_pci_enabled(pcms);
+}
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine, const char *pci_type)
 {
@@ -130,7 +141,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
      *  - 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)
+     *    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
@@ -160,7 +171,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
         }
         lowmem = pcms->max_ram_below_4g;
         if (machine->ram_size >= pcms->max_ram_below_4g) {
-            if (pcmc->gigabyte_align) {
+            if (gigabyte_align(pcms)) {
                 if (lowmem > 0xc0000000) {
                     lowmem = 0xc0000000;
                 }
@@ -818,7 +829,6 @@ static void isapc_machine_options(MachineClass *m)
     m->option_rom_has_mr = true;
     m->rom_file_has_mr = false;
     pcmc->smbios_defaults = false;
-    pcmc->gigabyte_align = false;
     pcmc->smbios_legacy_mode = true;
     pcmc->has_reserved_memory = false;
     m->default_nic = "ne2k_isa";
-- 
2.41.0

[RFC PATCH-for-9.1 18/29] hw/i386/pc: Inline has_reserved_memory()

[Philippe Mathieu-Daudé]

All PCI-based machines have the has_reserved_memory
field set to %true. Simplify by using an inlined helper
checking whether the machine is PCI-based or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h |  1 -
 hw/i386/pc.c         | 17 ++++++++++-------
 hw/i386/pc_piix.c    |  1 -
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 758dd5f29b..df4c813854 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -91,7 +91,6 @@ typedef struct PCMachineClass {
     SmbiosEntryPointType default_smbios_ep_type;
 
     /* RAM / address space compat: */
-    bool has_reserved_memory;
     bool broken_reserved_end;
     bool enforce_amd_1tb_hole;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 093a7c35f7..e36d76656b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -671,6 +671,11 @@ void xen_load_linux(PCMachineState *pcms)
 #define PC_ROM_ALIGN       0x800
 #define PC_ROM_SIZE        (PC_ROM_MAX - PC_ROM_MIN_VGA)
 
+static bool has_reserved_memory(PCMachineState *pcms)
+{
+    return pc_machine_is_pci_enabled(pcms);
+}
+
 static hwaddr pc_above_4g_end(PCMachineState *pcms)
 {
     X86MachineState *x86ms = X86_MACHINE(pcms);
@@ -702,12 +707,11 @@ static void pc_get_device_memory_range(PCMachineState *pcms,
 
 static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
 {
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     MachineState *ms = MACHINE(pcms);
     hwaddr cxl_base;
     ram_addr_t size;
 
-    if (pcmc->has_reserved_memory &&
+    if (has_reserved_memory(pcms) &&
         (ms->ram_size < ms->maxram_size)) {
         pc_get_device_memory_range(pcms, &cxl_base, &size);
         cxl_base += size;
@@ -760,7 +764,7 @@ static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
      * calculating the highest possible GPA so that we can properly report
      * if someone configures them on a CPU that cannot possibly address them.
      */
-    if (pcmc->has_reserved_memory &&
+    if (has_reserved_memory(pcms) &&
         (ms->ram_size < ms->maxram_size)) {
         hwaddr devmem_start;
         ram_addr_t devmem_size;
@@ -891,7 +895,7 @@ void pc_memory_init(PCMachineState *pcms,
         e820_add_entry(pcms->sgx_epc.base, pcms->sgx_epc.size, E820_RESERVED);
     }
 
-    if (!pcmc->has_reserved_memory &&
+    if (!has_reserved_memory(pcms) &&
         (machine->ram_slots ||
          (machine->maxram_size > machine->ram_size))) {
 
@@ -901,7 +905,7 @@ void pc_memory_init(PCMachineState *pcms,
     }
 
     /* initialize device memory address space */
-    if (pcmc->has_reserved_memory &&
+    if (has_reserved_memory(pcms) &&
         (machine->ram_size < machine->maxram_size)) {
         ram_addr_t device_mem_size;
         hwaddr device_mem_base;
@@ -1024,7 +1028,7 @@ uint64_t pc_pci_hole64_start(void)
 
     if (ppms->cxl_devices_state.is_enabled) {
         hole64_start = pc_get_cxl_range_end(pcms);
-    } else if (pcmc->has_reserved_memory && (ms->ram_size < ms->maxram_size)) {
+    } else if (has_reserved_memory(pcms) && (ms->ram_size < ms->maxram_size)) {
         pc_get_device_memory_range(pcms, &hole64_start, &size);
         if (!pcmc->broken_reserved_end) {
             hole64_start += size;
@@ -1752,7 +1756,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     pcmc->smbios_defaults = true;
-    pcmc->has_reserved_memory = true;
     pcmc->enforce_amd_1tb_hole = true;
     pcmc->pvh_enabled = true;
     pcmc->kvmclock_create_always = true;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0bc14da768..e6178f8653 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -830,7 +830,6 @@ static void isapc_machine_options(MachineClass *m)
     m->rom_file_has_mr = false;
     pcmc->smbios_defaults = 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->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
-- 
2.41.0

[RFC PATCH-for-9.1 19/29] hw/i386/pc: Pass PcPciMachineState argument to CXL helpers

[Philippe Mathieu-Daudé]

Since CXL helpers expect a PCI-based machine, we
can directly pass them a PcPciMachineState argument.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e36d76656b..d8e91d18b8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -705,14 +705,14 @@ static void pc_get_device_memory_range(PCMachineState *pcms,
     *device_mem_size = size;
 }
 
-static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
+static uint64_t pc_get_cxl_range_start(PcPciMachineState *ppms)
 {
+    PCMachineState *pcms = PC_MACHINE(ppms);
     MachineState *ms = MACHINE(pcms);
     hwaddr cxl_base;
     ram_addr_t size;
 
-    if (has_reserved_memory(pcms) &&
-        (ms->ram_size < ms->maxram_size)) {
+    if ((ms->ram_size < ms->maxram_size)) {
         pc_get_device_memory_range(pcms, &cxl_base, &size);
         cxl_base += size;
     } else {
@@ -722,10 +722,9 @@ static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
     return cxl_base;
 }
 
-static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
+static uint64_t pc_get_cxl_range_end(PcPciMachineState *ppms)
 {
-    PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
-    uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
+    uint64_t start = pc_get_cxl_range_start(ppms) + MiB;
 
     if (ppms->cxl_devices_state.fixed_windows) {
         GList *it;
@@ -937,7 +936,7 @@ void pc_memory_init(PCMachineState *pcms,
         MemoryRegion *mr = &ppms->cxl_devices_state.host_mr;
         hwaddr cxl_size = MiB;
 
-        cxl_base = pc_get_cxl_range_start(pcms);
+        cxl_base = pc_get_cxl_range_start(ppms);
         memory_region_init(mr, OBJECT(machine), "cxl_host_reg", cxl_size);
         memory_region_add_subregion(system_memory, cxl_base, mr);
         cxl_resv_end = cxl_base + cxl_size;
@@ -1027,7 +1026,7 @@ uint64_t pc_pci_hole64_start(void)
     ram_addr_t size = 0;
 
     if (ppms->cxl_devices_state.is_enabled) {
-        hole64_start = pc_get_cxl_range_end(pcms);
+        hole64_start = pc_get_cxl_range_end(ppms);
     } else if (has_reserved_memory(pcms) && (ms->ram_size < ms->maxram_size)) {
         pc_get_device_memory_range(pcms, &hole64_start, &size);
         if (!pcmc->broken_reserved_end) {
-- 
2.41.0

[RFC PATCH-for-9.1 20/29] hw/i386/pc: Pass PcPciMachineState argument to pc_pci_hole64_start()

[Philippe Mathieu-Daudé]

pc_pci_hole64_start() is only used by PCI-based
machines.  Pass it a PcPciMachineState argument,
removing a qdev_get_machine() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 2 +-
 hw/i386/pc.c         | 8 ++++----
 hw/pci-host/i440fx.c | 2 +-
 hw/pci-host/q35.c    | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index df4c813854..7da0bc8aa4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -157,7 +157,7 @@ void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
                     uint64_t pci_hole64_size);
-uint64_t pc_pci_hole64_start(void);
+uint64_t pc_pci_hole64_start(PcPciMachineState *ppms);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d8e91d18b8..b83abee8e9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -747,7 +747,8 @@ static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
 
     if (cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
         /* 64-bit systems */
-        return pc_pci_hole64_start() + pci_hole64_size - 1;
+        PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
+        return pc_pci_hole64_start(ppms) + pci_hole64_size - 1;
     }
 
     /* 32-bit systems */
@@ -1016,11 +1017,10 @@ void pc_memory_init(PCMachineState *pcms,
  * The 64bit pci hole starts after "above 4G RAM" and
  * potentially the space reserved for memory hotplug.
  */
-uint64_t pc_pci_hole64_start(void)
+uint64_t pc_pci_hole64_start(PcPciMachineState *ppms)
 {
-    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    PCMachineState *pcms = PC_MACHINE(ppms);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-    PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
     MachineState *ms = MACHINE(pcms);
     uint64_t hole64_start = 0;
     ram_addr_t size = 0;
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 4f0a0438d7..add99e4f76 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -180,7 +180,7 @@ static uint64_t i440fx_pcihost_get_pci_hole64_start_value(Object *obj)
     pci_bus_get_w64_range(h->bus, &w64);
     value = range_is_empty(&w64) ? 0 : range_lob(&w64);
     if (!value && s->pci_hole64_fix) {
-        value = pc_pci_hole64_start();
+        value = pc_pci_hole64_start(PC_PCI_MACHINE(qdev_get_machine()));
     }
     return value;
 }
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 0d7d4e3f08..baf55897b2 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -123,7 +123,7 @@ static uint64_t q35_host_get_pci_hole64_start_value(Object *obj)
     pci_bus_get_w64_range(h->bus, &w64);
     value = range_is_empty(&w64) ? 0 : range_lob(&w64);
     if (!value && s->pci_hole64_fix) {
-        value = pc_pci_hole64_start();
+        value = pc_pci_hole64_start(PC_PCI_MACHINE(qdev_get_machine()));
     }
     return value;
 }
-- 
2.41.0

[RFC PATCH-for-9.1 21/29] hw/i386/fw_cfg: Include missing 'qapi-types-machine.h' header

[Philippe Mathieu-Daudé]

"fw_cfg.h" declares fw_cfg_build_smbios() which use
SmbiosEntryPointType, itself declared in "qapi-types-machine.h".

  void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                           SmbiosEntryPointType ep_type);
                           ^^^^^^^^^^^^^^^^^^^^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
index 92e310f5fd..7a426119f8 100644
--- a/hw/i386/fw_cfg.h
+++ b/hw/i386/fw_cfg.h
@@ -12,6 +12,7 @@
 #include "hw/boards.h"
 #include "hw/i386/pc.h"
 #include "hw/nvram/fw_cfg.h"
+#include "qapi/qapi-types-machine.h"
 
 #define FW_CFG_IO_BASE     0x510
 
-- 
2.41.0

[RFC PATCH-for-9.1 22/29] hw/i386/fw_cfg: Define fw_cfg_build_smbios() stub

[Philippe Mathieu-Daudé]

We are going to refactor fw_cfg_build_smbios() in the
next patches. In order to avoid too much #ifdef'ry in
it, define a stub.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg-smbios-stub.c | 15 +++++++++++++++
 hw/i386/fw_cfg.c             |  4 ++--
 hw/i386/meson.build          |  1 +
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 hw/i386/fw_cfg-smbios-stub.c

diff --git a/hw/i386/fw_cfg-smbios-stub.c b/hw/i386/fw_cfg-smbios-stub.c
new file mode 100644
index 0000000000..37dbfdee7c
--- /dev/null
+++ b/hw/i386/fw_cfg-smbios-stub.c
@@ -0,0 +1,15 @@
+/*
+ * QEMU fw_cfg/SMBIOS stubs
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/firmware/smbios.h"
+#include "fw_cfg.h"
+
+void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
+                         SmbiosEntryPointType ep_type)
+{
+}
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index ecc4047a4b..14a7dfbdc9 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -48,10 +48,10 @@ const char *fw_cfg_arch_key_name(uint16_t key)
     return NULL;
 }
 
+#ifdef CONFIG_SMBIOS
 void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                          SmbiosEntryPointType ep_type)
 {
-#ifdef CONFIG_SMBIOS
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     struct smbios_phys_mem_area *mem_array;
@@ -100,8 +100,8 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
         fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
                         smbios_anchor, smbios_anchor_len);
     }
-#endif
 }
+#endif
 
 FWCfgState *fw_cfg_arch_create(MachineState *ms,
                                       uint16_t boot_cpus,
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index d8b70ef3e9..1a6e731196 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -6,6 +6,7 @@ i386_ss.add(files(
   'multiboot.c',
   'x86.c',
 ))
+i386_ss.add(when: 'CONFIG_SMBIOS', if_false: files('fw_cfg-smbios-stub.c'))
 
 i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
                                       if_false: files('x86-iommu-stub.c'))
-- 
2.41.0

[RFC PATCH-for-9.1 23/29] hw/i386/fw_cfg: Inline smbios_defaults()

[Philippe Mathieu-Daudé]

All PCI-based machines have the smbios_defaults field
set to %true. Simplify by using an inlined helper
checking whether the machine is PCI-based or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 1 -
 hw/i386/fw_cfg.c     | 7 ++++++-
 hw/i386/pc.c         | 1 -
 hw/i386/pc_piix.c    | 1 -
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 7da0bc8aa4..6a6a8df005 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -86,7 +86,6 @@ typedef struct PCMachineClass {
     int default_cpu_version;
 
     /* SMBIOS compat: */
-    bool smbios_defaults;
     bool smbios_legacy_mode;
     SmbiosEntryPointType default_smbios_ep_type;
 
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 14a7dfbdc9..f60390ed56 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -49,6 +49,11 @@ const char *fw_cfg_arch_key_name(uint16_t key)
 }
 
 #ifdef CONFIG_SMBIOS
+static bool smbios_defaults(PCMachineState *pcms)
+{
+    return pc_machine_is_pci_enabled(pcms);
+}
+
 void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                          SmbiosEntryPointType ep_type)
 {
@@ -61,7 +66,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
     X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
-    if (pcmc->smbios_defaults) {
+    if (smbios_defaults(pcms)) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", mc->desc, mc->name);
     }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b83abee8e9..7a758a2e84 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1754,7 +1754,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
-    pcmc->smbios_defaults = true;
     pcmc->enforce_amd_1tb_hole = true;
     pcmc->pvh_enabled = true;
     pcmc->kvmclock_create_always = true;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e6178f8653..70dc8686f9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -828,7 +828,6 @@ static void isapc_machine_options(MachineClass *m)
     m->max_cpus = 1;
     m->option_rom_has_mr = true;
     m->rom_file_has_mr = false;
-    pcmc->smbios_defaults = false;
     pcmc->smbios_legacy_mode = true;
     m->default_nic = "ne2k_isa";
     m->default_cpu_type = X86_CPU_TYPE_NAME("486");
-- 
2.41.0

[RFC PATCH-for-9.1 24/29] hw/i386/fw_cfg: Inline smbios_legacy_mode()

[Philippe Mathieu-Daudé]

All PCI-based machines have the smbios_legacy_mode
field set to %false. Simplify by using an inlined
helper checking whether the machine is PCI-based or
not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 1 -
 hw/i386/fw_cfg.c     | 8 ++++++--
 hw/i386/pc_piix.c    | 2 --
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6a6a8df005..6510914803 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -86,7 +86,6 @@ typedef struct PCMachineClass {
     int default_cpu_version;
 
     /* SMBIOS compat: */
-    bool smbios_legacy_mode;
     SmbiosEntryPointType default_smbios_ep_type;
 
     /* RAM / address space compat: */
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index f60390ed56..ffa60a4a33 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -54,6 +54,11 @@ static bool smbios_defaults(PCMachineState *pcms)
     return pc_machine_is_pci_enabled(pcms);
 }
 
+static bool smbios_legacy_mode(PCMachineState *pcms)
+{
+    return !pc_machine_is_pci_enabled(pcms);
+}
+
 void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                          SmbiosEntryPointType ep_type)
 {
@@ -62,7 +67,6 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
     struct smbios_phys_mem_area *mem_array;
     unsigned i, array_count;
     MachineState *ms = MACHINE(pcms);
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
     X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
@@ -74,7 +78,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
     /* tell smbios about cpuid version and features */
     smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 
-    if (pcmc->smbios_legacy_mode) {
+    if (smbios_legacy_mode(pcms)) {
         smbios_tables = smbios_get_table_legacy(&smbios_tables_len,
                                                 &error_fatal);
         fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 70dc8686f9..4f07476cfa 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -823,12 +823,10 @@ DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
 #ifdef CONFIG_ISAPC
 static void isapc_machine_options(MachineClass *m)
 {
-    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->smbios_legacy_mode = true;
     m->default_nic = "ne2k_isa";
     m->default_cpu_type = X86_CPU_TYPE_NAME("486");
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
-- 
2.41.0

[RFC PATCH-for-9.1 25/29] hw/i386/fw_cfg: Replace smbios_defaults() by !smbios_legacy_mode()

[Philippe Mathieu-Daudé]

smbios_defaults() and smbios_legacy_mode() are logical
opposite. Simplify using the latter.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index ffa60a4a33..df05fe060c 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -49,11 +49,6 @@ const char *fw_cfg_arch_key_name(uint16_t key)
 }
 
 #ifdef CONFIG_SMBIOS
-static bool smbios_defaults(PCMachineState *pcms)
-{
-    return pc_machine_is_pci_enabled(pcms);
-}
-
 static bool smbios_legacy_mode(PCMachineState *pcms)
 {
     return !pc_machine_is_pci_enabled(pcms);
@@ -70,7 +65,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
     X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
-    if (smbios_defaults(pcms)) {
+    if (!smbios_legacy_mode(pcms)) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", mc->desc, mc->name);
     }
-- 
2.41.0

[RFC PATCH-for-9.1 26/29] hw/i386/fw_cfg: Factor fw_cfg_build_smbios_legacy() out

[Philippe Mathieu-Daudé]

Factor fw_cfg_build_smbios_legacy() out of
fw_cfg_build_smbios().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg.h             |  1 +
 hw/i386/fw_cfg-smbios-stub.c |  4 ++++
 hw/i386/fw_cfg.c             | 33 ++++++++++++++++++++++-----------
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
index 7a426119f8..25ce86ec1b 100644
--- a/hw/i386/fw_cfg.h
+++ b/hw/i386/fw_cfg.h
@@ -24,6 +24,7 @@
 FWCfgState *fw_cfg_arch_create(MachineState *ms,
                                uint16_t boot_cpus,
                                uint16_t apic_id_limit);
+void fw_cfg_build_smbios_legacy(PCMachineState *pcms, FWCfgState *fw_cfg);
 void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                          SmbiosEntryPointType ep_type);
 void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg);
diff --git a/hw/i386/fw_cfg-smbios-stub.c b/hw/i386/fw_cfg-smbios-stub.c
index 37dbfdee7c..da00ffc9ae 100644
--- a/hw/i386/fw_cfg-smbios-stub.c
+++ b/hw/i386/fw_cfg-smbios-stub.c
@@ -13,3 +13,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                          SmbiosEntryPointType ep_type)
 {
 }
+
+void fw_cfg_build_smbios_legacy(PCMachineState *pcms, FWCfgState *fw_cfg)
+{
+}
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index df05fe060c..be37e28f46 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -54,6 +54,22 @@ static bool smbios_legacy_mode(PCMachineState *pcms)
     return !pc_machine_is_pci_enabled(pcms);
 }
 
+void fw_cfg_build_smbios_legacy(PCMachineState *pcms, FWCfgState *fw_cfg)
+{
+    uint8_t *smbios_tables;
+    size_t smbios_tables_len;
+    MachineState *ms = MACHINE(pcms);
+    X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
+
+    /* tell smbios about cpuid version and features */
+    smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
+
+    smbios_tables = smbios_get_table_legacy(&smbios_tables_len,
+                                            &error_fatal);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
+                     smbios_tables, smbios_tables_len);
+}
+
 void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
                          SmbiosEntryPointType ep_type)
 {
@@ -65,22 +81,17 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
     X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
-    if (!smbios_legacy_mode(pcms)) {
-        /* These values are guest ABI, do not change */
-        smbios_set_defaults("QEMU", mc->desc, mc->name);
+    if (smbios_legacy_mode(pcms)) {
+        fw_cfg_build_smbios_legacy(pcms, fw_cfg);
+        return;
     }
 
+    /* These values are guest ABI, do not change */
+    smbios_set_defaults("QEMU", mc->desc, mc->name);
+
     /* tell smbios about cpuid version and features */
     smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 
-    if (smbios_legacy_mode(pcms)) {
-        smbios_tables = smbios_get_table_legacy(&smbios_tables_len,
-                                                &error_fatal);
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
-                         smbios_tables, smbios_tables_len);
-        return;
-    }
-
     /* build the array of physical mem area from e820 table */
     mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());
     for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
-- 
2.41.0

[RFC PATCH-for-9.1 27/29] hw/i386/pc: Call fw_cfg_build_smbios_legacy() in pc_machine_done()

[Philippe Mathieu-Daudé]

Keep fw_cfg_build_smbios() for PCI-based machines, call
fw_cfg_build_smbios_legacy() directly from pc_machine_done().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg.c | 10 ----------
 hw/i386/pc.c     | 12 +++++++++++-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index be37e28f46..92e058446f 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -49,11 +49,6 @@ const char *fw_cfg_arch_key_name(uint16_t key)
 }
 
 #ifdef CONFIG_SMBIOS
-static bool smbios_legacy_mode(PCMachineState *pcms)
-{
-    return !pc_machine_is_pci_enabled(pcms);
-}
-
 void fw_cfg_build_smbios_legacy(PCMachineState *pcms, FWCfgState *fw_cfg)
 {
     uint8_t *smbios_tables;
@@ -81,11 +76,6 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg,
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
     X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
-    if (smbios_legacy_mode(pcms)) {
-        fw_cfg_build_smbios_legacy(pcms, fw_cfg);
-        return;
-    }
-
     /* These values are guest ABI, do not change */
     smbios_set_defaults("QEMU", mc->desc, mc->name);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7a758a2e84..7d06a088cf 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -591,6 +591,11 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
+static bool smbios_legacy_mode(PCMachineState *pcms)
+{
+    return !pc_machine_is_pci_enabled(pcms);
+}
+
 static
 void pc_machine_done(Notifier *notifier, void *data)
 {
@@ -602,7 +607,12 @@ void pc_machine_done(Notifier *notifier, void *data)
     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
     if (x86ms->fw_cfg) {
-        fw_cfg_build_smbios(pcms, x86ms->fw_cfg, pcms->smbios_entry_point_type);
+        if (smbios_legacy_mode(pcms)) {
+            fw_cfg_build_smbios_legacy(pcms, x86ms->fw_cfg);
+        } else {
+            fw_cfg_build_smbios(pcms, x86ms->fw_cfg,
+                                pcms->smbios_entry_point_type);
+        }
         fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
         /* update FW_CFG_NB_CPUS to account for -device added CPUs */
         fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
-- 
2.41.0

[RFC PATCH-for-9.1 28/29] hw/i386/pc: Rename pc_init1() -> pc_piix_init()

[Philippe Mathieu-Daudé]

pc_init1() is specific to the isapc and i440fx/piix machines,
rename it as pc_piix_init(). Expose it in "hw/i386/pc.h" to
be able to call it externally (see next patch).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 1 +
 hw/i386/pc_piix.c    | 8 ++++----
 hw/isa/piix.c        | 2 +-
 hw/pci-host/i440fx.c | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6510914803..9a11835b7e 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -163,6 +163,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
                           bool create_fdctrl,
                           uint32_t hpet_irqs);
 void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
+void pc_piix_init(MachineState *machine, const char *pci_type);
 
 void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4f07476cfa..4a3ae72fe4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -111,7 +111,7 @@ static bool gigabyte_align(PCMachineState *pcms)
 }
 
 /* PC hardware initialisation */
-static void pc_init1(MachineState *machine, const char *pci_type)
+void pc_piix_init(MachineState *machine, const char *pci_type)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
@@ -437,7 +437,7 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
-    pc_init1(machine, NULL);
+    pc_piix_init(machine, NULL);
 }
 #endif
 
@@ -447,7 +447,7 @@ 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);
+    pc_piix_init(machine, pci_type);
 }
 
 static void pc_xen_hvm_init(MachineState *machine)
@@ -472,7 +472,7 @@ static void pc_xen_hvm_init(MachineState *machine)
         if (compat) { \
             compat(machine); \
         } \
-        pc_init1(machine, TYPE_I440FX_PCI_DEVICE); \
+        pc_piix_init(machine, TYPE_I440FX_PCI_DEVICE); \
     } \
     DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn, \
                       TYPE_PC_PCI_MACHINE)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 2d30711b17..14dc9e78be 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -432,7 +432,7 @@ static void pci_piix_class_init(ObjectClass *klass, void *data)
     k->class_id     = PCI_CLASS_BRIDGE_ISA;
     /*
      * Reason: part of PIIX southbridge, needs to be wired up by e.g.
-     * pc_piix.c's pc_init1()
+     * pc_piix.c's pc_piix_init()
      */
     dc->user_creatable = false;
     device_class_set_props(dc, pci_piix_props);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index add99e4f76..9f47d5507a 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -374,7 +374,7 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
     dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
     device_class_set_props(dc, i440fx_props);
-    /* Reason: needs to be wired up by pc_init1 */
+    /* Reason: needs to be wired up by pc_piix_init */
     dc->user_creatable = false;
 
     object_class_property_add(klass, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
-- 
2.41.0

[RFC PATCH-for-9.1 29/29] hw/i386/pc: Move ISA-only PC machine to pc_isa.c

[Philippe Mathieu-Daudé]

Extract the ISA-only PC machine code from pc_piix.c
to a new file, pc_isa.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS         |  1 +
 hw/i386/pc_isa.c    | 33 +++++++++++++++++++++++++++++++++
 hw/i386/pc_piix.c   | 23 -----------------------
 hw/i386/meson.build |  1 +
 4 files changed, 35 insertions(+), 23 deletions(-)
 create mode 100644 hw/i386/pc_isa.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a07af6b9d4..a68fa813b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1812,6 +1812,7 @@ M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
 S: Supported
 F: include/hw/i386/
 F: hw/i386/
+X: hw/i386/pc_isa.c
 F: hw/pci-host/i440fx.c
 F: hw/pci-host/q35.c
 F: hw/pci-host/pam.c
diff --git a/hw/i386/pc_isa.c b/hw/i386/pc_isa.c
new file mode 100644
index 0000000000..a98c75f3ae
--- /dev/null
+++ b/hw/i386/pc_isa.c
@@ -0,0 +1,33 @@
+/*
+ * QEMU ISA PC System Emulator
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qom/object.h"
+#include "hw/boards.h"
+#include "hw/i386/pc.h"
+#include "hw/char/parallel-isa.h"
+#include "target/i386/cpu-qom.h"
+
+static void pc_init_isa(MachineState *machine)
+{
+    pc_piix_init(machine, NULL);
+}
+
+static void isapc_machine_options(MachineClass *m)
+{
+    m->desc = "ISA-only PC";
+    m->max_cpus = 1;
+    m->option_rom_has_mr = true;
+    m->rom_file_has_mr = false;
+    m->default_nic = "ne2k_isa";
+    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
+}
+
+DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
+                  isapc_machine_options, TYPE_PC_MACHINE);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4a3ae72fe4..f94221ab92 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -434,13 +434,6 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
  * hw_compat_*, pc_compat_*, or * pc_*_machine_options().
  */
 
-#ifdef CONFIG_ISAPC
-static void pc_init_isa(MachineState *machine)
-{
-    pc_piix_init(machine, NULL);
-}
-#endif
-
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init_pci(MachineState *machine)
 {
@@ -820,22 +813,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
 DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
                       pc_i440fx_2_4_machine_options)
 
-#ifdef CONFIG_ISAPC
-static void isapc_machine_options(MachineClass *m)
-{
-    m->desc = "ISA-only PC";
-    m->max_cpus = 1;
-    m->option_rom_has_mr = true;
-    m->rom_file_has_mr = false;
-    m->default_nic = "ne2k_isa";
-    m->default_cpu_type = X86_CPU_TYPE_NAME("486");
-    m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
-}
-
-DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
-                  isapc_machine_options, TYPE_PC_MACHINE);
-#endif
-
 #ifdef CONFIG_XEN
 static void xenfv_4_2_machine_options(MachineClass *m)
 {
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 1a6e731196..0576fc6541 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -12,6 +12,7 @@ i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
                                       if_false: files('x86-iommu-stub.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_ISAPC', if_true: files('pc_isa.c'))
 i386_ss.add(when: 'CONFIG_I440FX', if_true: files('pc_piix.c'))
 i386_ss.add(when: 'CONFIG_MICROVM', if_true: files('microvm.c', 'acpi-microvm.c', 'microvm-dt.c'))
 i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
-- 
2.41.0