VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Gavin Shan (4):
PCI: Rename struct pci_dev_reset_methods
PCI: Introduce list for device reset methods
PCI: Allow registering reset method
powerpc/powernv: Register PCI dev specific reset handlers
arch/powerpc/platforms/powernv/pci.c | 61 +++++++++++++++
drivers/pci/pci.h | 3 +-
drivers/pci/quirks.c | 139 ++++++++++++++++++++++++++++++-----
include/linux/pci.h | 9 +++
4 files changed, 192 insertions(+), 20 deletions(-)
--
1.8.3.2
The patch introduces list to hold the PCI device specific reset
methods. With help of the list, we can register PCI device specific
reset method dynamically as we do in next one patch.
Signed-off-by: Gavin Shan <redacted>
---
drivers/pci/pci.h | 1 +
drivers/pci/quirks.c | 75 +++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 57 insertions(+), 19 deletions(-)
Currently, PCI VFIO infrastructure depends on pci_reset_function()
to ensure the PCI device in clean state when passing to guest, or
being returned back to host. However, the function doesn't work
(or well) on some PCI devices, which potentially brings pending
traffic over the boundary between host/guest and usually causes
memory corruption.
The patch registers PCI device specific reset handlers for those
PCI devices, pci_reset_function() doesn't work or not well, to
translate the request to EEH PE reset if possible.
Signed-off-by: Gavin Shan <redacted>
---
arch/powerpc/platforms/powernv/pci.c | 61 ++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
On Fri, Feb 13, 2015 at 03:54:55PM +1100, Gavin Shan wrote:
VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Hi, Gavin.
I like your approach overall. That allows us to confine these quirks to
the platforms where they are relevant. I would make the quirks more
specific, though, instead of doing them for all IBM and Mellanox
devices.
I wonder if we should not have some form of domain reset, where we would
reset all the devices on the same group, and use that on vfio. Grouping
the devices would then be made platform-dependent, as well as the reset
method. On powernv, we would group by IOMMU group and issue a
fundamental reset.
Cascardo.
On Mon, Feb 16, 2015 at 11:14:27AM -0200, cascardo@linux.vnet.ibm.com wrote:
On Fri, Feb 13, 2015 at 03:54:55PM +1100, Gavin Shan wrote:
quoted
VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Hi, Gavin.
I like your approach overall. That allows us to confine these quirks to
the platforms where they are relevant. I would make the quirks more
specific, though, instead of doing them for all IBM and Mellanox
devices.
Yeah, we need have more specific vendor/device IDs for PATCH[4/4]. Could
you please take a look on PATCH[4/4] and then suggest the specific devices
that requries the platform-dependent reset quirk? Especially the device IDs
for IBM/Mellanox we need put add quirks for.
I wonder if we should not have some form of domain reset, where we would
reset all the devices on the same group, and use that on vfio. Grouping
the devices would then be made platform-dependent, as well as the reset
method. On powernv, we would group by IOMMU group and issue a
fundamental reset.
I'm assuming "domain reset" is PE reset, which is the specific reset handler
tries to do on PowerNV platform. The reason why we need platform specific
reset handler is some adapters can't support function level reset methods
(except pci_dev_specific_reset()) in __pci_dev_reset().
Thanks,
Gavin
On Tue, Feb 17, 2015 at 09:36:47AM +1100, Gavin Shan wrote:
On Mon, Feb 16, 2015 at 11:14:27AM -0200, cascardo@linux.vnet.ibm.com wrote:
quoted
On Fri, Feb 13, 2015 at 03:54:55PM +1100, Gavin Shan wrote:
quoted
VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Hi, Gavin.
I like your approach overall. That allows us to confine these quirks to
the platforms where they are relevant. I would make the quirks more
specific, though, instead of doing them for all IBM and Mellanox
devices.
Yeah, we need have more specific vendor/device IDs for PATCH[4/4]. Could
you please take a look on PATCH[4/4] and then suggest the specific devices
that requries the platform-dependent reset quirk? Especially the device IDs
for IBM/Mellanox we need put add quirks for.
quoted
I wonder if we should not have some form of domain reset, where we would
reset all the devices on the same group, and use that on vfio. Grouping
the devices would then be made platform-dependent, as well as the reset
method. On powernv, we would group by IOMMU group and issue a
fundamental reset.
I'm assuming "domain reset" is PE reset, which is the specific reset handler
tries to do on PowerNV platform. The reason why we need platform specific
reset handler is some adapters can't support function level reset methods
(except pci_dev_specific_reset()) in __pci_dev_reset().
Well, in the case of Power servers, this would be the PE reset, I am not
sure what this would be on other platforms. No other platform implements
pci_set_pcie_reset_state, which I think would be one possible to
implement such a reset.
What I am saying is that we should consider doing this on all platforms
and for all adapters, because this is not specific to Power and this is
not specific to this particular set of adapters. Otherwise, one can
simply program one adapter in the guest to write to host memory,
shutdown, and since no reset will take place, the card will simply write
to host memory when the guest is finished with and IOMMU is off.
Regards.
Cascardo.
On Thu, Feb 19, 2015 at 04:57:47PM -0200, cascardo@linux.vnet.ibm.com wrote:
On Tue, Feb 17, 2015 at 09:36:47AM +1100, Gavin Shan wrote:
quoted
On Mon, Feb 16, 2015 at 11:14:27AM -0200, cascardo@linux.vnet.ibm.com wrote:
quoted
On Fri, Feb 13, 2015 at 03:54:55PM +1100, Gavin Shan wrote:
quoted
VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Hi, Gavin.
I like your approach overall. That allows us to confine these quirks to
the platforms where they are relevant. I would make the quirks more
specific, though, instead of doing them for all IBM and Mellanox
devices.
Yeah, we need have more specific vendor/device IDs for PATCH[4/4]. Could
you please take a look on PATCH[4/4] and then suggest the specific devices
that requries the platform-dependent reset quirk? Especially the device IDs
for IBM/Mellanox we need put add quirks for.
quoted
I wonder if we should not have some form of domain reset, where we would
reset all the devices on the same group, and use that on vfio. Grouping
the devices would then be made platform-dependent, as well as the reset
method. On powernv, we would group by IOMMU group and issue a
fundamental reset.
I'm assuming "domain reset" is PE reset, which is the specific reset handler
tries to do on PowerNV platform. The reason why we need platform specific
reset handler is some adapters can't support function level reset methods
(except pci_dev_specific_reset()) in __pci_dev_reset().
Well, in the case of Power servers, this would be the PE reset, I am not
sure what this would be on other platforms. No other platform implements
pci_set_pcie_reset_state, which I think would be one possible to
implement such a reset.
What I am saying is that we should consider doing this on all platforms
and for all adapters, because this is not specific to Power and this is
not specific to this particular set of adapters. Otherwise, one can
simply program one adapter in the guest to write to host memory,
shutdown, and since no reset will take place, the card will simply write
to host memory when the guest is finished with and IOMMU is off.
Yes, I believe your way will make things simpler and we don't need the
code to support registering reset handler dynamically at atll. Please
confirm if following idea is what you're suggesting:
- Introduce function drivers/pci/quirks.c::pci_dev_specific_reset(), which
will be added to pci_dev_reset_methods[] for various vendor/device IDs.
- pci_dev_specific_reset() routes the reset (or probe) request to
pci_set_pcie_reset_state(), which needs one more syntax for reset probing.
In turn, pci_set_pcie_reset_state() calls pcibios_set_pcie_reset_state(),
which returns -ETTY by default.
For PowerPC, pcibios_set_pcie_reset_state() will do PE reset, which can't
be ported to other platforms as it depends on PowerPC unique EEH feature.
So other platforms have to override this function and do things similar to
PowerPC PE reset there.
Thanks,
Gavin
On Fri, Feb 20, 2015 at 04:53:08PM +1100, Gavin Shan wrote:
On Thu, Feb 19, 2015 at 04:57:47PM -0200, cascardo@linux.vnet.ibm.com wrote:
quoted
On Tue, Feb 17, 2015 at 09:36:47AM +1100, Gavin Shan wrote:
quoted
On Mon, Feb 16, 2015 at 11:14:27AM -0200, cascardo@linux.vnet.ibm.com wrote:
quoted
On Fri, Feb 13, 2015 at 03:54:55PM +1100, Gavin Shan wrote:
quoted
VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Hi, Gavin.
I like your approach overall. That allows us to confine these quirks to
the platforms where they are relevant. I would make the quirks more
specific, though, instead of doing them for all IBM and Mellanox
devices.
Yeah, we need have more specific vendor/device IDs for PATCH[4/4]. Could
you please take a look on PATCH[4/4] and then suggest the specific devices
that requries the platform-dependent reset quirk? Especially the device IDs
for IBM/Mellanox we need put add quirks for.
quoted
I wonder if we should not have some form of domain reset, where we would
reset all the devices on the same group, and use that on vfio. Grouping
the devices would then be made platform-dependent, as well as the reset
method. On powernv, we would group by IOMMU group and issue a
fundamental reset.
I'm assuming "domain reset" is PE reset, which is the specific reset handler
tries to do on PowerNV platform. The reason why we need platform specific
reset handler is some adapters can't support function level reset methods
(except pci_dev_specific_reset()) in __pci_dev_reset().
Well, in the case of Power servers, this would be the PE reset, I am not
sure what this would be on other platforms. No other platform implements
pci_set_pcie_reset_state, which I think would be one possible to
implement such a reset.
What I am saying is that we should consider doing this on all platforms
and for all adapters, because this is not specific to Power and this is
not specific to this particular set of adapters. Otherwise, one can
simply program one adapter in the guest to write to host memory,
shutdown, and since no reset will take place, the card will simply write
to host memory when the guest is finished with and IOMMU is off.
Yes, I believe your way will make things simpler and we don't need the
code to support registering reset handler dynamically at atll. Please
confirm if following idea is what you're suggesting:
- Introduce function drivers/pci/quirks.c::pci_dev_specific_reset(), which
will be added to pci_dev_reset_methods[] for various vendor/device IDs.
- pci_dev_specific_reset() routes the reset (or probe) request to
pci_set_pcie_reset_state(), which needs one more syntax for reset probing.
In turn, pci_set_pcie_reset_state() calls pcibios_set_pcie_reset_state(),
which returns -ETTY by default.
For PowerPC, pcibios_set_pcie_reset_state() will do PE reset, which can't
be ported to other platforms as it depends on PowerPC unique EEH feature.
So other platforms have to override this function and do things similar to
PowerPC PE reset there.
Dropping for now because it sounds like you are considering reworking based
on Cascardo's ideas. If not, please re-post these so they show up in
patchwork again.
Bjorn
On Fri, Mar 06, 2015 at 12:38:59PM -0600, Bjorn Helgaas wrote:
On Fri, Feb 20, 2015 at 04:53:08PM +1100, Gavin Shan wrote:
quoted
On Thu, Feb 19, 2015 at 04:57:47PM -0200, cascardo@linux.vnet.ibm.com wrote:
quoted
On Tue, Feb 17, 2015 at 09:36:47AM +1100, Gavin Shan wrote:
quoted
On Mon, Feb 16, 2015 at 11:14:27AM -0200, cascardo@linux.vnet.ibm.com wrote:
quoted
On Fri, Feb 13, 2015 at 03:54:55PM +1100, Gavin Shan wrote:
quoted
VFIO PCI infrastructure depends on pci_reset_function() to do reset on
PCI devices so that they would be in clean state when host or guest grabs
them. Unfortunately, the function doesn't work (or not well) on some PCI
devices that require EEH PE reset.
The patchset extends the quirk for PCI device speicific reset methods to
allow dynamically registration. With it, we can translate reset requests
for those special PCI devcies to EEH PE reset, which is only avaialble on
64-bits PowerPC platforms.
Hi, Gavin.
I like your approach overall. That allows us to confine these quirks to
the platforms where they are relevant. I would make the quirks more
specific, though, instead of doing them for all IBM and Mellanox
devices.
Yeah, we need have more specific vendor/device IDs for PATCH[4/4]. Could
you please take a look on PATCH[4/4] and then suggest the specific devices
that requries the platform-dependent reset quirk? Especially the device IDs
for IBM/Mellanox we need put add quirks for.
quoted
I wonder if we should not have some form of domain reset, where we would
reset all the devices on the same group, and use that on vfio. Grouping
the devices would then be made platform-dependent, as well as the reset
method. On powernv, we would group by IOMMU group and issue a
fundamental reset.
I'm assuming "domain reset" is PE reset, which is the specific reset handler
tries to do on PowerNV platform. The reason why we need platform specific
reset handler is some adapters can't support function level reset methods
(except pci_dev_specific_reset()) in __pci_dev_reset().
Well, in the case of Power servers, this would be the PE reset, I am not
sure what this would be on other platforms. No other platform implements
pci_set_pcie_reset_state, which I think would be one possible to
implement such a reset.
What I am saying is that we should consider doing this on all platforms
and for all adapters, because this is not specific to Power and this is
not specific to this particular set of adapters. Otherwise, one can
simply program one adapter in the guest to write to host memory,
shutdown, and since no reset will take place, the card will simply write
to host memory when the guest is finished with and IOMMU is off.
Yes, I believe your way will make things simpler and we don't need the
code to support registering reset handler dynamically at atll. Please
confirm if following idea is what you're suggesting:
- Introduce function drivers/pci/quirks.c::pci_dev_specific_reset(), which
will be added to pci_dev_reset_methods[] for various vendor/device IDs.
- pci_dev_specific_reset() routes the reset (or probe) request to
pci_set_pcie_reset_state(), which needs one more syntax for reset probing.
In turn, pci_set_pcie_reset_state() calls pcibios_set_pcie_reset_state(),
which returns -ETTY by default.
For PowerPC, pcibios_set_pcie_reset_state() will do PE reset, which can't
be ported to other platforms as it depends on PowerPC unique EEH feature.
So other platforms have to override this function and do things similar to
PowerPC PE reset there.
Dropping for now because it sounds like you are considering reworking based
on Cascardo's ideas. If not, please re-post these so they show up in
patchwork again.
Yes, I'll rework on this according to Cascardo's input and repost.
Thanks,
Gavin