[PATCH v2 00/13] Q35 PCI host fixes and QOM cleanup

[PATCH v2 00/13] Q35 PCI host fixes and QOM cleanup

[Bernhard Beschow]

This series mostly cleans up QOM-related initialization code. It also performs
some modernization and fixing.

The first patch originates from "PC and ICH9 clanups" series [1] which has been
dropped in v3 in favor of another series [2]. Review comments in [2] suggest it
needs more work, so bring the patch back here.

Patch 2 fixes a clangd warning and patch 3 modernizes usage of the memory API.

Patches 4-9 clean up initialization code.

The last four patches also clean up initialization code with the last patch
doing the actual cleanup.

Testing done:
* `make check`
* `make check-avocado`
* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
     manjaro-kde-21.3.2-220704-linux515.iso`

v2 (addresses Michael's comments):
- Patch "hw/pci-host/q35: Fix double, contradicting .endianness assignment"
  - Fix Fixes tag
  - Switch to native endian
  - Add clang warning
- Patch "Use memory_region_set_address() also for tseg_blackhole"
  - Rephrase commit message to avoid pseudo "Ammends" tag
- Introduce PCI_HOST_BYPASS_IOMMU macro to avoid duplicating the property name
- Patch "hw/pci-host/q35: Initialize properties just once"
  - Mention manual reassignment in commit message as the problem being fixed

Based-on: <20230213162004.2797-1-shentey@gmail.com>
         "[PATCH v4 0/9] PC cleanups"

[1] https://lore.kernel.org/qemu-devel/20230131115326.12454-1-shentey@gmail.com/
[2] https://lore.kernel.org/qemu-devel/20230203180914.49112-1-philmd@linaro.org/

Bernhard Beschow (13):
  hw/i386/pc_q35: Resolve redundant q35_host variable
  hw/pci-host/q35: Fix double, contradicting .endianness assignment
  hw/pci-host/q35: Use memory_region_set_address() also for
    tseg_blackhole
  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 "bypass-iommu" property from board code
  hw/pci-host/q35: Initialize properties just once
  hw/pci-host/q35: Initialize PCI hole boundaries just once
  hw/pci-host/q35: Turn PCI hole properties into class properties
  hw/pci-host/q35: Rename local variable to more idiomatic "phb"
  hw/pci-host/q35: Propagate to errp rather than doing error_fatal
  hw/pci-host/q35: Merge mch_realize() into q35_host_realize()
  hw/pci-host/q35: Move MemoryRegion pointers to host device

 include/hw/pci-host/q35.h |  17 +-
 include/hw/pci/pci_host.h |   2 +
 hw/i386/pc_q35.c          |  33 ++--
 hw/pci-host/q35.c         | 317 ++++++++++++++++++--------------------
 hw/pci/pci_host.c         |   2 +-
 5 files changed, 181 insertions(+), 190 deletions(-)

-- 
2.39.2

[PATCH v2 01/13] hw/i386/pc_q35: Resolve redundant q35_host variable

[Bernhard Beschow]

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>
---
 hw/i386/pc_q35.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 5a1e4976ce..68097bea55 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -119,8 +119,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;
@@ -206,10 +205,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);
     }
@@ -217,25 +216,25 @@ 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_SMRAM_MEM,
+    object_property_set_link(phb, MCH_HOST_PROP_SMRAM_MEM,
                              OBJECT(&x86ms->smram), 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.39.2

[PATCH v2 02/13] hw/pci-host/q35: Fix double, contradicting .endianness assignment

[Bernhard Beschow]

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 native endian which causes the least overhead.

Fixes: bafc90bdc594 ("q35: implement TSEG")
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 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 83f2a98c71..40bfe99910 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -294,7 +294,6 @@ static const MemoryRegionOps blackhole_ops = {
     .valid.max_access_size = 4,
     .impl.min_access_size = 4,
     .impl.max_access_size = 4,
-    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 /* PCIe MMCFG */
-- 
2.39.2

[PATCH v2 03/13] hw/pci-host/q35: Use memory_region_set_address() also for tseg_blackhole

[Bernhard Beschow]

Commit bafc90bdc594 ("q35: implement TSEG") uses
memory_region_set_address() for updating the address of mch->tseg_window
but uses memory_region_del_subregion() and
memory_region_add_subregion_overlap() for doing the same on mch-
>tseg_blackhole. The latter seems to be the old, cumbersome
way of changing a memory region's address. So make the code more
comprehensible by modernizing it.

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

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 40bfe99910..0497194983 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -404,12 +404,11 @@ static void mch_update_smram(MCHPCIState *mch)
     } else {
         tseg_size = 0;
     }
-    memory_region_del_subregion(mch->system_memory, &mch->tseg_blackhole);
+
     memory_region_set_enabled(&mch->tseg_blackhole, tseg_size);
     memory_region_set_size(&mch->tseg_blackhole, tseg_size);
-    memory_region_add_subregion_overlap(mch->system_memory,
-                                        mch->below_4g_mem_size - tseg_size,
-                                        &mch->tseg_blackhole, 1);
+    memory_region_set_address(&mch->tseg_blackhole,
+                              mch->below_4g_mem_size - tseg_size);
 
     memory_region_set_enabled(&mch->tseg_window, tseg_size);
     memory_region_set_size(&mch->tseg_window, tseg_size);
-- 
2.39.2

[PATCH v2 04/13] hw/pci-host/q35: Initialize PCMachineState::bus in board code

[Bernhard Beschow]

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>
---
 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 68097bea55..42e79433a5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -231,10 +231,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 0497194983..9d21915a55 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.39.2

[PATCH v2 05/13] hw/pci/pci_host: Introduce PCI_HOST_BYPASS_IOMMU macro

[Bernhard Beschow]

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>
---
 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.39.2

[PATCH v2 06/13] hw/pci-host/q35: Initialize "bypass-iommu" property from board code

[Bernhard Beschow]

The Q35 PCI host already has a "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>
---
 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 42e79433a5..d620d92214 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -231,6 +231,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 9d21915a55..f070842312 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.39.2

[PATCH v2 07/13] hw/pci-host/q35: Initialize properties just once

[Bernhard Beschow]

Although not used there, the attributes for Q35's "pci-hole64-size" and
"short_root_bus" properties currently reside in its child device. This
causes the default values to be overwritten during the child's
object_initialize() phase, requiring the host to re-assign the default
values manually again. Avoid this by moving both attributes into the
host device.

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

diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index fcbe57b42d..93e41ffbee 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -54,8 +54,6 @@ struct MCHPCIState {
     Range pci_hole;
     uint64_t below_4g_mem_size;
     uint64_t above_4g_mem_size;
-    uint64_t pci_hole64_size;
-    uint32_t short_root_bus;
     uint16_t ext_tseg_mbytes;
 };
 
@@ -64,7 +62,10 @@ struct Q35PCIHost {
     PCIExpressHost parent_obj;
     /*< public >*/
 
+    uint64_t pci_hole64_size;
+    uint32_t short_root_bus;
     bool pci_hole64_fix;
+
     MCHPCIState mch;
 };
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index f070842312..f20e092516 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -76,7 +76,7 @@ static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
     Q35PCIHost *s = Q35_HOST_DEVICE(host_bridge);
 
      /* For backwards compat with old device paths */
-    if (s->mch.short_root_bus) {
+    if (s->short_root_bus) {
         return "0000";
     }
     return "0000:00";
@@ -161,7 +161,7 @@ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
 
     pci_bus_get_w64_range(h->bus, &w64);
     value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
-    hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30);
+    hole64_end = ROUND_UP(hole64_start + s->pci_hole64_size, 1ULL << 30);
     if (s->pci_hole64_fix && value < hole64_end) {
         value = hole64_end;
     }
@@ -180,8 +180,8 @@ static Property q35_host_props[] = {
     DEFINE_PROP_UINT64(PCIE_HOST_MCFG_BASE, Q35PCIHost, parent_obj.base_addr,
                         MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT),
     DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost,
-                     mch.pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT),
-    DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0),
+                     pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT),
+    DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, short_root_bus, 0),
     DEFINE_PROP_SIZE(PCI_HOST_BELOW_4G_MEM_SIZE, Q35PCIHost,
                      mch.below_4g_mem_size, 0),
     DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MEM_SIZE, Q35PCIHost,
@@ -218,9 +218,7 @@ static void q35_host_initfn(Object *obj)
     object_initialize_child(OBJECT(s), "mch", &s->mch, TYPE_MCH_PCI_DEVICE);
     qdev_prop_set_int32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0));
     qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false);
-    /* mch's object_initialize resets the default value, set it again */
-    qdev_prop_set_uint64(DEVICE(s), PCI_HOST_PROP_PCI_HOLE64_SIZE,
-                         Q35_PCI_HOST_HOLE64_SIZE_DEFAULT);
+
     object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
                         q35_host_get_pci_hole_start,
                         NULL, NULL, NULL);
-- 
2.39.2

[PATCH v2 08/13] hw/pci-host/q35: Initialize PCI hole boundaries just once

[Bernhard Beschow]

The boundaries of the PCI hole depend on a property only which doesn't
change at runtime. There is no need to reevaluate the boundaries
whenever the PCI configuration space changes.

While at it, move the pci_hole attribute into the host device since it
is only used there.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/pci-host/q35.h |  2 +-
 hw/pci-host/q35.c         | 21 +++++++++------------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index 93e41ffbee..a04d5f1a17 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -51,7 +51,6 @@ struct MCHPCIState {
     MemoryRegion tseg_blackhole, tseg_window;
     MemoryRegion smbase_blackhole, smbase_window;
     bool has_smram_at_smbase;
-    Range pci_hole;
     uint64_t below_4g_mem_size;
     uint64_t above_4g_mem_size;
     uint16_t ext_tseg_mbytes;
@@ -62,6 +61,7 @@ struct Q35PCIHost {
     PCIExpressHost parent_obj;
     /*< public >*/
 
+    Range pci_hole;
     uint64_t pci_hole64_size;
     uint32_t short_root_bus;
     bool pci_hole64_fix;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index f20e092516..23df52a256 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -62,6 +62,13 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     memory_region_set_flush_coalesced(&pci->data_mem);
     memory_region_add_coalescing(&pci->conf_mem, 0, 4);
 
+    /*
+     * pci hole goes from end-of-low-ram to io-apic.
+     * mmconfig will be excluded by the dsdt builder.
+     */
+    range_set_bounds(&s->pci_hole, s->mch.below_4g_mem_size,
+                     IO_APIC_DEFAULT_ADDRESS - 1);
+
     pci->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
                                 s->mch.pci_address_space,
                                 s->mch.address_space_io,
@@ -90,8 +97,7 @@ static void q35_host_get_pci_hole_start(Object *obj, Visitor *v,
     uint64_t val64;
     uint32_t value;
 
-    val64 = range_is_empty(&s->mch.pci_hole)
-        ? 0 : range_lob(&s->mch.pci_hole);
+    val64 = range_is_empty(&s->pci_hole) ? 0 : range_lob(&s->pci_hole);
     value = val64;
     assert(value == val64);
     visit_type_uint32(v, name, &value, errp);
@@ -105,8 +111,7 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor *v,
     uint64_t val64;
     uint32_t value;
 
-    val64 = range_is_empty(&s->mch.pci_hole)
-        ? 0 : range_upb(&s->mch.pci_hole) + 1;
+    val64 = range_is_empty(&s->pci_hole) ? 0 : range_upb(&s->pci_hole) + 1;
     value = val64;
     assert(value == val64);
     visit_type_uint32(v, name, &value, errp);
@@ -506,14 +511,6 @@ static void mch_update(MCHPCIState *mch)
     mch_update_smram(mch);
     mch_update_ext_tseg_mbytes(mch);
     mch_update_smbase_smram(mch);
-
-    /*
-     * pci hole goes from end-of-low-ram to io-apic.
-     * mmconfig will be excluded by the dsdt builder.
-     */
-    range_set_bounds(&mch->pci_hole,
-                     mch->below_4g_mem_size,
-                     IO_APIC_DEFAULT_ADDRESS - 1);
 }
 
 static int mch_post_load(void *opaque, int version_id)
-- 
2.39.2

[PATCH v2 09/13] hw/pci-host/q35: Turn PCI hole properties into class properties

[Bernhard Beschow]

These properties are class properties in i440fx. No need to handle them
differently in q35.

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

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 23df52a256..afd192cc2a 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -207,6 +207,22 @@ static void q35_host_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = false;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->fw_name = "pci";
+
+    object_class_property_add(klass, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
+                              q35_host_get_pci_hole_start,
+                              NULL, NULL, NULL);
+
+    object_class_property_add(klass, PCI_HOST_PROP_PCI_HOLE_END, "uint32",
+                              q35_host_get_pci_hole_end,
+                              NULL, NULL, NULL);
+
+    object_class_property_add(klass, PCI_HOST_PROP_PCI_HOLE64_START, "uint64",
+                              q35_host_get_pci_hole64_start,
+                              NULL, NULL, NULL);
+
+    object_class_property_add(klass, PCI_HOST_PROP_PCI_HOLE64_END, "uint64",
+                              q35_host_get_pci_hole64_end,
+                              NULL, NULL, NULL);
 }
 
 static void q35_host_initfn(Object *obj)
@@ -224,22 +240,6 @@ static void q35_host_initfn(Object *obj)
     qdev_prop_set_int32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0));
     qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false);
 
-    object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
-                        q35_host_get_pci_hole_start,
-                        NULL, NULL, NULL);
-
-    object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_END, "uint32",
-                        q35_host_get_pci_hole_end,
-                        NULL, NULL, NULL);
-
-    object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_START, "uint64",
-                        q35_host_get_pci_hole64_start,
-                        NULL, NULL, NULL);
-
-    object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_END, "uint64",
-                        q35_host_get_pci_hole64_end,
-                        NULL, NULL, NULL);
-
     object_property_add_uint64_ptr(obj, PCIE_HOST_MCFG_SIZE,
                                    &pehb->size, OBJ_PROP_FLAG_READ);
 
-- 
2.39.2

[PATCH v2 10/13] hw/pci-host/q35: Rename local variable to more idiomatic "phb"

[Bernhard Beschow]

Variables of type PCIHostState* are typically named "phb" in QEMU.
Follow this convention here as well for consistency.

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

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index afd192cc2a..cf9fb35064 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -46,21 +46,21 @@
 
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
-    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     Q35PCIHost *s = Q35_HOST_DEVICE(dev);
+    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
     memory_region_add_subregion(s->mch.address_space_io,
-                                MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
+                                MCH_HOST_BRIDGE_CONFIG_ADDR, &phb->conf_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
     memory_region_add_subregion(s->mch.address_space_io,
-                                MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
+                                MCH_HOST_BRIDGE_CONFIG_DATA, &phb->data_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
     /* register q35 0xcf8 port as coalesced pio */
-    memory_region_set_flush_coalesced(&pci->data_mem);
-    memory_region_add_coalescing(&pci->conf_mem, 0, 4);
+    memory_region_set_flush_coalesced(&phb->data_mem);
+    memory_region_add_coalescing(&phb->conf_mem, 0, 4);
 
     /*
      * pci hole goes from end-of-low-ram to io-apic.
@@ -69,12 +69,12 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     range_set_bounds(&s->pci_hole, s->mch.below_4g_mem_size,
                      IO_APIC_DEFAULT_ADDRESS - 1);
 
-    pci->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
+    phb->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
                                 s->mch.pci_address_space,
                                 s->mch.address_space_io,
                                 0, TYPE_PCIE_BUS);
 
-    qdev_realize(DEVICE(&s->mch), BUS(pci->bus), &error_fatal);
+    qdev_realize(DEVICE(&s->mch), BUS(phb->bus), &error_fatal);
 }
 
 static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
-- 
2.39.2

[PATCH v2 11/13] hw/pci-host/q35: Propagate to errp rather than doing error_fatal

[Bernhard Beschow]

q35_host_realize() has an errp parameter. Use that to be able to
propagate the error instead of terminating abruptly.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/q35.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index cf9fb35064..39d70b9f59 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -46,6 +46,7 @@
 
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
+    ERRP_GUARD();
     Q35PCIHost *s = Q35_HOST_DEVICE(dev);
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
@@ -74,7 +75,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
                                 s->mch.address_space_io,
                                 0, TYPE_PCIE_BUS);
 
-    qdev_realize(DEVICE(&s->mch), BUS(phb->bus), &error_fatal);
+    qdev_realize(DEVICE(&s->mch), BUS(phb->bus), errp);
 }
 
 static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
-- 
2.39.2

[PATCH v2 12/13] hw/pci-host/q35: Merge mch_realize() into q35_host_realize()

[Bernhard Beschow]

This patch prepares movement of the MemoryRegion pointers (which are set
through properties) into the host state. Moreover, it's usually the
parent device which maps the memory regions of its child devices into
its address space. Do the same in q35.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/q35.c | 209 ++++++++++++++++++++++------------------------
 1 file changed, 101 insertions(+), 108 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 39d70b9f59..1e0f5b4fbf 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -44,12 +44,40 @@
 
 #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
 
+static uint64_t blackhole_read(void *ptr, hwaddr reg, unsigned size)
+{
+    return 0xffffffff;
+}
+
+static void blackhole_write(void *opaque, hwaddr addr, uint64_t val,
+                            unsigned width)
+{
+    /* nothing */
+}
+
+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,
+    .impl.max_access_size = 4,
+};
+
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
     ERRP_GUARD();
     Q35PCIHost *s = Q35_HOST_DEVICE(dev);
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    int i;
+
+    if (s->mch.ext_tseg_mbytes > MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX) {
+        error_setg(errp, "invalid extended-tseg-mbytes value: %" PRIu16,
+                   s->mch.ext_tseg_mbytes);
+        return;
+    }
 
     memory_region_add_subregion(s->mch.address_space_io,
                                 MCH_HOST_BRIDGE_CONFIG_ADDR, &phb->conf_mem);
@@ -70,6 +98,79 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     range_set_bounds(&s->pci_hole, s->mch.below_4g_mem_size,
                      IO_APIC_DEFAULT_ADDRESS - 1);
 
+    /* setup pci memory mapping */
+    pc_pci_as_mapping_init(s->mch.system_memory, s->mch.pci_address_space);
+
+    /* if *disabled* show SMRAM to all CPUs */
+    memory_region_init_alias(&s->mch.smram_region, OBJECT(s), "smram-region",
+                             s->mch.pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
+    memory_region_add_subregion_overlap(s->mch.system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                                        &s->mch.smram_region, 1);
+    memory_region_set_enabled(&s->mch.smram_region, true);
+
+    memory_region_init_alias(&s->mch.open_high_smram, OBJECT(s), "smram-open-high",
+                             s->mch.ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
+    memory_region_add_subregion_overlap(s->mch.system_memory, 0xfeda0000,
+                                        &s->mch.open_high_smram, 1);
+    memory_region_set_enabled(&s->mch.open_high_smram, false);
+
+    /* smram, as seen by SMM CPUs */
+    memory_region_init_alias(&s->mch.low_smram, OBJECT(s), "smram-low",
+                             s->mch.ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
+    memory_region_set_enabled(&s->mch.low_smram, true);
+    memory_region_add_subregion(s->mch.smram, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                                &s->mch.low_smram);
+    memory_region_init_alias(&s->mch.high_smram, OBJECT(s), "smram-high",
+                             s->mch.ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
+    memory_region_set_enabled(&s->mch.high_smram, true);
+    memory_region_add_subregion(s->mch.smram, 0xfeda0000, &s->mch.high_smram);
+
+    memory_region_init_io(&s->mch.tseg_blackhole, OBJECT(s),
+                          &blackhole_ops, NULL, "tseg-blackhole", 0);
+    memory_region_set_enabled(&s->mch.tseg_blackhole, false);
+    memory_region_add_subregion_overlap(s->mch.system_memory,
+                                        s->mch.below_4g_mem_size,
+                                        &s->mch.tseg_blackhole, 1);
+
+    memory_region_init_alias(&s->mch.tseg_window, OBJECT(s), "tseg-window",
+                             s->mch.ram_memory, s->mch.below_4g_mem_size, 0);
+    memory_region_set_enabled(&s->mch.tseg_window, false);
+    memory_region_add_subregion(s->mch.smram, s->mch.below_4g_mem_size,
+                                &s->mch.tseg_window);
+
+    /*
+     * This is not what hardware does, so it's QEMU specific hack.
+     * See commit message for details.
+     */
+    memory_region_init_io(&s->mch.smbase_blackhole, OBJECT(s), &blackhole_ops,
+                          NULL, "smbase-blackhole",
+                          MCH_HOST_BRIDGE_SMBASE_SIZE);
+    memory_region_set_enabled(&s->mch.smbase_blackhole, false);
+    memory_region_add_subregion_overlap(s->mch.system_memory,
+                                        MCH_HOST_BRIDGE_SMBASE_ADDR,
+                                        &s->mch.smbase_blackhole, 1);
+
+    memory_region_init_alias(&s->mch.smbase_window, OBJECT(s),
+                             "smbase-window", s->mch.ram_memory,
+                             MCH_HOST_BRIDGE_SMBASE_ADDR,
+                             MCH_HOST_BRIDGE_SMBASE_SIZE);
+    memory_region_set_enabled(&s->mch.smbase_window, false);
+    memory_region_add_subregion(s->mch.smram, MCH_HOST_BRIDGE_SMBASE_ADDR,
+                                &s->mch.smbase_window);
+
+    init_pam(&s->mch.pam_regions[0], OBJECT(s), s->mch.ram_memory,
+             s->mch.system_memory, s->mch.pci_address_space,
+             PAM_BIOS_BASE, PAM_BIOS_SIZE);
+    for (i = 0; i < ARRAY_SIZE(s->mch.pam_regions) - 1; ++i) {
+        init_pam(&s->mch.pam_regions[i + 1], OBJECT(s), s->mch.ram_memory,
+                 s->mch.system_memory, s->mch.pci_address_space,
+                 PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
+    }
+
     phb->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
                                 s->mch.pci_address_space,
                                 s->mch.address_space_io,
@@ -277,27 +378,6 @@ static const TypeInfo q35_host_info = {
  * MCH D0:F0
  */
 
-static uint64_t blackhole_read(void *ptr, hwaddr reg, unsigned size)
-{
-    return 0xffffffff;
-}
-
-static void blackhole_write(void *opaque, hwaddr addr, uint64_t val,
-                            unsigned width)
-{
-    /* nothing */
-}
-
-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,
-    .impl.max_access_size = 4,
-};
-
 /* PCIe MMCFG */
 static void mch_update_pciexbar(MCHPCIState *mch)
 {
@@ -560,92 +640,6 @@ static void mch_reset(DeviceState *qdev)
     mch_update(mch);
 }
 
-static void mch_realize(PCIDevice *d, Error **errp)
-{
-    int i;
-    MCHPCIState *mch = MCH_PCI_DEVICE(d);
-
-    if (mch->ext_tseg_mbytes > MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX) {
-        error_setg(errp, "invalid extended-tseg-mbytes value: %" PRIu16,
-                   mch->ext_tseg_mbytes);
-        return;
-    }
-
-    /* setup pci memory mapping */
-    pc_pci_as_mapping_init(mch->system_memory, mch->pci_address_space);
-
-    /* if *disabled* show SMRAM to all CPUs */
-    memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region",
-                             mch->pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE,
-                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
-    memory_region_add_subregion_overlap(mch->system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
-                                        &mch->smram_region, 1);
-    memory_region_set_enabled(&mch->smram_region, true);
-
-    memory_region_init_alias(&mch->open_high_smram, OBJECT(mch), "smram-open-high",
-                             mch->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
-                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
-    memory_region_add_subregion_overlap(mch->system_memory, 0xfeda0000,
-                                        &mch->open_high_smram, 1);
-    memory_region_set_enabled(&mch->open_high_smram, false);
-
-    /* smram, as seen by SMM CPUs */
-    memory_region_init_alias(&mch->low_smram, OBJECT(mch), "smram-low",
-                             mch->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
-                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
-    memory_region_set_enabled(&mch->low_smram, true);
-    memory_region_add_subregion(mch->smram, MCH_HOST_BRIDGE_SMRAM_C_BASE,
-                                &mch->low_smram);
-    memory_region_init_alias(&mch->high_smram, OBJECT(mch), "smram-high",
-                             mch->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
-                             MCH_HOST_BRIDGE_SMRAM_C_SIZE);
-    memory_region_set_enabled(&mch->high_smram, true);
-    memory_region_add_subregion(mch->smram, 0xfeda0000, &mch->high_smram);
-
-    memory_region_init_io(&mch->tseg_blackhole, OBJECT(mch),
-                          &blackhole_ops, NULL,
-                          "tseg-blackhole", 0);
-    memory_region_set_enabled(&mch->tseg_blackhole, false);
-    memory_region_add_subregion_overlap(mch->system_memory,
-                                        mch->below_4g_mem_size,
-                                        &mch->tseg_blackhole, 1);
-
-    memory_region_init_alias(&mch->tseg_window, OBJECT(mch), "tseg-window",
-                             mch->ram_memory, mch->below_4g_mem_size, 0);
-    memory_region_set_enabled(&mch->tseg_window, false);
-    memory_region_add_subregion(mch->smram, mch->below_4g_mem_size,
-                                &mch->tseg_window);
-
-    /*
-     * This is not what hardware does, so it's QEMU specific hack.
-     * See commit message for details.
-     */
-    memory_region_init_io(&mch->smbase_blackhole, OBJECT(mch), &blackhole_ops,
-                          NULL, "smbase-blackhole",
-                          MCH_HOST_BRIDGE_SMBASE_SIZE);
-    memory_region_set_enabled(&mch->smbase_blackhole, false);
-    memory_region_add_subregion_overlap(mch->system_memory,
-                                        MCH_HOST_BRIDGE_SMBASE_ADDR,
-                                        &mch->smbase_blackhole, 1);
-
-    memory_region_init_alias(&mch->smbase_window, OBJECT(mch),
-                             "smbase-window", mch->ram_memory,
-                             MCH_HOST_BRIDGE_SMBASE_ADDR,
-                             MCH_HOST_BRIDGE_SMBASE_SIZE);
-    memory_region_set_enabled(&mch->smbase_window, false);
-    memory_region_add_subregion(mch->smram, MCH_HOST_BRIDGE_SMBASE_ADDR,
-                                &mch->smbase_window);
-
-    init_pam(&mch->pam_regions[0], OBJECT(mch), mch->ram_memory,
-             mch->system_memory, mch->pci_address_space,
-             PAM_BIOS_BASE, PAM_BIOS_SIZE);
-    for (i = 0; i < ARRAY_SIZE(mch->pam_regions) - 1; ++i) {
-        init_pam(&mch->pam_regions[i + 1], OBJECT(mch), mch->ram_memory,
-                 mch->system_memory, mch->pci_address_space,
-                 PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
-    }
-}
-
 uint64_t mch_mcfg_base(void)
 {
     bool ambiguous;
@@ -668,7 +662,6 @@ static void mch_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->realize = mch_realize;
     k->config_write = mch_write_config;
     dc->reset = mch_reset;
     device_class_set_props(dc, mch_props);
-- 
2.39.2

[PATCH v2 13/13] hw/pci-host/q35: Move MemoryRegion pointers to host device

[Bernhard Beschow]

The pointers are set through the host device's properties and are only
used during its realization phase.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/pci-host/q35.h | 10 +++----
 hw/pci-host/q35.c         | 56 +++++++++++++++++++--------------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index a04d5f1a17..9b9ce48ca8 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -40,11 +40,6 @@ struct MCHPCIState {
     PCIDevice parent_obj;
     /*< public >*/
 
-    MemoryRegion *ram_memory;
-    MemoryRegion *pci_address_space;
-    MemoryRegion *system_memory;
-    MemoryRegion *address_space_io;
-    MemoryRegion *smram;
     PAMMemoryRegion pam_regions[PAM_REGIONS_COUNT];
     MemoryRegion smram_region, open_high_smram;
     MemoryRegion low_smram, high_smram;
@@ -61,6 +56,11 @@ struct Q35PCIHost {
     PCIExpressHost parent_obj;
     /*< public >*/
 
+    MemoryRegion *ram_memory;
+    MemoryRegion *pci_address_space;
+    MemoryRegion *system_memory;
+    MemoryRegion *address_space_io;
+    MemoryRegion *smram;
     Range pci_hole;
     uint64_t pci_hole64_size;
     uint32_t short_root_bus;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 1e0f5b4fbf..769bdffcc2 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -79,11 +79,11 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    memory_region_add_subregion(s->mch.address_space_io,
+    memory_region_add_subregion(s->address_space_io,
                                 MCH_HOST_BRIDGE_CONFIG_ADDR, &phb->conf_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
-    memory_region_add_subregion(s->mch.address_space_io,
+    memory_region_add_subregion(s->address_space_io,
                                 MCH_HOST_BRIDGE_CONFIG_DATA, &phb->data_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
@@ -99,47 +99,47 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
                      IO_APIC_DEFAULT_ADDRESS - 1);
 
     /* setup pci memory mapping */
-    pc_pci_as_mapping_init(s->mch.system_memory, s->mch.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(&s->mch.smram_region, OBJECT(s), "smram-region",
-                             s->mch.pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             s->pci_address_space, MCH_HOST_BRIDGE_SMRAM_C_BASE,
                              MCH_HOST_BRIDGE_SMRAM_C_SIZE);
-    memory_region_add_subregion_overlap(s->mch.system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+    memory_region_add_subregion_overlap(s->system_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
                                         &s->mch.smram_region, 1);
     memory_region_set_enabled(&s->mch.smram_region, true);
 
     memory_region_init_alias(&s->mch.open_high_smram, OBJECT(s), "smram-open-high",
-                             s->mch.ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             s->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
                              MCH_HOST_BRIDGE_SMRAM_C_SIZE);
-    memory_region_add_subregion_overlap(s->mch.system_memory, 0xfeda0000,
+    memory_region_add_subregion_overlap(s->system_memory, 0xfeda0000,
                                         &s->mch.open_high_smram, 1);
     memory_region_set_enabled(&s->mch.open_high_smram, false);
 
     /* smram, as seen by SMM CPUs */
     memory_region_init_alias(&s->mch.low_smram, OBJECT(s), "smram-low",
-                             s->mch.ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             s->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
                              MCH_HOST_BRIDGE_SMRAM_C_SIZE);
     memory_region_set_enabled(&s->mch.low_smram, true);
-    memory_region_add_subregion(s->mch.smram, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+    memory_region_add_subregion(s->smram, MCH_HOST_BRIDGE_SMRAM_C_BASE,
                                 &s->mch.low_smram);
     memory_region_init_alias(&s->mch.high_smram, OBJECT(s), "smram-high",
-                             s->mch.ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
+                             s->ram_memory, MCH_HOST_BRIDGE_SMRAM_C_BASE,
                              MCH_HOST_BRIDGE_SMRAM_C_SIZE);
     memory_region_set_enabled(&s->mch.high_smram, true);
-    memory_region_add_subregion(s->mch.smram, 0xfeda0000, &s->mch.high_smram);
+    memory_region_add_subregion(s->smram, 0xfeda0000, &s->mch.high_smram);
 
     memory_region_init_io(&s->mch.tseg_blackhole, OBJECT(s),
                           &blackhole_ops, NULL, "tseg-blackhole", 0);
     memory_region_set_enabled(&s->mch.tseg_blackhole, false);
-    memory_region_add_subregion_overlap(s->mch.system_memory,
+    memory_region_add_subregion_overlap(s->system_memory,
                                         s->mch.below_4g_mem_size,
                                         &s->mch.tseg_blackhole, 1);
 
     memory_region_init_alias(&s->mch.tseg_window, OBJECT(s), "tseg-window",
-                             s->mch.ram_memory, s->mch.below_4g_mem_size, 0);
+                             s->ram_memory, s->mch.below_4g_mem_size, 0);
     memory_region_set_enabled(&s->mch.tseg_window, false);
-    memory_region_add_subregion(s->mch.smram, s->mch.below_4g_mem_size,
+    memory_region_add_subregion(s->smram, s->mch.below_4g_mem_size,
                                 &s->mch.tseg_window);
 
     /*
@@ -150,30 +150,30 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
                           NULL, "smbase-blackhole",
                           MCH_HOST_BRIDGE_SMBASE_SIZE);
     memory_region_set_enabled(&s->mch.smbase_blackhole, false);
-    memory_region_add_subregion_overlap(s->mch.system_memory,
+    memory_region_add_subregion_overlap(s->system_memory,
                                         MCH_HOST_BRIDGE_SMBASE_ADDR,
                                         &s->mch.smbase_blackhole, 1);
 
     memory_region_init_alias(&s->mch.smbase_window, OBJECT(s),
-                             "smbase-window", s->mch.ram_memory,
+                             "smbase-window", s->ram_memory,
                              MCH_HOST_BRIDGE_SMBASE_ADDR,
                              MCH_HOST_BRIDGE_SMBASE_SIZE);
     memory_region_set_enabled(&s->mch.smbase_window, false);
-    memory_region_add_subregion(s->mch.smram, MCH_HOST_BRIDGE_SMBASE_ADDR,
+    memory_region_add_subregion(s->smram, MCH_HOST_BRIDGE_SMBASE_ADDR,
                                 &s->mch.smbase_window);
 
-    init_pam(&s->mch.pam_regions[0], OBJECT(s), s->mch.ram_memory,
-             s->mch.system_memory, s->mch.pci_address_space,
+    init_pam(&s->mch.pam_regions[0], OBJECT(s), s->ram_memory,
+             s->system_memory, s->pci_address_space,
              PAM_BIOS_BASE, PAM_BIOS_SIZE);
     for (i = 0; i < ARRAY_SIZE(s->mch.pam_regions) - 1; ++i) {
-        init_pam(&s->mch.pam_regions[i + 1], OBJECT(s), s->mch.ram_memory,
-                 s->mch.system_memory, s->mch.pci_address_space,
+        init_pam(&s->mch.pam_regions[i + 1], OBJECT(s), s->ram_memory,
+                 s->system_memory, s->pci_address_space,
                  PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
     }
 
     phb->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
-                                s->mch.pci_address_space,
-                                s->mch.address_space_io,
+                                s->pci_address_space,
+                                s->address_space_io,
                                 0, TYPE_PCIE_BUS);
 
     qdev_realize(DEVICE(&s->mch), BUS(phb->bus), errp);
@@ -346,23 +346,23 @@ static void q35_host_initfn(Object *obj)
                                    &pehb->size, OBJ_PROP_FLAG_READ);
 
     object_property_add_link(obj, MCH_HOST_PROP_RAM_MEM, TYPE_MEMORY_REGION,
-                             (Object **) &s->mch.ram_memory,
+                             (Object **) &s->ram_memory,
                              qdev_prop_allow_set_link_before_realize, 0);
 
     object_property_add_link(obj, MCH_HOST_PROP_SMRAM_MEM, TYPE_MEMORY_REGION,
-                             (Object **) &s->mch.smram,
+                             (Object **) &s->smram,
                              qdev_prop_allow_set_link_before_realize, 0);
 
     object_property_add_link(obj, MCH_HOST_PROP_PCI_MEM, TYPE_MEMORY_REGION,
-                             (Object **) &s->mch.pci_address_space,
+                             (Object **) &s->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 **) &s->mch.system_memory,
+                             (Object **) &s->system_memory,
                              qdev_prop_allow_set_link_before_realize, 0);
 
     object_property_add_link(obj, MCH_HOST_PROP_IO_MEM, TYPE_MEMORY_REGION,
-                             (Object **) &s->mch.address_space_io,
+                             (Object **) &s->address_space_io,
                              qdev_prop_allow_set_link_before_realize, 0);
 }
 
-- 
2.39.2