Re: [net-next PATCH v1 04/15] octeontx2-af: Handle inbound inline ipsec config in AF

Simon Horman
2025-05-07T09:19:26Z

On Fri, May 02, 2025 at 06:49:45PM +0530, Tanmay Jagdale wrote:
> From: Bharat Bhushan <bbhushan2@marvell.com>
>
> Now CPT context flush can be handled in AF as CPT LF
> can be attached to it. With that AF driver can completely
> handle inbound inline ipsec configuration mailbox, so
> forward this mailbox to AF driver.
>
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
> ---
> .../marvell/octeontx2/otx2_cpt_common.h | 1 -
> .../marvell/octeontx2/otx2_cptpf_mbox.c | 3 -
> .../net/ethernet/marvell/octeontx2/af/mbox.h | 2 +
> .../ethernet/marvell/octeontx2/af/rvu_cpt.c | 67 +++++++++----------
> .../ethernet/marvell/octeontx2/af/rvu_reg.h | 1 +
> 5 files changed, 34 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/crypto/marvell/octeontx2/otx2_cpt_common.h b/drivers/crypto/marvell/octeontx2/otx2_cpt_common.h
> index df735eab8f08..27a2dd997f73 100644
> --- a/drivers/crypto/marvell/octeontx2/otx2_cpt_common.h
> +++ b/drivers/crypto/marvell/octeontx2/otx2_cpt_common.h
> @@ -33,7 +33,6 @@
> #define BAD_OTX2_CPT_ENG_TYPE OTX2_CPT_MAX_ENG_TYPES
>
> /* Take mbox id from end of CPT mbox range in AF (range 0xA00 - 0xBFF) */
> -#define MBOX_MSG_RX_INLINE_IPSEC_LF_CFG 0xBFE
> #define MBOX_MSG_GET_ENG_GRP_NUM 0xBFF
> #define MBOX_MSG_GET_CAPS 0xBFD
> #define MBOX_MSG_GET_KVF_LIMITS 0xBFC
> diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_mbox.c b/drivers/crypto/marvell/octeontx2/otx2_cptpf_mbox.c
> index 5e6f70ac35a7..222419bd5ac9 100644
> --- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_mbox.c
> +++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_mbox.c
> @@ -326,9 +326,6 @@ static int cptpf_handle_vf_req(struct otx2_cptpf_dev *cptpf,
> case MBOX_MSG_GET_KVF_LIMITS:
> err = handle_msg_kvf_limits(cptpf, vf, req);
> break;
> - case MBOX_MSG_RX_INLINE_IPSEC_LF_CFG:
> - err = handle_msg_rx_inline_ipsec_lf_cfg(cptpf, req);
> - break;
>
> default:
> err = forward_to_af(cptpf, vf, req, size);

This removes the only caller of handle_msg_rx_inline_ipsec_lf_cfg()
Which in turn removes the only caller of rx_inline_ipsec_lf_cfg(),
and in turn send_inline_ipsec_inbound_msg().

Those functions should be removed by the same patch that makes the changes
above. Which I think could be split into a separate patch from the changes
below.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c

...

> @@ -1253,20 +1258,36 @@ int rvu_cpt_lf_teardown(struct rvu *rvu, u16 pcifunc, int blkaddr, int lf, int s
> return 0;
> }
>
> +static void cn10k_cpt_inst_flush(struct rvu *rvu, u64 *inst, u64 size)
> +{
> + u64 val = 0, tar_addr = 0;
> + void __iomem *io_addr;
> + u64 blkaddr = BLKADDR_CPT0;

nit: Please use reverse xmas tree order - longest line to shortest -
for local variable declarations in new Networking code.

Edward Cree's tool can be useful here.
https://github.com/ecree-solarflare/xmastree/

> +
> + io_addr = rvu->pfreg_base + CPT_RVU_FUNC_ADDR_S(blkaddr, 0, CPT_LF_NQX);
> +
> + /* Target address for LMTST flush tells HW how many 128bit
> + * words are present.
> + * tar_addr[6:4] size of first LMTST - 1 in units of 128b.
> + */
> + tar_addr |= (__force u64)io_addr | (((size / 16) - 1) & 0x7) << 4;

I see this pattern elsewhere. But, FWIIW, I don't think it
is entirely desirable to:

1) Cast away the __iomem annotation
2) Treat a u64 as an (io?) address
3) Open code the calculation of tar_addr, which
also seems to appear in several other places.

If these things are really necessary then I would
put them in some combination of cn10k_lmt_flush(),
helpers, and wrappers.

But as this is consistent with code elsewhere,
perhaps that is a topic for another time.

> + dma_wmb();
> + memcpy((u64 *)rvu->rvu_cpt.lmt_addr, inst, size);

FWIIW, I'm not sure that treating a u64 (the type of lmt_addr) as
an address is best either.

> + cn10k_lmt_flush(val, tar_addr);
> + dma_wmb();
> +}
> +
> #define CPT_RES_LEN 16
> #define CPT_SE_IE_EGRP 1ULL
>
> static int cpt_inline_inb_lf_cmd_send(struct rvu *rvu, int blkaddr,
> int nix_blkaddr)
> {
> - int cpt_pf_num = rvu->cpt_pf_num;
> - struct cpt_inst_lmtst_req *req;
> dma_addr_t res_daddr;
> int timeout = 3000;
> u8 cpt_idx;
> - u64 *inst;
> + u64 inst[8];
> u16 *res;
> - int rc;

nit: reverse xmas tree here too.

>
> res = kzalloc(CPT_RES_LEN, GFP_KERNEL);
> if (!res)

...