Re: [PATCH v5 3/3] selftests/lam: Add test cases for different LAM tag widths

[Sohil Mehta]

The subject needs a change. No new tests are being added now, right?

On 4/7/2026 10:45 AM, Maciej Wieczor-Retman wrote:
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx>
> 
> diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
> index 1919fa6daec0..d27f947ea694 100644
> --- a/tools/testing/selftests/x86/lam.c
> +++ b/tools/testing/selftests/x86/lam.c
> @@ -26,9 +26,9 @@
>  
>  /* LAM modes, these definitions were copied from kernel code */
>  #define LAM_NONE                0
> -#define LAM_U57_BITS            6
> +#define LAM_BITS                4
>  
> -#define LAM_U57_MASK            (0x3fULL << 57)
> +#define LAM_MASK                (0xfULL << 57)

How about calculating the LAM_MASK from the LAM bits similar to the core
code in patch 1?

Also, when copying over the defines it would be useful to name them
exactly the same as the core code. That way any name changes would
happen in sync.


> @@ -247,10 +247,10 @@ static uint64_t set_metadata(uint64_t src, unsigned long lam)
>  	srand(time(NULL));
>  
>  	switch (lam) {
> -	case LAM_U57_BITS: /* Set metadata in bits 62:57 */
> +	case LAM_BITS: /* Set metadata in bits 62:57 */

This needs to change to 60:57 similar to the change below.

>  		/* Get a random non-zero value as metadata */
> -		metadata = (rand() % ((1UL << LAM_U57_BITS) - 1) + 1) << 57;
> -		metadata |= (src & ~(LAM_U57_MASK));
> +		metadata = (rand() % ((1UL << LAM_BITS) - 1) + 1) << 57;
> +		metadata |= (src & ~(LAM_MASK));
>  		break;
>  	default:
>  		metadata = src;
> @@ -291,7 +291,7 @@ int handle_max_bits(struct testcases *test)
>  	unsigned long bits = 0;
>  
>  	if (exp_bits != LAM_NONE)
> -		exp_bits = LAM_U57_BITS;
> +		exp_bits = LAM_BITS;
>  
>  	/* Get LAM max tag bits */
>  	if (syscall(SYS_arch_prctl, ARCH_GET_MAX_TAG_BITS, &bits) == -1)
> @@ -719,8 +719,8 @@ int do_uring(unsigned long lam)
>  			uint64_t addr = ((uint64_t)fi->iovecs[i].iov_base);
>  
>  			switch (lam) {
> -			case LAM_U57_BITS: /* Clear bits 62:57 */
> -				addr = (addr & ~(LAM_U57_MASK));
> +			case LAM_BITS: /* Clear bits 60:57 */

Just like this one.