Re: [net-next PATCH v1 06/15] octeontx2-af: Add support for CPT second pass

[Tanmay Jagdale <tanmay@marvell.com>]

Hi Simon,


On 2025-05-07 at 18:06:22, Simon Horman (horms@kernel.org) wrote:
> On Fri, May 02, 2025 at 06:49:47PM +0530, Tanmay Jagdale wrote:
> > From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > 
> > Implemented mailbox to add mechanism to allocate a
> > rq_mask and apply to nixlf to toggle RQ context fields
> > for CPT second pass packets.
> > 
> > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> > index 7fa98aeb3663..18e2a48e2de1 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> > @@ -544,6 +544,7 @@ void rvu_program_channels(struct rvu *rvu)
> >  
> >  void rvu_nix_block_cn10k_init(struct rvu *rvu, struct nix_hw *nix_hw)
> >  {
> > +	struct rvu_hwinfo *hw = rvu->hw;
> >  	int blkaddr = nix_hw->blkaddr;
> >  	u64 cfg;
> >  
> > @@ -558,6 +559,16 @@ void rvu_nix_block_cn10k_init(struct rvu *rvu, struct nix_hw *nix_hw)
> >  	cfg = rvu_read64(rvu, blkaddr, NIX_AF_CFG);
> >  	cfg |= BIT_ULL(1) | BIT_ULL(2);
> 
> As per my comments on an earlier patch in this series:
> bits 1 and 2 have meaning. It would be nice to use a #define to
> convey this meaning to the reader.
Okay sure, I will update the patch series with macros that provide a
clear meaning.

> 
> >  	rvu_write64(rvu, blkaddr, NIX_AF_CFG, cfg);
> > +
> > +	cfg = rvu_read64(rvu, blkaddr, NIX_AF_CONST);
> > +
> > +	if (!(cfg & BIT_ULL(62))) {
> > +		hw->cap.second_cpt_pass = false;
> > +		return;
> > +	}
> > +
> > +	hw->cap.second_cpt_pass = true;
> > +	nix_hw->rq_msk.total = NIX_RQ_MSK_PROFILES;
> >  }
> >  
> >  void rvu_apr_block_cn10k_init(struct rvu *rvu)
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > index 6bd995c45dad..b15fd331facf 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > @@ -6612,3 +6612,123 @@ int rvu_mbox_handler_nix_mcast_grp_update(struct rvu *rvu,
> >  
> >  	return ret;
> >  }
> > +
> > +static inline void
> > +configure_rq_mask(struct rvu *rvu, int blkaddr, int nixlf,
> > +		  u8 rq_mask, bool enable)
> > +{
> > +	u64 cfg, reg;
> > +
> > +	cfg = rvu_read64(rvu, blkaddr, NIX_AF_LFX_RX_IPSEC_CFG1(nixlf));
> > +	reg = rvu_read64(rvu, blkaddr, NIX_AF_LFX_CFG(nixlf));
> > +	if (enable) {
> > +		cfg |= BIT_ULL(43);
> > +		reg = (reg & ~GENMASK_ULL(36, 35)) | ((u64)rq_mask << 35);
> > +	} else {
> > +		cfg &= ~BIT_ULL(43);
> > +		reg = (reg & ~GENMASK_ULL(36, 35));
> > +	}
> 
> Likewise for the bit, mask, and shift here.
> 
> And I think that using FIELD_PREP with another mask in place of the shift
> is also appropriate here.
ACK.

> 
> > +	rvu_write64(rvu, blkaddr, NIX_AF_LFX_RX_IPSEC_CFG1(nixlf), cfg);
> > +	rvu_write64(rvu, blkaddr, NIX_AF_LFX_CFG(nixlf), reg);
> > +}
> > +
> > +static inline void
> > +configure_spb_cpt(struct rvu *rvu, int blkaddr, int nixlf,
> > +		  struct nix_rq_cpt_field_mask_cfg_req *req, bool enable)
> > +{
> > +	u64 cfg;
> > +
> > +	cfg = rvu_read64(rvu, blkaddr, NIX_AF_LFX_RX_IPSEC_CFG1(nixlf));
> > +	if (enable) {
> > +		cfg |= BIT_ULL(37);
> > +		cfg &= ~GENMASK_ULL(42, 38);
> > +		cfg |= ((u64)req->ipsec_cfg1.spb_cpt_sizem1 << 38);
> > +		cfg &= ~GENMASK_ULL(63, 44);
> > +		cfg |= ((u64)req->ipsec_cfg1.spb_cpt_aura << 44);
> > +	} else {
> > +		cfg &= ~BIT_ULL(37);
> > +		cfg &= ~GENMASK_ULL(42, 38);
> > +		cfg &= ~GENMASK_ULL(63, 44);
> > +	}
> 
> And here too.
> 
> > +	rvu_write64(rvu, blkaddr, NIX_AF_LFX_RX_IPSEC_CFG1(nixlf), cfg);
> > +}
> 
> ...
> 
> > +int rvu_mbox_handler_nix_lf_inline_rq_cfg(struct rvu *rvu,
> > +					  struct nix_rq_cpt_field_mask_cfg_req *req,
> > +					  struct msg_rsp *rsp)
> 
> It would be nice to reduce this to 80 columns wide or less.
> Perhaps like this?
> 
> int
> rvu_mbox_handler_nix_lf_inline_rq_cfg(struct rvu *rvu,
> 				      struct nix_rq_cpt_field_mask_cfg_req *req,
> 				      struct msg_rsp *rsp)
> 
> Or perhaps by renaming nix_rq_cpt_field_mask_cfg_req to be shorter.
Okay sure. I'll go ahead with the first suggestion so that the function
name is in sync with the rest of the file.

> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_reg.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_reg.h
> > index 245e69fcbff9..e5e005d5d71e 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_reg.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_reg.h
> > @@ -433,6 +433,8 @@
> >  #define NIX_AF_MDQX_IN_MD_COUNT(a)	(0x14e0 | (a) << 16)
> >  #define NIX_AF_SMQX_STATUS(a)		(0x730 | (a) << 16)
> >  #define NIX_AF_MDQX_OUT_MD_COUNT(a)	(0xdb0 | (a) << 16)
> > +#define NIX_AF_RX_RQX_MASKX(a, b)       (0x4A40 | (a) << 16 | (b) << 3)
> > +#define NIX_AF_RX_RQX_SETX(a, b)        (0x4A80 | (a) << 16 | (b) << 3)
> 
> FIELD_PREP could be used here in conjunction with #defines
> for appropriate masks here too.
ACK.
> 
> >  
> >  #define NIX_PRIV_AF_INT_CFG		(0x8000000)
> >  #define NIX_PRIV_LFX_CFG		(0x8000010)
> 
> ...
Thanks,
Tanmay