[PATCH 0/7 v2] x86: memtest update

[PATCH 0/7 v2] x86: memtest update

[Andreas Herrmann]

Second try to cleanup the memtest code a little bit.
Addressing the issues with the first version of this patch set.

Please apply.


Thanks,

Andreas

[PATCH 1/7] x86: memtest: reuse test patterns when memtest parameter exceeds number of available patterns

[Andreas Herrmann]

Impact: fix unexpected behaviour when pattern number is out of range

Current implementation provides 4 patterns for memtest. The code doesn't
check whether the memtest parameter value exceeds the maximum pattern number.

Instead the memtest code pretends to test with non-existing patterns, e.g.
when booting with memtest=10 I've observed the following

  ...
  early_memtest: pattern num 10
  0000001000 - 0000006000 pattern 0
  ...
  0000001000 - 0000006000 pattern 1
  ...
  0000001000 - 0000006000 pattern 2
  ...
  0000001000 - 0000006000 pattern 3
  ...
  0000001000 - 0000006000 pattern 4
  ...
  0000001000 - 0000006000 pattern 5
  ...
  0000001000 - 0000006000 pattern 6
  ...
  0000001000 - 0000006000 pattern 7
  ...
  0000001000 - 0000006000 pattern 8
  ...
  0000001000 - 0000006000 pattern 9
  ...

But in fact Linux didn't test anything for patterns > 4 as the default
case in memtest() is to leave the function.

I suggest to use the memtest parameter as the number of tests to be
performed and to re-iterate over all existing patterns.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index 9cab18b..00b8bdc 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -9,6 +9,8 @@
 
 #include <asm/e820.h>
 
+#define _MAX_MEM_PATTERNS 4
+
 static void __init memtest(unsigned long start_phys, unsigned long size,
 				 unsigned pattern)
 {
@@ -21,6 +23,8 @@ static void __init memtest(unsigned long start_phys, unsigned long size,
 	unsigned long count;
 	unsigned long incr;
 
+	pattern = pattern % _MAX_MEM_PATTERNS;
+
 	switch (pattern) {
 	case 0:
 		val = 0UL;
@@ -110,8 +114,9 @@ void __init early_memtest(unsigned long start, unsigned long end)
 				t_size = end - t_start;
 
 			printk(KERN_CONT "\n  %010llx - %010llx pattern %d",
-				(unsigned long long)t_start,
-				(unsigned long long)t_start + t_size, pattern);
+			       (unsigned long long)t_start,
+			       (unsigned long long)t_start + t_size,
+			       pattern % _MAX_MEM_PATTERNS);
 
 			memtest(t_start, t_size, pattern);
 
-- 
1.6.1.3

[PATCH 2/7] x86: memtest: introduce array to select memtest patterns

[Andreas Herrmann]

Impact: code cleanup

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |   65 +++++++++++++++++-------------------------------
 1 files changed, 23 insertions(+), 42 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index 00b8bdc..827f940 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -9,48 +9,25 @@
 
 #include <asm/e820.h>
 
-#define _MAX_MEM_PATTERNS 4
+static u64 patterns[] __initdata = {
+	0,
+	0xffffffffffffffffULL,
+	0x5555555555555555ULL,
+	0xaaaaaaaaaaaaaaaaULL,
+};
 
 static void __init memtest(unsigned long start_phys, unsigned long size,
-				 unsigned pattern)
+			   u64 pattern)
 {
 	unsigned long i;
-	unsigned long *start;
+	u64 *start;
 	unsigned long start_bad;
 	unsigned long last_bad;
-	unsigned long val;
 	unsigned long start_phys_aligned;
 	unsigned long count;
 	unsigned long incr;
 
-	pattern = pattern % _MAX_MEM_PATTERNS;
-
-	switch (pattern) {
-	case 0:
-		val = 0UL;
-		break;
-	case 1:
-		val = -1UL;
-		break;
-	case 2:
-#ifdef CONFIG_X86_64
-		val = 0x5555555555555555UL;
-#else
-		val = 0x55555555UL;
-#endif
-		break;
-	case 3:
-#ifdef CONFIG_X86_64
-		val = 0xaaaaaaaaaaaaaaaaUL;
-#else
-		val = 0xaaaaaaaaUL;
-#endif
-		break;
-	default:
-		return;
-	}
-
-	incr = sizeof(unsigned long);
+	incr = sizeof(pattern);
 	start_phys_aligned = ALIGN(start_phys, incr);
 	count = (size - (start_phys_aligned - start_phys))/incr;
 	start = __va(start_phys_aligned);
@@ -58,15 +35,16 @@ static void __init memtest(unsigned long start_phys, unsigned long size,
 	last_bad = 0;
 
 	for (i = 0; i < count; i++)
-		start[i] = val;
+		start[i] = pattern;
 	for (i = 0; i < count; i++, start++, start_phys_aligned += incr) {
-		if (*start != val) {
+		if (*start != pattern) {
 			if (start_phys_aligned == last_bad + incr) {
 				last_bad += incr;
 			} else {
 				if (start_bad) {
-					printk(KERN_CONT "\n  %016lx bad mem addr %010lx - %010lx reserved",
-						val, start_bad, last_bad + incr);
+					printk(KERN_CONT "\n  %016llx bad mem addr %010lx - %010lx reserved",
+					       (unsigned long long) pattern,
+					       start_bad, last_bad + incr);
 					reserve_early(start_bad, last_bad + incr, "BAD RAM");
 				}
 				start_bad = last_bad = start_phys_aligned;
@@ -74,8 +52,9 @@ static void __init memtest(unsigned long start_phys, unsigned long size,
 		}
 	}
 	if (start_bad) {
-		printk(KERN_CONT "\n  %016lx bad mem addr %010lx - %010lx reserved",
-			val, start_bad, last_bad + incr);
+		printk(KERN_CONT "\n  %016llx bad mem addr %010lx - %010lx reserved",
+		       (unsigned long long) pattern, start_bad,
+		       last_bad + incr);
 		reserve_early(start_bad, last_bad + incr, "BAD RAM");
 	}
 }
@@ -95,13 +74,16 @@ early_param("memtest", parse_memtest);
 void __init early_memtest(unsigned long start, unsigned long end)
 {
 	u64 t_start, t_size;
-	unsigned pattern;
+	unsigned int i;
+	u64 pattern;
 
 	if (!memtest_pattern)
 		return;
 
 	printk(KERN_INFO "early_memtest: pattern num %d", memtest_pattern);
-	for (pattern = 0; pattern < memtest_pattern; pattern++) {
+	for (i = 0; i < memtest_pattern; i++) {
+		unsigned int idx = i % ARRAY_SIZE(patterns);
+		pattern = patterns[idx];
 		t_start = start;
 		t_size = 0;
 		while (t_start < end) {
@@ -115,8 +97,7 @@ void __init early_memtest(unsigned long start, unsigned long end)
 
 			printk(KERN_CONT "\n  %010llx - %010llx pattern %d",
 			       (unsigned long long)t_start,
-			       (unsigned long long)t_start + t_size,
-			       pattern % _MAX_MEM_PATTERNS);
+			       (unsigned long long)t_start + t_size, idx);
 
 			memtest(t_start, t_size, pattern);
 
-- 
1.6.1.3

[PATCH 3/7] x86: memtest: cleanup memtest function

[Andreas Herrmann]

Impact: code cleanup

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |   37 +++++++++++++++++++------------------
 1 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index 827f940..01a72d6 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -16,6 +16,15 @@ static u64 patterns[] __initdata = {
 	0xaaaaaaaaaaaaaaaaULL,
 };
 
+static void __init reserve_bad_mem(u64 pattern, unsigned long start_bad,
+			    unsigned long end_bad)
+{
+	printk(KERN_CONT "\n  %016llx bad mem addr "
+	       "%010lx - %010lx reserved",
+	       (unsigned long long) pattern, start_bad, end_bad);
+	reserve_early(start_bad, end_bad, "BAD RAM");
+}
+
 static void __init memtest(unsigned long start_phys, unsigned long size,
 			   u64 pattern)
 {
@@ -37,26 +46,18 @@ static void __init memtest(unsigned long start_phys, unsigned long size,
 	for (i = 0; i < count; i++)
 		start[i] = pattern;
 	for (i = 0; i < count; i++, start++, start_phys_aligned += incr) {
-		if (*start != pattern) {
-			if (start_phys_aligned == last_bad + incr) {
-				last_bad += incr;
-			} else {
-				if (start_bad) {
-					printk(KERN_CONT "\n  %016llx bad mem addr %010lx - %010lx reserved",
-					       (unsigned long long) pattern,
-					       start_bad, last_bad + incr);
-					reserve_early(start_bad, last_bad + incr, "BAD RAM");
-				}
-				start_bad = last_bad = start_phys_aligned;
-			}
+		if (*start == pattern)
+			continue;
+		if (start_phys_aligned == last_bad + incr) {
+			last_bad += incr;
+			continue;
 		}
+		if (start_bad)
+			reserve_bad_mem(pattern, start_bad, last_bad + incr);
+		start_bad = last_bad = start_phys_aligned;
 	}
-	if (start_bad) {
-		printk(KERN_CONT "\n  %016llx bad mem addr %010lx - %010lx reserved",
-		       (unsigned long long) pattern, start_bad,
-		       last_bad + incr);
-		reserve_early(start_bad, last_bad + incr, "BAD RAM");
-	}
+	if (start_bad)
+		reserve_bad_mem(pattern, start_bad, last_bad + incr);
 }
 
 /* default is disabled */
-- 
1.6.1.3

[PATCH 4/7] x86: memtest: adapt log messages

[Andreas Herrmann]

* print test pattern instead of pattern number,
* show pattern as stored in memory,
* use proper priority flags,
* consistent use of u64 throughout the code

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |   36 ++++++++++++++++--------------------
 1 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index 01a72d6..3232397 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -16,25 +16,22 @@ static u64 patterns[] __initdata = {
 	0xaaaaaaaaaaaaaaaaULL,
 };
 
-static void __init reserve_bad_mem(u64 pattern, unsigned long start_bad,
-			    unsigned long end_bad)
+static void __init reserve_bad_mem(u64 pattern, u64 start_bad, u64 end_bad)
 {
-	printk(KERN_CONT "\n  %016llx bad mem addr "
-	       "%010lx - %010lx reserved",
-	       (unsigned long long) pattern, start_bad, end_bad);
+	printk(KERN_INFO "  %016llx bad mem addr %010llx - %010llx reserved\n",
+	       (unsigned long long) pattern,
+	       (unsigned long long) start_bad,
+	       (unsigned long long) end_bad);
 	reserve_early(start_bad, end_bad, "BAD RAM");
 }
 
-static void __init memtest(unsigned long start_phys, unsigned long size,
-			   u64 pattern)
+static void __init memtest(u64 pattern, u64 start_phys, u64 size)
 {
-	unsigned long i;
+	u64 i, count;
 	u64 *start;
-	unsigned long start_bad;
-	unsigned long last_bad;
-	unsigned long start_phys_aligned;
-	unsigned long count;
-	unsigned long incr;
+	u64 start_bad, last_bad;
+	u64 start_phys_aligned;
+	size_t incr;
 
 	incr = sizeof(pattern);
 	start_phys_aligned = ALIGN(start_phys, incr);
@@ -81,7 +78,7 @@ void __init early_memtest(unsigned long start, unsigned long end)
 	if (!memtest_pattern)
 		return;
 
-	printk(KERN_INFO "early_memtest: pattern num %d", memtest_pattern);
+	printk(KERN_INFO "early_memtest: # of tests: %d\n", memtest_pattern);
 	for (i = 0; i < memtest_pattern; i++) {
 		unsigned int idx = i % ARRAY_SIZE(patterns);
 		pattern = patterns[idx];
@@ -96,14 +93,13 @@ void __init early_memtest(unsigned long start, unsigned long end)
 			if (t_start + t_size > end)
 				t_size = end - t_start;
 
-			printk(KERN_CONT "\n  %010llx - %010llx pattern %d",
-			       (unsigned long long)t_start,
-			       (unsigned long long)t_start + t_size, idx);
-
-			memtest(t_start, t_size, pattern);
+			printk(KERN_INFO "  %010llx - %010llx pattern %016llx\n",
+			       (unsigned long long) t_start,
+			       (unsigned long long) t_start + t_size,
+			       (unsigned long long) cpu_to_be64(pattern));
+			memtest(pattern, t_start, t_size);
 
 			t_start += t_size;
 		}
 	}
-	printk(KERN_CONT "\n");
 }
-- 
1.6.1.3

[PATCH 5/7] x86: memtest: wipe out test pattern from memory

[Andreas Herrmann]

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |   56 ++++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index 3232397..9c52ef1 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -57,6 +57,29 @@ static void __init memtest(u64 pattern, u64 start_phys, u64 size)
 		reserve_bad_mem(pattern, start_bad, last_bad + incr);
 }
 
+static void __init do_one_pass(u64 pattern, u64 start, u64 end)
+{
+	u64 size = 0;
+
+	while (start < end) {
+		start = find_e820_area_size(start, &size, 1);
+
+		/* done ? */
+		if (start >= end)
+			break;
+		if (start + size > end)
+			size = end - start;
+
+		printk(KERN_INFO "  %010llx - %010llx pattern %016llx\n",
+		       (unsigned long long) start,
+		       (unsigned long long) start + size,
+		       (unsigned long long) cpu_to_be64(pattern));
+		memtest(pattern, start, size);
+
+		start += size;
+	}
+}
+
 /* default is disabled */
 static int memtest_pattern __initdata;
 
@@ -71,35 +94,22 @@ early_param("memtest", parse_memtest);
 
 void __init early_memtest(unsigned long start, unsigned long end)
 {
-	u64 t_start, t_size;
 	unsigned int i;
-	u64 pattern;
+	unsigned int idx = 0;
 
 	if (!memtest_pattern)
 		return;
 
 	printk(KERN_INFO "early_memtest: # of tests: %d\n", memtest_pattern);
 	for (i = 0; i < memtest_pattern; i++) {
-		unsigned int idx = i % ARRAY_SIZE(patterns);
-		pattern = patterns[idx];
-		t_start = start;
-		t_size = 0;
-		while (t_start < end) {
-			t_start = find_e820_area_size(t_start, &t_size, 1);
-
-			/* done ? */
-			if (t_start >= end)
-				break;
-			if (t_start + t_size > end)
-				t_size = end - t_start;
-
-			printk(KERN_INFO "  %010llx - %010llx pattern %016llx\n",
-			       (unsigned long long) t_start,
-			       (unsigned long long) t_start + t_size,
-			       (unsigned long long) cpu_to_be64(pattern));
-			memtest(pattern, t_start, t_size);
-
-			t_start += t_size;
-		}
+		idx = i % ARRAY_SIZE(patterns);
+		do_one_pass(patterns[idx], start, end);
+	}
+
+	if (idx > 0) {
+		printk(KERN_INFO "early_memtest: wipe out "
+		       "test pattern from memory\n");
+		/* additional test with pattern 0 will do this */
+		do_one_pass(0, start, end);
 	}
 }
-- 
1.6.1.3

[PATCH 6/7] x86: update description for memtest boot parameter

[Andreas Herrmann]

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 Documentation/kernel-parameters.txt |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 319785b..8cc4518 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1308,8 +1308,13 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	memtest=	[KNL,X86] Enable memtest
 			Format: <integer>
-			range: 0,4 : pattern number
 			default : 0 <disable>
+			Specifies the number of memtest passes to be
+			performed. Each pass selects another test
+			pattern from a given set of patterns. Memtest
+			fills the memory with this pattern, validates
+			memory contents and reserves bad memory
+			regions that are detected.
 
 	meye.*=		[HW] Set MotionEye Camera parameters
 			See Documentation/video4linux/meye.txt.
-- 
1.6.1.3

[PATCH 7/7] x86: memtest: add additional (regular) test patterns

[Andreas Herrmann]

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index 9c52ef1..0bcd788 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -14,6 +14,19 @@ static u64 patterns[] __initdata = {
 	0xffffffffffffffffULL,
 	0x5555555555555555ULL,
 	0xaaaaaaaaaaaaaaaaULL,
+	0x1111111111111111ULL,
+	0x2222222222222222ULL,
+	0x4444444444444444ULL,
+	0x8888888888888888ULL,
+	0x3333333333333333ULL,
+	0x6666666666666666ULL,
+	0x9999999999999999ULL,
+	0xccccccccccccccccULL,
+	0x7777777777777777ULL,
+	0xbbbbbbbbbbbbbbbbULL,
+	0xddddddddddddddddULL,
+	0xeeeeeeeeeeeeeeeeULL,
+	0x7a6c7258554e494cULL, /* yeah ;-) */
 };
 
 static void __init reserve_bad_mem(u64 pattern, u64 start_bad, u64 end_bad)
-- 
1.6.1.3

Re: [PATCH 7/7] x86: memtest: add additional (regular) test patterns

[Ingo Molnar]

* Andreas Herrmann <andreas.herrmann3@amd.com> wrote:

> 
> Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
> ---
>  arch/x86/mm/memtest.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
> index 9c52ef1..0bcd788 100644
> --- a/arch/x86/mm/memtest.c
> +++ b/arch/x86/mm/memtest.c
> @@ -14,6 +14,19 @@ static u64 patterns[] __initdata = {
>  	0xffffffffffffffffULL,
>  	0x5555555555555555ULL,
>  	0xaaaaaaaaaaaaaaaaULL,
> +	0x1111111111111111ULL,
> +	0x2222222222222222ULL,
> +	0x4444444444444444ULL,
> +	0x8888888888888888ULL,
> +	0x3333333333333333ULL,
> +	0x6666666666666666ULL,
> +	0x9999999999999999ULL,
> +	0xccccccccccccccccULL,
> +	0x7777777777777777ULL,
> +	0xbbbbbbbbbbbbbbbbULL,
> +	0xddddddddddddddddULL,
> +	0xeeeeeeeeeeeeeeeeULL,
> +	0x7a6c7258554e494cULL, /* yeah ;-) */

it indeed does ;-)

	Ingo

Re: [PATCH 0/7 v2] x86: memtest update

[Ingo Molnar]

* Andreas Herrmann <andreas.herrmann3@amd.com> wrote:

> Second try to cleanup the memtest code a little bit. 
> Addressing the issues with the first version of this patch 
> set.
> 
> Please apply.

Applied to tip:x86/memtest, thanks Andreas!

	Ingo

[PATCH 0/7] x86: memtest update

[Andreas Herrmann]

Hi,

Following patches contain minor fixes and changes of memtest code.
Please apply.


Thanks,

Andreas

[PATCH 3/7] x86: memtest: cleanup memtest function

[Andreas Herrmann]

Impact: code cleanup

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/mm/memtest.c |   36 +++++++++++++++++++-----------------
 1 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/x86/mm/memtest.c b/arch/x86/mm/memtest.c
index a8a6c51..82321d9 100644
--- a/arch/x86/mm/memtest.c
+++ b/arch/x86/mm/memtest.c
@@ -21,6 +21,15 @@ static unsigned long patterns[] = {
 #endif
 };
 
+static void reserve_bad_mem(unsigned long pattern, unsigned long start_bad,
+			    unsigned long end_bad)
+{
+	printk(KERN_CONT "\n  %016lx bad mem addr "
+	       "%010lx - %010lx reserved",
+	       pattern, start_bad, end_bad);
+	reserve_early(start_bad, end_bad, "BAD RAM");
+}
+
 static void __init memtest(unsigned long start_phys, unsigned long size,
 			   unsigned long pattern)
 {
@@ -42,25 +51,18 @@ static void __init memtest(unsigned long start_phys, unsigned long size,
 	for (i = 0; i < count; i++)
 		start[i] = pattern;
 	for (i = 0; i < count; i++, start++, start_phys_aligned += incr) {
-		if (*start != pattern) {
-			if (start_phys_aligned == last_bad + incr) {
-				last_bad += incr;
-			} else {
-				if (start_bad) {
-					printk(KERN_CONT "\n  %016lx bad mem addr %010lx - %010lx reserved",
-					       pattern, start_bad,
-					       last_bad + incr);
-					reserve_early(start_bad, last_bad + incr, "BAD RAM");
-				}
-				start_bad = last_bad = start_phys_aligned;
-			}
+		if (*start == pattern)
+			continue;
+		if (start_phys_aligned == last_bad + incr) {
+			last_bad += incr;
+			continue;
 		}
+		if (start_bad)
+			reserve_bad_mem(pattern, start_bad, last_bad + incr);
+		start_bad = last_bad = start_phys_aligned;
 	}
-	if (start_bad) {
-		printk(KERN_CONT "\n  %016lx bad mem addr %010lx - %010lx reserved",
-		       pattern, start_bad, last_bad + incr);
-		reserve_early(start_bad, last_bad + incr, "BAD RAM");
-	}
+	if (start_bad)
+		reserve_bad_mem(pattern, start_bad, last_bad + incr);
 }
 
 /* default is disabled */
-- 
1.6.1.2