| Message ID | 20240325213623.747590-2-papaluri@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,RFC,1/3] x86 EFI: Bypass call to fdt_check_header() | expand |
On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote: > In some cases, KUT guest might fail to exit boot services due to a > possible memory map update that might have taken place between > efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI > spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need > to update the memory map and retry call to exit boot > services. > > Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com> > --- > lib/efi.c | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/lib/efi.c b/lib/efi.c > index 124e77685230..9d066bfad0b6 100644 > --- a/lib/efi.c > +++ b/lib/efi.c > @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab) > } > #endif > > - /* > + /* > * Exit EFI boot services, let kvm-unit-tests take full control of the > - * guest > + * guest. > */ > status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map); > - if (status != EFI_SUCCESS) { > - printf("Failed to exit boot services\n"); > - goto efi_main_error; > + > + /* > + * There is a possibility that memory map might have changed > + * between efi_get_memory_map() and efi_exit_boot_services in > + * which case status is EFI_INVALID_PARAMETER. As per UEFI spec > + * 2.10, we need to get the updated memory map and try again. > + */ > + if (status == EFI_INVALID_PARAMETER) { Shouldn't we loop on this? The spec doesn't make it clear that the second try should always work. > + efi_get_memory_map(&efi_bootinfo.mem_map); > + > + status = efi_exit_boot_services(handle, > + &efi_bootinfo.mem_map); > + if (status != EFI_SUCCESS) { > + printf("Failed to exit boot services\n"); > + goto efi_main_error; > + } > } > > /* Set up arch-specific resources */ > -- > 2.34.1 > Thanks, drew
On 3/26/2024 3:57 AM, Andrew Jones wrote: > On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote: >> In some cases, KUT guest might fail to exit boot services due to a >> possible memory map update that might have taken place between >> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI >> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need >> to update the memory map and retry call to exit boot >> services. >> >> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com> >> --- >> lib/efi.c | 23 ++++++++++++++++++----- >> 1 file changed, 18 insertions(+), 5 deletions(-) >> >> diff --git a/lib/efi.c b/lib/efi.c >> index 124e77685230..9d066bfad0b6 100644 >> --- a/lib/efi.c >> +++ b/lib/efi.c >> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab) >> } >> #endif >> >> - /* >> + /* >> * Exit EFI boot services, let kvm-unit-tests take full control of the >> - * guest >> + * guest. >> */ >> status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map); >> - if (status != EFI_SUCCESS) { >> - printf("Failed to exit boot services\n"); >> - goto efi_main_error; >> + >> + /* >> + * There is a possibility that memory map might have changed >> + * between efi_get_memory_map() and efi_exit_boot_services in >> + * which case status is EFI_INVALID_PARAMETER. As per UEFI spec >> + * 2.10, we need to get the updated memory map and try again. >> + */ >> + if (status == EFI_INVALID_PARAMETER) { > > Shouldn't we loop on this? The spec doesn't make it clear that the second > try should always work. > Indeed, will plug in a do-while to loop on it. Thanks, Pavan >> + efi_get_memory_map(&efi_bootinfo.mem_map); >> + >> + status = efi_exit_boot_services(handle, >> + &efi_bootinfo.mem_map); >> + if (status != EFI_SUCCESS) { >> + printf("Failed to exit boot services\n"); >> + goto efi_main_error; >> + } >> } >> >> /* Set up arch-specific resources */ >> -- >> 2.34.1 >> > > Thanks, > drew
On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote: > In some cases, KUT guest might fail to exit boot services due to a > possible memory map update that might have taken place between > efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI > spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need > to update the memory map and retry call to exit boot > services. > > Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com> > --- > lib/efi.c | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/lib/efi.c b/lib/efi.c > index 124e77685230..9d066bfad0b6 100644 > --- a/lib/efi.c > +++ b/lib/efi.c > @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab) > } > #endif > > - /* > + /* > * Exit EFI boot services, let kvm-unit-tests take full control of the > - * guest > + * guest. > */ > status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map); > - if (status != EFI_SUCCESS) { > - printf("Failed to exit boot services\n"); > - goto efi_main_error; With this change, error codes other than EFI_INVALID_PARAMETER are only handled if the first failure is EFI_INVALID_PARAMETER. Need to to re-add the previous handling for when the first EBS failure is something other than EFI_INVALID_PARAMETER. -Mike > + > + /* > + * There is a possibility that memory map might have changed > + * between efi_get_memory_map() and efi_exit_boot_services in > + * which case status is EFI_INVALID_PARAMETER. As per UEFI spec > + * 2.10, we need to get the updated memory map and try again. > + */ > + if (status == EFI_INVALID_PARAMETER) { > + efi_get_memory_map(&efi_bootinfo.mem_map); > + > + status = efi_exit_boot_services(handle, > + &efi_bootinfo.mem_map); > + if (status != EFI_SUCCESS) { > + printf("Failed to exit boot services\n"); > + goto efi_main_error; > + } > } > > /* Set up arch-specific resources */ > -- > 2.34.1 >
Hi Mike, On 3/26/2024 8:38 AM, Michael Roth wrote: > On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote: >> In some cases, KUT guest might fail to exit boot services due to a >> possible memory map update that might have taken place between >> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI >> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need >> to update the memory map and retry call to exit boot >> services. >> >> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com> >> --- >> lib/efi.c | 23 ++++++++++++++++++----- >> 1 file changed, 18 insertions(+), 5 deletions(-) >> >> diff --git a/lib/efi.c b/lib/efi.c >> index 124e77685230..9d066bfad0b6 100644 >> --- a/lib/efi.c >> +++ b/lib/efi.c >> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab) >> } >> #endif >> >> - /* >> + /* >> * Exit EFI boot services, let kvm-unit-tests take full control of the >> - * guest >> + * guest. >> */ >> status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map); >> - if (status != EFI_SUCCESS) { >> - printf("Failed to exit boot services\n"); >> - goto efi_main_error; > > With this change, error codes other than EFI_INVALID_PARAMETER are only > handled if the first failure is EFI_INVALID_PARAMETER. Need to to re-add > the previous handling for when the first EBS failure is something other > than EFI_INVALID_PARAMETER. > But the status codes that could be returned from efi_exit_boot_services() are only EFI_INVALID_PARAMETER/EFI_SUCCESS [1]. [1] UEFI 2.10 Section 7.4.6. Thanks, Pavan > -Mike > >> + >> + /* >> + * There is a possibility that memory map might have changed >> + * between efi_get_memory_map() and efi_exit_boot_services in >> + * which case status is EFI_INVALID_PARAMETER. As per UEFI spec >> + * 2.10, we need to get the updated memory map and try again. >> + */ >> + if (status == EFI_INVALID_PARAMETER) { >> + efi_get_memory_map(&efi_bootinfo.mem_map); >> + >> + status = efi_exit_boot_services(handle, >> + &efi_bootinfo.mem_map); >> + if (status != EFI_SUCCESS) { >> + printf("Failed to exit boot services\n"); >> + goto efi_main_error; >> + } >> } >> >> /* Set up arch-specific resources */ >> -- >> 2.34.1 >>
On Tue, Mar 26, 2024 at 08:45:53AM -0500, Paluri, PavanKumar wrote: > Hi Mike, > > On 3/26/2024 8:38 AM, Michael Roth wrote: > > On Mon, Mar 25, 2024 at 04:36:22PM -0500, Pavan Kumar Paluri wrote: > >> In some cases, KUT guest might fail to exit boot services due to a > >> possible memory map update that might have taken place between > >> efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI > >> spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need > >> to update the memory map and retry call to exit boot > >> services. > >> > >> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com> > >> --- > >> lib/efi.c | 23 ++++++++++++++++++----- > >> 1 file changed, 18 insertions(+), 5 deletions(-) > >> > >> diff --git a/lib/efi.c b/lib/efi.c > >> index 124e77685230..9d066bfad0b6 100644 > >> --- a/lib/efi.c > >> +++ b/lib/efi.c > >> @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab) > >> } > >> #endif > >> > >> - /* > >> + /* > >> * Exit EFI boot services, let kvm-unit-tests take full control of the > >> - * guest > >> + * guest. > >> */ > >> status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map); > >> - if (status != EFI_SUCCESS) { > >> - printf("Failed to exit boot services\n"); > >> - goto efi_main_error; > > > > With this change, error codes other than EFI_INVALID_PARAMETER are only > > handled if the first failure is EFI_INVALID_PARAMETER. Need to to re-add > > the previous handling for when the first EBS failure is something other > > than EFI_INVALID_PARAMETER. > > > But the status codes that could be returned from > efi_exit_boot_services() are only EFI_INVALID_PARAMETER/EFI_SUCCESS [1]. > > [1] UEFI 2.10 Section 7.4.6. New error codes can be added over time. Also, the block you added handles !EFI_SUCCESS generally for the 2nd call, so for consistency it makes sense to continue handling it similarly for the 1st call as well. -Mike > > Thanks, > Pavan > > > -Mike > > > >> + > >> + /* > >> + * There is a possibility that memory map might have changed > >> + * between efi_get_memory_map() and efi_exit_boot_services in > >> + * which case status is EFI_INVALID_PARAMETER. As per UEFI spec > >> + * 2.10, we need to get the updated memory map and try again. > >> + */ > >> + if (status == EFI_INVALID_PARAMETER) { > >> + efi_get_memory_map(&efi_bootinfo.mem_map); > >> + > >> + status = efi_exit_boot_services(handle, > >> + &efi_bootinfo.mem_map); > >> + if (status != EFI_SUCCESS) { > >> + printf("Failed to exit boot services\n"); > >> + goto efi_main_error; > >> + } > >> } > >> > >> /* Set up arch-specific resources */ > >> -- > >> 2.34.1 > >>
diff --git a/lib/efi.c b/lib/efi.c index 124e77685230..9d066bfad0b6 100644 --- a/lib/efi.c +++ b/lib/efi.c @@ -458,14 +458,27 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab) } #endif - /* + /* * Exit EFI boot services, let kvm-unit-tests take full control of the - * guest + * guest. */ status = efi_exit_boot_services(handle, &efi_bootinfo.mem_map); - if (status != EFI_SUCCESS) { - printf("Failed to exit boot services\n"); - goto efi_main_error; + + /* + * There is a possibility that memory map might have changed + * between efi_get_memory_map() and efi_exit_boot_services in + * which case status is EFI_INVALID_PARAMETER. As per UEFI spec + * 2.10, we need to get the updated memory map and try again. + */ + if (status == EFI_INVALID_PARAMETER) { + efi_get_memory_map(&efi_bootinfo.mem_map); + + status = efi_exit_boot_services(handle, + &efi_bootinfo.mem_map); + if (status != EFI_SUCCESS) { + printf("Failed to exit boot services\n"); + goto efi_main_error; + } } /* Set up arch-specific resources */
In some cases, KUT guest might fail to exit boot services due to a possible memory map update that might have taken place between efi_get_memory_map() and efi_exit_boot_services() calls. As per UEFI spec 2.10 (Section 7.4.6 EFI_BOOT_SERVICES.ExitBootServices()), we need to update the memory map and retry call to exit boot services. Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com> --- lib/efi.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)