Re: [PATCH v6 14/41] Add BCM2838 PCIE host

[Philippe Mathieu-Daudé]

Hi Sergey,

On 26/2/24 01:02, Sergey Kambalin wrote:

Signed-off-by: Sergey Kambalin 
---
  hw/arm/bcm2838_pcie.c | 217 +-
  hw/arm/trace-events   |   4 +
  include/hw/arm/bcm2838_pcie.h |  22 
  3 files changed, 241 insertions(+), 2 deletions(-)




+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(>cfg_regs, OBJECT(s), _pcie_host_ops, s,
+  "bcm2838_pcie_cfg_regs", BCM2838_PCIE_REGS_SIZE);
+sysbus_init_mmio(sbd, >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(>io_mmio, OBJECT(s), "bcm2838_pcie_mmio", 
UINT64_MAX);
+memory_region_init(>io_ioport, OBJECT(s), "bcm2838_pcie_ioport",
+   64 * 1024);
+
+memory_region_init_io(>io_mmio_window, OBJECT(s),
+_io_ops, OBJECT(s),
+"bcm2838_pcie_mmio_window", UINT64_MAX);
+memory_region_init_io(>io_ioport_window, OBJECT(s),
+_io_ops, OBJECT(s),
+"bcm2838_pcie_ioport_window", 64 * 1024);
+
+memory_region_add_subregion(>io_mmio_window, 0, >io_mmio);
+memory_region_add_subregion(>io_ioport_window, 0, >io_ioport);
+sysbus_init_mmio(sbd, >io_mmio_window);
+sysbus_init_mmio(sbd, >io_ioport_window);
+
+for (i = 0; i < BCM2838_PCIE_NUM_IRQS; i++) {
+sysbus_init_irq(sbd, >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, >io_mmio,
+ >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(>root_port), BUS(pci->bus), _fatal);
+}


Something is odd:

(qemu) info mtree
...
address-space: memory
  - (prio 0, i/o): system
...
[DETECTED OVERFLOW!] 0006-0005 (prio 0, 
i/o): bcm2838_pcie_mmio_window
[DETECTED OVERFLOW!]   0006-0005 (prio 0, 
i/o): bcm2838_pcie_mmio
0006-0006 (prio 1, i/o): alias 
pci_bridge_mem @pci_bridge_pci -
0006-0006 (prio 1, i/o): alias 
pci_bridge_pref_mem @pci_bridge_pci -

[PATCH v6 14/41] Add BCM2838 PCIE host

[Sergey Kambalin]

Signed-off-by: Sergey Kambalin 
---
 hw/arm/bcm2838_pcie.c | 217 +-
 hw/arm/trace-events   |   4 +
 include/hw/arm/bcm2838_pcie.h |  22 
 3 files changed, 241 insertions(+), 2 deletions(-)

diff --git a/hw/arm/bcm2838_pcie.c b/hw/arm/bcm2838_pcie.c
index cb1370433e..348263d9fb 100644
--- a/hw/arm/bcm2838_pcie.c
+++ b/hw/arm/bcm2838_pcie.c
@@ -12,11 +12,220 @@
 #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"
 
+static uint32_t bcm2838_pcie_config_read(PCIDevice *d,
+ uint32_t address, int len)
+{
+return pci_default_read_config(d, address, len);
+}
+
+static void bcm2838_pcie_config_write(PCIDevice *d, uint32_t addr, uint32_t 
val,
+  int len)
+{
+return pci_default_write_config(d, addr, val, len);
+}
+
+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);
+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 + 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(, 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);
+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 + 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, , 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(>cfg_regs, OBJECT(s), _pcie_host_ops, s,
+  "bcm2838_pcie_cfg_regs", BCM2838_PCIE_REGS_SIZE);
+sysbus_init_mmio(sbd, >cfg_regs);
+
+/*
+ * The MemoryRegions io_mmio and