WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH 1/5] pciback: user-space PCI quirks

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH 1/5] pciback: user-space PCI quirks
From: Chris <hap10@xxxxxxxxxxxxxx>
Date: Mon, 17 Jul 2006 15:15:19 -0400
Delivery-date: Mon, 17 Jul 2006 12:16:15 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5.0.4 (X11/20060614)
This patch, in combination with the a following patch to xend, allows
per-device configuration for fine-grained control over PCI configuration
space writes, with a goal that was also previously described by Ryan:

"Permissive mode should only be used as a fall back for unknown devices.
I think the correct solution for dealing with these device-specific
configuration space registers is to identify them and add the
device-specific fields to the overlay. This patch adds a special
configuration space handler for network cards based on the tg3 linux
network device driver. This handler should allow for reads/writes to all
of the configuration space registers that the tg3 driver requires."

This patch attempts to address concerns with Ryan's original submission
by moving policy from the dom0 kernel into dom0 user-space.  As new
quirky devices emerge they can be incorporated into the user-space
policy.  An added benefit is that changes to the policy are effective
for domains created after the changes are written (no need rebuild the
hypervisor or restart xend).

Signed-off-by: Chris Bookholt <hap10@xxxxxxxxxxxxxx>
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/Makefile
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/Makefile Wed Jul 12 16:34:39 
2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/Makefile Thu Jul 13 13:38:57 
2006 -0400
@@ -4,7 +4,8 @@ pciback-y += conf_space.o conf_space_hea
 pciback-y += conf_space.o conf_space_header.o \
             conf_space_capability.o \
             conf_space_capability_vpd.o \
-            conf_space_capability_pm.o
+            conf_space_capability_pm.o \
+             conf_space_quirks.o
 pciback-$(CONFIG_XEN_PCIDEV_BACKEND_VPCI) += vpci.o
 pciback-$(CONFIG_XEN_PCIDEV_BACKEND_PASS) += passthrough.o
 
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c     Wed Jul 12 
16:34:39 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.c     Thu Jul 13 
13:38:57 2006 -0400
@@ -13,6 +13,7 @@
 #include <linux/pci.h>
 #include "pciback.h"
 #include "conf_space.h"
+#include "conf_space_quirks.h"
 
 static int permissive = 0;
 module_param(permissive, bool, 0644);
@@ -81,7 +82,7 @@ static int conf_space_write(struct pci_d
        case 4:
                if (field->u.dw.write)
                        ret = field->u.dw.write(dev, offset, value,
-                                               entry->data);
+                                               entry->data);
                break;
        }
        return ret;
@@ -261,36 +262,56 @@ int pciback_config_write(struct pci_dev 
                        switch (size) {
                        case 1:
                                err = pci_write_config_byte(dev, offset,
-                                                           (u8)value);
+                                                           (u8) value);
                                break;
                        case 2:
                                err = pci_write_config_word(dev, offset,
-                                                           (u16)value);
+                                                           (u16) value);
                                break;
                        case 4:
                                err = pci_write_config_dword(dev, offset,
-                                                            (u32)value);
+                                                            (u32) value);
                                break;
                        }
                } else if (!dev_data->warned_on_write) {
                        dev_data->warned_on_write = 1;
-                       dev_warn(&dev->dev, "Driver wrote to a read-only "
-                                "configuration space field!\n");
-                       dev_warn(&dev->dev, "Write at offset 0x%x size %d\n",
-                               offset, size);
-                       dev_warn(&dev->dev, "This may be harmless, but if\n");
-                       dev_warn(&dev->dev, "you have problems with your "
-                                "device:\n");
-                       dev_warn(&dev->dev, "1) see the permissive "
-                                "attribute in sysfs.\n");
-                       dev_warn(&dev->dev, "2) report problems to the "
-                                "xen-devel mailing list along\n");
-                       dev_warn(&dev->dev, "   with details of your device "
-                                "obtained from lspci.\n");
+                       dev_warn(&dev->dev, "Driver tried to write to a "
+                                "read-only configuration space field at offset 
"
+                                "0x%x, size %d. This may be harmless, but if "
+                                "you have problems with your device:\n"
+                                "1) see permissive attribute in sysfs\n"
+                                "2) report problems to the xen-devel "
+                                "mailing list along with details of your "
+                                "device obtained from lspci.\n", offset, size);
                }
        }
 
        return pcibios_err_to_errno(err);
+}
+
+void pciback_config_free_dyn_fields(struct pci_dev *dev)
+{
+       struct pciback_dev_data *dev_data = pci_get_drvdata(dev);
+       struct config_field_entry *cfg_entry, *t;
+       struct config_field *field;
+
+       dev_dbg(&dev->dev,
+               "free-ing dynamically allocated virtual configuration space 
fields\n");
+
+       list_for_each_entry_safe(cfg_entry, t, &dev_data->config_fields, list) {
+               field = cfg_entry->field;
+
+               if (field->clean) {
+                       field->clean(field);
+
+                       if (cfg_entry->data)
+                               kfree(cfg_entry->data);
+
+                       list_del(&cfg_entry->list);
+                       kfree(cfg_entry);
+               }
+
+       }
 }
 
 void pciback_config_reset_dev(struct pci_dev *dev)
@@ -337,6 +358,10 @@ int pciback_config_add_field_offset(stru
        struct pciback_dev_data *dev_data = pci_get_drvdata(dev);
        struct config_field_entry *cfg_entry;
        void *tmp;
+
+       /* silently ignore duplicate fields */
+       if (pciback_field_is_dup(dev, field->offset))
+               goto out;
 
        cfg_entry = kmalloc(sizeof(*cfg_entry), GFP_KERNEL);
        if (!cfg_entry) {
@@ -388,6 +413,10 @@ int pciback_config_init_dev(struct pci_d
                goto out;
 
        err = pciback_config_capability_add_fields(dev);
+       if (err)
+               goto out;
+
+       err = pciback_config_quirks_init(dev);
 
       out:
        return err;
@@ -395,9 +424,5 @@ int pciback_config_init_dev(struct pci_d
 
 int pciback_config_init(void)
 {
-       int err;
-
-       err = pciback_config_capability_init();
-
-       return err;
-}
+       return pciback_config_capability_init();
+}
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h     Wed Jul 12 
16:34:39 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space.h     Thu Jul 13 
13:38:57 2006 -0400
@@ -33,11 +33,13 @@ typedef int (*conf_byte_read) (struct pc
  * values.
  */
 struct config_field {
-       unsigned int     offset;
-       unsigned int     size;
-       conf_field_init  init;
+       unsigned int offset;
+       unsigned int size;
+       unsigned int mask;
+       conf_field_init init;
        conf_field_reset reset;
-       conf_field_free  release;
+       conf_field_free release;
+       void (*clean) (struct config_field * field);
        union {
                struct {
                        conf_dword_write write;
@@ -52,6 +54,7 @@ struct config_field {
                        conf_byte_read read;
                } b;
        } u;
+       struct list_head list;
 };
 
 struct config_field_entry {
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c       Wed Jul 12 
16:34:39 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pci_stub.c       Thu Jul 13 
13:38:57 2006 -0400
@@ -1,7 +1,8 @@
 /*
  * PCI Stub Driver - Grabs devices in backend to be exported later
  *
- *   Author: Ryan Wilson <hap9@xxxxxxxxxxxxxx>
+ * Ryan Wilson <hap9@xxxxxxxxxxxxxx>
+ * Chris Bookholt <hap10@xxxxxxxxxxxxxx>
  */
 #include <linux/module.h>
 #include <linux/init.h>
@@ -10,6 +11,8 @@
 #include <linux/kref.h>
 #include <asm/atomic.h>
 #include "pciback.h"
+#include "conf_space.h"
+#include "conf_space_quirks.h"
 
 static char *pci_devs_to_hide = NULL;
 module_param_named(hide, pci_devs_to_hide, charp, 0444);
@@ -31,6 +34,7 @@ struct pcistub_device {
        struct pci_dev *dev;
        struct pciback_device *pdev;    /* non-NULL if struct pci_dev is in use 
*/
 };
+
 /* Access to pcistub_devices & seized_devices lists and the initialize_devices
  * flag must be locked with pcistub_devices_lock
  */
@@ -76,6 +80,7 @@ static void pcistub_device_release(struc
 
        /* Clean-up the device */
        pciback_reset_device(psdev->dev);
+       pciback_config_free_dyn_fields(psdev->dev);
        pciback_config_free_dev(psdev->dev);
        kfree(pci_get_drvdata(psdev->dev));
        pci_set_drvdata(psdev->dev, NULL);
@@ -93,6 +98,32 @@ static inline void pcistub_device_put(st
 static inline void pcistub_device_put(struct pcistub_device *psdev)
 {
        kref_put(&psdev->kref, pcistub_device_release);
+}
+
+static struct pcistub_device *pcistub_device_find(int domain, int bus,
+                                                 int slot, int func)
+{
+       struct pcistub_device *psdev = NULL;
+       unsigned long flags;
+
+       spin_lock_irqsave(&pcistub_devices_lock, flags);
+
+       list_for_each_entry(psdev, &pcistub_devices, dev_list) {
+               if (psdev->dev != NULL
+                   && domain == pci_domain_nr(psdev->dev->bus)
+                   && bus == psdev->dev->bus->number
+                   && PCI_DEVFN(slot, func) == psdev->dev->devfn) {
+                       pcistub_device_get(psdev);
+                       goto out;
+               }
+       }
+
+       /* didn't find it */
+       psdev = NULL;
+
+      out:
+       spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+       return psdev;
 }
 
 static struct pci_dev *pcistub_device_get_pci_dev(struct pciback_device *pdev,
@@ -180,6 +211,7 @@ void pcistub_put_pci_dev(struct pci_dev 
         * (so it's ready for the next domain)
         */
        pciback_reset_device(found_psdev->dev);
+       pciback_config_free_dyn_fields(found_psdev->dev);
        pciback_config_reset_dev(found_psdev->dev);
 
        spin_lock_irqsave(&found_psdev->lock, flags);
@@ -392,6 +424,8 @@ static void pcistub_remove(struct pci_de
 
        spin_lock_irqsave(&pcistub_devices_lock, flags);
 
+       pciback_config_quirk_release(dev);
+
        list_for_each_entry(psdev, &pcistub_devices, dev_list) {
                if (psdev->dev == dev) {
                        found_psdev = psdev;
@@ -471,6 +505,19 @@ static inline int str_to_slot(const char
        return -EINVAL;
 }
 
+static inline int str_to_quirk(const char *buf, int *domain, int *bus, int
+                              *slot, int *func, int *reg, int *size, int *mask)
+{
+       int err;
+
+       err =
+           sscanf(buf, " %04x:%02x:%02x.%1x-%08x:%1x:%08x", domain, bus, slot,
+                  func, reg, size, mask);
+       if (err == 7)
+               return 0;
+       return -EINVAL;
+}
+
 static int pcistub_device_id_add(int domain, int bus, int slot, int func)
 {
        struct pcistub_device_id *pci_dev_id;
@@ -523,6 +570,46 @@ static int pcistub_device_id_remove(int 
        return err;
 }
 
+static int pcistub_reg_add(int domain, int bus, int slot, int func, int reg,
+                          int size, int mask)
+{
+       int err = 0;
+       struct pcistub_device *psdev;
+       struct pci_dev *dev;
+       struct config_field *field;
+
+       psdev = pcistub_device_find(domain, bus, slot, func);
+       if (!psdev || !psdev->dev) {
+               err = -ENODEV;
+               goto out;
+       }
+       dev = psdev->dev;
+
+       /* check for duplicate field */
+       if (pciback_field_is_dup(dev, reg))
+               goto out;
+
+       field = kzalloc(sizeof(*field), GFP_ATOMIC);
+       if (!field) {
+               err = -ENOMEM;
+               goto out;
+       }
+
+       field->offset = reg;
+       field->size = size;
+       field->mask = mask;
+       field->init = NULL;
+       field->reset = NULL;
+       field->release = NULL;
+       field->clean = pciback_config_field_free;
+
+       err = pciback_config_quirks_add_field(dev, field);
+       if (err)
+               kfree(field);
+      out:
+       return err;
+}
+
 static ssize_t pcistub_slot_add(struct device_driver *drv, const char *buf,
                                size_t count)
 {
@@ -586,6 +673,71 @@ static ssize_t pcistub_slot_show(struct 
 }
 
 DRIVER_ATTR(slots, S_IRUSR, pcistub_slot_show, NULL);
+
+static ssize_t pcistub_quirk_add(struct device_driver *drv, const char *buf,
+                                size_t count)
+{
+       int domain, bus, slot, func, reg, size, mask;
+       int err;
+
+       err = str_to_quirk(buf, &domain, &bus, &slot, &func, &reg, &size,
+                          &mask);
+       if (err)
+               goto out;
+
+       err = pcistub_reg_add(domain, bus, slot, func, reg, size, mask);
+
+      out:
+       if (!err)
+               err = count;
+       return err;
+}
+
+static ssize_t pcistub_quirk_show(struct device_driver *drv, char *buf)
+{
+       int count = 0;
+       unsigned long flags;
+       extern struct list_head pciback_quirks;
+       struct pciback_config_quirk *quirk;
+       struct pciback_dev_data *dev_data;
+       struct config_field *field;
+       struct config_field_entry *cfg_entry;
+
+       spin_lock_irqsave(&device_ids_lock, flags);
+       list_for_each_entry(quirk, &pciback_quirks, quirks_list) {
+               if (count >= PAGE_SIZE)
+                       goto out;
+
+               count += scnprintf(buf + count, PAGE_SIZE - count,
+                                  "%02x:%02x.%01x\n\t%04x:%04x:%04x:%04x\n",
+                                  quirk->pdev->bus->number,
+                                  PCI_SLOT(quirk->pdev->devfn),
+                                  PCI_FUNC(quirk->pdev->devfn),
+                                  quirk->devid.vendor, quirk->devid.device,
+                                  quirk->devid.subvendor,
+                                  quirk->devid.subdevice);
+
+               dev_data = pci_get_drvdata(quirk->pdev);
+
+               list_for_each_entry(cfg_entry, &dev_data->config_fields, list) {
+                       field = cfg_entry->field;
+                       if (count >= PAGE_SIZE)
+                               goto out;
+
+                       count += scnprintf(buf + count, PAGE_SIZE -
+                                          count, "\t\t%08x:%01x:%08x\n",
+                                          field->offset, field->size,
+                                          field->mask);
+               }
+       }
+
+      out:
+       spin_unlock_irqrestore(&device_ids_lock, flags);
+
+       return count;
+}
+
+DRIVER_ATTR(quirks, S_IRUSR | S_IWUSR, pcistub_quirk_show, pcistub_quirk_add);
 
 static int __init pcistub_init(void)
 {
@@ -631,6 +783,7 @@ static int __init pcistub_init(void)
        driver_create_file(&pciback_pci_driver.driver,
                           &driver_attr_remove_slot);
        driver_create_file(&pciback_pci_driver.driver, &driver_attr_slots);
+       driver_create_file(&pciback_pci_driver.driver, &driver_attr_quirks);
 
       out:
        return err;
@@ -680,6 +833,7 @@ static void __exit pciback_cleanup(void)
        driver_remove_file(&pciback_pci_driver.driver,
                           &driver_attr_remove_slot);
        driver_remove_file(&pciback_pci_driver.driver, &driver_attr_slots);
+       driver_remove_file(&pciback_pci_driver.driver, &driver_attr_quirks);
 
        pci_unregister_driver(&pciback_pci_driver);
 }
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h        Wed Jul 12 
16:34:39 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/pciback.h        Thu Jul 13 
13:38:57 2006 -0400
@@ -61,6 +61,7 @@ void pciback_reset_device(struct pci_dev
 /* Access a virtual configuration space for a PCI device */
 int pciback_config_init(void);
 int pciback_config_init_dev(struct pci_dev *dev);
+void pciback_config_free_dyn_fields(struct pci_dev *dev);
 void pciback_config_reset_dev(struct pci_dev *dev);
 void pciback_config_free_dev(struct pci_dev *dev);
 int pciback_config_read(struct pci_dev *dev, int offset, int size,
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.c
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.c      Thu Jul 
13 13:38:57 2006 -0400
@@ -0,0 +1,128 @@
+/*
+ * PCI Backend - Handle special overlays for broken devices.
+ *
+ * Author: Ryan Wilson <hap9@xxxxxxxxxxxxxx>
+ * Author: Chris Bookholt <hap10@xxxxxxxxxxxxxx>
+ */
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include "pciback.h"
+#include "conf_space.h"
+#include "conf_space_quirks.h"
+
+LIST_HEAD(pciback_quirks);
+
+struct pciback_config_quirk *pciback_find_quirk(struct pci_dev *dev)
+{
+       struct pciback_config_quirk *tmp_quirk;
+
+       list_for_each_entry(tmp_quirk, &pciback_quirks, quirks_list)
+           if (pci_match_id(&tmp_quirk->devid, dev))
+               goto out;
+       tmp_quirk = NULL;
+       printk(KERN_DEBUG
+              "quirk didn't match any device pciback knows about\n");
+      out:
+       return tmp_quirk;
+}
+
+static inline void register_quirk(struct pciback_config_quirk *quirk)
+{
+       list_add_tail(&quirk->quirks_list, &pciback_quirks);
+}
+
+int pciback_field_is_dup(struct pci_dev *dev, int reg)
+{
+       int ret = 0;
+       struct pciback_dev_data *dev_data = pci_get_drvdata(dev);
+       struct config_field *field;
+       struct config_field_entry *cfg_entry;
+
+       list_for_each_entry(cfg_entry, &dev_data->config_fields, list) {
+               field = cfg_entry->field;
+               if (field->offset == reg) {
+                       ret = 1;
+                       break;
+               }
+       }
+       return ret;
+}
+
+int pciback_config_quirks_add_field(struct pci_dev *dev, struct config_field
+                                   *field)
+{
+       int err = 0;
+
+       switch (field->size) {
+       case 1:
+               field->u.b.read = pciback_read_config_byte;
+               field->u.b.write = pciback_write_config_byte;
+               break;
+       case 2:
+               field->u.w.read = pciback_read_config_word;
+               field->u.w.write = pciback_write_config_word;
+               break;
+       case 4:
+               field->u.dw.read = pciback_read_config_dword;
+               field->u.dw.write = pciback_write_config_dword;
+               break;
+       default:
+               err = -EINVAL;
+               goto out;
+       }
+
+       pciback_config_add_field(dev, field);
+
+      out:
+       return err;
+}
+
+int pciback_config_quirks_init(struct pci_dev *dev)
+{
+       struct pciback_config_quirk *quirk;
+       int ret = 0;
+
+       quirk = kzalloc(sizeof(*quirk), GFP_ATOMIC);
+       if (!quirk) {
+               ret = -ENOMEM;
+               goto out;
+       }
+
+       quirk->devid.vendor = dev->vendor;
+       quirk->devid.device = dev->device;
+       quirk->devid.subvendor = dev->subsystem_vendor;
+       quirk->devid.subdevice = dev->subsystem_device;
+       quirk->devid.class = 0;
+       quirk->devid.class_mask = 0;
+       quirk->devid.driver_data = 0UL;
+
+       quirk->pdev = dev;
+
+       register_quirk(quirk);
+      out:
+       return ret;
+}
+
+void pciback_config_field_free(struct config_field *field)
+{
+       kfree(field);
+}
+
+int pciback_config_quirk_release(struct pci_dev *dev)
+{
+       struct pciback_config_quirk *quirk;
+       int ret = 0;
+
+       quirk = pciback_find_quirk(dev);
+       if (!quirk) {
+               ret = -ENXIO;
+               goto out;
+       }
+
+       list_del(&quirk->quirks_list);
+       kfree(quirk);
+
+      out:
+       return ret;
+}
diff -r b20580cf7fc1 -r 94a0a82c91d1 
linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.h
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_quirks.h      Thu Jul 
13 13:38:57 2006 -0400
@@ -0,0 +1,35 @@
+/*
+ * PCI Backend - Data structures for special overlays for broken devices.
+ *
+ * Ryan Wilson <hap9@xxxxxxxxxxxxxx>
+ * Chris Bookholt <hap10@xxxxxxxxxxxxxx>
+ */
+
+#ifndef __XEN_PCIBACK_CONF_SPACE_QUIRKS_H__
+#define __XEN_PCIBACK_CONF_SPACE_QUIRKS_H__
+
+#include <linux/pci.h>
+#include <linux/list.h>
+
+struct pciback_config_quirk {
+       struct list_head quirks_list;
+       struct pci_device_id devid;
+       struct pci_dev *pdev;
+};
+
+struct pciback_config_quirk *pciback_find_quirk(struct pci_dev *dev);
+
+int pciback_config_quirks_add_field(struct pci_dev *dev, struct config_field
+                                   *field);
+
+int pciback_config_quirks_remove_field(struct pci_dev *dev, int reg);
+
+int pciback_config_quirks_init(struct pci_dev *dev);
+
+void pciback_config_field_free(struct config_field *field);
+
+int pciback_config_quirk_release(struct pci_dev *dev);
+
+int pciback_field_is_dup(struct pci_dev *dev, int reg);
+
+#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH 1/5] pciback: user-space PCI quirks, Chris <=