| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250507091918.GZ3339421@horms.kernel.org>
Date: Wed, 7 May 2025 10:19:18 +0100
From: Simon Horman <horms@...nel.org>
To: Tanmay Jagdale <tanmay@...vell.com>
Cc: bbrezillon@...nel.org, arno@...isbad.org, schalla@...vell.com,
herbert@...dor.apana.org.au, davem@...emloft.net,
sgoutham@...vell.com, lcherian@...vell.com, gakula@...vell.com,
jerinj@...vell.com, hkelam@...vell.com, sbhatta@...vell.com,
andrew+netdev@...n.ch, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, bbhushan2@...vell.com, bhelgaas@...gle.com,
pstanner@...hat.com, gregkh@...uxfoundation.org,
peterz@...radead.org, linux@...blig.org,
krzysztof.kozlowski@...aro.org, giovanni.cabiddu@...el.com,
linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, rkannoth@...vell.com, sumang@...vell.com,
gcherian@...vell.com
Subject: Re: [net-next PATCH v1 04/15] octeontx2-af: Handle inbound inline
ipsec config in AF
On Fri, May 02, 2025 at 06:49:45PM +0530, Tanmay Jagdale wrote:
> From: Bharat Bhushan <bbhushan2@...vell.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@...vell.com>
> Signed-off-by: Tanmay Jagdale <tanmay@...vell.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)
...
Powered by blists - more mailing lists