Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute

[boris.ostrovsky@xxxxxxxxxx]

On 10/19/20 6:43 PM, Rich Persaud wrote:
> On Oct 19, 2020, at 11:52, Håkon Alstadheim <hakon@xxxxxxxxxxxxxxxxxx> wrote:
>> 
>> Den 19.10.2020 17:16, skrev Håkon Alstadheim:
>>> Den 19.10.2020 13:00, skrev George Dunlap:
>>>>> On Jan 31, 2020, at 3:33 PM, Wei Liu <wl@xxxxxxx> wrote:
>>>>>
>>>>> On Fri, Jan 17, 2020 at 02:13:04PM -0500, Rich Persaud wrote:
>>>>>> On Aug 26, 2019, at 17:08, Pasi Kärkkäinen <pasik@xxxxxx> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>> On Mon, Oct 08, 2018 at 10:32:45AM -0400, Boris Ostrovsky wrote:
>>>>>>>>> On 10/3/18 11:51 AM, Pasi Kärkkäinen wrote:
>>>>>>>>> On Wed, Sep 19, 2018 at 11:05:26AM +0200, Roger Pau Monné wrote:
>>>>>>>>>> On Tue, Sep 18, 2018 at 02:09:53PM -0400, Boris Ostrovsky wrote:
>>>>>>>>>>> On 9/18/18 5:32 AM, George Dunlap wrote:
>>>>>>>>>>>>> On Sep 18, 2018, at 8:15 AM, Pasi Kärkkäinen <pasik@xxxxxx> wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>> On Mon, Sep 17, 2018 at 02:06:02PM -0400, Boris Ostrovsky wrote:
>>>>>>>>>>>>>>> What about the toolstack changes? Have they been accepted? I 
>>>>>>>>>>>>>>> vaguely
>>>>>>>>>>>>>>> recall there was a discussion about those changes but don't 
>>>>>>>>>>>>>>> remember how
>>>>>>>>>>>>>>> it ended.
>>>>>>>>>>>>>> I don't think toolstack/libxl patch has been applied yet either.
>>>>>>>>>>>>>> "[PATCH V1 0/1] Xen/Tools: PCI reset using 'reset' SysFS 
>>>>>>>>>>>>>> attribute":
>>>>>>>>>>>>>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg00664.html
>>>>>>>>>>>>>>  
>>>>>>>>>>>>>> "[PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS 
>>>>>>>>>>>>>> attribute":
>>>>>>>>>>>>>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg00663.html
>>>>>>>>>>>>>>  
>>>>>>>>>>>> Will this patch work for *BSD? Roger?
>>>>>>>>>>> At least FreeBSD don't support pci-passthrough, so none of this 
>>>>>>>>>>> works
>>>>>>>>>>> ATM. There's no sysfs on BSD, so much of what's in libxl_pci.c will
>>>>>>>>>>> have to be moved to libxl_linux.c when BSD support is added.
>>>>>>>>>> Ok. That sounds like it's OK for the initial pci 'reset' 
>>>>>>>>>> implementation in xl/libxl to be linux-only..
>>>>>>>>> Are these two patches still needed? ISTR they were written originally
>>>>>>>>> to deal with guest trying to use device that was previously assigned 
>>>>>>>>> to
>>>>>>>>> another guest. But pcistub_put_pci_dev() calls
>>>>>>>>> __pci_reset_function_locked() which first tries FLR, and it looks like
>>>>>>>>> it was added relatively recently.
>>>>>>>> Replying to an old thread.. I only now realized I forgot to reply to 
>>>>>>>> this message earlier.
>>>>>>>>
>>>>>>>> afaik these patches are still needed. Håkon (CC'd) wrote to me in 
>>>>>>>> private that
>>>>>>>> he gets a (dom0) Linux kernel crash if he doesn't have these patches 
>>>>>>>> applied.


If this is still crashing I'd be curious to see the splat.


>>>>>>>>
>>>>>>>>
>>>>>>>> Here are the links to both the linux kernel and libxl patches:
>>>>>>>>
>>>>>>>>
>>>>>>>> "[Xen-devel] [PATCH V3 0/2] Xen/PCIback: PCI reset using 'reset' SysFS 
>>>>>>>> attribute":
>>>>>>>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg00659.html
>>>>>>>>
>>>>>>>> [Note that PATCH V3 1/2 "Drivers/PCI: Export pcie_has_flr() interface" 
>>>>>>>> is already applied in upstream linux kernel, so it's not needed 
>>>>>>>> anymore]
>>>>>>>>
>>>>>>>> "[Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus 
>>>>>>>> reset with 'reset' SysFS attribute":
>>>>>>>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg00661.html
>>>>>>>>
>>>>>>>>
>>>>>>>> "[Xen-devel] [PATCH V1 0/1] Xen/Tools: PCI reset using 'reset' SysFS 
>>>>>>>> attribute":
>>>>>>>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg00664.html
>>>>>>>>
>>>>>>>> "[Xen-devel] [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' 
>>>>>>>> SysFS attribute":
>>>>>>>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg00663.html
>>>>>>> [dropping Linux mailing lists]
>>>>>>>
>>>>>>> What is required to get the Xen patches merged?  Rebasing against Xen
>>>>>>> master?  OpenXT has been carrying a similar patch for many years and
>>>>>>> we would like to move to an upstream implementation.  Xen users of PCI
>>>>>>> passthrough would benefit from more reliable device reset.
>>>>>> Rebase and resend?


If you are is going to resend then please address Jan's comments from 
https://lists.xen.org/archives/html/xen-devel/2017-12/msg00695.html (although I 
am not sure in usefulness of the last one)


>>>>>>
>>>>>> Skimming that thread I think the major concern was backward
>>>>>> compatibility. That seemed to have been addressed.
>>>>>>
>>>>>> Unfortunately I don't have the time to dig into Linux to see if the
>>>>>> claim there is true or not.
>>>>>>
>>>>>> It would be helpful to write a concise paragraph to say why backward
>>>>>> compatibility is not required.
>>>> Just going through my old “make sure something happens with this” mails.  
>>>> Did anything ever happen with this?  Who has the ball here / who is this 
>>>> stuck on?
>>> We're waiting for "somebody" to testify that fixing this will not adversely 
>>> affect anyone. I'm not qualified, but my strong belief is that since 
>>> "reset" or "do_flr"  in the linux kernel is not currently implemented/used 
>>> in any official distribution, it should be OK.
>>>
>>> Patches still work in current staging-4.14 btw.
>>>
>> Just for the record, attached are the patches I am running on top of linux 
>> gentoo-sources-5.9.1  and xen-staging-4.14 respectively. (I am also running 
>> with the patch to mark populated reserved memory that contains ACPI tables 
>> as "ACPI NVS", not attached here ).
>>
>> <pci_brute_reset-home-hack.patch>
>> <pci_brute_reset-home-hack-doc.patch>
>> <pci-reset-min-egen.patch>
> Is there any reason not to merge the Xen patch, while waiting for the Linux 
> patch to be upstreamed?  Similar versions have been deployed in downstream 
> production systems for years, we can at least de-dupe those Xen patches.
>
> Do (can) we have an in-tree location to queue Xen-relevant Linux patches 
> while they go through an upstreaming process that may last several (5+ here) 
> years?


No (at least as far as I can think of it) but then I can't remember another 
instance of patches taking that long.


-boris