Patchwork [1/9] PCI: PCI config space access overhaul

login
register
mail settings
Submitter Alexander Graf
Date Jan. 12, 2010, 11:58 a.m.
Message ID <1263297526-13518-2-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/42716/
State New
Headers show

Comments

Alexander Graf - Jan. 12, 2010, 11:58 a.m.
Different host buses may have different layouts for config space accessors.

The Mac U3 for example uses the following define to access Type 0 (directly
attached) devices:

  #define MACRISC_CFA0(devfn, off)        \
        ((1 << (unsigned int)PCI_SLOT(dev_fn)) \
        | (((unsigned int)PCI_FUNC(dev_fn)) << 8) \
        | (((unsigned int)(off)) & 0xFCUL))

Obviously, this is different from the encoding we interpret in qemu. In
order to let host controllers take some influence there, we can just
introduce a decoding function that takes whatever accessor we have and
decodes it into understandable fields.

To not touch all different code paths in qemu, I added this on top of
the existing config_reg element. In the future, we should work on gradually
moving references to config_reg to config_reg_dec and then phase out
config_reg.

This patch is the groundwork for Uninorth PCI config space fixes.

Signed-off-by: Alexander Graf <agraf@suse.de>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Isaku Yamahata <yamahata@valinux.co.jp>

---

v1 -> v2:

  - merge data access functions
  - use decoding function for config space bits
  - introduce encoding function for x86 style encodings

v2 -> v3:

  - call pci_addr_to_devfn pci_addr_to_config_reg
  - revert debug fix
  - call convert functions config_addr instead of config_reg
  - no spaces between braces
---
 hw/apb_pci.c               |    1 +
 hw/grackle_pci.c           |    1 +
 hw/gt64xxx.c               |    1 +
 hw/pci.h                   |   13 ++++++
 hw/pci_host.c              |   46 ++++++++++++++++++----
 hw/pci_host.h              |   16 ++++++++
 hw/pci_host_template.h     |   92 +++++++++++++------------------------------
 hw/pci_host_template_all.h |   23 +++++++++++
 hw/piix_pci.c              |    1 +
 hw/ppc4xx_pci.c            |    1 +
 hw/ppce500_pci.c           |    1 +
 hw/unin_pci.c              |    1 +
 12 files changed, 124 insertions(+), 73 deletions(-)
 create mode 100644 hw/pci_host_template_all.h

Patch

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index c14e1c0..05e7f38 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -300,6 +300,7 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
     /* mem_data */
     sysbus_mmio_map(s, 4, mem_base);
     d = FROM_SYSBUS(APBState, s);
+    pci_host_init(&d->host_state);
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_apb_set_irq, pci_pbm_map_irq, pic,
                                          0, 32);
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index ee4fed5..7feba63 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -88,6 +88,7 @@  PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(GrackleState, s);
+    pci_host_init(&d->host_state);
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_grackle_set_irq,
                                          pci_grackle_map_irq,
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index c8034e2..b3a951b 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1109,6 +1109,7 @@  PCIBus *pci_gt64120_init(qemu_irq *pic)
     s = qemu_mallocz(sizeof(GT64120State));
     s->pci = qemu_mallocz(sizeof(GT64120PCIState));
 
+    pci_host_init(s->pci);
     s->pci->bus = pci_register_bus(NULL, "pci",
                                    pci_gt64120_set_irq, pci_gt64120_map_irq,
                                    pic, 144, 4);
diff --git a/hw/pci.h b/hw/pci.h
index ed048f5..0106d3e 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -10,10 +10,23 @@ 
 
 /* PCI bus */
 
+typedef struct PCIAddress {
+    PCIBus *domain;
+    uint8_t bus;
+    uint8_t slot;
+    uint8_t fn;
+} PCIAddress;
+
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
 
+static inline uint32_t pci_addr_to_config_reg(PCIAddress *addr, uint32_t offset)
+{
+    return ((addr->bus & 0xff) << 16) | (PCI_DEVFN(addr->slot, addr->fn) << 8) |
+           offset;
+}
+
 /* Class, Vendor and Device IDs from Linux's pci_ids.h */
 #include "pci_ids.h"
 
diff --git a/hw/pci_host.c b/hw/pci_host.c
index 6289ead..e102644 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -32,13 +32,6 @@  do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0)
 #define PCI_DPRINTF(fmt, ...)
 #endif
 
-/*
- * PCI address
- * bit 16 - 24: bus number
- * bit  8 - 15: devfun number
- * bit  0 -  7: offset in configuration space of a given pci device
- */
-
 /* the helper functio to get a PCIDeice* for a given pci address */
 static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr)
 {
@@ -89,7 +82,9 @@  static void pci_host_config_writel(void *opaque, target_phys_addr_t addr,
 #endif
     PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
                 __func__, addr, val);
+
     s->config_reg = val;
+    s->decode_config_addr(s, val, &s->config_addr);
 }
 
 static uint32_t pci_host_config_readl(void *opaque, target_phys_addr_t addr)
@@ -131,7 +126,9 @@  static void pci_host_config_writel_noswap(void *opaque,
 
     PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
                 __func__, addr, val);
+
     s->config_reg = val;
+    s->decode_config_addr(s, val, &s->config_addr);
 }
 
 static uint32_t pci_host_config_readl_noswap(void *opaque,
@@ -169,7 +166,9 @@  static void pci_host_config_writel_ioport(void *opaque,
     PCIHostState *s = opaque;
 
     PCI_DPRINTF("%s addr %"PRIx32 " val %"PRIx32"\n", __func__, addr, val);
+
     s->config_reg = val;
+    s->decode_config_addr(s, val, &s->config_addr);
 }
 
 static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t addr)
@@ -190,7 +189,7 @@  void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s)
 #define PCI_ADDR_T      target_phys_addr_t
 #define PCI_HOST_SUFFIX _mmio
 
-#include "pci_host_template.h"
+#include "pci_host_template_all.h"
 
 static CPUWriteMemoryFunc * const pci_host_data_write_mmio[] = {
     pci_host_data_writeb_mmio,
@@ -217,7 +216,7 @@  int pci_host_data_register_mmio(PCIHostState *s)
 #define PCI_ADDR_T      uint32_t
 #define PCI_HOST_SUFFIX _ioport
 
-#include "pci_host_template.h"
+#include "pci_host_template_all.h"
 
 void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s)
 {
@@ -228,3 +227,32 @@  void pci_host_data_register_ioport(pio_addr_t ioport, PCIHostState *s)
     register_ioport_read(ioport, 4, 2, pci_host_data_readw_ioport, s);
     register_ioport_read(ioport, 4, 4, pci_host_data_readl_ioport, s);
 }
+
+/* This implements the default x86 way of interpreting a PCI address
+ *
+ * PCI address
+ * bit 16 - 24: bus number
+ * bit 11 - 15: slot number
+ * bit  8 - 10: function number
+ * bit  0 -  7: offset in configuration space of a given pci device
+ */
+
+void pci_host_decode_config_addr(PCIHostState *s, uint32_t config_reg,
+                                 PCIConfigAddress *decoded)
+{
+    uint32_t devfn;
+
+    decoded->addr.domain = s->bus;
+    decoded->addr.bus = (config_reg >> 16) & 0xff;
+    devfn = config_reg >> 8;
+    decoded->addr.slot = (config_reg >> 11) & 0x1f;
+    decoded->addr.fn = (config_reg >> 8) & 0x7;
+    decoded->offset = config_reg & 0xff;
+    decoded->addr_mask = 3;
+    decoded->valid = (config_reg & (1u << 31)) ? true : false;
+}
+
+void pci_host_init(PCIHostState *s)
+{
+    s->decode_config_addr = pci_host_decode_config_addr;
+}
diff --git a/hw/pci_host.h b/hw/pci_host.h
index a006687..064ad96 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,14 +30,30 @@ 
 
 #include "sysbus.h"
 
+/* for config space access */
+typedef struct PCIConfigAddress {
+    PCIAddress addr;
+    uint32_t addr_mask;
+    uint16_t offset;
+    bool valid;
+} PCIConfigAddress;
+
+typedef void (*pci_config_addr_fn)(PCIHostState *s, uint32_t config_reg,
+                                   PCIConfigAddress *conf);
+
 struct PCIHostState {
     SysBusDevice busdev;
+    pci_config_addr_fn decode_config_addr;
+    PCIConfigAddress config_addr;
     uint32_t config_reg;
     PCIBus *bus;
 };
 
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
+void pci_host_init(PCIHostState *s);
+void pci_host_decode_config_addr(PCIHostState *s, uint32_t config_reg,
+                                PCIConfigAddress *decoded);
 
 /* for mmio */
 int pci_host_conf_register_mmio(PCIHostState *s);
diff --git a/hw/pci_host_template.h b/hw/pci_host_template.h
index 11e6c88..bbc9694 100644
--- a/hw/pci_host_template.h
+++ b/hw/pci_host_template.h
@@ -25,85 +25,49 @@ 
 /* Worker routines for a PCI host controller that uses an {address,data}
    register pair to access PCI configuration space.  */
 
-static void glue(pci_host_data_writeb, PCI_HOST_SUFFIX)(
+static void glue(glue(pci_host_data_write, PCI_HOST_BWL), PCI_HOST_SUFFIX)(
     void* opaque, PCI_ADDR_T addr, uint32_t val)
 {
     PCIHostState *s = opaque;
+    PCIConfigAddress *config_reg = &s->config_addr;
+    int offset = config_reg->offset;
 
-    PCI_DPRINTF("writeb addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg | (addr & 3), val, 1);
-}
-
-static void glue(pci_host_data_writew, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr, uint32_t val)
-{
-    PCIHostState *s = opaque;
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap16(val);
+#if defined(TARGET_WORDS_BIGENDIAN) && (PCI_HOST_BITS > 8)
+    val = glue(bswap, PCI_HOST_BITS)(val);
 #endif
-    PCI_DPRINTF("writew addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg | (addr & 3), val, 2);
-}
 
-static void glue(pci_host_data_writel, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr, uint32_t val)
-{
-    PCIHostState *s = opaque;
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap32(val);
-#endif
-    PCI_DPRINTF("writel addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    if (s->config_reg & (1u << 31))
-        pci_data_write(s->bus, s->config_reg, val, 4);
+    offset |= (addr & config_reg->addr_mask);
+    PCI_DPRINTF("%s: addr " TARGET_FMT_plx " val %x\n",
+                __func__, (target_phys_addr_t)addr, val);
+    if (config_reg->valid) {
+        pci_data_write(s->bus,
+                       pci_addr_to_config_reg(&config_reg->addr, offset), val,
+                       sizeof(glue(glue(uint, PCI_HOST_BITS), _t)));
+    }
 }
 
-static uint32_t glue(pci_host_data_readb, PCI_HOST_SUFFIX)(
+static uint32_t glue(glue(pci_host_data_read, PCI_HOST_BWL), PCI_HOST_SUFFIX)(
     void* opaque, PCI_ADDR_T addr)
 {
     PCIHostState *s = opaque;
+    PCIConfigAddress *config_reg = &s->config_addr;
+    int offset = config_reg->offset;
     uint32_t val;
 
-    if (!(s->config_reg & (1 << 31)))
-        return 0xff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 1);
-    PCI_DPRINTF("readb addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-    return val;
-}
+    if (!config_reg->valid) {
+        return (1ULL << PCI_HOST_BITS) - 1;
+    }
 
-static uint32_t glue(pci_host_data_readw, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr)
-{
-    PCIHostState *s = opaque;
-    uint32_t val;
-    if (!(s->config_reg & (1 << 31)))
-        return 0xffff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 2);
-    PCI_DPRINTF("readw addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap16(val);
-#endif
-    return val;
-}
+    offset |= (addr & config_reg->addr_mask);
+    val = pci_data_read(s->bus,
+                        pci_addr_to_config_reg(&config_reg->addr, offset),
+                        sizeof(glue(glue(uint, PCI_HOST_BITS), _t)));
+    PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %x\n",
+                __func__, (target_phys_addr_t)addr, val);
 
-static uint32_t glue(pci_host_data_readl, PCI_HOST_SUFFIX)(
-    void* opaque, PCI_ADDR_T addr)
-{
-    PCIHostState *s = opaque;
-    uint32_t val;
-    if (!(s->config_reg & (1 << 31)))
-        return 0xffffffff;
-    val = pci_data_read(s->bus, s->config_reg | (addr & 3), 4);
-    PCI_DPRINTF("readl addr " TARGET_FMT_plx " val %x\n",
-                (target_phys_addr_t)addr, val);
-#ifdef TARGET_WORDS_BIGENDIAN
-    val = bswap32(val);
+#if defined(TARGET_WORDS_BIGENDIAN) && (PCI_HOST_BITS > 8)
+    val = glue(bswap, PCI_HOST_BITS)(val);
 #endif
+
     return val;
 }
diff --git a/hw/pci_host_template_all.h b/hw/pci_host_template_all.h
new file mode 100644
index 0000000..74b3e84
--- /dev/null
+++ b/hw/pci_host_template_all.h
@@ -0,0 +1,23 @@ 
+#define PCI_HOST_BWL    b
+#define PCI_HOST_BITS   8
+
+#include "pci_host_template.h"
+
+#undef PCI_HOST_BWL
+#undef PCI_HOST_BITS
+
+#define PCI_HOST_BWL    w
+#define PCI_HOST_BITS   16
+
+#include "pci_host_template.h"
+
+#undef PCI_HOST_BWL
+#undef PCI_HOST_BITS
+
+#define PCI_HOST_BWL    l
+#define PCI_HOST_BITS   32
+
+#include "pci_host_template.h"
+
+#undef PCI_HOST_BWL
+#undef PCI_HOST_BITS
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index cd12212..2bdf03b 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -223,6 +223,7 @@  PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
 
     dev = qdev_create(NULL, "i440FX-pcihost");
     s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
+    pci_host_init(s);
     b = pci_bus_new(&s->busdev.qdev, NULL, 0);
     s->bus = b;
     qdev_init_nofail(dev);
diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
index 2d00b61..dd8e290 100644
--- a/hw/ppc4xx_pci.c
+++ b/hw/ppc4xx_pci.c
@@ -357,6 +357,7 @@  PCIBus *ppc4xx_pci_init(CPUState *env, qemu_irq pci_irqs[4],
 
     controller = qemu_mallocz(sizeof(PPC4xxPCIState));
 
+    pci_host_init(&controller->pci_state);
     controller->pci_state.bus = pci_register_bus(NULL, "pci",
                                                  ppc4xx_pci_set_irq,
                                                  ppc4xx_pci_map_irq,
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index a72fb86..5914183 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -278,6 +278,7 @@  PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
 
     controller = qemu_mallocz(sizeof(PPCE500PCIState));
 
+    pci_host_init(&controller->pci_state);
     controller->pci_state.bus = pci_register_bus(NULL, "pci",
                                                  mpc85xx_pci_set_irq,
                                                  mpc85xx_pci_map_irq,
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 3ae4e7a..fdb9401 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -84,6 +84,7 @@  static int pci_unin_main_init_device(SysBusDevice *dev)
     /* Uninorth main bus */
     s = FROM_SYSBUS(UNINState, dev);
 
+    pci_host_init(&s->host_state);
     pci_mem_config = pci_host_conf_register_mmio(&s->host_state);
     pci_mem_data = pci_host_data_register_mmio(&s->host_state);
     sysbus_init_mmio(dev, 0x1000, pci_mem_config);