[net-next v2 0/3] bnge changes for RoCE driver

[net-next v2 0/3] bnge changes for RoCE driver

[Siva Reddy Kallam]

Add bnge specific changes needed for RoCE MPC channel
support.

v1->v2:
- Created pull request for RoCE MPC channel changes
  of bnge driver 
- Addressed Shasiko review comments

The following are changes since
commit 2c3418fffa9d ("Merge tag 'keys-v7.3-rc7' of
git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd")
   Merge tag 'for_linus' in Linus's tree
and are available in the git repository at:
  https://github.com/siva-kallam/linux bnge-roce-v2
for you to fetch changes up to
commit 4f4b78c1e577 ("bnge: reserve TX/completion rings for the
RoCE MPC channel")

Sachin Holla (1):
  bnge: reserve TX/completion rings for the RoCE MPC channel

Saswati Das (2):
  bnge: Add infrastructure support for RoCE MPC channels
  bnge: Update HSI

 .../net/ethernet/broadcom/bnge/bnge_auxr.c    |   1 +
 .../net/ethernet/broadcom/bnge/bnge_auxr.h    |   1 +
 .../net/ethernet/broadcom/bnge/bnge_resc.c    |  32 +++-
 include/linux/bnge/hsi.h                      | 161 ++++++++++++++++++
 4 files changed, 191 insertions(+), 4 deletions(-)

-- 
2.43.5

[net-next v2 1/3] bnge: Add infrastructure support for RoCE MPC channels

[Siva Reddy Kallam]

From: Saswati Das <saswati.das@broadcom.com>

Add the initial L2 driver plumbing for RoCE MPC (PRIMATE channel)
support:

- Plumb BAR1 through to the RoCE auxiliary device info, alongside the
  existing BAR0, so the RoCE driver can reach the doorbell BAR.

This will be used by follow-on patches to issue RoCE commands over the
MPC channels to the firmware.

Signed-off-by: Saswati Das <saswati.das@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnge/bnge_auxr.c | 1 +
 drivers/net/ethernet/broadcom/bnge/bnge_auxr.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
index 0955b488b6fe..87ae40eab592 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
@@ -181,6 +181,7 @@ static void bnge_set_auxr_dev_info(struct bnge_auxr_dev *auxr_dev,
 	auxr_dev->pf_port_id = bd->pf.port_id;
 	auxr_dev->en_state = bd->state;
 	auxr_dev->bar0 = bd->bar0;
+	auxr_dev->bar1 = bd->bar1;
 }
 
 void bnge_rdma_aux_device_add(struct bnge_dev *bd)
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h
index 6c5c15ef2b0a..13cb9289f5b8 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h
@@ -46,6 +46,7 @@ struct bnge_auxr_dev {
 	struct net_device	*net;
 	struct pci_dev		*pdev;
 	void __iomem		*bar0;
+	void __iomem		*bar1;
 
 	struct bnge_msix_info	msix_info[BNGE_MAX_ROCE_MSIX];
 
-- 
2.43.5

[net-next v2 2/3] bnge: Update HSI

[Siva Reddy Kallam]

From: Saswati Das <saswati.das@broadcom.com>

Add support for 64-bit doorbells and Mid Path Channel (MPC) firmware
commands/completions.

Signed-off-by: Saswati Das <saswati.das@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
---
 include/linux/bnge/hsi.h | 161 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/include/linux/bnge/hsi.h b/include/linux/bnge/hsi.h
index 1f7bd96415a5..a3383f2c7381 100644
--- a/include/linux/bnge/hsi.h
+++ b/include/linux/bnge/hsi.h
@@ -12510,6 +12510,43 @@ struct dbc_dbc {
 	#define DBC_DBC_TYPE_LAST          DBC_DBC_TYPE_NULL
 };
 
+/* dbc_dbc64 (size:64b/8B) */
+struct dbc_dbc64 {
+	__le64	dbc;
+	#define DBC_DBC64_INDEX_MASK        0xffffffUL
+	#define DBC_DBC64_INDEX_SFT         0
+	#define DBC_DBC64_EPOCH             0x1000000UL
+	#define DBC_DBC64_TOGGLE_MASK       0x6000000UL
+	#define DBC_DBC64_TOGGLE_SFT        25
+	#define DBC_DBC64_XID_MASK          0xfffff00000000ULL
+	#define DBC_DBC64_XID_SFT           32
+	#define DBC_DBC64_PATH_MASK         0x300000000000000ULL
+	#define DBC_DBC64_PATH_SFT          56
+	#define DBC_DBC64_PATH_ROCE           (0x0ULL << 56)
+	#define DBC_DBC64_PATH_L2             (0x1ULL << 56)
+	#define DBC_DBC64_PATH_ENGINE         (0x2ULL << 56)
+	#define DBC_DBC64_PATH_LAST          DBC_DBC64_PATH_ENGINE
+	#define DBC_DBC64_VALID             0x400000000000000ULL
+	#define DBC_DBC64_DEBUG_TRACE       0x800000000000000ULL
+	#define DBC_DBC64_TYPE_MASK         0xf000000000000000ULL
+	#define DBC_DBC64_TYPE_SFT          60
+	#define DBC_DBC64_TYPE_SQ             (0x0ULL << 60)
+	#define DBC_DBC64_TYPE_RQ             (0x1ULL << 60)
+	#define DBC_DBC64_TYPE_SRQ            (0x2ULL << 60)
+	#define DBC_DBC64_TYPE_SRQ_ARM        (0x3ULL << 60)
+	#define DBC_DBC64_TYPE_CQ             (0x4ULL << 60)
+	#define DBC_DBC64_TYPE_CQ_ARMSE       (0x5ULL << 60)
+	#define DBC_DBC64_TYPE_CQ_ARMALL      (0x6ULL << 60)
+	#define DBC_DBC64_TYPE_CQ_ARMENA      (0x7ULL << 60)
+	#define DBC_DBC64_TYPE_SRQ_ARMENA     (0x8ULL << 60)
+	#define DBC_DBC64_TYPE_CQ_CUTOFF_ACK  (0x9ULL << 60)
+	#define DBC_DBC64_TYPE_NQ             (0xaULL << 60)
+	#define DBC_DBC64_TYPE_NQ_ARM         (0xbULL << 60)
+	#define DBC_DBC64_TYPE_NQ_MASK        (0xeULL << 60)
+	#define DBC_DBC64_TYPE_NULL           (0xfULL << 60)
+	#define DBC_DBC64_TYPE_LAST          DBC_DBC64_TYPE_NULL
+};
+
 /* db_push_start (size:64b/8B) */
 struct db_push_start {
 	u64	db;
@@ -12605,4 +12642,128 @@ struct hcomm_status {
 
 #define HCOMM_STATUS_STRUCT_LOC 0x31001F0UL
 
+/* MPC macros */
+#define MPC_CMD_HDR_REQ_TYPE_ICA   0x1UL
+#define MPC_CMD_HDR_REQ_TYPE_RCA   0x2UL
+#define MPC_CMD_HDR_REQ_TYPE_PFVF  0x3UL
+#define MPC_CMD_HDR_REQ_TYPE_EVENT 0x4UL
+#define MPC_CMD_HDR_REQ_TYPE_LAST MPC_CMD_HDR_REQ_TYPE_EVENT
+
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SA_ENABLE                  0x0UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SM_PSP_SA_INIT             0x1UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SM_PSP_NEW_RX_ASSOC        0x2UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SM_PSP_KEY_ROTATE          0x3UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_CM_PSP_SA_INIT             0x4UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_CM_PSP_KEY_ROTATE          0x5UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_CM_PSP_KEY_SET             0x6UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SA_INFO_GET                0x7UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SA_STATS_GET               0x8UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SA_STATS_CLEAR             0x9UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SA_RX_MATCH_RULE_SET       0xaUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_ICA_SM_PSP_SA_SET              0xbUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_QP_MODIFY                  0x0UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_AH_MODIFY                  0x1UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_QP_MODIFY_CMPL             0x2UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_AH_MODIFY_CMPL             0x3UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_QP_MODIFY_BATCH            0x4UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_AH_MODIFY_BATCH            0x5UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_QP_MODIFY_BATCH_CMPL       0x6UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_AH_MODIFY_BATCH_CMPL       0x7UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_MPC_TEST_CMD               0xc8UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_RCA_MPC_TEST_CMPL              0xc9UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_EVENT_RESP_CMPL                0x0UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE                 0x0UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE                 0x1UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY                 0x2UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY                 0x3UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY                0x4UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY                0x5UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_CMPL            0xaUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_CMPL            0xbUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_CMPL            0xcUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_CMPL            0xdUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_CMPL           0xeUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_CMPL           0xfUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_FWD_REQ         0x14UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_FWD_REQ         0x15UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_FWD_REQ         0x16UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_FWD_REQ         0x17UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_FWD_REQ        0x18UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_FWD_REQ        0x19UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_FWD_RESP        0x1eUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_FWD_RESP        0x1fUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_FWD_RESP        0x20UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_FWD_RESP        0x21UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_FWD_RESP       0x22UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_FWD_RESP       0x23UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_ALL_FWD_RESP_CMPL         0x28UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_BATCH           0x32UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_BATCH           0x33UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_BATCH           0x34UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_BATCH           0x35UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_BATCH          0x36UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_BATCH          0x37UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_BATCH_CMPL      0x3cUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_BATCH_CMPL      0x3dUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_BATCH_CMPL      0x3eUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_BATCH_CMPL      0x3fUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_BATCH_CMPL     0x40UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_BATCH_CMPL     0x41UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_BATCH_FWD_REQ   0x46UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_BATCH_FWD_REQ   0x47UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_BATCH_FWD_REQ   0x48UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_BATCH_FWD_REQ   0x49UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_BATCH_FWD_REQ  0x4aUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_BATCH_FWD_REQ  0x4bUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_CREATE_BATCH_FWD_RESP  0x50UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_CREATE_BATCH_FWD_RESP  0x51UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_MODIFY_BATCH_FWD_RESP  0x52UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_MODIFY_BATCH_FWD_RESP  0x53UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_QP_DESTROY_BATCH_FWD_RESP 0x54UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_AH_DESTROY_BATCH_FWD_RESP 0x55UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_ALL_BATCH_FWD_RESP_CMPL   0x5aUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_DRV_RGTR                  0x64UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_DRV_RGTR_CMPL             0x65UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_DRV_UNRGTR                0x66UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_DRV_UNRGTR_CMPL           0x67UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_BUF_RGTR                  0x68UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_BUF_RGTR_CMPL             0x69UL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_BUF_UNRGTR                0x6aUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_BUF_UNRGTR_CMPL           0x6bUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_CMD              0xcaUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_CMPL             0xcbUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_FWD_CMD          0xccUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_FWD_CMPL         0xcdUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_FWD_REQ          0xceUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_FWD_RESP         0xcfUL
+#define MPC_CMD_HDR_REQ_SUB_TYPE_LAST    MPC_CMD_HDR_REQ_SUB_TYPE_PFVF_MPC_TEST_FWD_RESP
+
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_NONE                   0x0UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_BAD_TARGET_ID          0x1UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_SA_NOT_INIT_ENABLED    0x2UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_HW_CMD_FAILED          0x3UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_BAD_PSP_MODE           0x4UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_BAD_SPI                0x5UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_SPI_THRESHOLD_BREACHED 0x6UL
+#define MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_ACTIVE_QPS_EXIST       0x7UL
+#define MPC_CMPL_HDR_ERROR_CODE_LAST     MPC_CMPL_HDR_ERROR_CODE_ICA_ERR_ACTIVE_QPS_EXIST
+
+/* mpc_cmpl_hdr (size:64b/8B) */
+struct mpc_cmpl_hdr {
+	u8	cmpl_type_reserved;
+	#define CMPL_TYPE_RESERVED_CMPL_TYPE_MASK         0x3fUL
+	#define CMPL_TYPE_RESERVED_CMPL_TYPE_SFT          0
+	#define CMPL_TYPE_RESERVED_CMPL_TYPE_MPC_CMP_SHORT  0x1eUL
+	#define CMPL_TYPE_RESERVED_CMPL_TYPE_MPC_CMP_LONG   0x1fUL
+	#define CMPL_TYPE_RESERVED_CMPL_TYPE_LAST          CMPL_TYPE_RESERVED_CMPL_TYPE_MPC_CMP_LONG
+	u8	error_code_mp_client;
+	#define ERROR_CODE_MP_CLIENT_ERROR_CODE_MASK 0xfUL
+	#define ERROR_CODE_MP_CLIENT_ERROR_CODE_SFT 0
+	#define ERROR_CODE_MP_CLIENT_MP_CLIENT_MASK 0xf0UL
+	#define ERROR_CODE_MP_CLIENT_MP_CLIENT_SFT  4
+	u8	req_type;
+	u8	req_subtype;
+	__le32	opaque;
+};
+
 #endif /* _BNGE_HSI_H_ */
-- 
2.43.5

[net-next v2 3/3] bnge: reserve TX/completion rings for the RoCE MPC channel

[Siva Reddy Kallam]

From: Sachin Holla <sachin.holla@broadcom.com>

When RoCE is enabled, the bng_re driver allocates its own TX/CQ rings
for the MPC control channel out of the same per-function FW ring pool.
Enhanced bnge_reserve_rings() to account for this extra ring count to
avoid pool overflow while allocating netdev rings.

Signed-off-by: Sachin Holla <sachin.holla@broadcom.com>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
---
 .../net/ethernet/broadcom/bnge/bnge_resc.c    | 32 ++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
index 4711dd4945ff..9dfb22b7c7c7 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
@@ -110,9 +110,24 @@ static u16 bnge_nqs_demand(struct bnge_dev *bd)
 	return bd->nq_nr_rings + bnge_aux_get_msix(bd);
 }
 
+static u16 bnge_tx_rings_demand(struct bnge_dev *bd)
+{
+	u16 tx_rings = bd->tx_nr_rings;
+
+	if (bnge_is_roce_en(bd))
+		tx_rings += 1; /* For MPC TX ring */
+
+	return tx_rings;
+}
+
 static u16 bnge_cprs_demand(struct bnge_dev *bd)
 {
-	return bd->tx_nr_rings + bd->rx_nr_rings;
+	u16 cprs = bd->tx_nr_rings + bd->rx_nr_rings;
+
+	if (bnge_is_roce_en(bd))
+		cprs += 1; /* For MPC CQ ring */
+
+	return cprs;
 }
 
 static u16 bnge_get_avail_msix(struct bnge_dev *bd, int num)
@@ -250,7 +265,7 @@ static bool bnge_need_reserve_rings(struct bnge_dev *bd)
 	u16 nqs = bnge_nqs_demand(bd);
 	u16 vnic;
 
-	if (hw_resc->resv_tx_rings != bd->tx_nr_rings)
+	if (hw_resc->resv_tx_rings != bnge_tx_rings_demand(bd))
 		return true;
 
 	vnic = bnge_get_total_vnics(bd, rx);
@@ -275,6 +290,7 @@ int bnge_reserve_rings(struct bnge_dev *bd)
 	u16 nq = bd->nq_nr_rings;
 	u16 aux_msix = 0;
 	bool sh = false;
+	u16 tx_demand;
 	u16 tx_cp;
 	int rc;
 
@@ -293,11 +309,12 @@ int bnge_reserve_rings(struct bnge_dev *bd)
 		hwr.nq = bnge_nqs_demand(bd);
 	}
 
-	hwr.tx = bd->tx_nr_rings;
+	tx_demand = bnge_tx_rings_demand(bd);
+	hwr.tx = tx_demand;
 	hwr.rx = bd->rx_nr_rings;
 	if (bd->flags & BNGE_EN_SHARED_CHNL)
 		sh = true;
-	hwr.cmpl = hwr.rx + hwr.tx;
+	hwr.cmpl = bnge_cprs_demand(bd);
 
 	hwr.vnic = bnge_get_total_vnics(bd, hwr.rx);
 
@@ -327,6 +344,13 @@ int bnge_reserve_rings(struct bnge_dev *bd)
 	if (hwr.stat > bnge_aux_get_stat_ctxs(bd))
 		hwr.stat -= bnge_aux_get_stat_ctxs(bd);
 	hwr.nq = min_t(u16, hwr.nq, hwr.stat);
+	/* Drop MPC TX ring from L2 TX ring count; it is owned by RoCE driver */
+	if (bnge_is_roce_en(bd)) {
+		if (hwr.tx >= tx_demand)
+			hwr.tx -= 1;
+		else
+			return -ENOMEM;
+	}
 
 	/* Adjust the rings */
 	rc = bnge_adjust_rings(bd, &rx_rings, &hwr.tx, hwr.nq, sh);
-- 
2.43.5

Re: [net-next v2 1/3] bnge: Add infrastructure support for RoCE MPC channels

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch has no consumer, and its subject and body promise more
  than it does.

--

bnge: Add infrastructure support for RoCE MPC channels

This patch adds a bar1 pointer to struct bnge_auxr_dev. It also fills that
pointer in from bd->bar1 in bnge_set_auxr_dev_info(), next to the existing
bar0. The commit message says follow-on patches will use it to send RoCE
commands to the firmware over the MPC channels.

> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
> index 0955b488b6fea..87ae40eab5926 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.c
> @@ -181,6 +181,7 @@ static void bnge_set_auxr_dev_info(struct bnge_auxr_dev *auxr_dev,
>  	auxr_dev->pf_port_id = bd->pf.port_id;
>  	auxr_dev->en_state = bd->state;
>  	auxr_dev->bar0 = bd->bar0;
> +	auxr_dev->bar1 = bd->bar1;
>  }
>  
>  void bnge_rdma_aux_device_add(struct bnge_dev *bd)
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h
> index 6c5c15ef2b0a7..13cb9289f5b89 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_auxr.h
> @@ -46,6 +46,7 @@ struct bnge_auxr_dev {
>  	struct net_device	*net;
>  	struct pci_dev		*pdev;
>  	void __iomem		*bar0;
> +	void __iomem		*bar1;

[Severity: Low]
Does anything read auxr_dev->bar1? This commit has no user of the new
field. The later patches in the series ("bnge: Update HSI" and "bnge:
reserve TX/completion rings for the RoCE MPC channel") don't add one either.

drivers/infiniband/hw/bng_re/ dereferences neither ->bar0 nor ->bar1. It
still maps its own regions, in bng_re_map_cmdq_mbox():

	mbox->reg.bar_base = pci_resource_start(pdev, mbox->reg.bar_id);

and in bng_re_map_creq_db():

	creq_db->reg.bar_base = pci_resource_start(pdev, creq_db->reg.bar_id);

Could this assignment and the new field move into the bng_re patch that
first uses auxr_dev->bar1?

The subject and body also describe more than the patch does:

    bnge: Add infrastructure support for RoCE MPC channels

    Add the initial L2 driver plumbing for RoCE MPC (PRIMATE channel)
    support:

Nothing in the change is specific to MPC or PRIMATE. The body also has a
bullet list with only one item.

If the field stays in this patch, could the message say that it only
passes the existing L2 BAR1 doorbell mapping to the RoCE auxiliary device?

>  
>  	struct bnge_msix_info	msix_info[BNGE_MAX_ROCE_MSIX];
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006161256.1493672-1-siva.kallam%40broadcom.com

Re: [net-next v2 3/3] bnge: reserve TX/completion rings for the RoCE MPC channel

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] In bnge_reserve_rings()
  (drivers/net/ethernet/broadcom/bnge/bnge_resc.c:347-353), the new RoCE…
- [High] The patch adds an MPC TX ring and CQ on top of the L2 demand.
- [Medium] The commit message says the bng_re driver allocates its own
  TX/CQ rings for an MPC control channel from the per-function FW ring…

Pre-existing issues:
- [Low] bnge_reserve_rings() now reads bnge_is_roce_en() separately for
  need_reserve, tx_demand/hwr.tx (line 312), hwr.cmpl (line 317) and the…

--

bnge: reserve TX/completion rings for the RoCE MPC channel

When RoCE is enabled, this patch makes bnge_reserve_rings() request one
extra TX ring and one extra completion ring for an MPC control channel
that the commit says bng_re owns. Once firmware has made the reservation,
the extra TX ring is taken out of the L2 TX count. If firmware did not
grant the full TX demand, the function returns -ENOMEM.

> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
> index 4711dd4945ffe..9dfb22b7c7c7a 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_resc.c
> @@ -110,9 +110,24 @@ static u16 bnge_nqs_demand(struct bnge_dev *bd)
>  	return bd->nq_nr_rings + bnge_aux_get_msix(bd);
>  }
>  
> +static u16 bnge_tx_rings_demand(struct bnge_dev *bd)
> +{
> +	u16 tx_rings = bd->tx_nr_rings;
> +
> +	if (bnge_is_roce_en(bd))
> +		tx_rings += 1; /* For MPC TX ring */

[Severity: Medium]

The commit message says:

  When RoCE is enabled, the bng_re driver allocates its own TX/CQ rings
  for the MPC control channel out of the same per-function FW ring pool.

Is there an in-tree user of this ring? In
drivers/infiniband/hw/bng_re/bng_dev.c, the only firmware ring allocation
is in bng_re_dev_init(), and it allocates an NQ for the CREQ:

bng_re_dev_init() {
    ...
	type = RING_ALLOC_REQ_RING_TYPE_NQ;
	creq = &rdev->rcfw.creq;
    ...
	rc = bng_re_net_ring_alloc(rdev, &rattr, &creq->ring_id);
    ...
}

Nothing else calls bng_re_net_ring_alloc(), and the matching free in
bng_re_dev_uninit() is also for the NQ. A search for mpc in bng_re and
bnge only turns up the comments this patch adds.

In the current tree, each RoCE-enabled function gives up one TX ring and
one completion ring that nothing uses. The "pool overflow" in the commit
message also doesn't look reachable.

If this prepares for a future bng_re MPC series, could the commit message
say so, or could the patch go in with that series? The same goes for the
"it is owned by RoCE driver" comment in bnge_reserve_rings().

> +
> +	return tx_rings;
> +}
> +
>  static u16 bnge_cprs_demand(struct bnge_dev *bd)
>  {
> -	return bd->tx_nr_rings + bd->rx_nr_rings;
> +	u16 cprs = bd->tx_nr_rings + bd->rx_nr_rings;
> +
> +	if (bnge_is_roce_en(bd))
> +		cprs += 1; /* For MPC CQ ring */
> +
> +	return cprs;
>  }

[Severity: High]

Can the extra ring push the request past the firmware pool it is meant
to protect?

The L2 maximums still let L2 take the whole TX pool:

_bnge_get_max_rings() {
    ...
	*max_tx = hw_resc->max_tx_rings;
    ...
}

The RoCE branch of bnge_get_dflt_rings() lowers max_cp, max_irq and
max_stat, but leaves max_tx alone:

	max_cp -= BNGE_MIN_ROCE_CP_RINGS;
	max_irq -= BNGE_MIN_ROCE_CP_RINGS;
	max_stat -= BNGE_MIN_ROCE_STAT_CTXS;
	...
	rc = bnge_adjust_rings(bd, max_rx, max_tx, max_cp, shared);

Shared mode is the only mode used, via bnge_net_init_dflt_rings(bd, true).
In that mode TX and RX are each clamped to max_cp, not their sum.

bnge_net_init_dflt_rings() then does:

	bd->tx_nr_rings_per_tc = min_t(u16, dflt_rings, max_tx_rings);

If max_tx_rings is at or below netif_get_num_default_rss_queues(),
tx_nr_rings equals max_tx_rings. bnge_tx_rings_demand() then asks for
max_tx_rings + 1, and __bnge_hwrm_reserve_pf_rings() passes that to
FUNC_CFG as is:

	req->num_tx_rings = cpu_to_le16(hwr->tx);
	...
	req->num_cmpl_rings = cpu_to_le16(hwr->cmpl);

Completion rings have the same problem. _bnge_get_max_rings() only
enforces rx + tx <= max_cp, so rx + tx + 1 can go over it.

>From there firmware has two options:

- Reject FUNC_CFG. bnge_reserve_rings() then returns before any counts
  are adjusted.
- Grant at most max_tx_rings. That hits the new -ENOMEM check below.

In both cases the retry in bnge_net_init_dflt_rings() sends the same
request again and probe fails, even though (L2 TX - 1) plus MPC would fit.

Should the L2 max/default calculation hold back one TX ring and one
completion ring for MPC when RoCE is enabled? Relaxing the check below
would not fix the FUNC_CFG rejection case.

[ ... ]

> @@ -293,11 +309,12 @@ int bnge_reserve_rings(struct bnge_dev *bd)
>  		hwr.nq = bnge_nqs_demand(bd);
>  	}
>  
> -	hwr.tx = bd->tx_nr_rings;
> +	tx_demand = bnge_tx_rings_demand(bd);
> +	hwr.tx = tx_demand;
>  	hwr.rx = bd->rx_nr_rings;
>  	if (bd->flags & BNGE_EN_SHARED_CHNL)
>  		sh = true;
> -	hwr.cmpl = hwr.rx + hwr.tx;
> +	hwr.cmpl = bnge_cprs_demand(bd);

[Severity: Low]

This is a pre-existing issue and was not introduced by this patch. Could
BNGE_EN_ROCE change between reads here? bnge_reserve_rings() now reads
bnge_is_roce_en() in four places:

- bnge_need_reserve_rings()
- the tx_demand calculation
- bnge_cprs_demand()
- the MPC subtraction after the firmware exchange

The flag can be cleared without holding rtnl or the netdev lock:

bnge_rdma_aux_device_add() {
    ...
	bd->flags &= ~BNGE_EN_ROCE;
    ...
}

bnge_probe_one() calls this after bnge_netdev_alloc() has registered the
netdev. So ndo_open->bnge_open_core()->bnge_reserve_rings() could run at
the same time.

I don't think the result is harmful:

- If the flag clears between tx_demand and hwr.cmpl,
  bnge_hwrm_get_rings() sees cp < rx + tx and trims the reserved TX/RX
  counts through bnge_fix_rings_count().
- If it clears before the subtraction, L2 keeps one extra TX ring. That
  ring still fits inside the firmware reservation, and
  bnge_adjust_rings() clamps it.

At worst, one TX ring stays over-reserved until the next reservation.

Both the unlocked flag clear and the earlier flag-dependent reads (via
bnge_aux_get_msix() and bnge_aux_get_stat_ctxs()) existed before this
patch.

>  
>  	hwr.vnic = bnge_get_total_vnics(bd, hwr.rx);
>  
> @@ -327,6 +344,13 @@ int bnge_reserve_rings(struct bnge_dev *bd)
>  	if (hwr.stat > bnge_aux_get_stat_ctxs(bd))
>  		hwr.stat -= bnge_aux_get_stat_ctxs(bd);
>  	hwr.nq = min_t(u16, hwr.nq, hwr.stat);
> +	/* Drop MPC TX ring from L2 TX ring count; it is owned by RoCE driver */
> +	if (bnge_is_roce_en(bd)) {
> +		if (hwr.tx >= tx_demand)
> +			hwr.tx -= 1;
> +		else
> +			return -ENOMEM;
> +	}

[Severity: High]

Does this make any partial TX grant from firmware a hard failure?

By this point bnge_copy_reserved_rings() has replaced hwr.tx with the
granted count. That count can't be larger than tx_demand, so the check
in effect requires the full tx_nr_rings + 1.

bnge_hwrm_get_rings() can lower resv_tx_rings even when the HWRM command
succeeds. Firmware may return a smaller alloc_tx_rings, or the count is
trimmed to fit the granted completion rings:

	if (cp < (rx + tx)) {
		rc = bnge_fix_rings_count(&rx, &tx, cp, false);
		...
		hw_resc->resv_tx_rings = tx;

For example, take 8 L2 TX rings and 8 RX rings. The request is 9 TX and
17 completion rings. If firmware grants 16 completion rings, TX is
trimmed to 8. Since 8 < 9, this returns -ENOMEM, even though 7 L2 TX
rings plus MPC would have fit.

Before this patch, bnge_adjust_rings() absorbed a short grant and
bd->tx_nr_rings was set to the granted value.

Now the return comes after firmware has committed the reservation and
hw_resc has been updated. It also comes before any of these are updated:

- bd->tx_nr_rings, bd->rx_nr_rings and bd->nq_nr_rings
- the RSS table
- the aux MSI-X/stat-ctx split

tx_nr_rings is never lowered, so bnge_need_reserve_rings() stays true.
The re-reserve retry in bnge_net_init_dflt_rings() then sends the same
demand and fails again:

	if (bnge_need_reserve_rings(bd)) {
		rc = bnge_reserve_rings(bd);
		...
	}
	if (rc) {
		bd->tx_nr_rings = 0;
		bd->rx_nr_rings = 0;
	}

That leads to bnge_probe_one() failing with "Error setting up default
cfg". bnge_open_core() hits the same check, so the interface couldn't be
opened either.

There may also be a knock-on effect on the RSS table. rss_indir_tbl is
allocated with kmalloc_array() and only initialized when the reserved RX
count changes:

	if (old_rx_rings != bd->hw_resc.resv_rx_rings)
		bnge_set_dflt_rss_indir_tbl(bd);

Suppose a reservation changes resv_rx_rings and then fails here. A later
successful call with the same RX grant would then skip the
initialization. bnge_fill_hw_rss_tbl() uses the stale or uninitialized
entries as indexes:

	j = bd->rss_indir_tbl[i];
	rxr = &bn->rx_ring[j];

Can that index past the end of bn->rx_ring[]?

Would it work to only require room for MPC plus one L2 TX ring here? For
example, hwr.tx >= 2, like the hwr.rx >= 2 aggregation check above, and
then let bnge_adjust_rings() scale down as before.

>  
>  	/* Adjust the rings */
>  	rc = bnge_adjust_rings(bd, &rx_rings, &hwr.tx, hwr.nq, sh);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006161256.1493672-1-siva.kallam%40broadcom.com