Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services

From: "Paluri, PavanKumar" <papaluri@amd.com>
To: Michael Roth <michael.roth@amd.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, andrew.jones@linux.dev,
	nikos.nikoleris@arm.com, thomas.lendacky@amd.com,
	amit.shah@amd.com
Subject: Re: [kvm-unit-tests RFC PATCH 2/3] x86/efi: Retry call to efi exit boot services
Date: Tue, 26 Mar 2024 08:45:53 -0500	[thread overview]
Message-ID: <4c2c7598-2c07-0cd6-620c-1a603bcb1f0f@amd.com> (raw)
In-Reply-To: <20240326133801.p4eqegjor54mn3h5@amd.com>

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
>>