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

Geetha sowjanya posted 1 patch 1 week, 5 days ago
There is a newer version of this series
.../net/ethernet/marvell/octeontx2/nic/otx2_common.h  | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[net-next PATCH] octeontx2-pf: cn10k/cn20k: Update count_eot in NPA_LF_AURA_BATCH_FREE0
Posted by Geetha sowjanya 1 week, 5 days ago
The count_eot field determines how many valid pointers are present in
the last 128-bit word during buffer free operations. The current driver
only handled CN10K devices where count_eot feild is 1 bit wide.
And possible values, count_eot = 0 for an even number of
pointers and 1 for an odd number.

For CN20K devices,the count_eot feild extended to 2 bits.
The hardware expects count_eot to represent the least
significant two bits of the number of valid pointers to free. This patch
updates the calculation accordingly to ensure correct behavior on both
CN10K and CN20K platforms.

Signed-off-by: Geetha sowjanya <gakula@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/nic/otx2_common.h  | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

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;
 	/* Set AURA ID to free pointer */
 	ptrs[0] = (count_eot << 32) | (aura & 0xFFFFF);
 	/* Target address for LMTST flush tells HW how many 128bit
-- 
2.25.1
Re: [net-next] octeontx2-pf: cn10k/cn20k: Update count_eot in NPA_LF_AURA_BATCH_FREE0
Posted by Jakub Kicinski 1 week, 4 days ago
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