All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state
@ 2026-07-13 12:19 Ratheesh Kannoth
  2026-07-20 16:09 ` Simon Horman
  0 siblings, 1 reply; 2+ messages in thread
From: Ratheesh Kannoth @ 2026-07-13 12:19 UTC (permalink / raw)
  To: davem, gakula, linux-kernel, netdev, sgoutham
  Cc: andrew+netdev, edumazet, kuba, pabeni, Hariprasad Kelam,
	Ratheesh Kannoth

From: Hariprasad Kelam <hkelam@marvell.com>

PF and VF NIX LFs that share a CGX LMAC reuse the same hardware PKIND
programming. When HiGig2 or EDSA parsing is enabled, a VF NIX LF alloc must
not reset the LMAC RX PKIND or default TX parse config over the PF setup.

Add cgx_get_pkind() and rvu_cgx_is_pkind_config_permitted() so VFs skip
cgx_set_pkind(), rvu_npc_set_pkind(), and NIX_AF_LFX_TX_PARSE_CFG updates
when the LMAC is using NPC_RX_HIGIG_PKIND or NPC_RX_EDSA_PKIND.

Fixes: 94d942c5fb97 ("octeontx2-af: Config pkind for CGX mapped PFs")
Cc: Geetha sowjanya <gakula@marvell.com>
Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

---
v2 -> v3: Addressed simon comments
	https://lore.kernel.org/netdev/20260709122648.1552103-2-horms@kernel.org/
v1 -> v2: Addressed simon comments
	https://lore.kernel.org/netdev/20260619041002.1773822-1-rkannoth@marvell.com/
---
 .../net/ethernet/marvell/octeontx2/af/cgx.c   | 12 +++++++
 .../net/ethernet/marvell/octeontx2/af/cgx.h   |  1 +
 .../net/ethernet/marvell/octeontx2/af/rvu.h   |  1 +
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   | 32 +++++++++++++++++++
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   | 18 ++++++++---
 .../ethernet/marvell/octeontx2/af/rvu_npc.c   | 11 +++++--
 6 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index 2e94d5105016..f5fd6138c352 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -518,6 +518,18 @@ int cgx_set_pkind(void *cgxd, u8 lmac_id, int pkind)
 	return 0;
 }
 
+int cgx_get_pkind(void *cgxd, u8 lmac_id, int *pkind)
+{
+	struct cgx *cgx = cgxd;
+
+	if (!is_lmac_valid(cgx, lmac_id))
+		return -ENODEV;
+
+	*pkind = cgx_read(cgx, lmac_id, cgx->mac_ops->rxid_map_offset);
+	*pkind = *pkind & 0x3F;
+	return 0;
+}
+
 static u8 cgx_get_lmac_type(void *cgxd, int lmac_id)
 {
 	struct cgx *cgx = cgxd;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
index 92ccf343dfe0..8411a75dd723 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.h
@@ -141,6 +141,7 @@ int cgx_get_cgxid(void *cgxd);
 int cgx_get_lmac_cnt(void *cgxd);
 void *cgx_get_pdata(int cgx_id);
 int cgx_set_pkind(void *cgxd, u8 lmac_id, int pkind);
+int cgx_get_pkind(void *cgxd, u8 lmac_id, int *pkind);
 int cgx_lmac_evh_register(struct cgx_event_cb *cb, void *cgxd, int lmac_id);
 int cgx_lmac_evh_unregister(void *cgxd, int lmac_id);
 int cgx_get_tx_stats(void *cgxd, int lmac_id, int idx, u64 *tx_stat);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index 7f3505ae6860..bb671e2150aa 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -1115,6 +1115,7 @@ void npc_read_mcam_entry(struct rvu *rvu, struct npc_mcam *mcam,
 			 u8 *intf, u8 *ena);
 int npc_config_cntr_default_entries(struct rvu *rvu, bool enable);
 bool is_cgx_config_permitted(struct rvu *rvu, u16 pcifunc);
+bool rvu_cgx_is_pkind_config_permitted(struct rvu *rvu, u16 pcifunc);
 bool is_mac_feature_supported(struct rvu *rvu, int pf, int feature);
 u32  rvu_cgx_get_fifolen(struct rvu *rvu);
 void *rvu_first_cgx_pdata(struct rvu *rvu);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 4ff3935ed3fe..2be1da3476ac 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -1355,3 +1355,35 @@ void rvu_mac_reset(struct rvu *rvu, u16 pcifunc)
 	if (mac_ops->mac_reset(cgxd, lmac, !is_vf(pcifunc)))
 		dev_err(rvu->dev, "Failed to reset MAC\n");
 }
+
+/* Do not allow CGX-mapped VFs to overwrite PKIND when special parse kinds
+ * (HiGig, EDSA, etc.) are in use on the shared LMAC.
+ */
+bool rvu_cgx_is_pkind_config_permitted(struct rvu *rvu, u16 pcifunc)
+{
+	int pf, err, rxpkind;
+	u8 cgx_id, lmac_id;
+	void *cgxd;
+
+	pf = rvu_get_pf(rvu->pdev, pcifunc);
+
+	if (!(pcifunc & RVU_PFVF_FUNC_MASK))
+		return true;
+
+	if (!is_pf_cgxmapped(rvu, pf))
+		return true;
+
+	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
+	cgxd = rvu_cgx_pdata(cgx_id, rvu);
+	err = cgx_get_pkind(cgxd, lmac_id, &rxpkind);
+	if (err)
+		return false;
+
+	switch (rxpkind) {
+	case NPC_RX_HIGIG_PKIND:
+	case NPC_RX_EDSA_PKIND:
+		return false;
+	default:
+		return true;
+	}
+}
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 6a0ce2665031..040eb4c9b8d1 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -338,6 +338,7 @@ static int nix_interface_init(struct rvu *rvu, u16 pcifunc, int type, int nixlf,
 	struct sdp_node_info *sdp_info;
 	int pkind, pf, vf, lbkid, vfid;
 	u8 cgx_id, lmac_id;
+	struct cgx *cgxd;
 	bool from_vf;
 	int err;
 
@@ -363,8 +364,15 @@ static int nix_interface_init(struct rvu *rvu, u16 pcifunc, int type, int nixlf,
 		pfvf->tx_chan_cnt = 1;
 		rsp->tx_link = cgx_id * hw->lmac_per_cgx + lmac_id;
 
-		cgx_set_pkind(rvu_cgx_pdata(cgx_id, rvu), lmac_id, pkind);
-		rvu_npc_set_pkind(rvu, pkind, pfvf);
+		cgxd = rvu_cgx_pdata(cgx_id, rvu);
+
+		mutex_lock(&cgxd->lock);
+		if (rvu_cgx_is_pkind_config_permitted(rvu, pcifunc)) {
+			cgx_set_pkind(rvu_cgx_pdata(cgx_id, rvu), lmac_id,
+				      pkind);
+			rvu_npc_set_pkind(rvu, pkind, pfvf);
+		}
+		mutex_unlock(&cgxd->lock);
 		break;
 	case NIX_INTF_TYPE_LBK:
 		vf = (pcifunc & RVU_PFVF_FUNC_MASK) - 1;
@@ -1685,8 +1693,10 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
 	rvu_write64(rvu, blkaddr, NIX_AF_LFX_RX_CFG(nixlf), req->rx_cfg);
 
 	/* Configure pkind for TX parse config */
-	cfg = NPC_TX_DEF_PKIND;
-	rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf), cfg);
+	if (rvu_cgx_is_pkind_config_permitted(rvu, pcifunc)) {
+		cfg = NPC_TX_DEF_PKIND;
+		rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf), cfg);
+	}
 
 	if (is_rep_dev(rvu, pcifunc)) {
 		pfvf->tx_chan_base = RVU_SWITCH_LBK_CHAN;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
index c7bc0b3a29b9..1297c6681a01 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
@@ -17,6 +17,7 @@
 #include "npc_profile.h"
 #include "rvu_npc_hash.h"
 #include "cn20k/npc.h"
+#include "lmac_common.h"
 #include "rvu_npc.h"
 #include "cn20k/reg.h"
 
@@ -4204,6 +4205,7 @@ int rvu_npc_set_parse_mode(struct rvu *rvu, u16 pcifunc, u64 mode, u8 dir,
 	int pf = rvu_get_pf(rvu->pdev, pcifunc);
 	u64 rxpkind, txpkind;
 	u8 cgx_id, lmac_id;
+	struct cgx *cgxd;
 
 	/* use default pkind to disable edsa/higig */
 	rxpkind = rvu_npc_get_pkind(rvu, pf);
@@ -4228,9 +4230,11 @@ int rvu_npc_set_parse_mode(struct rvu *rvu, u16 pcifunc, u64 mode, u8 dir,
 		if (!is_cgx_config_permitted(rvu, pcifunc))
 			return 0;
 		rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
+		cgxd = rvu_cgx_pdata(cgx_id, rvu);
 
-		rc = cgx_set_pkind(rvu_cgx_pdata(cgx_id, rvu), lmac_id,
-				   rxpkind);
+		mutex_lock(&cgxd->lock);
+		rc = cgx_set_pkind(cgxd, lmac_id, rxpkind);
+		mutex_unlock(&cgxd->lock);
 		if (rc)
 			return rc;
 	}
@@ -4241,6 +4245,9 @@ int rvu_npc_set_parse_mode(struct rvu *rvu, u16 pcifunc, u64 mode, u8 dir,
 		if (rc)
 			return rc;
 
+		if (!rvu_cgx_is_pkind_config_permitted(rvu, pcifunc))
+			return -EINVAL;
+
 		rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf),
 			    txpkind);
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v3 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state
  2026-07-13 12:19 [PATCH v3 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state Ratheesh Kannoth
@ 2026-07-20 16:09 ` Simon Horman
  0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2026-07-20 16:09 UTC (permalink / raw)
  To: rkannoth
  Cc: Simon Horman, davem, gakula, linux-kernel, netdev, sgoutham,
	andrew+netdev, edumazet, kuba, pabeni, hkelam

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260713121902.3938875-1-rkannoth@marvell.com?part=1
---
octeontx2-af: Block VFs from clobbering special CGX PKIND state

This commit prevents CGX-mapped VFs from resetting the shared LMAC RX
PKIND and default TX parse configuration when the LMAC is running with
HiGig2 or EDSA parsing enabled. It adds cgx_get_pkind() and
rvu_cgx_is_pkind_config_permitted() to gate the affected register
writes.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> index 4ff3935ed3fe..2be1da3476ac 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> @@ -1355,3 +1355,35 @@ void rvu_mac_reset(struct rvu *rvu, u16 pcifunc)
>  	if (mac_ops->mac_reset(cgxd, lmac, !is_vf(pcifunc)))
>  		dev_err(rvu->dev, "Failed to reset MAC\n");
>  }
> +
> +/* Do not allow CGX-mapped VFs to overwrite PKIND when special parse kinds
> + * (HiGig, EDSA, etc.) are in use on the shared LMAC.
> + */
> +bool rvu_cgx_is_pkind_config_permitted(struct rvu *rvu, u16 pcifunc)
> +{
> +	int pf, err, rxpkind;
> +	u8 cgx_id, lmac_id;
> +	void *cgxd;
> +
> +	pf = rvu_get_pf(rvu->pdev, pcifunc);
> +
> +	if (!(pcifunc & RVU_PFVF_FUNC_MASK))
> +		return true;
> +
> +	if (!is_pf_cgxmapped(rvu, pf))
> +		return true;
> +
> +	rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
> +	cgxd = rvu_cgx_pdata(cgx_id, rvu);
> +	err = cgx_get_pkind(cgxd, lmac_id, &rxpkind);
> +	if (err)
> +		return false;
> +
> +	switch (rxpkind) {
> +	case NPC_RX_HIGIG_PKIND:
> +	case NPC_RX_EDSA_PKIND:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}

[Medium]
The three call sites of rvu_cgx_is_pkind_config_permitted() interpret a
false return in two different ways.  In nix_interface_init() and
rvu_mbox_handler_nix_lf_alloc() a false result silently skips the write,
but the TX branch of rvu_npc_set_parse_mode() converts it into an
-EINVAL to the caller.

Pre-patch, rvu_mbox_handler_npc_set_pkind() (via rvu_npc_set_parse_mode)
always wrote NIX_AF_LFX_TX_PARSE_CFG.  Post-patch, a VF whose parent
PF's LMAC is programmed with NPC_RX_HIGIG_PKIND or NPC_RX_EDSA_PKIND
now receives a hard error where the call previously succeeded.

The same path is also reached from rvu_nix_lf_teardown() ->
rvu_npc_set_parse_mode(), where the -EINVAL is ignored but the intended
reset write to NIX_AF_LFX_TX_PARSE_CFG is skipped.

Is this mbox-level behavior change intended?  The commit message uses
the word "skip", which reads like a silent no-op, but this call site
aborts with -EINVAL rather than skipping.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index 6a0ce2665031..040eb4c9b8d1 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c

[ ... ]

> @@ -1685,8 +1693,10 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
>  	rvu_write64(rvu, blkaddr, NIX_AF_LFX_RX_CFG(nixlf), req->rx_cfg);
>  
>  	/* Configure pkind for TX parse config */
> -	cfg = NPC_TX_DEF_PKIND;
> -	rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf), cfg);
> +	if (rvu_cgx_is_pkind_config_permitted(rvu, pcifunc)) {
> +		cfg = NPC_TX_DEF_PKIND;
> +		rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf), cfg);
> +	}

[Medium]
This check-then-write pair reads the CGX RX pkind register (via
cgx_get_pkind() inside rvu_cgx_is_pkind_config_permitted()) without
holding cgxd->lock, but the corresponding cgx_set_pkind() writers were
just wrapped in cgxd->lock in this same patch.

Consider the following interleaving:

    CPU0 (VF nix_lf_alloc)                CPU1 (PF set_parse_mode)
    -----------------------------------   ---------------------------
    rvu_cgx_is_pkind_config_permitted()
       cgx_get_pkind() -> normal pkind
       returns true
                                          mutex_lock(&cgxd->lock);
                                          cgx_set_pkind(..., HIGIG);
                                          mutex_unlock(&cgxd->lock);
    rvu_write64(NIX_AF_LFX_TX_PARSE_CFG,
                NPC_TX_DEF_PKIND);

Can a concurrent PF that transitions the LMAC to NPC_RX_HIGIG_PKIND or
NPC_RX_EDSA_PKIND between the check and the write flip the state the
check depends on?

The nix_interface_init() call site holds cgxd->lock across both the
check and the writes; should the same pattern be applied here (and in
the TX branch of rvu_npc_set_parse_mode()) for consistency?

[Medium]
NIX_AF_LFX_TX_PARSE_CFG(nixlf) is indexed by NIX LF, not by LMAC.
Skipping the write when the LMAC is in HiGig/EDSA mode means the
register is left at whatever value it currently holds — either the
hardware reset value on a fresh LF, or stale contents from a previous
owner when an LF is reused across configurations.

If a HiGig VF needs NIX_AF_LFX_TX_PARSE_CFG programmed (for example to
NPC_TX_HIGIG_PKIND) to emit correctly formatted frames, where is that
value installed?  This hunk unconditionally elides the write when the
LMAC is HiGig/EDSA, and the teardown path via rvu_nix_lf_teardown() ->
rvu_npc_set_parse_mode() is likewise blocked from restoring the
default, so a subsequent non-HiGig owner reusing the same LF would
observe the previous owner's TX parse configuration.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index c7bc0b3a29b9..1297c6681a01 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -4241,6 +4245,9 @@ int rvu_npc_set_parse_mode(struct rvu *rvu, u16 pcifunc, u64 mode, u8 dir,
>  		if (rc)
>  			return rc;
>  
> +		if (!rvu_cgx_is_pkind_config_permitted(rvu, pcifunc))
> +			return -EINVAL;
> +
>  		rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf),
>  			    txpkind);
>  	}

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-20 16:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 12:19 [PATCH v3 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state Ratheesh Kannoth
2026-07-20 16:09 ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.