Re: [RFC PATCH 3/3] x86, boot: Change the BIOS corruption checker to scan 640K

[Ingo Molnar <mingo@kernel.org>]

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

> On 11/11/2013 08:07 PM, Ingo Molnar wrote:
>
> > I agree with your patches so far, and I'd suggest we go even further: 
> > I'd say the config option is now a misnomer, it should probably be 
> > renamed to CONFIG_X86_FORCE_RESERVE_BIOS_LOW_1MB=y or so.
> 
> Why is that?  It doesn't seem to make much sense to me.  I think the 
> current option names seem to be just fine, but perhaps I'm missing 
> something.

My argument is that CONFIG_X86_CHECK_BIOS_CORRUPTION=y doesn't actually 
_check_ any 'BIOS corruption'. Its largest operative effect to the vast 
majority of users is to reserve the whole 1MB lower range to the BIOS and 
it performs no checking whatsoever.

That used to be different btw., it used to be on by default, that's why I 
named the option in such a way original. That original role has been 
almost completely eliminated though.

The _real_ option that turns on memory corruption checking is 
CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y. (The 
memory_corruption_check=1 option is something very few users and 
essentially no distro will use - distros will use the .config option.)

> > Btw., should we also force-reserve the remaining bits over 640K..1MB, 
> > if they are not marked as reserved in the memory maps, or do we 
> > already force-reserve them somewhere?
> 
> We do, in trim_bios_range().  We treat it as available for I/O 
> assignments, since that is necessary on some systems.

Ok, good.

> > The CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y option and the 
> > memory_corruption_check=1 boot option then allow the activation of the 
> > low memory corrupion checker - which debug facility can be used on 
> > systems where someone wants to live dangerously and not reserve the 
> > low 1MB of RAM to the firmware.
> 
> That is indeed what this patch does, I think...

Yes, and that's my argument for simply renaming the option, no other 
change is needed IMO.

Thanks,

	Ingo