PCI device locking and synchronization
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 4 Apr 2014 23:14:24 +0000 (16:14 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 4 Apr 2014 23:14:24 +0000 (16:14 -0700)
Adds some limited concurrency support to PCI operations.  The main list
of PCI devices is built at boot and is read-only, so we don't
(currently) need to worry much.

kern/arch/x86/msi.c
kern/arch/x86/pci.c
kern/arch/x86/pci.h

index e897776..2188374 100644 (file)
@@ -150,24 +150,27 @@ int pci_msi_enable(struct pci_device *p, uint64_t vec)
 {
        unsigned int c, f, datao;
 
+       spin_lock_irqsave(&p->lock);
        if (p->msix_ready) {
                printk("MSI: MSI-X is already enabled, aborting\n");
+               spin_unlock_irqsave(&p->lock);
                return -1;
        }
        if (p->msi_ready) {
                /* only allowing one enable of MSI per device (not supporting multiple
                 * vectors) */
                printk("MSI: MSI is already enabled, aborting\n");
+               spin_unlock_irqsave(&p->lock);
                return -1;
        }
        p->msi_ready = TRUE;
 
-       /* Get the offset of the MSI capability
-        * in the function's config space.
-        */
+       /* Get the offset of the MSI capability in the function's config space. */
        c = msicap(p);
-       if(c == 0)
+       if (!c) {
+               spin_unlock_irqsave(&p->lock);
                return -1;
+       }
 
        /* read it, clear out the Mmesgmsk bits. 
         * This means that there will be no multiple
@@ -175,8 +178,10 @@ int pci_msi_enable(struct pci_device *p, uint64_t vec)
         */
        f = pcidev_read16(p, c + 2) & ~Mmesgmsk;
 
-       if (msi_blacklist(p) != 0)
+       if (msi_blacklist(p) != 0) {
+               spin_unlock_irqsave(&p->lock);
                return -1;
+       }
 
        /* Data begins at 8 bytes in. */
        datao = 8;
@@ -211,16 +216,17 @@ int pci_msi_enable(struct pci_device *p, uint64_t vec)
         * we can mask on non-Vmask-supported HW. */
        printd("write @ %d %04lx\n",c + 2, f);
        pcidev_write16(p, c + 2, f);
+       spin_unlock_irqsave(&p->lock);
        return 0;
 }
 
-static void msix_mask_entry(struct msix_entry *entry)
+static void __msix_mask_entry(struct msix_entry *entry)
 {
        uintptr_t reg = (uintptr_t)&entry->vector;
        write_mmreg32(reg, read_mmreg32(reg) | 0x1);
 }
 
-static void msix_unmask_entry(struct msix_entry *entry)
+static void __msix_unmask_entry(struct msix_entry *entry)
 {
        uintptr_t reg = (uintptr_t)&entry->vector;
        write_mmreg32(reg, read_mmreg32(reg) & ~0x1);
@@ -250,8 +256,10 @@ static uintptr_t msix_get_capbar_paddr(struct pci_device *p, int offset)
 
 /* One time initialization of MSI-X for a PCI device.  -1 on error.  Otherwise,
  * the device will be ready to assign/route MSI-X entries/vectors.  All vectors
- * are masked, but the overall MSI-X function is unmasked. */
-static int pci_msix_init(struct pci_device *p)
+ * are masked, but the overall MSI-X function is unmasked.
+ *
+ * Hold the pci_device lock. */
+static int __pci_msix_init(struct pci_device *p)
 {
        unsigned int c;
        uint16_t f;
@@ -275,8 +283,11 @@ static int pci_msix_init(struct pci_device *p)
 
        p->msix_tbl_paddr = msix_get_capbar_paddr(p, c + 4);
        p->msix_pba_paddr = msix_get_capbar_paddr(p, c + 8);
-       if (!p->msix_tbl_paddr || !p->msix_pba_paddr)
+       if (!p->msix_tbl_paddr || !p->msix_pba_paddr) {
+               printk("MSI-X: Missing a tbl (%p) or PBA (%p) paddr!\n",
+                      p->msix_tbl_paddr, p->msix_pba_paddr);
                return -1;
+       }
        p->msix_nr_vec = (f & Msixtblsize) + 1;
        p->msix_tbl_vaddr = vmap_pmem_nocache(p->msix_tbl_paddr, p->msix_nr_vec *
                                              sizeof(struct msix_entry));
@@ -295,7 +306,7 @@ static int pci_msix_init(struct pci_device *p)
        /* they should all be masked already, but just in case */
        entry = (struct msix_entry*)p->msix_tbl_vaddr;
        for (int i = 0; i < p->msix_nr_vec; i++, entry++) {
-               msix_mask_entry(entry);
+               __msix_mask_entry(entry);
        }
        /* unmask the device, now that all the vectors are masked */
        f &= ~Msixmask;
@@ -314,10 +325,12 @@ struct msix_irq_vector *pci_msix_enable(struct pci_device *p, uint64_t vec)
        struct msix_irq_vector *linkage;
        unsigned int c, datao;
 
-       /* TODO: sync protection */
+       spin_lock_irqsave(&p->lock);
        if (!p->msix_ready) {
-               if (pci_msix_init(p) < 0)
+               if (__pci_msix_init(p) < 0) {
+                       spin_unlock_irqsave(&p->lock);
                        return 0;
+               }
                p->msix_ready = TRUE;
        }
        /* find an unused slot (no apic_vector assigned).  later, we might want to
@@ -326,8 +339,10 @@ struct msix_irq_vector *pci_msix_enable(struct pci_device *p, uint64_t vec)
        for (i = 0; i < p->msix_nr_vec; i++, entry++)
                if (!(read_mmreg32((uintptr_t)&entry->data) & 0xff))
                        break;
-       if (i == p->msix_nr_vec)
+       if (i == p->msix_nr_vec) {
+               spin_unlock_irqsave(&p->lock);
                return 0;
+       }
        linkage = kmalloc(sizeof(struct msix_irq_vector), KMALLOC_WAIT);
        linkage->pcidev = p;
        linkage->entry = entry;
@@ -337,18 +352,20 @@ struct msix_irq_vector *pci_msix_enable(struct pci_device *p, uint64_t vec)
        write_mmreg32((uintptr_t)&entry->data, linkage->data);
        write_mmreg32((uintptr_t)&entry->addr_lo, linkage->addr_lo);
        write_mmreg32((uintptr_t)&entry->addr_hi, linkage->addr_hi);
+       spin_unlock_irqsave(&p->lock);
        return linkage;
 }
 
-/* TODO: should lock in all of these PCI/MSI functions */
 void pci_msi_mask(struct pci_device *p)
 {
        unsigned int c, f;
        c = msicap(p);
        assert(c);
 
+       spin_lock_irqsave(&p->lock);
        f = pcidev_read16(p, c + 2);
        pcidev_write16(p, c + 2, f & ~Msienable);
+       spin_unlock_irqsave(&p->lock);
 }
 
 void pci_msi_unmask(struct pci_device *p)
@@ -357,37 +374,46 @@ void pci_msi_unmask(struct pci_device *p)
        c = msicap(p);
        assert(c);
 
+       spin_lock_irqsave(&p->lock);
        f = pcidev_read16(p, c + 2);
        pcidev_write16(p, c + 2, f | Msienable);
+       spin_unlock_irqsave(&p->lock);
 }
 
-int pci_msi_route(struct pci_device *p, int dest)
+void pci_msi_route(struct pci_device *p, int dest)
 {
        unsigned int c, f;
        c = msicap(p);
        assert(c);
 
+       spin_lock_irqsave(&p->lock);
        /* mask out the old destination, replace with new */
        p->msi_msg_addr_lo &= ~(((1 << 8) - 1) << 12);
        p->msi_msg_addr_lo |= (dest & 0xff) << 12;
        pcidev_write32(p, c + 4, p->msi_msg_addr_lo);
-       return 0;
+       spin_unlock_irqsave(&p->lock);
 }
 
 void pci_msix_mask_vector(struct msix_irq_vector *linkage)
 {
-       msix_mask_entry(linkage->entry);
+       spin_lock_irqsave(&linkage->pcidev->lock);
+       __msix_mask_entry(linkage->entry);
+       spin_unlock_irqsave(&linkage->pcidev->lock);
 }
 
 void pci_msix_unmask_vector(struct msix_irq_vector *linkage)
 {
-       msix_unmask_entry(linkage->entry);
+       spin_lock_irqsave(&linkage->pcidev->lock);
+       __msix_unmask_entry(linkage->entry);
+       spin_unlock_irqsave(&linkage->pcidev->lock);
 }
 
 void pci_msix_route_vector(struct msix_irq_vector *linkage, int dest)
 {
+       spin_lock_irqsave(&linkage->pcidev->lock);
        /* mask out the old destination, replace with new */
        linkage->addr_lo &= ~(((1 << 8) - 1) << 12);
        linkage->addr_lo |= (dest & 0xff) << 12;
        write_mmreg32((uintptr_t)&linkage->entry->addr_lo, linkage->addr_lo);
+       spin_unlock_irqsave(&linkage->pcidev->lock);
 }
index c1518d1..da5fc99 100644 (file)
@@ -23,10 +23,66 @@ static char STD_PCI_DEV[] = "Standard PCI Device";
 static char PCI2PCI[] = "PCI-to-PCI Bridge";
 static char PCI2CARDBUS[] = "PCI-Cardbus Bridge";
 
+/* Gets any old raw bar, with some catches based on type. */
+static uint32_t pci_getbar(struct pci_device *pcidev, unsigned int bar)
+{
+       uint8_t type;
+       if (bar >= MAX_PCI_BAR)
+               panic("Nonexistant bar requested!");
+       type = pcidev_read8(pcidev, PCI_HEADER_REG);
+       type &= ~0x80;  /* drop the MF bit */
+       /* Only types 0 and 1 have BARS */
+       if ((type != 0x00) && (type != 0x01))
+               return 0;
+       /* Only type 0 has BAR2 - BAR5 */
+       if ((bar > 1) && (type != 0x00))
+               return 0;
+       return pcidev_read32(pcidev, PCI_BAR0_STD + bar * PCI_BAR_OFF);
+}
+
+/* Determines if a given bar is IO (o/w, it's mem) */
+static bool pci_is_iobar(uint32_t bar)
+{
+       return bar & PCI_BAR_IO;
+}
+
+static bool pci_is_membar32(uint32_t bar)
+{
+       if (pci_is_iobar(bar))
+               return FALSE;
+       return (bar & PCI_MEMBAR_TYPE) == PCI_MEMBAR_32BIT;
+}
+
+static bool pci_is_membar64(uint32_t bar)
+{
+       if (pci_is_iobar(bar))
+               return FALSE;
+       return (bar & PCI_MEMBAR_TYPE) == PCI_MEMBAR_64BIT;
+}
+
+/* Helper to get the address from a membar.  Check the type beforehand */
+static uint32_t pci_getmembar32(uint32_t bar)
+{
+       uint8_t type = bar & PCI_MEMBAR_TYPE;
+       if (type != PCI_MEMBAR_32BIT) {
+               warn("Unhandled PCI membar type: %02p\n", type >> 1);
+               return 0;
+       }
+       return bar & 0xfffffff0;
+}
+
+/* Helper to get the address from an IObar.  Check the type beforehand */
+static uint32_t pci_getiobar32(uint32_t bar)
+{
+       return bar & 0xfffffffc;
+}
+
 /* memory bars have a little dance you go through to detect what the size of the
  * memory region is.  for 64 bit bars, i'm assuming you only need to do this to
- * the lower part (no device will need > 4GB, right?). */
-uint32_t pci_membar_get_sz(struct pci_device *pcidev, int bar)
+ * the lower part (no device will need > 4GB, right?).
+ *
+ * Hold the dev's lock, or o/w avoid sync issues. */
+static uint32_t __pci_membar_get_sz(struct pci_device *pcidev, int bar)
 {
        /* save the old value, write all 1s, invert, add 1, restore.
         * http://wiki.osdev.org/PCI for details. */
@@ -45,7 +101,7 @@ uint32_t pci_membar_get_sz(struct pci_device *pcidev, int bar)
  * where the base is.  fills results into pcidev.  i don't know if you can have
  * multiple bars with conflicting/different regions (like two separate PIO
  * ranges).  I'm assuming you don't, and will warn if we see one. */
-static void pci_handle_bars(struct pci_device *pcidev)
+static void __pci_handle_bars(struct pci_device *pcidev)
 {
        uint32_t bar_val;
        int max_bars;
@@ -66,7 +122,7 @@ static void pci_handle_bars(struct pci_device *pcidev)
                } else {
                        if (pci_is_membar32(bar_val)) {
                                pcidev->bar[i].mmio_base32 = bar_val & PCI_BAR_MEM_MASK;
-                               pcidev->bar[i].mmio_sz = pci_membar_get_sz(pcidev, i);
+                               pcidev->bar[i].mmio_sz = __pci_membar_get_sz(pcidev, i);
                        } else if (pci_is_membar64(bar_val)) {
                                /* 64 bit, the lower 32 are in this bar, the upper
                                 * are in the next bar */
@@ -76,7 +132,7 @@ static void pci_handle_bars(struct pci_device *pcidev)
                                /* note we don't check for IO or memsize.  the entire next bar
                                 * is supposed to be for the upper 32 bits. */
                                pcidev->bar[i].mmio_base64 |= (uint64_t)bar_val << 32;
-                               pcidev->bar[i].mmio_sz = pci_membar_get_sz(pcidev, i);
+                               pcidev->bar[i].mmio_sz = __pci_membar_get_sz(pcidev, i);
                                i++;
                        }
                }
@@ -86,7 +142,7 @@ static void pci_handle_bars(struct pci_device *pcidev)
        }
 }
 
-static void pci_parse_caps(struct pci_device *pcidev)
+static void __pci_parse_caps(struct pci_device *pcidev)
 {
        uint32_t cap_off;       /* not sure if this can be extended from u8 */
        uint8_t cap_id;
@@ -124,7 +180,8 @@ static void pci_parse_caps(struct pci_device *pcidev)
 
 /* Scans the PCI bus.  Won't actually work for anything other than bus 0, til we
  * sort out how to handle bridge devices. */
-void pci_init(void) {
+void pci_init(void)
+{
        uint32_t result = 0;
        uint16_t dev_id, ven_id;
        struct pci_device *pcidev;
@@ -140,6 +197,8 @@ void pci_init(void) {
                                if (ven_id == INVALID_VENDOR_ID) 
                                        break;  /* skip functions too, they won't exist */
                                pcidev = kzmalloc(sizeof(struct pci_device), 0);
+                               /* we don't need to lock it til we post the pcidev to the list*/
+                               spinlock_init_irqsave(&pcidev->lock);
                                pcidev->bus = i;
                                pcidev->dev = j;
                                pcidev->func = k;
@@ -168,8 +227,9 @@ void pci_init(void) {
                                        default:
                                                pcidev->header_type = "Unknown Header Type";
                                }
-                               pci_handle_bars(pcidev);
-                               pci_parse_caps(pcidev);
+                               __pci_handle_bars(pcidev);
+                               __pci_parse_caps(pcidev);
+                               /* we're the only writer at this point in the boot process */
                                STAILQ_INSERT_TAIL(&pci_devices, pcidev, all_dev);
                                #ifdef CONFIG_PCI_VERBOSE
                                pcidev_print_info(pcidev, 4);
@@ -290,72 +350,6 @@ void pcidev_write8(struct pci_device *pcidev, uint32_t offset, uint8_t value)
        pci_write8(pcidev->bus, pcidev->dev, pcidev->func, offset, value);
 }
 
-/* Gets any old raw bar, with some catches based on type. */
-uint32_t pci_getbar(struct pci_device *pcidev, unsigned int bar)
-{
-       uint8_t type;
-       if (bar >= MAX_PCI_BAR)
-               panic("Nonexistant bar requested!");
-       type = pcidev_read8(pcidev, PCI_HEADER_REG);
-       type &= ~0x80;  /* drop the MF bit */
-       /* Only types 0 and 1 have BARS */
-       if ((type != 0x00) && (type != 0x01))
-               return 0;
-       /* Only type 0 has BAR2 - BAR5 */
-       if ((bar > 1) && (type != 0x00))
-               return 0;
-       return pcidev_read32(pcidev, PCI_BAR0_STD + bar * PCI_BAR_OFF);
-}
-
-/* Determines if a given bar is IO (o/w, it's mem) */
-bool pci_is_iobar(uint32_t bar)
-{
-       return bar & PCI_BAR_IO;
-}
-
-bool pci_is_membar32(uint32_t bar)
-{
-       if (pci_is_iobar(bar))
-               return FALSE;
-       return (bar & PCI_MEMBAR_TYPE) == PCI_MEMBAR_32BIT;
-}
-
-bool pci_is_membar64(uint32_t bar)
-{
-       if (pci_is_iobar(bar))
-               return FALSE;
-       return (bar & PCI_MEMBAR_TYPE) == PCI_MEMBAR_64BIT;
-}
-
-/* Helper to get the address from a membar.  Check the type beforehand */
-uint32_t pci_getmembar32(uint32_t bar)
-{
-       uint8_t type = bar & PCI_MEMBAR_TYPE;
-       if (type != PCI_MEMBAR_32BIT) {
-               warn("Unhandled PCI membar type: %02p\n", type >> 1);
-               return 0;
-       }
-       return bar & 0xfffffff0;
-}
-
-/* Helper to get the address from an IObar.  Check the type beforehand */
-uint32_t pci_getiobar32(uint32_t bar)
-{
-       return bar & 0xfffffffc;
-}
-
-/* Helper to get the membar value for BAR index bir */
-uintptr_t pci_get_membar(struct pci_device *pcidev, int bir)
-{
-       if (bir >= pcidev->nr_bars)
-               return 0;
-       if (pci_is_iobar(pcidev->bar[bir].raw_bar))
-               return 0;
-       return (pci_is_membar64(pcidev->bar[bir].raw_bar) ?
-               pcidev->bar[bir].mmio_base64 :
-               pcidev->bar[bir].mmio_base32);
-}
-
 /* Helper to get the class description strings.  Adapted from
  * http://www.pcidatabase.com/reports.php?type=c-header */
 static void pcidev_get_cldesc(struct pci_device *pcidev, char **class,
@@ -463,16 +457,20 @@ void pcidev_print_info(struct pci_device *pcidev, int verbosity)
 
 void pci_set_bus_master(struct pci_device *pcidev)
 {
+       spin_lock_irqsave(&pcidev->lock);
        pcidev_write16(pcidev, PCI_CMD_REG, pcidev_read16(pcidev, PCI_CMD_REG) |
                                            PCI_CMD_BUS_MAS);
+       spin_unlock_irqsave(&pcidev->lock);
 }
 
 void pci_clr_bus_master(struct pci_device *pcidev)
 {
        uint16_t reg;
+       spin_lock_irqsave(&pcidev->lock);
        reg = pcidev_read16(pcidev, PCI_CMD_REG);
        reg &= ~PCI_CMD_BUS_MAS;
        pcidev_write16(pcidev, PCI_CMD_REG, reg);
+       spin_unlock_irqsave(&pcidev->lock);
 }
 
 struct pci_device *pci_match_tbdf(int tbdf)
@@ -491,3 +489,15 @@ struct pci_device *pci_match_tbdf(int tbdf)
        }
        return NULL;
 }
+
+/* Helper to get the membar value for BAR index bir */
+uintptr_t pci_get_membar(struct pci_device *pcidev, int bir)
+{
+       if (bir >= pcidev->nr_bars)
+               return 0;
+       if (pci_is_iobar(pcidev->bar[bir].raw_bar))
+               return 0;
+       return (pci_is_membar64(pcidev->bar[bir].raw_bar) ?
+               pcidev->bar[bir].mmio_base64 :
+               pcidev->bar[bir].mmio_base32);
+}
index 3aed52f..36275ca 100644 (file)
@@ -9,6 +9,7 @@
 
 #include <ros/common.h>
 #include <sys/queue.h>
+#include <atomic.h>
 
 #define pci_debug(...)  printk(__VA_ARGS__)  
 
@@ -316,10 +317,10 @@ struct pci_bar {
        uint32_t                                        mmio_sz;
 };
 
-/* Struct for some meager contents of a PCI device */
 struct pci_device {
        STAILQ_ENTRY(pci_device)        all_dev;        /* list of all devices */
        SLIST_ENTRY(pci_device)         irq_dev;        /* list of all devs off an irq */
+       spinlock_t                                      lock;
        bool                                            in_use;         /* prevent double discovery */
        uint8_t                                         bus;
        uint8_t                                         dev;
@@ -364,6 +365,21 @@ STAILQ_HEAD(pcidev_stailq, pci_device);
 SLIST_HEAD(pcidev_slist, pci_device);
 extern struct pcidev_stailq pci_devices;
 
+/* Sync rules for PCI: once a device is added to the list, it is never removed,
+ * and its read-only fields can be accessed at any time.  There is no need for
+ * refcnts or things like that.
+ *
+ * The device list is built early on when we're single threaded, so I'm not
+ * bothering with locks for that yet.  Append-only, singly-linked-list reads
+ * don't need a lock either.
+ *
+ * Other per-device accesses (like read-modify-writes to config space or MSI
+ * fields) require the device's lock.  If we ever want to unplug, we'll probably
+ * work out an RCU-like scheme for the pci_devices list.
+ *
+ * Note this is in addition to the config space global locking done by every
+ * pci_read or write call. */
+
 void pci_init(void);
 void pcidev_print_info(struct pci_device *pcidev, int verbosity);
 uint32_t pci_config_addr(uint8_t bus, uint8_t dev, uint8_t func, uint32_t reg);
@@ -386,27 +402,18 @@ void pcidev_write16(struct pci_device *pcidev, uint32_t offset, uint16_t value);
 uint8_t pcidev_read8(struct pci_device *pcidev, uint32_t offset);
 void pcidev_write8(struct pci_device *pcidev, uint32_t offset, uint8_t value);
 
-/* BAR helpers, some more helpful than others. */
-uint32_t pci_membar_get_sz(struct pci_device *pcidev, int bar);
-uint32_t pci_getbar(struct pci_device *pcidev, unsigned int bar);
-bool pci_is_iobar(uint32_t bar);
-bool pci_is_membar32(uint32_t bar);
-bool pci_is_membar64(uint32_t bar);
-uint32_t pci_getmembar32(uint32_t bar);
-uint32_t pci_getiobar32(uint32_t bar);
-uintptr_t pci_get_membar(struct pci_device *pcidev, int bir);
-
 /* Other common PCI functions */
 void pci_set_bus_master(struct pci_device *pcidev);
 void pci_clr_bus_master(struct pci_device *pcidev);
 struct pci_device *pci_match_tbdf(int tbdf);
+uintptr_t pci_get_membar(struct pci_device *pcidev, int bir);
 
 /* MSI functions, msi.c */
 int pci_msi_enable(struct pci_device *p, uint64_t vec);
 struct msix_irq_vector *pci_msix_enable(struct pci_device *p, uint64_t vec);
 void pci_msi_mask(struct pci_device *p);
 void pci_msi_unmask(struct pci_device *p);
-int pci_msi_route(struct pci_device *p, int dest);
+void pci_msi_route(struct pci_device *p, int dest);
 void pci_msix_mask_vector(struct msix_irq_vector *linkage);
 void pci_msix_unmask_vector(struct msix_irq_vector *linkage);
 void pci_msix_route_vector(struct msix_irq_vector *linkage, int dest);