fm10k: restore D0 power state upon probe

PATCH New Jacob Keller 3 participants 15 messages Jun 8, 2016 – Jun 9, 2016

Subsystems (2)

Network drivers Intel Ethernet

lore

Share & tools

lore

Jacob Keller

author

2016-06-08 00:20 UTC #0 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

It seems that under some circumstances, such as after performing an unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.

Fix this by always calling pci_set_power_state(pdev, PCI_D0) on driver device probe.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +2 -0

index b8245c734c96..f2b7b8447cb7 100644
@@ -1975,6 +1975,8 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_enable_pcie_error_reporting(pdev);
 
+	pci_set_power_state(pdev, PCI_D0);
+
 	pci_set_master(pdev);
 	pci_save_state(pdev);

Signature · 1 line

2.9.0.rc1.405.g81f467e

Alexander Duyck

2016-06-08 03:54 UTC #1 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 4 lines · Jacob Keller

It seems that under some circumstances, such as after performing an unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.

That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.

Quoted 2 lines

Fix this by always calling pci_set_power_state(pdev, PCI_D0) on driver device probe.

That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long as nobody else has enabled the device it will be ran.

Quoted 10 lines

Signed-off-by Jacob Keller <[email protected]>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0 -0
index b8245c734c96..f2b7b8447cb7 100644

drivers/net/ethernet/intel/fm10k/fm10k_pci.c static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent) · line 1981

Quoted 7 lines

drivers/net/ethernet/intel/fm10k/fm10k_pci.c +2 -0

@@ -1975,6 +1975,8 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
        pci_enable_pcie_error_reporting(pdev);
 
+       pci_set_power_state(pdev, PCI_D0);
+
        pci_set_master(pdev);
        pci_save_state(pdev);

The placement of this seems a bit suspicious to me. Is it that you cannot enable the D0 power state until after error reporting has been enabled or is there some other reason to place it out there rather than enabling D0 before performing any other operations on the device.

It might make more sense to offload this work to the PF and have it reset the power state on the VF from underneath rather than relying on it being performed by the VF once it is in the guest. There might be some hypervisors that prevent the guest from being able to make power state changes for devices on the host.

- Alex

Keller

2016-06-08 19:11 UTC #2 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 19 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 4 lines · Jacob Keller
It seems that under some circumstances, such as after performing an unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.

Quoted 7 lines

Quoted 2 lines
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on driver device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long as nobody else has enabled the device it will be ran.

That's weird. In this case, after doing aer-inject on the PF, if we unbind then bind the VF device, it fails in pci_enable_msix_range which I traced into eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it works fine...?

I don't really know why that'd be the case... thoughts?

Thanks,
Jake

Alexander Duyck

2016-06-08 21:02 UTC #3 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 22 lines · Jacob E

Quoted 22 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 4 lines · Jacob Keller
It seems that under some circumstances, such as after performing an unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 2 lines
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on driver device probe.

Quoted 8 lines

Quoted 4 lines
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long as nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we unbind then bind the VF device, it fails in pci_enable_msix_range which I traced into eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it works fine...?
I don't really know why that'd be the case... thoughts?

Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.

- Alex

Keller

2016-06-08 21:18 UTC #4 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 46 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 16 lines
Quoted 16 lines
probe
Quoted 4 lines · Jacob Keller
It seems that under some circumstances, such as after performing an unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 2 lines
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on driver device probe.
Quoted 6 lines
Quoted 4 lines
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long as nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we unbind
then bind the VF device, it fails in pci_enable_msix_range which I traced into eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it works fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?

Quoted 8 lines

Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
- Alex

I'm not sure either but without it we're ending up with the failure in pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...

It only happens after an unbind of the VF device then a bind. I think somehow the dev->current_state gets messed up but I don't know how this happens.

Thanks,
Jake

Alexander Duyck

2016-06-08 21:22 UTC #5 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 53 lines · Jacob E

Quoted 53 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 16 lines
Quoted 16 lines
probe
Quoted 4 lines · Jacob Keller
It seems that under some circumstances, such as after performing an unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 2 lines
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on driver device probe.
Quoted 6 lines
Quoted 4 lines
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long as nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we unbind
then bind the VF device, it fails in pci_enable_msix_range which I traced into eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it works fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.

Quoted 8 lines

Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
It only happens after an unbind of the VF device then a bind. I think somehow the dev->current_state gets messed up but I don't know how this happens.
Thanks,
Jake

When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?

- Alex

Keller

2016-06-08 21:23 UTC #6 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 79 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 10 lines
Quoted 10 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after performing
an
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 3 lines
unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long
as
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we unbind
then bind the VF device, it fails in pci_enable_msix_range which I traced
into
Quoted 13 lines
Quoted 13 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it works fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how this happens.
Quoted 1 line

Quoted 7 lines

Quoted 2 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex

Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to bind.

Thanks,
Jake

Alexander Duyck

2016-06-08 21:33 UTC #7 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 85 lines · Jacob E

Quoted 85 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 10 lines
Quoted 10 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after performing
an
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 3 lines
unbind following an AER injection recovery, the power state of a VF isn't set to PCI_D0. Following the unbind if we attempt to bind the device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As long
as
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we unbind
then bind the VF device, it fails in pci_enable_msix_range which I traced
into
Quoted 13 lines
Quoted 13 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it works fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how this happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?

Quoted 6 lines

Quoted 1 line
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to bind.
Thanks,
Jake

I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.

- Alex

Keller

2016-06-08 21:46 UTC #8 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 121 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:34 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 13 lines
Quoted 13 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after
performing
Quoted 2 lines
Quoted 2 lines
an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unbind following an AER injection recovery, the power state of a
VF
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
isn't set to PCI_D0. Following the unbind if we attempt to bind
the
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As
long
Quoted 5 lines
Quoted 5 lines
as
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we
unbind
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
then bind the VF device, it fails in pci_enable_msix_range which I
traced
Quoted 3 lines
Quoted 3 lines
into
Quoted 2 lines
Quoted 2 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it
works
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how
this
Quoted 11 lines
Quoted 9 lines
happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to
bind.
Quoted 3 lines
Thanks,
Jake
I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.

Quoted 1 line

- Alex

I will try to get this information.

Thanks,
Jake

Keller

2016-06-09 17:24 UTC #9 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

In reply to Alexander Duyck

Quoted 121 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:34 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 13 lines
Quoted 13 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after
performing
Quoted 2 lines
Quoted 2 lines
an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unbind following an AER injection recovery, the power state of a
VF
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
isn't set to PCI_D0. Following the unbind if we attempt to bind
the
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As
long
Quoted 5 lines
Quoted 5 lines
as
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we
unbind
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
then bind the VF device, it fails in pci_enable_msix_range which I
traced
Quoted 3 lines
Quoted 3 lines
into
Quoted 2 lines
Quoted 2 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it
works
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how
this
Quoted 11 lines
Quoted 9 lines
happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to
bind.
Quoted 3 lines
Thanks,
Jake
I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.

Quoted 1 line

- Alex

After a fresh bind, the current_state is PCI_UNKNOWN even after (a successful) pci_enable_device_mem...

It stays unknown all the way through until the pci_enable_msix_range function is called and failed.

I'm adding code to check the enable count now.

Thanks,
Jake

Keller

2016-06-09 17:31 UTC #10 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

In reply to Alexander Duyck

Quoted 121 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:34 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 13 lines
Quoted 13 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after
performing
Quoted 2 lines
Quoted 2 lines
an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unbind following an AER injection recovery, the power state of a
VF
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
isn't set to PCI_D0. Following the unbind if we attempt to bind
the
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As
long
Quoted 5 lines
Quoted 5 lines
as
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we
unbind
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
then bind the VF device, it fails in pci_enable_msix_range which I
traced
Quoted 3 lines
Quoted 3 lines
into
Quoted 2 lines
Quoted 2 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it
works
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how
this
Quoted 11 lines
Quoted 9 lines
happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to
bind.
Quoted 3 lines
Thanks,
Jake
I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.

Quoted 1 line

- Alex

It appears that after an AER injection error we end up leaking a pci device reference count. I'm currently investigating that now.

Thanks,
Jake

Keller

2016-06-09 17:38 UTC #11 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

In reply to Alexander Duyck

Quoted 123 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:34 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 13 lines
Quoted 13 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after
performing
Quoted 2 lines
Quoted 2 lines
an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unbind following an AER injection recovery, the power state of a
VF
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
isn't set to PCI_D0. Following the unbind if we attempt to bind
the
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As
long
Quoted 5 lines
Quoted 5 lines
as
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we
unbind
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
then bind the VF device, it fails in pci_enable_msix_range which I
traced
Quoted 3 lines
Quoted 3 lines
into
Quoted 2 lines
Quoted 2 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it
works
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how
this
Quoted 11 lines
Quoted 9 lines
happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to
bind.
Quoted 3 lines
Thanks,
Jake
I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.

Quoted 1 line

- Alex

Yep, when we call pci_enable_device_mem() in fm10k_io_slot_reset() we leak an enable count and I think that?s at least partially responsible for the failure since I bet VFs don't actually expose power state, so once enabled it won't change back from unknown?

Thanks,
Jake

Alexander Duyck

2016-06-09 17:54 UTC #12 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 123 lines · Jacob E

Quoted 123 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:34 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 13 lines
Quoted 13 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
upon
Quoted 3 lines
Quoted 3 lines
probe
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after
performing
Quoted 2 lines
Quoted 2 lines
an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unbind following an AER injection recovery, the power state of a
VF
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
isn't set to PCI_D0. Following the unbind if we attempt to bind
the
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
device, the MSI-X initialization fails due to incorrect power state.
That doesn't make much sense. I didn't think the VFs had their own power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0) on
driver
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called to transition the power state to D0 in pci_enable_device_mem. As
long
Quoted 5 lines
Quoted 5 lines
as
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we
unbind
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
then bind the VF device, it fails in pci_enable_msix_range which I
traced
Quoted 3 lines
Quoted 3 lines
into
Quoted 2 lines
Quoted 2 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it
works
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF. So I am not certain what this patch is even doing if you aren't reloading the PF driver to trigger it.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state != PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know how
this
Quoted 11 lines
Quoted 9 lines
happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to
bind.
Quoted 3 lines
Thanks,
Jake
I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.

Quoted 3 lines

Quoted 1 line
- Alex
Yep, when we call pci_enable_device_mem() in fm10k_io_slot_reset() we leak an enable count and I think that?s at least partially responsible for the failure since I bet VFs don't actually expose power state, so once enabled it won't change back from unknown?

No, it will but you have to actually be re-enabling the device to trigger it.

You might try replacing the call to pci_enable_device_mem with pci_reenable_device and see if that works for you. I suspect this is probably a bug that is common in many of the drivers out there.

- Alex

Keller

2016-06-09 18:30 UTC #13 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

Quoted 151 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Thursday, June 09, 2016 10:55 AM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state upon probe
Quoted 8 lines · Jacob E
Quoted 8 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:34 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 13 lines
Quoted 13 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:23 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power state
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Wednesday, June 08, 2016 2:02 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 15 lines
Quoted 15 lines
Quoted 11 lines
Quoted 11 lines
upon
Quoted 10 lines
Quoted 10 lines
probe
Quoted 6 lines · Jacob E
Quoted 6 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Tuesday, June 07, 2016 8:54 PM
To: Keller, Jacob E <[email protected]>
Cc: Intel Wired LAN <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH] fm10k: restore D0 power
state
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
upon
Quoted 2 lines
Quoted 2 lines
probe
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
On Tue, Jun 7, 2016 at 5:20 PM, Jacob Keller
<[email protected]>
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 1 line · wrote:
It seems that under some circumstances, such as after
performing
Quoted 2 lines
Quoted 2 lines
an
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
unbind following an AER injection recovery, the power state
of a
Quoted 4 lines
Quoted 4 lines
VF
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
isn't set to PCI_D0. Following the unbind if we attempt to bind
the
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
device, the MSI-X initialization fails due to incorrect power
state.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
That doesn't make much sense. I didn't think the VFs had their
own
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
power state control. After all it isn't as if they could switch off while the PF is active on the host.
Quoted 1 line
Fix this by always calling pci_set_power_state(pdev, PCI_D0)
on
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
driver
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
device probe.
That should be redundant. There is already code that is called
to
Quoted 14 lines
Quoted 14 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
transition the power state to D0 in pci_enable_device_mem. As
long
Quoted 5 lines
Quoted 5 lines
as
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
nobody else has enabled the device it will be ran.
That's weird. In this case, after doing aer-inject on the PF, if we
unbind
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
then bind the VF device, it fails in pci_enable_msix_range which I
traced
Quoted 3 lines
Quoted 3 lines
into
Quoted 2 lines
Quoted 2 lines
eventually a call that fails because the state isn't PCI_D0. If I add a "pci_set_power_state(pdev, PCI_D0)" call in the probe routine it
works
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
fine...?
Quoted 2 lines
I don't really know why that'd be the case... thoughts?
Does the VF even advertise support for PCI power management capability? If I recall a VF shouldn't. As such I think a call to pci_set_power_state should be returning an error and not doing anything so I would think this code only has any effect on the PF.
So
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
I am not certain what this patch is even doing if you aren't
reloading
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
the PF driver to trigger it.
- Alex
I'm not sure either but without it we're ending up with the failure in
pci_msi_supported failing with -EINVAL because dev->current_state
!=
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
PCI_D0...
Quoted 2 lines
It only happens after an unbind of the VF device then a bind. I think
somehow the dev->current_state gets messed up but I don't know
how
Quoted 25 lines
Quoted 25 lines
this
Quoted 11 lines
Quoted 9 lines
happens.
Quoted 3 lines
Thanks,
Jake
When you say unbind/bind are you talking about using the sysfs or is this via some other mechanism?
- Alex
Using sysfs. We echo the PCI address to fm10k's unbind, then echo it to
bind.
Quoted 3 lines
Thanks,
Jake
I'm wondering if we have a reference count bug being created somewhere. One thing you might try tracking is the dev->enable_cnt value for the VF through probe and after a remove to see if we are leaking that somewhere. The other thing to look for is to figure out where we are setting dev->current_state to something other than PCI_D0. I'd be interested in knowing where we are messing on this with a VF and what we are setting it to.
Quoted 3 lines
Quoted 1 line
- Alex
Yep, when we call pci_enable_device_mem() in fm10k_io_slot_reset() we
leak an enable count and I think that?s at least partially responsible for the failure since I bet VFs don't actually expose power state, so once enabled it won't change back from unknown?

Quoted 7 lines

No, it will but you have to actually be re-enabling the device to trigger it.
You might try replacing the call to pci_enable_device_mem with pci_reenable_device and see if that works for you. I suspect this is probably a bug that is common in many of the drivers out there.
- Alex

The other possible fix is to put pci_device_disable into the .io_error_detected handler, which used to be there in the past. The downside to this call is that we have to check when we remove if we've already disabled to avoid disabling the device twice.

I'll look into using reenable instead and see how that goes.

Thanks,
Jake

Keller

2016-06-09 18:57 UTC #14 lore raw

PATCH 1/1 fm10k: restore D0 power state upon probe

In reply to Alexander Duyck

Quoted 8 lines · Alexander Duyck

No, it will but you have to actually be re-enabling the device to trigger it.
You might try replacing the call to pci_enable_device_mem with pci_reenable_device and see if that works for you.??I suspect this is probably a bug that is common in many of the drivers out there.
- Alex

Changing the pci_enable_device_mem() to a pci_reenable_device() inside the .io_slot_reset seems to have worked, and is much cleaner than my alternative approach. I think this is the right way since we aren't really wanting to disable the device in this section and matches up with what the .slot_reset wants without breaking other flows.

I'll have a patch for that in a bit.

Thanks,
Jake