[U-Boot] [PATCH] Feature Removal: disable "mtest" command by default

Ira W. Snyder iws at ovro.caltech.edu
Fri Mar 8 20:41:31 CET 2013


On Fri, Mar 08, 2013 at 08:08:24PM +0100, Wolfgang Denk wrote:
> The "mtest" command is of little practical use (if any), and
> experience has shown that a large number of board configurations
> define useless or even dangerous start and end addresses.  If not even
> the board maintainers are able to figure out which memory range can be
> reliably tested, how can we expect such from the end users?  As this
> problem comes up repeatedly, we rather do not enable this command by
> default, so only people who know what they are doing will be
> confronted with it.
> 
> As this changes the user interface, we allow for a grace period
> before this change takes effect. For now, we make "mtest"
> configurable through the CONFIG_CMD_MEMTEST variable, which is defined
> in include/config_cmd_default.h;  we also add an entry to
> doc/feature-removal-schedule.txt which announces the removal of this
> default setting in two releases from now, i. e. with v2013.07.
> 
> Signed-off-by: Wolfgang Denk <wd at denx.de>
> Cc: Tom Rini <trini at ti.com>

Hi Wolfgang Denk,

I noticed a couple of small typos, and thought I'd point them out.

Hope it helps,
Ira

> ---
>  README                           |  3 +-
>  common/cmd_mem.c                 |  5 +-
>  doc/README.memory-test           | 98 ++++++++++++++++++++++++++++++++++++++++
>  doc/feature-removal-schedule.txt | 17 +++++++
>  include/config_cmd_all.h         |  3 +-
>  include/config_cmd_default.h     |  3 +-
>  6 files changed, 125 insertions(+), 4 deletions(-)
>  create mode 100644 doc/README.memory-test
> 
> diff --git a/README b/README
> index 42544ce..869694f 100644
> --- a/README
> +++ b/README
> @@ -860,7 +860,8 @@ The following options need to be configured:
>  					  (requires CONFIG_CMD_MEMORY and CONFIG_MD5)
>  		CONFIG_CMD_MEMINFO	* Display detailed memory information
>  		CONFIG_CMD_MEMORY	  md, mm, nm, mw, cp, cmp, crc, base,
> -					  loop, loopw, mtest
> +					  loop, loopw
> +		CONFIG_CMD_MEMTEST	  mtest
>  		CONFIG_CMD_MISC		  Misc functions like sleep etc
>  		CONFIG_CMD_MMC		* MMC memory mapped support
>  		CONFIG_CMD_MII		* MII utility commands
> diff --git a/common/cmd_mem.c b/common/cmd_mem.c
> index 042c994..53572f7 100644
> --- a/common/cmd_mem.c
> +++ b/common/cmd_mem.c
> @@ -910,6 +910,7 @@ static ulong mem_test_quick(vu_long *buf, ulong start_addr, ulong end_addr,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_CMD_MEMTEST
>  /*
>   * Perform a memory test. A more complete alternative test can be
>   * configured using CONFIG_SYS_ALT_MEMTEST. The complete test loops until
> @@ -1002,7 +1003,7 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int argc,
>  
>  	return ret;	/* not reached */
>  }
> -
> +#endif	/* CONFIG_CMD_MEMTEST */
>  
>  /* Modify memory.
>   *
> @@ -1240,11 +1241,13 @@ U_BOOT_CMD(
>  );
>  #endif /* CONFIG_LOOPW */
>  
> +#ifdef CONFIG_CMD_MEMTEST
>  U_BOOT_CMD(
>  	mtest,	5,	1,	do_mem_mtest,
>  	"simple RAM read/write test",
>  	"[start [end [pattern [iterations]]]]"
>  );
> +#endif	/* CONFIG_CMD_MEMTEST */
>  
>  #ifdef CONFIG_MX_CYCLIC
>  U_BOOT_CMD(
> diff --git a/doc/README.memory-test b/doc/README.memory-test
> new file mode 100644
> index 0000000..1e9520b
> --- /dev/null
> +++ b/doc/README.memory-test
> @@ -0,0 +1,98 @@
> +The most frequent cause of problems when porting U-Boot to new
> +hardware, or when using a sloppy port on some board, is memory errors.
> +In most cases these are not caused by failing hardware, but by
> +incorrect initialization of the memory controller.  So it appears to
> +be a good idea to always test if the memory is working correctly,
> +before looking for any other potential causes of any problems.
> +
> +U-Boot implements 3 different approaches to perform memory tests:
> +
> +1. The get_ram_size() function (see "common/memsize.c").
> +
> +   This function is supposed to be used in each and every U-Boot port
> +   determine the presence and actual size of each of the potential
> +   memory banks on this piece of hardware.  The code is supposed to be
> +   very fast, so running it for each reboot does not hurt.  It is a
> +   little known and generally underrated fact that this code will also
> +   catch 99% of hardware related (i. e. reliably reproducable) memory
> +   errors.  It is strongly recommended to always use this function, in
> +   each and every port of U-Boot.
> +
> +2. The "mtest" command.
> +
> +   This is probably the best known memory test utility in U-Boot.
> +   Unfortunately, it is also the most problematic, and the most
> +   useless one.
> +
> +   There are a number of serious problems with this command:
> +
> +   - It is terribly slow.  Running "mtest" on the whole system RAM
> +     takes a _long_ time before there is any significance in the fact
> +     that no errors have been found so far.
> +
> +   - It is difficult to configure, and to use.  And any errors here
> +     will reliably crash or hang your system.  "mtest" is dump and has
                                                             ^dumb

> +     no knowledge about memory ranges that may be in use for other
> +     purposes, like exception code, U-Boot code and data, stack,
> +     malloc arena, video buffer, log buffer, etc.  If you let it, it
> +     will happily "test" all such areas, which of corse will cause
                                                     ^course

> +     some problems.
> +
> +   - It is not easy to configure and use, and a large number of
> +     systems are seriously misconfigured.  The original idea was to
> +     test basically the whole system RAM, with only excempting the
                                                       ^exempting

> +     areas used by U-Bot itself - on most systems these are the areas
                      ^U-Boot

> +     used for the exception vectors (usually at the very lower end of
> +     system memory) and for U-Boot (code, data, etc. - see above;
> +     these are usually at the very upper end of system memory).  But
> +     experience has shown that a very large number of ports use
> +     pretty much bogus settings of CONFIG_SYS_MEMTEST_START and
> +     CONFIG_SYS_MEMTEST_END; this results in useless tests (because
> +     the ranges is too small and/or badly located) or in critical
> +     failures (system crashes).
> +
> +   Because of these issues, the "mtest" command is considered depre-
> +   cated.  It should not be enabled in most normal ports of U-Boot,
> +   especially not in production.  If you really need a memory test,
> +   then see 1. and 3. above resp. below.
> +
> +3. The most thorough memory test facility is available as part of the
> +   POST (Power-On Self Test) sub-system, see "post/drivers/memory.c".
> +
> +   If you areally need to perform memory tests (for example, because
             ^really

> +   it is mandatory part of your requirement specification), then
> +   enable this test which is generic and should work on all archi-
> +   tectures.
> +
> +WARNING:
> +
> +It should pointed out that _all_ these memory tests have one
> +fundamental, unfixable design flaw:  they are based on the assumption
> +that memory errors can be found by writing to and reading from memory.
> +Unfortunaltely, this is only true for the relatively harmless, usually
   ^Unfortunately

> +static errors like shorts between data or address lines, unconnected
> +pins, etc.  All the really nasty errors which will first turn your
> +hair grey, only to make you tear it out later, are dynamical errors,
> +which usually happen not with simple read or write cycles on the bus,
> +but when performing back-to-back data transfers in burst mode.  Such
> +accesses usually happen only for certain DMA operations, or for heavy
> +cache use (instruction fetching, cache flushing).  So far I am not
> +aware of any freely available code that implements a generic, and
> +efficient, memory test like that.  The best known test case to stress
> +a system like that is to boot Linux with root file system mounted over
> +NFS, and then build some larger software package natively (say,
> +compile a Linux kernel on the system) - this will cause enough context
> +switches, network traffic (and thus DMA transfers from the network
> +controller), varying RAM use, etc. to trigger any weak spots in this
> +area.
> +
> +Note: An attempt was made once to implement such a test to catch
> +memory problems on a specific board.  The code is pretty much board
> +specific (for example, it includes setting specific GPIO signals to
> +provide triggers for an attached logic analyzer), but you can get an
> +idea how it works: see "examples/standalone/test_burst*".
> +
> +Note 2: Ironically enough, the "test_burst" did not catch any RAM
> +errors, not a single one ever.  The problems this code was supposed
> +to catch did not happen when accessing the RAM, but when reading from
> +NOR flash.
> diff --git a/doc/feature-removal-schedule.txt b/doc/feature-removal-schedule.txt
> index e04ba2d..d9a0cc2 100644
> --- a/doc/feature-removal-schedule.txt
> +++ b/doc/feature-removal-schedule.txt
> @@ -7,6 +7,23 @@ file.
>  
>  ---------------------------
>  
> +What:	Remove CONFIG_CMD_MEMTEST from default list
> +When:	Release v2013.07
> +
> +Why:	The "mtest" command is of little practical use (if any), and
> +	experience has shown that a large number of board configu-
> +	rations define useless or even dangerous start and end
> +	addresses.  If not even the board maintainers are able to
> +	figure out which memory range can be reliably tested, how can
> +	we expect such from the end users?  As this problem comes up
> +	repeatedly, we rather do not enable this command by default,
> +	so only people who know what they are doing will be confronted
> +	with it.
> +
> +Who:	Wolfgang Denk <wd at denx.de>
> +
> +---------------------------
> +
>  What:	Users of the legacy miiphy_* code
>  When:	undetermined
>  
> diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
> index 0930781..53a2f05 100644
> --- a/include/config_cmd_all.h
> +++ b/include/config_cmd_all.h
> @@ -57,7 +57,8 @@
>  #define CONFIG_CMD_LOADB	/* loadb			*/
>  #define CONFIG_CMD_LOADS	/* loads			*/
>  #define CONFIG_CMD_MEMINFO	/* meminfo			*/
> -#define CONFIG_CMD_MEMORY	/* md mm nm mw cp cmp crc base loop mtest */
> +#define CONFIG_CMD_MEMORY	/* md mm nm mw cp cmp crc base loop */
> +#define CONFIG_CMD_MEMTEST	/* mtest			*/
>  #define CONFIG_CMD_MFSL		/* FSL support for Microblaze	*/
>  #define CONFIG_CMD_MII		/* MII support			*/
>  #define CONFIG_CMD_MISC		/* Misc functions like sleep etc*/
> diff --git a/include/config_cmd_default.h b/include/config_cmd_default.h
> index 6e3903c..a521103 100644
> --- a/include/config_cmd_default.h
> +++ b/include/config_cmd_default.h
> @@ -30,7 +30,8 @@
>  #endif
>  #define CONFIG_CMD_LOADB	/* loadb			*/
>  #define CONFIG_CMD_LOADS	/* loads			*/
> -#define CONFIG_CMD_MEMORY	/* md mm nm mw cp cmp crc base loop mtest */
> +#define CONFIG_CMD_MEMORY	/* md mm nm mw cp cmp crc base loop */
> +#define CONFIG_CMD_MEMTEST	/* mtest			*/
>  #define CONFIG_CMD_MISC		/* Misc functions like sleep etc*/
>  #define CONFIG_CMD_NET		/* bootp, tftpboot, rarpboot	*/
>  #define CONFIG_CMD_NFS		/* NFS support			*/
> -- 
> 1.7.11.7
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot


More information about the U-Boot mailing list