* [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR
@ 2014-07-09 14:09 David Vrabel
2014-07-09 14:22 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 6+ messages in thread
From: David Vrabel @ 2014-07-09 14:09 UTC (permalink / raw)
To: xen-devel; +Cc: Boris Ostrovsky, David Vrabel
The standard implementation of pci_reset_function() does not try an
SBR if there are other sibling devices. This is a common
configuration for multi-function devices (e.g., GPUs with a HDMI audio
device function).
If all the functions are co-assigned to the same domain at the same
time, then it is safe to perform an SBR to reset all functions at the
same time.
Add a "reset" sysfs file with the same interface as the standard one
(i.e., write 1 to reset) that will try an SBR if all sibling devices
are unbound or bound to pciback.
Note that this is weaker than the requirement for functions to be
co-assigned, but the toolstack is expected to ensure this.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
Changes in v2:
- defer creating sysfs node to late init.
---
drivers/xen/xen-pciback/pci_stub.c | 125 +++++++++++++++++++++++++++++++++++-
drivers/xen/xen-pciback/pciback.h | 1 +
2 files changed, 123 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index d57a173..5697c2a 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -17,6 +17,7 @@
#include <linux/wait.h>
#include <linux/sched.h>
#include <linux/atomic.h>
+#include <linux/delay.h>
#include <xen/events.h>
#include <asm/xen/pci.h>
#include <asm/xen/hypervisor.h>
@@ -63,6 +64,117 @@ 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 pci_dev *pci)
+{
+ struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
+ struct device *dev = &pci->dev;
+ struct kernfs_node *reset_dirent;
+ int ret;
+
+ reset_dirent = sysfs_get_dirent(dev->kobj.sd, "reset");
+ if (reset_dirent) {
+ sysfs_put(reset_dirent);
+ return 0;
+ }
+
+ ret = device_create_file(dev, &dev_attr_reset);
+ if (ret < 0)
+ return ret;
+ dev_data->created_reset_file = true;
+ return 0;
+}
+
+static void pcistub_remove_reset_file(struct pci_dev *pci)
+{
+ struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
+
+ if (dev_data && dev_data->created_reset_file)
+ device_remove_file(&pci->dev, &dev_attr_reset);
+}
+
static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev)
{
struct pcistub_device *psdev;
@@ -103,7 +215,8 @@ static void pcistub_device_release(struct kref *kref)
/* 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
@@ -280,7 +393,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(dev);
pci_restore_state(dev);
/* This disables the device. */
@@ -404,7 +517,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
@@ -413,6 +526,10 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(&dev->dev, "reset device\n");
xen_pcibk_reset_device(dev);
+ err = pcistub_try_create_reset_file(dev);
+ if (err < 0)
+ goto config_release;
+
dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
return 0;
@@ -540,6 +657,8 @@ static void pcistub_remove(struct pci_dev *dev)
dev_dbg(&dev->dev, "removing\n");
+ pcistub_remove_reset_file(dev);
+
spin_lock_irqsave(&pcistub_devices_lock, flags);
xen_pcibk_config_quirk_release(dev);
diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
index f72af87..708ade9 100644
--- a/drivers/xen/xen-pciback/pciback.h
+++ b/drivers/xen/xen-pciback/pciback.h
@@ -42,6 +42,7 @@ struct xen_pcibk_device {
struct xen_pcibk_dev_data {
struct list_head config_fields;
struct pci_saved_state *pci_saved_state;
+ bool created_reset_file;
unsigned int permissive:1;
unsigned int warned_on_write:1;
unsigned int enable_intx:1;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR
2014-07-09 14:09 [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR David Vrabel
@ 2014-07-09 14:22 ` Konrad Rzeszutek Wilk
2014-07-09 14:56 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-07-09 14:22 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Boris Ostrovsky
On Wed, Jul 09, 2014 at 03:09:59PM +0100, David Vrabel wrote:
> The standard implementation of pci_reset_function() does not try an
> SBR if there are other sibling devices. This is a common
> configuration for multi-function devices (e.g., GPUs with a HDMI audio
> device function).
>
> If all the functions are co-assigned to the same domain at the same
> time, then it is safe to perform an SBR to reset all functions at the
> same time.
Is there a particular reason you don't want to lift some of the
code (see my pcistub_reset_pci_dev function in xen/pciback: Implement
PCI reset slot or bus with 'do_flr' SysFS attribute) to check
for this?
>
> Add a "reset" sysfs file with the same interface as the standard one
> (i.e., write 1 to reset) that will try an SBR if all sibling devices
> are unbound or bound to pciback.
>
> Note that this is weaker than the requirement for functions to be
> co-assigned, but the toolstack is expected to ensure this.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> Changes in v2:
> - defer creating sysfs node to late init.
> ---
> drivers/xen/xen-pciback/pci_stub.c | 125 +++++++++++++++++++++++++++++++++++-
> drivers/xen/xen-pciback/pciback.h | 1 +
> 2 files changed, 123 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
> index d57a173..5697c2a 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -17,6 +17,7 @@
> #include <linux/wait.h>
> #include <linux/sched.h>
> #include <linux/atomic.h>
> +#include <linux/delay.h>
> #include <xen/events.h>
> #include <asm/xen/pci.h>
> #include <asm/xen/hypervisor.h>
> @@ -63,6 +64,117 @@ 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);
Why not use pci_try_reset_slot(dev->slot) or pci_try_reset_bus(dev->bus)?
> +
> + 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 pci_dev *pci)
> +{
> + struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
> + struct device *dev = &pci->dev;
> + struct kernfs_node *reset_dirent;
> + int ret;
> +
> + reset_dirent = sysfs_get_dirent(dev->kobj.sd, "reset");
> + if (reset_dirent) {
> + sysfs_put(reset_dirent);
> + return 0;
> + }
> +
> + ret = device_create_file(dev, &dev_attr_reset);
> + if (ret < 0)
> + return ret;
> + dev_data->created_reset_file = true;
> + return 0;
> +}
> +
> +static void pcistub_remove_reset_file(struct pci_dev *pci)
> +{
> + struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
> +
> + if (dev_data && dev_data->created_reset_file)
> + device_remove_file(&pci->dev, &dev_attr_reset);
> +}
> +
> static struct pcistub_device *pcistub_device_alloc(struct pci_dev *dev)
> {
> struct pcistub_device *psdev;
> @@ -103,7 +215,8 @@ static void pcistub_device_release(struct kref *kref)
> /* 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
> @@ -280,7 +393,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(dev);
> pci_restore_state(dev);
>
> /* This disables the device. */
> @@ -404,7 +517,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
> @@ -413,6 +526,10 @@ static int pcistub_init_device(struct pci_dev *dev)
> dev_dbg(&dev->dev, "reset device\n");
> xen_pcibk_reset_device(dev);
>
> + err = pcistub_try_create_reset_file(dev);
> + if (err < 0)
> + goto config_release;
> +
> dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
> return 0;
>
> @@ -540,6 +657,8 @@ static void pcistub_remove(struct pci_dev *dev)
>
> dev_dbg(&dev->dev, "removing\n");
>
> + pcistub_remove_reset_file(dev);
> +
> spin_lock_irqsave(&pcistub_devices_lock, flags);
>
> xen_pcibk_config_quirk_release(dev);
> diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
> index f72af87..708ade9 100644
> --- a/drivers/xen/xen-pciback/pciback.h
> +++ b/drivers/xen/xen-pciback/pciback.h
> @@ -42,6 +42,7 @@ struct xen_pcibk_device {
> struct xen_pcibk_dev_data {
> struct list_head config_fields;
> struct pci_saved_state *pci_saved_state;
> + bool created_reset_file;
> unsigned int permissive:1;
> unsigned int warned_on_write:1;
> unsigned int enable_intx:1;
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR
2014-07-09 14:22 ` Konrad Rzeszutek Wilk
@ 2014-07-09 14:56 ` Konrad Rzeszutek Wilk
2014-07-09 16:03 ` David Vrabel
0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-07-09 14:56 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Boris Ostrovsky
On Wed, Jul 09, 2014 at 10:22:12AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 09, 2014 at 03:09:59PM +0100, David Vrabel wrote:
> > The standard implementation of pci_reset_function() does not try an
> > SBR if there are other sibling devices. This is a common
> > configuration for multi-function devices (e.g., GPUs with a HDMI audio
> > device function).
> >
> > If all the functions are co-assigned to the same domain at the same
> > time, then it is safe to perform an SBR to reset all functions at the
> > same time.
>
> Is there a particular reason you don't want to lift some of the
> code (see my pcistub_reset_pci_dev function in xen/pciback: Implement
> PCI reset slot or bus with 'do_flr' SysFS attribute) to check
> for this?
>
> >
> > Add a "reset" sysfs file with the same interface as the standard one
> > (i.e., write 1 to reset) that will try an SBR if all sibling devices
> > are unbound or bound to pciback.
> >
> > Note that this is weaker than the requirement for functions to be
> > co-assigned, but the toolstack is expected to ensure this.
> >
> > Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> > ---
> > Changes in v2:
> > - defer creating sysfs node to late init.
I still get:
[ 11.899354] xen_pciback: wants to seize 0000:04:00.0
[ 11.904648] xen_pciback: wants to seize 0000:07:00.0
[ 11.909951] xen_pciback: wants to seize 0000:00:16.0
...
[ 11.952468] xen_pciback: wants to seize 0000:03:01.0
[ 11.957764] xen_pciback: wants to seize 0000:03:02.0
[ 11.963072] pciback 0000:00:00.0: probing...
[ 11.967640] pciback 0000:00:01.0: probing...
[ 11.972214] pciback 0000:00:02.0: probing...
[ 11.976792] pciback 0000:00:16.0: probing...
[ 11.981358] pciback 0000:00:16.0: seizing device
[ 11.986301] pciback 0000:00:16.0: pcistub_device_alloc
[ 11.991795] pciback 0000:00:16.0: deferring initialization
[ 11.997654] pciback 0000:00:16.2: probing...
..
[ 15.674241] pciback 0000:07:00.0: initializing...
[ 15.679211] pciback 0000:07:00.0: initializing config
[ 15.684625] pciback 0000:07:00.0: initializing virtual configuration space
[ 15.691969] pciback 0000:07:00.0: added config field at offset 0x00
[ 15.698656] pciback 0000:07:00.0: added config field at offset 0x02
..
[ 15.825575] pciback 0000:07:00.0: enabling device
[ 15.830674] xen: registering gsi 18 triggering 0 polarity 1
[ 15.836587] Already setup the GSI :18
[ 15.840618] pciback 0000:07:00.0: save state of device
[ 15.846178] pciback 0000:07:00.0: resetting (FLR, D3, etc) the device
[ 16.856902] pciback 0000:07:00.0: reset device
[ 16.861637] pciback 0000:05:00.0: initializing...
[ 16.866635] pciback 0000:05:00.0: initializing config
..
19.611695] registered taskstats version 1
[ 19.618056] ------------[ cut here ]------------
[ 19.622945] WARNING: CPU: 0 PID: 1 at /home/konrad/ssd/konrad/linux/fs/sysfs/dir.c:31 sysfs_warn_dup+0x6a/0x80()
[ 19.633729] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:1c.0/0000:02:00.0/0000:03:01.0/0000:04:00.0/reset'
[ 19.646265] Modules linked in:
[ 19.649558] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc4-00064-g90c5a05 #1
[ 19.657593] Hardware name: /DQ67SW, BIOS SWQ6710H.86A.0066.2012.1105.1504 11/05/2012
[ 19.667404] 000000000000001f ffff8801192abcc8 ffffffff816e72f1 000000000000001f
[ 19.675267] ffff8801192abd18 ffff8801192abd08 ffffffff810a11b7 000000000000fffe
[ 19.683146] ffff880118dee000 ffffffff819b6ac6 ffff880118e57a78 ffff880118e540a8
[ 19.691022] Call Trace:
[ 19.693683] [<ffffffff816e72f1>] dump_stack+0x51/0x6b
[ 19.699172] [<ffffffff810a11b7>] warn_slowpath_common+0x87/0xb0
[ 19.705568] [<ffffffff810a1281>] warn_slowpath_fmt+0x41/0x50
[ 19.711698] [<ffffffff81246693>] ? kernfs_path+0x53/0x70
[ 19.717462] [<ffffffff81248aaa>] sysfs_warn_dup+0x6a/0x80
[ 19.723334] [<ffffffff81248506>] sysfs_add_file_mode_ns+0x126/0x160
[ 19.730104] [<ffffffff81248565>] sysfs_create_file_ns+0x25/0x30
[ 19.736506] [<ffffffff81477953>] device_create_file+0x43/0xb0
[ 19.742714] [<ffffffff8136b843>] pci_create_sysfs_dev_files+0x2c3/0x3e0
[ 19.749867] [<ffffffff81d1fec9>] pci_sysfs_init+0x1f/0x51
[ 19.755732] [<ffffffff81d1feaa>] ? pci_driver_init+0x12/0x12
[ 19.761865] [<ffffffff81d1feaa>] ? pci_driver_init+0x12/0x12
[ 19.768006] [<ffffffff81002089>] do_one_initcall+0x89/0x1b0
[ 19.774067] [<ffffffff81ce4b30>] kernel_init_freeable+0x167/0x1fd
[ 19.780642] [<ffffffff81ce4bc6>] ? kernel_init_freeable+0x1fd/0x1fd
[ 19.787412] [<ffffffff816df0e0>] ? rest_init+0x90/0x90
[ 19.792999] [<ffffffff816df0e9>] kernel_init+0x9/0xf0
[ 19.798498] [<ffffffff816ec97c>] ret_from_fork+0x7c/0xb0
[ 19.804262] [<ffffffff816df0e0>] ? rest_init+0x90/0x90
[ 19.809851] ---[ end trace 515783e1a29a2558 ]---
This is with (ignore the rest please):
90c5a05 xen-pciback: provide a "reset" sysfs file to try harder at an SBR
64b4480 pciback: debug
da3cb64 *DEBUG*/HVM: send VCPUOP_send_nmi for booting CPUs.
904ba1e RFC: VCPU_reset_cpu_info
8cb120e xen PVonHVM: use E820_Reserved area for shared_info
b30f53d kexec/pvhvm: Disable Xen FIFO events.
3b8624e Revert "x86/mce: Improve mcheck_init_device() error handling"
c7c15b8 Fix ERROR: "dma_get_required_mask" [drivers/scsi/qla2xxx/qla2xxx.ko] undefined!
33c97c6 x86/xen: Disable APIC PM for Xen PV guests
13776ef x86,xen: correct dma_get_required_mask() for Xen PV guests
826472b dma: add dma_get_required_mask_from_max_pfn()
1607417 xen/pvhvm: Support more than 32 VCPUs when migrating (v2).
a3b0e9c x86: skip check for spurious faults for non-present faults
718f1f2 x86, fpu: remove the logic of non-eager fpu mem allocation at the first usage
b68a3ce Introduce XEN scsiback module
f13ac1e Introduce xen-scsifront module
5e84e1d add xen pvscsi maintainer
50e590c Add XEN pvSCSI protocol description
1845de2 arch/x86: Comment out efi_set_rtc_mmss()
5507170 arch/x86: Remove redundant set_bit() call
e8d88ed arch/x86: Replace plain strings with constants
e4b397a xen: Put EFI machinery in place
74fb794 xen: Define EFI related stuff
553fc3b efi: Introduce EFI_NO_DIRECT flag
fe5e3f7 efi: Use early_mem*() instead of early_io*()
bdac02c xen/PMU: PMU emulation code
42bde6d xen/PMU: Intercept PMU-related MSR and APIC accesses
c9a8f2a xen/PMU: Describe vendor-specific PMU registers
f1594d1 x86: JAn's fixes
454222a xen/PMU: Initialization code for Xen PMU
02ea3d8 xen/PMU: Sysfs interface for setting Xen PMU mode
aaf260f xen: xensyms support
61d8551 Revert "x86/xen: fix memory setup for PVH dom0"
9aca7cb Revert "Revert "xen/pvh: Update E820 to work with PVH (v2)""
912c8f6 xen-netback: Adding debugfs "io_ring_qX" files
fc8c90e xen: resume console a bit later
96cbfb5 xen-netfront: don't nest queue locks in xennet_connect()
1f8673d xen-netfront: call netif_carrier_off() only once when disconnecting
322283e xen/balloon: set ballooned out pages as invalid in p2m
acbd582 xen/grant-table: remove support for V2 tables
cf7fe4d x86/xen: safely map and unmap grant frames when in atomic context
9ce66d6 Revert "irq: Enable all irqs unconditionally in irq_resume"
d5859c7 genirq: Fix error path for resuming irqs
22c46db x86: initialize secondary CPU only if master CPU will wait for it
I think for your patch to work it has to de-register what the
generic PCI code does in pci_create_capabilities_sysfs:
1339 if (!pci_probe_reset_function(dev)) {
1340 retval = device_create_file(&dev->dev, &reset_attr);
1341 if (retval)
1342 goto error;
1343 dev->reset_fn = 1;
1344 }
which is called by 'pci_create_sysfs_dev_files' which is called
by pci_bus_add_device which is called before the device is 'binded'
to PCI back. So Xen PCIback would need to figure out the 'reset_attr' pointer
, save it, unregister it. Then when the device is unbinded it needs to
unregister its own reset and then register the one it saved. Yuck.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR
2014-07-09 14:56 ` Konrad Rzeszutek Wilk
@ 2014-07-09 16:03 ` David Vrabel
2014-07-09 16:10 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 6+ messages in thread
From: David Vrabel @ 2014-07-09 16:03 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Boris Ostrovsky
On 09/07/14 15:56, Konrad Rzeszutek Wilk wrote:
>
> I think for your patch to work it has to de-register what the
> generic PCI code does in pci_create_capabilities_sysfs:
>
> 1339 if (!pci_probe_reset_function(dev)) {
> 1340 retval = device_create_file(&dev->dev, &reset_attr);
> 1341 if (retval)
> 1342 goto error;
> 1343 dev->reset_fn = 1;
> 1344 }
It just needs to have the inverse test.
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -151,14 +151,11 @@ static int pcistub_try_create_reset_file(struct
pci_dev *pci)
{
struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
struct device *dev = &pci->dev;
- struct kernfs_node *reset_dirent;
int ret;
- reset_dirent = sysfs_get_dirent(dev->kobj.sd, "reset");
- if (reset_dirent) {
- sysfs_put(reset_dirent);
+ /* Already have a per-function reset? */
+ if (pci_dev_reset(dev, 1) == 0)
return 0;
- }
ret = device_create_file(dev, &dev_attr_reset);
if (ret < 0)
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR
2014-07-09 16:03 ` David Vrabel
@ 2014-07-09 16:10 ` Konrad Rzeszutek Wilk
2014-07-10 13:37 ` David Vrabel
0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-07-09 16:10 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Boris Ostrovsky
On Wed, Jul 09, 2014 at 05:03:17PM +0100, David Vrabel wrote:
> On 09/07/14 15:56, Konrad Rzeszutek Wilk wrote:
> >
> > I think for your patch to work it has to de-register what the
> > generic PCI code does in pci_create_capabilities_sysfs:
> >
> > 1339 if (!pci_probe_reset_function(dev)) {
> > 1340 retval = device_create_file(&dev->dev, &reset_attr);
> > 1341 if (retval)
> > 1342 goto error;
> > 1343 dev->reset_fn = 1;
> > 1344 }
>
> It just needs to have the inverse test.
>
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -151,14 +151,11 @@ static int pcistub_try_create_reset_file(struct
> pci_dev *pci)
> {
> struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
> struct device *dev = &pci->dev;
> - struct kernfs_node *reset_dirent;
> int ret;
>
> - reset_dirent = sysfs_get_dirent(dev->kobj.sd, "reset");
> - if (reset_dirent) {
> - sysfs_put(reset_dirent);
> + /* Already have a per-function reset? */
> + if (pci_dev_reset(dev, 1) == 0)
OK, however the'reset' in SysFS does not do bus/slot reset.
It only calls 'pci_reset_function' - which is inadequate
in certain cases (GPUs, device does not really go in D3 when
asked too, etc) and that is where bus/slot reset functionality
is needed.
> return 0;
> - }
>
> ret = device_create_file(dev, &dev_attr_reset);
> if (ret < 0)
>
> David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR
2014-07-09 16:10 ` Konrad Rzeszutek Wilk
@ 2014-07-10 13:37 ` David Vrabel
0 siblings, 0 replies; 6+ messages in thread
From: David Vrabel @ 2014-07-10 13:37 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Boris Ostrovsky
On 09/07/14 17:10, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 09, 2014 at 05:03:17PM +0100, David Vrabel wrote:
>> On 09/07/14 15:56, Konrad Rzeszutek Wilk wrote:
>>>
>>> I think for your patch to work it has to de-register what the
>>> generic PCI code does in pci_create_capabilities_sysfs:
>>>
>>> 1339 if (!pci_probe_reset_function(dev)) {
>>> 1340 retval = device_create_file(&dev->dev, &reset_attr);
>>> 1341 if (retval)
>>> 1342 goto error;
>>> 1343 dev->reset_fn = 1;
>>> 1344 }
>>
>> It just needs to have the inverse test.
>>
>> --- a/drivers/xen/xen-pciback/pci_stub.c
>> +++ b/drivers/xen/xen-pciback/pci_stub.c
>> @@ -151,14 +151,11 @@ static int pcistub_try_create_reset_file(struct
>> pci_dev *pci)
>> {
>> struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(pci);
>> struct device *dev = &pci->dev;
>> - struct kernfs_node *reset_dirent;
>> int ret;
>>
>> - reset_dirent = sysfs_get_dirent(dev->kobj.sd, "reset");
>> - if (reset_dirent) {
>> - sysfs_put(reset_dirent);
>> + /* Already have a per-function reset? */
>> + if (pci_dev_reset(dev, 1) == 0)
>
> OK, however the'reset' in SysFS does not do bus/slot reset.
Um. This is probing if a per function reset is available, if it isn't
it creates the "reset" sysfs file. Which will then try an SBR.
(This snippet is broken though since pci_dev_reset() is not extern, see
latest series for the correct fix.)
David
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-10 13:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-09 14:09 [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR David Vrabel
2014-07-09 14:22 ` Konrad Rzeszutek Wilk
2014-07-09 14:56 ` Konrad Rzeszutek Wilk
2014-07-09 16:03 ` David Vrabel
2014-07-09 16:10 ` Konrad Rzeszutek Wilk
2014-07-10 13:37 ` David Vrabel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.