[PATCH 00/10] Designware PCIe host fixes

[PATCH 00/10] Designware PCIe host fixes

[Bernhard Beschow]

This series fixes the Designware PCIe host to work with cards other than
virio-net-pci, e.g. e1000. It was tested on the imx8mp-evk machine.

The series is structured as follows: The first part refactors the device
model to create memory regions for inbound/outbound PCI mappings on demand
rather than upfront since this approach doesn't scale for adding I/O space
support. The second part consists of fixing the memory mapping by adding I/O
space support and fixing default inbound viewport mapping. The third part
concludes the series by implementing device reset and cleaning up the imx8mp SoC
implementation.

Testing done:
* Boot imx8mp-evk machine with Buildroot while having an e1000 card attached.
Observe that it gets an IP address via DHCP and allows for downloading an HTML
file via HTTP.

Bernhard Beschow (10):
  hw/pci-host/designware: Eliminate some helper variables
  hw/pci-host/designware: Create viewport memory regions on demand
  hw/pci-host/designware: Determine PCIDevice of configuration region
    once
  hw/pci-host/designware: Distinguish stronger between viewport memory
    types
  hw/pci-host/designware: Implement I/O space
  hw/pci-host/designware: Fix I/O range
  hw/pci-host/designware: Don't map PCI memory space into PCI inbound
    window
  hw/pci-host/designware: Fix default inbound viewport mapping
  hw/pci-host/designware: Implement device reset
  hw/arm/fsl-imx8mp: Do not map PCI window as unimplemented

 include/hw/pci-host/designware.h |   5 +-
 hw/arm/fsl-imx8mp.c              |   1 +
 hw/pci-host/designware.c         | 284 ++++++++++++++++---------------
 3 files changed, 151 insertions(+), 139 deletions(-)

-- 
2.50.1

[PATCH 01/10] hw/pci-host/designware: Eliminate some helper variables

[Bernhard Beschow]

In the next step, code gets moved around where the helper variables aren't used
any longer. Eliminate them now to make the code movement clearer.

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

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index f6e49ce9b8..3a10dc9f99 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -390,8 +390,6 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
 {
     DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(dev);
     DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
-    MemoryRegion *host_mem = get_system_memory();
-    MemoryRegion *address_space = &host->pci.memory;
     PCIBridge *br = PCI_BRIDGE(dev);
     DesignwarePCIEViewport *viewport;
     /*
@@ -419,7 +417,6 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
     msi_init(dev, 0x50, 32, true, true, &error_fatal);
 
     for (i = 0; i < DESIGNWARE_PCIE_NUM_VIEWPORTS; i++) {
-        MemoryRegion *source, *destination, *mem;
         const char *direction;
         char *name;
 
@@ -430,20 +427,18 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
         viewport->limit   = UINT32_MAX;
         viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
 
-        source      = &host->pci.address_space_root;
-        destination = host_mem;
         direction   = "Inbound";
 
         /*
          * Configure MemoryRegion implementing PCI -> CPU memory
          * access
          */
-        mem  = &viewport->mem;
         name = designware_pcie_viewport_name(direction, i, "MEM");
-        memory_region_init_alias(mem, OBJECT(root), name, destination,
-                                 dummy_offset, dummy_size);
-        memory_region_add_subregion_overlap(source, dummy_offset, mem, -1);
-        memory_region_set_enabled(mem, false);
+        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];
@@ -454,33 +449,31 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
         viewport->limit   = UINT32_MAX;
         viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
 
-        destination = &host->pci.memory;
         direction   = "Outbound";
-        source      = host_mem;
 
         /*
          * Configure MemoryRegion implementing CPU -> PCI memory
          * access
          */
-        mem  = &viewport->mem;
         name = designware_pcie_viewport_name(direction, i, "MEM");
-        memory_region_init_alias(mem, OBJECT(root), name, destination,
-                                 dummy_offset, dummy_size);
-        memory_region_add_subregion(source, dummy_offset, mem);
-        memory_region_set_enabled(mem, false);
+        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
          */
-        mem  = &viewport->cfg;
         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(source, dummy_offset, mem);
-        memory_region_set_enabled(mem, false);
+        memory_region_add_subregion(get_system_memory(), dummy_offset,
+                                    &viewport->cfg);
+        memory_region_set_enabled(&viewport->cfg, false);
         g_free(name);
     }
 
@@ -506,7 +499,8 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
      * in designware_pcie_root_update_msi_mapping() as a part of
      * initialization done by guest OS
      */
-    memory_region_add_subregion(address_space, dummy_offset, &root->msi.iomem);
+    memory_region_add_subregion(&host->pci.memory, dummy_offset,
+                                &root->msi.iomem);
     memory_region_set_enabled(&root->msi.iomem, false);
 }
 
-- 
2.50.1

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

[Bernhard Beschow]

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

[PATCH 03/10] hw/pci-host/designware: Determine PCIDevice of configuration region once

[Bernhard Beschow]

Now that viewport memory regions are created on demand, the PCIDevice of the
configuration memory can be determined once upon creation of the memory region
rather than dynamically resolving it upon each access. This is both more
efficient at runtime and resolves an attribute. Furthermore, if an invalid PCI
device is configured, a guest error is now logged since the error is now
direclty related to a guest action.

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

diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index 7dc8af049d..34beee1285 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -39,7 +39,6 @@ struct DesignwarePCIERootBus {
 };
 
 typedef struct DesignwarePCIEViewport {
-    DesignwarePCIERoot *root;
     const char *name;
     MemoryRegion mem;
 
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 7d47d8228f..2a676c65a2 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -222,26 +222,18 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
 static uint64_t designware_pcie_root_data_access(void *opaque, hwaddr addr,
                                                  uint64_t *val, unsigned len)
 {
-    DesignwarePCIEViewport *viewport = opaque;
-    DesignwarePCIERoot *root = viewport->root;
-
-    const uint8_t busnum = DESIGNWARE_PCIE_ATU_BUS(viewport->target);
-    const uint8_t devfn  = DESIGNWARE_PCIE_ATU_DEVFN(viewport->target);
-    PCIBus    *pcibus    = pci_get_bus(PCI_DEVICE(root));
-    PCIDevice *pcidev    = pci_find_device(pcibus, busnum, devfn);
-
-    if (pcidev) {
-        addr &= pci_config_size(pcidev) - 1;
-
-        if (val) {
-            pci_host_config_write_common(pcidev, addr,
-                                         pci_config_size(pcidev),
-                                         *val, len);
-        } else {
-            return pci_host_config_read_common(pcidev, addr,
-                                               pci_config_size(pcidev),
-                                               len);
-        }
+    PCIDevice *pcidev = opaque;
+
+    addr &= pci_config_size(pcidev) - 1;
+
+    if (val) {
+        pci_host_config_write_common(pcidev, addr,
+                                     pci_config_size(pcidev),
+                                     *val, len);
+    } else {
+        return pci_host_config_read_common(pcidev, addr,
+                                           pci_config_size(pcidev),
+                                           len);
     }
 
     return UINT64_MAX;
@@ -312,11 +304,22 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
                  * 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);
+                const uint8_t busnum = DESIGNWARE_PCIE_ATU_BUS(viewport->target);
+                const uint8_t devfn = DESIGNWARE_PCIE_ATU_DEVFN(viewport->target);
+                PCIBus *pcibus = pci_get_bus(PCI_DEVICE(root));
+                PCIDevice *pcidev = pci_find_device(pcibus, busnum, devfn);
+
+                if (pcidev) {
+                    memory_region_init_io(&viewport->mem, OBJECT(root),
+                                          &designware_pci_host_conf_ops,
+                                          pcidev, viewport->name, size);
+                    memory_region_add_subregion(get_system_memory(), base,
+                                                &viewport->mem);
+                } else {
+                    qemu_log_mask(LOG_GUEST_ERROR, "%s: No PCI device attached"
+                                  " under busnum: %d, devfn: %d\n", __func__,
+                                  (int)busnum, (int)devfn);
+                }
             }
         }
     }
@@ -444,7 +447,6 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
 
         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;
-- 
2.50.1

[PATCH 04/10] hw/pci-host/designware: Distinguish stronger between viewport memory types

[Bernhard Beschow]

This is a preparation for implementing I/O space support. At the same
time, unimplemented memory types and guest errors are logged.

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

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 2a676c65a2..5ad7574924 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -44,7 +44,11 @@
 #define DESIGNWARE_PCIE_ATU_VIEWPORT               0x900
 #define DESIGNWARE_PCIE_ATU_REGION_INBOUND         BIT(31)
 #define DESIGNWARE_PCIE_ATU_CR1                    0x904
-#define DESIGNWARE_PCIE_ATU_TYPE_MEM               (0x0 << 0)
+#define DESIGNWARE_PCIE_ATU_TYPE_MEM               0x0
+#define DESIGNWARE_PCIE_ATU_TYPE_IO                0x2
+#define DESIGNWARE_PCIE_ATU_TYPE_CFG0              0x4
+#define DESIGNWARE_PCIE_ATU_TYPE_CFG1              0x5
+#define DESIGNWARE_PCIE_ATU_TYPE_MSG               0x10
 #define DESIGNWARE_PCIE_ATU_CR2                    0x908
 #define DESIGNWARE_PCIE_ATU_ENABLE                 BIT(31)
 #define DESIGNWARE_PCIE_ATU_LOWER_BASE             0x90C
@@ -268,6 +272,7 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
     const uint64_t target = viewport->target;
     const uint64_t base   = viewport->base;
     const uint64_t size   = (uint64_t)viewport->limit - base + 1;
+    const int iatu_type   = viewport->cr[0];
     const bool enabled    = viewport->cr[1] & DESIGNWARE_PCIE_ATU_ENABLE;
 
     if (memory_region_is_mapped(&viewport->mem)) {
@@ -276,7 +281,8 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
     object_unparent(OBJECT(&viewport->mem));
 
     if (enabled) {
-        if (viewport->cr[0] == DESIGNWARE_PCIE_ATU_TYPE_MEM) {
+        switch (iatu_type) {
+        case DESIGNWARE_PCIE_ATU_TYPE_MEM:
             if (viewport->inbound) {
                 /*
                  * Configure MemoryRegion implementing PCI -> CPU memory
@@ -298,7 +304,10 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
                 memory_region_add_subregion(get_system_memory(), base,
                                             &viewport->mem);
             }
-        } else {
+            break;
+
+        case DESIGNWARE_PCIE_ATU_TYPE_CFG0:
+        case DESIGNWARE_PCIE_ATU_TYPE_CFG1:
             if (!viewport->inbound) {
                 /*
                  * Configure MemoryRegion implementing access to configuration
@@ -321,6 +330,18 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
                                   (int)busnum, (int)devfn);
                 }
             }
+            break;
+
+        case DESIGNWARE_PCIE_ATU_TYPE_IO:
+        case DESIGNWARE_PCIE_ATU_TYPE_MSG:
+            qemu_log_mask(LOG_UNIMP, "%s: Unimplemented iATU type %d", __func__,
+                          iatu_type);
+            break;
+
+        default:
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Illegal iATU type %d", __func__,
+                          iatu_type);
+            break;
         }
     }
 }
-- 
2.50.1

[PATCH 05/10] hw/pci-host/designware: Implement I/O space

[Bernhard Beschow]

Mapping I/O space works via viewports in this device but isn't
implemented in the model. Fix that.

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

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 5ad7574924..7342207eb3 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -283,6 +283,7 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
     if (enabled) {
         switch (iatu_type) {
         case DESIGNWARE_PCIE_ATU_TYPE_MEM:
+        case DESIGNWARE_PCIE_ATU_TYPE_IO:
             if (viewport->inbound) {
                 /*
                  * Configure MemoryRegion implementing PCI -> CPU memory
@@ -298,9 +299,12 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
                  * Configure MemoryRegion implementing CPU -> PCI memory
                  * access
                  */
+                MemoryRegion *mr = iatu_type == DESIGNWARE_PCIE_ATU_TYPE_IO
+                                       ? &host->pci.io
+                                       : &host->pci.memory;
+
                 memory_region_init_alias(&viewport->mem, OBJECT(root),
-                                         viewport->name, &host->pci.memory,
-                                         target, size);
+                                         viewport->name, mr, target, size);
                 memory_region_add_subregion(get_system_memory(), base,
                                             &viewport->mem);
             }
@@ -332,7 +336,6 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
             }
             break;
 
-        case DESIGNWARE_PCIE_ATU_TYPE_IO:
         case DESIGNWARE_PCIE_ATU_TYPE_MSG:
             qemu_log_mask(LOG_UNIMP, "%s: Unimplemented iATU type %d", __func__,
                           iatu_type);
-- 
2.50.1

[PATCH 06/10] hw/pci-host/designware: Fix I/O range

[Bernhard Beschow]

Fix the size of the I/O space to be 64KiB, as defined by the PCI
specification. This fixes illegal memory access by guests in the
imx8mp-evk machine such that the FSL_IMX8MP_PCIE1_MEM unimplemented
region can be omitted there.

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

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 7342207eb3..1e29b7e6be 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -684,7 +684,7 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
                           "pcie.reg", 4 * 1024);
     sysbus_init_mmio(sbd, &s->mmio);
 
-    memory_region_init(&s->pci.io, OBJECT(s), "pcie-pio", 16);
+    memory_region_init(&s->pci.io, OBJECT(s), "pcie-pio", UINT16_MAX);
     memory_region_init(&s->pci.memory, OBJECT(s),
                        "pcie-bus-memory",
                        UINT64_MAX);
-- 
2.50.1

[PATCH 07/10] hw/pci-host/designware: Don't map PCI memory space into PCI inbound window

[Bernhard Beschow]

The real device has a default mapping which disappears as soon as an inbound
iATU is configured. Furthermore, inbound and outbound mappings are entirely
defined by iATUs. Remove the hardcoded mapping of PCI memory space to match
real hardware.

Note that the device model attempts to implement the default inbound mapping
by reusing an inbound iATU. However, Linux clears all iATUs during boot,
including the one purposefully set up by the device model, resulting in no
default mapping. This will be fixed in the next patch.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/designware.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 1e29b7e6be..2fd60a4817 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -703,8 +703,6 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
                        OBJECT(s),
                        "pcie-bus-address-space-root",
                        UINT64_MAX);
-    memory_region_add_subregion(&s->pci.address_space_root,
-                                0x0, &s->pci.memory);
     address_space_init(&s->pci.address_space,
                        &s->pci.address_space_root,
                        "pcie-bus-address-space");
-- 
2.50.1

[PATCH 08/10] hw/pci-host/designware: Fix default inbound viewport mapping

[Bernhard Beschow]

Linux clears all inbound viewport mappings which results in the default mapping
set up during realize() to be cleared. Fix that by introducing a fallback
memory region which gets enabled if no inbound viewports are configured, as the
real HW would do.

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

diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index 34beee1285..342b09fd08 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -86,6 +86,7 @@ struct DesignwarePCIEHost {
     struct {
         AddressSpace address_space;
         MemoryRegion address_space_root;
+        MemoryRegion address_space_root_default;
 
         MemoryRegion memory;
         MemoryRegion io;
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 2fd60a4817..d71133a456 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -347,6 +347,17 @@ static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
             break;
         }
     }
+
+    bool one_mapped = false;
+    for (int j = 0; j < DESIGNWARE_PCIE_NUM_VIEWPORTS; j++) {
+        one_mapped |= memory_region_is_mapped(&root->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][j].mem);
+    }
+
+    /*
+     * If no inbound iATU windows are configured, HW defaults to
+     * letting inbound TLPs to pass in.
+     */
+    memory_region_set_enabled(&host->pci.address_space_root_default, !one_mapped);
 }
 
 static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
@@ -477,19 +488,7 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
             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;
+            viewport->cr[1]   = 0;
 
             designware_pcie_update_viewport(root, viewport);
         }
@@ -706,6 +705,11 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
     address_space_init(&s->pci.address_space,
                        &s->pci.address_space_root,
                        "pcie-bus-address-space");
+    memory_region_init_alias(&s->pci.address_space_root_default, OBJECT(s),
+                             "pcie-bus-inbound-default", get_system_memory(),
+                             0, UINT32_MAX);
+    memory_region_add_subregion_overlap(&s->pci.address_space_root, 0,
+                                        &s->pci.address_space_root_default, -10);
     pci_setup_iommu(pci->bus, &designware_iommu_ops, s);
 
     qdev_realize(DEVICE(&s->root), BUS(pci->bus), &error_fatal);
-- 
2.50.1

[PATCH 09/10] hw/pci-host/designware: Implement device reset

[Bernhard Beschow]

Fixes the memory mapping to be cleared during reset, like real hardware would
do.

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

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index d71133a456..2dd4937e52 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -442,11 +442,6 @@ 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);
-    /*
-     * Dummy values used for initial configuration of MemoryRegions
-     * that belong to a given viewport
-     */
-    const hwaddr dummy_offset = 0;
 
     br->bus_name  = "dw-pcie";
 
@@ -484,6 +479,26 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
             DesignwarePCIEViewport *viewport = &root->viewports[i][j];
             viewport->name    = names[i][j];
             viewport->inbound = i == DESIGNWARE_PCIE_VIEWPORT_INBOUND;
+        }
+    }
+
+    memory_region_init_io(&root->msi.iomem, OBJECT(root),
+                          &designware_pci_host_msi_ops,
+                          root, "pcie-msi", 0x4);
+    memory_region_add_subregion(&host->pci.memory, 0, &root->msi.iomem);
+}
+
+static void designware_pcie_root_reset(DeviceState *dev)
+{
+    DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(dev);
+    DesignwarePCIEViewport *viewport;
+
+    pci_bridge_reset(dev);
+
+    for (int i = 0; i < ARRAY_SIZE(root->viewports); i++) {
+        for (int j = 0; j < DESIGNWARE_PCIE_NUM_VIEWPORTS; j++) {
+            viewport = &root->viewports[i][j];
+
             viewport->base    = 0x0000000000000000ULL;
             viewport->target  = 0x0000000000000000ULL;
             viewport->limit   = UINT32_MAX;
@@ -494,17 +509,13 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
         }
     }
 
-    memory_region_init_io(&root->msi.iomem, OBJECT(root),
-                          &designware_pci_host_msi_ops,
-                          root, "pcie-msi", 0x4);
     /*
      * We initially place MSI interrupt I/O region at address 0 and
      * disable it. It'll be later moved to correct offset and enabled
      * in designware_pcie_root_update_msi_mapping() as a part of
      * initialization done by guest OS
      */
-    memory_region_add_subregion(&host->pci.memory, dummy_offset,
-                                &root->msi.iomem);
+    memory_region_set_address(&root->msi.iomem, 0);
     memory_region_set_enabled(&root->msi.iomem, false);
 }
 
@@ -602,7 +613,7 @@ static void designware_pcie_root_class_init(ObjectClass *klass,
     k->config_read = designware_pcie_root_config_read;
     k->config_write = designware_pcie_root_config_write;
 
-    device_class_set_legacy_reset(dc, pci_bridge_reset);
+    device_class_set_legacy_reset(dc, designware_pcie_root_reset);
     /*
      * PCI-facing part of the host bridge, not usable without the
      * host-facing part, which can't be device_add'ed, yet.
-- 
2.50.1

[PATCH 10/10] hw/arm/fsl-imx8mp: Do not map PCI window as unimplemented

[Bernhard Beschow]

Now that populating the PCI window via iATUs is functional, it's no longer
necessary to cover the PCI window with an unimplemented memory region.
Previously, this workaround was required because the device model failed to map
all configured memory regions, which caused Linux to emit backtraces during
access attempts.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/arm/fsl-imx8mp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
index 866f4d1d74..d66783c101 100644
--- a/hw/arm/fsl-imx8mp.c
+++ b/hw/arm/fsl-imx8mp.c
@@ -669,6 +669,7 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error **errp)
         case FSL_IMX8MP_I2C1 ... FSL_IMX8MP_I2C6:
         case FSL_IMX8MP_OCRAM:
         case FSL_IMX8MP_PCIE1:
+        case FSL_IMX8MP_PCIE1_MEM:
         case FSL_IMX8MP_PCIE_PHY1:
         case FSL_IMX8MP_RAM:
         case FSL_IMX8MP_SNVS_HP:
-- 
2.50.1

Re: [PATCH 00/10] Designware PCIe host fixes

[Guenter Roeck]

On 8/20/25 14:19, Bernhard Beschow wrote:
> This series fixes the Designware PCIe host to work with cards other than
> virio-net-pci, e.g. e1000. It was tested on the imx8mp-evk machine.
> 
> The series is structured as follows: The first part refactors the device
> model to create memory regions for inbound/outbound PCI mappings on demand
> rather than upfront since this approach doesn't scale for adding I/O space
> support. The second part consists of fixing the memory mapping by adding I/O
> space support and fixing default inbound viewport mapping. The third part
> concludes the series by implementing device reset and cleaning up the imx8mp SoC
> implementation.
> 
> Testing done:
> * Boot imx8mp-evk machine with Buildroot while having an e1000 card attached.
> Observe that it gets an IP address via DHCP and allows for downloading an HTML
> file via HTTP.
> 
Crashing for me even if no PCIe card is attached. This is with the series applied
on top of 10.1.0-rc4 or 10.0.3. I have not tried to track down the problem.

Guenter

---
Build reference: v6.17-rc1-287-g685de850cabf
Compiler version: aarch64-linux-gcc (GCC) 13.4.0
Qemu version: 10.0.94 (v10.1.0-rc4-55-g320ed12bd9)

[    7.748393] Internal error: synchronous external abort: 0000000096000010 [#1]  SMP
[    7.748917] Modules linked in:
[    7.749533] CPU: 3 UID: 0 PID: 12 Comm: kworker/u16:0 Tainted: G   M             N  6.17.0-rc2-g685de850cabf #1 PREEMPT
[    7.749689] Tainted: [M]=MACHINE_CHECK, [N]=TEST
[    7.749745] Hardware name: NXP i.MX8MPlus EVK board (DT)
[    7.750118]         ok 1 block_bits=10 cluster_bits=3 blocks_per_group=8192 group_count=4 desc_size=64
[    7.749922] Workqueue: async async_run_entry_fn
[    7.751174] pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    7.751267] pc : pci_generic_config_read+0x38/0xb8
[    7.751370] lr : pci_generic_config_read+0x24/0xb8
[    7.751474] sp : ffff8000845fb730
[    7.751532] x29: ffff8000845fb730 x28: 00000000000000ff x27: 0000000000000000
[    7.751686] x26: 0000000000000001 x25: ffff80008290c008 x24: ffff8000832cbe80
[    7.751770] x23: 0000000000000000 x22: ffff8000845fb844 x21: ffff000008a73800
[    7.751859] x20: ffff8000845fb7a4 x19: 0000000000000004 x18: 00000000ffffffff
[    7.751942] x17: ffff800080e2f918 x16: ffff800080e2f7ac x15: ffff800080e2e6d8
[    7.752027] x14: 0000000000000000 x13: 0000000000000000 x12: ffff80008427cab8
[    7.752120] x11: 0000000000000326 x10: 0000000000000326 x9 : 00000000000c0326
[    7.752225] x8 : 000000006973b6c5 x7 : ffff800085c00000 x6 : 0000000000000000
[    7.752320] x5 : ffff000007cc5080 x4 : 0000000000000000 x3 : 0000000000000000
[    7.752403] x2 : 0000000000000000 x1 : ffff000004bf5680 x0 : ffff800085900000
[    7.752577] Call trace:
[    7.752694]  pci_generic_config_read+0x38/0xb8 (P)
[    7.752794]  dw_pcie_rd_other_conf+0x38/0xb0
[    7.752851]  pci_bus_read_config_dword+0x80/0xe4
[    7.752903]  pci_bus_generic_read_dev_vendor_id+0x30/0x190
[    7.752964]  pci_scan_device+0xdc/0x184
[    7.753010]  pci_scan_slot+0xe0/0x23c
[    7.753056]  pci_scan_child_bus_extend+0x44/0x2cc
[    7.753110]  pci_scan_bridge_extend+0x514/0x598
[    7.753166]  pci_scan_child_bus_extend+0x104/0x2cc
[    7.753220]  pci_scan_root_bus_bridge+0x64/0xd8
[    7.753273]  pci_host_probe+0x34/0x10c
[    7.753328]  dw_pcie_host_init+0x2f0/0x4b0
[    7.753376]  imx_pcie_probe+0x33c/0x70c
[    7.753439]  platform_probe+0x5c/0x9c
[    7.753485]  really_probe+0xc0/0x390
[    7.753530]  __driver_probe_device+0x7c/0x15c
[    7.753581]  driver_probe_device+0x3c/0x110
[    7.753633]  __device_attach_driver+0xbc/0x158

Re: [PATCH 00/10] Designware PCIe host fixes

[Bernhard Beschow]

Am 21. August 2025 03:36:44 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 8/20/25 14:19, Bernhard Beschow wrote:
>> This series fixes the Designware PCIe host to work with cards other than
>> virio-net-pci, e.g. e1000. It was tested on the imx8mp-evk machine.
>> 
>> The series is structured as follows: The first part refactors the device
>> model to create memory regions for inbound/outbound PCI mappings on demand
>> rather than upfront since this approach doesn't scale for adding I/O space
>> support. The second part consists of fixing the memory mapping by adding I/O
>> space support and fixing default inbound viewport mapping. The third part
>> concludes the series by implementing device reset and cleaning up the imx8mp SoC
>> implementation.
>> 
>> Testing done:
>> * Boot imx8mp-evk machine with Buildroot while having an e1000 card attached.
>> Observe that it gets an IP address via DHCP and allows for downloading an HTML
>> file via HTTP.
>> 
>Crashing for me even if no PCIe card is attached. This is with the series applied
>on top of 10.1.0-rc4 or 10.0.3. I have not tried to track down the problem.
>
>Guenter

Hi Guenther,

Thanks for testing this series! I can reproduce the issue with Buildroot while the functional test passes...

I guess that I was too optimistic in having resolved the issue mentioned in the last patch. Does it work for you if you omit it?

Thanks,
Bernhard

>
>---
>Build reference: v6.17-rc1-287-g685de850cabf
>Compiler version: aarch64-linux-gcc (GCC) 13.4.0
>Qemu version: 10.0.94 (v10.1.0-rc4-55-g320ed12bd9)
>
>[    7.748393] Internal error: synchronous external abort: 0000000096000010 [#1]  SMP
>[    7.748917] Modules linked in:
>[    7.749533] CPU: 3 UID: 0 PID: 12 Comm: kworker/u16:0 Tainted: G   M             N  6.17.0-rc2-g685de850cabf #1 PREEMPT
>[    7.749689] Tainted: [M]=MACHINE_CHECK, [N]=TEST
>[    7.749745] Hardware name: NXP i.MX8MPlus EVK board (DT)
>[    7.750118]         ok 1 block_bits=10 cluster_bits=3 blocks_per_group=8192 group_count=4 desc_size=64
>[    7.749922] Workqueue: async async_run_entry_fn
>[    7.751174] pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>[    7.751267] pc : pci_generic_config_read+0x38/0xb8
>[    7.751370] lr : pci_generic_config_read+0x24/0xb8
>[    7.751474] sp : ffff8000845fb730
>[    7.751532] x29: ffff8000845fb730 x28: 00000000000000ff x27: 0000000000000000
>[    7.751686] x26: 0000000000000001 x25: ffff80008290c008 x24: ffff8000832cbe80
>[    7.751770] x23: 0000000000000000 x22: ffff8000845fb844 x21: ffff000008a73800
>[    7.751859] x20: ffff8000845fb7a4 x19: 0000000000000004 x18: 00000000ffffffff
>[    7.751942] x17: ffff800080e2f918 x16: ffff800080e2f7ac x15: ffff800080e2e6d8
>[    7.752027] x14: 0000000000000000 x13: 0000000000000000 x12: ffff80008427cab8
>[    7.752120] x11: 0000000000000326 x10: 0000000000000326 x9 : 00000000000c0326
>[    7.752225] x8 : 000000006973b6c5 x7 : ffff800085c00000 x6 : 0000000000000000
>[    7.752320] x5 : ffff000007cc5080 x4 : 0000000000000000 x3 : 0000000000000000
>[    7.752403] x2 : 0000000000000000 x1 : ffff000004bf5680 x0 : ffff800085900000
>[    7.752577] Call trace:
>[    7.752694]  pci_generic_config_read+0x38/0xb8 (P)
>[    7.752794]  dw_pcie_rd_other_conf+0x38/0xb0
>[    7.752851]  pci_bus_read_config_dword+0x80/0xe4
>[    7.752903]  pci_bus_generic_read_dev_vendor_id+0x30/0x190
>[    7.752964]  pci_scan_device+0xdc/0x184
>[    7.753010]  pci_scan_slot+0xe0/0x23c
>[    7.753056]  pci_scan_child_bus_extend+0x44/0x2cc
>[    7.753110]  pci_scan_bridge_extend+0x514/0x598
>[    7.753166]  pci_scan_child_bus_extend+0x104/0x2cc
>[    7.753220]  pci_scan_root_bus_bridge+0x64/0xd8
>[    7.753273]  pci_host_probe+0x34/0x10c
>[    7.753328]  dw_pcie_host_init+0x2f0/0x4b0
>[    7.753376]  imx_pcie_probe+0x33c/0x70c
>[    7.753439]  platform_probe+0x5c/0x9c
>[    7.753485]  really_probe+0xc0/0x390
>[    7.753530]  __driver_probe_device+0x7c/0x15c
>[    7.753581]  driver_probe_device+0x3c/0x110
>[    7.753633]  __device_attach_driver+0xbc/0x158
>

Re: [PATCH 00/10] Designware PCIe host fixes

[Guenter Roeck]

On Thu, Aug 21, 2025 at 10:24:02AM +0000, Bernhard Beschow wrote:
> 
> 
> Am 21. August 2025 03:36:44 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
> >On 8/20/25 14:19, Bernhard Beschow wrote:
> >> This series fixes the Designware PCIe host to work with cards other than
> >> virio-net-pci, e.g. e1000. It was tested on the imx8mp-evk machine.
> >> 
> >> The series is structured as follows: The first part refactors the device
> >> model to create memory regions for inbound/outbound PCI mappings on demand
> >> rather than upfront since this approach doesn't scale for adding I/O space
> >> support. The second part consists of fixing the memory mapping by adding I/O
> >> space support and fixing default inbound viewport mapping. The third part
> >> concludes the series by implementing device reset and cleaning up the imx8mp SoC
> >> implementation.
> >> 
> >> Testing done:
> >> * Boot imx8mp-evk machine with Buildroot while having an e1000 card attached.
> >> Observe that it gets an IP address via DHCP and allows for downloading an HTML
> >> file via HTTP.
> >> 
> >Crashing for me even if no PCIe card is attached. This is with the series applied
> >on top of 10.1.0-rc4 or 10.0.3. I have not tried to track down the problem.
> >
> >Guenter
> 
> Hi Guenther,
> 
> Thanks for testing this series! I can reproduce the issue with Buildroot while the functional test passes...
> 
> I guess that I was too optimistic in having resolved the issue mentioned in the last patch. Does it work for you if you omit it?
> 
I'll give it another try tonight.

Guenter

> Thanks,
> Bernhard
> 
> >
> >---
> >Build reference: v6.17-rc1-287-g685de850cabf
> >Compiler version: aarch64-linux-gcc (GCC) 13.4.0
> >Qemu version: 10.0.94 (v10.1.0-rc4-55-g320ed12bd9)
> >
> >[    7.748393] Internal error: synchronous external abort: 0000000096000010 [#1]  SMP
> >[    7.748917] Modules linked in:
> >[    7.749533] CPU: 3 UID: 0 PID: 12 Comm: kworker/u16:0 Tainted: G   M             N  6.17.0-rc2-g685de850cabf #1 PREEMPT
> >[    7.749689] Tainted: [M]=MACHINE_CHECK, [N]=TEST
> >[    7.749745] Hardware name: NXP i.MX8MPlus EVK board (DT)
> >[    7.750118]         ok 1 block_bits=10 cluster_bits=3 blocks_per_group=8192 group_count=4 desc_size=64
> >[    7.749922] Workqueue: async async_run_entry_fn
> >[    7.751174] pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >[    7.751267] pc : pci_generic_config_read+0x38/0xb8
> >[    7.751370] lr : pci_generic_config_read+0x24/0xb8
> >[    7.751474] sp : ffff8000845fb730
> >[    7.751532] x29: ffff8000845fb730 x28: 00000000000000ff x27: 0000000000000000
> >[    7.751686] x26: 0000000000000001 x25: ffff80008290c008 x24: ffff8000832cbe80
> >[    7.751770] x23: 0000000000000000 x22: ffff8000845fb844 x21: ffff000008a73800
> >[    7.751859] x20: ffff8000845fb7a4 x19: 0000000000000004 x18: 00000000ffffffff
> >[    7.751942] x17: ffff800080e2f918 x16: ffff800080e2f7ac x15: ffff800080e2e6d8
> >[    7.752027] x14: 0000000000000000 x13: 0000000000000000 x12: ffff80008427cab8
> >[    7.752120] x11: 0000000000000326 x10: 0000000000000326 x9 : 00000000000c0326
> >[    7.752225] x8 : 000000006973b6c5 x7 : ffff800085c00000 x6 : 0000000000000000
> >[    7.752320] x5 : ffff000007cc5080 x4 : 0000000000000000 x3 : 0000000000000000
> >[    7.752403] x2 : 0000000000000000 x1 : ffff000004bf5680 x0 : ffff800085900000
> >[    7.752577] Call trace:
> >[    7.752694]  pci_generic_config_read+0x38/0xb8 (P)
> >[    7.752794]  dw_pcie_rd_other_conf+0x38/0xb0
> >[    7.752851]  pci_bus_read_config_dword+0x80/0xe4
> >[    7.752903]  pci_bus_generic_read_dev_vendor_id+0x30/0x190
> >[    7.752964]  pci_scan_device+0xdc/0x184
> >[    7.753010]  pci_scan_slot+0xe0/0x23c
> >[    7.753056]  pci_scan_child_bus_extend+0x44/0x2cc
> >[    7.753110]  pci_scan_bridge_extend+0x514/0x598
> >[    7.753166]  pci_scan_child_bus_extend+0x104/0x2cc
> >[    7.753220]  pci_scan_root_bus_bridge+0x64/0xd8
> >[    7.753273]  pci_host_probe+0x34/0x10c
> >[    7.753328]  dw_pcie_host_init+0x2f0/0x4b0
> >[    7.753376]  imx_pcie_probe+0x33c/0x70c
> >[    7.753439]  platform_probe+0x5c/0x9c
> >[    7.753485]  really_probe+0xc0/0x390
> >[    7.753530]  __driver_probe_device+0x7c/0x15c
> >[    7.753581]  driver_probe_device+0x3c/0x110
> >[    7.753633]  __device_attach_driver+0xbc/0x158
> >

Re: [PATCH 00/10] Designware PCIe host fixes

[Guenter Roeck]

On Thu, Aug 21, 2025 at 10:24:02AM +0000, Bernhard Beschow wrote:
> 
> 
> Am 21. August 2025 03:36:44 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
> >On 8/20/25 14:19, Bernhard Beschow wrote:
> >> This series fixes the Designware PCIe host to work with cards other than
> >> virio-net-pci, e.g. e1000. It was tested on the imx8mp-evk machine.
> >> 
> >> The series is structured as follows: The first part refactors the device
> >> model to create memory regions for inbound/outbound PCI mappings on demand
> >> rather than upfront since this approach doesn't scale for adding I/O space
> >> support. The second part consists of fixing the memory mapping by adding I/O
> >> space support and fixing default inbound viewport mapping. The third part
> >> concludes the series by implementing device reset and cleaning up the imx8mp SoC
> >> implementation.
> >> 
> >> Testing done:
> >> * Boot imx8mp-evk machine with Buildroot while having an e1000 card attached.
> >> Observe that it gets an IP address via DHCP and allows for downloading an HTML
> >> file via HTTP.
> >> 
> >Crashing for me even if no PCIe card is attached. This is with the series applied
> >on top of 10.1.0-rc4 or 10.0.3. I have not tried to track down the problem.
> >
> >Guenter
> 
> Hi Guenther,
> 
> Thanks for testing this series! I can reproduce the issue with Buildroot while the functional test passes...
> 
> I guess that I was too optimistic in having resolved the issue mentioned in the last patch. Does it work for you if you omit it?
> 

It gives me hung task crashes when trying to boot from virtio-pci.
I'll need some time for debugging.

Guenter

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

[Peter Maydell]

On Wed, 20 Aug 2025 at 22:19, Bernhard Beschow <shentey@gmail.com> wrote:
>
> 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.

On the other hand, docs/devel/memory.rst says:

> as a general rule do not create or destroy memory regions dynamically
> during a device's lifetime

with a flat ban on doing this for MRs that aren't container or alias MRs,
and even though it makes an exception for the container/alias case it says
"Exploiting this exception is rarely necessary, and therefore it
is discouraged".

(I also find it harder to reason about, because it means I have
to think about MR lifetimes and references and so on, whereas
"create MRs at realize and they exist as long as the device
does" is easy because it's the same way every other device works.)

thanks
-- PMM

Re: [PATCH 06/10] hw/pci-host/designware: Fix I/O range

[Peter Maydell]

On Wed, 20 Aug 2025 at 22:19, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Fix the size of the I/O space to be 64KiB, as defined by the PCI
> specification. This fixes illegal memory access by guests in the
> imx8mp-evk machine such that the FSL_IMX8MP_PCIE1_MEM unimplemented
> region can be omitted there.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/pci-host/designware.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 7342207eb3..1e29b7e6be 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -684,7 +684,7 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
>                            "pcie.reg", 4 * 1024);
>      sysbus_init_mmio(sbd, &s->mmio);
>
> -    memory_region_init(&s->pci.io, OBJECT(s), "pcie-pio", 16);
> +    memory_region_init(&s->pci.io, OBJECT(s), "pcie-pio", UINT16_MAX);

This isn't 64K, it's 1 byte less than 64K. You want
64 * KiB   or something similar.

(The memory APIs have a special case for "size 2^64" where
you can pass it UINT64_MAX, but any sizes below that you
pass in the actual size: the special case is just so we
can use a uint64_t type for the argument without losing
the ability to specify a size that covers the full 64-bit
address space.)

>      memory_region_init(&s->pci.memory, OBJECT(s),
>                         "pcie-bus-memory",
>                         UINT64_MAX);

(We seem to have a similar bug in xilinx-pcie.c.)

-- PMM