From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Philipson Subject: Re: Status of FLR in Xen 4.4 Date: Thu, 26 Sep 2013 13:48:37 -0400 Message-ID: <52447375.60304@citrix.com> References: <52445EC3.9020600@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52445EC3.9020600@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "xen-devel@lists.xen.org" Cc: Matthias , David Vrabel List-Id: xen-devel@lists.xenproject.org On 09/26/2013 12:20 PM, David Vrabel wrote: > On 26/09/13 17:05, Matthias wrote: >> Hi everyone, >> >> I would like to ask what the current status of FLR, or better of FLR >> emulation is in latest Xen and if we can expect better support in the >> future. > > What are these cards, are they multi-function and do they actually > support FLR? Many graphics cards do not. > > I have the following hack to pciback to fallback to a bus reset for > multi-function devices without FLR. Does it help for your use case? > You will need to ensure that all functions are co-assigned to the same > domain. New kernels (e.g. 3.8) have full support for PCI-e and PCI AF FLRs as well as fallback support for D0-D3 and secondary bus resets. This functionality is also in the some of the last 2.6 kernels like 2.6.39. If you are using an older kernel I guess you might need to patch it. Also depending on your hw there might be a specific quirk you need (e.g. the 82599 quirk in pci/quirks.c). Ross > > David > > 8<--------------------------------------- > diff --git a/drivers/xen/xen-pciback/pci_stub.c > b/drivers/xen/xen-pciback/pci_stub.c > index 4e8ba38..5a03e63 100644 > --- a/drivers/xen/xen-pciback/pci_stub.c > +++ b/drivers/xen/xen-pciback/pci_stub.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -43,6 +44,7 @@ struct pcistub_device { > struct kref kref; > struct list_head dev_list; > spinlock_t lock; > + bool created_reset_file; > > struct pci_dev *dev; > struct xen_pcibk_device *pdev;/* non-NULL if struct pci_dev is in use */ > @@ -60,6 +62,114 @@ static LIST_HEAD(pcistub_devices); > static int initialize_devices; > static LIST_HEAD(seized_devices); > > +/* > + * pci_reset_function() will only work if there is a mechanism to > + * reset that single function (e.g., FLR or a D-state transition). > + * For PCI hardware that has two or more functions but no per-function > + * reset, we can do a bus reset iff all the functions are co-assigned > + * to the same domain. > + * > + * If a function has no per-function reset mechanism the 'reset' sysfs > + * file that the toolstack uses to reset a function prior to assigning > + * the device will be missing. In this case, pciback adds its own > + * which will try a bus reset. > + * > + * Note: pciback does not check for co-assigment before doing a bus > + * reset, only that the devices are bound to pciback. The toolstack > + * is assumed to have done the right thing. > + */ > +static int __pcistub_reset_function(struct pci_dev *dev) > +{ > + struct pci_dev *pdev; > + u16 ctrl; > + int ret; > + > + ret = __pci_reset_function_locked(dev); > + if (ret == 0) > + return 0; > + > + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) > + return -ENOTTY; > + > + list_for_each_entry(pdev, &dev->bus->devices, bus_list) { > + if (pdev != dev && (!pdev->driver > + || strcmp(pdev->driver->name, "pciback"))) > + return -ENOTTY; > + pci_save_state(pdev); > + } > + > + pci_read_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, &ctrl); > + ctrl |= PCI_BRIDGE_CTL_BUS_RESET; > + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); > + msleep(200); > + > + ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; > + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); > + msleep(200); > + > + list_for_each_entry(pdev, &dev->bus->devices, bus_list) > + pci_restore_state(pdev); > + > + return 0; > +} > + > +static int pcistub_reset_function(struct pci_dev *dev) > +{ > + int ret; > + > + device_lock(&dev->dev); > + ret = __pcistub_reset_function(dev); > + device_unlock(&dev->dev); > + > + return ret; > +} > + > +static ssize_t pcistub_reset_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + unsigned long val; > + ssize_t result = strict_strtoul(buf, 0, &val); > + > + if (result < 0) > + return result; > + > + if (val != 1) > + return -EINVAL; > + > + result = pcistub_reset_function(pdev); > + if (result < 0) > + return result; > + return count; > +} > +static DEVICE_ATTR(reset, 0200, NULL, pcistub_reset_store); > + > +static int pcistub_try_create_reset_file(struct pcistub_device *psdev) > +{ > + struct device *dev = &psdev->dev->dev; > + struct sysfs_dirent *reset_dirent; > + int ret; > + > + reset_dirent = sysfs_get_dirent(dev->kobj.sd, NULL, "reset"); > + if (reset_dirent) { > + sysfs_put(reset_dirent); > + return 0; > + } > + > + ret = device_create_file(dev, &dev_attr_reset); > + if (ret < 0) > + return ret; > + psdev->created_reset_file = true; > + return 0; > +} > + > +static void pcistub_remove_reset_file(struct pcistub_device *psdev) > +{ > + if (psdev && psdev->created_reset_file) > + device_remove_file(&psdev->dev->dev, &dev_attr_reset); > +} > + > static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev) > { > struct pcistub_device *psdev; > @@ -95,12 +205,15 @@ static void pcistub_device_release(struct kref *kref) > > dev_dbg(&dev->dev, "pcistub_device_release\n"); > > + pcistub_remove_reset_file(psdev); > + > xen_unregister_device_domain_owner(dev); > > /* Call the reset function which does not take lock as this > * is called from "unbind" which takes a device_lock mutex. > */ > - __pci_reset_function_locked(dev); > + __pcistub_reset_function(psdev->dev); > + > if (pci_load_and_free_saved_state(dev, &dev_data->pci_saved_state)) > dev_dbg(&dev->dev, "Could not reload PCI state\n"); > else > @@ -268,7 +381,7 @@ void pcistub_put_pci_dev(struct pci_dev *dev) > /* This is OK - we are running from workqueue context > * and want to inhibit the user from fiddling with 'reset' > */ > - pci_reset_function(dev); > + pcistub_reset_function(psdev->dev); > pci_restore_state(psdev->dev); > > /* This disables the device. */ > @@ -392,7 +505,7 @@ static int pcistub_init_device(struct pci_dev *dev) > dev_err(&dev->dev, "Could not store PCI conf saved state!\n"); > else { > dev_dbg(&dev->dev, "resetting (FLR, D3, etc) the device\n"); > - __pci_reset_function_locked(dev); > + __pcistub_reset_function(dev); > pci_restore_state(dev); > } > /* Now disable the device (this also ensures some private device > @@ -467,6 +580,10 @@ static int pcistub_seize(struct pci_dev *dev) > if (!psdev) > return -ENOMEM; > > + err = pcistub_try_create_reset_file(psdev); > + if (err < 0) > + goto out; > + > spin_lock_irqsave(&pcistub_devices_lock, flags); > > if (initialize_devices) { > @@ -485,10 +602,9 @@ static int pcistub_seize(struct pci_dev *dev) > } > > spin_unlock_irqrestore(&pcistub_devices_lock, flags); > - > +out: > if (err) > pcistub_device_put(psdev); > - > return err; > } > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >