linux-kernel - Re: [PATCH net,2/6] octeontx2-af: reduce cpt flt interrupt vectors for cn10kb

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 2 Jul 2024 11:28:43 +0100
From: Simon Horman <horms@...nel.org>
To: Srujana Challa <schalla@...vell.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, kuba@...nel.org,
	davem@...emloft.net, pabeni@...hat.com, sgoutham@...vell.com,
	lcherian@...vell.com, gakula@...vell.com, jerinj@...vell.com,
	hkelam@...vell.com, sbhatta@...vell.com
Subject: Re: [PATCH net,2/6] octeontx2-af: reduce cpt flt interrupt vectors
 for cn10kb

On Mon, Jul 01, 2024 at 02:37:42PM +0530, Srujana Challa wrote:
> On new silicon(cn10kb), the number of FLT interrupt vectors has
> been reduced. Hence, this patch modifies the code to make
> it work for both cn10ka and cn10kb.
> 

I am tempted to think this is more about enabling new hardware
than fixing a bug. But I do also see how one might argue otherwise.

In any case, if this is a fix then a fixes tag should go here.
> Signed-off-by: Srujana Challa <schalla@...vell.com>
> ---
>  .../net/ethernet/marvell/octeontx2/af/mbox.h  |  5 +-
>  .../ethernet/marvell/octeontx2/af/rvu_cpt.c   | 73 ++++++++++++++++---
>  .../marvell/octeontx2/af/rvu_struct.h         |  5 +-
>  3 files changed, 65 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> index 4a77f6fe2622..41b46724cb3d 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -1848,8 +1848,9 @@ struct cpt_flt_eng_info_req {
>  
>  struct cpt_flt_eng_info_rsp {
>  	struct mbox_msghdr hdr;
> -	u64 flt_eng_map[CPT_10K_AF_INT_VEC_RVU];
> -	u64 rcvrd_eng_map[CPT_10K_AF_INT_VEC_RVU];
> +#define CPT_AF_MAX_FLT_INT_VECS 3
> +	u64 flt_eng_map[CPT_AF_MAX_FLT_INT_VECS];
> +	u64 rcvrd_eng_map[CPT_AF_MAX_FLT_INT_VECS];
>  	u64 rsvd;
>  };
>  
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> index 98440a0241a2..38363ea56c6c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cpt.c
> @@ -37,6 +37,38 @@
>  	(_rsp)->free_sts_##etype = free_sts;                        \
>  })
>  
> +#define MAX_AE  GENMASK_ULL(47, 32)
> +#define MAX_IE  GENMASK_ULL(31, 16)
> +#define MAX_SE  GENMASK_ULL(15, 0)
> +static u32 cpt_max_engines_get(struct rvu *rvu)
> +{
> +	u16 max_ses, max_ies, max_aes;
> +	u64 reg;
> +
> +	reg = rvu_read64(rvu, BLKADDR_CPT0, CPT_AF_CONSTANTS1);
> +	max_ses = FIELD_GET(MAX_SE, reg);
> +	max_ies = FIELD_GET(MAX_IE, reg);
> +	max_aes = FIELD_GET(MAX_AE, reg);
> +
> +	return max_ses + max_ies + max_aes;
> +}
> +
> +/* Number of flt interrupt vectors are depends on number of engines that
> + * the chip has. Each flt vector represents 64 engines.
> + */
> +static int cpt_10k_flt_nvecs_get(struct rvu *rvu)
> +{
> +	u32 max_engs;
> +	int flt_vecs;
> +
> +	max_engs = cpt_max_engines_get(rvu);
> +
> +	flt_vecs = (max_engs / 64);
> +	flt_vecs += (max_engs % 64) ? 1 : 0;
> +
> +	return flt_vecs;
> +}
> +

I think the callers of this function assume it will never return a value
greater than 3. Perhaps it would be worth enforcing that, or WARNing if it
not so.  I'm thinking of a case a fw/hw revision comes along and this
assumption no longer holds.

>  static irqreturn_t cpt_af_flt_intr_handler(int vec, void *ptr)
>  {
>  	struct rvu_block *block = ptr;

...