[PATCH v5] cmd: mem: fix range of bitflip test

[Stefan Roese]

On 09.09.20 18:10, Ralph Siemsen wrote:
> The bitflip test uses two equal sized memory buffers. This is achieved
> by splitting the range of memory into two pieces. The address of the
> second buffer, as well as the length of each buffer, were not correctly
> calculated. This caused bitflip test to access beyond the end of range.
> This patch fixes the pointer arithmetic problem.
> 
> A second problem arises because u-boot "mtest" command expects the
> ending address to be inclusive. When computing (end - start) this
> results in missing 1 byte of the requested length. The bitflip test
> expects a count rather than an "ending" address. Thus it fails to test
> the last word of the requested range. Fixed by using (end - start + 1).
> 
> Added Kconfig option to optionally disable the bitflip test, since it
> does add significantly to the time taken for "mtest".
> 
> Fixes: 8e434cb705d463bc8cff935160e4fb4c77cb99ab ("cmd: mem: Add bitflip
> memory test to alternate mtest")
> 
> Signed-off-by: Ralph Siemsen <ralph.siemsen at linaro.org>

Reviewed-by: Stefan Roese <sr at denx.de>

Thanks,
Stefan

> --
> 
> Changes in v5:
> - Correct logic for updating error count
> 
> Changes in v4:
> - Avoid #ifdef in the code
> 
> Change-Id: Ie641d04e731fc5bc6a3bbef914bf7fad136cdc94
> ---
>   cmd/Kconfig | 12 ++++++++++++
>   cmd/mem.c   | 21 ++++++++++++++++-----
>   2 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index d54acf2cfd..275bf7fbfe 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -779,6 +779,18 @@ config SYS_ALT_MEMTEST
>   	help
>   	  Use a more complete alternative memory test.
>   
> +if SYS_ALT_MEMTEST
> +
> +config SYS_ALT_MEMTEST_BITFLIP
> +	bool "Bitflip test"
> +	default y
> +	help
> +	  The alternative memory test includes bitflip test since 2020.07.
> +	  The bitflip test significantly increases the overall test time.
> +	  Bitflip test can optionally be disabled here.
> +
> +endif
> +
>   config SYS_MEMTEST_START
>   	hex "default start address for mtest"
>   	default 0
> diff --git a/cmd/mem.c b/cmd/mem.c
> index 9b97f7bf69..dc4a0ffab3 100644
> --- a/cmd/mem.c
> +++ b/cmd/mem.c
> @@ -867,6 +867,18 @@ static ulong test_bitflip_comparison(volatile unsigned long *bufa,
>   	return errs;
>   }
>   
> +static ulong mem_test_bitflip(vu_long *buf, ulong start, ulong end)
> +{
> +	/*
> +	 * Split the specified range into two halves.
> +	 * Note that mtest range is inclusive of start,end.
> +	 * Bitflip test instead uses a count (of 32-bit words).
> +	 */
> +	ulong half_size = (end - start + 1) / 2 / sizeof(unsigned long);
> +
> +	return test_bitflip_comparison(buf, buf + half_size, half_size);
> +}
> +
>   static ulong mem_test_quick(vu_long *buf, ulong start_addr, ulong end_addr,
>   			    vu_long pattern, int iteration)
>   {
> @@ -986,11 +998,10 @@ static int do_mem_mtest(struct cmd_tbl *cmdtp, int flag, int argc,
>   			errs = mem_test_alt(buf, start, end, dummy);
>   			if (errs == -1UL)
>   				break;
> -			count += errs;
> -			errs = test_bitflip_comparison(buf,
> -						       buf + (end - start) / 2,
> -						       (end - start) /
> -						       sizeof(unsigned long));
> +			if (IS_ENABLED(CONFIG_SYS_ALT_MEMTEST_BITFLIP)) {
> +				count += errs;
> +				errs = mem_test_bitflip(buf, start, end);
> +			}
>   		} else {
>   			errs = mem_test_quick(buf, start, end, pattern,
>   					      iteration);
>