[PATCH net-next v2 0/6] bnxt_en: Driver update

[PATCH net-next v2 0/6] bnxt_en: Driver update

[Michael Chan]

The first patch configures context memory for RoCE resources based
on FW limits.  The next 4 patches restrict certain ethtool
operations when they are not supported.  The last patch adds Pavan
Chebbi as co-maintainer of the driver.

v2: Use extack in patch 2
    Fix uninitialized variable in patch 2
    Use void function in patch 4

v1: https://lore.kernel.org/netdev/20241215205943.2341612-1-michael.chan@broadcom.com/

Hongguang Gao (1):
  bnxt_en: Use FW defined resource limits for RoCE

Michael Chan (5):
  bnxt_en: Do not allow ethtool -m on an untrusted VF
  bnxt_en: Skip PHY loopback ethtool selftest if unsupported by FW
  bnxt_en: Skip MAC loopback selftest if it is unsupported by FW
  bnxt_en: Skip reading PXP registers during ethtool -d if unsupported
  MAINTAINERS: bnxt_en: Add Pavan Chebbi as co-maintainer

 MAINTAINERS                                   |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 71 +++++++++++++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     | 13 ++++
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 44 +++++++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  2 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  2 +
 6 files changed, 109 insertions(+), 24 deletions(-)

-- 
2.30.1

[PATCH net-next v2 1/6] bnxt_en: Use FW defined resource limits for RoCE

[Michael Chan]

From: Hongguang Gao <hongguang.gao@broadcom.com>

If FW supports setting resource limits for RoCE, then just use the
FW limits instead of using some fixed values in the driver.  These
limits will be used to allocate context memory for QP, SRQ, AH, and
MR resources for RoCE.

Reviewed-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Hongguang Gao <hongguang.gao@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 41 +++++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  3 ++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c |  2 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  2 +
 4 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b86f980fa7ea..469352ac1f7e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -9117,10 +9117,18 @@ static int bnxt_alloc_ctx_mem(struct bnxt *bp)
 	ena = 0;
 	if ((bp->flags & BNXT_FLAG_ROCE_CAP) && !is_kdump_kernel()) {
 		pg_lvl = 2;
-		extra_qps = min_t(u32, 65536, max_qps - l2_qps - qp1_qps);
-		/* allocate extra qps if fw supports RoCE fast qp destroy feature */
-		extra_qps += fast_qpmd_qps;
-		extra_srqs = min_t(u32, 8192, max_srqs - srqs);
+		if (BNXT_SW_RES_LMT(bp)) {
+			extra_qps = max_qps - l2_qps - qp1_qps;
+			extra_srqs = max_srqs - srqs;
+		} else {
+			extra_qps = min_t(u32, 65536,
+					  max_qps - l2_qps - qp1_qps);
+			/* allocate extra qps if fw supports RoCE fast qp
+			 * destroy feature
+			 */
+			extra_qps += fast_qpmd_qps;
+			extra_srqs = min_t(u32, 8192, max_srqs - srqs);
+		}
 		if (fast_qpmd_qps)
 			ena |= FUNC_BACKING_STORE_CFG_REQ_ENABLES_QP_FAST_QPMD;
 	}
@@ -9156,14 +9164,20 @@ static int bnxt_alloc_ctx_mem(struct bnxt *bp)
 		goto skip_rdma;
 
 	ctxm = &ctx->ctx_arr[BNXT_CTX_MRAV];
-	/* 128K extra is needed to accommodate static AH context
-	 * allocation by f/w.
-	 */
-	num_mr = min_t(u32, ctxm->max_entries / 2, 1024 * 256);
-	num_ah = min_t(u32, num_mr, 1024 * 128);
-	ctxm->split_entry_cnt = BNXT_CTX_MRAV_AV_SPLIT_ENTRY + 1;
-	if (!ctxm->mrav_av_entries || ctxm->mrav_av_entries > num_ah)
-		ctxm->mrav_av_entries = num_ah;
+	if (BNXT_SW_RES_LMT(bp) &&
+	    ctxm->split_entry_cnt == BNXT_CTX_MRAV_AV_SPLIT_ENTRY + 1) {
+		num_ah = ctxm->mrav_av_entries;
+		num_mr = ctxm->max_entries - num_ah;
+	} else {
+		/* 128K extra is needed to accommodate static AH context
+		 * allocation by f/w.
+		 */
+		num_mr = min_t(u32, ctxm->max_entries / 2, 1024 * 256);
+		num_ah = min_t(u32, num_mr, 1024 * 128);
+		ctxm->split_entry_cnt = BNXT_CTX_MRAV_AV_SPLIT_ENTRY + 1;
+		if (!ctxm->mrav_av_entries || ctxm->mrav_av_entries > num_ah)
+			ctxm->mrav_av_entries = num_ah;
+	}
 
 	rc = bnxt_setup_ctxm_pg_tbls(bp, ctxm, num_mr + num_ah, 2);
 	if (rc)
@@ -9470,6 +9484,9 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 		bp->flags |= BNXT_FLAG_UDP_GSO_CAP;
 	if (flags_ext2 & FUNC_QCAPS_RESP_FLAGS_EXT2_TX_PKT_TS_CMPL_SUPPORTED)
 		bp->fw_cap |= BNXT_FW_CAP_TX_TS_CMP;
+	if (flags_ext2 &
+	    FUNC_QCAPS_RESP_FLAGS_EXT2_SW_MAX_RESOURCE_LIMITS_SUPPORTED)
+		bp->fw_cap |= BNXT_FW_CAP_SW_MAX_RESOURCE_LIMITS;
 	if (BNXT_PF(bp) &&
 	    (flags_ext2 & FUNC_QCAPS_RESP_FLAGS_EXT2_ROCE_VF_RESOURCE_MGMT_SUPPORTED))
 		bp->fw_cap |= BNXT_FW_CAP_ROCE_VF_RESC_MGMT_SUPPORTED;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 7df7a2233307..3e20d200da62 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2482,6 +2482,7 @@ struct bnxt {
 	#define BNXT_FW_CAP_CFA_NTUPLE_RX_EXT_IP_PROTO	BIT_ULL(38)
 	#define BNXT_FW_CAP_CFA_RFS_RING_TBL_IDX_V3	BIT_ULL(39)
 	#define BNXT_FW_CAP_VNIC_RE_FLUSH		BIT_ULL(40)
+	#define BNXT_FW_CAP_SW_MAX_RESOURCE_LIMITS	BIT_ULL(41)
 
 	u32			fw_dbg_cap;
 
@@ -2501,6 +2502,8 @@ struct bnxt {
 	((bp)->fw_cap & BNXT_FW_CAP_ENABLE_RDMA_SRIOV)
 #define BNXT_ROCE_VF_RESC_CAP(bp)	\
 	((bp)->fw_cap & BNXT_FW_CAP_ROCE_VF_RESC_MGMT_SUPPORTED)
+#define BNXT_SW_RES_LMT(bp)		\
+	((bp)->fw_cap & BNXT_FW_CAP_SW_MAX_RESOURCE_LIMITS)
 
 	u32			hwrm_spec_code;
 	u16			hwrm_cmd_seq;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index b771c84cdd89..94c6a0928ca0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -416,6 +416,8 @@ static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 		edev->flags |= BNXT_EN_FLAG_VF;
 	if (BNXT_ROCE_VF_RESC_CAP(bp))
 		edev->flags |= BNXT_EN_FLAG_ROCE_VF_RES_MGMT;
+	if (BNXT_SW_RES_LMT(bp))
+		edev->flags |= BNXT_EN_FLAG_SW_RES_LMT;
 
 	edev->chip_num = bp->chip_num;
 	edev->hw_ring_stats_size = bp->hw_ring_stats_size;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 5d6aac60f236..54ad9f8273d7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -65,6 +65,8 @@ struct bnxt_en_dev {
 	#define BNXT_EN_FLAG_VF			0x10
 #define BNXT_EN_VF(edev)	((edev)->flags & BNXT_EN_FLAG_VF)
 	#define BNXT_EN_FLAG_ROCE_VF_RES_MGMT	0x20
+	#define BNXT_EN_FLAG_SW_RES_LMT		0x40
+#define BNXT_EN_SW_RES_LMT(edev) ((edev)->flags & BNXT_EN_FLAG_SW_RES_LMT)
 
 	struct bnxt_ulp			*ulp_tbl;
 	int				l2_db_size;	/* Doorbell BAR size in
-- 
2.30.1

[PATCH net-next v2 2/6] bnxt_en: Do not allow ethtool -m on an untrusted VF

[Michael Chan]

Block all ethtool module operations on an untrusted VF.  The firmware
won't allow it and will return error.

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v2: Use extack for bnxt_get_module_eeprom_by_page()
    Fix uninitialized variable
Cc: Ido Schimmel <idosch@idosch.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         |  6 +++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  5 +++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 12 ++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 469352ac1f7e..c0728d5ff8bc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -8279,16 +8279,20 @@ static int bnxt_hwrm_func_qcfg(struct bnxt *bp)
 	if (rc)
 		goto func_qcfg_exit;
 
+	flags = le16_to_cpu(resp->flags);
 #ifdef CONFIG_BNXT_SRIOV
 	if (BNXT_VF(bp)) {
 		struct bnxt_vf_info *vf = &bp->vf;
 
 		vf->vlan = le16_to_cpu(resp->vlan) & VLAN_VID_MASK;
+		if (flags & FUNC_QCFG_RESP_FLAGS_TRUSTED_VF)
+			vf->flags |= BNXT_VF_TRUST;
+		else
+			vf->flags &= ~BNXT_VF_TRUST;
 	} else {
 		bp->pf.registered_vfs = le16_to_cpu(resp->registered_vfs);
 	}
 #endif
-	flags = le16_to_cpu(resp->flags);
 	if (flags & (FUNC_QCFG_RESP_FLAGS_FW_DCBX_AGENT_ENABLED |
 		     FUNC_QCFG_RESP_FLAGS_FW_LLDP_AGENT_ENABLED)) {
 		bp->fw_cap |= BNXT_FW_CAP_LLDP_AGENT;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 3e20d200da62..d5e81e008ab5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2270,6 +2270,11 @@ struct bnxt {
 
 #define BNXT_PF(bp)		(!((bp)->flags & BNXT_FLAG_VF))
 #define BNXT_VF(bp)		((bp)->flags & BNXT_FLAG_VF)
+#ifdef CONFIG_BNXT_SRIOV
+#define	BNXT_VF_IS_TRUSTED(bp)	((bp)->vf.flags & BNXT_VF_TRUST)
+#else
+#define	BNXT_VF_IS_TRUSTED(bp)	0
+#endif
 #define BNXT_NPAR(bp)		((bp)->port_partition_type)
 #define BNXT_MH(bp)		((bp)->flags & BNXT_FLAG_MULTI_HOST)
 #define BNXT_SINGLE_PF(bp)	(BNXT_PF(bp) && !BNXT_NPAR(bp) && !BNXT_MH(bp))
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index d87681d71106..28f2c471652c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -4375,6 +4375,9 @@ static int bnxt_get_module_info(struct net_device *dev,
 	struct bnxt *bp = netdev_priv(dev);
 	int rc;
 
+	if (BNXT_VF(bp) && !BNXT_VF_IS_TRUSTED(bp))
+		return -EPERM;
+
 	/* No point in going further if phy status indicates
 	 * module is not inserted or if it is powered down or
 	 * if it is of type 10GBase-T
@@ -4426,6 +4429,9 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
 	u16  start = eeprom->offset, length = eeprom->len;
 	int rc = 0;
 
+	if (BNXT_VF(bp) && !BNXT_VF_IS_TRUSTED(bp))
+		return -EPERM;
+
 	memset(data, 0, eeprom->len);
 
 	/* Read A0 portion of the EEPROM */
@@ -4480,6 +4486,12 @@ static int bnxt_get_module_eeprom_by_page(struct net_device *dev,
 	struct bnxt *bp = netdev_priv(dev);
 	int rc;
 
+	if (BNXT_VF(bp) && !BNXT_VF_IS_TRUSTED(bp)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Module read not permitted on untrusted VF");
+		return -EPERM;
+	}
+
 	rc = bnxt_get_module_status(bp, extack);
 	if (rc)
 		return rc;
-- 
2.30.1

[PATCH net-next v2 3/6] bnxt_en: Skip PHY loopback ethtool selftest if unsupported by FW

[Michael Chan]

Skip PHY loopback selftest if firmware advertises that it is unsupported
in the HWRM_PORT_PHY_QCAPS call.  Only show PHY loopback test result to
be 0 if the test has run and passes.  Do the same for external loopback
to be consistent.

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c  | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 28f2c471652c..8001849af879 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -4914,20 +4914,26 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 			buf[BNXT_MACLPBK_TEST_IDX] = 0;
 
 		bnxt_hwrm_mac_loopback(bp, false);
+		buf[BNXT_PHYLPBK_TEST_IDX] = 1;
+		if (bp->phy_flags & BNXT_PHY_FL_NO_PHY_LPBK)
+			goto skip_phy_loopback;
+
 		bnxt_hwrm_phy_loopback(bp, true, false);
 		msleep(1000);
-		if (bnxt_run_loopback(bp)) {
-			buf[BNXT_PHYLPBK_TEST_IDX] = 1;
+		if (bnxt_run_loopback(bp))
 			etest->flags |= ETH_TEST_FL_FAILED;
-		}
+		else
+			buf[BNXT_PHYLPBK_TEST_IDX] = 0;
+skip_phy_loopback:
+		buf[BNXT_EXTLPBK_TEST_IDX] = 1;
 		if (do_ext_lpbk) {
 			etest->flags |= ETH_TEST_FL_EXTERNAL_LB_DONE;
 			bnxt_hwrm_phy_loopback(bp, true, true);
 			msleep(1000);
-			if (bnxt_run_loopback(bp)) {
-				buf[BNXT_EXTLPBK_TEST_IDX] = 1;
+			if (bnxt_run_loopback(bp))
 				etest->flags |= ETH_TEST_FL_FAILED;
-			}
+			else
+				buf[BNXT_EXTLPBK_TEST_IDX] = 0;
 		}
 		bnxt_hwrm_phy_loopback(bp, false, false);
 		bnxt_half_close_nic(bp);
-- 
2.30.1

[PATCH net-next v2 4/6] bnxt_en: Skip MAC loopback selftest if it is unsupported by FW

[Michael Chan]

Call the new HWRM_PORT_MAC_QCAPS to check if mac loopback is
supported.  Skip the MAC loopback ethtool self test if it is
not supported.

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v2: Change bnxt_hwrm_mac_qcaps() to void
Cc: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 24 +++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  5 ++++
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 11 +++++----
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c0728d5ff8bc..46edea75e062 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11551,6 +11551,26 @@ static int bnxt_hwrm_phy_qcaps(struct bnxt *bp)
 	return rc;
 }
 
+static void bnxt_hwrm_mac_qcaps(struct bnxt *bp)
+{
+	struct hwrm_port_mac_qcaps_output *resp;
+	struct hwrm_port_mac_qcaps_input *req;
+	int rc;
+
+	if (bp->hwrm_spec_code < 0x10a03)
+		return;
+
+	rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_QCAPS);
+	if (rc)
+		return;
+
+	resp = hwrm_req_hold(bp, req);
+	rc = hwrm_req_send_silent(bp, req);
+	if (!rc)
+		bp->mac_flags = resp->flags;
+	hwrm_req_drop(bp, req);
+}
+
 static bool bnxt_support_dropped(u16 advertising, u16 supported)
 {
 	u16 diff = advertising ^ supported;
@@ -15679,6 +15699,10 @@ static int bnxt_probe_phy(struct bnxt *bp, bool fw_dflt)
 		bp->dev->priv_flags |= IFF_SUPP_NOFCS;
 	else
 		bp->dev->priv_flags &= ~IFF_SUPP_NOFCS;
+
+	bp->mac_flags = 0;
+	bnxt_hwrm_mac_qcaps(bp);
+
 	if (!fw_dflt)
 		return 0;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index d5e81e008ab5..094c9e95b463 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2668,6 +2668,11 @@ struct bnxt {
 #define BNXT_PHY_FL_BANK_SEL		(PORT_PHY_QCAPS_RESP_FLAGS2_BANK_ADDR_SUPPORTED << 8)
 #define BNXT_PHY_FL_SPEEDS2		(PORT_PHY_QCAPS_RESP_FLAGS2_SPEEDS2_SUPPORTED << 8)
 
+	/* copied from flags in hwrm_port_mac_qcaps_output */
+	u8			mac_flags;
+#define BNXT_MAC_FL_NO_MAC_LPBK		\
+	PORT_MAC_QCAPS_RESP_FLAGS_LOCAL_LPBK_NOT_SUPPORTED
+
 	u8			num_tests;
 	struct bnxt_test_info	*test_info;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 8001849af879..c094abfa1ebc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -4899,21 +4899,24 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 		bnxt_close_nic(bp, true, false);
 		bnxt_run_fw_tests(bp, test_mask, &test_results);
 
-		buf[BNXT_MACLPBK_TEST_IDX] = 1;
-		bnxt_hwrm_mac_loopback(bp, true);
-		msleep(250);
 		rc = bnxt_half_open_nic(bp);
 		if (rc) {
-			bnxt_hwrm_mac_loopback(bp, false);
 			etest->flags |= ETH_TEST_FL_FAILED;
 			return;
 		}
+		buf[BNXT_MACLPBK_TEST_IDX] = 1;
+		if (bp->mac_flags & BNXT_MAC_FL_NO_MAC_LPBK)
+			goto skip_mac_loopback;
+
+		bnxt_hwrm_mac_loopback(bp, true);
+		msleep(250);
 		if (bnxt_run_loopback(bp))
 			etest->flags |= ETH_TEST_FL_FAILED;
 		else
 			buf[BNXT_MACLPBK_TEST_IDX] = 0;
 
 		bnxt_hwrm_mac_loopback(bp, false);
+skip_mac_loopback:
 		buf[BNXT_PHYLPBK_TEST_IDX] = 1;
 		if (bp->phy_flags & BNXT_PHY_FL_NO_PHY_LPBK)
 			goto skip_phy_loopback;
-- 
2.30.1

Re: [PATCH net-next v2 4/6] bnxt_en: Skip MAC loopback selftest if it is unsupported by FW

[Michal Swiatkowski]

On Tue, Dec 17, 2024 at 10:26:18AM -0800, Michael Chan wrote:
> Call the new HWRM_PORT_MAC_QCAPS to check if mac loopback is
> supported.  Skip the MAC loopback ethtool self test if it is
> not supported.
> 
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> v2: Change bnxt_hwrm_mac_qcaps() to void
> Cc: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 24 +++++++++++++++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  5 ++++
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 11 +++++----
>  3 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index c0728d5ff8bc..46edea75e062 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -11551,6 +11551,26 @@ static int bnxt_hwrm_phy_qcaps(struct bnxt *bp)
>  	return rc;
>  }
>  
> +static void bnxt_hwrm_mac_qcaps(struct bnxt *bp)
> +{
> +	struct hwrm_port_mac_qcaps_output *resp;
> +	struct hwrm_port_mac_qcaps_input *req;
> +	int rc;
> +
> +	if (bp->hwrm_spec_code < 0x10a03)
> +		return;
> +
> +	rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_QCAPS);
> +	if (rc)
> +		return;
> +
> +	resp = hwrm_req_hold(bp, req);
> +	rc = hwrm_req_send_silent(bp, req);
> +	if (!rc)
> +		bp->mac_flags = resp->flags;
> +	hwrm_req_drop(bp, req);
> +}
> +
>  static bool bnxt_support_dropped(u16 advertising, u16 supported)
>  {
>  	u16 diff = advertising ^ supported;
> @@ -15679,6 +15699,10 @@ static int bnxt_probe_phy(struct bnxt *bp, bool fw_dflt)
>  		bp->dev->priv_flags |= IFF_SUPP_NOFCS;
>  	else
>  		bp->dev->priv_flags &= ~IFF_SUPP_NOFCS;
> +
> +	bp->mac_flags = 0;
> +	bnxt_hwrm_mac_qcaps(bp);
> +
>  	if (!fw_dflt)
>  		return 0;
>
Thanks for changing the return value
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

[...]
> -- 
> 2.30.1
> 

[PATCH net-next v2 5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported

[Michael Chan]

Newer firmware does not allow reading the PXP registers during
ethtool -d, so skip the firmware call in that case.  Userspace
(bnxt.c) always expects the register block to be populated so
zeroes will be returned instead.

Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index c094abfa1ebc..75a59dd72bce 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2050,7 +2050,8 @@ static void bnxt_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	int rc;
 
 	regs->version = 0;
-	bnxt_dbg_hwrm_rd_reg(bp, 0, BNXT_PXP_REG_LEN / 4, _p);
+	if (!(bp->fw_dbg_cap & DBG_QCAPS_RESP_FLAGS_REG_ACCESS_RESTRICTED))
+		bnxt_dbg_hwrm_rd_reg(bp, 0, BNXT_PXP_REG_LEN / 4, _p);
 
 	if (!(bp->fw_cap & BNXT_FW_CAP_PCIE_STATS_SUPPORTED))
 		return;
-- 
2.30.1

Re: [PATCH net-next v2 5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported

[Jakub Kicinski]

On Tue, 17 Dec 2024 10:26:19 -0800 Michael Chan wrote:
> Newer firmware does not allow reading the PXP registers during
> ethtool -d, so skip the firmware call in that case.  Userspace
> (bnxt.c) always expects the register block to be populated so
> zeroes will be returned instead.

We have both the ability to return the number of registers (regs_len),
and the regs->version. Are you sure you don't want to use either option
to let user space know the regs aren't there?

Re: [PATCH net-next v2 5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Wed, Dec 18, 2024 at 7:13 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Dec 2024 10:26:19 -0800 Michael Chan wrote:
> > Newer firmware does not allow reading the PXP registers during
> > ethtool -d, so skip the firmware call in that case.  Userspace
> > (bnxt.c) always expects the register block to be populated so
> > zeroes will be returned instead.
>
> We have both the ability to return the number of registers (regs_len),
> and the regs->version. Are you sure you don't want to use either option
> to let user space know the regs aren't there?

The existing bnxt.c in userspace since 2020 always assumes that the
beginning part always contains the PXP register block regardless of
regs->version as long as the register length >= the length of the
register block.  I guess we didn't anticipate that this PXP block
would ever be changed or FW would disallow reading it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

Re: [PATCH net-next v2 5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported

[Jakub Kicinski]

On Wed, 18 Dec 2024 22:57:09 -0800 Michael Chan wrote:
> > On Tue, 17 Dec 2024 10:26:19 -0800 Michael Chan wrote:  
> > > Newer firmware does not allow reading the PXP registers during
> > > ethtool -d, so skip the firmware call in that case.  Userspace
> > > (bnxt.c) always expects the register block to be populated so
> > > zeroes will be returned instead.  
> >
> > We have both the ability to return the number of registers (regs_len),
> > and the regs->version. Are you sure you don't want to use either option
> > to let user space know the regs aren't there?  
> 
> The existing bnxt.c in userspace since 2020 always assumes that the
> beginning part always contains the PXP register block regardless of
> regs->version as long as the register length >= the length of the
> register block.  I guess we didn't anticipate that this PXP block
> would ever be changed or FW would disallow reading it.

So if you bumped the version the existing userspace wouldn't care but
then you _could_ follow up and update user space to ignore these
registers when version is 1?

It's alright, it's just debug, I got curious recently about how little
use the version field gets. I'm not sure anyone has a good idea on
what to do with it.

Re: [PATCH net-next v2 5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On Thu, Dec 19, 2024 at 6:59 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 18 Dec 2024 22:57:09 -0800 Michael Chan wrote:
> > The existing bnxt.c in userspace since 2020 always assumes that the
> > beginning part always contains the PXP register block regardless of
> > regs->version as long as the register length >= the length of the
> > register block.  I guess we didn't anticipate that this PXP block
> > would ever be changed or FW would disallow reading it.
>
> So if you bumped the version the existing userspace wouldn't care but
> then you _could_ follow up and update user space to ignore these
> registers when version is 1?

Sure, will do.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

Re: [PATCH net-next v2 5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

On Thu, Dec 19, 2024 at 9:53 AM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Thu, Dec 19, 2024 at 6:59 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 18 Dec 2024 22:57:09 -0800 Michael Chan wrote:
> > > The existing bnxt.c in userspace since 2020 always assumes that the
> > > beginning part always contains the PXP register block regardless of
> > > regs->version as long as the register length >= the length of the
> > > register block.  I guess we didn't anticipate that this PXP block
> > > would ever be changed or FW would disallow reading it.
> >
> > So if you bumped the version the existing userspace wouldn't care but
> > then you _could_ follow up and update user space to ignore these
> > registers when version is 1?
>
> Sure, will do.

Actually, it's not that easy.  We already use version 1 to indicate
that there are PCIe stats following the PXP registers.  To make this
work, version will have to be used as flags.

Userspace bnxt.c can detect valid PXP registers in a different way.
If certain non-zero PXP registers are returned as zeros, we know the
PXP register block is not valid and can skip displaying it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

[PATCH net-next v2 6/6] MAINTAINERS: bnxt_en: Add Pavan Chebbi as co-maintainer

[Michael Chan]

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6cced90772fc..2c73a3aacafb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4611,6 +4611,7 @@ F:	drivers/net/ethernet/broadcom/bnx2x/
 
 BROADCOM BNXT_EN 50 GIGABIT ETHERNET DRIVER
 M:	Michael Chan <michael.chan@broadcom.com>
+M:	Pavan Chebbi <pavan.chebbi@broadcom.com>
 L:	netdev@vger.kernel.org
 S:	Supported
 F:	drivers/firmware/broadcom/tee_bnxt_fw.c
-- 
2.30.1

Re: [PATCH net-next v2 0/6] bnxt_en: Driver update

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 17 Dec 2024 10:26:14 -0800 you wrote:
> The first patch configures context memory for RoCE resources based
> on FW limits.  The next 4 patches restrict certain ethtool
> operations when they are not supported.  The last patch adds Pavan
> Chebbi as co-maintainer of the driver.
> 
> v2: Use extack in patch 2
>     Fix uninitialized variable in patch 2
>     Use void function in patch 4
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/6] bnxt_en: Use FW defined resource limits for RoCE
    https://git.kernel.org/netdev/net-next/c/b1b66ae094cd
  - [net-next,v2,2/6] bnxt_en: Do not allow ethtool -m on an untrusted VF
    https://git.kernel.org/netdev/net-next/c/fac5472fc845
  - [net-next,v2,3/6] bnxt_en: Skip PHY loopback ethtool selftest if unsupported by FW
    https://git.kernel.org/netdev/net-next/c/36d1e70a90e9
  - [net-next,v2,4/6] bnxt_en: Skip MAC loopback selftest if it is unsupported by FW
    https://git.kernel.org/netdev/net-next/c/b45a850585ca
  - [net-next,v2,5/6] bnxt_en: Skip reading PXP registers during ethtool -d if unsupported
    https://git.kernel.org/netdev/net-next/c/bf2afe0f1493
  - [net-next,v2,6/6] MAINTAINERS: bnxt_en: Add Pavan Chebbi as co-maintainer
    https://git.kernel.org/netdev/net-next/c/73df38b097a6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html