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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.05.15 at 15:30, <ross.lagerwall@xxxxxxxxxx> 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(L"Unable 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(L"Unable to allocate memory for EFI memory 
> map");
> +                else
> +                    blexit(L"Unable 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel