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

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

[Rich Persaud]

[-- Attachment #1.1: Type: text/plain, Size: 3441 bytes --]

On Aug 26, 2019, at 17:08, Pasi Kärkkäinen <pasik@iki.fi> 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@iki.fi> 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.
> 
> 
> 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.

  2017 thread, including OpenXT patch: https://lists.gt.net/xen/devel/492945
  2017-2019 thread: https://lists.gt.net/xen/devel/532648

Rich

[-- Attachment #1.2: Type: text/html, Size: 10134 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Pasi Kärkkäinen]

Hi,

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@iki.fi> 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@iki.fi>
>                  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.
>      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.
>      2017 thread, including OpenXT
>    patch: [1]https://lists.gt.net/xen/devel/492945
>      2017-2019 thread: [2]https://lists.gt.net/xen/devel/532648
>

Yes, rebasing the kernel patch against the current Linux kernel, and also rebasing the libxl bits against current master/staging.
That should be a good start!

I'd like to see the reset functionality merged aswell.


>    Rich
> 


Thanks,

-- Pasi


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Wei Liu]

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@iki.fi> 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@iki.fi> 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.
> > 
> > 
> > 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?

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.

Wei.

> 
>   2017 thread, including OpenXT patch: https://lists.gt.net/xen/devel/492945
>   2017-2019 thread: https://lists.gt.net/xen/devel/532648
> 
> Rich

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[George Dunlap]

> On Jan 31, 2020, at 3:33 PM, Wei Liu <wl@xen.org> 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@iki.fi> 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@iki.fi> 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.
>>> 
>>> 
>>> 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?
> 
> 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?

 -George

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

[Håkon Alstadheim]

Den 19.10.2020 13:00, skrev George Dunlap:
>
>> On Jan 31, 2020, at 3:33 PM, Wei Liu <wl@xen.org> 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@iki.fi> 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@iki.fi> 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.
>>>>
>>>>
>>>> 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?
>>
>> 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.

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

[Håkon Alstadheim]

[-- Attachment #1: Type: text/plain, Size: 5063 bytes --]


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@xen.org> 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@iki.fi> 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@iki.fi> 
>>>>>>>>>>> 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.
>>>>>
>>>>>
>>>>> 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?
>>>
>>> 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 ).


[-- Attachment #2: pci_brute_reset-home-hack.patch --]
[-- Type: text/x-patch, Size: 3558 bytes --]

--- a/drivers/xen/xen-pciback/pci_stub.c	2020-03-30 21:08:39.406994339 +0200
+++ b/drivers/xen/xen-pciback/pci_stub.c	2020-03-30 20:56:18.225810279 +0200
@@ -245,6 +245,90 @@
 	return found_dev;
 }
 
+struct pcistub_args {
+	struct pci_dev *dev;
+	unsigned int dcount;
+};
+
+static int pcistub_search_dev(struct pci_dev *dev, void *data)
+{
+	struct pcistub_device *psdev;
+	struct pcistub_args *arg = data;
+	bool found_dev = false;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
+
+	list_for_each_entry(psdev, &pcistub_devices, dev_list) {
+		if (psdev->dev == dev) {
+			found_dev = true;
+			arg->dcount++;
+			break;
+		}
+	}
+
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+
+	/* Device not owned by pcistub, someone owns it. Abort the walk */
+	if (!found_dev)
+		arg->dev = dev;
+
+	return found_dev ? 0 : 1;
+}
+
+static int pcistub_reset_dev(struct pci_dev *dev)
+{
+	struct xen_pcibk_dev_data *dev_data;
+	bool slot = false, bus = false;
+	struct pcistub_args arg = {};
+
+	if (!dev)
+		return -EINVAL;
+
+	dev_dbg(&dev->dev, "[%s]\n", __func__);
+
+	if (!pci_probe_reset_slot(dev->slot))
+		slot = true;
+	else if ((!pci_probe_reset_bus(dev->bus)) &&
+		 (!pci_is_root_bus(dev->bus)))
+		bus = true;
+
+	if (!bus && !slot)
+		return -EOPNOTSUPP;
+
+	/*
+	 * Make sure all devices on this bus are owned by the
+	 * PCI backend so that we can safely reset the whole bus.
+	 */
+	pci_walk_bus(dev->bus, pcistub_search_dev, &arg);
+
+	/* All devices under the bus should be part of pcistub! */
+	if (arg.dev) {
+		dev_err(&dev->dev, "%s device on bus 0x%x is not owned by pcistub\n",
+			pci_name(arg.dev), dev->bus->number);
+
+		return -EBUSY;
+	}
+
+	dev_dbg(&dev->dev, "pcistub owns %d devices on bus 0x%x\n",
+		arg.dcount, dev->bus->number);
+
+	dev_data = pci_get_drvdata(dev);
+	if (!pci_load_saved_state(dev, dev_data->pci_saved_state))
+		pci_restore_state(dev);
+
+	/* This disables the device. */
+	xen_pcibk_reset_device(dev);
+
+	/* Cleanup up any emulated fields */
+	xen_pcibk_config_reset_dev(dev);
+
+	dev_dbg(&dev->dev, "resetting %s device using %s reset\n",
+		pci_name(dev), slot ? "slot" : "bus");
+
+	return pci_reset_bus(dev);
+}
+
 /*
  * Called when:
  *  - XenBus state has been reconfigure (pci unplug). See xen_pcibk_remove_device
@@ -1492,6 +1576,33 @@
 }
 static DRIVER_ATTR_RW(allow_interrupt_control);
 
+static ssize_t reset_store(struct device_driver *drv, const char *buf,
+			      size_t count)
+{
+	struct pcistub_device *psdev;
+	int domain, bus, slot, func;
+	int err;
+
+	err = str_to_slot(buf, &domain, &bus, &slot, &func);
+	if (err)
+		return err;
+
+	psdev = pcistub_device_find(domain, bus, slot, func);
+	if (psdev) {
+		err = pcistub_reset_dev(psdev->dev);
+		pcistub_device_put(psdev);
+	} else {
+		err = -ENODEV;
+	}
+
+	if (!err)
+		err = count;
+
+	return err;
+}
+
+static DRIVER_ATTR_WO(reset);
+
 static void pcistub_exit(void)
 {
 	driver_remove_file(&xen_pcibk_pci_driver.driver, &driver_attr_new_slot);
@@ -1507,6 +1618,8 @@
 			   &driver_attr_irq_handlers);
 	driver_remove_file(&xen_pcibk_pci_driver.driver,
 			   &driver_attr_irq_handler_state);
+	driver_remove_file(&xen_pcibk_pci_driver.driver,
+			   &driver_attr_reset);
 	pci_unregister_driver(&xen_pcibk_pci_driver);
 }
 
@@ -1603,6 +1716,11 @@
 	if (!err)
 		err = driver_create_file(&xen_pcibk_pci_driver.driver,
 					&driver_attr_irq_handler_state);
+
+	if (!err)
+		err = driver_create_file(&xen_pcibk_pci_driver.driver,
+					 &driver_attr_reset);
+
 	if (err)
 		pcistub_exit();
 

[-- Attachment #3: pci_brute_reset-home-hack-doc.patch --]
[-- Type: text/x-patch, Size: 931 bytes --]

--- a/Documentation/ABI/testing/sysfs-driver-pciback	2020-03-30 00:25:41.000000000 +0200
+++ b/Documentation/ABI/testing/sysfs-driver-pciback	2020-03-30 21:01:58.909583316 +0200
@@ -12,6 +12,19 @@
                 will allow the guest to read and write to the configuration
                 register 0x0E.
 
+
+What:           /sys/bus/pci/drivers/pciback/reset
+Date:           Nov 2017
+KernelVersion:  none
+Contact:        xen-devel@lists.xenproject.org
+Description:
+                An option to perform a slot or bus reset when a PCI device
+		is owned by Xen PCI backend. Writing a string of DDDD:BB:DD.F
+		will cause the pciback driver to perform a slot or bus reset
+		if the device supports it. It also checks to make sure that
+		all of the devices under the bridge are owned by Xen PCI
+		backend.
+
 What:           /sys/bus/pci/drivers/pciback/allow_interrupt_control
 Date:           Jan 2020
 KernelVersion:  5.6

[-- Attachment #4: pci-reset-min-egen.patch --]
[-- Type: text/x-patch, Size: 426 bytes --]

--- a/tools/libxl/libxl_pci.c	2020-04-09 09:26:54.000000000 +0200
+++ b/tools/libxl/libxl_pci.c	2020-04-14 23:39:12.830752851 +0200
@@ -1452,7 +1452,7 @@
     char *reset;
     int fd, rc;
 
-    reset = GCSPRINTF("%s/do_flr", SYSFS_PCIBACK_DRIVER);
+    reset = GCSPRINTF("%s/reset", SYSFS_PCIBACK_DRIVER);
     fd = open(reset, O_WRONLY);
     if (fd >= 0) {
         char *buf = GCSPRINTF(PCI_BDF, domain, bus, dev, func);

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

[Rich Persaud]

On Oct 19, 2020, at 11:52, Håkon Alstadheim <hakon@alstadheim.priv.no> 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@xen.org> 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@iki.fi> 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@iki.fi> 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.
>>>>>>> 
>>>>>>> 
>>>>>>> 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?
>>>>> 
>>>>> 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?

Rich

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

[boris.ostrovsky]

On 10/19/20 6:43 PM, Rich Persaud wrote:
> On Oct 19, 2020, at 11:52, Håkon Alstadheim <hakon@alstadheim.priv.no> 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@xen.org> 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@iki.fi> 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@iki.fi> 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

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

[Pasi Kärkkäinen]

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@iki.fi> 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.


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


> 
> -boris


Thanks,

-- Pasi

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

[Pasi Kärkkäinen]

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@iki.fi> 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.


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


> 
> -boris


Thanks,

-- Pasi


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Boris Ostrovsky]

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@iki.fi> 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.


-boris

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

[Pasi Kärkkäinen]

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@iki.fi> 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.. 


Thanks,

-- Pasi


> Thanks, Roger.

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

[Roger Pau Monné]

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@iki.fi> 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.

Thanks, Roger.

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

[Boris Ostrovsky]

On 9/18/18 5:32 AM, George Dunlap wrote:
>
>> On Sep 18, 2018, at 8:15 AM, Pasi Kärkkäinen <pasik@iki.fi> 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?


>>
>> George asked for some clarifications:
>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg01044.html
>> https://lists.xen.org/archives/html/xen-devel/2017-12/msg01116.html
> Right, the description of the patch didn’t actually tell you what was going on.  It should have said something like, “xl currently attempts to reset a device using X; but that’s never been implemented in Linux.  Instead, use Y, which [is better for whatever reason]”.

Yes, the description can be tightened a bit ;-)

-boris

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

[George Dunlap]

> On Sep 18, 2018, at 8:15 AM, Pasi Kärkkäinen <pasik@iki.fi> wrote:
> 
> Hi,
> 
> On Mon, Sep 17, 2018 at 02:06:02PM -0400, Boris Ostrovsky wrote:
>> On 9/16/18 7:43 AM, Pasi Kärkkäinen wrote:
>>> Hi,
>>> 
>>> On Mon, Dec 18, 2017 at 12:32:11PM -0500, Boris Ostrovsky wrote:
>>>> On 12/18/2017 02:36 AM, Jan Beulich wrote:
>>>>>>>> On 15.12.17 at 20:52, <Govinda.Tatti@Oracle.COM> wrote:
>>>>>>>>> +static int pcistub_device_reset(struct pci_dev *dev)
>>>>>>>>> +{
>>>>>>>>> +	struct xen_pcibk_dev_data *dev_data;
>>>>>>>>> +	bool slot = false, bus = false;
>>>>>>>>> +	struct pcistub_args arg = {};
>>>>>>>>> +
>>>>>>>>> +	if (!dev)
>>>>>>>>> +		return -EINVAL;
>>>>>>>>> +
>>>>>>>>> +	dev_dbg(&dev->dev, "[%s]\n", __func__);
>>>>>>>>> +
>>>>>>>>> +	/* First check and try FLR */
>>>>>>>>> +	if (pcie_has_flr(dev)) {
>>>>>>>>> +		dev_dbg(&dev->dev, "resetting %s device using FLR\n",
>>>>>>>>> +			pci_name(dev));
>>>>>>>>> +		pcie_flr(dev);
>>>>>>>> The lack of error check here puzzled me, but I see the function
>>>>>>>> indeed returns void right now. I think the prereq patch should
>>>>>>>> change this along with exporting the function - you really don't
>>>>>>>> want the device to be handed to a guest when the FLR timed
>>>>>>>> out.
>>>>>>> We will change pcie_flr() to return error code. I will make this change
>>>>>>> in the next version of this patch.
>>>>>> I exchanged some emails with Bjorn/Christoph and it looks like Christoph
>>>>>> as some planto restructure pcie flr specific functions but I don't know
>>>>>> the exact time-frame. For now,I am planning to use existing pcie_flr()
>>>>>> after checking FLR capability. We will switchto revised pcie_flr() once
>>>>>> it is available.
>>>>>> 
>>>>>> I hope you are fine with this approach. Please let me know. Thanks.
>>>>> I've seen that other discussion. I don't think the change here
>>>>> should be done prior to the error reporting being put in place,
>>>>> for security reasons. But in the end it'll be Konrad as the
>>>>> maintainer to judge.
>>>>> 
>>>>> Or wait, looks like there's some confusion in ./MAINTAINERS:
>>>>> Konrad is listed as maintainer for "XEN PCI SUBSYSTEM", but the
>>>>> list of files doesn't include pciback. So it would instead be Boris
>>>>> or Jürgen to give you a final word.
>>>> 
>>>> This is now 4.16 material so we can at least wait until closer to
>>>> opening of the merge window when we may have the PCI updates. (And I
>>>> just noticed that you responded to Christoph.)
>>>> 
>>>> Besides, we don't want to make kernel changes until the interface is
>>>> settled (i.e the toolstack changes are accepted).
>>>> 
>>> It seems Govinda's email address is giving an error, so I assume someone else needs to pick up this pciback 'reset' feature.
>>> Is it likely someone else from Oracle can/will pick up and refresh this patch, with the review comments addressed?
>> 
>> 
>> Govinda is no longer at Oracle.
>> 
> 
> Yep, thought so. Removed from CC list.
> 
> 
>> 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
> 
> George asked for some clarifications:
> https://lists.xen.org/archives/html/xen-devel/2017-12/msg01044.html
> https://lists.xen.org/archives/html/xen-devel/2017-12/msg01116.html

Right, the description of the patch didn’t actually tell you what was going on.  It should have said something like, “xl currently attempts to reset a device using X; but that’s never been implemented in Linux.  Instead, use Y, which [is better for whatever reason]”.

 -George

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

[George Dunlap]

DQoNCj4gT24gU2VwIDE4LCAyMDE4LCBhdCA4OjE1IEFNLCBQYXNpIEvDpHJra8OkaW5lbiA8cGFz
aWtAaWtpLmZpPiB3cm90ZToNCj4gDQo+IEhpLA0KPiANCj4gT24gTW9uLCBTZXAgMTcsIDIwMTgg
YXQgMDI6MDY6MDJQTSAtMDQwMCwgQm9yaXMgT3N0cm92c2t5IHdyb3RlOg0KPj4gT24gOS8xNi8x
OCA3OjQzIEFNLCBQYXNpIEvDpHJra8OkaW5lbiB3cm90ZToNCj4+PiBIaSwNCj4+PiANCj4+PiBP
biBNb24sIERlYyAxOCwgMjAxNyBhdCAxMjozMjoxMVBNIC0wNTAwLCBCb3JpcyBPc3Ryb3Zza3kg
d3JvdGU6DQo+Pj4+IE9uIDEyLzE4LzIwMTcgMDI6MzYgQU0sIEphbiBCZXVsaWNoIHdyb3RlOg0K
Pj4+Pj4+Pj4gT24gMTUuMTIuMTcgYXQgMjA6NTIsIDxHb3ZpbmRhLlRhdHRpQE9yYWNsZS5DT00+
IHdyb3RlOg0KPj4+Pj4+Pj4+ICtzdGF0aWMgaW50IHBjaXN0dWJfZGV2aWNlX3Jlc2V0KHN0cnVj
dCBwY2lfZGV2ICpkZXYpDQo+Pj4+Pj4+Pj4gK3sNCj4+Pj4+Pj4+PiArCXN0cnVjdCB4ZW5fcGNp
YmtfZGV2X2RhdGEgKmRldl9kYXRhOw0KPj4+Pj4+Pj4+ICsJYm9vbCBzbG90ID0gZmFsc2UsIGJ1
cyA9IGZhbHNlOw0KPj4+Pj4+Pj4+ICsJc3RydWN0IHBjaXN0dWJfYXJncyBhcmcgPSB7fTsNCj4+
Pj4+Pj4+PiArDQo+Pj4+Pj4+Pj4gKwlpZiAoIWRldikNCj4+Pj4+Pj4+PiArCQlyZXR1cm4gLUVJ
TlZBTDsNCj4+Pj4+Pj4+PiArDQo+Pj4+Pj4+Pj4gKwlkZXZfZGJnKCZkZXYtPmRldiwgIlslc11c
biIsIF9fZnVuY19fKTsNCj4+Pj4+Pj4+PiArDQo+Pj4+Pj4+Pj4gKwkvKiBGaXJzdCBjaGVjayBh
bmQgdHJ5IEZMUiAqLw0KPj4+Pj4+Pj4+ICsJaWYgKHBjaWVfaGFzX2ZscihkZXYpKSB7DQo+Pj4+
Pj4+Pj4gKwkJZGV2X2RiZygmZGV2LT5kZXYsICJyZXNldHRpbmcgJXMgZGV2aWNlIHVzaW5nIEZM
UlxuIiwNCj4+Pj4+Pj4+PiArCQkJcGNpX25hbWUoZGV2KSk7DQo+Pj4+Pj4+Pj4gKwkJcGNpZV9m
bHIoZGV2KTsNCj4+Pj4+Pj4+IFRoZSBsYWNrIG9mIGVycm9yIGNoZWNrIGhlcmUgcHV6emxlZCBt
ZSwgYnV0IEkgc2VlIHRoZSBmdW5jdGlvbg0KPj4+Pj4+Pj4gaW5kZWVkIHJldHVybnMgdm9pZCBy
aWdodCBub3cuIEkgdGhpbmsgdGhlIHByZXJlcSBwYXRjaCBzaG91bGQNCj4+Pj4+Pj4+IGNoYW5n
ZSB0aGlzIGFsb25nIHdpdGggZXhwb3J0aW5nIHRoZSBmdW5jdGlvbiAtIHlvdSByZWFsbHkgZG9u
J3QNCj4+Pj4+Pj4+IHdhbnQgdGhlIGRldmljZSB0byBiZSBoYW5kZWQgdG8gYSBndWVzdCB3aGVu
IHRoZSBGTFIgdGltZWQNCj4+Pj4+Pj4+IG91dC4NCj4+Pj4+Pj4gV2Ugd2lsbCBjaGFuZ2UgcGNp
ZV9mbHIoKSB0byByZXR1cm4gZXJyb3IgY29kZS4gSSB3aWxsIG1ha2UgdGhpcyBjaGFuZ2UNCj4+
Pj4+Pj4gaW4gdGhlIG5leHQgdmVyc2lvbiBvZiB0aGlzIHBhdGNoLg0KPj4+Pj4+IEkgZXhjaGFu
Z2VkIHNvbWUgZW1haWxzIHdpdGggQmpvcm4vQ2hyaXN0b3BoIGFuZCBpdCBsb29rcyBsaWtlIENo
cmlzdG9waA0KPj4+Pj4+IGFzIHNvbWUgcGxhbnRvIHJlc3RydWN0dXJlIHBjaWUgZmxyIHNwZWNp
ZmljIGZ1bmN0aW9ucyBidXQgSSBkb24ndCBrbm93DQo+Pj4+Pj4gdGhlIGV4YWN0IHRpbWUtZnJh
bWUuIEZvciBub3csSSBhbSBwbGFubmluZyB0byB1c2UgZXhpc3RpbmcgcGNpZV9mbHIoKQ0KPj4+
Pj4+IGFmdGVyIGNoZWNraW5nIEZMUiBjYXBhYmlsaXR5LiBXZSB3aWxsIHN3aXRjaHRvIHJldmlz
ZWQgcGNpZV9mbHIoKSBvbmNlDQo+Pj4+Pj4gaXQgaXMgYXZhaWxhYmxlLg0KPj4+Pj4+IA0KPj4+
Pj4+IEkgaG9wZSB5b3UgYXJlIGZpbmUgd2l0aCB0aGlzIGFwcHJvYWNoLiBQbGVhc2UgbGV0IG1l
IGtub3cuIFRoYW5rcy4NCj4+Pj4+IEkndmUgc2VlbiB0aGF0IG90aGVyIGRpc2N1c3Npb24uIEkg
ZG9uJ3QgdGhpbmsgdGhlIGNoYW5nZSBoZXJlDQo+Pj4+PiBzaG91bGQgYmUgZG9uZSBwcmlvciB0
byB0aGUgZXJyb3IgcmVwb3J0aW5nIGJlaW5nIHB1dCBpbiBwbGFjZSwNCj4+Pj4+IGZvciBzZWN1
cml0eSByZWFzb25zLiBCdXQgaW4gdGhlIGVuZCBpdCdsbCBiZSBLb25yYWQgYXMgdGhlDQo+Pj4+
PiBtYWludGFpbmVyIHRvIGp1ZGdlLg0KPj4+Pj4gDQo+Pj4+PiBPciB3YWl0LCBsb29rcyBsaWtl
IHRoZXJlJ3Mgc29tZSBjb25mdXNpb24gaW4gLi9NQUlOVEFJTkVSUzoNCj4+Pj4+IEtvbnJhZCBp
cyBsaXN0ZWQgYXMgbWFpbnRhaW5lciBmb3IgIlhFTiBQQ0kgU1VCU1lTVEVNIiwgYnV0IHRoZQ0K
Pj4+Pj4gbGlzdCBvZiBmaWxlcyBkb2Vzbid0IGluY2x1ZGUgcGNpYmFjay4gU28gaXQgd291bGQg
aW5zdGVhZCBiZSBCb3Jpcw0KPj4+Pj4gb3IgSsO8cmdlbiB0byBnaXZlIHlvdSBhIGZpbmFsIHdv
cmQuDQo+Pj4+IA0KPj4+PiBUaGlzIGlzIG5vdyA0LjE2IG1hdGVyaWFsIHNvIHdlIGNhbiBhdCBs
ZWFzdCB3YWl0IHVudGlsIGNsb3NlciB0bw0KPj4+PiBvcGVuaW5nIG9mIHRoZSBtZXJnZSB3aW5k
b3cgd2hlbiB3ZSBtYXkgaGF2ZSB0aGUgUENJIHVwZGF0ZXMuIChBbmQgSQ0KPj4+PiBqdXN0IG5v
dGljZWQgdGhhdCB5b3UgcmVzcG9uZGVkIHRvIENocmlzdG9waC4pDQo+Pj4+IA0KPj4+PiBCZXNp
ZGVzLCB3ZSBkb24ndCB3YW50IHRvIG1ha2Uga2VybmVsIGNoYW5nZXMgdW50aWwgdGhlIGludGVy
ZmFjZSBpcw0KPj4+PiBzZXR0bGVkIChpLmUgdGhlIHRvb2xzdGFjayBjaGFuZ2VzIGFyZSBhY2Nl
cHRlZCkuDQo+Pj4+IA0KPj4+IEl0IHNlZW1zIEdvdmluZGEncyBlbWFpbCBhZGRyZXNzIGlzIGdp
dmluZyBhbiBlcnJvciwgc28gSSBhc3N1bWUgc29tZW9uZSBlbHNlIG5lZWRzIHRvIHBpY2sgdXAg
dGhpcyBwY2liYWNrICdyZXNldCcgZmVhdHVyZS4NCj4+PiBJcyBpdCBsaWtlbHkgc29tZW9uZSBl
bHNlIGZyb20gT3JhY2xlIGNhbi93aWxsIHBpY2sgdXAgYW5kIHJlZnJlc2ggdGhpcyBwYXRjaCwg
d2l0aCB0aGUgcmV2aWV3IGNvbW1lbnRzIGFkZHJlc3NlZD8NCj4+IA0KPj4gDQo+PiBHb3ZpbmRh
IGlzIG5vIGxvbmdlciBhdCBPcmFjbGUuDQo+PiANCj4gDQo+IFllcCwgdGhvdWdodCBzby4gUmVt
b3ZlZCBmcm9tIENDIGxpc3QuDQo+IA0KPiANCj4+IFdoYXQgYWJvdXQgdGhlIHRvb2xzdGFjayBj
aGFuZ2VzPyBIYXZlIHRoZXkgYmVlbiBhY2NlcHRlZD8gSSB2YWd1ZWx5DQo+PiByZWNhbGwgdGhl
cmUgd2FzIGEgZGlzY3Vzc2lvbiBhYm91dCB0aG9zZSBjaGFuZ2VzIGJ1dCBkb24ndCByZW1lbWJl
ciBob3cNCj4+IGl0IGVuZGVkLg0KPj4gDQo+IA0KPiBJIGRvbid0IHRoaW5rIHRvb2xzdGFjay9s
aWJ4bCBwYXRjaCBoYXMgYmVlbiBhcHBsaWVkIHlldCBlaXRoZXIuDQo+IA0KPiANCj4gIltQQVRD
SCBWMSAwLzFdIFhlbi9Ub29sczogUENJIHJlc2V0IHVzaW5nICdyZXNldCcgU3lzRlMgYXR0cmli
dXRlIjoNCj4gaHR0cHM6Ly9saXN0cy54ZW4ub3JnL2FyY2hpdmVzL2h0bWwveGVuLWRldmVsLzIw
MTctMTIvbXNnMDA2NjQuaHRtbA0KPiANCj4gIltQQVRDSCBWMSAxLzFdIFhlbi9saWJ4bDogUGVy
Zm9ybSBQQ0kgcmVzZXQgdXNpbmcgJ3Jlc2V0JyBTeXNGUyBhdHRyaWJ1dGUiOg0KPiBodHRwczov
L2xpc3RzLnhlbi5vcmcvYXJjaGl2ZXMvaHRtbC94ZW4tZGV2ZWwvMjAxNy0xMi9tc2cwMDY2My5o
dG1sDQo+IA0KPiBHZW9yZ2UgYXNrZWQgZm9yIHNvbWUgY2xhcmlmaWNhdGlvbnM6DQo+IGh0dHBz
Oi8vbGlzdHMueGVuLm9yZy9hcmNoaXZlcy9odG1sL3hlbi1kZXZlbC8yMDE3LTEyL21zZzAxMDQ0
Lmh0bWwNCj4gaHR0cHM6Ly9saXN0cy54ZW4ub3JnL2FyY2hpdmVzL2h0bWwveGVuLWRldmVsLzIw
MTctMTIvbXNnMDExMTYuaHRtbA0KDQpSaWdodCwgdGhlIGRlc2NyaXB0aW9uIG9mIHRoZSBwYXRj
aCBkaWRu4oCZdCBhY3R1YWxseSB0ZWxsIHlvdSB3aGF0IHdhcyBnb2luZyBvbi4gIEl0IHNob3Vs
ZCBoYXZlIHNhaWQgc29tZXRoaW5nIGxpa2UsIOKAnHhsIGN1cnJlbnRseSBhdHRlbXB0cyB0byBy
ZXNldCBhIGRldmljZSB1c2luZyBYOyBidXQgdGhhdOKAmXMgbmV2ZXIgYmVlbiBpbXBsZW1lbnRl
ZCBpbiBMaW51eC4gIEluc3RlYWQsIHVzZSBZLCB3aGljaCBbaXMgYmV0dGVyIGZvciB3aGF0ZXZl
ciByZWFzb25d4oCdLg0KDQogLUdlb3JnZQ0KDQo=

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

[Pasi Kärkkäinen]

Hi,

On Mon, Sep 17, 2018 at 02:06:02PM -0400, Boris Ostrovsky wrote:
> On 9/16/18 7:43 AM, Pasi Kärkkäinen wrote:
> > Hi,
> >
> > On Mon, Dec 18, 2017 at 12:32:11PM -0500, Boris Ostrovsky wrote:
> >> On 12/18/2017 02:36 AM, Jan Beulich wrote:
> >>>>>> On 15.12.17 at 20:52, <Govinda.Tatti@Oracle.COM> wrote:
> >>>>>>> +static int pcistub_device_reset(struct pci_dev *dev)
> >>>>>>> +{
> >>>>>>> +	struct xen_pcibk_dev_data *dev_data;
> >>>>>>> +	bool slot = false, bus = false;
> >>>>>>> +	struct pcistub_args arg = {};
> >>>>>>> +
> >>>>>>> +	if (!dev)
> >>>>>>> +		return -EINVAL;
> >>>>>>> +
> >>>>>>> +	dev_dbg(&dev->dev, "[%s]\n", __func__);
> >>>>>>> +
> >>>>>>> +	/* First check and try FLR */
> >>>>>>> +	if (pcie_has_flr(dev)) {
> >>>>>>> +		dev_dbg(&dev->dev, "resetting %s device using FLR\n",
> >>>>>>> +			pci_name(dev));
> >>>>>>> +		pcie_flr(dev);
> >>>>>> The lack of error check here puzzled me, but I see the function
> >>>>>> indeed returns void right now. I think the prereq patch should
> >>>>>> change this along with exporting the function - you really don't
> >>>>>> want the device to be handed to a guest when the FLR timed
> >>>>>> out.
> >>>>> We will change pcie_flr() to return error code. I will make this change
> >>>>> in the next version of this patch.
> >>>> I exchanged some emails with Bjorn/Christoph and it looks like Christoph
> >>>> as some planto restructure pcie flr specific functions but I don't know
> >>>> the exact time-frame. For now,I am planning to use existing pcie_flr()
> >>>> after checking FLR capability. We will switchto revised pcie_flr() once
> >>>> it is available.
> >>>>
> >>>> I hope you are fine with this approach. Please let me know. Thanks.
> >>> I've seen that other discussion. I don't think the change here
> >>> should be done prior to the error reporting being put in place,
> >>> for security reasons. But in the end it'll be Konrad as the
> >>> maintainer to judge.
> >>>
> >>> Or wait, looks like there's some confusion in ./MAINTAINERS:
> >>> Konrad is listed as maintainer for "XEN PCI SUBSYSTEM", but the
> >>> list of files doesn't include pciback. So it would instead be Boris
> >>> or Jürgen to give you a final word.
> >>
> >> This is now 4.16 material so we can at least wait until closer to
> >> opening of the merge window when we may have the PCI updates. (And I
> >> just noticed that you responded to Christoph.)
> >>
> >> Besides, we don't want to make kernel changes until the interface is
> >> settled (i.e the toolstack changes are accepted).
> >>
> > It seems Govinda's email address is giving an error, so I assume someone else needs to pick up this pciback 'reset' feature.
> > Is it likely someone else from Oracle can/will pick up and refresh this patch, with the review comments addressed?
> 
> 
> Govinda is no longer at Oracle.
> 

Yep, thought so. Removed from CC list.


> 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

George asked for some clarifications:
https://lists.xen.org/archives/html/xen-devel/2017-12/msg01044.html
https://lists.xen.org/archives/html/xen-devel/2017-12/msg01116.html

> 
> -boris
> 


Thanks,

-- Pasi

> 
> >
> >
> > Meanwhile the pcie_has_flr() has been exported in upstream Linux kernel, so that's already available for use now.
> >
> > "PCI: Export pcie_has_flr()":
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2d2917f7747805a1f4188672f308d82a8ba01700
> >
> >
> >
> 

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

[Boris Ostrovsky]

On 9/16/18 7:43 AM, Pasi Kärkkäinen wrote:
> Hi,
>
> On Mon, Dec 18, 2017 at 12:32:11PM -0500, Boris Ostrovsky wrote:
>> On 12/18/2017 02:36 AM, Jan Beulich wrote:
>>>>>> On 15.12.17 at 20:52, <Govinda.Tatti@Oracle.COM> wrote:
>>>>>>> +static int pcistub_device_reset(struct pci_dev *dev)
>>>>>>> +{
>>>>>>> +	struct xen_pcibk_dev_data *dev_data;
>>>>>>> +	bool slot = false, bus = false;
>>>>>>> +	struct pcistub_args arg = {};
>>>>>>> +
>>>>>>> +	if (!dev)
>>>>>>> +		return -EINVAL;
>>>>>>> +
>>>>>>> +	dev_dbg(&dev->dev, "[%s]\n", __func__);
>>>>>>> +
>>>>>>> +	/* First check and try FLR */
>>>>>>> +	if (pcie_has_flr(dev)) {
>>>>>>> +		dev_dbg(&dev->dev, "resetting %s device using FLR\n",
>>>>>>> +			pci_name(dev));
>>>>>>> +		pcie_flr(dev);
>>>>>> The lack of error check here puzzled me, but I see the function
>>>>>> indeed returns void right now. I think the prereq patch should
>>>>>> change this along with exporting the function - you really don't
>>>>>> want the device to be handed to a guest when the FLR timed
>>>>>> out.
>>>>> We will change pcie_flr() to return error code. I will make this change
>>>>> in the next version of this patch.
>>>> I exchanged some emails with Bjorn/Christoph and it looks like Christoph
>>>> as some planto restructure pcie flr specific functions but I don't know
>>>> the exact time-frame. For now,I am planning to use existing pcie_flr()
>>>> after checking FLR capability. We will switchto revised pcie_flr() once
>>>> it is available.
>>>>
>>>> I hope you are fine with this approach. Please let me know. Thanks.
>>> I've seen that other discussion. I don't think the change here
>>> should be done prior to the error reporting being put in place,
>>> for security reasons. But in the end it'll be Konrad as the
>>> maintainer to judge.
>>>
>>> Or wait, looks like there's some confusion in ./MAINTAINERS:
>>> Konrad is listed as maintainer for "XEN PCI SUBSYSTEM", but the
>>> list of files doesn't include pciback. So it would instead be Boris
>>> or Jürgen to give you a final word.
>>
>> This is now 4.16 material so we can at least wait until closer to
>> opening of the merge window when we may have the PCI updates. (And I
>> just noticed that you responded to Christoph.)
>>
>> Besides, we don't want to make kernel changes until the interface is
>> settled (i.e the toolstack changes are accepted).
>>
> It seems Govinda's email address is giving an error, so I assume someone else needs to pick up this pciback 'reset' feature.
> Is it likely someone else from Oracle can/will pick up and refresh this patch, with the review comments addressed?


Govinda is no longer at Oracle.

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.


-boris


>
>
> Meanwhile the pcie_has_flr() has been exported in upstream Linux kernel, so that's already available for use now.
>
> "PCI: Export pcie_has_flr()":
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2d2917f7747805a1f4188672f308d82a8ba01700
>
>
>

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

[Pasi Kärkkäinen]

Hi,

On Mon, Dec 18, 2017 at 12:32:11PM -0500, Boris Ostrovsky wrote:
> On 12/18/2017 02:36 AM, Jan Beulich wrote:
> >>>> On 15.12.17 at 20:52, <Govinda.Tatti@Oracle.COM> wrote:
> >>>>> +static int pcistub_device_reset(struct pci_dev *dev)
> >>>>> +{
> >>>>> +	struct xen_pcibk_dev_data *dev_data;
> >>>>> +	bool slot = false, bus = false;
> >>>>> +	struct pcistub_args arg = {};
> >>>>> +
> >>>>> +	if (!dev)
> >>>>> +		return -EINVAL;
> >>>>> +
> >>>>> +	dev_dbg(&dev->dev, "[%s]\n", __func__);
> >>>>> +
> >>>>> +	/* First check and try FLR */
> >>>>> +	if (pcie_has_flr(dev)) {
> >>>>> +		dev_dbg(&dev->dev, "resetting %s device using FLR\n",
> >>>>> +			pci_name(dev));
> >>>>> +		pcie_flr(dev);
> >>>> The lack of error check here puzzled me, but I see the function
> >>>> indeed returns void right now. I think the prereq patch should
> >>>> change this along with exporting the function - you really don't
> >>>> want the device to be handed to a guest when the FLR timed
> >>>> out.
> >>> We will change pcie_flr() to return error code. I will make this change
> >>> in the next version of this patch.
> >> I exchanged some emails with Bjorn/Christoph and it looks like Christoph
> >> as some planto restructure pcie flr specific functions but I don't know
> >> the exact time-frame. For now,I am planning to use existing pcie_flr()
> >> after checking FLR capability. We will switchto revised pcie_flr() once
> >> it is available.
> >>
> >> I hope you are fine with this approach. Please let me know. Thanks.
> > I've seen that other discussion. I don't think the change here
> > should be done prior to the error reporting being put in place,
> > for security reasons. But in the end it'll be Konrad as the
> > maintainer to judge.
> >
> > Or wait, looks like there's some confusion in ./MAINTAINERS:
> > Konrad is listed as maintainer for "XEN PCI SUBSYSTEM", but the
> > list of files doesn't include pciback. So it would instead be Boris
> > or Jürgen to give you a final word.
> 
> 
> This is now 4.16 material so we can at least wait until closer to
> opening of the merge window when we may have the PCI updates. (And I
> just noticed that you responded to Christoph.)
> 
> Besides, we don't want to make kernel changes until the interface is
> settled (i.e the toolstack changes are accepted).
> 

It seems Govinda's email address is giving an error, so I assume someone else needs to pick up this pciback 'reset' feature.
Is it likely someone else from Oracle can/will pick up and refresh this patch, with the review comments addressed?


Meanwhile the pcie_has_flr() has been exported in upstream Linux kernel, so that's already available for use now.

"PCI: Export pcie_has_flr()":
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2d2917f7747805a1f4188672f308d82a8ba01700


> -boris
>


Thanks,

-- Pasi

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

[Govinda Tatti]

On 12/12/2017 9:01 AM, Jan Beulich wrote:
>>>> On 12.12.17 at 15:48, <Govinda.Tatti@Oracle.COM> wrote:
>> Thanks Jan for your review comments. Please see below for my comments.
> First of all - can you please do something about your reply style?
> HTML mail should be avoided. You'll see that the (plain text) reply
> as a result is rather hard to follow, too.
Sorry about it. I had an issue with my Thunderbird setting.
>
>> --- a/Documentation/ABI/testing/sysfs-driver-pciback
>> +++ b/Documentation/ABI/testing/sysfs-driver-pciback
>> @@ -11,3 +11,18 @@ Description:
>>                   #echo 00:19.0-E0:2:FF > /sys/bus/pci/drivers/pciback/quirks
>>                   will allow the guest to read and write to the configuration
>>                   register 0x0E.
>> +
>> +What:           /sys/bus/pci/drivers/pciback/reset
>> +Date:           Dec 2017
>> +KernelVersion:  4.15
>> +Contact:        xen-devel@lists.xenproject.org
>> +Description:
>> +                An option to perform a flr/slot/bus reset when a PCI device
>> + is owned by Xen PCI backend. Writing a string of DDDD:BB:DD.F
>> SSSS:BB:DD.F (or else the D-s are ambiguous, the more that "domain"
>> in Xen code is ambiguous anyway - I continue to be mislead by struct
>> pcistub_device_id's domain field)  Thanks for catching this issue. I will
>> fix it.
>>
>>
>> Also I assume the SSSS part is optional (default zero), which
>> probably can and should be expressed in some way.  SSSS can be 0 or
>> non-zero, subject to system configuration.
> The question isn't system configuration, but whether the field can
> be omitted on input, with zero being assumed in such a case. That's
> a common shorthand, considering that the vast majority of x86
> (and maybe other) systems aren't using segments other than zero
Yes, it can be omitted if SSSS is zero.I will add this information
to above documentation file.

Cheers
GOVINDA