[PATCH net 0/2] bnxt_en: Fix PM resume on VFs

[PATCH net 0/2] bnxt_en: Fix PM resume on VFs

[Michael Chan]

The 2 patches fix Power Management resume on VFs when the PF is running
in the same kernel.  The first patch maps the specific PF_DOWN FW error
code to a unique errno.  The second patch adds pre-polling on a VF
during PM resume when the error warrants further polling.  This will
allow the PF to finish resume before the VF continues.

This issue was found in the lab when testing system hibernate on a
BCM57608 NIC with 64 VFs created.  The FW version is 238.1.59.0.

Keegan Freyhof (2):
  bnxt_en: Map HWRM_ERR_CODE_PF_UNAVAILABLE to -ECOMM
  bnxt_en: Wait for the PF in bnxt_resume()

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 51 +++++++++++++++----
 .../net/ethernet/broadcom/bnxt/bnxt_hwrm.c    |  4 +-
 2 files changed, 43 insertions(+), 12 deletions(-)

-- 
2.51.0

[PATCH net 1/2] bnxt_en: Map HWRM_ERR_CODE_PF_UNAVAILABLE to -ECOMM

[Michael Chan]

From: Keegan Freyhof <keegan.freyhof@broadcom.com>

The current use of -ENODEV for both PCI being disabled and FW's
HWRM_ERR_CODE_PF_UNAVAILABLE errors makes it impossible for the
caller to distinguish between the 2 errors.  As a result, a disabled
VF device will cause the driver to pointlessly retry setting L2
filters believing that the PF is down.

Fix it by using -ECOMM instead for HWRM_ERR_CODE_PF_UNAVAILABLE
which means that the FW is failing the command on the VF because the
PF is down.  This new -ECOMM error code which is more accurate will
now propagate to userspace instead of -ENODEV on some VF failure
paths when the PF is down.

Fixes: 662c9b22f5b5 ("bnxt_en: improve VF error messages when PF is unavailable")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Keegan Freyhof <keegan.freyhof@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c      | 14 +++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c |  4 +++-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 33c4cacc8ffe..9d232d835758 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11319,7 +11319,7 @@ static int bnxt_init_chip(struct bnxt *bp, bool irq_re_init)
 	/* Filter for default vnic 0 */
 	rc = bnxt_hwrm_set_vnic_filter(bp, 0, 0, bp->dev->dev_addr);
 	if (rc) {
-		if (BNXT_VF(bp) && rc == -ENODEV)
+		if (rc == -ECOMM)
 			netdev_err(bp->dev, "Cannot configure L2 filter while PF is unavailable\n");
 		else
 			netdev_err(bp->dev, "HWRM vnic filter failure rc: %x\n", rc);
@@ -12477,7 +12477,7 @@ int bnxt_update_link(struct bnxt *bp, bool chng_link_state)
 	rc = hwrm_req_send(bp, req);
 	if (rc) {
 		hwrm_req_drop(bp, req);
-		if (BNXT_VF(bp) && rc == -ENODEV) {
+		if (rc == -ECOMM) {
 			netdev_warn(bp->dev, "Cannot obtain link state while PF unavailable.\n");
 			rc = 0;
 		}
@@ -13954,7 +13954,7 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp, struct netdev_hw_addr_list *uc,
 	for (i = 1, off = 0; i < vnic->uc_filter_count; i++, off += ETH_ALEN) {
 		rc = bnxt_hwrm_set_vnic_filter(bp, 0, i, vnic->uc_list + off);
 		if (rc) {
-			if (BNXT_VF(bp) && rc == -ENODEV) {
+			if (rc == -ECOMM) {
 				netdev_warn(bp->dev, "Cannot configure L2 filters while PF is unavailable, will retry\n");
 				rc = -EAGAIN;
 			} else if (rc == -EAGAIN) {
@@ -16997,7 +16997,7 @@ static int bnxt_set_dflt_rings(struct bnxt *bp, bool sh)
 	}
 
 	rc = __bnxt_reserve_rings(bp);
-	if (rc && rc != -ENODEV)
+	if (rc && rc != -ECOMM)
 		netdev_warn(bp->dev, "Unable to reserve tx rings\n");
 
 	bnxt_adj_tx_rings(bp);
@@ -17007,7 +17007,7 @@ static int bnxt_set_dflt_rings(struct bnxt *bp, bool sh)
 	/* Rings may have been trimmed, re-reserve the trimmed rings. */
 	if (bnxt_need_reserve_rings(bp)) {
 		rc = __bnxt_reserve_rings(bp);
-		if (rc && rc != -ENODEV)
+		if (rc && rc != -ECOMM)
 			netdev_warn(bp->dev, "2nd rings reservation failed.\n");
 		bnxt_adj_tx_rings(bp);
 	}
@@ -17033,7 +17033,7 @@ static int bnxt_init_dflt_ring_mode(struct bnxt *bp)
 	bnxt_clear_int_mode(bp);
 	rc = bnxt_set_dflt_rings(bp, true);
 	if (rc) {
-		if (BNXT_VF(bp) && rc == -ENODEV)
+		if (rc == -ECOMM)
 			netdev_err(bp->dev, "Cannot configure VF rings while PF is unavailable.\n");
 		else
 			netdev_err(bp->dev, "Not enough rings available.\n");
@@ -17411,7 +17411,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		bnxt_aux_devices_init(bp);
 	rc = bnxt_set_dflt_rings(bp, true);
 	if (rc) {
-		if (BNXT_VF(bp) && rc == -ENODEV) {
+		if (rc == -ECOMM) {
 			netdev_err(bp->dev, "Cannot configure VF rings while PF is unavailable.\n");
 		} else {
 			netdev_err(bp->dev, "Not enough rings available.\n");
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
index 5bfabdca7d0e..de02d1baa751 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
@@ -360,7 +360,7 @@ static int __hwrm_to_stderr(u32 hwrm_err)
 	case HWRM_ERR_CODE_CMD_NOT_SUPPORTED:
 		return -EOPNOTSUPP;
 	case HWRM_ERR_CODE_PF_UNAVAILABLE:
-		return -ENODEV;
+		return -ECOMM;
 	default:
 		return -EIO;
 	}
@@ -716,6 +716,8 @@ static int __hwrm_send(struct bnxt *bp, struct bnxt_hwrm_ctx *ctx)
  *	ENOMEM: HWRM has no buffers.
  *	EAGAIN: HWRM busy or reset in progress.
  *	EOPNOTSUPP: Invalid request type.
+ *	ENODEV: PCI device is disabled.
+ *	ECOMM: The parent PF is unavailable when issued on VFs.
  *	EIO: Any other error.
  * Error handling is orthogonal to request ownership. An unowned request will
  * still be consumed on error. If the caller owns the request, then the caller
-- 
2.51.0

[PATCH net 2/2] bnxt_en: Wait for the PF in bnxt_resume()

[Michael Chan]

From: Keegan Freyhof <keegan.freyhof@broadcom.com>

When bnxt_resume() is called on a VF with the PF running in the
same kernel, the initial FW message may timeout if the PF has not
been initialized.  Use the more appropriate bnxt_hwrm_poll() with
the silent flag to suppress warning messages and loop up to 10 times
if the error code warrants additional polling.  This initial polling
is done without the rtnl_lock() to allow the PF running in the same
kernel (which will also require the rtnl_lock) to finish its
initialization.

This fixes a failure found in the lab when running system hibernate
on a BCM57608 NIC with 64 VFs created.  The FW version is 238.1.59.0.

bnxt_en 0000:23:08.5 eno4v5: hwrm req_type 0x0 seq id 0xb0 error 0xa
bnxt_en 0000:23:08.5: PM: dpm_run_callback(): pci_pm_restore returns -19
bnxt_en 0000:23:08.5: PM: failed to restore async: error -19
bnxt_en 0000:23:08.3 eno4v3: hwrm req_type 0x0 seq id 0xb0 error 0xa
bnxt_en 0000:23:08.4 eno4v4: hwrm req_type 0x0 seq id 0xb0 error 0xa
bnxt_en 0000:23:08.7 eno4v7: hwrm req_type 0x0 seq id 0xb0 error 0xa
bnxt_en 0000:23:08.3: PM: dpm_run_callback(): pci_pm_restore returns -19
bnxt_en 0000:23:08.4: PM: dpm_run_callback(): pci_pm_restore returns -19

Tested on dual-port BCM57608 with 64 VFs on each port.  The polling
loop provides adequate margins.

Fixes: f65a2044a8c9 ("bnxt_en: Add suspend/resume callbacks.")
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Keegan Freyhof <keegan.freyhof@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 37 ++++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 9d232d835758..03682d9182c6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -17567,21 +17567,50 @@ static int bnxt_suspend(struct device *device)
 	return rc;
 }
 
+#define BNXT_RESUME_MAX_RETRY	10
+
 static int bnxt_resume(struct device *device)
 {
 	struct net_device *dev = dev_get_drvdata(device);
 	struct bnxt *bp = netdev_priv(dev);
-	int rc = 0;
+	int i, rc;
 
-	rtnl_lock();
-	netdev_lock(dev);
 	rc = pci_enable_device(bp->pdev);
 	if (rc) {
 		netdev_err(dev, "Cannot re-enable PCI device during resume, err = %d\n",
 			   rc);
-		goto resume_exit;
+		return rc;
 	}
 	pci_set_master(bp->pdev);
+	if (BNXT_VF(bp)) {
+		for (i = 0; i < BNXT_RESUME_MAX_RETRY; i++) {
+			rc = bnxt_hwrm_poll(bp);
+			if (rc != -EAGAIN && rc != -EBUSY && rc != -ECOMM)
+				break;
+
+			/* Wait briefly for -EAGAIN (FW busy), a bit longer for
+			 * -ECOMM (PF down) to allow the PF to fully initialize.
+			 * For -EBUSY, the message typically has timed out, so
+			 * try again immediately.
+			 */
+			if (i < BNXT_RESUME_MAX_RETRY - 1) {
+				if (rc == -EAGAIN)
+					msleep(250);
+				else if (rc == -ECOMM)
+					msleep(750);
+			}
+		}
+		if (rc) {
+			netdev_err(dev,
+				   "Firmware not responding during resume, rc = %d\n",
+				   rc);
+			return -ENODEV;
+		}
+	}
+
+	rtnl_lock();
+	netdev_lock(dev);
+
 	if (bnxt_hwrm_ver_get(bp)) {
 		rc = -ENODEV;
 		goto resume_exit;
-- 
2.51.0