Re: [edk2] Corrupted EFI region

[Laszlo Ersek <lersek@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4476 bytes --]

On 08/07/13 17:19, Borislav Petkov wrote:
> On Tue, Aug 06, 2013 at 05:31:29PM +0200, Laszlo Ersek wrote:
>> Can you capture the OVMF debug output? Do you see
>>
>>   ConvertPages: Incompatible memory types
>>
>> there?
>>
>> Can you set the following bits too in the debug mask?
>>
>> #define DEBUG_POOL      0x00000010  // Alloc & Free's
>> #define DEBUG_PAGE      0x00000020  // Alloc & Free's
> 
> Ok, I got debug output; I have to be careful now of not missing
> anything. Ok, so here we go:
> 
> First of all, I changed debugging mask to:
> 
>   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8010007F
> 
> (I just set all three bits you requested).
> 
> Using the new OVMF.id changed the addresses, of course, so we're looking
> at 0x7dc59XXX ones now.
> 
> [    0.000000] memblock_reserve: [0x0000007dc59018-0x0000007dc59618] efi_memblock_x86_reserve_range+0x70/0x75
> 
> So, I've attached an archive of the debug logs. The initial observations
> I could do is that the region still gets "squashed" to:
> 
> [    0.014041] efi: mem11: type=4, attr=0xf, range=[0x000000007dc59000-0x000000007dc59000) (0MB)
> 
> from
> 
> [    0.000000] efi: mem11: type=4, attr=0xf, range=[0x000000007dc59000-0x000000007e146000) (4MB)
> 
> And the interesting stuff in the OVMF output is right at the end:
> 
> ConvertRange: 7DC59000-7DC5AFFF to 4
> AddRange: 7DC59000-7DC5AFFF to 4
> AllocatePoolI: Type 4, Addr 7DC59018 (len 16F0) 26,735,072
> Jumping to kernel
> 
> We get that same output no matter if I boot it with "-enable-kvm" or
> not.
> 
> If the order of the debug messages is the same as the calls actually
> happen, we AllocatePoolI to address 7DC59018 which we already have added
> as a range. But I'm not going to pretend I even know the code so I'll
> let you comment instead :).

I think this allows us to solve the bug :)

First, forget everything I said :) I was completely lost.

Remember this?

01 efi_main()
02  exit_boot()
03    low_alloc()
04    GetMemoryMap()
05    ExitBootServices()
06
07 start_kernel()
08   setup_arch()
09    efi_memblock_x86_reserve_range()
10    efi_reserve_boot_services()
11  efi_enter_virtual_mode()
12    SetVirtualAddressMap()

Now, lines 01 to 05 *do not happen*.

More precisely, they don't happen in the kernel. They happen in the firmware. Specifically, "OvmfPkg/Library/LoadLinuxLib/Linux.c".

You're booting the kernel from the qemu command line. The kernel you run is also an "[o]ld kernel[] without EFI handover protocol". So what happens is, OVMF downloads the kernel image from qemu over fw_cfg, figures it's an old kernel...

PlatformBdsPolicyBehavior() [OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c]
  // Process QEMU's -kernel command line option:
  TryRunningQemuKernel()    [OvmfPkg/Library/PlatformBdsLib/QemuKernel.c]
    LoadLinux()             [OvmfPkg/Library/LoadLinuxLib/Linux.c]
      // Old kernels without EFI handover protocol
      SetupLinuxBootParams()
        SetupLinuxMemmap()
          AllocatePool() <-------------- !!!
          gBS->GetMemoryMap()
          gBS->ExitBootServices()
      prints "Jumping to kernel"
      JumpToKernel()

Now pull up efi_memblock_x86_reserve_range(). It reserves "boot_params.efi_info->efi_memmap".

I assumed this field would come from the exit_boot() kernel function. It doesn't. It comes from SetupLinuxMemmap(). The former allocates the backing store as EFI_LOADER_DATA. The latter, alas, marked with !!! above, as boot services data. :)

So, what you're seeing in the OVMF debug log:

> ConvertRange: 7DC59000-7DC5AFFF to 4
> AddRange: 7DC59000-7DC5AFFF to 4
> AllocatePoolI: Type 4, Addr 7DC59018 (len 16F0) 26,735,072

This is self-consistent. It just documents that the AllocatePool() call marked with !!! needs to grab two full pages first (two first lines), carve them up into pool chunks, and then serve the request from them (third line).

The address displayed here shows up in the linux dmesg later on because the storage for the memory map itself is allocated, and populated, by OVMF, not the EFI stub in the kernel.

In one sentence, efi_memblock_x86_reserve_range() expects that "boot_params.efi_info->efi_memmap" has been allocated as "loader data" (by whomever), but SetupLinuxMemmap() violates this by allocating the storage as "boot services data".

This leads to double reservation attempts between efi_memblock_x86_reserve_range(), and efi_reserve_boot_services().

The attached edk2 patch should fix it. Please confirm.

Thanks,
Laszlo


[-- Attachment #2: 0001-OvmfPkg-allocate-the-EFI-memory-map-for-Linux-as-Loa.patch --]
[-- Type: text/plain, Size: 1729 bytes --]

From 4a9e1f10fa2d06496f1983c25c47c6a1373d2f42 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Wed, 7 Aug 2013 19:39:30 +0200
Subject: [PATCH] OvmfPkg: allocate the EFI memory map for Linux as Loader Data

In Linux, efi_memblock_x86_reserve_range() and efi_reserve_boot_services()
expect that whoever allocates the EFI memmap allocates it in Loader Data
type memory. Linux's own exit_boot()-->low_alloc() complies, but
SetupLinuxMemmap() in LoadLinuxLib doesn't.

The memory type discrepancy leads to efi_memblock_x86_reserve_range() and
efi_reserve_boot_services() both trying to reserve the range backing the
memmap, resulting in memmap entry truncation in
efi_reserve_boot_services().

This fix also makes this allocation consistent with all other persistent
allocations in  "OvmfPkg/Library/LoadLinuxLib/Linux.c".

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/LoadLinuxLib/Linux.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Library/LoadLinuxLib/Linux.c b/OvmfPkg/Library/LoadLinuxLib/Linux.c
index cd673aa..4a3e2c1 100644
--- a/OvmfPkg/Library/LoadLinuxLib/Linux.c
+++ b/OvmfPkg/Library/LoadLinuxLib/Linux.c
@@ -280,8 +280,12 @@ SetupLinuxMemmap (
   // Enlarge space here, because we will allocate pool now.
   //
   MemoryMapSize += EFI_PAGE_SIZE;
-  MemoryMap = AllocatePool (MemoryMapSize);
-  ASSERT (MemoryMap != NULL);
+  Status = gBS->AllocatePool (
+                  EfiLoaderData,
+                  MemoryMapSize,
+                  (VOID **) &MemoryMap
+                  );
+  ASSERT_EFI_ERROR (Status);
 
   //
   // Get System MemoryMap
-- 
1.7.1