x86/boot: Further reserve_boot_regions() cleanups
cover x86/boot: Further reserve_boot_regions() cleanups
This follows up on Ingo's cleanup. The first patch fixes the docs and the second makes the code even more comprehensible.
Andy Lutomirski (2):
x86/boot: Clarify what x86_legacy_features.reserve_bios_regions does x86/boot: Simplify EBDA-vs-BIOS reservation logic
arch/x86/include/asm/x86_init.h | 5 +++-- arch/x86/kernel/ebda.c | 34 +++++++++++----------------------- 2 files changed, 14 insertions(+), 25 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/2 x86/boot: Clarify what x86_legacy_features.reserve_bios_regions does
It doesn't just control probing for the EBDA -- it controls whether we detect and reserve the <1MB BIOS regions in general.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/include/asm/x86_init.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/include/asm/x86_init.h +3 -2
@@ -168,8 +168,9 @@ struct x86_legacy_devices {* struct x86_legacy_features - legacy x86 features * * @rtc: this device has a CMOS real-time clock present -* @reserve_bios_regions: it's safe to search for the EBDA signature in the hardware's -* low RAM +* @reserve_bios_regions: boot code will search for the EBDA address and the +* start of the 640k - 1M BIOS region. If false, the platform must +* ensure that its memory map correctly reserves sub-1MB regions as needed. * @devices: legacy x86 devices, refer to struct x86_legacy_devices * documentation for further details. */
Signature · 1 line
2.7.4
tip-bot for Andy Lutomirski
merge bot[tip:x86/boot] x86/boot: Clarify what x86_legacy_features.reserve_bios_regions does
x86/boot: Clarify what x86_legacy_features.reserve_bios_regions does
Commit-ID: 30f027398b329c75c8f23a3c13be240b50866fdc Gitweb: http://git.kernel.org/tip/30f027398b329c75c8f23a3c13be240b50866fdc Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 21 Jul 2016 14:16:51 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 22 Jul 2016 11:46:01 +0200
x86/boot: Clarify what x86_legacy_features.reserve_bios_regions does
It doesn't just control probing for the EBDA -- it controls whether we detect and reserve the <1MB BIOS regions in general.
Signed-off-by Andy Lutomirski <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Kees Cook <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Luis R. Rodriguez <[email protected]>
Cc Mario Limonciello <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Toshi Kani <[email protected]>
Link http://lkml.kernel.org/r/55bd591115498440d461857a7b64f349a5d911f3.1469135598.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/x86_init.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/include/asm/x86_init.h +3 -2
@@ -168,8 +168,9 @@ struct x86_legacy_devices {* struct x86_legacy_features - legacy x86 features * * @rtc: this device has a CMOS real-time clock present -* @reserve_bios_regions: it's safe to search for the EBDA signature in the hardware's -* low RAM +* @reserve_bios_regions: boot code will search for the EBDA address and the +* start of the 640k - 1M BIOS region. If false, the platform must +* ensure that its memory map correctly reserves sub-1MB regions as needed. * @devices: legacy x86 devices, refer to struct x86_legacy_devices * documentation for further details. */
PATCH 2/2 x86/boot: Simplify EBDA-vs-BIOS reservation logic
In reply to Andy LutomirskiBoth the intent and the effect of reserve_bios_regions() is simple:
reserve the range from the apparent BIOS start (suitably filtered) through 1MB and, if the EBDA start address is sensible, extend that reservation downward to cover the EBDA as well.
The code is overcomplicated, though, and contains head-scratchers like:
if (ebda_start < BIOS_START_MIN) ebda_start = BIOS_START_MAX;
That snipped is trying to say "if ebda_start < BIOS_START_MIN, ignore it".
Simplify it: reorder the code so that it makes sense. This should have no functional effect under any circumstances.
Signed-off-by Andy Lutomirski <[email protected]>
--- arch/x86/kernel/ebda.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-)
arch/x86/kernel/ebda.c +11 -23
@@ -65,22 +65,6 @@ void __init reserve_bios_regions(void)if (!x86_platform.legacy.reserve_bios_regions)return ; -/* Get the start address of the EBDA page: */ - ebda_start =get_bios_ebda (); - -/* -* Quirk: some old Dells seem to have a 4k EBDA without -* reporting so in their BIOS RAM size value, so just -* consider the memory above 640K to be off limits -* (bugzilla 2990). -* -* We detect this case by filtering for nonsensical EBDA -* addresses below 128K, where we can assume that they -* are bogus and bump it up to a fixed 640K value: -*/ -if (ebda_start <BIOS_START_MIN ) - ebda_start =BIOS_START_MAX ; -/* * BIOS RAM size is encoded in kilobytes, convert it * to bytes to get a first guess at where the BIOS
@@ -91,18 +75,22 @@ void __init reserve_bios_regions(void)/* * If bios_start is less than 128K, assume it is bogus -* and bump it up to 640K: +* and bump it up to 640K. Similarly, if bios_start is above 640K, +* don't trust it. */ -if (bios_start <BIOS_START_MIN ) +if (bios_start <BIOS_START_MIN || bios_start >BIOS_START_MAX ) bios_start =BIOS_START_MAX ; +/* Get the start address of the EBDA page: */ + ebda_start =get_bios_ebda (); +/* -* Use the lower of the bios_start and ebda_start -* as the starting point, but don't allow it to -* go beyond 640K: +* If the EBDA start address is sane and is below the BIOS region, +* then also reserve everything from the EBDA start address up to +* the BIOS region. */ - bios_start =min (bios_start, ebda_start); - bios_start =min (bios_start,BIOS_START_MAX ); +if (ebda_start >=BIOS_START_MIN && ebda_start < bios_start) + bios_start = ebda_start;/* Reserve all memory between bios_start and the 1MB mark: */ memblock_reserve (bios_start,0x100000 - bios_start);
Signature · 1 line
2.7.4
tip-bot for Andy Lutomirski
merge bot[tip:x86/boot] x86/boot: Simplify EBDA-vs-BIOS reservation logic
x86/boot: Simplify EBDA-vs-BIOS reservation logic
Commit-ID: 6a79296cb15d947bcb4558011fe066e5d8252b35 Gitweb: http://git.kernel.org/tip/6a79296cb15d947bcb4558011fe066e5d8252b35 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 21 Jul 2016 14:16:52 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 22 Jul 2016 11:46:01 +0200
x86/boot: Simplify EBDA-vs-BIOS reservation logic
Both the intent and the effect of reserve_bios_regions() is simple:
reserve the range from the apparent BIOS start (suitably filtered) through 1MB and, if the EBDA start address is sensible, extend that reservation downward to cover the EBDA as well.
The code is overcomplicated, though, and contains head-scratchers like:
if (ebda_start < BIOS_START_MIN) ebda_start = BIOS_START_MAX;
That snipped is trying to say "if ebda_start < BIOS_START_MIN, ignore it".
Simplify it: reorder the code so that it makes sense. This should have no functional effect under any circumstances.
Signed-off-by Andy Lutomirski <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Kees Cook <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Luis R. Rodriguez <[email protected]>
Cc Mario Limonciello <[email protected]>
Cc Matthew Garrett <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Toshi Kani <[email protected]>
Link http://lkml.kernel.org/r/ef89c0c761be20ead8bd9a3275743e6259b6092a.1469135598.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/kernel/ebda.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-)
arch/x86/kernel/ebda.c +11 -23
@@ -65,22 +65,6 @@ void __init reserve_bios_regions(void)if (!x86_platform.legacy.reserve_bios_regions)return ; -/* Get the start address of the EBDA page: */ - ebda_start =get_bios_ebda (); - -/* -* Quirk: some old Dells seem to have a 4k EBDA without -* reporting so in their BIOS RAM size value, so just -* consider the memory above 640K to be off limits -* (bugzilla 2990). -* -* We detect this case by filtering for nonsensical EBDA -* addresses below 128K, where we can assume that they -* are bogus and bump it up to a fixed 640K value: -*/ -if (ebda_start <BIOS_START_MIN ) - ebda_start =BIOS_START_MAX ; -/* * BIOS RAM size is encoded in kilobytes, convert it * to bytes to get a first guess at where the BIOS
@@ -91,18 +75,22 @@ void __init reserve_bios_regions(void)/* * If bios_start is less than 128K, assume it is bogus -* and bump it up to 640K: +* and bump it up to 640K. Similarly, if bios_start is above 640K, +* don't trust it. */ -if (bios_start <BIOS_START_MIN ) +if (bios_start <BIOS_START_MIN || bios_start >BIOS_START_MAX ) bios_start =BIOS_START_MAX ; +/* Get the start address of the EBDA page: */ + ebda_start =get_bios_ebda (); +/* -* Use the lower of the bios_start and ebda_start -* as the starting point, but don't allow it to -* go beyond 640K: +* If the EBDA start address is sane and is below the BIOS region, +* then also reserve everything from the EBDA start address up to +* the BIOS region. */ - bios_start =min (bios_start, ebda_start); - bios_start =min (bios_start,BIOS_START_MAX ); +if (ebda_start >=BIOS_START_MIN && ebda_start < bios_start) + bios_start = ebda_start;/* Reserve all memory between bios_start and the 1MB mark: */ memblock_reserve (bios_start,0x100000 - bios_start);