PCI and PCIe devices may support a number of possible reset mechanisms
for example Function Level Reset (FLR) provided via Advanced Feature or
PCIe capabilities, Power Management reset, bus reset, or device specific reset.
Currently the PCI subsystem creates a policy prioritizing these reset methods
which provides neither visibility nor control to userspace.
Expose the reset methods available per device to userspace, via sysfs
and allow an administrative user or device owner to have ability to
manage per device reset method priorities or exclusions.
This feature aims to allow greater control of a device for use cases
as device assignment, where specific device or platform issues may
interact poorly with a given reset method, and for which device specific
quirks have not been developed.
Changes in v14:
- Remove duplicate entries from pdev->reset_methods as per
Shanker's suggestion
Changes in v13:
- Added "PCI: Cache PCIe FLR capability"
- Removed memcpy in pci_init_reset_methods() and reset_method_show
- Moved reset_method sysfs attribute code from pci-sysfs.c to
pci.c
Changes in v12:
- Corrected subject in 0/8 (cover letter).
Changes in v11:
- Alex's suggestion fallback back to other resets if the ACPI RST
fails. Fix "s/-EINVAL/-ENOTTY/" in 7/8 patch.
Changes in v10:
- Fix build error on ppc as reported by build bot
Changes in v9:
- Renamed has_flr bitfield to has_pcie_flr and restored
use of PCI_DEV_FLAGS_NO_FLR_RESET in quirk_no_flr()
- Cleaned up sysfs code
Changes in v8:
- Added has_flr bitfield to struct pci_dev to cache flr
capability
- Updated encoding scheme used in reset_methods array as per
Bjorn's suggestion
- Updated Shanker's ACPI patches
Changes in v7:
- Fix the pci_dev_acpi_reset() prototype mismatch
in case of CONFIG_ACPI=n
Changes in v6:
- Address Bjorn's and Krzysztof's review comments
- Add Shanker's updated patches along with new
"PCI: Setup ACPI_COMPANION early" patch
Changes in v5:
- Rebase the series over pci/reset branch of
Bjorn's pci tree to avoid merge conflicts
caused by recent changes in existing reset
sysfs attribute
Changes in v4:
- Change the order or strlen and strim in reset_method_store
function to avoid extra strlen call.
- Use consistent terminology in new
pci_reset_mode enum and rename the probe argument
of reset functions.
Changes in v3:
- Dropped "PCI: merge slot and bus reset implementations" which was
already accepted separately
- Grammar fixes
- Added Shanker's patches which were rebased on v2 of this series
- Added "PCI: Change the type of probe argument in reset functions"
and additional user input sanitization code in reset_method_store
function per review feedback from Krzysztof
Changes in v2:
- Use byte array instead of bitmap to keep track of
ordering of reset methods
- Fix incorrect use of reset_fn field in octeon driver
- Allow writing comma separated list of names of supported reset
methods to reset_method sysfs attribute
- Writing empty string instead of "none" to reset_method attribute
disables ability of reset the device
Amey Narkhede (6):
PCI: Cache PCIe FLR capability
PCI: Add pcie_reset_flr to follow calling convention of other reset
methods
PCI: Add new array for keeping track of ordering of reset methods
PCI: Remove reset_fn field from pci_dev
PCI: Allow userspace to query and set device reset mechanism
PCI: Change the type of probe argument in reset functions
Shanker Donthineni (3):
PCI: Define a function to set ACPI_COMPANION in pci_dev
PCI: Setup ACPI fwnode early and at the same time with OF
PCI: Add support for ACPI _RST reset method
Documentation/ABI/testing/sysfs-bus-pci | 19 ++
drivers/crypto/cavium/nitrox/nitrox_main.c | 4 +-
.../ethernet/cavium/liquidio/lio_vf_main.c | 2 +-
drivers/pci/hotplug/pciehp.h | 2 +-
drivers/pci/hotplug/pciehp_hpc.c | 2 +-
drivers/pci/hotplug/pnv_php.c | 4 +-
drivers/pci/pci-acpi.c | 35 ++-
drivers/pci/pci-sysfs.c | 3 +-
drivers/pci/pci.c | 286 +++++++++++++-----
drivers/pci/pci.h | 24 +-
drivers/pci/pcie/aer.c | 12 +-
drivers/pci/probe.c | 16 +-
drivers/pci/quirks.c | 25 +-
drivers/pci/remove.c | 1 -
include/linux/pci.h | 14 +-
include/linux/pci_hotplug.h | 2 +-
16 files changed, 331 insertions(+), 120 deletions(-)
--
2.32.0
From: Shanker Donthineni <[email protected]>
Move the existing code logic from acpi_pci_bridge_d3() to a separate
function pci_set_acpi_fwnode() to set the ACPI fwnode.
No functional change with this patch.
Signed-off-by: Shanker Donthineni <[email protected]>
---
drivers/pci/pci-acpi.c | 12 ++++++++----
drivers/pci/pci.h | 2 ++
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 36bc23e21759..eaddbf701759 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -934,6 +934,13 @@ static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
static struct acpi_device *acpi_pci_find_companion(struct device *dev);
+void pci_set_acpi_fwnode(struct pci_dev *dev)
+{
+ if (!ACPI_COMPANION(&dev->dev) && !pci_dev_is_added(dev))
+ ACPI_COMPANION_SET(&dev->dev,
+ acpi_pci_find_companion(&dev->dev));
+}
+
static bool acpi_pci_bridge_d3(struct pci_dev *dev)
{
const struct fwnode_handle *fwnode;
@@ -945,11 +952,8 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev)
return false;
/* Assume D3 support if the bridge is power-manageable by ACPI. */
+ pci_set_acpi_fwnode(dev);
adev = ACPI_COMPANION(&dev->dev);
- if (!adev && !pci_dev_is_added(dev)) {
- adev = acpi_pci_find_companion(&dev->dev);
- ACPI_COMPANION_SET(&dev->dev, adev);
- }
if (adev && acpi_device_power_manageable(adev))
return true;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 31458d48eda7..8ef379b6cfad 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -703,7 +703,9 @@ static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL
#ifdef CONFIG_ACPI
int pci_acpi_program_hp_params(struct pci_dev *dev);
extern const struct attribute_group pci_dev_acpi_attr_group;
+void pci_set_acpi_fwnode(struct pci_dev *dev);
#else
+static inline void pci_set_acpi_fwnode(struct pci_dev *dev) {}
static inline int pci_acpi_program_hp_params(struct pci_dev *dev)
{
return -ENODEV;
--
2.32.0
Currently there is separate function pcie_has_flr() to probe if PCIe FLR
is supported by the device which does not match the calling convention
followed by reset methods which use second function argument to decide
whether to probe or not. Add new function pcie_reset_flr() that follows
the calling convention of reset methods.
Signed-off-by: Amey Narkhede <[email protected]>
---
drivers/crypto/cavium/nitrox/nitrox_main.c | 4 +--
drivers/pci/pci.c | 40 +++++++++++++++-------
drivers/pci/pcie/aer.c | 12 +++----
drivers/pci/quirks.c | 9 ++---
include/linux/pci.h | 2 +-
5 files changed, 38 insertions(+), 29 deletions(-)
diff --git a/drivers/crypto/cavium/nitrox/nitrox_main.c b/drivers/crypto/cavium/nitrox/nitrox_main.c
index facc8e6bc580..15d6c8452807 100644
--- a/drivers/crypto/cavium/nitrox/nitrox_main.c
+++ b/drivers/crypto/cavium/nitrox/nitrox_main.c
@@ -306,9 +306,7 @@ static int nitrox_device_flr(struct pci_dev *pdev)
return -ENOMEM;
}
- /* check flr support */
- if (pcie_has_flr(pdev))
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, 0);
pci_restore_state(pdev);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1fafd05caa41..7d1d9671160b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4619,22 +4619,20 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
* Returns true if the device advertises support for PCIe function level
* resets.
*/
-bool pcie_has_flr(struct pci_dev *dev)
+static bool pcie_has_flr(struct pci_dev *dev)
{
if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
return false;
return FIELD_GET(PCI_EXP_DEVCAP_FLR, dev->devcap) == 1;
}
-EXPORT_SYMBOL_GPL(pcie_has_flr);
/**
* pcie_flr - initiate a PCIe function level reset
* @dev: device to reset
*
- * Initiate a function level reset on @dev. The caller should ensure the
- * device supports FLR before calling this function, e.g. by using the
- * pcie_has_flr() helper.
+ * Initiate a function level reset unconditionally on @dev without
+ * checking any flags and DEVCAP
*/
int pcie_flr(struct pci_dev *dev)
{
@@ -4657,6 +4655,25 @@ int pcie_flr(struct pci_dev *dev)
}
EXPORT_SYMBOL_GPL(pcie_flr);
+/**
+ * pcie_reset_flr - initiate a PCIe function level reset
+ * @dev: device to reset
+ * @probe: If set, only check if the device can be reset this way.
+ *
+ * Initiate a function level reset on @dev.
+ */
+int pcie_reset_flr(struct pci_dev *dev, int probe)
+{
+ if (!pcie_has_flr(dev))
+ return -ENOTTY;
+
+ if (probe)
+ return 0;
+
+ return pcie_flr(dev);
+}
+EXPORT_SYMBOL_GPL(pcie_reset_flr);
+
static int pci_af_flr(struct pci_dev *dev, int probe)
{
int pos;
@@ -5137,11 +5154,9 @@ int __pci_reset_function_locked(struct pci_dev *dev)
rc = pci_dev_specific_reset(dev, 0);
if (rc != -ENOTTY)
return rc;
- if (pcie_has_flr(dev)) {
- rc = pcie_flr(dev);
- if (rc != -ENOTTY)
- return rc;
- }
+ rc = pcie_reset_flr(dev, 0);
+ if (rc != -ENOTTY)
+ return rc;
rc = pci_af_flr(dev, 0);
if (rc != -ENOTTY)
return rc;
@@ -5172,8 +5187,9 @@ int pci_probe_reset_function(struct pci_dev *dev)
rc = pci_dev_specific_reset(dev, 1);
if (rc != -ENOTTY)
return rc;
- if (pcie_has_flr(dev))
- return 0;
+ rc = pcie_reset_flr(dev, 1);
+ if (rc != -ENOTTY)
+ return rc;
rc = pci_af_flr(dev, 1);
if (rc != -ENOTTY)
return rc;
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ec943cee5ecc..98077595a73e 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1405,13 +1405,11 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
}
if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
- if (pcie_has_flr(dev)) {
- rc = pcie_flr(dev);
- pci_info(dev, "has been reset (%d)\n", rc);
- } else {
- pci_info(dev, "not reset (no FLR support)\n");
- rc = -ENOTTY;
- }
+ rc = pcie_reset_flr(dev, 0);
+ if (!rc)
+ pci_info(dev, "has been reset\n");
+ else
+ pci_info(dev, "not reset (no FLR support: %d)\n", rc);
} else {
rc = pci_bus_error_reset(dev);
pci_info(dev, "%s Port link has been reset (%d)\n",
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index d85914afe65a..b48e7ef8b641 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3819,7 +3819,7 @@ static int nvme_disable_and_flr(struct pci_dev *dev, int probe)
u32 cfg;
if (dev->class != PCI_CLASS_STORAGE_EXPRESS ||
- !pcie_has_flr(dev) || !pci_resource_start(dev, 0))
+ pcie_reset_flr(dev, 1) || !pci_resource_start(dev, 0))
return -ENOTTY;
if (probe)
@@ -3888,13 +3888,10 @@ static int nvme_disable_and_flr(struct pci_dev *dev, int probe)
*/
static int delay_250ms_after_flr(struct pci_dev *dev, int probe)
{
- if (!pcie_has_flr(dev))
- return -ENOTTY;
-
if (probe)
- return 0;
+ return pcie_reset_flr(dev, 1);
- pcie_flr(dev);
+ pcie_reset_flr(dev, 0);
msleep(250);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 697b1f085c7b..aa85e7d3147e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1226,7 +1226,7 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
enum pci_bus_speed *speed,
enum pcie_link_width *width);
void pcie_print_link_status(struct pci_dev *dev);
-bool pcie_has_flr(struct pci_dev *dev);
+int pcie_reset_flr(struct pci_dev *dev, int probe);
int pcie_flr(struct pci_dev *dev);
int __pci_reset_function_locked(struct pci_dev *dev);
int pci_reset_function(struct pci_dev *dev);
--
2.32.0
Introduce a new array reset_methods in struct pci_dev to keep track of
reset mechanisms supported by the device and their ordering.
Also refactor probing and reset functions to take advantage of calling
convention of reset functions.
Co-developed-by: Alex Williamson <[email protected]>
Signed-off-by: Alex Williamson <[email protected]>
Signed-off-by: Amey Narkhede <[email protected]>
---
drivers/pci/pci.c | 95 ++++++++++++++++++++++++++-------------------
drivers/pci/pci.h | 8 +++-
drivers/pci/probe.c | 5 +--
include/linux/pci.h | 7 ++++
4 files changed, 71 insertions(+), 44 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7d1d9671160b..67eab3d29cb3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -73,6 +73,11 @@ static void pci_dev_d3_sleep(struct pci_dev *dev)
msleep(delay);
}
+bool pci_reset_supported(struct pci_dev *dev)
+{
+ return dev->reset_methods[0] != 0;
+}
+
#ifdef CONFIG_PCI_DOMAINS
int pci_domains_supported = 1;
#endif
@@ -5117,6 +5122,16 @@ static void pci_dev_restore(struct pci_dev *dev)
err_handler->reset_done(dev);
}
+/* dev->reset_methods[] is a 0-terminated list of indices into this array */
+static const struct pci_reset_fn_method pci_reset_fn_methods[] = {
+ { },
+ { pci_dev_specific_reset, .name = "device_specific" },
+ { pcie_reset_flr, .name = "flr" },
+ { pci_af_flr, .name = "af_flr" },
+ { pci_pm_reset, .name = "pm" },
+ { pci_reset_bus_function, .name = "bus" },
+};
+
/**
* __pci_reset_function_locked - reset a PCI device function while holding
* the @dev mutex lock.
@@ -5139,65 +5154,65 @@ static void pci_dev_restore(struct pci_dev *dev)
*/
int __pci_reset_function_locked(struct pci_dev *dev)
{
- int rc;
+ int i, m, rc = -ENOTTY;
might_sleep();
/*
- * A reset method returns -ENOTTY if it doesn't support this device
- * and we should try the next method.
+ * A reset method returns -ENOTTY if it doesn't support this device and
+ * we should try the next method.
*
- * If it returns 0 (success), we're finished. If it returns any
- * other error, we're also finished: this indicates that further
- * reset mechanisms might be broken on the device.
+ * If it returns 0 (success), we're finished. If it returns any other
+ * error, we're also finished: this indicates that further reset
+ * mechanisms might be broken on the device.
*/
- rc = pci_dev_specific_reset(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pcie_reset_flr(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_af_flr(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_pm_reset(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- return pci_reset_bus_function(dev, 0);
+ for (i = 0; i < PCI_NUM_RESET_METHODS; i++) {
+ m = dev->reset_methods[i];
+ if (!m)
+ return -ENOTTY;
+
+ rc = pci_reset_fn_methods[m].reset_fn(dev, 0);
+ if (!rc)
+ return 0;
+ if (rc != -ENOTTY)
+ return rc;
+ }
+
+ return -ENOTTY;
}
EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
/**
- * pci_probe_reset_function - check whether the device can be safely reset
- * @dev: PCI device to reset
+ * pci_init_reset_methods - check whether device can be safely reset
+ * and store supported reset mechanisms.
+ * @dev: PCI device to check for reset mechanisms
*
* Some devices allow an individual function to be reset without affecting
- * other functions in the same device. The PCI device must be responsive
- * to PCI config space in order to use this function.
+ * other functions in the same device. The PCI device must be in D0-D3hot
+ * state.
*
- * Returns 0 if the device function can be reset or negative if the
- * device doesn't support resetting a single function.
+ * Stores reset mechanisms supported by device in reset_methods byte array
+ * which is a member of struct pci_dev.
*/
-int pci_probe_reset_function(struct pci_dev *dev)
+void pci_init_reset_methods(struct pci_dev *dev)
{
- int rc;
+ int m, i, rc;
+
+ BUILD_BUG_ON(ARRAY_SIZE(pci_reset_fn_methods) != PCI_NUM_RESET_METHODS);
might_sleep();
- rc = pci_dev_specific_reset(dev, 1);
- if (rc != -ENOTTY)
- return rc;
- rc = pcie_reset_flr(dev, 1);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_af_flr(dev, 1);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_pm_reset(dev, 1);
- if (rc != -ENOTTY)
- return rc;
+ i = 0;
+
+ for (m = 1; m < PCI_NUM_RESET_METHODS; m++) {
+ rc = pci_reset_fn_methods[m].reset_fn(dev, 1);
+ if (!rc)
+ dev->reset_methods[i++] = m;
+ else if (rc != -ENOTTY)
+ break;
+ }
- return pci_reset_bus_function(dev, 1);
+ dev->reset_methods[i] = 0;
}
/**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 37c913bbc6e1..7438953745e0 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -33,7 +33,8 @@ enum pci_mmap_api {
int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vmai,
enum pci_mmap_api mmap_api);
-int pci_probe_reset_function(struct pci_dev *dev);
+bool pci_reset_supported(struct pci_dev *dev);
+void pci_init_reset_methods(struct pci_dev *dev);
int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
int pci_bus_error_reset(struct pci_dev *dev);
@@ -606,6 +607,11 @@ struct pci_dev_reset_methods {
int (*reset)(struct pci_dev *dev, int probe);
};
+struct pci_reset_fn_method {
+ int (*reset_fn)(struct pci_dev *pdev, int probe);
+ char *name;
+};
+
#ifdef CONFIG_PCI_QUIRKS
int pci_dev_specific_reset(struct pci_dev *dev, int probe);
#else
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index df3f9db6e151..5d8ad230f7d0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2405,9 +2405,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
pci_rcec_init(dev); /* Root Complex Event Collector */
pcie_report_downtraining(dev);
-
- if (pci_probe_reset_function(dev) == 0)
- dev->reset_fn = 1;
+ pci_init_reset_methods(dev);
+ dev->reset_fn = pci_reset_supported(dev);
}
/*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aa85e7d3147e..d1a9a232d08e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -49,6 +49,9 @@
PCI_STATUS_SIG_TARGET_ABORT | \
PCI_STATUS_PARITY)
+/* Number of reset methods used in pci_reset_fn_methods array in pci.c */
+#define PCI_NUM_RESET_METHODS 6
+
/*
* The PCI interface treats multi-function devices as independent
* devices. The slot/function address of each device is encoded
@@ -506,6 +509,10 @@ struct pci_dev {
char *driver_override; /* Driver name to force a match */
unsigned long priv_flags; /* Private flags for the PCI driver */
+ /*
+ * See pci_reset_fn_methods array in pci.c for ordering.
+ */
+ u8 reset_methods[PCI_NUM_RESET_METHODS]; /* Reset methods ordered by priority */
};
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
--
2.32.0
Hi Amey,
On 8/4/21 3:41 PM, Amey Narkhede wrote:
> From: Shanker Donthineni <[email protected]>
>
> Move the existing code logic from acpi_pci_bridge_d3() to a separate
> function pci_set_acpi_fwnode() to set the ACPI fwnode.
>
> No functional change with this patch.
>
> Signed-off-by: Shanker Donthineni <[email protected]>
Alex's reviewed-by has been dropped from v13 and this patch series, could you you add it?