[PATCH v2 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state

[PATCH v2 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state

[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>

---
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   | 29 ++++++++++++++---
 5 files changed, 71 insertions(+), 4 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 0297c7ab0614..4e72d6e072d5 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;
@@ -1509,11 +1517,14 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
 	u16 bcast, mcast, promisc, ucast;
 	struct rvu_hwinfo *hw = rvu->hw;
 	u16 pcifunc = req->hdr.pcifunc;
+	u8 cgx_id = 0, lmac_id = 0;
 	bool rules_created = false;
 	struct rvu_block *block;
 	struct rvu_pfvf *pfvf;
 	u64 cfg, ctx_cfg;
+	struct cgx *cgxd;
 	int blkaddr;
+	int pf;
 
 	if (!req->rq_cnt || !req->sq_cnt || !req->cq_cnt)
 		return NIX_AF_ERR_PARAM;
@@ -1685,8 +1696,18 @@ 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 (is_pf_cgxmapped(rvu, rvu_get_pf(rvu->pdev, pcifunc))) {
+		pf = rvu_get_pf(rvu->pdev, pcifunc);
+		rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
+		cgxd = rvu_cgx_pdata(cgx_id, rvu);
+
+		mutex_lock(&cgxd->lock);
+		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);
+		}
+		mutex_unlock(&cgxd->lock);
+	}
 
 	if (is_rep_dev(rvu, pcifunc)) {
 		pfvf->tx_chan_base = RVU_SWITCH_LBK_CHAN;
-- 
2.43.0

Re: [PATCH v2 net] octeontx2-af: Block VFs from clobbering special CGX PKIND state

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2-af: Block VFs from clobbering special CGX PKIND state

This patch prevents VF NIX LF allocation from resetting the CGX RX PKIND
and default TX parse config when the shared LMAC has HiGig2 or EDSA
parsing enabled. It adds cgx_get_pkind() and a new
rvu_cgx_is_pkind_config_permitted() helper, then gates the pkind and TX
parse writes in nix_interface_init() and rvu_mbox_handler_nix_lf_alloc().

> 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;
> +	}
> +}

[High]
Is the exclusion actually enforced against the other writers of these
registers?

rvu_npc_set_parse_mode() also calls cgx_set_pkind() and writes
NIX_AF_LFX_TX_PARSE_CFG(nixlf) directly, and it is reachable from the
rvu_mbox_handler_npc_set_pkind mbox handler as well as from
rvu_nix_lf_teardown(). None of those paths take cgxd->lock.

Consider this interleaving on a shared LMAC:

    CPU A (PF)                       CPU B (VF)
    rvu_mbox_handler_npc_set_pkind
      rvu_npc_set_parse_mode
        (no cgxd->lock held)
                                     mutex_lock(&cgxd->lock)
                                     cgx_get_pkind() -> default
                                     rvu_cgx_is_pkind_config_permitted()
                                       returns true
                                     cgx_set_pkind(default)
                                     rvu_npc_set_pkind(default)
                                     mutex_unlock(&cgxd->lock)
        cgx_set_pkind(HIGIG)
        write NIX_AF_LFX_TX_PARSE_CFG

Or the reverse order, where CPU B's write clobbers CPU A's HiGig
programming after CPU A has already installed it. Since
rvu_npc_set_parse_mode() does not acquire cgxd->lock, does the check in
rvu_cgx_is_pkind_config_permitted() effectively close the race the
commit message describes?

Additionally, the comment on struct cgx's lock in lmac_common.h reads:

    /* Lock to serialize read/write of global csrs like
     * RPMX_MTI_STAT_DATA_HI_CDC etc
     */
    struct mutex		lock;

Is cgxd->lock the intended lock to serialize per-NIX-LF register writes
like NIX_AF_LFX_TX_PARSE_CFG, or should a different mechanism be used?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index 0297c7ab0614..4e72d6e072d5 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c

[ ... ]

> @@ -1509,11 +1517,14 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu *rvu,
>  	u16 bcast, mcast, promisc, ucast;
>  	struct rvu_hwinfo *hw = rvu->hw;
>  	u16 pcifunc = req->hdr.pcifunc;
> +	u8 cgx_id = 0, lmac_id = 0;
>  	bool rules_created = false;
>  	struct rvu_block *block;
>  	struct rvu_pfvf *pfvf;
>  	u64 cfg, ctx_cfg;
> +	struct cgx *cgxd;
>  	int blkaddr;
> +	int pf;
>  
>  	if (!req->rq_cnt || !req->sq_cnt || !req->cq_cnt)
>  		return NIX_AF_ERR_PARAM;
> @@ -1685,8 +1696,18 @@ 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 (is_pf_cgxmapped(rvu, rvu_get_pf(rvu->pdev, pcifunc))) {
> +		pf = rvu_get_pf(rvu->pdev, pcifunc);
> +		rvu_get_cgx_lmac_id(rvu->pf2cgxlmac_map[pf], &cgx_id, &lmac_id);
> +		cgxd = rvu_cgx_pdata(cgx_id, rvu);
> +
> +		mutex_lock(&cgxd->lock);
> +		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);
> +		}
> +		mutex_unlock(&cgxd->lock);
> +	}

[High]
Does this outer is_pf_cgxmapped() guard change behavior for LBK, SDP and
representor NIX LFs?

Prior to this patch the write of NPC_TX_DEF_PKIND to
NIX_AF_LFX_TX_PARSE_CFG(nixlf) was unconditional for every NIX LF that
reached this point. is_pf_cgxmapped() returns false when
pf < PF_CGXMAP_BASE (e.g. AF PF0 that owns LBK VFs) and for SDP PFs (via
the is_sdp_pf() exclusion):

    static inline bool is_pf_cgxmapped(struct rvu *rvu, u8 pf)
    {
        return (pf >= PF_CGXMAP_BASE && pf <= rvu->cgx_mapped_pfs) &&
            !is_sdp_pf(rvu, rvu_make_pcifunc(rvu->pdev, pf, 0));
    }

For those non-CGX cases the register is now left at whatever
rvu_lf_reset() leaves behind rather than NPC_TX_DEF_PKIND.

The commit message describes the intent narrowly:

    a VF NIX LF alloc must not reset the LMAC RX PKIND or default TX
    parse config over the PF setup.

rvu_cgx_is_pkind_config_permitted() already returns true for non-VF and
for non-CGX-mapped pcifuncs. Would gating solely on
rvu_cgx_is_pkind_config_permitted() (without the outer
is_pf_cgxmapped()) match the stated intent and preserve prior behavior
for LBK, SDP and representor NIX LFs?