[Xen-devel] [PATCH v2] efi: Reallocate memory map if ExitBootServices() fails

[Ross Lagerwall]

If calling ExitBootServices() fails, the memory map size may increase.
Allocate an extra page the first time around so that we hopefully
shouldn't have to reallocate the memory map. Allocate a new memory map
if needed, although this only a fallback, because some arches may use
boot services to allocate memory which may not work after the first call
to ExitBootServices().

This was seen on the following machine when using the iscsidxe UEFI
driver. The machine would consistently fail the first call to
ExitBootServices().
System Information
Manufacturer: Supermicro
Product Name: X10SLE-F/HF
BIOS Information
Vendor: American Megatrends Inc.
Version: 2.00
Release Date: 04/24/2014

Signed-off-by: Ross Lagerwall ross.lagerw...@citrix.com
---
 xen/common/efi/boot.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index ef8476c..7d09b39 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -700,7 +700,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
 EFI_STATUS status;
 unsigned int i, argc;
 CHAR16 **argv, *file_name, *cfg_file_name = NULL, *options = NULL;
-UINTN map_key, info_size, gop_mode = ~0;
+UINTN memmap_size, map_key, info_size, gop_mode = ~0;
 EFI_HANDLE *handles = NULL;
 EFI_SHIM_LOCK_PROTOCOL *shim_lock;
 EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
@@ -1053,14 +1053,23 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
 efi_arch_video_init(gop, info_size, mode_info);
 }
 
-efi_bs-GetMemoryMap(efi_memmap_size, NULL, map_key,
- efi_mdesc_size, mdesc_ver);
-efi_memmap = efi_arch_allocate_mmap_buffer(efi_memmap_size);
-if ( !efi_memmap )
-blexit(LUnable to allocate memory for EFI memory map);
-
 for ( retry = 0; ; retry = 1 )
 {
+efi_bs-GetMemoryMap(memmap_size, NULL, map_key,
+ efi_mdesc_size, mdesc_ver);
+if ( memmap_size  efi_memmap_size )
+{
+efi_memmap_size = memmap_size + EFI_PAGE_SIZE;
+efi_memmap = efi_arch_allocate_mmap_buffer(efi_memmap_size);
+if ( !efi_memmap )
+{
+if ( retry )
+PrintErr(LUnable to allocate memory for EFI memory map);
+else
+blexit(LUnable to allocate memory for EFI memory map);
+}
+}
+
 status = efi_bs-GetMemoryMap(efi_memmap_size, efi_memmap, map_key,
   efi_mdesc_size, mdesc_ver);
 if ( EFI_ERROR(status) )
-- 
2.1.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] efi: Reallocate memory map if ExitBootServices() fails

[Jan Beulich]

 On 29.05.15 at 15:30, ross.lagerw...@citrix.com wrote:
 @@ -1053,14 +1053,23 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
 *SystemTable)
  efi_arch_video_init(gop, info_size, mode_info);
  }
  
 -efi_bs-GetMemoryMap(efi_memmap_size, NULL, map_key,
 - efi_mdesc_size, mdesc_ver);
 -efi_memmap = efi_arch_allocate_mmap_buffer(efi_memmap_size);
 -if ( !efi_memmap )
 -blexit(LUnable to allocate memory for EFI memory map);
 -
  for ( retry = 0; ; retry = 1 )
  {
 +efi_bs-GetMemoryMap(memmap_size, NULL, map_key,
 + efi_mdesc_size, mdesc_ver);
 +if ( memmap_size  efi_memmap_size )
 +{
 +efi_memmap_size = memmap_size + EFI_PAGE_SIZE;

I'd really like to make sure the value is a multiple of efi_mdesc_size
(i.e. simply adding EFI_PAGE_SIZE will not do). And that's leaving
aside whether the increase really needs to be that big.

 +efi_memmap = efi_arch_allocate_mmap_buffer(efi_memmap_size);

So considering the ARM side (which this breaks) - did you try at all
the alternative of allocating a slightly larger memory map ahead of
the loop?

 +if ( !efi_memmap )
 +{
 +if ( retry )
 +PrintErr(LUnable to allocate memory for EFI memory 
 map);
 +else
 +blexit(LUnable to allocate memory for EFI memory map);
 +}

Looking at this again (and remembering that PrintErrMesg(), as used
a few lines down, also calls blexit()), I think we should handle this
inside blexit(): If ExitBootServices() was already used enter an
infinite loop right after printing the message(s). I wonder whether,
to signal this, we shouldn't set efi_bs to NULL right after calling
ExitBootServices(), replacing the single remaining valid use with
SystemTable-BootServices-GetMemoryMap().

In no case should you allow execution to fall through here.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel