Re: [PATCH v3 4/7] xen/pciback: Implement PCI reset slot or bus with 'do_flr' SysFS attribute

[David Vrabel <david.vrabel@citrix.com>]

On 08/07/14 19:46, Konrad Rzeszutek Wilk wrote:
> On Tue, Jul 08, 2014 at 07:02:51PM +0100, David Vrabel wrote:
>> On 08/07/14 19:58, konrad@kernel.org wrote:
>>> --- a/Documentation/ABI/testing/sysfs-driver-pciback
>>> +++ b/Documentation/ABI/testing/sysfs-driver-pciback
>>> @@ -82,3 +82,14 @@ Description:
>>>                  device is shared, enabled, or on a level interrupt line.
>>>                  Writing a string of DDDD:BB:DD.F will toggle the state.
>>>                  This is Domain:Bus:Device.Function where domain is optional.
>>> +
>>> +What:           /sys/bus/pci/drivers/pciback/do_flr
>>> +Date:           July 2014
>>> +KernelVersion:  3.16
>>> +Contact:        xen-devel@lists.xenproject.org
>>> +Description:
>>> +                An option to slot or bus reset an PCI device owned by
>>> +                Xen PCI backend. Writing a string of DDDD:BB:DD.F will cause
>>> +                the driver to perform an slot or bus reset if the device
>>> +                supports. It also checks to make sure that all of the devices
>>> +                under the bridge are owned by Xen PCI backend.
>>
>> Not sure I like this new interface.  I solved this by adding a new reset
>> file that looked like the regular one the pci would have if it supported
>> FLR.  I'm fairly sure I posted a series for this.  Was there a reason
>> you didn't do this?
> 
> It did not work.
> 
> During bootup kobject would complain about a secondary 'reset' SysFS
> on the PCI device.

I think this because of pciback registering a driver too early, before
the device is fully initialized.  You can see in the trace that it is
the common pci code that is trying to add the "reset" file so it must be
doing this /after/ pciback's probe has been called.

I would consider:

1. Removing the "hide" module parameter -- it doesn't work if pciback is
a module anyway.

2. Making pciback initialize like a regular driver module (no
fs_initcall() shenanigans).

3. Require userspace to sort out binding the device to pciback (e.g.,
libxl already does the unbind if requested).

4. Finally, I would consider generic driver core functionality for
prioritizing drivers so they get probed first.

David