[Qemu-devel] [PATCH 2/4] pc: consolidate mapping of PCI address space into system address space

[Igor Mammedov <imammedo@redhat.com>]

Reduce code duplication by moving PCI address space mappings
into a common helper, and reuse code in PIIX and Q35.
Also rename variables to reflect that PCI memory region mappings
are not the same as PCI holes which are programmed by BIOS to
avoid confusion with PCI holes.

PS:
Although keep user visible via "info mtree" names for regions
"pci-hole" and "pci-hole64" to avoid management tools regressions.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c         |   40 ++++++++++++++++++++++++++++------------
 hw/pci-host/piix.c   |   28 ++++++++--------------------
 hw/pci-host/q35.c    |   28 ++++++++--------------------
 include/hw/i386/pc.h |   15 +++++----------
 4 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0c313fe..7ecb028 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1053,21 +1053,37 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
     return guest_info;
 }
 
-void pc_init_pci64_hole(PcPciInfo *pci_info, uint64_t pci_hole64_start,
-                        uint64_t pci_hole64_size)
+/* setup pci memory regions mappings into system address space */
+void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
+                            MemoryRegion *pci_address_space,
+                            MemoryRegion *pci32_as, MemoryRegion *pci64_as,
+                            uint32_t pci32_as_start, uint32_t pci32_as_size,
+                            uint64_t pci64_as_start, uint64_t pci64_as_size)
 {
-    if ((sizeof(hwaddr) == 4) || (!pci_hole64_size)) {
+    memory_region_init_alias(pci32_as, owner, "pci-hole", pci_address_space,
+                             pci32_as_start, pci32_as_size);
+    memory_region_add_subregion(system_memory, pci32_as_start, pci32_as);
+
+    if (sizeof(hwaddr) == 4) {
         return;
     }
-    /*
-     * BIOS does not set MTRR entries for the 64 bit window, so no need to
-     * align address to power of two.  Align address at 1G, this makes sure
-     * it can be exactly covered with a PAT entry even when using huge
-     * pages.
-     */
-    pci_info->w64.begin = ROUND_UP(pci_hole64_start, 0x1ULL << 30);
-    pci_info->w64.end = pci_info->w64.begin + pci_hole64_size;
-    assert(pci_info->w64.begin <= pci_info->w64.end);
+
+    if (pci64_as_size == DEFAULT_PCI_HOLE64_SIZE) {
+        pci64_as_size = 1ULL << 62;
+    }
+    memory_region_init_alias(pci64_as, owner, "pci-hole64", pci_address_space,
+                             pci64_as_start, pci64_as_size);
+    if (pci64_as_size) {
+        /*
+         * BIOS does not set MTRR entries for the 64 bit window, so no need to
+         * align address to power of two.  Align address at 1G, this makes sure
+         * it can be exactly covered with a PAT entry even when using huge
+         * pages.
+         */
+        memory_region_add_subregion(system_memory,
+                                    ROUND_UP(pci64_as_start, 0x1ULL << 30),
+                                    pci64_as);
+    }
 }
 
 void pc_acpi_init(const char *default_dsdt)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 5b9cc1f..d422b25 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -324,7 +324,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
     PCII440FXState *f;
     unsigned i;
     I440FXState *i440fx;
-    uint64_t pci_hole64_size;
     ram_addr_t ram_size = below_4g_mem_size + above_4g_mem_size;
 
     dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
@@ -353,25 +352,14 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
         i440fx->pci_info.w32.begin = 0xe0000000;
     }
 
-    memory_region_init_alias(&f->pci_hole, OBJECT(d), "pci-hole", f->pci_address_space,
-                             below_4g_mem_size,
-                             0x100000000ULL - below_4g_mem_size);
-    memory_region_add_subregion(f->system_memory, below_4g_mem_size,
-                                &f->pci_hole);
-
-    pci_hole64_size = pci_host_get_hole64_size(i440fx->pci_hole64_size);
-
-    pc_init_pci64_hole(&i440fx->pci_info, 0x100000000ULL + above_4g_mem_size,
-                       pci_hole64_size);
-    memory_region_init_alias(&f->pci_hole_64bit, OBJECT(d), "pci-hole64",
-                             f->pci_address_space,
-                             i440fx->pci_info.w64.begin,
-                             pci_hole64_size);
-    if (pci_hole64_size) {
-        memory_region_add_subregion(f->system_memory,
-                                    i440fx->pci_info.w64.begin,
-                                    &f->pci_hole_64bit);
-    }
+    pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
+                           f->pci_address_space,
+                           &f->pci_hole, &f->pci_hole_64bit,
+                           below_4g_mem_size,
+                           0x100000000ULL - below_4g_mem_size,
+                           0x100000000ULL + above_4g_mem_size,
+                           i440fx->pci_hole64_size);
+
     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,
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ad703a4..cb8aea0 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -336,28 +336,16 @@ static int mch_init(PCIDevice *d)
 {
     int i;
     MCHPCIState *mch = MCH_PCI_DEVICE(d);
-    uint64_t pci_hole64_size;
 
     /* setup pci memory regions */
-    memory_region_init_alias(&mch->pci_hole, OBJECT(mch), "pci-hole",
-                             mch->pci_address_space,
-                             mch->below_4g_mem_size,
-                             0x100000000ULL - mch->below_4g_mem_size);
-    memory_region_add_subregion(mch->system_memory, mch->below_4g_mem_size,
-                                &mch->pci_hole);
-
-    pci_hole64_size = pci_host_get_hole64_size(mch->pci_hole64_size);
-    pc_init_pci64_hole(&mch->pci_info, 0x100000000ULL + mch->above_4g_mem_size,
-                       pci_hole64_size);
-    memory_region_init_alias(&mch->pci_hole_64bit, OBJECT(mch), "pci-hole64",
-                             mch->pci_address_space,
-                             mch->pci_info.w64.begin,
-                             pci_hole64_size);
-    if (pci_hole64_size) {
-        memory_region_add_subregion(mch->system_memory,
-                                    mch->pci_info.w64.begin,
-                                    &mch->pci_hole_64bit);
-    }
+    pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
+                           mch->pci_address_space,
+                           &mch->pci_hole, &mch->pci_hole_64bit,
+                           mch->below_4g_mem_size,
+                           0x100000000ULL - mch->below_4g_mem_size,
+                           0x100000000ULL + mch->above_4g_mem_size,
+                           mch->pci_hole64_size);
+
     /* smram */
     cpu_smm_register(&mch_set_smm, mch);
     memory_region_init_alias(&mch->smram_region, OBJECT(mch), "smram-region",
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 95fc6cc..ed86642 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -108,17 +108,12 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
 #define PCI_HOST_PROP_PCI_HOLE64_SIZE  "pci-hole64-size"
 #define DEFAULT_PCI_HOLE64_SIZE (~0x0ULL)
 
-static inline uint64_t pci_host_get_hole64_size(uint64_t pci_hole64_size)
-{
-    if (pci_hole64_size == DEFAULT_PCI_HOLE64_SIZE) {
-        return 1ULL << 62;
-    } else {
-        return pci_hole64_size;
-    }
-}
 
-void pc_init_pci64_hole(PcPciInfo *pci_info, uint64_t pci_hole64_start,
-                        uint64_t pci_hole64_size);
+void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
+                            MemoryRegion *pci_address_space,
+                            MemoryRegion *pci32_as, MemoryRegion *pci64_as,
+                            uint32_t pci32_as_start, uint32_t pci32_as_size,
+                            uint64_t pci64_as_start, uint64_t pci64_as_size);
 
 FWCfgState *pc_memory_init(MemoryRegion *system_memory,
                            const char *kernel_filename,
-- 
1.7.1