Corrupted low memory in v3.9+

Corrupted low memory in v3.9+

[Olof Johansson]

Hi Peter,

When booting a newer kernel on a Chromebox (Samsung SNB system,
coreboot firmware, verified boot and not using seabios), I get reports
of:

[    1.727520] Corrupted low memory at ffff880000002a90 (2a90 phys) = 100000000
[    1.734565] Corrupted low memory at ffff880000002a98 (2a98 phys) =
50000000000

I bisected it down to:

commit 95c9608478d639dcffc14ea47b31bff021a99ed1
Author: H. Peter Anvin <hpa@zytor.com>
Date:   Thu Feb 14 14:02:52 2013 -0800

    x86, mm: Move reserving low memory later in initialization

    Move the reservation of low memory, except for the 4K which actually
    does belong to the BIOS, later in the initialization; in particular,
    after we have already reserved the trampoline.

    The current code locates the trampoline as high as possible, so by
    deferring the allocation we will still be able to reserve as much
    memory as is possible.  This allows us to run with reservelow=640k
    without getting a crash on system startup.


My config has:

CONFIG_X86_RESERVE_LOW=64

...but /proc/iomem says:

00000000-00000fff : reserved
00001000-0009ffff : System RAM
000a0000-000fffff : reserved
[...]


While before the above patch it said:

00000000-00000fff : reserved
00001000-0000ffff : reserved
00010000-0009ffff : System RAM
[...]

And the low memory checker never even ran before, since it had nothing
to check. Earlier the lower reserved region would be included in the
e820-reserved area if I read the code correctly, and now it's just
marked reserved by the memblock code.

I guess it could be argued either way whether this is a regression or
not; but at the end of the day we now have systems where this warning
pops when it didn't use to. :(


-Olof

Re: Corrupted low memory in v3.9+

[H. Peter Anvin]

On 10/17/2013 11:57 AM, Olof Johansson wrote:
> 
> And the low memory checker never even ran before, since it had nothing
> to check. Earlier the lower reserved region would be included in the
> e820-reserved area if I read the code correctly, and now it's just
> marked reserved by the memblock code.
> 
> I guess it could be argued either way whether this is a regression or
> not; but at the end of the day we now have systems where this warning
> pops when it didn't use to. :(
> 

I'm wondering if this is a problem with the low memory checker (the
residual value of which I have to admit to being skeptical of) or
something else.

Could you boot the box with "debug memblock=debug" and earlyprintk
turned on and send the boot output?

	-hpa

Re: Corrupted low memory in v3.9+

[Olof Johansson]

On Thu, Oct 17, 2013 at 12:39 PM, H. Peter Anvin <hpa@linux.intel.com> wrote:
> On 10/17/2013 11:57 AM, Olof Johansson wrote:
>>
>> And the low memory checker never even ran before, since it had nothing
>> to check. Earlier the lower reserved region would be included in the
>> e820-reserved area if I read the code correctly, and now it's just
>> marked reserved by the memblock code.
>>
>> I guess it could be argued either way whether this is a regression or
>> not; but at the end of the day we now have systems where this warning
>> pops when it didn't use to. :(
>>
>
> I'm wondering if this is a problem with the low memory checker (the
> residual value of which I have to admit to being skeptical of) or
> something else.

There's a chance that it's a valid trip of the low-memory checker,
i.e. that we do have a bios (or more likely smm), that stomps on that
memory -- it was never checked for in the past and definitely not
warned about. I'm not sure if that was intentional behavior or not (to
not check this area), I lack history on the topic.

> Could you boot the box with "debug memblock=debug" and earlyprintk
> turned on and send the boot output?

Ah, yes, I did verify that the first 64K were indeed set aside as
reserved by doing just that:

[    0.000000] MEMBLOCK configuration:
[    0.000000]  memory size = 0x7c750000 reserved size = 0xb05000
[    0.000000]  memory.cnt  = 0x6
[    0.000000]  memory[0x0] [0x00000000010000-0x0000000009ffff], 0x90000 bytes
[    0.000000]  memory[0x1] [0x00000000100000-0x00000000efffff], 0xe00000 bytes
[    0.000000]  memory[0x2] [0x00000001000000-0x0000001fffffff],
0x1f000000 bytes
[    0.000000]  memory[0x3] [0x00000020200000-0x0000003fffffff],
0x1fe00000 bytes
[    0.000000]  memory[0x4] [0x00000040200000-0x0000007c6bffff],
0x3c4c0000 bytes
[    0.000000]  memory[0x5] [0x00000100000000-0x000001005fffff], 0x600000 bytes
[    0.000000]  reserved.cnt  = 0x2
[    0.000000]  reserved[0x0] [0x0000000009f000-0x000000000fffff], 0x61000 bytes
[    0.000000]  reserved[0x1] [0x00000001000000-0x00000001aa3fff],
0xaa4000 bytes
[    0.000000] memblock_reserve: [0x00000000099000-0x0000000009f000]
reserve_real_mode+0x61/0x87
[    0.000000] Base memory trampoline at [ffff880000099000] 99000 size 24576
[    0.000000] reserving inaccessible SNB gfx pages
[    0.000000] memblock_reserve: [0x00000000000000-0x00000000100000]
setup_arch+0xa2d/0xa41
[...]

Unfortunately x86 doesn't keep the memblock structures around, so
there's no way to verify after booting in debugfs, but based on the
above it should have been reserved properly.


-Olof

Re: Corrupted low memory in v3.9+

[Olof Johansson]

On Thu, Oct 17, 2013 at 1:39 PM, Olof Johansson <olof@lixom.net> wrote:
> On Thu, Oct 17, 2013 at 12:39 PM, H. Peter Anvin <hpa@linux.intel.com> wrote:
>> On 10/17/2013 11:57 AM, Olof Johansson wrote:
>>>
>>> And the low memory checker never even ran before, since it had nothing
>>> to check. Earlier the lower reserved region would be included in the
>>> e820-reserved area if I read the code correctly, and now it's just
>>> marked reserved by the memblock code.
>>>
>>> I guess it could be argued either way whether this is a regression or
>>> not; but at the end of the day we now have systems where this warning
>>> pops when it didn't use to. :(
>>>
>>
>> I'm wondering if this is a problem with the low memory checker (the
>> residual value of which I have to admit to being skeptical of) or
>> something else.
>
> There's a chance that it's a valid trip of the low-memory checker,
> i.e. that we do have a bios (or more likely smm), that stomps on that
> memory -- it was never checked for in the past and definitely not
> warned about. I'm not sure if that was intentional behavior or not (to
> not check this area), I lack history on the topic.
>
>> Could you boot the box with "debug memblock=debug" and earlyprintk
>> turned on and send the boot output?
>
> Ah, yes, I did verify that the first 64K were indeed set aside as
> reserved by doing just that:
>
> [    0.000000] MEMBLOCK configuration:
> [    0.000000]  memory size = 0x7c750000 reserved size = 0xb05000
> [    0.000000]  memory.cnt  = 0x6
> [    0.000000]  memory[0x0] [0x00000000010000-0x0000000009ffff], 0x90000 bytes
> [    0.000000]  memory[0x1] [0x00000000100000-0x00000000efffff], 0xe00000 bytes
> [    0.000000]  memory[0x2] [0x00000001000000-0x0000001fffffff],
> 0x1f000000 bytes
> [    0.000000]  memory[0x3] [0x00000020200000-0x0000003fffffff],
> 0x1fe00000 bytes
> [    0.000000]  memory[0x4] [0x00000040200000-0x0000007c6bffff],
> 0x3c4c0000 bytes
> [    0.000000]  memory[0x5] [0x00000100000000-0x000001005fffff], 0x600000 bytes
> [    0.000000]  reserved.cnt  = 0x2
> [    0.000000]  reserved[0x0] [0x0000000009f000-0x000000000fffff], 0x61000 bytes
> [    0.000000]  reserved[0x1] [0x00000001000000-0x00000001aa3fff],
> 0xaa4000 bytes
> [    0.000000] memblock_reserve: [0x00000000099000-0x0000000009f000]
> reserve_real_mode+0x61/0x87
> [    0.000000] Base memory trampoline at [ffff880000099000] 99000 size 24576
> [    0.000000] reserving inaccessible SNB gfx pages
> [    0.000000] memblock_reserve: [0x00000000000000-0x00000000100000]
> setup_arch+0xa2d/0xa41
> [...]
>
> Unfortunately x86 doesn't keep the memblock structures around, so
> there's no way to verify after booting in debugfs, but based on the
> above it should have been reserved properly.

*prod*

So, got a preference on solution for this? The warning seems harmless
but still annoying to get used to ignoring false positives, etc.

Disable the low memory checker by default? Hide it behind a debug
option (runtime or build time)?


-Olof

Re: Corrupted low memory in v3.9+

[H. Peter Anvin]

On 11/07/2013 11:02 AM, Olof Johansson wrote:
>> [    0.000000] reserving inaccessible SNB gfx pages
>> [    0.000000] memblock_reserve: [0x00000000000000-0x00000000100000]

This is on a Sandy Bridge system, which I guess I managed to miss the
first time.  Unfortunately low memory corruption is expected with SNB
graphics... this is why we unconditionally reserve all low memory on SNB.

>> setup_arch+0xa2d/0xa41
>> [...]
>>
>> Unfortunately x86 doesn't keep the memblock structures around, so
>> there's no way to verify after booting in debugfs, but based on the
>> above it should have been reserved properly.
> 
> *prod*
> 
> So, got a preference on solution for this? The warning seems harmless
> but still annoying to get used to ignoring false positives, etc.
> 
> Disable the low memory checker by default? Hide it behind a debug
> option (runtime or build time)?
> 

I'm inclined to say disable it by default, but I'll let Ingo comment.
These days we default to reserving all of low memory other than the
trampoline (which we really can't avoid); leaving it in as a debug
option seems reasonable, but it is really questionable to me how much it
is useful to a general user.

	-hpa

Re: Corrupted low memory in v3.9+

[Ingo Molnar]

* H. Peter Anvin <hpa@linux.intel.com> wrote:

> On 11/07/2013 11:02 AM, Olof Johansson wrote:
> >> [    0.000000] reserving inaccessible SNB gfx pages
> >> [    0.000000] memblock_reserve: [0x00000000000000-0x00000000100000]
> 
> This is on a Sandy Bridge system, which I guess I managed to miss the 
> first time.  Unfortunately low memory corruption is expected with SNB 
> graphics... this is why we unconditionally reserve all low memory on 
> SNB.
> 
> >> setup_arch+0xa2d/0xa41
> >> [...]
> >>
> >> Unfortunately x86 doesn't keep the memblock structures around, so 
> >> there's no way to verify after booting in debugfs, but based on the 
> >> above it should have been reserved properly.
> > 
> > *prod*
> > 
> > So, got a preference on solution for this? The warning seems harmless 
> > but still annoying to get used to ignoring false positives, etc.
> > 
> > Disable the low memory checker by default? Hide it behind a debug 
> > option (runtime or build time)?
> 
> I'm inclined to say disable it by default, but I'll let Ingo comment. 
> These days we default to reserving all of low memory other than the 
> trampoline (which we really can't avoid); leaving it in as a debug 
> option seems reasonable, but it is really questionable to me how much it 
> is useful to a general user.

If we reserve everything in low memory, all the time (which I very much 
argue we should do) then the checker becomes a no-op and can be removed.

Thanks,

	Ingo

Re: Corrupted low memory in v3.9+

[H. Peter Anvin]

On 11/11/2013 03:35 AM, Ingo Molnar wrote:
> 
> If we reserve everything in low memory, all the time (which I very much 
> argue we should do) then the checker becomes a no-op and can be removed.
> 

Oops!  I had misunderstood how the checker worked -- I thought it
checked the *reserved* memory, but it in fact reserves memory
*independently* and then checks it.

The problem is fundamentally that setup_bios_corruption_check(); is
called too early in setup_arch() -- quite possibly due to other code
movement around it:

#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
	setup_bios_corruption_check();
#endif

	reserve_real_mode();

	trim_platform_memory_ranges();
	trim_low_memory_range();

	init_mem_mapping();


setup_bios_corruption_check() should presumably be called between
trim_low_memory_range() and init_mem_mapping().  I'm actually surprised
that we don't trip on this *all the time* since the realmode trampoline
falls in this area...

At the same time, we should change the default for
CONFIG_X86_RESERVE_LOW to 640, and perhaps move it under EXPERT.

What do you guys think?

	-hpa

Re: Corrupted low memory in v3.9+

[H. Peter Anvin]

On 11/11/2013 03:31 PM, H. Peter Anvin wrote:
> On 11/11/2013 03:35 AM, Ingo Molnar wrote:
> 
> Oops!  I had misunderstood how the checker worked -- I thought it
> checked the *reserved* memory, but it in fact reserves memory
> *independently* and then checks it.
> 

And now I understand why I had that misconception:

> 	  Periodically check for memory corruption in low memory, which
> 	  is suspected to be caused by BIOS.  Even when enabled in the
> 	  configuration, it is disabled at runtime.  Enable it by
> 	  setting "memory_corruption_check=1" on the kernel command
> 	  line.  By default it scans the low 64k of memory every 60
> 	  seconds; see the memory_corruption_check_size and
> 	  memory_corruption_check_period parameters in
> 	  Documentation/kernel-parameters.txt to adjust this.

This makes it sound like it scans the low 64K when it is turned on, i.e.
the bit that is reserved by default.

I think we should change that default too to 640K.  I will post a
patchset shortly.

	-hpa