Re: [tip:x86/mm] x86: make e820_update_range() handle small range update

[Ingo Molnar <mingo@elte.hu>]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> >> Ingo Molnar wrote:
> >>> * Yinghai Lu <yinghai@kernel.org> wrote:
> >>>
> >>>> Ingo Molnar wrote:
> >>>>> * Yinghai Lu <yinghai@kernel.org> wrote:
> >>>>>
> >>>>>> Commit-ID:  78a8b35bc7abf8b8333d6f625e08c0f7cc1c3742
> >>>>>> Gitweb:     http://git.kernel.org/tip/78a8b35bc7abf8b8333d6f625e08c0f7cc1c3742
> >>>>>> Author:     Yinghai Lu <yinghai@kernel.org>
> >>>>>> AuthorDate: Thu, 12 Mar 2009 22:36:01 -0700
> >>>>>> Commit:     Ingo Molnar <mingo@elte.hu>
> >>>>>> CommitDate: Sat, 14 Mar 2009 12:20:07 +0100
> >>>>>>
> >>>>>> x86: make e820_update_range() handle small range update
> >>>>>>
> >>>>>> Impact: enhance e820 code to handle more cases
> >>>>>>
> >>>>>> Try to handle new range which could be covered by one entry.
> >>>>>>
> >>>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >>>>>> Cc: jbeulich@novell.com
> >>>>>> LKML-Reference: <49B9F0C1.10402@kernel.org>
> >>>>>> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> >>>>> -tip testing found a bootup crash + reboot due to this patch.
> >>>>>
> >>>>> config attached. The bootup crashes here:
> >>>>>
> >>>>> [    0.000000] MTRR variable ranges enabled:
> >>>>> [    0.000000]   0 base 0000000000 mask FFC0000000 write-back
> >>>>> [    0.000000]   1 disabled
> >>>>> [    0.000000]   2 disabled
> >>>>> [    0.000000]   3 disabled
> >>>>> [    0.000000]   4 disabled
> >>>>> [    0.000000]   5 disabled
> >>>>> [    0.000000]   6 disabled
> >>>>> [    0.000000]   7 disabled
> >>>>> [    0.000000] x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106
> >>>>> [    0.000000]   get_mtrr: cpu0 reg00 base=0000000000 size=0000040000 write-back
> >>>>> [    reboot  ]
> >>>>>
> >>>> what does the e820 map look like?
> >>> [    0.000000] BIOS-provided physical RAM map:
> >>> [    0.000000]  BIOS-e820: 0000000000000000 - 000000000009f800 (usable)
> >>> [    0.000000]  BIOS-e820: 000000000009f800 - 00000000000a0000 (reserved)
> >>> [    0.000000]  BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
> >>> [    0.000000]  BIOS-e820: 0000000000100000 - 000000003fff0000 (usable)
> >>> [    0.000000]  BIOS-e820: 000000003fff0000 - 000000003fff3000 (ACPI NVS)
> >>> [    0.000000]  BIOS-e820: 000000003fff3000 - 0000000040000000 (ACPI data)
> >>> [    0.000000]  BIOS-e820: 00000000e0000000 - 00000000f0000000 (reserved)
> >>> [    0.000000]  BIOS-e820: 00000000fec00000 - 0000000100000000 (reserved)
> >>>
> >> it seems that patch revealed the bug in 
> >> setup_bios_corruption_check
> > 
> > Cool. I'm wondering, could this explain:
> > 
> >    http://bugzilla.kernel.org/show_bug.cgi?id=12645
> > 
> 
> i wonder if we need to add print out about range from 
> e820_update_range() or add sth like boundary checking in 
> e820_update_range() to stop crazy calling?

Boundary checks - yes! The e820 engine should be a safe black 
box, as much as possible. In case of errors, if there's no 
secure way to proceed, we should emit a meaningful warning and 
should panic().

	Ingo