Re: [PATCH v7 04/10] net/mlx5: Add ONCE and MMIO accessor variants to mlx5_ifc_macros.h

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mlx5: missing array element size check in MLX5_ARRAY_GET64
--

commit f06ec3773a4462f249bea83375fa4f6bce249d9b
Author: Jason Gunthorpe <jgg@ziepe.ca>
net/mlx5: Add ONCE and MMIO accessor variants to mlx5_ifc_macros.h

This commit adds ONCE and MMIO variants to the MLX5 IFC macros for touching
ownership bits and accessing the init segment using IFC offsets. It also adds
MLX5_ARRAY_GET64() as the missing counterpart to MLX5_ARRAY_SET64().

> --- a/include/linux/mlx5/mlx5_ifc_macros.h
> +++ b/include/linux/mlx5/mlx5_ifc_macros.h
> @@ -85,6 +85,13 @@ __mlx5_mask(typ, fld))
>  	__MLX5_SET64(typ, p, fld[idx], v); \
>  } while (0)
>  
> +#define MLX5_ARRAY_GET64(typ, p, fld, idx)                                   \
> +	({                                                                   \
> +		BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64);                 \

[Severity: Medium]
Should there be a compile-time check for the element size here?

MLX5_ARRAY_GET64 performs pointer arithmetic on a __be64 *, which
implicitly scales the index by 8 bytes. If this macro is used on an array
with elements that are not exactly 64 bits (e.g., 32-bit elements), could
it silently compute incorrect memory offsets and read from the wrong
location?

The direct counterpart API, MLX5_ARRAY_SET64, enforces this exact
precondition via BUILD_BUG_ON(__mlx5_bit_sz(typ, fld) != 64) inside
__MLX5_SET64(). Would it be safer to add a similar BUILD_BUG_ON() to
MLX5_ARRAY_GET64 to avoid an asymmetric safety gap?

> +		be64_to_cpu(                                                 \
> +			*((__be64 *)(p) + __mlx5_64_off(typ, fld) + (idx))); \
> +	})
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0-v7-c6d30e8ce1e4+3dfa6-mlx5st_jgg@nvidia.com?part=4