[PATCH 02/10] hw/pci-host/designware: Create viewport memory regions on demand

[Bernhard Beschow <shentey@gmail.com>]

Currently, all viewport memory regions are created upfront in the realize phase.
This has several drawbacks: First, two MemoryRegion members are needed per
viewport while maximum only one is ever used at a time. Second, for inbound
viewports, the `cfg` member is never used nor initialized. Third, adding
support for other types of memory such as I/O space would require adding even
more MemoryRegion members, one for each type. Fix these limiations by having
just one MemoryRegion member which gets configured on demand.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/pci-host/designware.h |   3 +-
 hw/pci-host/designware.c         | 181 ++++++++++++++-----------------
 2 files changed, 81 insertions(+), 103 deletions(-)

diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index a35a3bd06c..7dc8af049d 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -40,8 +40,7 @@ struct DesignwarePCIERootBus {
 
 typedef struct DesignwarePCIEViewport {
     DesignwarePCIERoot *root;
-
-    MemoryRegion cfg;
+    const char *name;
     MemoryRegion mem;
 
     uint64_t base;
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 3a10dc9f99..7d47d8228f 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -272,33 +272,54 @@ static const MemoryRegionOps designware_pci_host_conf_ops = {
 static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
                                             DesignwarePCIEViewport *viewport)
 {
+    DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
     const uint64_t target = viewport->target;
     const uint64_t base   = viewport->base;
     const uint64_t size   = (uint64_t)viewport->limit - base + 1;
     const bool enabled    = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
 
-    MemoryRegion *current, *other;
-
-    if (viewport->cr[0] == DESIGNWARE_PCIE_ATU_TYPE_MEM) {
-        current = &viewport->mem;
-        other   = &viewport->cfg;
-        memory_region_set_alias_offset(current, target);
-    } else {
-        current = &viewport->cfg;
-        other   = &viewport->mem;
+    if (memory_region_is_mapped(&viewport->mem)) {
+        memory_region_del_subregion(viewport->mem.container, &viewport->mem);
     }
+    object_unparent(OBJECT(&viewport->mem));
 
-    /*
-     * An outbound viewport can be reconfigure from being MEM to CFG,
-     * to account for that we disable the "other" memory region that
-     * becomes unused due to that fact.
-     */
-    memory_region_set_enabled(other, false);
     if (enabled) {
-        memory_region_set_size(current, size);
-        memory_region_set_address(current, base);
+        if (viewport->cr[0] == DESIGNWARE_PCIE_ATU_TYPE_MEM) {
+            if (viewport->inbound) {
+                /*
+                 * Configure MemoryRegion implementing PCI -> CPU memory
+                 * access
+                 */
+                memory_region_init_alias(&viewport->mem, OBJECT(root),
+                                         viewport->name, get_system_memory(),
+                                         target, size);
+                memory_region_add_subregion_overlap(&host->pci.address_space_root,
+                                                    base, &viewport->mem, -1);
+            } else {
+                /*
+                 * Configure MemoryRegion implementing CPU -> PCI memory
+                 * access
+                 */
+                memory_region_init_alias(&viewport->mem, OBJECT(root),
+                                         viewport->name, &host->pci.memory,
+                                         target, size);
+                memory_region_add_subregion(get_system_memory(), base,
+                                            &viewport->mem);
+            }
+        } else {
+            if (!viewport->inbound) {
+                /*
+                 * Configure MemoryRegion implementing access to configuration
+                 * space
+                 */
+                memory_region_init_io(&viewport->mem, OBJECT(root),
+                                      &designware_pci_host_conf_ops,
+                                      viewport, viewport->name, size);
+                memory_region_add_subregion(get_system_memory(), base,
+                                            &viewport->mem);
+            }
+        }
     }
-    memory_region_set_enabled(current, enabled);
 }
 
 static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
@@ -378,27 +399,16 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
     }
 }
 
-static char *designware_pcie_viewport_name(const char *direction,
-                                           unsigned int i,
-                                           const char *type)
-{
-    return g_strdup_printf("PCI %s Viewport %u [%s]",
-                           direction, i, type);
-}
-
 static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
 {
     DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(dev);
     DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
     PCIBridge *br = PCI_BRIDGE(dev);
-    DesignwarePCIEViewport *viewport;
     /*
      * Dummy values used for initial configuration of MemoryRegions
      * that belong to a given viewport
      */
     const hwaddr dummy_offset = 0;
-    const uint64_t dummy_size = 4;
-    size_t i;
 
     br->bus_name  = "dw-pcie";
 
@@ -416,80 +426,49 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
     msi_nonbroken = true;
     msi_init(dev, 0x50, 32, true, true, &error_fatal);
 
-    for (i = 0; i < DESIGNWARE_PCIE_NUM_VIEWPORTS; i++) {
-        const char *direction;
-        char *name;
-
-        viewport = &root->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][i];
-        viewport->inbound = true;
-        viewport->base    = 0x0000000000000000ULL;
-        viewport->target  = 0x0000000000000000ULL;
-        viewport->limit   = UINT32_MAX;
-        viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
-
-        direction   = "Inbound";
-
-        /*
-         * Configure MemoryRegion implementing PCI -> CPU memory
-         * access
-         */
-        name = designware_pcie_viewport_name(direction, i, "MEM");
-        memory_region_init_alias(&viewport->mem, OBJECT(root), name,
-                                 get_system_memory(), dummy_offset, dummy_size);
-        memory_region_add_subregion_overlap(&host->pci.address_space_root,
-                                            dummy_offset, &viewport->mem, -1);
-        memory_region_set_enabled(&viewport->mem, false);
-        g_free(name);
-
-        viewport = &root->viewports[DESIGNWARE_PCIE_VIEWPORT_OUTBOUND][i];
-        viewport->root    = root;
-        viewport->inbound = false;
-        viewport->base    = 0x0000000000000000ULL;
-        viewport->target  = 0x0000000000000000ULL;
-        viewport->limit   = UINT32_MAX;
-        viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
-
-        direction   = "Outbound";
-
-        /*
-         * Configure MemoryRegion implementing CPU -> PCI memory
-         * access
-         */
-        name = designware_pcie_viewport_name(direction, i, "MEM");
-        memory_region_init_alias(&viewport->mem, OBJECT(root), name,
-                                 &host->pci.memory, dummy_offset, dummy_size);
-        memory_region_add_subregion(get_system_memory(), dummy_offset,
-                                    &viewport->mem);
-        memory_region_set_enabled(&viewport->mem, false);
-        g_free(name);
-
-        /*
-         * Configure MemoryRegion implementing access to configuration
-         * space
-         */
-        name = designware_pcie_viewport_name(direction, i, "CFG");
-        memory_region_init_io(&viewport->cfg, OBJECT(root),
-                              &designware_pci_host_conf_ops,
-                              viewport, name, dummy_size);
-        memory_region_add_subregion(get_system_memory(), dummy_offset,
-                                    &viewport->cfg);
-        memory_region_set_enabled(&viewport->cfg, false);
-        g_free(name);
+    for (size_t i = 0; i < ARRAY_SIZE(root->viewports); i++) {
+        static const char *names[][DESIGNWARE_PCIE_NUM_VIEWPORTS] = {
+            {
+                "PCI Outbound Viewport 0",
+                "PCI Outbound Viewport 1",
+                "PCI Outbound Viewport 2",
+                "PCI Outbound Viewport 3"
+            },
+            {
+                "PCI Inbound Viewport 0",
+                "PCI Inbound Viewport 1",
+                "PCI Inbound Viewport 2",
+                "PCI Inbound Viewport 3"
+            },
+        };
+
+        for (size_t j = 0; j < DESIGNWARE_PCIE_NUM_VIEWPORTS; j++) {
+            DesignwarePCIEViewport *viewport = &root->viewports[i][j];
+            viewport->root    = root;
+            viewport->name    = names[i][j];
+            viewport->inbound = i == DESIGNWARE_PCIE_VIEWPORT_INBOUND;
+            viewport->base    = 0x0000000000000000ULL;
+            viewport->target  = 0x0000000000000000ULL;
+            viewport->limit   = UINT32_MAX;
+            viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
+
+            /*
+             * If no inbound iATU windows are configured, HW defaults to
+             * letting inbound TLPs to pass in. We emulate that by explicitly
+             * configuring first inbound window to cover all of target's
+             * address space.
+             *
+             * NOTE: This will not work correctly for the case when first
+             * configured inbound window is window 0
+             */
+            viewport->cr[1]   = (viewport->inbound && j == 0)
+                                  ? DESIGNWARE_PCIE_ATU_ENABLE
+                                  : 0;
+
+            designware_pcie_update_viewport(root, viewport);
+        }
     }
 
-    /*
-     * If no inbound iATU windows are configured, HW defaults to
-     * letting inbound TLPs to pass in. We emulate that by explicitly
-     * configuring first inbound window to cover all of target's
-     * address space.
-     *
-     * NOTE: This will not work correctly for the case when first
-     * configured inbound window is window 0
-     */
-    viewport = &root->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][0];
-    viewport->cr[1] = DESIGNWARE_PCIE_ATU_ENABLE;
-    designware_pcie_update_viewport(root, viewport);
-
     memory_region_init_io(&root->msi.iomem, OBJECT(root),
                           &designware_pci_host_msi_ops,
                           root, "pcie-msi", 0x4);
-- 
2.50.1