xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
@ 2020-01-17 19:13 Rich Persaud
  2020-01-23  9:12 ` Pasi Kärkkäinen
  2020-01-31 15:33 ` Wei Liu
  0 siblings, 2 replies; 9+ messages in thread
From: Rich Persaud @ 2020-01-17 19:13 UTC (permalink / raw)
  To: Pasi Kärkkäinen, xen-devel, Boris Ostrovsky
  Cc: Juergen Gross, Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper,
	Paul Durrant, Jason Andryuk, George Dunlap,
	Marek Marczykowski-Górecki, Anthony Perard, Jan Beulich,
	bhelgaas, Ian Jackson, Roger Pau Monné,
	Håkon Alstadheim


[-- 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-01-17 19:13 [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute Rich Persaud
@ 2020-01-23  9:12 ` Pasi Kärkkäinen
  2020-01-31 15:33 ` Wei Liu
  1 sibling, 0 replies; 9+ messages in thread
From: Pasi Kärkkäinen @ 2020-01-23  9:12 UTC (permalink / raw)
  To: Rich Persaud
  Cc: Juergen Gross, Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper,
	Paul Durrant, Jason Andryuk, George Dunlap,
	Marek Marczykowski-Górecki, Anthony Perard, Ian Jackson,
	bhelgaas, xen-devel, Boris Ostrovsky, Roger Pau Monné,
	Håkon Alstadheim

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-01-17 19:13 [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute Rich Persaud
  2020-01-23  9:12 ` Pasi Kärkkäinen
@ 2020-01-31 15:33 ` Wei Liu
  2020-10-19 11:00   ` George Dunlap
  1 sibling, 1 reply; 9+ messages in thread
From: Wei Liu @ 2020-01-31 15:33 UTC (permalink / raw)
  To: Rich Persaud
  Cc: Juergen Gross, Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper,
	Paul Durrant, Jason Andryuk, George Dunlap,
	Marek Marczykowski-Górecki, Anthony Perard, Ian Jackson,
	bhelgaas, xen-devel, Boris Ostrovsky, Roger Pau Monné,
	Håkon Alstadheim

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-01-31 15:33 ` Wei Liu
@ 2020-10-19 11:00   ` George Dunlap
  2020-10-19 15:16     ` Håkon Alstadheim
  0 siblings, 1 reply; 9+ messages in thread
From: George Dunlap @ 2020-10-19 11:00 UTC (permalink / raw)
  To: Wei Liu
  Cc: Rich Persaud, Pasi Kärkkäinen, xen-devel,
	Boris Ostrovsky, Juergen Gross, Konrad Rzeszutek Wilk,
	Jan Beulich, bhelgaas, Håkon Alstadheim, Roger Pau Monne,
	Marek Marczykowski-Górecki, Jason Andryuk, Andrew Cooper,
	Ian Jackson, Paul Durrant, Anthony Perard



> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-10-19 11:00   ` George Dunlap
@ 2020-10-19 15:16     ` Håkon Alstadheim
  2020-10-19 15:52       ` Håkon Alstadheim
  0 siblings, 1 reply; 9+ messages in thread
From: Håkon Alstadheim @ 2020-10-19 15:16 UTC (permalink / raw)
  To: George Dunlap, Wei Liu
  Cc: Rich Persaud, Pasi Kärkkäinen, xen-devel,
	Boris Ostrovsky, Juergen Gross, Konrad Rzeszutek Wilk,
	Jan Beulich, bhelgaas, Roger Pau Monne,
	Marek Marczykowski-Górecki, Jason Andryuk, Andrew Cooper,
	Ian Jackson, Paul Durrant, Anthony Perard

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.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-10-19 15:16     ` Håkon Alstadheim
@ 2020-10-19 15:52       ` Håkon Alstadheim
  2020-10-19 22:43         ` Rich Persaud
  0 siblings, 1 reply; 9+ messages in thread
From: Håkon Alstadheim @ 2020-10-19 15:52 UTC (permalink / raw)
  To: George Dunlap, Wei Liu
  Cc: Rich Persaud, Pasi Kärkkäinen, xen-devel,
	Boris Ostrovsky, Juergen Gross, Konrad Rzeszutek Wilk,
	Jan Beulich, bhelgaas, Roger Pau Monne,
	Marek Marczykowski-Górecki, Jason Andryuk, Andrew Cooper,
	Ian Jackson, Paul Durrant, Anthony Perard

[-- 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);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-10-19 15:52       ` Håkon Alstadheim
@ 2020-10-19 22:43         ` Rich Persaud
  2020-10-19 23:49           ` boris.ostrovsky
  0 siblings, 1 reply; 9+ messages in thread
From: Rich Persaud @ 2020-10-19 22:43 UTC (permalink / raw)
  To: Håkon Alstadheim, George Dunlap
  Cc: Wei Liu, Pasi Kärkkäinen, xen-devel, Boris Ostrovsky,
	Juergen Gross, Konrad Rzeszutek Wilk, Jan Beulich, bhelgaas,
	Roger Pau Monne, Marek Marczykowski-Górecki, Jason Andryuk,
	Andrew Cooper, Ian Jackson, Paul Durrant, Anthony Perard

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
  2020-10-19 22:43         ` Rich Persaud
@ 2020-10-19 23:49           ` boris.ostrovsky
  0 siblings, 0 replies; 9+ messages in thread
From: boris.ostrovsky @ 2020-10-19 23:49 UTC (permalink / raw)
  To: Rich Persaud, Håkon Alstadheim, George Dunlap
  Cc: Wei Liu, Pasi Kärkkäinen, xen-devel, Juergen Gross,
	Konrad Rzeszutek Wilk, Jan Beulich, bhelgaas, Roger Pau Monne,
	Marek Marczykowski-Górecki, Jason Andryuk, Andrew Cooper,
	Ian Jackson, Paul Durrant, Anthony Perard


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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
       [not found]                 ` <f6b8e055-7afc-b4de-af88-425d799dcd28@oracle.com>
@ 2019-08-26 21:05                   ` Pasi Kärkkäinen
  0 siblings, 0 replies; 9+ messages in thread
From: Pasi Kärkkäinen @ 2019-08-26 21:05 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Juergen Gross, Konrad Rzeszutek Wilk, linux-pci, linux-kernel,
	George Dunlap, Jan Beulich, bhelgaas, xen-devel,
	Håkon Alstadheim, Roger Pau Monné

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-10-19 23:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 19:13 [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute Rich Persaud
2020-01-23  9:12 ` Pasi Kärkkäinen
2020-01-31 15:33 ` Wei Liu
2020-10-19 11:00   ` George Dunlap
2020-10-19 15:16     ` Håkon Alstadheim
2020-10-19 15:52       ` Håkon Alstadheim
2020-10-19 22:43         ` Rich Persaud
2020-10-19 23:49           ` boris.ostrovsky
     [not found] <5A377E020200007800197FFA@prv-mh.provo.novell.com>
     [not found] ` <559ffd12-b541-8a69-60bd-fbe10e3dc159@oracle.com>
     [not found]   ` <20180916114306.GF18222@reaktio.net>
     [not found]     ` <a726840b-8a5c-0890-73c6-3a95a7205153@oracle.com>
     [not found]       ` <20180918071519.GG18222@reaktio.net>
     [not found]         ` <5E7DDB68-4E68-48A5-AEEC-EE1B21A50E9E@citrix.com>
     [not found]           ` <352310b3-ec9b-2ceb-83f0-4550718120c3@oracle.com>
     [not found]             ` <20180919090526.s3ahnemrt2ik2tx3@mac.bytemobile.com>
     [not found]               ` <20181003155104.GH5318@reaktio.net>
     [not found]                 ` <f6b8e055-7afc-b4de-af88-425d799dcd28@oracle.com>
2019-08-26 21:05                   ` Pasi Kärkkäinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).