[PATCH v4 15/45] Add BCM2838 PCIE host

Sergey Kambalin posted 45 patches 11 months, 3 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jason Wang <jasowang@redhat.com>, Cleber Rosa <crosa@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v4 15/45] Add BCM2838 PCIE host
Posted by Sergey Kambalin 11 months, 3 weeks ago
Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
---
 hw/arm/bcm2838_pcie.c         | 216 +++++++++++++++++++++++++++++++++-
 include/hw/arm/bcm2838_pcie.h |  22 ++++
 2 files changed, 236 insertions(+), 2 deletions(-)

diff --git a/hw/arm/bcm2838_pcie.c b/hw/arm/bcm2838_pcie.c
index 3b4373c6a6..75146d6c2e 100644
--- a/hw/arm/bcm2838_pcie.c
+++ b/hw/arm/bcm2838_pcie.c
@@ -12,11 +12,222 @@
 #include "hw/irq.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/qdev-properties.h"
-#include "migration/vmstate.h"
-#include "qemu/module.h"
 #include "hw/arm/bcm2838_pcie.h"
 #include "trace.h"
 
+/*
+ * RC host part
+ */
+
+static uint64_t bcm2838_pcie_host_read(void *opaque, hwaddr offset,
+                                       unsigned size) {
+    hwaddr mmcfg_addr;
+    uint64_t value = ~0;
+    BCM2838PcieHostState *s = opaque;
+    PCIExpressHost *pcie_hb = PCIE_HOST_BRIDGE(s);
+    PCIDevice *root_pci_dev = PCI_DEVICE(&s->root_port);
+    uint8_t *root_regs = s->root_port.regs;
+    uint32_t *cfg_idx = (uint32_t *)(root_regs + BCM2838_PCIE_EXT_CFG_INDEX
+                                     - PCIE_CONFIG_SPACE_SIZE);
+
+    if (offset < PCIE_CONFIG_SPACE_SIZE) {
+        value = pci_host_config_read_common(root_pci_dev, offset,
+                                            PCIE_CONFIG_SPACE_SIZE, size);
+    } else if (offset - PCIE_CONFIG_SPACE_SIZE + size
+               <= sizeof(s->root_port.regs)) {
+        switch (offset) {
+        case BCM2838_PCIE_EXT_CFG_DATA
+            ... BCM2838_PCIE_EXT_CFG_DATA + PCIE_CONFIG_SPACE_SIZE - 1:
+            mmcfg_addr = *cfg_idx
+                | PCIE_MMCFG_CONFOFFSET(offset - BCM2838_PCIE_EXT_CFG_DATA);
+            value = pcie_hb->mmio.ops->read(opaque, mmcfg_addr, size);
+            break;
+        default:
+            memcpy(&value, root_regs + offset - PCIE_CONFIG_SPACE_SIZE, size);
+        }
+    } else {
+        qemu_log_mask(
+            LOG_GUEST_ERROR,
+            "%s: out-of-range access, %u bytes @ offset 0x%04" PRIx64 "\n",
+            __func__, size, offset);
+    }
+
+    trace_bcm2838_pcie_host_read(size, offset, value);
+    return value;
+}
+
+static void bcm2838_pcie_host_write(void *opaque, hwaddr offset,
+                                    uint64_t value, unsigned size) {
+    hwaddr mmcfg_addr;
+    BCM2838PcieHostState *s = opaque;
+    PCIExpressHost *pcie_hb = PCIE_HOST_BRIDGE(s);
+    PCIDevice *root_pci_dev = PCI_DEVICE(&s->root_port);
+    uint8_t *root_regs = s->root_port.regs;
+    uint32_t *cfg_idx = (uint32_t *)(root_regs + BCM2838_PCIE_EXT_CFG_INDEX
+                                     - PCIE_CONFIG_SPACE_SIZE);
+
+    trace_bcm2838_pcie_host_write(size, offset, value);
+
+    if (offset < PCIE_CONFIG_SPACE_SIZE) {
+        pci_host_config_write_common(root_pci_dev, offset,
+                                     PCIE_CONFIG_SPACE_SIZE, value, size);
+    } else if (offset - PCIE_CONFIG_SPACE_SIZE + size
+               <= sizeof(s->root_port.regs)) {
+        switch (offset) {
+        case BCM2838_PCIE_EXT_CFG_DATA
+            ... BCM2838_PCIE_EXT_CFG_DATA + PCIE_CONFIG_SPACE_SIZE - 1:
+            mmcfg_addr = *cfg_idx
+                | PCIE_MMCFG_CONFOFFSET(offset - BCM2838_PCIE_EXT_CFG_DATA);
+            pcie_hb->mmio.ops->write(opaque, mmcfg_addr, value, size);
+            break;
+        default:
+            memcpy(root_regs + offset - PCIE_CONFIG_SPACE_SIZE, &value, size);
+        }
+    } else {
+        qemu_log_mask(
+            LOG_GUEST_ERROR,
+            "%s: out-of-range access, %u bytes @ offset 0x%04" PRIx64 "\n",
+            __func__, size, offset);
+    }
+}
+
+static const MemoryRegionOps bcm2838_pcie_host_ops = {
+    .read = bcm2838_pcie_host_read,
+    .write = bcm2838_pcie_host_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {.max_access_size = sizeof(uint64_t)},
+};
+
+int bcm2838_pcie_host_set_irq_num(BCM2838PcieHostState *s, int index, int spi)
+{
+    if (index >= BCM2838_PCIE_NUM_IRQS) {
+        return -EINVAL;
+    }
+
+    s->irq_num[index] = spi;
+    return 0;
+}
+
+static void bcm2838_pcie_host_set_irq(void *opaque, int irq_num, int level)
+{
+    BCM2838PcieHostState *s = opaque;
+
+    qemu_set_irq(s->irq[irq_num], level);
+}
+
+static PCIINTxRoute bcm2838_pcie_host_route_intx_pin_to_irq(void *opaque,
+                                                            int pin)
+{
+    PCIINTxRoute route;
+    BCM2838PcieHostState *s = opaque;
+
+    route.irq = s->irq_num[pin];
+    route.mode = route.irq < 0 ? PCI_INTX_DISABLED : PCI_INTX_ENABLED;
+
+    return route;
+}
+
+static int bcm2838_pcie_host_map_irq(PCIDevice *pci_dev, int pin)
+{
+    return pin;
+}
+
+static void bcm2838_pcie_host_realize(DeviceState *dev, Error **errp)
+{
+    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+    BCM2838PcieHostState *s = BCM2838_PCIE_HOST(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    int i;
+
+    memory_region_init_io(&s->cfg_regs, OBJECT(s), &bcm2838_pcie_host_ops, s,
+                          "bcm2838_pcie_cfg_regs", BCM2838_PCIE_REGS_SIZE);
+    sysbus_init_mmio(sbd, &s->cfg_regs);
+
+    /*
+     * The MemoryRegions io_mmio and io_ioport that we pass to
+     * pci_register_root_bus() are not the same as the MemoryRegions
+     * io_mmio_window and io_ioport_window that we expose as SysBus MRs.
+     * The difference is in the behavior of accesses to addresses where no PCI
+     * device has been mapped.
+     *
+     * io_mmio and io_ioport are the underlying PCI view of the PCI address
+     * space, and when a PCI device does a bus master access to a bad address
+     * this is reported back to it as a transaction failure.
+     *
+     * io_mmio_window and io_ioport_window implement "unmapped addresses read as
+     * -1 and ignore writes"; this is a traditional x86 PC behavior, which is
+     * not mandated properly by the PCI spec but expected by the majority of
+     * PCI-using guest software, including Linux.
+     *
+     * We implement it in the PCIe host controller, by providing the *_window
+     * MRs, which are containers with io ops that implement the 'background'
+     * behavior and which hold the real PCI MRs as sub-regions.
+     */
+    memory_region_init(&s->io_mmio, OBJECT(s), "bcm2838_pcie_mmio", UINT64_MAX);
+    memory_region_init(&s->io_ioport, OBJECT(s), "bcm2838_pcie_ioport",
+                       64 * 1024);
+
+    memory_region_init_io(&s->io_mmio_window, OBJECT(s),
+                            &unassigned_io_ops, OBJECT(s),
+                            "bcm2838_pcie_mmio_window", UINT64_MAX);
+    memory_region_init_io(&s->io_ioport_window, OBJECT(s),
+                            &unassigned_io_ops, OBJECT(s),
+                            "bcm2838_pcie_ioport_window", 64 * 1024);
+
+    memory_region_add_subregion(&s->io_mmio_window, 0, &s->io_mmio);
+    memory_region_add_subregion(&s->io_ioport_window, 0, &s->io_ioport);
+    sysbus_init_mmio(sbd, &s->io_mmio_window);
+    sysbus_init_mmio(sbd, &s->io_ioport_window);
+
+    for (i = 0; i < BCM2838_PCIE_NUM_IRQS; i++) {
+        sysbus_init_irq(sbd, &s->irq[i]);
+        s->irq_num[i] = -1;
+    }
+
+    pci->bus = pci_register_root_bus(dev, "pcie.0", bcm2838_pcie_host_set_irq,
+                                     bcm2838_pcie_host_map_irq, s, &s->io_mmio,
+                                     &s->io_ioport, 0, BCM2838_PCIE_NUM_IRQS,
+                                     TYPE_PCIE_BUS);
+    pci_bus_set_route_irq_fn(pci->bus, bcm2838_pcie_host_route_intx_pin_to_irq);
+    qdev_realize(DEVICE(&s->root_port), BUS(pci->bus), &error_fatal);
+}
+
+static const char *bcm2838_pcie_host_root_bus_path(PCIHostState *host_bridge,
+                                                   PCIBus *rootbus)
+{
+    return "0000:00";
+}
+
+static void bcm2838_pcie_host_class_init(ObjectClass *class, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(class);
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
+
+    hc->root_bus_path = bcm2838_pcie_host_root_bus_path;
+    dc->realize = bcm2838_pcie_host_realize;
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+    dc->fw_name = "pci";
+}
+
+static void bcm2838_pcie_host_initfn(Object *obj)
+{
+    BCM2838PcieHostState *s = BCM2838_PCIE_HOST(obj);
+    BCM2838PcieRootState *root = &s->root_port;
+
+    object_initialize_child(obj, "root_port", root, TYPE_BCM2838_PCIE_ROOT);
+    qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
+    qdev_prop_set_bit(DEVICE(root), "multifunction", false);
+}
+
+static const TypeInfo bcm2838_pcie_host_info = {
+    .name       = TYPE_BCM2838_PCIE_HOST,
+    .parent     = TYPE_PCIE_HOST_BRIDGE,
+    .instance_size = sizeof(BCM2838PcieHostState),
+    .instance_init = bcm2838_pcie_host_initfn,
+    .class_init = bcm2838_pcie_host_class_init,
+};
+
 /*
  * RC root part (D0:F0)
  */
@@ -69,6 +280,7 @@ static const TypeInfo bcm2838_pcie_root_info = {
 static void bcm2838_pcie_register(void)
 {
     type_register_static(&bcm2838_pcie_root_info);
+    type_register_static(&bcm2838_pcie_host_info);
 }
 
 type_init(bcm2838_pcie_register)
diff --git a/include/hw/arm/bcm2838_pcie.h b/include/hw/arm/bcm2838_pcie.h
index 39828f817f..58c3a0efe7 100644
--- a/include/hw/arm/bcm2838_pcie.h
+++ b/include/hw/arm/bcm2838_pcie.h
@@ -16,6 +16,9 @@
 #include "hw/pci/pcie_port.h"
 #include "qom/object.h"
 
+#define TYPE_BCM2838_PCIE_HOST "bcm2838-pcie-host"
+OBJECT_DECLARE_SIMPLE_TYPE(BCM2838PcieHostState, BCM2838_PCIE_HOST)
+
 #define TYPE_BCM2838_PCIE_ROOT "bcm2838-pcie-root"
 OBJECT_DECLARE_TYPE(BCM2838PcieRootState, BCM2838PcieRootClass,
                     BCM2838_PCIE_ROOT)
@@ -50,4 +53,23 @@ struct BCM2838PcieRootClass {
 };
 
 
+struct BCM2838PcieHostState {
+    /*< private >*/
+    PCIExpressHost parent_obj;
+
+    /*< public >*/
+    BCM2838PcieRootState root_port;
+
+    MemoryRegion cfg_regs;
+    MemoryRegion io_ioport;
+    MemoryRegion io_mmio;
+    MemoryRegion io_ioport_window;
+    MemoryRegion io_mmio_window;
+
+    qemu_irq irq[BCM2838_PCIE_NUM_IRQS];
+    int irq_num[BCM2838_PCIE_NUM_IRQS];
+};
+
+int bcm2838_pcie_host_set_irq_num(BCM2838PcieHostState *s, int index, int spi);
+
 #endif /* BCM2838_PCIE_H */
-- 
2.34.1
Re: [PATCH v4 15/45] Add BCM2838 PCIE host
Posted by Sergei Kambalin 10 months ago
I'm not super familiar with how QEMU models PCI controllers, but I'm not 
sure this handling of the root port config registers is right. On other 
controllers it looks like the root config reads and writes are handled 
by setting the PCIDeviceClass::config_read and PCIDevice::config_write 
methods.
It's not only for config space access but also to configuration area beyond PCIE config space.
I'm going to split into two access methods:
1) bcm2838_pcie_config_{write,read}() for access to addresses within the config space. Will be assigned to the dev->config_write.
2) bcm2838_pcie_mmio_{write,read}() for access to addresses beyond the config space. Will be implemented as 'normal' MemoryRegionOps structure.

Isn't the important thing here not what guest software thinks, but what 
the actual hardware in this bcm2838 SoC implements ?

The thing is that the code is not fully abides the PCIe spec. It's been written mostly basing on kernel code and boot logs analysis.
That's why I think it worth to add a detailed comment why it was written that way.

On 12/7/23 20:31, Sergey Kambalin wrote:
> Signed-off-by: Sergey Kambalin<sergey.kambalin@auriga.com>
> ---
>   hw/arm/bcm2838_pcie.c         | 216 +++++++++++++++++++++++++++++++++-
>   include/hw/arm/bcm2838_pcie.h |  22 ++++
>   2 files changed, 236 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/bcm2838_pcie.c b/hw/arm/bcm2838_pcie.c
> index 3b4373c6a6..75146d6c2e 100644
> --- a/hw/arm/bcm2838_pcie.c
> +++ b/hw/arm/bcm2838_pcie.c
> @@ -12,11 +12,222 @@
>   #include "hw/irq.h"
>   #include "hw/pci-host/gpex.h"
>   #include "hw/qdev-properties.h"
> -#include "migration/vmstate.h"
> -#include "qemu/module.h"
>   #include "hw/arm/bcm2838_pcie.h"
>   #include "trace.h"
>   
> +/*
> + * RC host part
> + */
> +
> +static uint64_t bcm2838_pcie_host_read(void *opaque, hwaddr offset,
> +                                       unsigned size) {
> +    hwaddr mmcfg_addr;
> +    uint64_t value = ~0;
> +    BCM2838PcieHostState *s = opaque;
> +    PCIExpressHost *pcie_hb = PCIE_HOST_BRIDGE(s);
> +    PCIDevice *root_pci_dev = PCI_DEVICE(&s->root_port);
> +    uint8_t *root_regs = s->root_port.regs;
> +    uint32_t *cfg_idx = (uint32_t *)(root_regs + BCM2838_PCIE_EXT_CFG_INDEX
> +                                     - PCIE_CONFIG_SPACE_SIZE);
> +
> +    if (offset < PCIE_CONFIG_SPACE_SIZE) {
> +        value = pci_host_config_read_common(root_pci_dev, offset,
> +                                            PCIE_CONFIG_SPACE_SIZE, size);
> +    } else if (offset - PCIE_CONFIG_SPACE_SIZE + size
> +               <= sizeof(s->root_port.regs)) {
> +        switch (offset) {
> +        case BCM2838_PCIE_EXT_CFG_DATA
> +            ... BCM2838_PCIE_EXT_CFG_DATA + PCIE_CONFIG_SPACE_SIZE - 1:
> +            mmcfg_addr = *cfg_idx
> +                | PCIE_MMCFG_CONFOFFSET(offset - BCM2838_PCIE_EXT_CFG_DATA);
> +            value = pcie_hb->mmio.ops->read(opaque, mmcfg_addr, size);
> +            break;
> +        default:
> +            memcpy(&value, root_regs + offset - PCIE_CONFIG_SPACE_SIZE, size);
> +        }
> +    } else {
> +        qemu_log_mask(
> +            LOG_GUEST_ERROR,
> +            "%s: out-of-range access, %u bytes @ offset 0x%04" PRIx64 "\n",
> +            __func__, size, offset);
> +    }
> +
> +    trace_bcm2838_pcie_host_read(size, offset, value);
> +    return value;
> +}
> +
> +static void bcm2838_pcie_host_write(void *opaque, hwaddr offset,
> +                                    uint64_t value, unsigned size) {
> +    hwaddr mmcfg_addr;
> +    BCM2838PcieHostState *s = opaque;
> +    PCIExpressHost *pcie_hb = PCIE_HOST_BRIDGE(s);
> +    PCIDevice *root_pci_dev = PCI_DEVICE(&s->root_port);
> +    uint8_t *root_regs = s->root_port.regs;
> +    uint32_t *cfg_idx = (uint32_t *)(root_regs + BCM2838_PCIE_EXT_CFG_INDEX
> +                                     - PCIE_CONFIG_SPACE_SIZE);
> +
> +    trace_bcm2838_pcie_host_write(size, offset, value);
> +
> +    if (offset < PCIE_CONFIG_SPACE_SIZE) {
> +        pci_host_config_write_common(root_pci_dev, offset,
> +                                     PCIE_CONFIG_SPACE_SIZE, value, size);
> +    } else if (offset - PCIE_CONFIG_SPACE_SIZE + size
> +               <= sizeof(s->root_port.regs)) {
> +        switch (offset) {
> +        case BCM2838_PCIE_EXT_CFG_DATA
> +            ... BCM2838_PCIE_EXT_CFG_DATA + PCIE_CONFIG_SPACE_SIZE - 1:
> +            mmcfg_addr = *cfg_idx
> +                | PCIE_MMCFG_CONFOFFSET(offset - BCM2838_PCIE_EXT_CFG_DATA);
> +            pcie_hb->mmio.ops->write(opaque, mmcfg_addr, value, size);
> +            break;
> +        default:
> +            memcpy(root_regs + offset - PCIE_CONFIG_SPACE_SIZE, &value, size);
> +        }
> +    } else {
> +        qemu_log_mask(
> +            LOG_GUEST_ERROR,
> +            "%s: out-of-range access, %u bytes @ offset 0x%04" PRIx64 "\n",
> +            __func__, size, offset);
> +    }
> +}
> +
> +static const MemoryRegionOps bcm2838_pcie_host_ops = {
> +    .read = bcm2838_pcie_host_read,
> +    .write = bcm2838_pcie_host_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {.max_access_size = sizeof(uint64_t)},
> +};
> +
> +int bcm2838_pcie_host_set_irq_num(BCM2838PcieHostState *s, int index, int spi)
> +{
> +    if (index >= BCM2838_PCIE_NUM_IRQS) {
> +        return -EINVAL;
> +    }
> +
> +    s->irq_num[index] = spi;
> +    return 0;
> +}
> +
> +static void bcm2838_pcie_host_set_irq(void *opaque, int irq_num, int level)
> +{
> +    BCM2838PcieHostState *s = opaque;
> +
> +    qemu_set_irq(s->irq[irq_num], level);
> +}
> +
> +static PCIINTxRoute bcm2838_pcie_host_route_intx_pin_to_irq(void *opaque,
> +                                                            int pin)
> +{
> +    PCIINTxRoute route;
> +    BCM2838PcieHostState *s = opaque;
> +
> +    route.irq = s->irq_num[pin];
> +    route.mode = route.irq < 0 ? PCI_INTX_DISABLED : PCI_INTX_ENABLED;
> +
> +    return route;
> +}
> +
> +static int bcm2838_pcie_host_map_irq(PCIDevice *pci_dev, int pin)
> +{
> +    return pin;
> +}
> +
> +static void bcm2838_pcie_host_realize(DeviceState *dev, Error **errp)
> +{
> +    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> +    BCM2838PcieHostState *s = BCM2838_PCIE_HOST(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    int i;
> +
> +    memory_region_init_io(&s->cfg_regs, OBJECT(s), &bcm2838_pcie_host_ops, s,
> +                          "bcm2838_pcie_cfg_regs", BCM2838_PCIE_REGS_SIZE);
> +    sysbus_init_mmio(sbd, &s->cfg_regs);
> +
> +    /*
> +     * The MemoryRegions io_mmio and io_ioport that we pass to
> +     * pci_register_root_bus() are not the same as the MemoryRegions
> +     * io_mmio_window and io_ioport_window that we expose as SysBus MRs.
> +     * The difference is in the behavior of accesses to addresses where no PCI
> +     * device has been mapped.
> +     *
> +     * io_mmio and io_ioport are the underlying PCI view of the PCI address
> +     * space, and when a PCI device does a bus master access to a bad address
> +     * this is reported back to it as a transaction failure.
> +     *
> +     * io_mmio_window and io_ioport_window implement "unmapped addresses read as
> +     * -1 and ignore writes"; this is a traditional x86 PC behavior, which is
> +     * not mandated properly by the PCI spec but expected by the majority of
> +     * PCI-using guest software, including Linux.
> +     *
> +     * We implement it in the PCIe host controller, by providing the *_window
> +     * MRs, which are containers with io ops that implement the 'background'
> +     * behavior and which hold the real PCI MRs as sub-regions.
> +     */
> +    memory_region_init(&s->io_mmio, OBJECT(s), "bcm2838_pcie_mmio", UINT64_MAX);
> +    memory_region_init(&s->io_ioport, OBJECT(s), "bcm2838_pcie_ioport",
> +                       64 * 1024);
> +
> +    memory_region_init_io(&s->io_mmio_window, OBJECT(s),
> +                            &unassigned_io_ops, OBJECT(s),
> +                            "bcm2838_pcie_mmio_window", UINT64_MAX);
> +    memory_region_init_io(&s->io_ioport_window, OBJECT(s),
> +                            &unassigned_io_ops, OBJECT(s),
> +                            "bcm2838_pcie_ioport_window", 64 * 1024);
> +
> +    memory_region_add_subregion(&s->io_mmio_window, 0, &s->io_mmio);
> +    memory_region_add_subregion(&s->io_ioport_window, 0, &s->io_ioport);
> +    sysbus_init_mmio(sbd, &s->io_mmio_window);
> +    sysbus_init_mmio(sbd, &s->io_ioport_window);
> +
> +    for (i = 0; i < BCM2838_PCIE_NUM_IRQS; i++) {
> +        sysbus_init_irq(sbd, &s->irq[i]);
> +        s->irq_num[i] = -1;
> +    }
> +
> +    pci->bus = pci_register_root_bus(dev, "pcie.0", bcm2838_pcie_host_set_irq,
> +                                     bcm2838_pcie_host_map_irq, s, &s->io_mmio,
> +                                     &s->io_ioport, 0, BCM2838_PCIE_NUM_IRQS,
> +                                     TYPE_PCIE_BUS);
> +    pci_bus_set_route_irq_fn(pci->bus, bcm2838_pcie_host_route_intx_pin_to_irq);
> +    qdev_realize(DEVICE(&s->root_port), BUS(pci->bus), &error_fatal);
> +}
> +
> +static const char *bcm2838_pcie_host_root_bus_path(PCIHostState *host_bridge,
> +                                                   PCIBus *rootbus)
> +{
> +    return "0000:00";
> +}
> +
> +static void bcm2838_pcie_host_class_init(ObjectClass *class, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(class);
> +    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
> +
> +    hc->root_bus_path = bcm2838_pcie_host_root_bus_path;
> +    dc->realize = bcm2838_pcie_host_realize;
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->fw_name = "pci";
> +}
> +
> +static void bcm2838_pcie_host_initfn(Object *obj)
> +{
> +    BCM2838PcieHostState *s = BCM2838_PCIE_HOST(obj);
> +    BCM2838PcieRootState *root = &s->root_port;
> +
> +    object_initialize_child(obj, "root_port", root, TYPE_BCM2838_PCIE_ROOT);
> +    qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
> +    qdev_prop_set_bit(DEVICE(root), "multifunction", false);
> +}
> +
> +static const TypeInfo bcm2838_pcie_host_info = {
> +    .name       = TYPE_BCM2838_PCIE_HOST,
> +    .parent     = TYPE_PCIE_HOST_BRIDGE,
> +    .instance_size = sizeof(BCM2838PcieHostState),
> +    .instance_init = bcm2838_pcie_host_initfn,
> +    .class_init = bcm2838_pcie_host_class_init,
> +};
> +
>   /*
>    * RC root part (D0:F0)
>    */
> @@ -69,6 +280,7 @@ static const TypeInfo bcm2838_pcie_root_info = {
>   static void bcm2838_pcie_register(void)
>   {
>       type_register_static(&bcm2838_pcie_root_info);
> +    type_register_static(&bcm2838_pcie_host_info);
>   }
>   
>   type_init(bcm2838_pcie_register)
> diff --git a/include/hw/arm/bcm2838_pcie.h b/include/hw/arm/bcm2838_pcie.h
> index 39828f817f..58c3a0efe7 100644
> --- a/include/hw/arm/bcm2838_pcie.h
> +++ b/include/hw/arm/bcm2838_pcie.h
> @@ -16,6 +16,9 @@
>   #include "hw/pci/pcie_port.h"
>   #include "qom/object.h"
>   
> +#define TYPE_BCM2838_PCIE_HOST "bcm2838-pcie-host"
> +OBJECT_DECLARE_SIMPLE_TYPE(BCM2838PcieHostState, BCM2838_PCIE_HOST)
> +
>   #define TYPE_BCM2838_PCIE_ROOT "bcm2838-pcie-root"
>   OBJECT_DECLARE_TYPE(BCM2838PcieRootState, BCM2838PcieRootClass,
>                       BCM2838_PCIE_ROOT)
> @@ -50,4 +53,23 @@ struct BCM2838PcieRootClass {
>   };
>   
>   
> +struct BCM2838PcieHostState {
> +    /*< private >*/
> +    PCIExpressHost parent_obj;
> +
> +    /*< public >*/
> +    BCM2838PcieRootState root_port;
> +
> +    MemoryRegion cfg_regs;
> +    MemoryRegion io_ioport;
> +    MemoryRegion io_mmio;
> +    MemoryRegion io_ioport_window;
> +    MemoryRegion io_mmio_window;
> +
> +    qemu_irq irq[BCM2838_PCIE_NUM_IRQS];
> +    int irq_num[BCM2838_PCIE_NUM_IRQS];
> +};
> +
> +int bcm2838_pcie_host_set_irq_num(BCM2838PcieHostState *s, int index, int spi);
> +
>   #endif /* BCM2838_PCIE_H */
Re: [PATCH v4 15/45] Add BCM2838 PCIE host
Posted by Peter Maydell 10 months, 2 weeks ago
(I've cc'd our PCI subsystem maintainers, because I'm not really
confident in reviewing a PCI controller model. Marcel, Michael:
this is one of 3 patches adding the pcie controller for the raspberry
pi 4 SoC. You can find the relevant patches also here:
https://patchew.org/QEMU/20231208023145.1385775-1-sergey.kambalin@auriga.com
/20231208023145.1385775-15-sergey.kambalin@auriga.com/
(link is to patch 14, the PCIE patches are 14 and 15, plus 16
is the one that wires up the controller into the SoC).

On Fri, 8 Dec 2023 at 02:33, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/arm/bcm2838_pcie.c         | 216 +++++++++++++++++++++++++++++++++-
>  include/hw/arm/bcm2838_pcie.h |  22 ++++
>  2 files changed, 236 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/bcm2838_pcie.c b/hw/arm/bcm2838_pcie.c
> index 3b4373c6a6..75146d6c2e 100644
> --- a/hw/arm/bcm2838_pcie.c
> +++ b/hw/arm/bcm2838_pcie.c
> @@ -12,11 +12,222 @@
>  #include "hw/irq.h"
>  #include "hw/pci-host/gpex.h"
>  #include "hw/qdev-properties.h"
> -#include "migration/vmstate.h"
> -#include "qemu/module.h"
>  #include "hw/arm/bcm2838_pcie.h"
>  #include "trace.h"
>
> +/*
> + * RC host part
> + */
> +
> +static uint64_t bcm2838_pcie_host_read(void *opaque, hwaddr offset,
> +                                       unsigned size) {
> +    hwaddr mmcfg_addr;
> +    uint64_t value = ~0;
> +    BCM2838PcieHostState *s = opaque;
> +    PCIExpressHost *pcie_hb = PCIE_HOST_BRIDGE(s);
> +    PCIDevice *root_pci_dev = PCI_DEVICE(&s->root_port);
> +    uint8_t *root_regs = s->root_port.regs;
> +    uint32_t *cfg_idx = (uint32_t *)(root_regs + BCM2838_PCIE_EXT_CFG_INDEX
> +                                     - PCIE_CONFIG_SPACE_SIZE);
> +
> +    if (offset < PCIE_CONFIG_SPACE_SIZE) {
> +        value = pci_host_config_read_common(root_pci_dev, offset,
> +                                            PCIE_CONFIG_SPACE_SIZE, size);
> +    } else if (offset - PCIE_CONFIG_SPACE_SIZE + size
> +               <= sizeof(s->root_port.regs)) {
> +        switch (offset) {
> +        case BCM2838_PCIE_EXT_CFG_DATA
> +            ... BCM2838_PCIE_EXT_CFG_DATA + PCIE_CONFIG_SPACE_SIZE - 1:
> +            mmcfg_addr = *cfg_idx
> +                | PCIE_MMCFG_CONFOFFSET(offset - BCM2838_PCIE_EXT_CFG_DATA);
> +            value = pcie_hb->mmio.ops->read(opaque, mmcfg_addr, size);
> +            break;
> +        default:
> +            memcpy(&value, root_regs + offset - PCIE_CONFIG_SPACE_SIZE, size);
> +        }
> +    } else {
> +        qemu_log_mask(
> +            LOG_GUEST_ERROR,
> +            "%s: out-of-range access, %u bytes @ offset 0x%04" PRIx64 "\n",
> +            __func__, size, offset);
> +    }
> +
> +    trace_bcm2838_pcie_host_read(size, offset, value);
> +    return value;
> +}
> +
> +static void bcm2838_pcie_host_write(void *opaque, hwaddr offset,
> +                                    uint64_t value, unsigned size) {
> +    hwaddr mmcfg_addr;
> +    BCM2838PcieHostState *s = opaque;
> +    PCIExpressHost *pcie_hb = PCIE_HOST_BRIDGE(s);
> +    PCIDevice *root_pci_dev = PCI_DEVICE(&s->root_port);
> +    uint8_t *root_regs = s->root_port.regs;
> +    uint32_t *cfg_idx = (uint32_t *)(root_regs + BCM2838_PCIE_EXT_CFG_INDEX
> +                                     - PCIE_CONFIG_SPACE_SIZE);
> +
> +    trace_bcm2838_pcie_host_write(size, offset, value);
> +
> +    if (offset < PCIE_CONFIG_SPACE_SIZE) {
> +        pci_host_config_write_common(root_pci_dev, offset,
> +                                     PCIE_CONFIG_SPACE_SIZE, value, size);
> +    } else if (offset - PCIE_CONFIG_SPACE_SIZE + size
> +               <= sizeof(s->root_port.regs)) {
> +        switch (offset) {
> +        case BCM2838_PCIE_EXT_CFG_DATA
> +            ... BCM2838_PCIE_EXT_CFG_DATA + PCIE_CONFIG_SPACE_SIZE - 1:
> +            mmcfg_addr = *cfg_idx
> +                | PCIE_MMCFG_CONFOFFSET(offset - BCM2838_PCIE_EXT_CFG_DATA);
> +            pcie_hb->mmio.ops->write(opaque, mmcfg_addr, value, size);
> +            break;
> +        default:
> +            memcpy(root_regs + offset - PCIE_CONFIG_SPACE_SIZE, &value, size);
> +        }
> +    } else {
> +        qemu_log_mask(
> +            LOG_GUEST_ERROR,
> +            "%s: out-of-range access, %u bytes @ offset 0x%04" PRIx64 "\n",
> +            __func__, size, offset);
> +    }
> +}
> +
> +static const MemoryRegionOps bcm2838_pcie_host_ops = {
> +    .read = bcm2838_pcie_host_read,
> +    .write = bcm2838_pcie_host_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {.max_access_size = sizeof(uint64_t)},
> +};

I'm not super familiar with how QEMU models PCI controllers,
but I'm not sure this handling of the root port config registers
is right. On other controllers it looks like the
root config reads and writes are handled by setting the
PCIDeviceClass::config_read and PCIDevice::config_write
methods.

> +
> +int bcm2838_pcie_host_set_irq_num(BCM2838PcieHostState *s, int index, int spi)
> +{
> +    if (index >= BCM2838_PCIE_NUM_IRQS) {
> +        return -EINVAL;
> +    }
> +
> +    s->irq_num[index] = spi;
> +    return 0;
> +}
> +
> +static void bcm2838_pcie_host_set_irq(void *opaque, int irq_num, int level)
> +{
> +    BCM2838PcieHostState *s = opaque;
> +
> +    qemu_set_irq(s->irq[irq_num], level);
> +}
> +
> +static PCIINTxRoute bcm2838_pcie_host_route_intx_pin_to_irq(void *opaque,
> +                                                            int pin)
> +{
> +    PCIINTxRoute route;
> +    BCM2838PcieHostState *s = opaque;
> +
> +    route.irq = s->irq_num[pin];
> +    route.mode = route.irq < 0 ? PCI_INTX_DISABLED : PCI_INTX_ENABLED;
> +
> +    return route;
> +}
> +
> +static int bcm2838_pcie_host_map_irq(PCIDevice *pci_dev, int pin)
> +{
> +    return pin;
> +}
> +
> +static void bcm2838_pcie_host_realize(DeviceState *dev, Error **errp)
> +{
> +    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> +    BCM2838PcieHostState *s = BCM2838_PCIE_HOST(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    int i;
> +
> +    memory_region_init_io(&s->cfg_regs, OBJECT(s), &bcm2838_pcie_host_ops, s,
> +                          "bcm2838_pcie_cfg_regs", BCM2838_PCIE_REGS_SIZE);
> +    sysbus_init_mmio(sbd, &s->cfg_regs);
> +
> +    /*
> +     * The MemoryRegions io_mmio and io_ioport that we pass to
> +     * pci_register_root_bus() are not the same as the MemoryRegions
> +     * io_mmio_window and io_ioport_window that we expose as SysBus MRs.
> +     * The difference is in the behavior of accesses to addresses where no PCI
> +     * device has been mapped.
> +     *
> +     * io_mmio and io_ioport are the underlying PCI view of the PCI address
> +     * space, and when a PCI device does a bus master access to a bad address
> +     * this is reported back to it as a transaction failure.
> +     *
> +     * io_mmio_window and io_ioport_window implement "unmapped addresses read as
> +     * -1 and ignore writes"; this is a traditional x86 PC behavior, which is
> +     * not mandated properly by the PCI spec but expected by the majority of
> +     * PCI-using guest software, including Linux.

Isn't the important thing here not what guest software thinks, but
what the actual hardware in this bcm2838 SoC implements ?

> +     *
> +     * We implement it in the PCIe host controller, by providing the *_window
> +     * MRs, which are containers with io ops that implement the 'background'
> +     * behavior and which hold the real PCI MRs as sub-regions.
> +     */
> +    memory_region_init(&s->io_mmio, OBJECT(s), "bcm2838_pcie_mmio", UINT64_MAX);
> +    memory_region_init(&s->io_ioport, OBJECT(s), "bcm2838_pcie_ioport",
> +                       64 * 1024);
> +
> +    memory_region_init_io(&s->io_mmio_window, OBJECT(s),
> +                            &unassigned_io_ops, OBJECT(s),
> +                            "bcm2838_pcie_mmio_window", UINT64_MAX);
> +    memory_region_init_io(&s->io_ioport_window, OBJECT(s),
> +                            &unassigned_io_ops, OBJECT(s),
> +                            "bcm2838_pcie_ioport_window", 64 * 1024);
> +
> +    memory_region_add_subregion(&s->io_mmio_window, 0, &s->io_mmio);
> +    memory_region_add_subregion(&s->io_ioport_window, 0, &s->io_ioport);
> +    sysbus_init_mmio(sbd, &s->io_mmio_window);
> +    sysbus_init_mmio(sbd, &s->io_ioport_window);
> +
> +    for (i = 0; i < BCM2838_PCIE_NUM_IRQS; i++) {
> +        sysbus_init_irq(sbd, &s->irq[i]);
> +        s->irq_num[i] = -1;
> +    }
> +
> +    pci->bus = pci_register_root_bus(dev, "pcie.0", bcm2838_pcie_host_set_irq,
> +                                     bcm2838_pcie_host_map_irq, s, &s->io_mmio,
> +                                     &s->io_ioport, 0, BCM2838_PCIE_NUM_IRQS,
> +                                     TYPE_PCIE_BUS);
> +    pci_bus_set_route_irq_fn(pci->bus, bcm2838_pcie_host_route_intx_pin_to_irq);
> +    qdev_realize(DEVICE(&s->root_port), BUS(pci->bus), &error_fatal);
> +}
> +
> +static const char *bcm2838_pcie_host_root_bus_path(PCIHostState *host_bridge,
> +                                                   PCIBus *rootbus)
> +{
> +    return "0000:00";
> +}
> +
> +static void bcm2838_pcie_host_class_init(ObjectClass *class, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(class);
> +    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
> +
> +    hc->root_bus_path = bcm2838_pcie_host_root_bus_path;
> +    dc->realize = bcm2838_pcie_host_realize;
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->fw_name = "pci";
> +}
> +
> +static void bcm2838_pcie_host_initfn(Object *obj)
> +{
> +    BCM2838PcieHostState *s = BCM2838_PCIE_HOST(obj);
> +    BCM2838PcieRootState *root = &s->root_port;
> +
> +    object_initialize_child(obj, "root_port", root, TYPE_BCM2838_PCIE_ROOT);
> +    qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
> +    qdev_prop_set_bit(DEVICE(root), "multifunction", false);
> +}
> +
> +static const TypeInfo bcm2838_pcie_host_info = {
> +    .name       = TYPE_BCM2838_PCIE_HOST,
> +    .parent     = TYPE_PCIE_HOST_BRIDGE,
> +    .instance_size = sizeof(BCM2838PcieHostState),
> +    .instance_init = bcm2838_pcie_host_initfn,
> +    .class_init = bcm2838_pcie_host_class_init,
> +};
> +
>  /*
>   * RC root part (D0:F0)
>   */
> @@ -69,6 +280,7 @@ static const TypeInfo bcm2838_pcie_root_info = {
>  static void bcm2838_pcie_register(void)
>  {
>      type_register_static(&bcm2838_pcie_root_info);
> +    type_register_static(&bcm2838_pcie_host_info);
>  }
>
>  type_init(bcm2838_pcie_register)
> diff --git a/include/hw/arm/bcm2838_pcie.h b/include/hw/arm/bcm2838_pcie.h
> index 39828f817f..58c3a0efe7 100644
> --- a/include/hw/arm/bcm2838_pcie.h
> +++ b/include/hw/arm/bcm2838_pcie.h
> @@ -16,6 +16,9 @@
>  #include "hw/pci/pcie_port.h"
>  #include "qom/object.h"
>
> +#define TYPE_BCM2838_PCIE_HOST "bcm2838-pcie-host"
> +OBJECT_DECLARE_SIMPLE_TYPE(BCM2838PcieHostState, BCM2838_PCIE_HOST)
> +
>  #define TYPE_BCM2838_PCIE_ROOT "bcm2838-pcie-root"
>  OBJECT_DECLARE_TYPE(BCM2838PcieRootState, BCM2838PcieRootClass,
>                      BCM2838_PCIE_ROOT)
> @@ -50,4 +53,23 @@ struct BCM2838PcieRootClass {
>  };
>
>
> +struct BCM2838PcieHostState {
> +    /*< private >*/
> +    PCIExpressHost parent_obj;
> +
> +    /*< public >*/
> +    BCM2838PcieRootState root_port;
> +
> +    MemoryRegion cfg_regs;
> +    MemoryRegion io_ioport;
> +    MemoryRegion io_mmio;
> +    MemoryRegion io_ioport_window;
> +    MemoryRegion io_mmio_window;
> +
> +    qemu_irq irq[BCM2838_PCIE_NUM_IRQS];
> +    int irq_num[BCM2838_PCIE_NUM_IRQS];
> +};
> +
> +int bcm2838_pcie_host_set_irq_num(BCM2838PcieHostState *s, int index, int spi);
> +
>  #endif /* BCM2838_PCIE_H */
> --
> 2.34.1

thanks
-- PMM