xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V1 0/1] Xen/Tools: PCI reset using 'reset' SysFS attribute
@ 2017-12-07 22:26 Govinda Tatti
  2017-12-07 22:26 ` [PATCH V1 1/1] Xen/libxl: Perform " Govinda Tatti
  0 siblings, 1 reply; 10+ messages in thread
From: Govinda Tatti @ 2017-12-07 22:26 UTC (permalink / raw)
  To: xen-devel, ian.jackson, wei.liu2; +Cc: ross.philipson

This patch contains Xen/libxl specific changes to support PCI reset
(flr/slot/bus) in Xen pciback driver based on SysFS 'reset' attribute.
It depends on the following kernel patch.
- Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS
  attribute

Govinda Tatti (1):
  Xen/libxl: Perform PCI reset using 'reset' SysFS attribute

 tools/libxl/libxl_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.9.5


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

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

* [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-07 22:26 [PATCH V1 0/1] Xen/Tools: PCI reset using 'reset' SysFS attribute Govinda Tatti
@ 2017-12-07 22:26 ` Govinda Tatti
  2017-12-13  1:11   ` Govinda Tatti
  2017-12-13 14:01   ` George Dunlap
  0 siblings, 2 replies; 10+ messages in thread
From: Govinda Tatti @ 2017-12-07 22:26 UTC (permalink / raw)
  To: xen-devel, ian.jackson, wei.liu2; +Cc: ross.philipson

The life-cycle of a PCI device in Xen pciback is complex and is constrained
by the generic PCI locking mechanism.

- It starts with the device being bound to us, for which we do a function
  reset (done via SysFS so the PCI lock is held).
- If the device is unbound from us, we also do a function reset
  (done via SysFS so the PCI lock is held).
- If the device is un-assigned from a guest - we do a function reset
  (no PCI lock is held).

All reset operations are done on the individual PCI function level
(so bus:device:function).

The reset for an individual PCI function means device must support FLR
(PCIe or AF), PM reset on D3hot->D0 device specific reset, or a secondary
bus reset for a singleton device on a bus but FLR does not have widespread
support or it is not reliable in some cases. So, we need to provide an
alternate mechanism to users to perform a slot or bus level reset.

Currently, a slot or bus reset is not exposed in SysFS as there is no good
way of exposing a bus topology there. This is due to the complexity -
we MUST know that the different functions of a PCIe device are not in use
by other drivers, or if they are in use (say one of them is assigned to a
guest and the other is  idle) - it is still OK to reset the slot (assuming
both of them are owned by Xen pciback).

This patch does that providing an option to perform a flr/slot/bus reset
when a PCI device is owned by Xen PCI backend. It will try to execute one
of these reset method, starting with FLR if it is supported. Otherwise,
it tries slot or bus reset method. For slot or bus reset method, it also
checks to make sure that all of the devices under the bridge are owned by
Xen PCI backend before applying those resets.

Due to the complexity with the PCI lock we cannot do the reset when a
device is bound ('echo $BDF > bind') or when unbound ('echo $BDF > unbind')
as the pci_[slot|bus]_reset also takes the same lock resulting in a
dead-lock.

Putting the reset function in a work-queue or thread won't work either -
as we have to do the reset function outside the 'unbind' context (it holds
the PCI lock). But once you 'unbind' a device the device is no longer under
the ownership of Xen pciback and the pci_set_drvdata has been reset, so
we cannot use a thread for this.

Instead of doing all this complex dance, we depend on the tool-stack doing
the right thing. As such, we implement 'reset' SysFS attribute which 'xl'
uses when a device is detached or attached from/to a guest. It bypasses
the need to worry about the PCI lock. BTW, previously defined "do_flr"
attribute has been renamed to "reset" since "do_flr" name doesn't represent
all PCI reset methods and plus, currently it is not being used. So, there
is no impact in renaming this sysfs attribute.

To not inadvertently do a bus reset that would affect devices that are in
use by other drivers (other than Xen pciback) prior to the reset, we check
that all of the devices under the bridge are owned by Xen pciback. If they
are not, we refrain from executing the bus (or slot) reset.

Signed-off-by: Govinda Tatti <Govinda.Tatti@Oracle.COM>
---
v1: -New

 tools/libxl/libxl_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b14df16..9d00cb1 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1126,7 +1126,7 @@ static int libxl__device_pci_reset(libxl__gc *gc, unsigned int domain, unsigned
     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);
-- 
2.9.5


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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-07 22:26 ` [PATCH V1 1/1] Xen/libxl: Perform " Govinda Tatti
@ 2017-12-13  1:11   ` Govinda Tatti
  2017-12-13 14:01   ` George Dunlap
  1 sibling, 0 replies; 10+ messages in thread
From: Govinda Tatti @ 2017-12-13  1:11 UTC (permalink / raw)
  To: xen-devel, ian.jackson, wei.liu2; +Cc: ross.philipson

Any comments on this patch?. Thanks.

On 12/7/2017 4:26 PM, Govinda Tatti wrote:
> The life-cycle of a PCI device in Xen pciback is complex and is constrained
> by the generic PCI locking mechanism.
>
> - It starts with the device being bound to us, for which we do a function
>    reset (done via SysFS so the PCI lock is held).
> - If the device is unbound from us, we also do a function reset
>    (done via SysFS so the PCI lock is held).
> - If the device is un-assigned from a guest - we do a function reset
>    (no PCI lock is held).
>
> All reset operations are done on the individual PCI function level
> (so bus:device:function).
>
> The reset for an individual PCI function means device must support FLR
> (PCIe or AF), PM reset on D3hot->D0 device specific reset, or a secondary
> bus reset for a singleton device on a bus but FLR does not have widespread
> support or it is not reliable in some cases. So, we need to provide an
> alternate mechanism to users to perform a slot or bus level reset.
>
> Currently, a slot or bus reset is not exposed in SysFS as there is no good
> way of exposing a bus topology there. This is due to the complexity -
> we MUST know that the different functions of a PCIe device are not in use
> by other drivers, or if they are in use (say one of them is assigned to a
> guest and the other is  idle) - it is still OK to reset the slot (assuming
> both of them are owned by Xen pciback).
>
> This patch does that providing an option to perform a flr/slot/bus reset
> when a PCI device is owned by Xen PCI backend. It will try to execute one
> of these reset method, starting with FLR if it is supported. Otherwise,
> it tries slot or bus reset method. For slot or bus reset method, it also
> checks to make sure that all of the devices under the bridge are owned by
> Xen PCI backend before applying those resets.
>
> Due to the complexity with the PCI lock we cannot do the reset when a
> device is bound ('echo $BDF > bind') or when unbound ('echo $BDF > unbind')
> as the pci_[slot|bus]_reset also takes the same lock resulting in a
> dead-lock.
>
> Putting the reset function in a work-queue or thread won't work either -
> as we have to do the reset function outside the 'unbind' context (it holds
> the PCI lock). But once you 'unbind' a device the device is no longer under
> the ownership of Xen pciback and the pci_set_drvdata has been reset, so
> we cannot use a thread for this.
>
> Instead of doing all this complex dance, we depend on the tool-stack doing
> the right thing. As such, we implement 'reset' SysFS attribute which 'xl'
> uses when a device is detached or attached from/to a guest. It bypasses
> the need to worry about the PCI lock. BTW, previously defined "do_flr"
> attribute has been renamed to "reset" since "do_flr" name doesn't represent
> all PCI reset methods and plus, currently it is not being used. So, there
> is no impact in renaming this sysfs attribute.
>
> To not inadvertently do a bus reset that would affect devices that are in
> use by other drivers (other than Xen pciback) prior to the reset, we check
> that all of the devices under the bridge are owned by Xen pciback. If they
> are not, we refrain from executing the bus (or slot) reset.
>
> Signed-off-by: Govinda Tatti <Govinda.Tatti@Oracle.COM>
> ---
> v1: -New
>
>   tools/libxl/libxl_pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index b14df16..9d00cb1 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1126,7 +1126,7 @@ static int libxl__device_pci_reset(libxl__gc *gc, unsigned int domain, unsigned
>       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);
>

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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-07 22:26 ` [PATCH V1 1/1] Xen/libxl: Perform " Govinda Tatti
  2017-12-13  1:11   ` Govinda Tatti
@ 2017-12-13 14:01   ` George Dunlap
  2017-12-13 21:05     ` Govinda Tatti
  1 sibling, 1 reply; 10+ messages in thread
From: George Dunlap @ 2017-12-13 14:01 UTC (permalink / raw)
  To: Govinda Tatti; +Cc: Wei Liu, Ian Jackson, ross.philipson, xen-devel

On Thu, Dec 7, 2017 at 10:26 PM, Govinda Tatti <Govinda.Tatti@oracle.com> wrote:
> The life-cycle of a PCI device in Xen pciback is complex and is constrained
> by the generic PCI locking mechanism.
>
> - It starts with the device being bound to us, for which we do a function
>   reset (done via SysFS so the PCI lock is held).
> - If the device is unbound from us, we also do a function reset
>   (done via SysFS so the PCI lock is held).
> - If the device is un-assigned from a guest - we do a function reset
>   (no PCI lock is held).
>
> All reset operations are done on the individual PCI function level
> (so bus:device:function).
>
> The reset for an individual PCI function means device must support FLR
> (PCIe or AF), PM reset on D3hot->D0 device specific reset, or a secondary
> bus reset for a singleton device on a bus but FLR does not have widespread
> support or it is not reliable in some cases. So, we need to provide an
> alternate mechanism to users to perform a slot or bus level reset.
>
> Currently, a slot or bus reset is not exposed in SysFS as there is no good
> way of exposing a bus topology there. This is due to the complexity -
> we MUST know that the different functions of a PCIe device are not in use
> by other drivers, or if they are in use (say one of them is assigned to a
> guest and the other is  idle) - it is still OK to reset the slot (assuming
> both of them are owned by Xen pciback).
>
> This patch does that providing an option to perform a flr/slot/bus reset
> when a PCI device is owned by Xen PCI backend. It will try to execute one
> of these reset method, starting with FLR if it is supported. Otherwise,
> it tries slot or bus reset method. For slot or bus reset method, it also
> checks to make sure that all of the devices under the bridge are owned by
> Xen PCI backend before applying those resets.
>
> Due to the complexity with the PCI lock we cannot do the reset when a
> device is bound ('echo $BDF > bind') or when unbound ('echo $BDF > unbind')
> as the pci_[slot|bus]_reset also takes the same lock resulting in a
> dead-lock.
>
> Putting the reset function in a work-queue or thread won't work either -
> as we have to do the reset function outside the 'unbind' context (it holds
> the PCI lock). But once you 'unbind' a device the device is no longer under
> the ownership of Xen pciback and the pci_set_drvdata has been reset, so
> we cannot use a thread for this.
>
> Instead of doing all this complex dance, we depend on the tool-stack doing
> the right thing. As such, we implement 'reset' SysFS attribute which 'xl'
> uses when a device is detached or attached from/to a guest. It bypasses
> the need to worry about the PCI lock. BTW, previously defined "do_flr"
> attribute has been renamed to "reset" since "do_flr" name doesn't represent
> all PCI reset methods and plus, currently it is not being used. So, there
> is no impact in renaming this sysfs attribute.
>
> To not inadvertently do a bus reset that would affect devices that are in
> use by other drivers (other than Xen pciback) prior to the reset, we check
> that all of the devices under the bridge are owned by Xen pciback. If they
> are not, we refrain from executing the bus (or slot) reset.

There's an awful lot of stuff here, but only a single line of code
change, which makes it difficult to tell what's going on.

It sounds like you're making changes to Linux to solve the problems
you describe, and modifying xl so that it works with this new
interface?

In which case, xl needs to be backwards-compatible with kernels that
don't have your new feature: it will have to check for %s/reset, and
if it's not there, then try %/do_flr.

 -George

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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-13 14:01   ` George Dunlap
@ 2017-12-13 21:05     ` Govinda Tatti
  2017-12-14 10:31       ` George Dunlap
  0 siblings, 1 reply; 10+ messages in thread
From: Govinda Tatti @ 2017-12-13 21:05 UTC (permalink / raw)
  To: George Dunlap; +Cc: Wei Liu, Ian Jackson, ross.philipson, xen-devel


Thanks George for your review comments. Please see below for my comments.

On 12/13/2017 8:01 AM, George Dunlap wrote:
> On Thu, Dec 7, 2017 at 10:26 PM, Govinda Tatti <Govinda.Tatti@oracle.com> wrote:
>> The life-cycle of a PCI device in Xen pciback is complex and is constrained
>> by the generic PCI locking mechanism.
>>
>> - It starts with the device being bound to us, for which we do a function
>>    reset (done via SysFS so the PCI lock is held).
>> - If the device is unbound from us, we also do a function reset
>>    (done via SysFS so the PCI lock is held).
>> - If the device is un-assigned from a guest - we do a function reset
>>    (no PCI lock is held).
>>
>> All reset operations are done on the individual PCI function level
>> (so bus:device:function).
>>
>> The reset for an individual PCI function means device must support FLR
>> (PCIe or AF), PM reset on D3hot->D0 device specific reset, or a secondary
>> bus reset for a singleton device on a bus but FLR does not have widespread
>> support or it is not reliable in some cases. So, we need to provide an
>> alternate mechanism to users to perform a slot or bus level reset.
>>
>> Currently, a slot or bus reset is not exposed in SysFS as there is no good
>> way of exposing a bus topology there. This is due to the complexity -
>> we MUST know that the different functions of a PCIe device are not in use
>> by other drivers, or if they are in use (say one of them is assigned to a
>> guest and the other is  idle) - it is still OK to reset the slot (assuming
>> both of them are owned by Xen pciback).
>>
>> This patch does that providing an option to perform a flr/slot/bus reset
>> when a PCI device is owned by Xen PCI backend. It will try to execute one
>> of these reset method, starting with FLR if it is supported. Otherwise,
>> it tries slot or bus reset method. For slot or bus reset method, it also
>> checks to make sure that all of the devices under the bridge are owned by
>> Xen PCI backend before applying those resets.
>>
>> Due to the complexity with the PCI lock we cannot do the reset when a
>> device is bound ('echo $BDF > bind') or when unbound ('echo $BDF > unbind')
>> as the pci_[slot|bus]_reset also takes the same lock resulting in a
>> dead-lock.
>>
>> Putting the reset function in a work-queue or thread won't work either -
>> as we have to do the reset function outside the 'unbind' context (it holds
>> the PCI lock). But once you 'unbind' a device the device is no longer under
>> the ownership of Xen pciback and the pci_set_drvdata has been reset, so
>> we cannot use a thread for this.
>>
>> Instead of doing all this complex dance, we depend on the tool-stack doing
>> the right thing. As such, we implement 'reset' SysFS attribute which 'xl'
>> uses when a device is detached or attached from/to a guest. It bypasses
>> the need to worry about the PCI lock. BTW, previously defined "do_flr"
>> attribute has been renamed to "reset" since "do_flr" name doesn't represent
>> all PCI reset methods and plus, currently it is not being used. So, there
>> is no impact in renaming this sysfs attribute.
>>
>> To not inadvertently do a bus reset that would affect devices that are in
>> use by other drivers (other than Xen pciback) prior to the reset, we check
>> that all of the devices under the bridge are owned by Xen pciback. If they
>> are not, we refrain from executing the bus (or slot) reset.
> There's an awful lot of stuff here, but only a single line of code
> change, which makes it difficult to tell what's going on.
>
> It sounds like you're making changes to Linux to solve the problems
> you describe, and modifying xl so that it works with this new
> interface?
The "reset" SysFS attribute provides an option for xl to perform a PCI reset
(FLR/SLOT/BUS, one of them and based on its support)when a PCI device is 
being
added/removed to/from Xen guest.


>
> In which case, xl needs to be backwards-compatible with kernels that
> don't have your new feature: it will have to check for %s/reset, and
> if it's not there, then try %/do_flr.
I think this fix was planned more than a year back and even we pushed 
libxl fix
("do_flr" SysFSattribute) but linux kernel fix was not integrated for 
some reason.
Now, we are revisitingboth linux kernel and libxl changes. In other-words,
"do_flr" change is not being usedtoday since we don't have required code 
changes
in the linux kernel.

Cheers
GOVINDA

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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-13 21:05     ` Govinda Tatti
@ 2017-12-14 10:31       ` George Dunlap
  2017-12-14 20:22         ` Govinda Tatti
  0 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2017-12-14 10:31 UTC (permalink / raw)
  To: Govinda Tatti; +Cc: Wei Liu, Ian Jackson, ross.philipson, xen-devel

On Wed, Dec 13, 2017 at 9:05 PM, Govinda Tatti <Govinda.Tatti@oracle.com> wrote:
>
> Thanks George for your review comments. Please see below for my comments.
>
>
> On 12/13/2017 8:01 AM, George Dunlap wrote:
>>
>> On Thu, Dec 7, 2017 at 10:26 PM, Govinda Tatti <Govinda.Tatti@oracle.com>
>> wrote:
>>>
>>> The life-cycle of a PCI device in Xen pciback is complex and is
>>> constrained
>>> by the generic PCI locking mechanism.
>>>
>>> - It starts with the device being bound to us, for which we do a function
>>>    reset (done via SysFS so the PCI lock is held).
>>> - If the device is unbound from us, we also do a function reset
>>>    (done via SysFS so the PCI lock is held).
>>> - If the device is un-assigned from a guest - we do a function reset
>>>    (no PCI lock is held).
>>>
>>> All reset operations are done on the individual PCI function level
>>> (so bus:device:function).
>>>
>>> The reset for an individual PCI function means device must support FLR
>>> (PCIe or AF), PM reset on D3hot->D0 device specific reset, or a secondary
>>> bus reset for a singleton device on a bus but FLR does not have
>>> widespread
>>> support or it is not reliable in some cases. So, we need to provide an
>>> alternate mechanism to users to perform a slot or bus level reset.
>>>
>>> Currently, a slot or bus reset is not exposed in SysFS as there is no
>>> good
>>> way of exposing a bus topology there. This is due to the complexity -
>>> we MUST know that the different functions of a PCIe device are not in use
>>> by other drivers, or if they are in use (say one of them is assigned to a
>>> guest and the other is  idle) - it is still OK to reset the slot
>>> (assuming
>>> both of them are owned by Xen pciback).
>>>
>>> This patch does that providing an option to perform a flr/slot/bus reset
>>> when a PCI device is owned by Xen PCI backend. It will try to execute one
>>> of these reset method, starting with FLR if it is supported. Otherwise,
>>> it tries slot or bus reset method. For slot or bus reset method, it also
>>> checks to make sure that all of the devices under the bridge are owned by
>>> Xen PCI backend before applying those resets.
>>>
>>> Due to the complexity with the PCI lock we cannot do the reset when a
>>> device is bound ('echo $BDF > bind') or when unbound ('echo $BDF >
>>> unbind')
>>> as the pci_[slot|bus]_reset also takes the same lock resulting in a
>>> dead-lock.
>>>
>>> Putting the reset function in a work-queue or thread won't work either -
>>> as we have to do the reset function outside the 'unbind' context (it
>>> holds
>>> the PCI lock). But once you 'unbind' a device the device is no longer
>>> under
>>> the ownership of Xen pciback and the pci_set_drvdata has been reset, so
>>> we cannot use a thread for this.
>>>
>>> Instead of doing all this complex dance, we depend on the tool-stack
>>> doing
>>> the right thing. As such, we implement 'reset' SysFS attribute which 'xl'
>>> uses when a device is detached or attached from/to a guest. It bypasses
>>> the need to worry about the PCI lock. BTW, previously defined "do_flr"
>>> attribute has been renamed to "reset" since "do_flr" name doesn't
>>> represent
>>> all PCI reset methods and plus, currently it is not being used. So, there
>>> is no impact in renaming this sysfs attribute.
>>>
>>> To not inadvertently do a bus reset that would affect devices that are in
>>> use by other drivers (other than Xen pciback) prior to the reset, we
>>> check
>>> that all of the devices under the bridge are owned by Xen pciback. If
>>> they
>>> are not, we refrain from executing the bus (or slot) reset.
>>
>> There's an awful lot of stuff here, but only a single line of code
>> change, which makes it difficult to tell what's going on.
>>
>> It sounds like you're making changes to Linux to solve the problems
>> you describe, and modifying xl so that it works with this new
>> interface?
>
> The "reset" SysFS attribute provides an option for xl to perform a PCI reset
> (FLR/SLOT/BUS, one of them and based on its support)when a PCI device is
> being
> added/removed to/from Xen guest.

But you didn't answer my question.  The changeset here says,
"Implement the reset SysFS attribute".  But this changset doesn't do
any such thing.

Is it the case that most of the "fixes" you describe with locking are
happening inside of Linux, and that all this changset does is modify
libxl to use 'reset' instead of 'do_flr'?

>> In which case, xl needs to be backwards-compatible with kernels that
>> don't have your new feature: it will have to check for %s/reset, and
>> if it's not there, then try %/do_flr.
>
> I think this fix was planned more than a year back and even we pushed libxl
> fix
> ("do_flr" SysFSattribute) but linux kernel fix was not integrated for some
> reason.
> Now, we are revisitingboth linux kernel and libxl changes. In other-words,
> "do_flr" change is not being usedtoday since we don't have required code
> changes
> in the linux kernel.

Are you saying do_flr doesn't exist at all in any version of Linux,
and as such the line you're removing is currently pointless?

 -George

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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-14 10:31       ` George Dunlap
@ 2017-12-14 20:22         ` Govinda Tatti
  2017-12-14 21:31           ` Håkon Alstadheim
  0 siblings, 1 reply; 10+ messages in thread
From: Govinda Tatti @ 2017-12-14 20:22 UTC (permalink / raw)
  To: George Dunlap; +Cc: Wei Liu, Ian Jackson, ross.philipson, xen-devel


>>> In which case, xl needs to be backwards-compatible with kernels that
>>> don't have your new feature: it will have to check for %s/reset, and
>>> if it's not there, then try %/do_flr.
>> I think this fix was planned more than a year back and even we pushed libxl
>> fix
>> ("do_flr" SysFSattribute) but linux kernel fix was not integrated for some
>> reason.
>> Now, we are revisitingboth linux kernel and libxl changes. In other-words,
>> "do_flr" change is not being usedtoday since we don't have required code
>> changes
>> in the linux kernel.
> Are you saying do_flr doesn't exist at all in any version of Linux,
> and as such the line you're removing is currently pointless?
Yes, that's correct. In other-words, it will not break any existing code
or functionality.

Cheers
GOVINDA

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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-14 20:22         ` Govinda Tatti
@ 2017-12-14 21:31           ` Håkon Alstadheim
  2017-12-18 18:33             ` Govinda Tatti
  0 siblings, 1 reply; 10+ messages in thread
From: Håkon Alstadheim @ 2017-12-14 21:31 UTC (permalink / raw)
  To: xen-devel



Den 14. des. 2017 21:22, skrev Govinda Tatti:
> 
>>>> In which case, xl needs to be backwards-compatible with kernels that
>>>> don't have your new feature: it will have to check for %s/reset, and
>>>> if it's not there, then try %/do_flr.
>>> I think this fix was planned more than a year back and even we pushed
>>> libxl
>>> fix
>>> ("do_flr" SysFSattribute) but linux kernel fix was not integrated for
>>> some
>>> reason.
>>> Now, we are revisitingboth linux kernel and libxl changes. In
>>> other-words,
>>> "do_flr" change is not being usedtoday since we don't have required code
>>> changes
>>> in the linux kernel.
>> Are you saying do_flr doesn't exist at all in any version of Linux,
>> and as such the line you're removing is currently pointless?
> Yes, that's correct. In other-words, it will not break any existing code
> or functionality.

Except for people, like me, running unofficial patches to linux. It
should be OK to assume they are watching this thread.

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

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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-14 21:31           ` Håkon Alstadheim
@ 2017-12-18 18:33             ` Govinda Tatti
  2017-12-20 21:39               ` Håkon Alstadheim
  0 siblings, 1 reply; 10+ messages in thread
From: Govinda Tatti @ 2017-12-18 18:33 UTC (permalink / raw)
  To: Håkon Alstadheim; +Cc: xen-devel

>>> Are you saying do_flr doesn't exist at all in any version of Linux,
>>> and as such the line you're removing is currently pointless?
>> Yes, that's correct. In other-words, it will not break any existing code
>> or functionality.
> Except for people, like me, running unofficial patches to linux. It
> should be OK to assume they are watching this thread.
Do we need to account for unofficial patches or usage of do_flr?. If yes,
we need to maintainbackward compatibility for do_flr attribute.

Cheers
GOVINDA


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

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

* Re: [PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS attribute
  2017-12-18 18:33             ` Govinda Tatti
@ 2017-12-20 21:39               ` Håkon Alstadheim
  0 siblings, 0 replies; 10+ messages in thread
From: Håkon Alstadheim @ 2017-12-20 21:39 UTC (permalink / raw)
  To: xen-devel



Den 18. des. 2017 19:33, skrev Govinda Tatti:
>>>> Are you saying do_flr doesn't exist at all in any version of Linux,
>>>> and as such the line you're removing is currently pointless?
>>> Yes, that's correct. In other-words, it will not break any existing code
>>> or functionality.
>> Except for people, like me, running unofficial patches to linux. It
>> should be OK to assume they are watching this thread.
> Do we need to account for unofficial patches or usage of do_flr?. If yes,
> we need to maintainbackward compatibility for do_flr attribute.

When the final, official change to the linux backend driver goes in,
local patches will no longer apply.

The cause should be obvious to anybody dealing with patching the linux
sources on their own. Running newer kernels as dom0 on old Xen would
just mean those people would need a different (simpler) patch.

As a convenience, I'd like a heads up before the interface on the
xen/libxl side changes, so I can make sure to run a compatible dom0
kernel. An entry in changelog should suffice.

A wishlist item might be to have libxl error message be as fool-proof as
possible. "Failed to access pciback path %s" could perhaps be improved
upon. Perhaps add "make sure pciback version is compatible with libxl
version".

Adding backwards compatibility code just for this would be unnecessary
cruft as far as I can see.

I'll go back to lurking now.


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

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

end of thread, other threads:[~2017-12-20 21:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-07 22:26 [PATCH V1 0/1] Xen/Tools: PCI reset using 'reset' SysFS attribute Govinda Tatti
2017-12-07 22:26 ` [PATCH V1 1/1] Xen/libxl: Perform " Govinda Tatti
2017-12-13  1:11   ` Govinda Tatti
2017-12-13 14:01   ` George Dunlap
2017-12-13 21:05     ` Govinda Tatti
2017-12-14 10:31       ` George Dunlap
2017-12-14 20:22         ` Govinda Tatti
2017-12-14 21:31           ` Håkon Alstadheim
2017-12-18 18:33             ` Govinda Tatti
2017-12-20 21:39               ` Håkon Alstadheim

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).