x86: mtrr_cleanup: print out correct type

PATCH In review Yinghai Lu 3 participants 11 messages Oct 4, 2008 – Oct 5, 2008 x86

lore

Share & tools

lore

Yinghai Lu

author

2008-10-04 21:51 UTC #0 lore raw

PATCH 1/3 x86: mtrr_cleanup: print out correct type

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/kernel/cpu/mtrr/main.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/mtrr/main.c +3 -1

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
@@ -1273,12 +1273,14 @@ static int __init mtrr_cleanup(unsigned
 		size_base = to_size_factor(size_base, &size_factor),
 		start_base = range_state[i].base_pfn << (PAGE_SHIFT - 10);
 		start_base = to_size_factor(start_base, &start_factor),
+		type = range_state[i].type;
 
 		printk(KERN_DEBUG "reg %d, base: %ld%cB, range: %ld%cB, type %s\n",
 			i, start_base, start_factor,
 			size_base, size_factor,
 			(type == MTRR_TYPE_UNCACHABLE) ? "UC" :
-			    ((type == MTRR_TYPE_WRBACK) ? "WB" : "Other")
+			    ((type == MTRR_TYPE_WRPROT) ? "WPROT" :
+			     ((type == MTRR_TYPE_WRBACK) ? "WB" : "Other"))
 			);
 	}

Yinghai Lu

author

2008-10-04 21:51 UTC #1 lore raw

PATCH 2/3 x86: mtrr_cleanup: first 1M should be coverred in var mtrrs

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/kernel/cpu/mtrr/main.c |    9 +++++++++
 1 file changed, 9 insertions(+)

arch/x86/kernel/cpu/mtrr/main.c +9 -0

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
@@ -1292,6 +1292,15 @@ static int __init mtrr_cleanup(unsigned
 			(mtrr_tom2 >> PAGE_SHIFT) - extra_remove_base;
 	nr_range = x86_get_mtrr_mem_range(range, 0, extra_remove_base,
 					  extra_remove_size);
+	/*
+	 * [0, 1M) should always be coverred by var mtrr with WB
+	 * and fixed mtrrs should take effective before var mtrr for it
+	 */
+	nr_range = add_range_with_merge(range, nr_range, 0,
+					(1ULL<<(20 - PAGE_SHIFT)) - 1);
+	/* sort the ranges */
+	sort(range, nr_range, sizeof(struct res_range), cmp_range, NULL);
+
 	range_sums = sum_ranges(range, nr_range);
 	printk(KERN_INFO "total RAM coverred: %ldM\n",
 	       range_sums >> (20 - PAGE_SHIFT));

H. Peter Anvin

reviewer

2008-10-05 00:46 UTC #2 lore raw

Re: [PATCH 2/3] x86: mtrr_cleanup: first 1M should be coverred in var mtrrs

arch/x86/kernel/cpu/mtrr/main.c static int __init mtrr_cleanup(unsigned · line 1303

Quoted 9 lines · Yinghai Lu

arch/x86/kernel/cpu/mtrr/main.c +9 -0

+	/*
+	 * [0, 1M) should always be coverred by var mtrr with WB
+	 * and fixed mtrrs should take effective before var mtrr for it
+	 */
+	nr_range = add_range_with_merge(range, nr_range, 0,
+					(1ULL<<(20 - PAGE_SHIFT)) - 1);
+	/* sort the ranges */
+	sort(range, nr_range, sizeof(struct res_range), cmp_range, NULL);
+

The first 1 MB is a total don't care for the variable MTRRs -- they don't have to be covered *or* uncovered, since the entire first 1 MB is addressed by fixed MTRRs.

In practice, it is *likely* that you're going to want to merge it with a WB MTRR, but with various vendors doing all kinds of strange things on EFI-damaged platforms, it may not always be that way.

-hpa

Yinghai Lu

2008-10-05 01:49 UTC #3 lore raw

Re: [PATCH 2/3] x86: mtrr_cleanup: first 1M should be coverred in var mtrrs

Quoted 12 lines · H. Peter Anvin

arch/x86/kernel/cpu/mtrr/main.c static int __init mtrr_cleanup(unsigned · line 1303

Quoted 10 lines · Yinghai Lu

arch/x86/kernel/cpu/mtrr/main.c +9 -0

+       /*
+        * [0, 1M) should always be coverred by var mtrr with WB
+        * and fixed mtrrs should take effective before var mtrr for it
+        */
+       nr_range = add_range_with_merge(range, nr_range, 0,
+                                       (1ULL<<(20 - PAGE_SHIFT)) - 1);
+       /* sort the ranges */
+       sort(range, nr_range, sizeof(struct res_range), cmp_range, NULL);
+

Quoted 3 lines

The first 1 MB is a total don't care for the variable MTRRs -- they don't have to be covered *or* uncovered, since the entire first 1 MB is addressed by fixed MTRRs.

so it is safe to put it in WB, and can be spare some regs because it start from base 0.

Quoted 4 lines

In practice, it is *likely* that you're going to want to merge it with a WB MTRR, but with various vendors doing all kinds of strange things on EFI-damaged platforms, it may not always be that way.

EFI will not use fixed-mtrr for first 1M? and not cover first 1M in var mtrr with WB? that will run out mtrr regs..

H. Peter Anvin

reviewer

2008-10-05 01:54 UTC #4 lore raw

Re: [PATCH 2/3] x86: mtrr_cleanup: first 1M should be coverred in var mtrrs

Quoted 6 lines · Yinghai Lu

Quoted 3 lines
The first 1 MB is a total don't care for the variable MTRRs -- they don't have to be covered *or* uncovered, since the entire first 1 MB is addressed by fixed MTRRs.
so it is safe to put it in WB, and can be spare some regs because it start from base 0.

Yes, my point was that you can make those whatever you want. It doesn't have to be WB; it can be any value.

Quoted 6 lines

Quoted 3 lines
In practice, it is *likely* that you're going to want to merge it with a WB MTRR, but with various vendors doing all kinds of strange things on EFI-damaged platforms, it may not always be that way.
EFI will not use fixed-mtrr for first 1M? and not cover first 1M in var mtrr with WB? that will run out mtrr regs..

There are some systems which, apparently, don't have RAM there. Don't ask me how or why.

-hpa

Yinghai Lu

2008-10-05 01:59 UTC #5 lore raw

Re: [PATCH 2/3] x86: mtrr_cleanup: first 1M should be coverred in var mtrrs

Quoted 2 lines · H. Peter Anvin

There are some systems which, apparently, don't have RAM there. Don't ask me how or why.

very old system or
very new system only have super big cache?

H. Peter Anvin

reviewer

2008-10-05 02:07 UTC #6 lore raw

Re: [PATCH 2/3] x86: mtrr_cleanup: first 1M should be coverred in var mtrrs

Quoted 8 lines · Yinghai Lu

Quoted 2 lines · H. Peter Anvin
There are some systems which, apparently, don't have RAM there. Don't ask me how or why.
very old system or
very new system only have super big cache?

Very new system. That's all I know. Don't know if they actually shipped, but it caused a lot of problems and I recommended to the OEM that they fix that.

-hpa

Yinghai Lu

author

2008-10-04 21:52 UTC #7 lore raw

PATCH 3/3 x86: mtrr_cleanup: treat WRPROT as UNCACHEABLE

In reply to Yinghai Lu

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/kernel/cpu/mtrr/main.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/mtrr/main.c +4 -1

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
@@ -759,7 +759,8 @@ x86_get_mtrr_mem_range(struct res_range
 	/* take out UC ranges */
 	for (i = 0; i < num_var_ranges; i++) {
 		type = range_state[i].type;
-		if (type != MTRR_TYPE_UNCACHABLE)
+		if (type != MTRR_TYPE_UNCACHABLE &&
+		    type != MTRR_TYPE_WRPROT)
 			continue;
 		size = range_state[i].size_pfn;
 		if (!size)

@@ -1248,6 +1249,8 @@ static int __init mtrr_cleanup(unsigned
 			continue;
 		if (!size)
 			type = MTRR_NUM_TYPES;
+		if (type == MTRR_TYPE_WRPROT)
+			type = MTRR_TYPE_UNCACHABLE;
 		num[type]++;
 	}

H. Peter Anvin

reviewer

2008-10-05 00:43 UTC #8 lore raw

In reply to Yinghai Lu

Quoted 9 lines · Yinghai Lu

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
arch/x86/kernel/cpu/mtrr/main.c +0 -0
@@ -1273,12 +1273,14 @@ static int __init mtrr_cleanup(unsigned
 		size_base = to_size_factor(size_base, &size_factor),
 		start_base = range_state[i].base_pfn << (PAGE_SHIFT - 10);
 		start_base = to_size_factor(start_base, &start_factor),

arch/x86/kernel/cpu/mtrr/main.c static int __init mtrr_cleanup(unsigned · line 1284

Quoted 10 lines

arch/x86/kernel/cpu/mtrr/main.c +3 -1

+		type = range_state[i].type;
 
 		printk(KERN_DEBUG "reg %d, base: %ld%cB, range: %ld%cB, type %s\n",
 			i, start_base, start_factor,
 			size_base, size_factor,
 			(type == MTRR_TYPE_UNCACHABLE) ? "UC" :
-			    ((type == MTRR_TYPE_WRBACK) ? "WB" : "Other")
+			    ((type == MTRR_TYPE_WRPROT) ? "WPROT" :
+			     ((type == MTRR_TYPE_WRBACK) ? "WB" : "Other"))
 			);

If you're going to abbreviate, please use the standard abbreviations as documented in the manuals. The abbreviation for write protect is WP.

-hpa

Yinghai Lu

2008-10-05 01:42 UTC #9 lore raw

Quoted 16 lines · H. Peter Anvin

Quoted 15 lines · Yinghai Lu

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================

arch/x86/kernel/cpu/mtrr/main.c +1 -0

@@ -1273,12 +1273,14 @@ static int __init mtrr_cleanup(unsigned
               size_base = to_size_factor(size_base, &size_factor),
               start_base = range_state[i].base_pfn << (PAGE_SHIFT - 10);
               start_base = to_size_factor(start_base, &start_factor),
+               type = range_state[i].type;
                 printk(KERN_DEBUG "reg %d, base: %ld%cB, range: %ld%cB,
 type %s\n",
                       i, start_base, start_factor,
                       size_base, size_factor,
                       (type == MTRR_TYPE_UNCACHABLE) ? "UC" :

Quoted 9 lines

arch/x86/kernel/cpu/mtrr/main.c static int __init mtrr_cleanup(unsigned · line 1284
Quoted 5 lines
arch/x86/kernel/cpu/mtrr/main.c +2 -1
-                           ((type == MTRR_TYPE_WRBACK) ? "WB" : "Other")
+                           ((type == MTRR_TYPE_WRPROT) ? "WPROT" :
+                            ((type == MTRR_TYPE_WRBACK) ? "WB" :
 "Other"))
                       );
If you're going to abbreviate, please use the standard abbreviations as documented in the manuals. The abbreviation for write protect is WP.

ok, will use WP.

H. Peter Anvin

reviewer

2008-10-05 03:11 UTC #10 lore raw

In reply to Yinghai Lu

Patch series applied to tip:x86/mtrr, thanks!

-hpa