x86.git: mtrr trimming removes all memory under kvm

x86.git: mtrr trimming removes all memory under kvm

[Jeremy Fitzhardinge]

When booting a current x86.git kernel under kvm, I get this:

(qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
early_ioremap_init()
BIOS-provided physical RAM map:
 BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
 BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
 BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
 BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
 BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
 BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
console [earlyser0] enabled
0MB HIGHMEM available.
511MB LOWMEM available.
Scan SMP from c0000000 for 1024 bytes.
Scan SMP from c009fc00 for 1024 bytes.
Scan SMP from c00f0000 for 65536 bytes.
Scan SMP from c009fc00 for 1024 bytes.
***************
**** WARNING: likely BIOS bug
**** MTRRs don't cover all of memory, trimmed 131056 pages
***************
update e820 for mtrr
modified physical RAM map:
 modified: 0000000000000000 - 000000001fff0000 (reserved)
 modified: 000000001fff0000 - 0000000020000000 (ACPI data)
 modified: 00000000fffc0000 - 0000000100000000 (reserved)
highmem size specified (0MB) is bigger than pages available (0MB)!.
0MB HIGHMEM available.
0MB LOWMEM available.
BUG: Int 6: CR2 00000000
     EDI c055de40  ESI c04f7bec  EBP c04cff68  ESP c04cff4c
     EBX 00100000  EDX 00000006  ECX 0047efff  EAX 00000100
     err 00000000  EIP c04e5c7d   CS 00000060  flg 00010006
Stack: 00000000 c04f7bec c051eb0c c04cff70 c04e5cd1 c04cff80 c04da360 00000010
       c04f7bec c04cff98 c04da4e5 c042ac8d 00000000 c042ac70 00000000 c04cffd0
       c04da7c0 c04cffe8 c04cb680 c051e920 c051e8e0 00000800 00099800 c04c1000
��Pid: 0, comm: swapper Not tainted 2.6.24-rc8 #1928
 [<c04e5c7d>] ? reserve_bootmem_core+0x1e/0x61
 [<c04e5cd1>] ? reserve_bootmem+0x11/0x13
 [<c04da360>] ? setup_bootmem_allocator+0x42/0x141
 [<c04da4e5>] ? setup_memory+0x86/0x8d
 [<c04da7c0>] ? setup_arch+0x28c/0x3c2
 [<c0128f4a>] ? printk+0x15/0x17
 [<c04d4689>] ? start_kernel+0x60/0x311
 =======================



I can well believe this is a qemu bug, but it used to work fine, so this 
is a regression.

    J

Re: x86.git: mtrr trimming removes all memory under kvm

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> When booting a current x86.git kernel under kvm, I get this:
> 
> (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 
> (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> early_ioremap_init()
> BIOS-provided physical RAM map:
> BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> console [earlyser0] enabled
> 0MB HIGHMEM available.
> 511MB LOWMEM available.
> Scan SMP from c0000000 for 1024 bytes.
> Scan SMP from c009fc00 for 1024 bytes.
> Scan SMP from c00f0000 for 65536 bytes.
> Scan SMP from c009fc00 for 1024 bytes.
> ***************
> **** WARNING: likely BIOS bug
> **** MTRRs don't cover all of memory, trimmed 131056 pages
> ***************

Looks like the code doesn't check that the CPU *has* MTRRs...

	-hpa

Re: x86.git: mtrr trimming removes all memory under kvm

[Yinghai Lu]

On Thursday 24 January 2008 05:49:00 pm H. Peter Anvin wrote:
> Jeremy Fitzhardinge wrote:
> > When booting a current x86.git kernel under kvm, I get this:
> > 
> > (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 
> > (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> > early_ioremap_init()
> > BIOS-provided physical RAM map:
> > BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> > BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> > BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> > BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> > BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> > BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> > console [earlyser0] enabled
> > 0MB HIGHMEM available.
> > 511MB LOWMEM available.
> > Scan SMP from c0000000 for 1024 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > Scan SMP from c00f0000 for 65536 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > ***************
> > **** WARNING: likely BIOS bug
> > **** MTRRs don't cover all of memory, trimmed 131056 pages
> > ***************
> 
> Looks like the code doesn't check that the CPU *has* MTRRs...
> 

on 32 bit, max_pfn is supposed to be 0?

will look at it.

YH

Re: x86.git: mtrr trimming removes all memory under kvm

[Yinghai Lu]

On Thursday 24 January 2008 05:49:00 pm H. Peter Anvin wrote:
> Jeremy Fitzhardinge wrote:
> > When booting a current x86.git kernel under kvm, I get this:
> > 
> > (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 
> > (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> > early_ioremap_init()
> > BIOS-provided physical RAM map:
> > BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> > BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> > BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> > BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> > BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> > BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> > console [earlyser0] enabled
> > 0MB HIGHMEM available.
> > 511MB LOWMEM available.
> > Scan SMP from c0000000 for 1024 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > Scan SMP from c00f0000 for 65536 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > ***************
> > **** WARNING: likely BIOS bug
> > **** MTRRs don't cover all of memory, trimmed 131056 pages
> > ***************
> 
> Looks like the code doesn't check that the CPU *has* MTRRs...

please try this

[PATCH] x86: trim RAM need to check if mtrr is there

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index a1551d0..a0b6f55 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -646,9 +646,6 @@ static __init int amd_special_default_mtrr(unsigned long end_pfn)
 {
 	u32 l, h;
 
-	/* Doesn't apply to memory < 4GB */
-	if (end_pfn <= (0xffffffff >> PAGE_SHIFT))
-		return 0;
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return 0;
 	if (boot_cpu_data.x86 < 0xf || boot_cpu_data.x86 > 0x11)
@@ -682,6 +679,12 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
 	mtrr_type type;
 	u64 trim_start, trim_size;
 
+	/* Doesn't apply to memory < 4GB */
+	if (end_pfn <= (0xffffffffUL >> PAGE_SHIFT))
+		return 0;
+
+	if (!cpu_has_mtrr)
+		return 0;
 	/*
 	 * Make sure we only trim uncachable memory on machines that
 	 * support the Intel MTRR architecture:

Re: x86.git: mtrr trimming removes all memory under kvm

[Andi Kleen]

Yinghai Lu <Yinghai.Lu@Sun.COM> writes:
>  
> +	/* Doesn't apply to memory < 4GB */
> +	if (end_pfn <= (0xffffffffUL >> PAGE_SHIFT))
> +		return 0;

I don't think this makes sense to move outside the AMD workaround
because the normal MTRRs don't have an implicit hidden 4GB semantics.

-Andi

[PATCH] x86: trim ram need to check if mtrr is there v2

[Yinghai Lu]

[PATCH] x86: trim ram need to check if mtrr is there v2

> >Jeremy Fitzhardinge wrote:
> > When booting a current x86.git kernel under kvm, I get this:
> > 
> > (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 
> > (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> > early_ioremap_init()
> > BIOS-provided physical RAM map:
> > BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> > BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> > BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> > BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> > BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> > BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> > console [earlyser0] enabled
> > 0MB HIGHMEM available.
> > 511MB LOWMEM available.
> > Scan SMP from c0000000 for 1024 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > Scan SMP from c00f0000 for 65536 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > ***************
> > **** WARNING: likely BIOS bug
> > **** MTRRs don't cover all of memory, trimmed 131056 pages
> > ***************
> 
> H. Peter Anvin wrote:
> Looks like the code doesn't check that the CPU *has* MTRRs...

so check it mtrr is there, also check if mem less 4G and is AMD as early

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
@@ -642,13 +642,10 @@ early_param("disable_mtrr_trim", disable
 #define Tom2Enabled (1U << 21)
 #define Tom2ForceMemTypeWB (1U << 22)
 
-static __init int amd_special_default_mtrr(unsigned long end_pfn)
+static __init int amd_special_default_mtrr(void)
 {
 	u32 l, h;
 
-	/* Doesn't apply to memory < 4GB */
-	if (end_pfn <= (0xffffffff >> PAGE_SHIFT))
-		return 0;
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return 0;
 	if (boot_cpu_data.x86 < 0xf || boot_cpu_data.x86 > 0x11)
@@ -682,6 +679,13 @@ int __init mtrr_trim_uncached_memory(uns
 	mtrr_type type;
 	u64 trim_start, trim_size;
 
+	if (!cpu_has_mtrr)
+		return 0;
+
+	/* Doesn't apply to memory < 4GB */
+	if (end_pfn <= (0xffffffffUL >> PAGE_SHIFT))
+		return 0;
+
 	/*
 	 * Make sure we only trim uncachable memory on machines that
 	 * support the Intel MTRR architecture:
@@ -691,6 +695,9 @@ int __init mtrr_trim_uncached_memory(uns
 	if (!is_cpu(INTEL) || disable_mtrr_trim || def != MTRR_TYPE_UNCACHABLE)
 		return 0;
 
+	if (amd_special_default_mtrr())
+		return 0;
+
 	/* Find highest cached pfn */
 	for (i = 0; i < num_var_ranges; i++) {
 		mtrr_if->get(i, &base, &size, &type);
@@ -702,9 +709,6 @@ int __init mtrr_trim_uncached_memory(uns
 			highest_addr = base + size;
 	}
 
-	if (amd_special_default_mtrr(end_pfn))
-		return 0;
-
 	if ((highest_addr >> PAGE_SHIFT) < end_pfn) {
 		printk(KERN_WARNING "***************\n");
 		printk(KERN_WARNING "**** WARNING: likely BIOS bug\n");

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[H. Peter Anvin]

Yinghai Lu wrote:
>> H. Peter Anvin wrote:
>> Looks like the code doesn't check that the CPU *has* MTRRs...
> 
> so check it mtrr is there, also check if mem less 4G and is AMD as early

Why the check for < 4 GB?  The same thing applies to memory below the 4 
GB limit -- in fact, we've had a number of that kind of systems in the past.

	-hpa

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[Yinghai Lu]

On Jan 24, 2008 9:44 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
> >> H. Peter Anvin wrote:
> >> Looks like the code doesn't check that the CPU *has* MTRRs...
> >
> > so check it mtrr is there, also check if mem less 4G and is AMD as early
>
> Why the check for < 4 GB?  The same thing applies to memory below the 4
> GB limit -- in fact, we've had a number of that kind of systems in the past.

then we could remove that.

YH

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[Jeremy Fitzhardinge]

Yinghai Lu wrote:
> [PATCH] x86: trim ram need to check if mtrr is there v2
>
>   
>>> Jeremy Fitzhardinge wrote:
>>> When booting a current x86.git kernel under kvm, I get this:
>>>
>>> (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 
>>> (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
>>> early_ioremap_init()
>>> BIOS-provided physical RAM map:
>>> BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
>>> BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
>>> BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
>>> BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
>>> BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
>>> BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
>>> console [earlyser0] enabled
>>> 0MB HIGHMEM available.
>>> 511MB LOWMEM available.
>>> Scan SMP from c0000000 for 1024 bytes.
>>> Scan SMP from c009fc00 for 1024 bytes.
>>> Scan SMP from c00f0000 for 65536 bytes.
>>> Scan SMP from c009fc00 for 1024 bytes.
>>> ***************
>>> **** WARNING: likely BIOS bug
>>> **** MTRRs don't cover all of memory, trimmed 131056 pages
>>> ***************
>>>       
>> H. Peter Anvin wrote:
>> Looks like the code doesn't check that the CPU *has* MTRRs...
>>     
>
> so check it mtrr is there, also check if mem less 4G and is AMD as early
>   

Thanks, this gets me to usermode under kvm.

    J

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[Yinghai Lu]

On Jan 24, 2008 11:43 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
> Yinghai Lu wrote:
> > [PATCH] x86: trim ram need to check if mtrr is there v2
> >
> >
> >>> Jeremy Fitzhardinge wrote:
> >>> When booting a current x86.git kernel under kvm, I get this:
> >>>
> >>> (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925
> >>> (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> >>> early_ioremap_init()
> >>> BIOS-provided physical RAM map:
> >>> BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> >>> BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> >>> BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> >>> BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> >>> BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> >>> BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> >>> console [earlyser0] enabled
> >>> 0MB HIGHMEM available.
> >>> 511MB LOWMEM available.
> >>> Scan SMP from c0000000 for 1024 bytes.
> >>> Scan SMP from c009fc00 for 1024 bytes.
> >>> Scan SMP from c00f0000 for 65536 bytes.
> >>> Scan SMP from c009fc00 for 1024 bytes.
> >>> ***************
> >>> **** WARNING: likely BIOS bug
> >>> **** MTRRs don't cover all of memory, trimmed 131056 pages
> >>> ***************
> >>>
> >> H. Peter Anvin wrote:
> >> Looks like the code doesn't check that the CPU *has* MTRRs...
> >>
> >
> > so check it mtrr is there, also check if mem less 4G and is AMD as early
> >
>
> Thanks, this gets me to usermode under kvm.

i think the is 0xffffffff to 0xffffffffUL make the difference.

the cpu mode in KVM/qemu may cpu_has_mtrr, but doesn't really have
return mtrr msr with correct value.

is_cpu(INTEL) already make sure we have mtrr_if got assigned and cpu_has_mtrr.

may need to fix qemu instead...

Can you try post /proc/mtrr for your guest in kvm/qemu?

Thanks

YH

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[Yinghai Lu]

On Jan 25, 2008 12:13 AM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>
> On Jan 24, 2008 11:43 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> >
> > Yinghai Lu wrote:
> > > [PATCH] x86: trim ram need to check if mtrr is there v2
> > >
> > >
> > >>> Jeremy Fitzhardinge wrote:
> > >>> When booting a current x86.git kernel under kvm, I get this:
> > >>>
> > >>> (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925
> > >>> (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> > >>> early_ioremap_init()
> > >>> BIOS-provided physical RAM map:
> > >>> BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> > >>> BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> > >>> BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> > >>> BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> > >>> BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> > >>> BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> > >>> console [earlyser0] enabled
> > >>> 0MB HIGHMEM available.
> > >>> 511MB LOWMEM available.
> > >>> Scan SMP from c0000000 for 1024 bytes.
> > >>> Scan SMP from c009fc00 for 1024 bytes.
> > >>> Scan SMP from c00f0000 for 65536 bytes.
> > >>> Scan SMP from c009fc00 for 1024 bytes.
> > >>> ***************
> > >>> **** WARNING: likely BIOS bug
> > >>> **** MTRRs don't cover all of memory, trimmed 131056 pages
> > >>> ***************
> > >>>
> > >> H. Peter Anvin wrote:
> > >> Looks like the code doesn't check that the CPU *has* MTRRs...
> > >>
> > >
> > > so check it mtrr is there, also check if mem less 4G and is AMD as early
> > >
> >
> > Thanks, this gets me to usermode under kvm.
>
> i think the is 0xffffffff to 0xffffffffUL make the difference.
>
> the cpu mode in KVM/qemu may cpu_has_mtrr, but doesn't really have
> return mtrr msr with correct value.
>
> is_cpu(INTEL) already make sure we have mtrr_if got assigned and cpu_has_mtrr.
>
> may need to fix qemu instead...
>
> Can you try post /proc/mtrr for your guest in kvm/qemu?

jeremy,

can you try v3 patch?

Thanks

YH

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> is_cpu(INTEL) already make sure we have mtrr_if got assigned and 
> cpu_has_mtrr.
> 
> may need to fix qemu instead...

no ...

lets not do non-sensical trimming of RAM, ok? Emit a warning but never 
trim all of RAM and make the system unbootable. Trimmed RAM is something 
that users can pester board/BIOS vendors with. Non-booting kernels is 
something _we_ get pestered with ;-)

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v2

[H. Peter Anvin]

Ingo Molnar wrote:
> * Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> 
>> is_cpu(INTEL) already make sure we have mtrr_if got assigned and 
>> cpu_has_mtrr.
>>
>> may need to fix qemu instead...
> 
> no ...
> 
> lets not do non-sensical trimming of RAM, ok? Emit a warning but never 
> trim all of RAM and make the system unbootable. Trimmed RAM is something 
> that users can pester board/BIOS vendors with. Non-booting kernels is 
> something _we_ get pestered with ;-)
> 

*And* let's push a fix to Qemu/KVM as appropriate.

Last I checked Qemu never even turns caching on in %cr0, never mind gets 
the MTRRs right.  If it's advertising MTRRs, this is a problem.

	-hpa

[PATCH] x86: trim ram need to check if mtrr is there v3

[Yinghai Lu]

[PATCH] x86: trim ram need to check if mtrr is there v3

> >Jeremy Fitzhardinge wrote:
> > When booting a current x86.git kernel under kvm, I get this:
> > 
> > (qemu) Linux version 2.6.24-rc8 (jeremy@ezr) (gcc version 4.1.2 20070925 
> > (Red Hat 4.1.2-33)) #1928 SMP PREEMPT Thu Jan 24 17:09:04 PST 2008
> > early_ioremap_init()
> > BIOS-provided physical RAM map:
> > BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
> > BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
> > BIOS-e820: 00000000000e8000 - 0000000000100000 (reserved)
> > BIOS-e820: 0000000000100000 - 000000001fff0000 (usable)
> > BIOS-e820: 000000001fff0000 - 0000000020000000 (ACPI data)
> > BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
> > console [earlyser0] enabled
> > 0MB HIGHMEM available.
> > 511MB LOWMEM available.
> > Scan SMP from c0000000 for 1024 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > Scan SMP from c00f0000 for 65536 bytes.
> > Scan SMP from c009fc00 for 1024 bytes.
> > ***************
> > **** WARNING: likely BIOS bug
> > **** MTRRs don't cover all of memory, trimmed 131056 pages
> > ***************
> 
> H. Peter Anvin wrote:
> Looks like the code doesn't check that the CPU *has* MTRRs...

so more strict check if mtrr is there really.
bail out if mtrr all blank when qemu cpu model is used

and check if is AMD as early
also remove 4G less check, according to hpa.

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
@@ -642,13 +642,10 @@ early_param("disable_mtrr_trim", disable
 #define Tom2Enabled (1U << 21)
 #define Tom2ForceMemTypeWB (1U << 22)
 
-static __init int amd_special_default_mtrr(unsigned long end_pfn)
+static __init int amd_special_default_mtrr(void)
 {
 	u32 l, h;
 
-	/* Doesn't apply to memory < 4GB */
-	if (end_pfn <= (0xffffffff >> PAGE_SHIFT))
-		return 0;
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return 0;
 	if (boot_cpu_data.x86 < 0xf || boot_cpu_data.x86 > 0x11)
@@ -686,9 +683,14 @@ int __init mtrr_trim_uncached_memory(uns
 	 * Make sure we only trim uncachable memory on machines that
 	 * support the Intel MTRR architecture:
 	 */
+	if (!is_cpu(INTEL) || disable_mtrr_trim)
+		return 0;
 	rdmsr(MTRRdefType_MSR, def, dummy);
 	def &= 0xff;
-	if (!is_cpu(INTEL) || disable_mtrr_trim || def != MTRR_TYPE_UNCACHABLE)
+	if (def != MTRR_TYPE_UNCACHABLE)
+		return 0;
+
+	if (amd_special_default_mtrr())
 		return 0;
 
 	/* Find highest cached pfn */
@@ -702,8 +704,14 @@ int __init mtrr_trim_uncached_memory(uns
 			highest_addr = base + size;
 	}
 
-	if (amd_special_default_mtrr(end_pfn))
+	/* kvm/qemu doesn't have mtrr set right, don't trim them all */
+	if (!highest_addr) {
+		printk(KERN_WARNING "***************\n");
+		printk(KERN_WARNING "**** WARNING: likely strange cpu\n");
+		printk(KERN_WARNING "**** MTRRs all blank, cpu in qemu?\n");
+		printk(KERN_WARNING "***************\n");
 		return 0;
+	}
 
 	if ((highest_addr >> PAGE_SHIFT) < end_pfn) {
 		printk(KERN_WARNING "***************\n");

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> [PATCH] x86: trim ram need to check if mtrr is there v3

> > H. Peter Anvin wrote:
> > Looks like the code doesn't check that the CPU *has* MTRRs...
> 
> so more strict check if mtrr is there really.
> bail out if mtrr all blank when qemu cpu model is used
> 
> and check if is AMD as early
> also remove 4G less check, according to hpa.

thanks, applied. Shouldnt we put in an exception for when there is MTRR 
support, but they dont cover anything. Still emit a warning - but 
booting up real slow is still better than losing all of RAM and crashing 
...

i also updated the messages, they now go like this:

  WARNING: strange, CPU MTRRs all blank?

and:

  WARNING: BIOS bug: CPU MTRRs don't cover all of memory, losing 45MB of RAM.

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
>
>   
>> [PATCH] x86: trim ram need to check if mtrr is there v3
>>     
>
>   
>>> H. Peter Anvin wrote:
>>> Looks like the code doesn't check that the CPU *has* MTRRs...
>>>       
>> so more strict check if mtrr is there really.
>> bail out if mtrr all blank when qemu cpu model is used
>>
>> and check if is AMD as early
>> also remove 4G less check, according to hpa.
>>     
>
> thanks, applied. Shouldnt we put in an exception for when there is MTRR 
> support, but they dont cover anything. Still emit a warning - but 
> booting up real slow is still better than losing all of RAM and crashing 
>   

The problem is re-occuring for me with current x86.git.  Looks like v2 
did the trick, and v3 is broken...

    J

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

>> thanks, applied. Shouldnt we put in an exception for when there is 
>> MTRR support, but they dont cover anything. Still emit a warning - 
>> but booting up real slow is still better than losing all of RAM and 
>> crashing
>
> The problem is re-occuring for me with current x86.git.  Looks like v2 
> did the trick, and v3 is broken...

which git head is that? I'm pushing out the queue with v3 included this 
very minute, so i doubt you can have tested that already! :-)

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>   
>>> thanks, applied. Shouldnt we put in an exception for when there is 
>>> MTRR support, but they dont cover anything. Still emit a warning - 
>>> but booting up real slow is still better than losing all of RAM and 
>>> crashing
>>>       
>> The problem is re-occuring for me with current x86.git.  Looks like v2 
>> did the trick, and v3 is broken...
>>     
>
> which git head is that? I'm pushing out the queue with v3 included this 
> very minute, so i doubt you can have tested that already! :-)
>   

I was referring to:

commit 6a4544a9c8b54b82893044cb53695502cc386f00
Author: Yinghai Lu <Yinghai.Lu@Sun.COM>
Date:   Fri Jan 25 00:15:29 2008 +0100

    x86_32: trim memory by updating e820 v3


but you've answered the question...

    J

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> I was referring to:
>
> commit 6a4544a9c8b54b82893044cb53695502cc386f00
> Author: Yinghai Lu <Yinghai.Lu@Sun.COM>
> Date:   Fri Jan 25 00:15:29 2008 +0100
>
>    x86_32: trim memory by updating e820 v3
>
> but you've answered the question...

yeah, i guess what i've pushed out just now is v3++. Does it work for 
you?

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>   
>> I was referring to:
>>
>> commit 6a4544a9c8b54b82893044cb53695502cc386f00
>> Author: Yinghai Lu <Yinghai.Lu@Sun.COM>
>> Date:   Fri Jan 25 00:15:29 2008 +0100
>>
>>    x86_32: trim memory by updating e820 v3
>>
>> but you've answered the question...
>>     
>
> yeah, i guess what i've pushed out just now is v3++. Does it work for 
> you?
>   

Yes, it does.  And for the record:

[root@qemu ~]# cat /proc/mtrr 
[root@qemu ~]# 

	J

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

>> yeah, i guess what i've pushed out just now is v3++. Does it work for 
>> you?
>
> Yes, it does.  And for the record:
>
> [root@qemu ~]# cat /proc/mtrr [root@qemu ~]# 
> 	J

great, thanks for checking.

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Yinghai Lu]

On Jan 25, 2008 11:27 AM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> Ingo Molnar wrote:
> > * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> >
> >
> >> I was referring to:
> >>
> >> commit 6a4544a9c8b54b82893044cb53695502cc386f00
> >> Author: Yinghai Lu <Yinghai.Lu@Sun.COM>
> >> Date:   Fri Jan 25 00:15:29 2008 +0100
> >>
> >>    x86_32: trim memory by updating e820 v3
> >>
> >> but you've answered the question...
> >>
> >
> > yeah, i guess what i've pushed out just now is v3++. Does it work for
> > you?
> >
>
> Yes, it does.  And for the record:
>
> [root@qemu ~]# cat /proc/mtrr
> [root@qemu ~]#

so the boot msg say:
WARNING: strange, CPU MTRRs all blank?

YH

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Jeremy Fitzhardinge]

Yinghai Lu wrote:
> On Jan 25, 2008 11:27 AM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>   
>> Ingo Molnar wrote:
>>     
>>> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>>>
>>>
>>>       
>>>> I was referring to:
>>>>
>>>> commit 6a4544a9c8b54b82893044cb53695502cc386f00
>>>> Author: Yinghai Lu <Yinghai.Lu@Sun.COM>
>>>> Date:   Fri Jan 25 00:15:29 2008 +0100
>>>>
>>>>    x86_32: trim memory by updating e820 v3
>>>>
>>>> but you've answered the question...
>>>>
>>>>         
>>> yeah, i guess what i've pushed out just now is v3++. Does it work for
>>> you?
>>>
>>>       
>> Yes, it does.  And for the record:
>>
>> [root@qemu ~]# cat /proc/mtrr
>> [root@qemu ~]#
>>     
>
> so the boot msg say:
> WARNING: strange, CPU MTRRs all blank?
>   

Yes.

WARNING: strange, CPU MTRRs all blank?
------------[ cut here ]------------
WARNING: at /home/jeremy/hg/xen/paravirt/linux/arch/x86/kernel/cpu/mtrr/main.c:710 mtrr_trim_uncached_memory+0xf8/0x17b()
Modules linked in:
Pid: 0, comm: swapper Not tainted 2.6.24 #1938
 [<c012804c>] warn_on_slowpath+0x41/0x51
 [<c01285d4>] ? __call_console_drivers+0x4e/0x5a
 [<c03c3e85>] ? _spin_unlock_irqrestore+0xf/0x2f
 [<c03c3e91>] ? _spin_unlock_irqrestore+0x1b/0x2f
 [<c012880c>] ? release_console_sem+0x1a0/0x1a8
 [<c0128c1f>] ? vprintk+0x294/0x2ee
 [<c0128c2b>] ? vprintk+0x2a0/0x2ee
 [<c04e811b>] ? __alloc_bootmem_core+0x100/0x27a
 [<c0128c8e>] ? printk+0x15/0x17
 [<c0128c8e>] ? printk+0x15/0x17
 [<c04de51e>] mtrr_trim_uncached_memory+0xf8/0x17b
 [<c04de7ae>] ? mtrr_bp_init+0x20d/0x215
 [<c04dc79b>] setup_arch+0x283/0x3c2
 [<c0128c8e>] ? printk+0x15/0x17
 [<c04d6689>] start_kernel+0x60/0x311
 =======================

	J

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Yinghai Lu]

On Friday 25 January 2008 10:55:31 am Jeremy Fitzhardinge wrote:
> Ingo Molnar wrote:
> > * Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
> >
> >   
> >> [PATCH] x86: trim ram need to check if mtrr is there v3
> >>     
> >
> >   
> >>> H. Peter Anvin wrote:
> >>> Looks like the code doesn't check that the CPU *has* MTRRs...
> >>>       
> >> so more strict check if mtrr is there really.
> >> bail out if mtrr all blank when qemu cpu model is used
> >>
> >> and check if is AMD as early
> >> also remove 4G less check, according to hpa.
> >>     
> >
> > thanks, applied. Shouldnt we put in an exception for when there is MTRR 
> > support, but they dont cover anything. Still emit a warning - but 
> > booting up real slow is still better than losing all of RAM and crashing 
> >   
> 
> The problem is re-occuring for me with current x86.git.  Looks like v2 
> did the trick, and v3 is broken...

so the ram size less 4g is way out for your case.

again, can you post /proc/mtrrs with v2 patch?

Thanks

YH

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Jeremy Fitzhardinge]

Yinghai Lu wrote:
> On Friday 25 January 2008 10:55:31 am Jeremy Fitzhardinge wrote:
>   
>> Ingo Molnar wrote:
>>     
>>> * Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
>>>
>>>   
>>>       
>>>> [PATCH] x86: trim ram need to check if mtrr is there v3
>>>>     
>>>>         
>>>   
>>>       
>>>>> H. Peter Anvin wrote:
>>>>> Looks like the code doesn't check that the CPU *has* MTRRs...
>>>>>       
>>>>>           
>>>> so more strict check if mtrr is there really.
>>>> bail out if mtrr all blank when qemu cpu model is used
>>>>
>>>> and check if is AMD as early
>>>> also remove 4G less check, according to hpa.
>>>>     
>>>>         
>>> thanks, applied. Shouldnt we put in an exception for when there is MTRR 
>>> support, but they dont cover anything. Still emit a warning - but 
>>> booting up real slow is still better than losing all of RAM and crashing 
>>>   
>>>       
>> The problem is re-occuring for me with current x86.git.  Looks like v2 
>> did the trick, and v3 is broken...
>>     
>
> so the ram size less 4g is way out for your case.
>
> again, can you post /proc/mtrrs with v2 patch?
>   

It was empty.  But ignore this report; it wasn't against the right 
version of x86.git.

    J

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
>
>   
>> [PATCH] x86: trim ram need to check if mtrr is there v3
>>     
>
>   
>>> H. Peter Anvin wrote:
>>> Looks like the code doesn't check that the CPU *has* MTRRs...
>>>       
>> so more strict check if mtrr is there really.
>> bail out if mtrr all blank when qemu cpu model is used
>>
>> and check if is AMD as early
>> also remove 4G less check, according to hpa.
>>     
>
> thanks, applied. Shouldnt we put in an exception for when there is MTRR 
> support, but they dont cover anything. Still emit a warning - but 
> booting up real slow is still better than losing all of RAM and crashing 
> ...
>
> i also updated the messages, they now go like this:
>
>   WARNING: strange, CPU MTRRs all blank?
>
> and:
>
>   WARNING: BIOS bug: CPU MTRRs don't cover all of memory, losing 45MB of RAM.
>   

Though I don't see this form of message; have you pushed your changes 
out to the public x86.git#mm tree?

    J

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

>> and:
>>
>>   WARNING: BIOS bug: CPU MTRRs don't cover all of memory, losing 45MB 
>>   of RAM.
>
> Though I don't see this form of message; have you pushed your changes 
> out to the public x86.git#mm tree?

it's available ... 3 .. 2 ... 1 ... NOW! ;-)

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> -static __init int amd_special_default_mtrr(unsigned long end_pfn)
> +static __init int amd_special_default_mtrr(void)
>  {
>  	u32 l, h;
>  
> -	/* Doesn't apply to memory < 4GB */
> -	if (end_pfn <= (0xffffffff >> PAGE_SHIFT))
> -		return 0;
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
>  		return 0;

The logic we ideally would like to have is something like this:

find _any_ RAM that is not mapped via any MTRRs (be that special MTRR 
extensions like Tom2) and clear that from the e820 maps.

not just 'end of RAM'.

And in that sense the amd_special_default_mtrr() check is wrong, because 
it just checks that Tom2 is set and then does no other checking. And the 
original MTRR check is wrong too because it just finds the highest 
cacheable MTRR-covered address and compares that to the kernel-known end 
of RAM.

what we should probably do instead is to have a filter function:

   new_end = trim_range_to_mtrr_cached(start, end);

and then we could iterate through every e820 map entry that is marked as 
usable RAM, and send it through this filter. If the filter returns the 
same value that got passed in, we keep the e820 entry unchanged. If the 
filter returns a new "end" value, we use that in the e820 map.

that way, the current Tom2 hack is just a natural extension to the 
filter function: it would (on AMD CPUs) recognize (within 
trim_range_to_mtrr_cached filter) that all memory addresses above 4GB 
are marked as cacheable via Tom2.

Or something like this. Hm?

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Andi Kleen]

Ingo Molnar <mingo@elte.hu> writes:
>
> what we should probably do instead is to have a filter function:
>
>    new_end = trim_range_to_mtrr_cached(start, end);
>
> and then we could iterate through every e820 map entry that is marked as 
> usable RAM, and send it through this filter. If the filter returns the 
> same value that got passed in, we keep the e820 entry unchanged. If the 
> filter returns a new "end" value, we use that in the e820 map.

To be fully generic you would need to allow it to adjust start too.

> that way, the current Tom2 hack is just a natural extension to the 
> filter function: it would (on AMD CPUs) recognize (within 
> trim_range_to_mtrr_cached filter) that all memory addresses above 4GB 
> are marked as cacheable via Tom2.
>
> Or something like this. Hm?

I agree that would be the correct way to do it.

Later on with PAT that filter could also do PAT related checks
and something like this will likely be needed anyways.

-Andi

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> >
> > what we should probably do instead is to have a filter function:
> >
> >    new_end = trim_range_to_mtrr_cached(start, end);
> >
> > and then we could iterate through every e820 map entry that is 
> > marked as usable RAM, and send it through this filter. If the filter 
> > returns the same value that got passed in, we keep the e820 entry 
> > unchanged. If the filter returns a new "end" value, we use that in 
> > the e820 map.
> 
> To be fully generic you would need to allow it to adjust start too.

no, to be fully generic it would have to be able to 'split' e820 entries 
up and punch holes into them - but we dont want to go that far i think. 
The most common problem is mismatch at the end of a range.

but what matters more is to have full, generic _detection_ of the 
problem - and that's what we dont do right now. (and that's what my 
reply outlines)

The _fixup_ which we base on this information can then be anything from 
"trivially trim the end" up to a complex "punch holes" solution or the 
simplest "print nasty warning message and do nothing else" solution.

> > that way, the current Tom2 hack is just a natural extension to the 
> > filter function: it would (on AMD CPUs) recognize (within 
> > trim_range_to_mtrr_cached filter) that all memory addresses above 
> > 4GB are marked as cacheable via Tom2.
> >
> > Or something like this. Hm?
> 
> I agree that would be the correct way to do it.
> 
> Later on with PAT that filter could also do PAT related checks and 
> something like this will likely be needed anyways.

a "what is the effective MTRR caching attribute of this physical 
address" type of function would benefit PAT too, yes.

	Ingo

Re: [PATCH] x86: trim ram need to check if mtrr is there v3

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

Ingo Molnar wrote:
> 
> no, to be fully generic it would have to be able to 'split' e820 entries 
> up and punch holes into them - but we dont want to go that far i think. 
> The most common problem is mismatch at the end of a range.
> 

For what it's worth, I have a set of code to do this, written in order 
to canonicalize and modify e820 data structures for memdisk.

The key to it is the observation that the e820-delivered (address, len, 
type) tuples isn't actually the data structure you want -- what you want 
is an ordered list of (address, type) tuples, where type may includes 
undefined (the e820 default type - i.e. unused, available address space).

In addition to the attached code, to do this right, we probably want a 
function to do filtering (only clobber entries of a specfic type, in 
this case type 1) as opposed to blind clobber; with an ordered array 
this is quite trivial.

	-hpa

[-- Attachment #2: e820.h --]
[-- Type: text/x-chdr, Size: 1000 bytes --]

/* ----------------------------------------------------------------------- *
 *
 *   Copyright 2001-2008 H. Peter Anvin - All Rights Reserved
 *
 *   This program is free software; you can redistribute it and/or modify
 *   it under the terms of the GNU General Public License as published by
 *   the Free Software Foundation, Inc., 53 Temple Place Ste 330,
 *   Boston MA 02111-1307, USA; either version 2 of the License, or
 *   (at your option) any later version; incorporated herein by reference.
 *
 * ----------------------------------------------------------------------- */

/*
 * e820.h
 *
 * Common routines for e820 memory map management
 */

#include <stdint.h>

struct e820range {
  uint64_t start;
  uint32_t type;
} __attribute__((packed));

extern struct e820range ranges[];
extern int nranges;
extern uint32_t dos_mem, low_mem, high_mem;

extern void e820map_init(void);
extern void insertrange(uint64_t, uint64_t, uint32_t);
extern void get_mem(void);
extern void parse_mem(void);

[-- Attachment #3: e820func.c --]
[-- Type: text/x-csrc, Size: 2503 bytes --]

/* ----------------------------------------------------------------------- *
 *
 *   Copyright 2001-2008 H. Peter Anvin - All Rights Reserved
 *
 *   This program is free software; you can redistribute it and/or modify
 *   it under the terms of the GNU General Public License as published by
 *   the Free Software Foundation, Inc., 53 Temple Place Ste 330,
 *   Boston MA 02111-1307, USA; either version 2 of the License, or
 *   (at your option) any later version; incorporated herein by reference.
 *
 * ----------------------------------------------------------------------- */

/*
 * e820func.c
 *
 * E820 range database manager
 */

#include <stdint.h>
#include "memdisk.h"		/* For memset() */
#include "e820.h"

#define MAXRANGES	64
/* All of memory starts out as one range of "indeterminate" type */
struct e820range ranges[MAXRANGES];
int nranges;


void e820map_init(void)
{
  memset(ranges, 0, sizeof(ranges));
  nranges = 1;
  ranges[1].type = -1;
}

static void insertrange_at(int where, uint64_t start, uint32_t type)
{
  int i;

  for ( i = nranges ; i > where ; i-- )
    ranges[i] = ranges[i-1];

  ranges[where].start = start;
  ranges[where].type  = type;

  nranges++;
  ranges[nranges].start = 0ULL;
  ranges[nranges].type  = (uint32_t)-1;
}

void insertrange(uint64_t start, uint64_t len, uint32_t type)
{
  uint64_t last;
  uint32_t oldtype;
  int i, j;

  /* Remove this to make len == 0 mean all of memory */
  if ( len == 0 )
    return;			/* Nothing to insert */

  last = start+len-1;		/* May roll over */

  i = 0;
  oldtype = -2;
  while ( start > ranges[i].start && ranges[i].type != -1 ) {
    oldtype = ranges[i].type;
    i++;
  }

  /* Consider the replacement policy.  This current one is "overwrite." */

  if ( start < ranges[i].start || ranges[i].type == -1 )
    insertrange_at(i++, start, type);

  while ( i == 0 || last > ranges[i].start-1 ) {
    oldtype = ranges[i].type;
    ranges[i].type = type;
    i++;
  }

  if ( last < ranges[i].start-1 )
    insertrange_at(i, last+1, oldtype);

  /* Now the map is correct, but quite possibly not optimal.  Scan the
     map for ranges which are redundant and remove them. */
  i = j = 1;
  oldtype = ranges[0].type;
  while ( i < nranges ) {
    if ( ranges[i].type == oldtype ) {
      i++;
    } else {
      oldtype = ranges[i].type;
      if ( i != j )
	ranges[j] = ranges[i];
      i++; j++;
    }
  }

  if ( i != j ) {
    ranges[j] = ranges[i];	/* Termination sentinel copy */
    nranges -= (i-j);
  }
}