* [RFC PATCH v1] PCI: Remove pcie_flr() and convert all callers to use pcie_reset_flr()
@ 2026-06-15 19:21 Farhan Ali
2026-06-16 19:22 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Farhan Ali @ 2026-06-15 19:21 UTC (permalink / raw)
To: linux-kernel, linux-pci
Cc: helgaas, giovanni.cabiddu, alifm, herbert, davem,
dennis.dalessandro, jgg, leon, vikas.gupta, edumazet, kuba,
pabeni, michael.chan, pavan.chebbi, claudiu.manoil,
vladimir.oltean, wei.fang, xiaoning.wang, anthony.l.nguyen,
przemyslaw.kitszel, kys, haiyangz, wei.liu, decui, longli,
richardcochran, Andrew Lunn, Bjorn Helgaas, open list:QAT DRIVER,
open list:CRYPTO API, open list:HFI1 DRIVER,
open list:BROADCOM BNG_EN 800 GIGABIT ETHERNET DRIVER,
open list:FREESCALE ENETC ETHERNET DRIVERS,
moderated list:INTEL ETHERNET DRIVERS,
open list:Hyper-V/Azure CORE AND DRIVERS
The pcie_reset_flr() function includes validation checks to verify FLR
support before performing the reset, while pcie_flr() performs the reset
unconditionally. Having both functions creates unnecessary complexity.
Commit 56f107d7813f ("PCI: Add pcie_reset_flr() with 'probe' argument")
introduced pcie_reset_flr() and removed pcie_has_flr(), converting callers
that previously used the pcie_has_flr() + pcie_flr() to use
pcie_reset_flr() instead. However, it did not convert all pcie_flr()
callers, leaving two different FLR mechanisms in the kernel.
One of the callers of pcie_flr(), the Intel 82599 Virtual Function has a
defect where FLR works despite not advertising FLR support in the PCIe
Device Capability register. Rather than using pcie_flr() to work around
this, enable the FLR capability bit in devcap via an early quirk. This
allows the device to use the standard pcie_reset_flr() path instead of
requiring a device-specific reset method.
Remove pcie_flr() entirely and convert all remaining callers to
pcie_reset_flr(), ensuring consistent validation across the kernel.
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
drivers/crypto/intel/qat/qat_common/adf_aer.c | 2 +-
drivers/infiniband/hw/hfi1/chip.c | 4 +-
.../net/ethernet/broadcom/bnge/bnge_core.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 +-
.../ethernet/cavium/liquidio/lio_vf_main.c | 2 +-
.../ethernet/cavium/liquidio/octeon_mailbox.c | 3 +-
drivers/net/ethernet/freescale/enetc/enetc.c | 2 +-
.../ethernet/freescale/enetc/enetc_pci_mdio.c | 2 +-
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/microsoft/mana/mana_en.c | 3 +-
drivers/pci/pci.c | 38 ++++++------------
drivers/pci/quirks.c | 40 +++++++++----------
drivers/ptp/ptp_netc.c | 2 +-
include/linux/pci.h | 1 -
15 files changed, 49 insertions(+), 60 deletions(-)
diff --git a/drivers/crypto/intel/qat/qat_common/adf_aer.c b/drivers/crypto/intel/qat/qat_common/adf_aer.c
index ed01fb9ad74e..a2364a59bc7f 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_aer.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_aer.c
@@ -89,7 +89,7 @@ EXPORT_SYMBOL_GPL(adf_reset_sbr);
void adf_reset_flr(struct adf_accel_dev *accel_dev)
{
- pcie_flr(accel_to_pci_dev(accel_dev));
+ pcie_reset_flr(accel_to_pci_dev(accel_dev), PCI_RESET_DO_RESET);
}
EXPORT_SYMBOL_GPL(adf_reset_flr);
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 44c524e45396..9f53d73e5e76 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -14042,7 +14042,7 @@ static int init_chip(struct hfi1_devdata *dd)
dd_dev_info(dd, "Resetting CSRs with FLR\n");
/* do the FLR, the DC reset will remain */
- pcie_flr(dd->pcidev);
+ pcie_reset_flr(dd->pcidev, PCI_RESET_DO_RESET);
/* restore command and BARs */
ret = restore_pci_variables(dd);
@@ -14054,7 +14054,7 @@ static int init_chip(struct hfi1_devdata *dd)
if (is_ax(dd)) {
dd_dev_info(dd, "Resetting CSRs with FLR\n");
- pcie_flr(dd->pcidev);
+ pcie_reset_flr(dd->pcidev, PCI_RESET_DO_RESET);
ret = restore_pci_variables(dd);
if (ret) {
dd_dev_err(dd, "%s: Could not restore PCI variables\n",
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_core.c b/drivers/net/ethernet/broadcom/bnge/bnge_core.c
index 68b74eb2c3a2..4aec01f53e54 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_core.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_core.c
@@ -274,7 +274,7 @@ static int bnge_probe_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (is_kdump_kernel()) {
pci_clear_master(pdev);
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
}
rc = bnge_pci_enable(pdev);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 35e1f8f663c7..21f8dcbe671e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16918,7 +16918,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
if (is_kdump_kernel()) {
pci_clear_master(pdev);
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
}
max_irqs = bnxt_get_max_irq(pdev);
@@ -17203,7 +17203,7 @@ static void bnxt_shutdown(struct pci_dev *pdev)
netif_close(dev);
if (bnxt_hwrm_func_drv_unrgtr(bp)) {
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
goto shutdown_exit;
}
bnxt_ptp_clear(bp);
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 43c595f3b84e..7f3557d36341 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -429,7 +429,7 @@ static void octeon_pci_flr(struct octeon_device *oct)
pci_write_config_word(oct->pci_dev, PCI_COMMAND,
PCI_COMMAND_INTX_DISABLE);
- pcie_flr(oct->pci_dev);
+ pcie_reset_flr(oct->pci_dev, PCI_RESET_DO_RESET);
pci_cfg_access_unlock(oct->pci_dev);
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
index ad685f5d0a13..be08e213aa9a 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
@@ -260,7 +260,8 @@ static int octeon_mbox_process_cmd(struct octeon_mbox *mbox,
dev_info(&oct->pci_dev->dev,
"got a request for FLR from VF that owns DPI ring %u\n",
mbox->q_no);
- pcie_flr(oct->sriov_info.dpiring_to_vfpcidev_lut[mbox->q_no]);
+ pcie_reset_flr(oct->sriov_info.dpiring_to_vfpcidev_lut[mbox->q_no],
+ PCI_RESET_DO_RESET);
break;
case OCTEON_PF_CHANGED_VF_MACADDR:
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index aa8a87124b10..c1c1b523abb5 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -3635,7 +3635,7 @@ int enetc_pci_probe(struct pci_dev *pdev, const char *name, int sizeof_priv)
size_t alloc_size;
int err, len;
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
err = pci_enable_device_mem(pdev);
if (err)
return dev_err_probe(&pdev->dev, err, "device enable failed\n");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
index e108cac8288d..cfccfca1981d 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pci_mdio.c
@@ -73,7 +73,7 @@ static int enetc_pci_mdio_probe(struct pci_dev *pdev,
mdio_priv->mdio_base = ENETC_EMDIO_BASE;
snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
err = pci_enable_device_mem(pdev);
if (err) {
dev_err(dev, "device enable failed\n");
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index e2fbe111f849..14b8a90625a8 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5180,7 +5180,7 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
if (is_kdump_kernel()) {
pci_save_state(pdev);
pci_clear_master(pdev);
- err = pcie_flr(pdev);
+ err = pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
if (err)
return err;
pci_restore_state(pdev);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2646ee6f295f..d8796a68094f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8318,7 +8318,7 @@ static void ixgbe_check_for_bad_vf(struct ixgbe_adapter *adapter)
if (status_reg != IXGBE_FAILED_READ_CFG_WORD &&
status_reg & PCI_STATUS_REC_MASTER_ABORT) {
ixgbe_bad_vf_abort(adapter, vf);
- pcie_flr(vfdev);
+ pcie_reset_flr(vfdev, PCI_RESET_DO_RESET);
}
}
}
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index c9b1df1ed109..e51c1170aba7 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -3305,7 +3305,8 @@ static int mana_dealloc_queues(struct net_device *ndev)
}
if (atomic_read(&txq->pending_sends)) {
err =
- pcie_flr(to_pci_dev(gd->gdma_context->dev));
+ pcie_reset_flr(to_pci_dev(gd->gdma_context->dev),
+ PCI_RESET_DO_RESET);
if (err) {
netdev_err(ndev, "flr failed %d with %d pkts pending in txq %u\n",
err,
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d34266651ad0..878556ea50de 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4321,16 +4321,25 @@ int pci_wait_for_pending_transaction(struct pci_dev *dev)
EXPORT_SYMBOL(pci_wait_for_pending_transaction);
/**
- * pcie_flr - initiate a PCIe function level reset
+ * pcie_reset_flr - initiate a PCIe function level reset
* @dev: device to reset
+ * @probe: if true, return 0 if device can be reset this way
*
- * Initiate a function level reset unconditionally on @dev without
- * checking any flags and DEVCAP
+ * Initiate a function level reset on @dev.
*/
-int pcie_flr(struct pci_dev *dev)
+int pcie_reset_flr(struct pci_dev *dev, bool probe)
{
int ret;
+ if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
+ return -ENOTTY;
+
+ if (!(dev->devcap & PCI_EXP_DEVCAP_FLR))
+ return -ENOTTY;
+
+ if (probe)
+ return 0;
+
if (!pci_wait_for_pending_transaction(dev))
pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n");
@@ -4357,28 +4366,7 @@ int pcie_flr(struct pci_dev *dev)
done:
pci_dev_reset_iommu_done(dev);
return ret;
-}
-EXPORT_SYMBOL_GPL(pcie_flr);
-
-/**
- * pcie_reset_flr - initiate a PCIe function level reset
- * @dev: device to reset
- * @probe: if true, return 0 if device can be reset this way
- *
- * Initiate a function level reset on @dev.
- */
-int pcie_reset_flr(struct pci_dev *dev, bool probe)
-{
- if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
- return -ENOTTY;
-
- if (!(dev->devcap & PCI_EXP_DEVCAP_FLR))
- return -ENOTTY;
-
- if (probe)
- return 0;
- return pcie_flr(dev);
}
EXPORT_SYMBOL_GPL(pcie_reset_flr);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index caaed1a01dc0..564f581599b8 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2019,6 +2019,23 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PXH_0, quirk_pc
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PXH_1, quirk_pcie_pxh);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PXHV, quirk_pcie_pxh);
+#define PCI_DEVICE_ID_INTEL_82599_SFP_VF 0x10ed
+static void quirk_intel_82599_sfp_virtfn(struct pci_dev *dev)
+{
+ /*
+ * http://www.intel.com/content/dam/doc/datasheet/82599-10-gbe-controller-datasheet.pdf
+ *
+ * The 82599 supports FLR on VFs, but FLR support is reported only
+ * in the PF DEVCAP (sec 9.3.10.4), not in the VF DEVCAP (sec 9.5).
+ * So enable PCI_EXP_DEVCAP_FLR directly without first checking if it is
+ * supported.
+ */
+
+ dev->devcap |= PCI_EXP_DEVCAP_FLR;
+}
+
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82599_SFP_VF, quirk_intel_82599_sfp_virtfn);
+
/*
* Some Intel PCI Express chipsets have trouble with downstream device
* power management.
@@ -3944,20 +3961,6 @@ DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
* reset a single function if other methods (e.g. FLR, PM D0->D3) are
* not available.
*/
-static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, bool probe)
-{
- /*
- * http://www.intel.com/content/dam/doc/datasheet/82599-10-gbe-controller-datasheet.pdf
- *
- * The 82599 supports FLR on VFs, but FLR support is reported only
- * in the PF DEVCAP (sec 9.3.10.4), not in the VF DEVCAP (sec 9.5).
- * Thus we must call pcie_flr() directly without first checking if it is
- * supported.
- */
- if (!probe)
- pcie_flr(dev);
- return 0;
-}
#define SOUTH_CHICKEN2 0xc2004
#define PCH_PP_STATUS 0xc7200
@@ -4058,7 +4061,7 @@ static int reset_chelsio_generic_dev(struct pci_dev *dev, bool probe)
PCI_MSIX_FLAGS_ENABLE |
PCI_MSIX_FLAGS_MASKALL);
- pcie_flr(dev);
+ pcie_reset_flr(dev, PCI_RESET_DO_RESET);
/*
* Restore the configuration information (BAR values, etc.) including
@@ -4070,7 +4073,6 @@ static int reset_chelsio_generic_dev(struct pci_dev *dev, bool probe)
return 0;
}
-#define PCI_DEVICE_ID_INTEL_82599_SFP_VF 0x10ed
#define PCI_DEVICE_ID_INTEL_IVB_M_VGA 0x0156
#define PCI_DEVICE_ID_INTEL_IVB_M2_VGA 0x0166
@@ -4150,7 +4152,7 @@ static int nvme_disable_and_flr(struct pci_dev *dev, bool probe)
pci_iounmap(dev, bar);
- pcie_flr(dev);
+ pcie_reset_flr(dev, PCI_RESET_DO_RESET);
return 0;
}
@@ -4207,7 +4209,7 @@ static int reset_hinic_vf_dev(struct pci_dev *pdev, bool probe)
val = val | HINIC_VF_FLR_PROC_BIT;
iowrite32be(val, bar + HINIC_VF_OP);
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
/*
* The device must recapture its Bus and Device Numbers after FLR
@@ -4238,8 +4240,6 @@ static int reset_hinic_vf_dev(struct pci_dev *pdev, bool probe)
}
static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
- { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82599_SFP_VF,
- reset_intel_82599_sfp_virtfn },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IVB_M_VGA,
reset_ivb_igd },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IVB_M2_VGA,
diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 94e952ee6990..24bae237926a 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -802,7 +802,7 @@ static int netc_timer_pci_probe(struct pci_dev *pdev)
if (!priv)
return -ENOMEM;
- pcie_flr(pdev);
+ pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
err = pci_enable_device_mem(pdev);
if (err)
return dev_err_probe(dev, err, "Failed to enable device\n");
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2c4454583c11..345f0821471a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1468,7 +1468,6 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
int pcie_link_speed_mbps(struct pci_dev *pdev);
void pcie_print_link_status(struct pci_dev *dev);
int pcie_reset_flr(struct pci_dev *dev, bool 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);
int pci_reset_function_locked(struct pci_dev *dev);
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [RFC PATCH v1] PCI: Remove pcie_flr() and convert all callers to use pcie_reset_flr()
2026-06-15 19:21 [RFC PATCH v1] PCI: Remove pcie_flr() and convert all callers to use pcie_reset_flr() Farhan Ali
@ 2026-06-16 19:22 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-16 19:22 UTC (permalink / raw)
To: Farhan Ali; +Cc: imx, Frank.Li, linux-hyperv, linux-pci
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Critical] Unchecked VF FLR failure introduces a reliable DMA Use-After-Free (UAF) in the MANA driver during queue teardown.
- [High] Replacing `pcie_flr()` with `pcie_reset_flr()` silently breaks Function Level Reset (FLR) for all SR-IOV Virtual Functions (VFs) across multiple drivers.
- [High] Silent FLR failure in PCI reset quirks causes the kernel to block for 20 seconds during device teardown or initialization.
- [Low] The patch breaks the kernel build because it missed converting a caller of `pcie_flr()` in the `ixgbe` driver.
Pre-existing issues:
- [Critical] An unbounded MMIO read loop causes an infinite loop and hard lockup if the PCIe device is hot-unplugged or encounters a fatal hardware error.
--
commit e494ba565a5af359d72294da06691da5e6507322
Author: Farhan Ali <alifm@linux.ibm.com>
Subject: PCI: Remove pcie_flr() and convert all callers to use pcie_reset_flr()
This patch removes the unconditional pcie_flr() function and converts its
remaining callers to use pcie_reset_flr(). This ensures validation checks,
such as verifying FLR support in the device capabilities, are applied
consistently across the kernel.
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
[ ... ]
> int pcie_reset_flr(struct pci_dev *dev, bool probe)
> {
> int ret;
>
> if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
> return -ENOTTY;
>
> if (!(dev->devcap & PCI_EXP_DEVCAP_FLR))
> return -ENOTTY;
[Severity: High]
According to the PCIe Base Specification, SR-IOV Virtual Functions do not
report FLR capability in their Device Capabilities register, so this check
will fail for VFs.
Is this a regression that inadvertently breaks FLR for VFs that previously
relied on pcie_flr() bypassing this check? Callers that ignore the return
value will silently skip resetting the hardware.
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
[ ... ]
> @@ -8319,7 +8319,7 @@ static void ixgbe_check_for_bad_vf(struct ixgbe_adapter *adapter)
> if (status_reg != IXGBE_FAILED_READ_CFG_WORD &&
> status_reg & PCI_STATUS_REC_MASTER_ABORT) {
> ixgbe_bad_vf_abort(adapter, vf);
> - pcie_flr(vfdev);
> + pcie_reset_flr(vfdev, PCI_RESET_DO_RESET);
> }
[Severity: Low]
Does this patch introduce a build regression by missing another caller in
ixgbe_main.c?
Searching the tree might reveal that ixgbe_io_error_detected() still calls
the now-removed pcie_flr(), which would cause a build failure when compiling
with CONFIG_IXGBE.
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -3306,7 +3306,8 @@ static int mana_dealloc_queues(struct net_device *ndev)
> }
> if (atomic_read(&txq->pending_sends)) {
> err =
> - pcie_flr(to_pci_dev(gd->gdma_context->dev));
> + pcie_reset_flr(to_pci_dev(gd->gdma_context->dev),
> + PCI_RESET_DO_RESET);
> if (err) {
> netdev_err(ndev, "flr failed %d with %d pkts pending in txq %u\n",
> err,
[Severity: Critical]
If pcie_reset_flr() returns -ENOTTY for VFs, the FLR fails and this loop
breaks early.
Will the driver then proceed to unmap and free active SKB buffers in the
subsequent loop:
mana_dealloc_queues() {
...
for (i = 0; i < apc->num_queues; i++) {
txq = &apc->tx_qp[i].txq;
while ((skb = skb_dequeue(&txq->pending_skbs))) {
mana_unmap_skb(skb, apc);
dev_kfree_skb_any(skb);
}
...
}
Since the hardware wasn't reset and remains active, can this lead to a DMA
use-after-free regression as the hardware continues to process the freed
memory?
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
[ ... ]
> @@ -4207,7 +4209,7 @@ static int reset_hinic_vf_dev(struct pci_dev *pdev, bool probe)
> val = val | HINIC_VF_FLR_PROC_BIT;
> iowrite32be(val, bar + HINIC_VF_OP);
>
> - pcie_flr(pdev);
> + pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
>
> /*
> * The device must recapture its Bus and Device Numbers after FLR
[Severity: High]
If pcie_reset_flr() returns -ENOTTY without actually issuing a reset to the
hardware, will the firmware ever clear the HINIC_VF_FLR_PROC_BIT?
Looking at the rest of reset_hinic_vf_dev(), it appears the code will
subsequently spin in a busy-wait loop for the full 20-second timeout
waiting for that bit to clear:
reset_hinic_vf_dev() {
...
do {
val = ioread32be(bar + HINIC_VF_OP);
if (!(val & HINIC_VF_FLR_PROC_BIT))
goto reset_complete;
msleep(20);
} while (time_before(jiffies, timeout));
...
}
Could this regression silently block the thread for 20 seconds during
device teardown?
> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c
[ ... ]
> @@ -802,7 +802,7 @@ static int netc_timer_pci_probe(struct pci_dev *pdev)
> if (!priv)
> return -ENOMEM;
>
> - pcie_flr(pdev);
> + pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
> err = pci_enable_device_mem(pdev);
[Severity: Critical]
This is a pre-existing issue, but while reviewing this driver's PCIe
handling, does the interrupt handler have an unbounded loop on PCIe
disconnect?
In netc_timer_handle_etts_event(), the code continuously reads a
memory-mapped register:
netc_timer_handle_etts_event() {
...
while (netc_timer_rd(priv, NETC_TMR_STAT) & TMR_STAT_ETS_VLD(index)) {
...
}
...
}
If the device is removed or experiences a fatal error, the MMIO read will
return all 1s (0xFFFFFFFF). Because 0xFFFFFFFF masked with the bit is
always true, would this loop spin infinitely and cause a hard lockup?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615192200.3489-1-alifm@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-16 19:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 19:21 [RFC PATCH v1] PCI: Remove pcie_flr() and convert all callers to use pcie_reset_flr() Farhan Ali
2026-06-16 19:22 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox