All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup
@ 2023-06-30  7:37 Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 01/17] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
                   ` (16 more replies)
  0 siblings, 17 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow

This series resolves the legacy i440fx_init() function and instantiates the
I440FX host bridge the QOM way. As a preparation the Q35 host bridge receives
some cleanup as well.

Most of the Q35 patches have been submitted under [1] before. This series
incorporates only the changes making the two device models consistent with
each other.

The original plan of [1] was to clean up Q35 first and then submit a separate
follow-up I440FX QOM'ification series. This series takes a more direct approach
by cutting down on the changes in both device models while still allowing both
device models to be instantiated the same way. The remaining patches in [1]
would still be doable.

The series is structured as follows: The first 5 patches clean up Q35, the next
patch massages Q35 to share its property names with I440FX. The rest of the
series resolves i440fx_init().

Tesging done:
* `make check`
* `make check-avocado`
* Run `xl create` under Xen with the following config:
    name = "Manjaro"
    type = 'hvm'
    memory = 1536
    apic = 1
    usb = 1
    disk = [ "file:manjaro-kde-21.2.6-220416-linux515.iso,hdc:cdrom,r" ]
    device_model_override = "/usr/bin/qemu-system-x86_64"
    vga = "stdvga"
    sdl = 1

v3:
* Establish i440fx' parent-child relation earlier (Phil)
* Rename i440fx_host -> phb earlier to avoid some churn in last patch

v2:
* Rename `address_space_io` to `io_memory` (Phil)
* Eliminate one else branch in pc_piix (Igor)
* Make Q35's blackhole_ops DEVICE_LITTLE_ENDIAN again (Igor)
* Possibly ongoing discussion regarding bringing together i440fx new and realize

[1] https://patchew.org/QEMU/20230304152648.103749-1-shentey@gmail.com/

Bernhard Beschow (17):
  hw/i386/pc_q35: Resolve redundant q35_host variable
  hw/pci-host/q35: Fix double, contradicting .endianness assignment
  hw/pci-host/q35: Initialize PCMachineState::bus in board code
  hw/pci/pci_host: Introduce PCI_HOST_BYPASS_IOMMU macro
  hw/pci-host/q35: Initialize PCI_HOST_BYPASS_IOMMU property from board
    code
  hw/pci-host/q35: Make some property name macros reusable by i440fx
  hw/i386/pc_piix: Turn some local variables into initializers
  hw/pci-host/i440fx: Add "i440fx" child property in board code
  hw/pci-host/i440fx: Replace magic values by existing constants
  hw/pci-host/i440fx: Have common names for some local variables
  hw/pci-host/i440fx: Move i440fx_realize() into PCII440FXState section
  hw/pci-host/i440fx: Make MemoryRegion pointers accessible as
    properties
  hw/pci-host/i440fx: Add PCI_HOST_PROP_IO_MEM property
  hw/pci-host/i440fx: Add PCI_HOST_{ABOVE, BELOW}_4G_MEM_SIZE properties
  hw/pci-host/i440fx: Add I440FX_HOST_PROP_PCI_TYPE property
  hw/pci-host/i440fx: Resolve i440fx_init()
  hw/i386/pc_piix: Move i440fx' realize near its qdev_new()

 include/hw/i386/pc.h         |   4 ++
 include/hw/pci-host/i440fx.h |  16 +----
 include/hw/pci-host/q35.h    |   5 --
 include/hw/pci/pci_host.h    |   2 +
 hw/i386/pc_piix.c            |  59 +++++++++-------
 hw/i386/pc_q35.c             |  31 +++++----
 hw/pci-host/i440fx.c         | 128 +++++++++++++++++++----------------
 hw/pci-host/q35.c            |  13 ++--
 hw/pci/pci_host.c            |   2 +-
 9 files changed, 135 insertions(+), 125 deletions(-)

-- 
2.41.0



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

* [PATCH v3 01/17] hw/i386/pc_q35: Resolve redundant q35_host variable
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 02/17] hw/pci-host/q35: Fix double, contradicting .endianness assignment Bernhard Beschow
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow, Thomas Huth,
	Philippe Mathieu-Daudé

The variable is redundant to "phb" and is never used by its real type.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_q35.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 11a7084ea1..d9f3764184 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -120,8 +120,7 @@ static void pc_q35_init(MachineState *machine)
     PCMachineState *pcms = PC_MACHINE(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     X86MachineState *x86ms = X86_MACHINE(machine);
-    Q35PCIHost *q35_host;
-    PCIHostState *phb;
+    Object *phb;
     PCIBus *host_bus;
     PCIDevice *lpc;
     DeviceState *lpc_dev;
@@ -207,10 +206,10 @@ static void pc_q35_init(MachineState *machine)
     }
 
     /* create pci host bus */
-    q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
+    phb = OBJECT(qdev_new(TYPE_Q35_HOST_DEVICE));
 
     if (pcmc->pci_enabled) {
-        pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
+        pci_hole64_size = object_property_get_uint(phb,
                                                    PCI_HOST_PROP_PCI_HOLE64_SIZE,
                                                    &error_abort);
     }
@@ -218,23 +217,23 @@ static void pc_q35_init(MachineState *machine)
     /* allocate ram and load rom/bios */
     pc_memory_init(pcms, system_memory, rom_memory, pci_hole64_size);
 
-    object_property_add_child(OBJECT(machine), "q35", OBJECT(q35_host));
-    object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
+    object_property_add_child(OBJECT(machine), "q35", phb);
+    object_property_set_link(phb, MCH_HOST_PROP_RAM_MEM,
                              OBJECT(machine->ram), NULL);
-    object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
+    object_property_set_link(phb, MCH_HOST_PROP_PCI_MEM,
                              OBJECT(pci_memory), NULL);
-    object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SYSTEM_MEM,
+    object_property_set_link(phb, MCH_HOST_PROP_SYSTEM_MEM,
                              OBJECT(system_memory), NULL);
-    object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_IO_MEM,
+    object_property_set_link(phb, MCH_HOST_PROP_IO_MEM,
                              OBJECT(system_io), NULL);
-    object_property_set_int(OBJECT(q35_host), PCI_HOST_BELOW_4G_MEM_SIZE,
+    object_property_set_int(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
                             x86ms->below_4g_mem_size, NULL);
-    object_property_set_int(OBJECT(q35_host), PCI_HOST_ABOVE_4G_MEM_SIZE,
+    object_property_set_int(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
                             x86ms->above_4g_mem_size, NULL);
+
     /* pci */
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(q35_host), &error_fatal);
-    phb = PCI_HOST_BRIDGE(q35_host);
-    host_bus = phb->bus;
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+    host_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pcie.0"));
     /* create ISA bus */
     lpc = pci_new_multifunction(PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC), true,
                                 TYPE_ICH9_LPC_DEVICE);
-- 
2.41.0



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

* [PATCH v3 02/17] hw/pci-host/q35: Fix double, contradicting .endianness assignment
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 01/17] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 03/17] hw/pci-host/q35: Initialize PCMachineState::bus in board code Bernhard Beschow
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Philippe Mathieu-Daudé

Fixes the following clangd warning (-Winitializer-overrides):

  q35.c:297:19: Initializer overrides prior initialization of this subobject
  q35.c:292:19: previous initialization is here

Settle on little endian which is consistent with using pci_host_conf_le_ops.

Fixes: bafc90bdc594 ("q35: implement TSEG")
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/q35.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index fd18920e7f..84137b9ad9 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -285,7 +285,6 @@ static void blackhole_write(void *opaque, hwaddr addr, uint64_t val,
 static const MemoryRegionOps blackhole_ops = {
     .read = blackhole_read,
     .write = blackhole_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
     .valid.min_access_size = 1,
     .valid.max_access_size = 4,
     .impl.min_access_size = 4,
-- 
2.41.0



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

* [PATCH v3 03/17] hw/pci-host/q35: Initialize PCMachineState::bus in board code
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 01/17] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 02/17] hw/pci-host/q35: Fix double, contradicting .endianness assignment Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 04/17] hw/pci/pci_host: Introduce PCI_HOST_BYPASS_IOMMU macro Bernhard Beschow
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Igor Mammedov, Philippe Mathieu-Daudé

The Q35 PCI host currently sets the PC machine's PCI bus attribute
through global state, thereby assuming the machine to be a PC machine.
The Q35 machine code already holds on to Q35's pci bus attribute, so can
easily set its own property while preserving encapsulation.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_q35.c  | 4 +++-
 hw/pci-host/q35.c | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d9f3764184..4edc0b35f4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -230,10 +230,12 @@ static void pc_q35_init(MachineState *machine)
                             x86ms->below_4g_mem_size, NULL);
     object_property_set_int(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
                             x86ms->above_4g_mem_size, NULL);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
 
     /* pci */
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
     host_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pcie.0"));
+    pcms->bus = host_bus;
+
     /* create ISA bus */
     lpc = pci_new_multifunction(PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC), true,
                                 TYPE_ICH9_LPC_DEVICE);
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 84137b9ad9..0604464074 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -66,7 +66,6 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
                                 s->mch.pci_address_space,
                                 s->mch.address_space_io,
                                 0, TYPE_PCIE_BUS);
-    PC_MACHINE(qdev_get_machine())->bus = pci->bus;
     pci->bypass_iommu =
         PC_MACHINE(qdev_get_machine())->default_bus_bypass_iommu;
     qdev_realize(DEVICE(&s->mch), BUS(pci->bus), &error_fatal);
-- 
2.41.0



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

* [PATCH v3 04/17] hw/pci/pci_host: Introduce PCI_HOST_BYPASS_IOMMU macro
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (2 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 03/17] hw/pci-host/q35: Initialize PCMachineState::bus in board code Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 05/17] hw/pci-host/q35: Initialize PCI_HOST_BYPASS_IOMMU property from board code Bernhard Beschow
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Igor Mammedov

Introduce a macro to avoid copy and pasting strings which can easily
cause typos.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/pci/pci_host.h | 2 ++
 hw/pci/pci_host.c         | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index c6f4eb4585..e52d8ec2cd 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -31,6 +31,8 @@
 #include "hw/sysbus.h"
 #include "qom/object.h"
 
+#define PCI_HOST_BYPASS_IOMMU "bypass-iommu"
+
 #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge"
 OBJECT_DECLARE_TYPE(PCIHostState, PCIHostBridgeClass, PCI_HOST_BRIDGE)
 
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index dfd185bbb4..7af8afdcbe 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -232,7 +232,7 @@ const VMStateDescription vmstate_pcihost = {
 static Property pci_host_properties_common[] = {
     DEFINE_PROP_BOOL("x-config-reg-migration-enabled", PCIHostState,
                      mig_enabled, true),
-    DEFINE_PROP_BOOL("bypass-iommu", PCIHostState, bypass_iommu, false),
+    DEFINE_PROP_BOOL(PCI_HOST_BYPASS_IOMMU, PCIHostState, bypass_iommu, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.41.0



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

* [PATCH v3 05/17] hw/pci-host/q35: Initialize PCI_HOST_BYPASS_IOMMU property from board code
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (3 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 04/17] hw/pci/pci_host: Introduce PCI_HOST_BYPASS_IOMMU macro Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 06/17] hw/pci-host/q35: Make some property name macros reusable by i440fx Bernhard Beschow
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Igor Mammedov, Philippe Mathieu-Daudé

The Q35 PCI host already has a PCI_HOST_BYPASS_IOMMU property. However, the
host initializes this property itself by accessing global machine state,
thereby assuming it to be a PC machine. Avoid this by having board code
set this property.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_q35.c  | 2 ++
 hw/pci-host/q35.c | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4edc0b35f4..852250e8cb 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -230,6 +230,8 @@ static void pc_q35_init(MachineState *machine)
                             x86ms->below_4g_mem_size, NULL);
     object_property_set_int(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
                             x86ms->above_4g_mem_size, NULL);
+    object_property_set_bool(phb, PCI_HOST_BYPASS_IOMMU,
+                             pcms->default_bus_bypass_iommu, NULL);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
 
     /* pci */
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 0604464074..d2830cee34 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -66,8 +66,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
                                 s->mch.pci_address_space,
                                 s->mch.address_space_io,
                                 0, TYPE_PCIE_BUS);
-    pci->bypass_iommu =
-        PC_MACHINE(qdev_get_machine())->default_bus_bypass_iommu;
+
     qdev_realize(DEVICE(&s->mch), BUS(pci->bus), &error_fatal);
 }
 
-- 
2.41.0



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

* [PATCH v3 06/17] hw/pci-host/q35: Make some property name macros reusable by i440fx
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (4 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 05/17] hw/pci-host/q35: Initialize PCI_HOST_BYPASS_IOMMU property from board code Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 07/17] hw/i386/pc_piix: Turn some local variables into initializers Bernhard Beschow
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/i386/pc.h      | 4 ++++
 include/hw/pci-host/q35.h | 5 -----
 hw/i386/pc_q35.c          | 8 ++++----
 hw/pci-host/q35.c         | 8 ++++----
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6eec0fc51d..c34c698cdd 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -146,6 +146,10 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_guest_info_init(PCMachineState *pcms);
 
+#define PCI_HOST_PROP_RAM_MEM          "ram-mem"
+#define PCI_HOST_PROP_PCI_MEM          "pci-mem"
+#define PCI_HOST_PROP_SYSTEM_MEM       "system-mem"
+#define PCI_HOST_PROP_IO_MEM           "io-mem"
 #define PCI_HOST_PROP_PCI_HOLE_START   "pci-hole-start"
 #define PCI_HOST_PROP_PCI_HOLE_END     "pci-hole-end"
 #define PCI_HOST_PROP_PCI_HOLE64_START "pci-hole64-start"
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index e89329c51e..1d98bbfe0d 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -74,11 +74,6 @@ struct Q35PCIHost {
  * gmch part
  */
 
-#define MCH_HOST_PROP_RAM_MEM "ram-mem"
-#define MCH_HOST_PROP_PCI_MEM "pci-mem"
-#define MCH_HOST_PROP_SYSTEM_MEM "system-mem"
-#define MCH_HOST_PROP_IO_MEM "io-mem"
-
 /* PCI configuration */
 #define MCH_HOST_BRIDGE                        "MCH"
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 852250e8cb..02dd274276 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -218,13 +218,13 @@ static void pc_q35_init(MachineState *machine)
     pc_memory_init(pcms, system_memory, rom_memory, pci_hole64_size);
 
     object_property_add_child(OBJECT(machine), "q35", phb);
-    object_property_set_link(phb, MCH_HOST_PROP_RAM_MEM,
+    object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
                              OBJECT(machine->ram), NULL);
-    object_property_set_link(phb, MCH_HOST_PROP_PCI_MEM,
+    object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
                              OBJECT(pci_memory), NULL);
-    object_property_set_link(phb, MCH_HOST_PROP_SYSTEM_MEM,
+    object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
                              OBJECT(system_memory), NULL);
-    object_property_set_link(phb, MCH_HOST_PROP_IO_MEM,
+    object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
                              OBJECT(system_io), NULL);
     object_property_set_int(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
                             x86ms->below_4g_mem_size, NULL);
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index d2830cee34..91c46df9ae 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -240,19 +240,19 @@ static void q35_host_initfn(Object *obj)
     object_property_add_uint64_ptr(obj, PCIE_HOST_MCFG_SIZE,
                                    &pehb->size, OBJ_PROP_FLAG_READ);
 
-    object_property_add_link(obj, MCH_HOST_PROP_RAM_MEM, TYPE_MEMORY_REGION,
+    object_property_add_link(obj, PCI_HOST_PROP_RAM_MEM, TYPE_MEMORY_REGION,
                              (Object **) &s->mch.ram_memory,
                              qdev_prop_allow_set_link_before_realize, 0);
 
-    object_property_add_link(obj, MCH_HOST_PROP_PCI_MEM, TYPE_MEMORY_REGION,
+    object_property_add_link(obj, PCI_HOST_PROP_PCI_MEM, TYPE_MEMORY_REGION,
                              (Object **) &s->mch.pci_address_space,
                              qdev_prop_allow_set_link_before_realize, 0);
 
-    object_property_add_link(obj, MCH_HOST_PROP_SYSTEM_MEM, TYPE_MEMORY_REGION,
+    object_property_add_link(obj, PCI_HOST_PROP_SYSTEM_MEM, TYPE_MEMORY_REGION,
                              (Object **) &s->mch.system_memory,
                              qdev_prop_allow_set_link_before_realize, 0);
 
-    object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
+    object_property_add_link(obj, PCI_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
                              (Object **) &s->mch.address_space_io,
                              qdev_prop_allow_set_link_before_realize, 0);
 }
-- 
2.41.0



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

* [PATCH v3 07/17] hw/i386/pc_piix: Turn some local variables into initializers
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (5 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 06/17] hw/pci-host/q35: Make some property name macros reusable by i440fx Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 08/17] hw/pci-host/i440fx: Add "i440fx" child property in board code Bernhard Beschow
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Igor Mammedov, Philippe Mathieu-Daudé

Eliminates an else branch.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f9947fbc10..6a5b6dad2f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -122,11 +122,11 @@ static void pc_init1(MachineState *machine,
     BusState *idebus[MAX_IDE_BUS];
     ISADevice *rtc_state;
     MemoryRegion *ram_memory;
-    MemoryRegion *pci_memory;
-    MemoryRegion *rom_memory;
+    MemoryRegion *pci_memory = NULL;
+    MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
-    uint64_t hole64_size;
-    DeviceState *i440fx_host;
+    uint64_t hole64_size = 0;
+    DeviceState *i440fx_host = NULL;
 
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
@@ -205,11 +205,6 @@ static void pc_init1(MachineState *machine,
         hole64_size = object_property_get_uint(OBJECT(i440fx_host),
                                                PCI_HOST_PROP_PCI_HOLE64_SIZE,
                                                &error_abort);
-    } else {
-        pci_memory = NULL;
-        rom_memory = system_memory;
-        i440fx_host = NULL;
-        hole64_size = 0;
     }
 
     pc_guest_info_init(pcms);
-- 
2.41.0



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

* [PATCH v3 08/17] hw/pci-host/i440fx: Add "i440fx" child property in board code
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (6 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 07/17] hw/i386/pc_piix: Turn some local variables into initializers Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 09/17] hw/pci-host/i440fx: Replace magic values by existing constants Bernhard Beschow
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Philippe Mathieu-Daudé

The parent-child relation is usually established near a child's qdev_new(). For
i440fx this allows for reusing the machine parameter, thus avoiding
qdev_get_machine() which relies on a global variable.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c    | 2 ++
 hw/pci-host/i440fx.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6a5b6dad2f..26e8473a4d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -202,6 +202,8 @@ static void pc_init1(MachineState *machine,
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
         i440fx_host = qdev_new(host_type);
+        object_property_add_child(OBJECT(machine), "i440fx",
+                                  OBJECT(i440fx_host));
         hole64_size = object_property_get_uint(OBJECT(i440fx_host),
                                                PCI_HOST_PROP_PCI_HOLE64_SIZE,
                                                &error_abort);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 61e7b97ff4..d95d9229d3 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -259,7 +259,6 @@ PCIBus *i440fx_init(const char *pci_type,
     b = pci_root_bus_new(dev, NULL, pci_address_space,
                          address_space_io, 0, TYPE_PCI_BUS);
     s->bus = b;
-    object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
     d = pci_create_simple(b, 0, pci_type);
-- 
2.41.0



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

* [PATCH v3 09/17] hw/pci-host/i440fx: Replace magic values by existing constants
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (7 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 08/17] hw/pci-host/i440fx: Add "i440fx" child property in board code Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 10/17] hw/pci-host/i440fx: Have common names for some local variables Bernhard Beschow
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Igor Mammedov, Philippe Mathieu-Daudé

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/i440fx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index d95d9229d3..b7c24a4e1d 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -276,8 +276,8 @@ PCIBus *i440fx_init(const char *pci_type,
 
     /* if *disabled* show SMRAM to all CPUs */
     memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
-                             f->pci_address_space, 0xa0000, 0x20000);
-    memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
+                             f->pci_address_space, SMRAM_C_BASE, SMRAM_C_SIZE);
+    memory_region_add_subregion_overlap(f->system_memory, SMRAM_C_BASE,
                                         &f->smram_region, 1);
     memory_region_set_enabled(&f->smram_region, true);
 
@@ -285,9 +285,9 @@ PCIBus *i440fx_init(const char *pci_type,
     memory_region_init(&f->smram, OBJECT(d), "smram", 4 * GiB);
     memory_region_set_enabled(&f->smram, true);
     memory_region_init_alias(&f->low_smram, OBJECT(d), "smram-low",
-                             f->ram_memory, 0xa0000, 0x20000);
+                             f->ram_memory, SMRAM_C_BASE, SMRAM_C_SIZE);
     memory_region_set_enabled(&f->low_smram, true);
-    memory_region_add_subregion(&f->smram, 0xa0000, &f->low_smram);
+    memory_region_add_subregion(&f->smram, SMRAM_C_BASE, &f->low_smram);
     object_property_add_const_link(qdev_get_machine(), "smram",
                                    OBJECT(&f->smram));
 
-- 
2.41.0



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

* [PATCH v3 10/17] hw/pci-host/i440fx: Have common names for some local variables
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (8 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 09/17] hw/pci-host/i440fx: Replace magic values by existing constants Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 11/17] hw/pci-host/i440fx: Move i440fx_realize() into PCII440FXState section Bernhard Beschow
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Philippe Mathieu-Daudé

`PCIHostState` is often referred to as `phb`, own device state usually as `s`.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/i440fx.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index b7c24a4e1d..0b76fe71af 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -205,28 +205,28 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v,
 
 static void i440fx_pcihost_initfn(Object *obj)
 {
-    PCIHostState *s = PCI_HOST_BRIDGE(obj);
+    PCIHostState *phb = PCI_HOST_BRIDGE(obj);
 
-    memory_region_init_io(&s->conf_mem, obj, &pci_host_conf_le_ops, s,
+    memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb,
                           "pci-conf-idx", 4);
-    memory_region_init_io(&s->data_mem, obj, &pci_host_data_le_ops, s,
+    memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb,
                           "pci-conf-data", 4);
 }
 
 static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
 {
-    PCIHostState *s = PCI_HOST_BRIDGE(dev);
+    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    memory_region_add_subregion(s->bus->address_space_io, 0xcf8, &s->conf_mem);
+    memory_region_add_subregion(phb->bus->address_space_io, 0xcf8, &phb->conf_mem);
     sysbus_init_ioports(sbd, 0xcf8, 4);
 
-    memory_region_add_subregion(s->bus->address_space_io, 0xcfc, &s->data_mem);
+    memory_region_add_subregion(phb->bus->address_space_io, 0xcfc, &phb->data_mem);
     sysbus_init_ioports(sbd, 0xcfc, 4);
 
     /* register i440fx 0xcf8 port as coalesced pio */
-    memory_region_set_flush_coalesced(&s->data_mem);
-    memory_region_add_coalescing(&s->conf_mem, 0, 4);
+    memory_region_set_flush_coalesced(&phb->data_mem);
+    memory_region_add_coalescing(&phb->conf_mem, 0, 4);
 }
 
 static void i440fx_realize(PCIDevice *dev, Error **errp)
@@ -248,17 +248,16 @@ PCIBus *i440fx_init(const char *pci_type,
                     MemoryRegion *pci_address_space,
                     MemoryRegion *ram_memory)
 {
+    I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
+    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     PCIBus *b;
     PCIDevice *d;
-    PCIHostState *s;
     PCII440FXState *f;
     unsigned i;
-    I440FXState *i440fx;
 
-    s = PCI_HOST_BRIDGE(dev);
     b = pci_root_bus_new(dev, NULL, pci_address_space,
                          address_space_io, 0, TYPE_PCI_BUS);
-    s->bus = b;
+    phb->bus = b;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
     d = pci_create_simple(b, 0, pci_type);
@@ -267,8 +266,7 @@ PCIBus *i440fx_init(const char *pci_type,
     f->pci_address_space = pci_address_space;
     f->ram_memory = ram_memory;
 
-    i440fx = I440FX_PCI_HOST_BRIDGE(dev);
-    range_set_bounds(&i440fx->pci_hole, below_4g_mem_size,
+    range_set_bounds(&s->pci_hole, below_4g_mem_size,
                      IO_APIC_DEFAULT_ADDRESS - 1);
 
     /* setup pci memory mapping */
-- 
2.41.0



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

* [PATCH v3 11/17] hw/pci-host/i440fx: Move i440fx_realize() into PCII440FXState section
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (9 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 10/17] hw/pci-host/i440fx: Have common names for some local variables Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 12/17] hw/pci-host/i440fx: Make MemoryRegion pointers accessible as properties Bernhard Beschow
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Philippe Mathieu-Daudé

i440fx_realize() realizes the PCI device inside the host bridge
(PCII440FXState), but is implemented between i440fx_pcihost_realize() and
i440fx_init() which deal with the host bridge itself (I440FXState). Since we
want to append i440fx_init() to i440fx_pcihost_realize() later let's move
i440fx_realize() out of the way.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/i440fx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 0b76fe71af..e84fcd50b6 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -65,6 +65,15 @@ struct I440FXState {
  */
 #define I440FX_COREBOOT_RAM_SIZE 0x57
 
+static void i440fx_realize(PCIDevice *dev, Error **errp)
+{
+    dev->config[I440FX_SMRAM] = 0x02;
+
+    if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) {
+        warn_report("i440fx doesn't support emulated iommu");
+    }
+}
+
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
@@ -229,15 +238,6 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
     memory_region_add_coalescing(&phb->conf_mem, 0, 4);
 }
 
-static void i440fx_realize(PCIDevice *dev, Error **errp)
-{
-    dev->config[I440FX_SMRAM] = 0x02;
-
-    if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) {
-        warn_report("i440fx doesn't support emulated iommu");
-    }
-}
-
 PCIBus *i440fx_init(const char *pci_type,
                     DeviceState *dev,
                     MemoryRegion *address_space_mem,
-- 
2.41.0



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

* [PATCH v3 12/17] hw/pci-host/i440fx: Make MemoryRegion pointers accessible as properties
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (10 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 11/17] hw/pci-host/i440fx: Move i440fx_realize() into PCII440FXState section Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 13/17] hw/pci-host/i440fx: Add PCI_HOST_PROP_IO_MEM property Bernhard Beschow
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Philippe Mathieu-Daudé

The goal is to eliminate i440fx_init() which is a legacy init function. This
neccessitates the memory regions to be properties, like in Q35, which will be
assigned in board code.

Since i440fx needs different PCI devices in Xen mode, and since i440fx shall
be self-contained, the PCI device will be created during realization of the
host. Thus the pointers need to be moved to the host structure to be usable as
properties.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/pci-host/i440fx.h |  3 ---
 hw/pci-host/i440fx.c         | 42 +++++++++++++++++++++++++-----------
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index bf57216c78..e3a550021e 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -25,9 +25,6 @@ struct PCII440FXState {
     PCIDevice parent_obj;
     /*< public >*/
 
-    MemoryRegion *system_memory;
-    MemoryRegion *pci_address_space;
-    MemoryRegion *ram_memory;
     PAMMemoryRegion pam_regions[PAM_REGIONS_COUNT];
     MemoryRegion smram_region;
     MemoryRegion smram, low_smram;
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index e84fcd50b6..b9530fc3a0 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -47,6 +47,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(I440FXState, I440FX_PCI_HOST_BRIDGE)
 
 struct I440FXState {
     PCIHostState parent_obj;
+
+    MemoryRegion *system_memory;
+    MemoryRegion *pci_address_space;
+    MemoryRegion *ram_memory;
     Range pci_hole;
     uint64_t pci_hole64_size;
     bool pci_hole64_fix;
@@ -214,12 +218,25 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v,
 
 static void i440fx_pcihost_initfn(Object *obj)
 {
+    I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
     PCIHostState *phb = PCI_HOST_BRIDGE(obj);
 
     memory_region_init_io(&phb->conf_mem, obj, &pci_host_conf_le_ops, phb,
                           "pci-conf-idx", 4);
     memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb,
                           "pci-conf-data", 4);
+
+    object_property_add_link(obj, PCI_HOST_PROP_RAM_MEM, TYPE_MEMORY_REGION,
+                             (Object **) &s->ram_memory,
+                             qdev_prop_allow_set_link_before_realize, 0);
+
+    object_property_add_link(obj, PCI_HOST_PROP_PCI_MEM, TYPE_MEMORY_REGION,
+                             (Object **) &s->pci_address_space,
+                             qdev_prop_allow_set_link_before_realize, 0);
+
+    object_property_add_link(obj, PCI_HOST_PROP_SYSTEM_MEM, TYPE_MEMORY_REGION,
+                             (Object **) &s->system_memory,
+                             qdev_prop_allow_set_link_before_realize, 0);
 }
 
 static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
@@ -255,27 +272,28 @@ PCIBus *i440fx_init(const char *pci_type,
     PCII440FXState *f;
     unsigned i;
 
-    b = pci_root_bus_new(dev, NULL, pci_address_space,
+    s->system_memory = address_space_mem;
+    s->pci_address_space = pci_address_space;
+    s->ram_memory = ram_memory;
+
+    b = pci_root_bus_new(dev, NULL, s->pci_address_space,
                          address_space_io, 0, TYPE_PCI_BUS);
     phb->bus = b;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
     d = pci_create_simple(b, 0, pci_type);
     f = I440FX_PCI_DEVICE(d);
-    f->system_memory = address_space_mem;
-    f->pci_address_space = pci_address_space;
-    f->ram_memory = ram_memory;
 
     range_set_bounds(&s->pci_hole, below_4g_mem_size,
                      IO_APIC_DEFAULT_ADDRESS - 1);
 
     /* setup pci memory mapping */
-    pc_pci_as_mapping_init(f->system_memory, f->pci_address_space);
+    pc_pci_as_mapping_init(s->system_memory, s->pci_address_space);
 
     /* if *disabled* show SMRAM to all CPUs */
     memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
-                             f->pci_address_space, SMRAM_C_BASE, SMRAM_C_SIZE);
-    memory_region_add_subregion_overlap(f->system_memory, SMRAM_C_BASE,
+                             s->pci_address_space, SMRAM_C_BASE, SMRAM_C_SIZE);
+    memory_region_add_subregion_overlap(s->system_memory, SMRAM_C_BASE,
                                         &f->smram_region, 1);
     memory_region_set_enabled(&f->smram_region, true);
 
@@ -283,17 +301,17 @@ PCIBus *i440fx_init(const char *pci_type,
     memory_region_init(&f->smram, OBJECT(d), "smram", 4 * GiB);
     memory_region_set_enabled(&f->smram, true);
     memory_region_init_alias(&f->low_smram, OBJECT(d), "smram-low",
-                             f->ram_memory, SMRAM_C_BASE, SMRAM_C_SIZE);
+                             s->ram_memory, SMRAM_C_BASE, SMRAM_C_SIZE);
     memory_region_set_enabled(&f->low_smram, true);
     memory_region_add_subregion(&f->smram, SMRAM_C_BASE, &f->low_smram);
     object_property_add_const_link(qdev_get_machine(), "smram",
                                    OBJECT(&f->smram));
 
-    init_pam(&f->pam_regions[0], OBJECT(d), f->ram_memory, f->system_memory,
-             f->pci_address_space, PAM_BIOS_BASE, PAM_BIOS_SIZE);
+    init_pam(&f->pam_regions[0], OBJECT(d), s->ram_memory, s->system_memory,
+             s->pci_address_space, PAM_BIOS_BASE, PAM_BIOS_SIZE);
     for (i = 0; i < ARRAY_SIZE(f->pam_regions) - 1; ++i) {
-        init_pam(&f->pam_regions[i + 1], OBJECT(d), f->ram_memory,
-                 f->system_memory, f->pci_address_space,
+        init_pam(&f->pam_regions[i + 1], OBJECT(d), s->ram_memory,
+                 s->system_memory, s->pci_address_space,
                  PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
     }
 
-- 
2.41.0



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

* [PATCH v3 13/17] hw/pci-host/i440fx: Add PCI_HOST_PROP_IO_MEM property
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (11 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 12/17] hw/pci-host/i440fx: Make MemoryRegion pointers accessible as properties Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 14/17] hw/pci-host/i440fx: Add PCI_HOST_{ABOVE, BELOW}_4G_MEM_SIZE properties Bernhard Beschow
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow

Introduce the property in anticipation of QOM'ification; Q35 has the same
property.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/i440fx.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index b9530fc3a0..de14c75e95 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -27,7 +27,6 @@
 #include "qemu/range.h"
 #include "hw/i386/pc.h"
 #include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_host.h"
 #include "hw/pci-host/i440fx.h"
 #include "hw/qdev-properties.h"
@@ -49,6 +48,7 @@ struct I440FXState {
     PCIHostState parent_obj;
 
     MemoryRegion *system_memory;
+    MemoryRegion *io_memory;
     MemoryRegion *pci_address_space;
     MemoryRegion *ram_memory;
     Range pci_hole;
@@ -237,17 +237,22 @@ static void i440fx_pcihost_initfn(Object *obj)
     object_property_add_link(obj, PCI_HOST_PROP_SYSTEM_MEM, TYPE_MEMORY_REGION,
                              (Object **) &s->system_memory,
                              qdev_prop_allow_set_link_before_realize, 0);
+
+    object_property_add_link(obj, PCI_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
+                             (Object **) &s->io_memory,
+                             qdev_prop_allow_set_link_before_realize, 0);
 }
 
 static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
 {
+    I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    memory_region_add_subregion(phb->bus->address_space_io, 0xcf8, &phb->conf_mem);
+    memory_region_add_subregion(s->io_memory, 0xcf8, &phb->conf_mem);
     sysbus_init_ioports(sbd, 0xcf8, 4);
 
-    memory_region_add_subregion(phb->bus->address_space_io, 0xcfc, &phb->data_mem);
+    memory_region_add_subregion(s->io_memory, 0xcfc, &phb->data_mem);
     sysbus_init_ioports(sbd, 0xcfc, 4);
 
     /* register i440fx 0xcf8 port as coalesced pio */
@@ -273,11 +278,12 @@ PCIBus *i440fx_init(const char *pci_type,
     unsigned i;
 
     s->system_memory = address_space_mem;
+    s->io_memory = address_space_io;
     s->pci_address_space = pci_address_space;
     s->ram_memory = ram_memory;
 
     b = pci_root_bus_new(dev, NULL, s->pci_address_space,
-                         address_space_io, 0, TYPE_PCI_BUS);
+                         s->io_memory, 0, TYPE_PCI_BUS);
     phb->bus = b;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
-- 
2.41.0



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

* [PATCH v3 14/17] hw/pci-host/i440fx: Add PCI_HOST_{ABOVE, BELOW}_4G_MEM_SIZE properties
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (12 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 13/17] hw/pci-host/i440fx: Add PCI_HOST_PROP_IO_MEM property Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 15/17] hw/pci-host/i440fx: Add I440FX_HOST_PROP_PCI_TYPE property Bernhard Beschow
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow

Introduce the properties in anticipation of QOM'ification; Q35 has the same
properties.

Note that we want to avoid a "ram size" property in the QOM interface since it
seems redundant to both properties introduced in this change. Thus the removal
of the ram_size parameter. We assume the invariant of both properties to sum up
to "ram size" which is already asserted in pc_memory_init(). Under Xen the
invariant seems to hold as well, so we now also check it there.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/pci-host/i440fx.h |  1 -
 hw/i386/pc_piix.c            |  5 ++++-
 hw/pci-host/i440fx.c         | 12 ++++++++++--
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index e3a550021e..7e38456ebb 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -36,7 +36,6 @@ PCIBus *i440fx_init(const char *pci_type,
                     DeviceState *dev,
                     MemoryRegion *address_space_mem,
                     MemoryRegion *address_space_io,
-                    ram_addr_t ram_size,
                     ram_addr_t below_4g_mem_size,
                     ram_addr_t above_4g_mem_size,
                     MemoryRegion *pci_memory,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 26e8473a4d..c36783809f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -224,6 +224,9 @@ static void pc_init1(MachineState *machine,
     if (!xen_enabled()) {
         pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
     } else {
+        assert(machine->ram_size == x86ms->below_4g_mem_size +
+                                    x86ms->above_4g_mem_size);
+
         pc_system_flash_cleanup_unused(pcms);
         if (machine->kernel_filename != NULL) {
             /* For xen HVM direct kernel boot, load linux here */
@@ -239,7 +242,7 @@ static void pc_init1(MachineState *machine,
 
         pci_bus = i440fx_init(pci_type,
                               i440fx_host,
-                              system_memory, system_io, machine->ram_size,
+                              system_memory, system_io,
                               x86ms->below_4g_mem_size,
                               x86ms->above_4g_mem_size,
                               pci_memory, ram_memory);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index de14c75e95..8731740a1b 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -52,6 +52,8 @@ struct I440FXState {
     MemoryRegion *pci_address_space;
     MemoryRegion *ram_memory;
     Range pci_hole;
+    uint64_t below_4g_mem_size;
+    uint64_t above_4g_mem_size;
     uint64_t pci_hole64_size;
     bool pci_hole64_fix;
     uint32_t short_root_bus;
@@ -264,7 +266,6 @@ PCIBus *i440fx_init(const char *pci_type,
                     DeviceState *dev,
                     MemoryRegion *address_space_mem,
                     MemoryRegion *address_space_io,
-                    ram_addr_t ram_size,
                     ram_addr_t below_4g_mem_size,
                     ram_addr_t above_4g_mem_size,
                     MemoryRegion *pci_address_space,
@@ -281,6 +282,8 @@ PCIBus *i440fx_init(const char *pci_type,
     s->io_memory = address_space_io;
     s->pci_address_space = pci_address_space;
     s->ram_memory = ram_memory;
+    s->below_4g_mem_size = below_4g_mem_size;
+    s->above_4g_mem_size = above_4g_mem_size;
 
     b = pci_root_bus_new(dev, NULL, s->pci_address_space,
                          s->io_memory, 0, TYPE_PCI_BUS);
@@ -290,7 +293,7 @@ PCIBus *i440fx_init(const char *pci_type,
     d = pci_create_simple(b, 0, pci_type);
     f = I440FX_PCI_DEVICE(d);
 
-    range_set_bounds(&s->pci_hole, below_4g_mem_size,
+    range_set_bounds(&s->pci_hole, s->below_4g_mem_size,
                      IO_APIC_DEFAULT_ADDRESS - 1);
 
     /* setup pci memory mapping */
@@ -321,6 +324,7 @@ PCIBus *i440fx_init(const char *pci_type,
                  PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
     }
 
+    ram_addr_t ram_size = s->below_4g_mem_size + s->above_4g_mem_size;
     ram_size = ram_size / 8 / 1024 / 1024;
     if (ram_size > 255) {
         ram_size = 255;
@@ -380,6 +384,10 @@ static Property i440fx_props[] = {
     DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState,
                      pci_hole64_size, I440FX_PCI_HOST_HOLE64_SIZE_DEFAULT),
     DEFINE_PROP_UINT32("short_root_bus", I440FXState, short_root_bus, 0),
+    DEFINE_PROP_SIZE(PCI_HOST_BELOW_4G_MEM_SIZE, I440FXState,
+                     below_4g_mem_size, 0),
+    DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MEM_SIZE, I440FXState,
+                     above_4g_mem_size, 0),
     DEFINE_PROP_BOOL("x-pci-hole64-fix", I440FXState, pci_hole64_fix, true),
     DEFINE_PROP_END_OF_LIST(),
 };
-- 
2.41.0



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

* [PATCH v3 15/17] hw/pci-host/i440fx: Add I440FX_HOST_PROP_PCI_TYPE property
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (13 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 14/17] hw/pci-host/i440fx: Add PCI_HOST_{ABOVE, BELOW}_4G_MEM_SIZE properties Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 16/17] hw/pci-host/i440fx: Resolve i440fx_init() Bernhard Beschow
  2023-06-30  7:37 ` [PATCH v3 17/17] hw/i386/pc_piix: Move i440fx' realize near its qdev_new() Bernhard Beschow
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow

I440FX needs a different PCI device model if the "igd-passthru" property is
enabled. The type name is currently passed as a parameter to i440fx_init(). This
parameter will be replaced by a property assignment once i440fx_init() gets
resolved.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/pci-host/i440fx.h | 2 ++
 hw/pci-host/i440fx.c         | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index 7e38456ebb..2d7bae5a45 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -15,6 +15,8 @@
 #include "hw/pci-host/pam.h"
 #include "qom/object.h"
 
+#define I440FX_HOST_PROP_PCI_TYPE "pci-type"
+
 #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define TYPE_I440FX_PCI_DEVICE "i440FX"
 
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 8731740a1b..c458987405 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -57,6 +57,8 @@ struct I440FXState {
     uint64_t pci_hole64_size;
     bool pci_hole64_fix;
     uint32_t short_root_bus;
+
+    char *pci_type;
 };
 
 #define I440FX_PAM      0x59
@@ -284,13 +286,14 @@ PCIBus *i440fx_init(const char *pci_type,
     s->ram_memory = ram_memory;
     s->below_4g_mem_size = below_4g_mem_size;
     s->above_4g_mem_size = above_4g_mem_size;
+    s->pci_type = (char *)pci_type;
 
     b = pci_root_bus_new(dev, NULL, s->pci_address_space,
                          s->io_memory, 0, TYPE_PCI_BUS);
     phb->bus = b;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
-    d = pci_create_simple(b, 0, pci_type);
+    d = pci_create_simple(b, 0, s->pci_type);
     f = I440FX_PCI_DEVICE(d);
 
     range_set_bounds(&s->pci_hole, s->below_4g_mem_size,
@@ -389,6 +392,7 @@ static Property i440fx_props[] = {
     DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MEM_SIZE, I440FXState,
                      above_4g_mem_size, 0),
     DEFINE_PROP_BOOL("x-pci-hole64-fix", I440FXState, pci_hole64_fix, true),
+    DEFINE_PROP_STRING(I440FX_HOST_PROP_PCI_TYPE, I440FXState, pci_type),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.41.0



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

* [PATCH v3 16/17] hw/pci-host/i440fx: Resolve i440fx_init()
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (14 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 15/17] hw/pci-host/i440fx: Add I440FX_HOST_PROP_PCI_TYPE property Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  2023-06-30 10:09   ` Philippe Mathieu-Daudé
  2023-06-30  7:37 ` [PATCH v3 17/17] hw/i386/pc_piix: Move i440fx' realize near its qdev_new() Bernhard Beschow
  16 siblings, 1 reply; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow

i440fx_init() is a legacy init function. The previous patches worked towards
TYPE_I440FX_PCI_HOST_BRIDGE to be instantiated the QOM way. Do this now by
transforming the parameters passed to i440fx_init() into property assignments.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/pci-host/i440fx.h | 10 ----------
 hw/i386/pc_piix.c            | 32 +++++++++++++++++++++-----------
 hw/pci-host/i440fx.c         | 33 +++++----------------------------
 3 files changed, 26 insertions(+), 49 deletions(-)

diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index 2d7bae5a45..c988f70890 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -34,14 +34,4 @@ struct PCII440FXState {
 
 #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"
 
-PCIBus *i440fx_init(const char *pci_type,
-                    DeviceState *dev,
-                    MemoryRegion *address_space_mem,
-                    MemoryRegion *address_space_io,
-                    ram_addr_t below_4g_mem_size,
-                    ram_addr_t above_4g_mem_size,
-                    MemoryRegion *pci_memory,
-                    MemoryRegion *ram_memory);
-
-
 #endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c36783809f..62148d7636 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -126,7 +126,7 @@ static void pc_init1(MachineState *machine,
     MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
-    DeviceState *i440fx_host = NULL;
+    Object *phb = NULL;
 
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
@@ -201,10 +201,9 @@ static void pc_init1(MachineState *machine,
         pci_memory = g_new(MemoryRegion, 1);
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
-        i440fx_host = qdev_new(host_type);
-        object_property_add_child(OBJECT(machine), "i440fx",
-                                  OBJECT(i440fx_host));
-        hole64_size = object_property_get_uint(OBJECT(i440fx_host),
+        phb = OBJECT(qdev_new(host_type));
+        object_property_add_child(OBJECT(machine), "i440fx", phb);
+        hole64_size = object_property_get_uint(phb,
                                                PCI_HOST_PROP_PCI_HOLE64_SIZE,
                                                &error_abort);
     }
@@ -240,12 +239,23 @@ static void pc_init1(MachineState *machine,
         PIIX3State *piix3;
         PCIDevice *pci_dev;
 
-        pci_bus = i440fx_init(pci_type,
-                              i440fx_host,
-                              system_memory, system_io,
-                              x86ms->below_4g_mem_size,
-                              x86ms->above_4g_mem_size,
-                              pci_memory, ram_memory);
+        object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
+                                 OBJECT(ram_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
+                                 OBJECT(pci_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
+                                 OBJECT(system_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
+                                 OBJECT(system_io), &error_fatal);
+        object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
+                                 x86ms->below_4g_mem_size, &error_fatal);
+        object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
+                                 x86ms->above_4g_mem_size, &error_fatal);
+        object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE,
+                                pci_type, &error_fatal);
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+
+        pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
         pci_bus_map_irqs(pci_bus,
                          xen_enabled() ? xen_pci_slot_get_pirq
                                        : pc_pci_slot_get_pirq);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index c458987405..62d6287681 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -249,9 +249,14 @@ static void i440fx_pcihost_initfn(Object *obj)
 
 static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
 {
+    ERRP_GUARD();
     I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    PCIBus *b;
+    PCIDevice *d;
+    PCII440FXState *f;
+    unsigned i;
 
     memory_region_add_subregion(s->io_memory, 0xcf8, &phb->conf_mem);
     sysbus_init_ioports(sbd, 0xcf8, 4);
@@ -262,36 +267,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
     /* register i440fx 0xcf8 port as coalesced pio */
     memory_region_set_flush_coalesced(&phb->data_mem);
     memory_region_add_coalescing(&phb->conf_mem, 0, 4);
-}
-
-PCIBus *i440fx_init(const char *pci_type,
-                    DeviceState *dev,
-                    MemoryRegion *address_space_mem,
-                    MemoryRegion *address_space_io,
-                    ram_addr_t below_4g_mem_size,
-                    ram_addr_t above_4g_mem_size,
-                    MemoryRegion *pci_address_space,
-                    MemoryRegion *ram_memory)
-{
-    I440FXState *s = I440FX_PCI_HOST_BRIDGE(dev);
-    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
-    PCIBus *b;
-    PCIDevice *d;
-    PCII440FXState *f;
-    unsigned i;
-
-    s->system_memory = address_space_mem;
-    s->io_memory = address_space_io;
-    s->pci_address_space = pci_address_space;
-    s->ram_memory = ram_memory;
-    s->below_4g_mem_size = below_4g_mem_size;
-    s->above_4g_mem_size = above_4g_mem_size;
-    s->pci_type = (char *)pci_type;
 
     b = pci_root_bus_new(dev, NULL, s->pci_address_space,
                          s->io_memory, 0, TYPE_PCI_BUS);
     phb->bus = b;
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
     d = pci_create_simple(b, 0, s->pci_type);
     f = I440FX_PCI_DEVICE(d);
@@ -335,8 +314,6 @@ PCIBus *i440fx_init(const char *pci_type,
     d->config[I440FX_COREBOOT_RAM_SIZE] = ram_size;
 
     i440fx_update_memory_mappings(f);
-
-    return b;
 }
 
 static void i440fx_class_init(ObjectClass *klass, void *data)
-- 
2.41.0



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

* [PATCH v3 17/17] hw/i386/pc_piix: Move i440fx' realize near its qdev_new()
  2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
                   ` (15 preceding siblings ...)
  2023-06-30  7:37 ` [PATCH v3 16/17] hw/pci-host/i440fx: Resolve i440fx_init() Bernhard Beschow
@ 2023-06-30  7:37 ` Bernhard Beschow
  16 siblings, 0 replies; 19+ messages in thread
From: Bernhard Beschow @ 2023-06-30  7:37 UTC (permalink / raw
  To: qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin, Bernhard Beschow,
	Philippe Mathieu-Daudé

I440FX realization is currently mixed with PIIX3 creation. Furthermore, it is
common practice to only set properties between a device's qdev_new() and
qdev_realize(). Clean up to resolve both issues.

Since I440FX spawns a PCI bus let's also move the pci_bus initialization there.

Note that when running `qemu-system-x86_64 -M pc -S` before and after this
patch, `info mtree` in the QEMU console doesn't show any differences except that
the ordering is different.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc_piix.c | 51 ++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 62148d7636..b18443d3df 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -114,7 +114,7 @@ static void pc_init1(MachineState *machine,
     X86MachineState *x86ms = X86_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *system_io = get_system_io();
-    PCIBus *pci_bus;
+    PCIBus *pci_bus = NULL;
     ISABus *isa_bus;
     int piix3_devfn = -1;
     qemu_irq smi_irq;
@@ -126,7 +126,6 @@ static void pc_init1(MachineState *machine,
     MemoryRegion *rom_memory = system_memory;
     ram_addr_t lowmem;
     uint64_t hole64_size = 0;
-    Object *phb = NULL;
 
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
@@ -198,11 +197,36 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pcmc->pci_enabled) {
+        Object *phb;
+
         pci_memory = g_new(MemoryRegion, 1);
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
+
         phb = OBJECT(qdev_new(host_type));
         object_property_add_child(OBJECT(machine), "i440fx", phb);
+        object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
+                                 OBJECT(ram_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
+                                 OBJECT(pci_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
+                                 OBJECT(system_memory), &error_fatal);
+        object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
+                                 OBJECT(system_io), &error_fatal);
+        object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
+                                 x86ms->below_4g_mem_size, &error_fatal);
+        object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
+                                 x86ms->above_4g_mem_size, &error_fatal);
+        object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
+                                &error_fatal);
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+
+        pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
+        pci_bus_map_irqs(pci_bus,
+                         xen_enabled() ? xen_pci_slot_get_pirq
+                                       : pc_pci_slot_get_pirq);
+        pcms->bus = pci_bus;
+
         hole64_size = object_property_get_uint(phb,
                                                PCI_HOST_PROP_PCI_HOLE64_SIZE,
                                                &error_abort);
@@ -239,28 +263,6 @@ static void pc_init1(MachineState *machine,
         PIIX3State *piix3;
         PCIDevice *pci_dev;
 
-        object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
-                                 OBJECT(ram_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
-                                 OBJECT(pci_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
-                                 OBJECT(system_memory), &error_fatal);
-        object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
-                                 OBJECT(system_io), &error_fatal);
-        object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
-                                 x86ms->below_4g_mem_size, &error_fatal);
-        object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
-                                 x86ms->above_4g_mem_size, &error_fatal);
-        object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE,
-                                pci_type, &error_fatal);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
-
-        pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
-        pci_bus_map_irqs(pci_bus,
-                         xen_enabled() ? xen_pci_slot_get_pirq
-                                       : pc_pci_slot_get_pirq);
-        pcms->bus = pci_bus;
-
         pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
                                                   TYPE_PIIX3_DEVICE);
 
@@ -285,7 +287,6 @@ static void pc_init1(MachineState *machine,
         rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
                                                              "rtc"));
     } else {
-        pci_bus = NULL;
         isa_bus = isa_bus_new(NULL, system_memory, system_io,
                               &error_abort);
 
-- 
2.41.0



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

* Re: [PATCH v3 16/17] hw/pci-host/i440fx: Resolve i440fx_init()
  2023-06-30  7:37 ` [PATCH v3 16/17] hw/pci-host/i440fx: Resolve i440fx_init() Bernhard Beschow
@ 2023-06-30 10:09   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 19+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-30 10:09 UTC (permalink / raw
  To: Bernhard Beschow, qemu-devel
  Cc: Richard Henderson, Marcel Apfelbaum, Eduardo Habkost,
	Paolo Bonzini, Michael S. Tsirkin

On 30/6/23 09:37, Bernhard Beschow wrote:
> i440fx_init() is a legacy init function. The previous patches worked towards
> TYPE_I440FX_PCI_HOST_BRIDGE to be instantiated the QOM way. Do this now by
> transforming the parameters passed to i440fx_init() into property assignments.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/pci-host/i440fx.h | 10 ----------
>   hw/i386/pc_piix.c            | 32 +++++++++++++++++++++-----------
>   hw/pci-host/i440fx.c         | 33 +++++----------------------------
>   3 files changed, 26 insertions(+), 49 deletions(-)

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


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

end of thread, other threads:[~2023-06-30 10:10 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-30  7:37 [PATCH v3 00/17] Q35 and I440FX host bridge QOM cleanup Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 01/17] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 02/17] hw/pci-host/q35: Fix double, contradicting .endianness assignment Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 03/17] hw/pci-host/q35: Initialize PCMachineState::bus in board code Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 04/17] hw/pci/pci_host: Introduce PCI_HOST_BYPASS_IOMMU macro Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 05/17] hw/pci-host/q35: Initialize PCI_HOST_BYPASS_IOMMU property from board code Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 06/17] hw/pci-host/q35: Make some property name macros reusable by i440fx Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 07/17] hw/i386/pc_piix: Turn some local variables into initializers Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 08/17] hw/pci-host/i440fx: Add "i440fx" child property in board code Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 09/17] hw/pci-host/i440fx: Replace magic values by existing constants Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 10/17] hw/pci-host/i440fx: Have common names for some local variables Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 11/17] hw/pci-host/i440fx: Move i440fx_realize() into PCII440FXState section Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 12/17] hw/pci-host/i440fx: Make MemoryRegion pointers accessible as properties Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 13/17] hw/pci-host/i440fx: Add PCI_HOST_PROP_IO_MEM property Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 14/17] hw/pci-host/i440fx: Add PCI_HOST_{ABOVE, BELOW}_4G_MEM_SIZE properties Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 15/17] hw/pci-host/i440fx: Add I440FX_HOST_PROP_PCI_TYPE property Bernhard Beschow
2023-06-30  7:37 ` [PATCH v3 16/17] hw/pci-host/i440fx: Resolve i440fx_init() Bernhard Beschow
2023-06-30 10:09   ` Philippe Mathieu-Daudé
2023-06-30  7:37 ` [PATCH v3 17/17] hw/i386/pc_piix: Move i440fx' realize near its qdev_new() Bernhard Beschow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.