Re: [Xen-devel] [PATCH] efi: Reallocate memory map if ExitBootServices() fails
>>> On 29.05.15 at 11:57, wrote:
> On 05/29/2015 10:45 AM, Jan Beulich wrote:
> On 29.05.15 at 09:48, wrote:
>>> --- a/xen/common/efi/boot.c
>>> +++ b/xen/common/efi/boot.c
>>> @@ -1053,14 +1053,14 @@ 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(&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");
>>
>> You can't blexit() anymore after having called ExitBootServices() once.
>> Admittedly even the PrintErrMesg() used for "error handling" a few
>> lines down is on the edge of being invalid (but this is a best effort
>> thing anyway).
>
> OK, I'll convert it into a PrintErrMesg.
Conditionally upon being in the second iteration.
>> Since, finally, this is only a workaround, as the spec clearly says:
>> "After an Operating System calls ExitBootServices(), firmware boot
>> services are no longer available and it is illegal to call any boot
>> service." Even our re-invocation of GetMemoryMap() is bending
>> that (and I only hesitantly agreed for it to be added).
>
> The spec kinda disagrees with that:
> "It is suggested that GetMemoryMap()be called immediately before calling
> ExitBootServices(). If MapKey value is incorrect, ExitBootServices()
> returns EFI_INVALID_PARAMETER and GetMemoryMap() with ExitBootServices()
> must be called again. Firmware implementation may choose to do a partial
> shutdown of the boot services during the first call to
> ExitBootServices(). EFI OS loader should not make calls to any boot
> service function other then GetMemoryMap() after the first call to
> ExitBootServices()."
This was the grounds on which the current retry loop was added.
> I think the patch does the right thing in the regard.
For x86 yes, since efi_arch_allocate_mmap_buffer() doesn't use
boot services there. For ARM no, due to its use of AllocatePool().
Iirc Daniel was also planning on changing the x86 side too, and
such a change would break things in non-obvious ways. Hence I
think depending on the ability to do another allocation after the
first ExitBootServices() call is not really a good idea.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] efi: Reallocate memory map if ExitBootServices() fails
On 05/29/2015 10:45 AM, Jan Beulich wrote:
On 29.05.15 at 09:48, wrote:
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1053,14 +1053,14 @@ 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(&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");
You can't blexit() anymore after having called ExitBootServices() once.
Admittedly even the PrintErrMesg() used for "error handling" a few
lines down is on the edge of being invalid (but this is a best effort
thing anyway).
OK, I'll convert it into a PrintErrMesg.
Further you should do a second allocation only if you positively
identified that the new size is larger than what the existing buffer
can hold. It may be worth allocating a couple of extra entries the
first time through anyway; perhaps that would even avoid th need
for this workaround.
OK.
Since, finally, this is only a workaround, as the spec clearly says:
"After an Operating System calls ExitBootServices(), firmware boot
services are no longer available and it is illegal to call any boot
service." Even our re-invocation of GetMemoryMap() is bending
that (and I only hesitantly agreed for it to be added).
The spec kinda disagrees with that:
"It is suggested that GetMemoryMap()be called immediately before calling
ExitBootServices(). If MapKey value is incorrect, ExitBootServices()
returns EFI_INVALID_PARAMETER and GetMemoryMap() with ExitBootServices()
must be called again. Firmware implementation may choose to do a partial
shutdown of the boot services during the first call to
ExitBootServices(). EFI OS loader should not make calls to any boot
service function other then GetMemoryMap() after the first call to
ExitBootServices()."
I think the patch does the right thing in the regard.
--
Ross Lagerwall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] efi: Reallocate memory map if ExitBootServices() fails
>>> On 29.05.15 at 09:48, wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1053,14 +1053,14 @@ 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(&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");
You can't blexit() anymore after having called ExitBootServices() once.
Admittedly even the PrintErrMesg() used for "error handling" a few
lines down is on the edge of being invalid (but this is a best effort
thing anyway).
Further you should do a second allocation only if you positively
identified that the new size is larger than what the existing buffer
can hold. It may be worth allocating a couple of extra entries the
first time through anyway; perhaps that would even avoid th need
for this workaround.
Since, finally, this is only a workaround, as the spec clearly says:
"After an Operating System calls ExitBootServices(), firmware boot
services are no longer available and it is illegal to call any boot
service." Even our re-invocation of GetMemoryMap() is bending
that (and I only hesitantly agreed for it to be added).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] efi: Reallocate memory map if ExitBootServices() fails
On 29/05/15 08:48, Ross Lagerwall wrote:
> If calling ExitBootServices() fails, the memory map size may have
> increased, so determine the new size and reallocate the memory map
> before calling GetMemoryMap() again.
>
> 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
Reviewed-by: Andrew Cooper
> ---
> xen/common/efi/boot.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index ef8476c..078f9b8 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1053,14 +1053,14 @@ 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(&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");
> +
> status = efi_bs->GetMemoryMap(&efi_memmap_size, efi_memmap, &map_key,
>&efi_mdesc_size, &mdesc_ver);
> if ( EFI_ERROR(status) )
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] efi: Reallocate memory map if ExitBootServices() fails
If calling ExitBootServices() fails, the memory map size may have
increased, so determine the new size and reallocate the memory map
before calling GetMemoryMap() again.
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
---
xen/common/efi/boot.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index ef8476c..078f9b8 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1053,14 +1053,14 @@ 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(&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");
+
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
