Re: [Xen-devel] [PATCH v5 9/9] xen/pciback: Implement PCI reset slot or bus with 'do_flr' SysFS attribute

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

On 04/12/14 13:10, Sander Eikelenboom wrote:
> 
> Thursday, December 4, 2014, 1:24:47 PM, you wrote:
> 
>> On 04/12/14 12:06, Konrad Rzeszutek Wilk wrote:
>>>
>>> On Dec 4, 2014 6:30 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>>>>
>>>> On 03/12/14 21:40, Konrad Rzeszutek Wilk wrote: 
>>>>>
>>>>> Instead of doing all this complex dance, we depend on the toolstack 
>>>>> doing the right thing. As such implement the 'do_flr' SysFS attribute 
>>>>> which 'xl' uses when a device is detached or attached from/to a guest. 
>>>>> It bypasses the need to worry about the PCI lock. 
>>>>
>>>> No.  Get pciback to add its own "reset" sysfs file (as I have repeatedly 
>>>> proposed). 
>>>>
>>>
>>> Which does not work as the kobj will complain (as there is already an 'reset' associated with the PCI device).
> 
>> It is only needed if the core won't provide one.
> 
>> +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;
>> +       int ret;
>> +
>> +       /* Already have a per-function reset? */
>> +       if (pci_probe_reset_function(pci) == 0)
>> +               return 0;
>> +
>> +       ret = device_create_file(dev, &dev_attr_reset);
>> +       if (ret < 0)
>> +               return ret;
> +       dev_data->>created_reset_file = true;
>> +       return 0;
>> +}
> 
> Wouldn't the "core-reset-sysfs-file" be still wired to the end up calling 
> "pci.c:__pci_dev_reset" ?
> 
> The problem with that function is that from my testing it seems that the 
> first option "pci_dev_specific_reset" always seems to return succes, so all the
> other options are skipped (flr, pm, slot, bus). However the device it self is 
> not properly reset enough (perhaps the pci_dev_specific_reset is good enough for 
> none virtualization purposes and it's probably the least intrusive. For 
> virtualization however it would be nice to be sure it resets properly, or have a 
> way to force a specific reset routine.)

Then you need work with the maintainer for those specific devices or
drivers to fix their specific reset function.

I'm not adding stuff to pciback to workaround broken quirks.

David