x86/ebda: If the EBDA is in lowmem, reserve only 4k for the EBDA

Under some conditions, my Dell XPS 13 9350 puts the EBDA at 0x2c000 but reports the lowmem cutoff as 0. The old code reserves everything above 0x2c000 and I can't boot [1].

Due to an old quirk, we assume that lowmem ends at 0x9f000. Nonetheless, the old code would reserve everything from 0x2c000 to 0xfffff.

Be a little less conservative: when the EBDA is in lowmem, reserve 4k the EBDA and reserve highmem separately. On my laptop, this ends up being more or less a no-op: the EBDA shows up as a single-page of runtime data in the EFI memmap. Go Dell for getting this right.

[1] This only breaks boot in practice when some other firmware or GRUB oddity that I don't fully understand kicks in causing the memory below 0x2c000 to be unusable.

---

This is intentionally not tagged for -stable. I think it's -stable material *eventually*, but the problem that's fixed is not widespread (it's apparently just me for now) and there's plenty of potential to regress something that was worked around by the old code.

arch/x86/kernel/ebda.c | 6 ++++--

 1 file changed, 4 insertions(+), 2 deletions(-)

index afe65dffee80..2183a7eac646 100644
@@ -62,10 +62,12 @@ void __init reserve_ebda_region(void)
 	if (lowmem < INSANE_CUTOFF)
 		lowmem = LOWMEM_CAP;
 
-	/* Use the lower of the lowmem and EBDA markers as the cutoff */
-	lowmem = min(lowmem, ebda_addr);
 	lowmem = min(lowmem, LOWMEM_CAP); /* Absolute cap */
 
 	/* reserve all memory between lowmem and the 1MB mark */
 	memblock_reserve(lowmem, 0x100000 - lowmem);
+
+	/* if the EBDA is in lowmem, reserve it separately. */
+	if (ebda_addr < lowmem)
+		memblock_reserve(ebda_addr, 4096);
 }

2.7.4

Exactly why can't Linux boot if *more* memory is reserved? Is it perhaps the SMP trampoline that cannot be allocated? Or does this shift some firmware memory corruption pattern that makes it more lethal than it already is?

Without more information we don't really know the root cause here and I'm hesitant to touch such really old quirks without first understanding what's going on on your machine.

Is the boot failure deterministic - if yes, could you try to dig a bit more into this?

My guess it's the SMP trampoline, and I think we should robustify that in a different way: lets put it aside very early as a reservation (possibly in this very function), to guarantee that we have a below 1MB buffer for the SMP trampoline. This would be a lot more robust ...

We already reserve the first megabyte by default. There is something really bizarre about this; the bug report simply doesn't seem to make any sense.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Maybe the first megabyte reservation happens after the SMP trampoline allocation and hence the EBDA/BIOS reservation matters to our ability to allocate the SMP trampoline specifically?

Anyway, agreed that we need more information.

Thanks,

Ingo

Yes, of course - it has to. However, 0x2c000 although braindead shouldn't be a problem.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Btw., are you sure about this? We do reserve the first 64KB:

config X86_RESERVE_LOW
int "Amount of low memory, in kilobytes, to reserve for the BIOS" default 64
range 4 640
---help---
Specify the amount of low memory to reserve for the BIOS.

But I cannot find the place that allocates the first 1MB.

Thanks,

Ingo

I guess it helps if I attach the patch for real :) ...

=====================>

From: Ingo Molnar <[email protected]>
Date: Thu, 21 Jul 2016 09:53:52 +0200
Subject: [PATCH] x86/boot: Reorganize and clean up the BIOS area reservation code

So the reserve_ebda_region() code has accumulated a number of problems over the years that make it really difficult to read and understand:

- The calculation of 'lowmem' and 'ebda_addr' is an unnecessarily interleaved mess of first lowmem, then ebda_addr, then lowmem tweaks...

- 'lowmem' here means 'super low mem' - i.e. 16-bit addressable memory. In other parts of the x86 code 'lowmem' means 32-bit addressable memory... This makes it super confusing to read.

- It does not help at all that we have various memory range markers, half of which are 'start of range', half of which are 'end of range' - but this crucial property is not obvious in the naming at all ... gave me a headache trying to understand all this.

- Also, the 'ebda_addr' name sucks: it highlights that it's an address (which is obvious, all values here are addresses!), while it does not highlight that it's the _start_ of the EBDA region ...

- 'BIOS_LOWMEM_KILOBYTES' says a lot of things, except that this is the only value that is a pointer to a value, not a memory range address!

- The function name itself is a misnomer: it says 'reserve_ebda_region()' while its main purpose is to reserve all the firmware ROM typically between 640K and 1MB, while the 'EBDA' part is only a small part of that ...

- Likewise, the paravirt quirk flag name 'ebda_search' is misleading as well: this too should be about whether to reserve firmware areas in the paravirt case.

- In fact thinking about this as 'end of RAM' is confusing: what this function *really* wants to reserve is firmware data and code areas! Once the thinking is inverted from a mixed 'ram' and 'reserved firmware area' notion to a pure 'reserved area' notion everything becomes a lot clearer.

To improve all this rewrite the whole code (without changing the logic):

- Firstly invert the naming from 'lowmem end' to 'BIOS reserved area start' and propagate this concept through all the variable names and constants.

	BIOS_RAM_SIZE_KB_PTR		// was: BIOS_LOWMEM_KILOBYTES

	BIOS_START_MIN			// was: INSANE_CUTOFF

	ebda_start			// was: ebda_addr
	bios_start			// was: lowmem

	BIOS_START_MAX			// was: LOWMEM_CAP

- Then clean up the name of the function itself by renaming it to reserve_bios_regions() and renaming the ::ebda_search paravirt flag to ::reserve_bios_regions.

- Fix up all the comments (fix typos), harmonize and simplify their formulation and remove comments that become unnecessary due to the much better naming all around.

---
 arch/x86/include/asm/bios_ebda.h  |   2 +-
 arch/x86/include/asm/x86_init.h   |   4 +-
 arch/x86/kernel/ebda.c            | 124 +++++++++++++++++++++++++-------------
 arch/x86/kernel/head32.c          |   2 +-
 arch/x86/kernel/head64.c          |   2 +-
 arch/x86/kernel/platform-quirks.c |   4 +-
 6 files changed, 88 insertions(+), 50 deletions(-)

index 2b00c776f223..4b7b8e71607e 100644
@@ -17,7 +17,7 @@ static inline unsigned int get_bios_ebda(void)
 	return address;	/* 0 means none */
 }
 
-void reserve_ebda_region(void);
+void reserve_bios_regions(void);
 
 #ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
 /*

index 4dcdf74dfed8..c519c052700a 100644
@@ -168,14 +168,14 @@ struct x86_legacy_devices {
  * struct x86_legacy_features - legacy x86 features
  *
  * @rtc: this device has a CMOS real-time clock present
- * @ebda_search: it's safe to search for the EBDA signature in the hardware's
+ * @reserve_bios_regions: it's safe to search for the EBDA signature in the hardware's
  * 	low RAM
  * @devices: legacy x86 devices, refer to struct x86_legacy_devices
  * 	documentation for further details.
  */
 struct x86_legacy_features {
 	int rtc;
-	int ebda_search;
+	int reserve_bios_regions;
 	struct x86_legacy_devices devices;
 };
 

index d784bb547a9d..2dda0bc4576e 100644
@@ -26,7 +26,7 @@ static void __init i386_default_early_setup(void)
 	x86_init.resources.reserve_resources = i386_reserve_resources;
 	x86_init.mpparse.setup_ioapic_ids = setup_ioapic_ids_from_mpc;
 
-	reserve_ebda_region();
+	reserve_bios_regions();
 }
 
 asmlinkage __visible void __init i386_start_kernel(void)

index b72fb0b71dd1..99d48e7d2974 100644
@@ -183,7 +183,7 @@ void __init x86_64_start_reservations(char *real_mode_data)
 		copy_bootdata(__va(real_mode_data));
 
 	x86_early_init_platform_quirks();
-	reserve_ebda_region();
+	reserve_bios_regions();
 
 	switch (boot_params.hdr.hardware_subarch) {
 	case X86_SUBARCH_INTEL_MID:

index b2f8a33b36ff..24a50301f150 100644
@@ -7,12 +7,12 @@
 void __init x86_early_init_platform_quirks(void)
 {
 	x86_platform.legacy.rtc = 1;
-	x86_platform.legacy.ebda_search = 0;
+	x86_platform.legacy.reserve_bios_regions = 0;
 	x86_platform.legacy.devices.pnpbios = 1;
 
 	switch (boot_params.hdr.hardware_subarch) {
 	case X86_SUBARCH_PC:
-		x86_platform.legacy.ebda_search = 1;
+		x86_platform.legacy.reserve_bios_regions = 1;
 		break;
 	case X86_SUBARCH_XEN:
 	case X86_SUBARCH_LGUEST:

Yes, exactly.

It's mostly deterministic. I hit it every time if I use Dell's latest BIOS (1.4.4), enable SGX in BIOS (no SGX kernel patches involved), and boot using Fedora's grub2-efi on the hard disk. I don't hit it on a USB stick or if I boot using the EFI stub via the EFI shell. Using EFI shell causes 1000-27fff to be conventional memory instead of boot data -- see below.

Here's my memory map:

[    0.000000] efi: mem00: [Runtime Data       |RUN|  |  |  |  |  |  |   |WB|WT|
WC|UC] range=[0x0000000000000000-0x0000000000000fff] (0MB)
[    0.000000] efi: mem01: [Boot Data          |   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x0000000000001000-0x0000000000027fff] (0MB)
[    0.000000] efi: mem02: [Loader Data        |   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x0000000000028000-0x0000000000029fff] (0MB)
[    0.000000] efi: mem03: [Reserved           |   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x000000000002a000-0x000000000002bfff] (0MB)
[    0.000000] efi: mem04: [Runtime Data       |RUN|  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x000000000002c000-0x000000000002cfff] (0MB)
[    0.000000] efi: mem05: [Loader Data        |   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x000000000002d000-0x000000000002dfff] (0MB)
[    0.000000] efi: mem06: [Conventional Memory|   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x000000000002e000-0x0000000000057fff] (0MB)
[    0.000000] efi: mem07: [Reserved           |   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x0000000000058000-0x0000000000058fff] (0MB)
[    0.000000] efi: mem08: [Conventional Memory|   |  |  |  |  |  |  |
  |WB|WT|WC|UC] range=[0x0000000000059000-0x000000000009ffff] (0MB)

[

The EFI quirk to reserve boot data kills 1000-27fff. The EBDA reservation code kills the rest, leaving no <1MB memory at all.

If we really want to robustify that, I would suggest that we change the way that the trampoline works. In particular, I don't see any reason why we need to call setup_real_mode until we're actually ready to initialize APs, and we should be done with the boot services data quirk by then (am I right, Matt?). So if we can get the allocation code right, we shouldn't have any problem putting the trampoline in the boot services range.

It would be very easy to implement this if we could handle overlapping memblocks precisely or set a lower limit on the memblock allocator. Then we could block off everything below 1MB or 2MB very early and then unblock it or temporarily change the lower limit and ask for a single page for the trampoline after that.

So my suggestion was/is to _permanently_ allocate the SMP trampoline page, and leave it also reserved.

'Reserving' a memory area is really just a kernel internal matter. We can still use it. No need to unreserve/allocate/re-reserve ... unless I'm missing something.

Thanks,

Ingo

I don't think you're missing anything particularly deep. I'm just talking about an implementation issue. We need to make sure that the page we pick for the trampoline isn't reserved in the memory map or by some other quirk (including the EBDA). The kernel currently uses memblock for this, which means that we should probably play nicely with the memblock code.

To fix my laptop, though, I think we either need to change the EBDA reservation (i.e. be willing to pick a page above the EBDA but below the BIOS) or rework the code so that it can use a BOOT_SERVICES_DATA page.

--Andy

Oh... this is booting in EFI mode. Now it makes a bit more sense. This is a headache because I believe the same company has put out systems which crash if boot services memory is reclaimed before driver initialization, which is the only reason we can't just treat it as plain memory immediately after invoking ExitBootServices.

In theory EBDA in EFI mode is nonsense, too, but not trying to be smart about it might break some systems due to interaction with ACPI.

The real mode trampoline code isn't all that large; it is fundamentally limited to 64K but I think it is about 24K right now. This is why we have the 128K "insane" threshold: it is still big enough that we can fit.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

It's easy to make reserve_bios_regions() do nothing if we have an EFI memory map. Is there any decent reason *not* to do that? Or we could be more conservative: if the EFI memory map reserves 640K-1M and reserves the EBDA (if any), then don't reserve the range between the EBDA and 640K. (That would fix my laptop, too.)

Allowing the trampoline to use boot services addresses is fine, too, but that looks harder to implement.

I looked at the code some more. The boot services quirk is weird and maybe buggy. trim_snb_memory uses memblock_reserve to reserve the bottom 1MB. If efi_reserve_real_mode has already reserved that range, then trim_snb_memory's reservation will have no effect because the efi code will just free it later on. The same issue will hit any code that reserves >1MB memory after efi has tried to temporarily reserve it.

I don't have any great suggestions for cleaning it up. Perhaps the efi code should instead skip adding boot services memory to the memory map in the first place and then add it late and hand any unreserved bits to the buddy allocator?

--Andy

Yeah, that looks like a bug. memblock_reserve() reference counting, anyone?

The issue is that some data required at runtime may be contained in those boot services data regions; the EFI System Resource Table is a good example or the ACPI BGRT table. esrt_init() happens pretty early but efi_bgrt_init() is really late in boot because we need the ACPI subsystem to have been brought up.

Fundamentally, you can't know whether you can use the boot services regions for allocation until after SetVirtualAddressMap() has been called (the original bug that required the reservation quirks occurs at SVAM time) and after drivers have read the EFI config tables and marked their regions as reserved.

I suppose we could rewrite the page table mapping for those precious <1MB regions to coerce the firmware into accessing different pages instead of the 1:1 addresses and copy the regions elsewhere. Maybe. That assumes we don't hit other firmware bugs though.

.. it also assumes that firmware honors our page tables. Which at a minimum won't be true for things like SMM, which happens with paging entirely disabled.

And that's exactly the kind of code that touches some of the memory in the low 1M region - things like the legacy keyboard state bits in the low 4kB etc, for crazy old DOS days, iirc.

Linus

I still think my suggestion works. Let me clarify it:

The memblock allocator (AFAICT) has separate tracking of ranges that exist and ranges that are reserved. That is, there are four possible states a range can be in:

existing, non-reserved: these are available for use
existing, reserved: these ranges are present but either in use or blacklisted non-existent, reserved: not present but blacklisted anyway non-existent, non-reserved: nothing here

Currently, boot services data is marked as existing (because the e820 code thinks it's real memory) and reserved (because the EFI code reserves it).

I'm proposing that it work the other way around: the EFI and e820 code should, during early boot, treat it just like runtime data, reserved space, or non-present space: simply don't add it to the memory map in the first place. That will cause it to be non-existent and non-reserved. Nothing will clobber it because it's not available to the memblock allocator.

Then, in late boot, either add it back in to the memblock allocator. Then blacklisted portions will be reserved and non-blacklisted portions will be non-reserved. If this is after we switch from memblock to the normal page allocator, then the code will have to be structured differently, but the same concept applies.

IOW, just pretend that the boot services memory is initially not present and then treat it as hot-added memory after SVAM is done.

--Andy