[PATCH net,0/6] Fixes for CPT and RSS configuration

[PATCH net,0/6] Fixes for CPT and RSS configuration

[Srujana Challa]

This series of patches fixes various issues related to CPT
configuration and RSS configuration.

Kiran Kumar K (1):
  octeontx2-af: Fix issue with IPv6 ext match for RSS

Michal Mazur (1):
  octeontx2-af: fix detection of IP layer

Nithin Dabilpuram (1):
  octeontx2-af: replace cpt slot with lf id on reg write

Satheesh Paul (1):
  octeontx2-af: fix issue with IPv4 match for RSS

Srujana Challa (2):
  octeontx2-af: reduce cpt flt interrupt vectors for cn10kb
  octeontx2-af: fix a issue with cpt_lf_alloc mailbox

 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  7 +-
 .../net/ethernet/marvell/octeontx2/af/npc.h   |  8 +-
 .../ethernet/marvell/octeontx2/af/rvu_cpt.c   | 78 +++++++++++++++----
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   |  7 +-
 .../marvell/octeontx2/af/rvu_struct.h         |  5 +-
 5 files changed, 79 insertions(+), 26 deletions(-)

-- 
2.25.1

[PATCH net,1/6] octeontx2-af: replace cpt slot with lf id on reg write

[Srujana Challa]

From: Nithin Dabilpuram <ndabilpuram@marvell.com>

Replace cpt slot id with lf id on reg read/write as
CPTPF/VF driver would send slot number instead of lf id
in the reg offset.

Fixes: ae454086e3c2 ("octeontx2-af: add mailbox interface for CPT")
Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
index f047185f38e0..98440a0241a2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
@@ -663,6 +663,8 @@ static bool is_valid_offset(struct rvu *rvu, struct cpt_rd_wr_reg_msg *req)
 		if (lf < 0)
 			return false;
 
+		req->reg_offset &= 0xFF000;
+		req->reg_offset += lf << 3;
 		return true;
 	} else if (!(req->hdr.pcifunc & RVU_PFVF_FUNC_MASK)) {
 		/* Registers that can be accessed from PF */
@@ -707,12 +709,13 @@ int rvu_mbox_handler_cpt_rd_wr_register(struct rvu *rvu,
 	    !is_cpt_vf(rvu, req->hdr.pcifunc))
 		return CPT_AF_ERR_ACCESS_DENIED;
 
+	if (!is_valid_offset(rvu, req))
+		return CPT_AF_ERR_ACCESS_DENIED;
+
 	rsp->reg_offset = req->reg_offset;
 	rsp->ret_val = req->ret_val;
 	rsp->is_write = req->is_write;
 
-	if (!is_valid_offset(rvu, req))
-		return CPT_AF_ERR_ACCESS_DENIED;
 
 	if (req->is_write)
 		rvu_write64(rvu, blkaddr, req->reg_offset, req->val);
-- 
2.25.1

[PATCH net,2/6] octeontx2-af: reduce cpt flt interrupt vectors for cn10kb

[Srujana Challa]

On new silicon(cn10kb), the number of FLT interrupt vectors has
been reduced. Hence, this patch modifies the code to make
it work for both cn10ka and cn10kb.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  5 +-
 .../ethernet/marvell/octeontx2/af/rvu_cpt.c   | 73 ++++++++++++++++---
 .../marvell/octeontx2/af/rvu_struct.h         |  5 +-
 3 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 4a77f6fe2622..41b46724cb3d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -1848,8 +1848,9 @@ struct cpt_flt_eng_info_req {
 
 struct cpt_flt_eng_info_rsp {
 	struct mbox_msghdr hdr;
-	u64 flt_eng_map[CPT_10K_AF_INT_VEC_RVU];
-	u64 rcvrd_eng_map[CPT_10K_AF_INT_VEC_RVU];
+#define CPT_AF_MAX_FLT_INT_VECS 3
+	u64 flt_eng_map[CPT_AF_MAX_FLT_INT_VECS];
+	u64 rcvrd_eng_map[CPT_AF_MAX_FLT_INT_VECS];
 	u64 rsvd;
 };
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
index 98440a0241a2..38363ea56c6c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
@@ -37,6 +37,38 @@
 	(_rsp)->free_sts_##etype = free_sts;                        \
 })
 
+#define MAX_AE  GENMASK_ULL(47, 32)
+#define MAX_IE  GENMASK_ULL(31, 16)
+#define MAX_SE  GENMASK_ULL(15, 0)
+static u32 cpt_max_engines_get(struct rvu *rvu)
+{
+	u16 max_ses, max_ies, max_aes;
+	u64 reg;
+
+	reg = rvu_read64(rvu, BLKADDR_CPT0, CPT_AF_CONSTANTS1);
+	max_ses = FIELD_GET(MAX_SE, reg);
+	max_ies = FIELD_GET(MAX_IE, reg);
+	max_aes = FIELD_GET(MAX_AE, reg);
+
+	return max_ses + max_ies + max_aes;
+}
+
+/* Number of flt interrupt vectors are depends on number of engines that
+ * the chip has. Each flt vector represents 64 engines.
+ */
+static int cpt_10k_flt_nvecs_get(struct rvu *rvu)
+{
+	u32 max_engs;
+	int flt_vecs;
+
+	max_engs = cpt_max_engines_get(rvu);
+
+	flt_vecs = (max_engs / 64);
+	flt_vecs += (max_engs % 64) ? 1 : 0;
+
+	return flt_vecs;
+}
+
 static irqreturn_t cpt_af_flt_intr_handler(int vec, void *ptr)
 {
 	struct rvu_block *block = ptr;
@@ -150,17 +182,25 @@ static void cpt_10k_unregister_interrupts(struct rvu_block *block, int off)
 {
 	struct rvu *rvu = block->rvu;
 	int blkaddr = block->addr;
+	u32 max_engs;
+	u8 nr;
 	int i;
 
+	max_engs = cpt_max_engines_get(rvu);
+
 	/* Disable all CPT AF interrupts */
-	rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1C(0), ~0ULL);
-	rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1C(1), ~0ULL);
-	rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1C(2), 0xFFFF);
+	for (i = CPT_10K_AF_INT_VEC_FLT0; i < cpt_10k_flt_nvecs_get(rvu); i++) {
+		nr = (max_engs > 64) ? 64 : max_engs;
+		max_engs -= nr;
+		rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1C(i),
+			    INTR_MASK(nr));
+	}
 
 	rvu_write64(rvu, blkaddr, CPT_AF_RVU_INT_ENA_W1C, 0x1);
 	rvu_write64(rvu, blkaddr, CPT_AF_RAS_INT_ENA_W1C, 0x1);
 
-	for (i = 0; i < CPT_10K_AF_INT_VEC_CNT; i++)
+	/* CPT AF interrupt vectors are flt_int, rvu_int and ras_int. */
+	for (i = 0; i < cpt_10k_flt_nvecs_get(rvu) + 2; i++)
 		if (rvu->irq_allocated[off + i]) {
 			free_irq(pci_irq_vector(rvu->pdev, off + i), block);
 			rvu->irq_allocated[off + i] = false;
@@ -206,12 +246,17 @@ void rvu_cpt_unregister_interrupts(struct rvu *rvu)
 
 static int cpt_10k_register_interrupts(struct rvu_block *block, int off)
 {
+	int rvu_intr_vec, ras_intr_vec;
 	struct rvu *rvu = block->rvu;
 	int blkaddr = block->addr;
 	irq_handler_t flt_fn;
+	u32 max_engs;
 	int i, ret;
+	u8 nr;
+
+	max_engs = cpt_max_engines_get(rvu);
 
-	for (i = CPT_10K_AF_INT_VEC_FLT0; i < CPT_10K_AF_INT_VEC_RVU; i++) {
+	for (i = CPT_10K_AF_INT_VEC_FLT0; i < cpt_10k_flt_nvecs_get(rvu); i++) {
 		sprintf(&rvu->irq_name[(off + i) * NAME_SIZE], "CPTAF FLT%d", i);
 
 		switch (i) {
@@ -229,20 +274,24 @@ static int cpt_10k_register_interrupts(struct rvu_block *block, int off)
 						    flt_fn, &rvu->irq_name[(off + i) * NAME_SIZE]);
 		if (ret)
 			goto err;
-		if (i == CPT_10K_AF_INT_VEC_FLT2)
-			rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1S(i), 0xFFFF);
-		else
-			rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1S(i), ~0ULL);
+
+		nr = (max_engs > 64) ? 64 : max_engs;
+		max_engs -= nr;
+		rvu_write64(rvu, blkaddr, CPT_AF_FLTX_INT_ENA_W1S(i),
+			    INTR_MASK(nr));
 	}
 
-	ret = rvu_cpt_do_register_interrupt(block, off + CPT_10K_AF_INT_VEC_RVU,
+	rvu_intr_vec = cpt_10k_flt_nvecs_get(rvu);
+	ras_intr_vec = rvu_intr_vec + 1;
+
+	ret = rvu_cpt_do_register_interrupt(block, off + rvu_intr_vec,
 					    rvu_cpt_af_rvu_intr_handler,
 					    "CPTAF RVU");
 	if (ret)
 		goto err;
 	rvu_write64(rvu, blkaddr, CPT_AF_RVU_INT_ENA_W1S, 0x1);
 
-	ret = rvu_cpt_do_register_interrupt(block, off + CPT_10K_AF_INT_VEC_RAS,
+	ret = rvu_cpt_do_register_interrupt(block, off + ras_intr_vec,
 					    rvu_cpt_af_ras_intr_handler,
 					    "CPTAF RAS");
 	if (ret)
@@ -922,7 +971,7 @@ int rvu_mbox_handler_cpt_flt_eng_info(struct rvu *rvu, struct cpt_flt_eng_info_r
 		return blkaddr;
 
 	block = &rvu->hw->block[blkaddr];
-	for (vec = 0; vec < CPT_10K_AF_INT_VEC_RVU; vec++) {
+	for (vec = 0; vec < cpt_10k_flt_nvecs_get(block->rvu); vec++) {
 		spin_lock_irqsave(&rvu->cpt_intr_lock, flags);
 		rsp->flt_eng_map[vec] = block->cpt_flt_eng_map[vec];
 		rsp->rcvrd_eng_map[vec] = block->cpt_rcvrd_eng_map[vec];
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
index 5ef406c7e8a4..120776063e86 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
@@ -71,13 +71,10 @@ enum cpt_af_int_vec_e {
 	CPT_AF_INT_VEC_CNT	= 0x4,
 };
 
-enum cpt_10k_af_int_vec_e {
+enum cpt_cn10k_flt_int_vec_e {
 	CPT_10K_AF_INT_VEC_FLT0	= 0x0,
 	CPT_10K_AF_INT_VEC_FLT1	= 0x1,
 	CPT_10K_AF_INT_VEC_FLT2	= 0x2,
-	CPT_10K_AF_INT_VEC_RVU	= 0x3,
-	CPT_10K_AF_INT_VEC_RAS	= 0x4,
-	CPT_10K_AF_INT_VEC_CNT	= 0x5,
 };
 
 /* NPA Admin function Interrupt Vector Enumeration */
-- 
2.25.1

[PATCH net,3/6] Fix CPT_LF_ALLOC mailbox error due to incompatible mailbox message format

[Srujana Challa]

This patch addresses the issue introduced by commit de2854c87c64
("octeontx2-af: Mailbox changes for 98xx CPT block"). Specifically, it
corrects the `blkaddr` field type from `int` to `u8`.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 41b46724cb3d..799aa54103a2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -1745,7 +1745,7 @@ struct cpt_lf_alloc_req_msg {
 	u16 nix_pf_func;
 	u16 sso_pf_func;
 	u16 eng_grpmsk;
-	int blkaddr;
+	u8 blkaddr;
 	u8 ctx_ilen_valid : 1;
 	u8 ctx_ilen : 7;
 };
-- 
2.25.1

[PATCH net,4/6] octeontx2-af: Fix issue with IPv6 ext match for RSS

[Srujana Challa]

From: Kiran Kumar K <kirankumark@marvell.com>

While performing RSS based on IPv6, extension ltype
is not being considered. This will be problem for
fragmented packets or packets with extension header.
Adding changes to match IPv6 ext header along with IPv6
ltype.

Fixes: 41a7aa7b800d ("octeontx2-af: NIX Rx flowkey configuration for RSS")
Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 00af8888e329..bf5c9cc3df87 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -3990,7 +3990,7 @@ static int set_flowkey_fields(struct nix_rx_flowkey_alg *alg, u32 flow_cfg)
 					field->bytesm1 = 15; /* DIP,16 bytes */
 				}
 			}
-			field->ltype_mask = 0xF; /* Match only IPv6 */
+			field->ltype_mask = 0xE; /* Match IPv6 and IPv6_ext */
 			break;
 		case NIX_FLOW_KEY_TYPE_TCP:
 		case NIX_FLOW_KEY_TYPE_UDP:
-- 
2.25.1

[PATCH net,5/6] octeontx2-af: fix issue with IPv4 match for RSS

[Srujana Challa]

From: Satheesh Paul <psatheesh@marvell.com>

While performing RSS based on IPv4, packets with
IPv4 options are not being considered. Adding changes
to match both plain IPv4 and IPv4 with option header.

Fixes: 41a7aa7b800d ("octeontx2-af: NIX Rx flowkey configuration for RSS")
Signed-off-by: Satheesh Paul <psatheesh@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index bf5c9cc3df87..da090d8b9046 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -3933,7 +3933,7 @@ static int set_flowkey_fields(struct nix_rx_flowkey_alg *alg, u32 flow_cfg)
 			field->hdr_offset = 9; /* offset */
 			field->bytesm1 = 0; /* 1 byte */
 			field->ltype_match = NPC_LT_LC_IP;
-			field->ltype_mask = 0xF;
+			field->ltype_mask = 0xE;
 			break;
 		case NIX_FLOW_KEY_TYPE_IPV4:
 		case NIX_FLOW_KEY_TYPE_INNR_IPV4:
@@ -3961,7 +3961,10 @@ static int set_flowkey_fields(struct nix_rx_flowkey_alg *alg, u32 flow_cfg)
 				}
 			}
 
-			field->ltype_mask = 0xF; /* Match only IPv4 */
+			/* Match only IPv4; both NPC_LT_LC_IP and
+			 * NPC_LT_LC_IP_OPT
+			 */
+			field->ltype_mask = 0xE;
 			keyoff_marker = false;
 			break;
 		case NIX_FLOW_KEY_TYPE_IPV6:
-- 
2.25.1

[PATCH net,6/6] octeontx2-af: fix detection of IP layer

[Srujana Challa]

From: Michal Mazur <mmazur2@marvell.com>

Checksum and length checks are not enabled for IPv4 header with
options and IPv6 with extension headers.
To fix this a change in enum npc_kpu_lc_ltype is required which will
allow adjustment of LTYPE_MASK to detect all types of IP headers.

Fixes: 21e6699e5cd6 ("octeontx2-af: Add NPC KPU profile")
Signed-off-by: Michal Mazur <mmazur2@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/npc.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
index d883157393ea..6c3aca6f278d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/npc.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
@@ -63,8 +63,13 @@ enum npc_kpu_lb_ltype {
 	NPC_LT_LB_CUSTOM1 = 0xF,
 };
 
+/* Don't modify ltypes up to IP6_EXT, otherwise length and checksum of IP
+ * headers may not be checked correctly. IPv4 ltypes and IPv6 ltypes must
+ * differ only at bit 0 so mask 0xE can be used to detect extended headers.
+ */
 enum npc_kpu_lc_ltype {
-	NPC_LT_LC_IP = 1,
+	NPC_LT_LC_PTP = 1,
+	NPC_LT_LC_IP,
 	NPC_LT_LC_IP_OPT,
 	NPC_LT_LC_IP6,
 	NPC_LT_LC_IP6_EXT,
@@ -72,7 +77,6 @@ enum npc_kpu_lc_ltype {
 	NPC_LT_LC_RARP,
 	NPC_LT_LC_MPLS,
 	NPC_LT_LC_NSH,
-	NPC_LT_LC_PTP,
 	NPC_LT_LC_FCOE,
 	NPC_LT_LC_NGIO,
 	NPC_LT_LC_CUSTOM0 = 0xE,
-- 
2.25.1

Re: [PATCH net,4/6] octeontx2-af: Fix issue with IPv6 ext match for RSS

[Kalesh Anakkur Purayil]

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

On Mon, Jul 1, 2024 at 2:38 PM Srujana Challa <schalla@marvell.com> wrote:
>
> From: Kiran Kumar K <kirankumark@marvell.com>
>
> While performing RSS based on IPv6, extension ltype
> is not being considered. This will be problem for
> fragmented packets or packets with extension header.
> Adding changes to match IPv6 ext header along with IPv6
> ltype.
>
> Fixes: 41a7aa7b800d ("octeontx2-af: NIX Rx flowkey configuration for RSS")
> Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index 00af8888e329..bf5c9cc3df87 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> @@ -3990,7 +3990,7 @@ static int set_flowkey_fields(struct nix_rx_flowkey_alg *alg, u32 flow_cfg)
>                                         field->bytesm1 = 15; /* DIP,16 bytes */
>                                 }
>                         }
> -                       field->ltype_mask = 0xF; /* Match only IPv6 */
> +                       field->ltype_mask = 0xE; /* Match IPv6 and IPv6_ext */
[Kalesh] I would suggest you to have a macro + short comment for the
magic number. This magic number is used at multiple places(subsequent
patches in this series).

>                         break;
>                 case NIX_FLOW_KEY_TYPE_TCP:
>                 case NIX_FLOW_KEY_TYPE_UDP:
> --
> 2.25.1
>
>


-- 
Regards,
Kalesh A P

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

Re: [PATCH net,3/6] Fix CPT_LF_ALLOC mailbox error due to incompatible mailbox message format

[Kalesh Anakkur Purayil]

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

On Mon, Jul 1, 2024 at 2:39 PM Srujana Challa <schalla@marvell.com> wrote:
>
> This patch addresses the issue introduced by commit de2854c87c64
> ("octeontx2-af: Mailbox changes for 98xx CPT block"). Specifically, it
> corrects the `blkaddr` field type from `int` to `u8`.
>
> Signed-off-by: Srujana Challa <schalla@marvell.com>
[Kalesh] Missing Fixes tag?
> ---
>  drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 41b46724cb3d..799aa54103a2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -1745,7 +1745,7 @@ struct cpt_lf_alloc_req_msg {
>         u16 nix_pf_func;
>         u16 sso_pf_func;
>         u16 eng_grpmsk;
> -       int blkaddr;
> +       u8 blkaddr;
>         u8 ctx_ilen_valid : 1;
>         u8 ctx_ilen : 7;
>  };
> --
> 2.25.1
>
>


-- 
Regards,
Kalesh A P

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

Re: [PATCH net,2/6] octeontx2-af: reduce cpt flt interrupt vectors for cn10kb

[Simon Horman]

On Mon, Jul 01, 2024 at 02:37:42PM +0530, Srujana Challa wrote:
> On new silicon(cn10kb), the number of FLT interrupt vectors has
> been reduced. Hence, this patch modifies the code to make
> it work for both cn10ka and cn10kb.
> 

I am tempted to think this is more about enabling new hardware
than fixing a bug. But I do also see how one might argue otherwise.

In any case, if this is a fix then a fixes tag should go here.
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  5 +-
>  .../ethernet/marvell/octeontx2/af/rvu_cpt.c   | 73 ++++++++++++++++---
>  .../marvell/octeontx2/af/rvu_struct.h         |  5 +-
>  3 files changed, 65 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 4a77f6fe2622..41b46724cb3d 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -1848,8 +1848,9 @@ struct cpt_flt_eng_info_req {
>  
>  struct cpt_flt_eng_info_rsp {
>  	struct mbox_msghdr hdr;
> -	u64 flt_eng_map[CPT_10K_AF_INT_VEC_RVU];
> -	u64 rcvrd_eng_map[CPT_10K_AF_INT_VEC_RVU];
> +#define CPT_AF_MAX_FLT_INT_VECS 3
> +	u64 flt_eng_map[CPT_AF_MAX_FLT_INT_VECS];
> +	u64 rcvrd_eng_map[CPT_AF_MAX_FLT_INT_VECS];
>  	u64 rsvd;
>  };
>  
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> index 98440a0241a2..38363ea56c6c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> @@ -37,6 +37,38 @@
>  	(_rsp)->free_sts_##etype = free_sts;                        \
>  })
>  
> +#define MAX_AE  GENMASK_ULL(47, 32)
> +#define MAX_IE  GENMASK_ULL(31, 16)
> +#define MAX_SE  GENMASK_ULL(15, 0)
> +static u32 cpt_max_engines_get(struct rvu *rvu)
> +{
> +	u16 max_ses, max_ies, max_aes;
> +	u64 reg;
> +
> +	reg = rvu_read64(rvu, BLKADDR_CPT0, CPT_AF_CONSTANTS1);
> +	max_ses = FIELD_GET(MAX_SE, reg);
> +	max_ies = FIELD_GET(MAX_IE, reg);
> +	max_aes = FIELD_GET(MAX_AE, reg);
> +
> +	return max_ses + max_ies + max_aes;
> +}
> +
> +/* Number of flt interrupt vectors are depends on number of engines that
> + * the chip has. Each flt vector represents 64 engines.
> + */
> +static int cpt_10k_flt_nvecs_get(struct rvu *rvu)
> +{
> +	u32 max_engs;
> +	int flt_vecs;
> +
> +	max_engs = cpt_max_engines_get(rvu);
> +
> +	flt_vecs = (max_engs / 64);
> +	flt_vecs += (max_engs % 64) ? 1 : 0;
> +
> +	return flt_vecs;
> +}
> +

I think the callers of this function assume it will never return a value
greater than 3. Perhaps it would be worth enforcing that, or WARNing if it
not so.  I'm thinking of a case a fw/hw revision comes along and this
assumption no longer holds.

>  static irqreturn_t cpt_af_flt_intr_handler(int vec, void *ptr)
>  {
>  	struct rvu_block *block = ptr;

...

Re: [PATCH net,1/6] octeontx2-af: replace cpt slot with lf id on reg write

[Michal Kubiak]

On Mon, Jul 01, 2024 at 02:37:41PM +0530, Srujana Challa wrote:
> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> 
> Replace cpt slot id with lf id on reg read/write as
> CPTPF/VF driver would send slot number instead of lf id
> in the reg offset.
> 
> Fixes: ae454086e3c2 ("octeontx2-af: add mailbox interface for CPT")
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> index f047185f38e0..98440a0241a2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> @@ -663,6 +663,8 @@ static bool is_valid_offset(struct rvu *rvu, struct cpt_rd_wr_reg_msg *req)
>  		if (lf < 0)
>  			return false;
>  
> +		req->reg_offset &= 0xFF000;
> +		req->reg_offset += lf << 3;

I think it's not great to modify an input parameter from the function
named like "is_valid_offset()". From the function like that I would
rather expect doing just a simple check if the parameter is correct.
It seems calling that function from a different context can be risky
now.

>  		return true;
>  	} else if (!(req->hdr.pcifunc & RVU_PFVF_FUNC_MASK)) {
>  		/* Registers that can be accessed from PF */
> @@ -707,12 +709,13 @@ int rvu_mbox_handler_cpt_rd_wr_register(struct rvu *rvu,
>  	    !is_cpt_vf(rvu, req->hdr.pcifunc))
>  		return CPT_AF_ERR_ACCESS_DENIED;
>  
> +	if (!is_valid_offset(rvu, req))
> +		return CPT_AF_ERR_ACCESS_DENIED;
> +
>  	rsp->reg_offset = req->reg_offset;
>  	rsp->ret_val = req->ret_val;
>  	rsp->is_write = req->is_write;
>  
> -	if (!is_valid_offset(rvu, req))
> -		return CPT_AF_ERR_ACCESS_DENIED;

Is moving that call also a necessary part of the fix? Or is it just an extra
improvement?
Maybe it's worth mentioning in the commit message?

>  
>  	if (req->is_write)
>  		rvu_write64(rvu, blkaddr, req->reg_offset, req->val);
> -- 
> 2.25.1
> 
> 


Thanks,
Michal

RE: [EXTERNAL] Re: [PATCH net,1/6] octeontx2-af: replace cpt slot with lf id on reg write

[Srujana Challa]

> > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >
> > Replace cpt slot id with lf id on reg read/write as CPTPF/VF driver
> > would send slot number instead of lf id in the reg offset.
> >
> > Fixes: ae454086e3c2 ("octeontx2-af: add mailbox interface for CPT")
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> >  drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > index f047185f38e0..98440a0241a2 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > @@ -663,6 +663,8 @@ static bool is_valid_offset(struct rvu *rvu, struct
> cpt_rd_wr_reg_msg *req)
> >  		if (lf < 0)
> >  			return false;
> >
> > +		req->reg_offset &= 0xFF000;
> > +		req->reg_offset += lf << 3;
> 
> I think it's not great to modify an input parameter from the function named
> like "is_valid_offset()". From the function like that I would rather expect doing
> just a simple check if the parameter is correct.
> It seems calling that function from a different context can be risky now.
I’ll make the necessary changes to the code to ensure that we avoid modifying
the input parameter within the is_valid_offset() function in next version.

> 
> >  		return true;
> >  	} else if (!(req->hdr.pcifunc & RVU_PFVF_FUNC_MASK)) {
> >  		/* Registers that can be accessed from PF */ @@ -707,12
> +709,13 @@
> > int rvu_mbox_handler_cpt_rd_wr_register(struct rvu *rvu,
> >  	    !is_cpt_vf(rvu, req->hdr.pcifunc))
> >  		return CPT_AF_ERR_ACCESS_DENIED;
> >
> > +	if (!is_valid_offset(rvu, req))
> > +		return CPT_AF_ERR_ACCESS_DENIED;
> > +
> >  	rsp->reg_offset = req->reg_offset;
> >  	rsp->ret_val = req->ret_val;
> >  	rsp->is_write = req->is_write;
> >
> > -	if (!is_valid_offset(rvu, req))
> > -		return CPT_AF_ERR_ACCESS_DENIED;
> 
> Is moving that call also a necessary part of the fix? Or is it just an extra
> improvement?
> Maybe it's worth mentioning in the commit message?
is_valid_offset() call is moved to ensure that rsp->reg_offset is correctly updated
with the modified req->reg_offset.

Thanks for the feedback.
> 
> >
> >  	if (req->is_write)
> >  		rvu_write64(rvu, blkaddr, req->reg_offset, req->val);
> > --
> > 2.25.1
> >
> >
> 
> 
> Thanks,
> Michal

RE: [EXTERNAL] Re: [PATCH net,2/6] octeontx2-af: reduce cpt flt interrupt vectors for cn10kb

[Srujana Challa]

> On Mon, Jul 01, 2024 at 02:37:42PM +0530, Srujana Challa wrote:
> > On new silicon(cn10kb), the number of FLT interrupt vectors has been
> > reduced. Hence, this patch modifies the code to make it work for both
> > cn10ka and cn10kb.
> >
> 
> I am tempted to think this is more about enabling new hardware than fixing a
> bug. But I do also see how one might argue otherwise.
> 
> In any case, if this is a fix then a fixes tag should go here.
I’ll exclude the patch from this series and submit it to net-next.

> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > ---
> >  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  5 +-
> >  .../ethernet/marvell/octeontx2/af/rvu_cpt.c   | 73 ++++++++++++++++---
> >  .../marvell/octeontx2/af/rvu_struct.h         |  5 +-
> >  3 files changed, 65 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > index 4a77f6fe2622..41b46724cb3d 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> > @@ -1848,8 +1848,9 @@ struct cpt_flt_eng_info_req {
> >
> >  struct cpt_flt_eng_info_rsp {
> >  	struct mbox_msghdr hdr;
> > -	u64 flt_eng_map[CPT_10K_AF_INT_VEC_RVU];
> > -	u64 rcvrd_eng_map[CPT_10K_AF_INT_VEC_RVU];
> > +#define CPT_AF_MAX_FLT_INT_VECS 3
> > +	u64 flt_eng_map[CPT_AF_MAX_FLT_INT_VECS];
> > +	u64 rcvrd_eng_map[CPT_AF_MAX_FLT_INT_VECS];
> >  	u64 rsvd;
> >  };
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > index 98440a0241a2..38363ea56c6c 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> > @@ -37,6 +37,38 @@
> >  	(_rsp)->free_sts_##etype = free_sts;                        \
> >  })
> >
> > +#define MAX_AE  GENMASK_ULL(47, 32)
> > +#define MAX_IE  GENMASK_ULL(31, 16)
> > +#define MAX_SE  GENMASK_ULL(15, 0)
> > +static u32 cpt_max_engines_get(struct rvu *rvu) {
> > +	u16 max_ses, max_ies, max_aes;
> > +	u64 reg;
> > +
> > +	reg = rvu_read64(rvu, BLKADDR_CPT0, CPT_AF_CONSTANTS1);
> > +	max_ses = FIELD_GET(MAX_SE, reg);
> > +	max_ies = FIELD_GET(MAX_IE, reg);
> > +	max_aes = FIELD_GET(MAX_AE, reg);
> > +
> > +	return max_ses + max_ies + max_aes;
> > +}
> > +
> > +/* Number of flt interrupt vectors are depends on number of engines
> > +that
> > + * the chip has. Each flt vector represents 64 engines.
> > + */
> > +static int cpt_10k_flt_nvecs_get(struct rvu *rvu) {
> > +	u32 max_engs;
> > +	int flt_vecs;
> > +
> > +	max_engs = cpt_max_engines_get(rvu);
> > +
> > +	flt_vecs = (max_engs / 64);
> > +	flt_vecs += (max_engs % 64) ? 1 : 0;
> > +
> > +	return flt_vecs;
> > +}
> > +
> 
> I think the callers of this function assume it will never return a value greater
> than 3. Perhaps it would be worth enforcing that, or WARNing if it not so.  I'm
> thinking of a case a fw/hw revision comes along and this assumption no
> longer holds.
> 
> >  static irqreturn_t cpt_af_flt_intr_handler(int vec, void *ptr)  {
> >  	struct rvu_block *block = ptr;
> 
> ...