Re: [net-next] octeontx2-pf: cn10k/cn20k: Update count_eot in NPA_LF_AURA_BATCH_FREE0

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
octeontx2-pf: cn10k/cn20k: Update count_eot in NPA_LF_AURA_BATCH_FREE0

This commit extends the count_eot calculation for CN20K devices where
the field is 2 bits wide, while maintaining CN10K compatibility where
only bit 0 is used.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> index 8cdfc36d79d2..c6023e1ce92b 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> @@ -789,8 +789,15 @@ static inline void __cn10k_aura_freeptr(struct otx2_nic *pfvf, u64 aura,
>  	tar_addr = (__force u64)otx2_get_regaddr(pfvf, NPA_LF_AURA_BATCH_FREE0);
>  	/* LMTID is same as AURA Id */
>  	val = (lmt_info->lmt_id & 0x7FF) | BIT_ULL(63);
> -	/* Set if [127:64] of last 128bit word has a valid pointer */
> -	count_eot = (num_ptrs % 2) ? 0ULL : 1ULL;
> +	/* Meaning of count_eot
> +	 * CN10K: count_eot = 0 if the number of pointers to free is even,
> +	 *	  count_eot = 1 if the number of pointers to free is odd.
> +	 *
> +	 * CN20K: count_eot represents the least significant 2 bits of the
> +	 *	  total number of valid pointers to free.
> +	 *        Example: if 7 pointers are freed (0b111), count_eot = 0b11.
> +	 */
> +	count_eot = (num_ptrs - 1) & 0x3ULL;

The new count_eot formula looks correct for passing to hardware, but does
it break the size calculation further down in this function? The old code
produced count_eot values of only 0 or 1, while the new formula can produce
0, 1, 2, or 3.

Later in this same function there is:

    if (!count_eot)
        size++;

With the old formula, for num_ptrs=3 (2 buffers), count_eot was 0, so size
got incremented from 1 to 2, which is correct for 24 bytes needing 2
128-bit words.

With the new formula, for num_ptrs=3, count_eot becomes 2, which is truthy,
so size stays at 1. But 3 u64 values still require 2 128-bit words.

This appears to cause incorrect LMTST size for num_ptrs values of 3, 7, 11,
and 15, where the driver would tell hardware about fewer 128-bit words than
are actually present.

Should the size adjustment check be updated to something like:

    if (!(count_eot & 1))
        size++;

to preserve the original behavior of incrementing size when the buffer
count (num_ptrs - 1) is even?
-- 
pw-bot: cr