Re: [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls

From: Andrew Jones <drjones@redhat.com>
To: Zixuan Wang <zixuanwang@google.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, marcorr@google.com,
	baekhw@google.com, tmroeder@google.com, erdemaktas@google.com,
	rientjes@google.com, seanjc@google.com, brijesh.singh@amd.com,
	Thomas.Lendacky@amd.com, varad.gautam@suse.com, jroedel@suse.de,
	bp@suse.de
Subject: Re: [kvm-unit-tests PATCH v2 02/17] x86 UEFI: Implement UEFI function calls
Date: Tue, 21 Sep 2021 18:43:34 +0200	[thread overview]
Message-ID: <20210921164334.zo6bbi77hbh2vdjz@gator.home> (raw)
In-Reply-To: <20210827031222.2778522-3-zixuanwang@google.com>

On Fri, Aug 27, 2021 at 03:12:07AM +0000, Zixuan Wang wrote:
> From: Varad Gautam <varad.gautam@suse.com>
> 
> This commit implements helper functions that call UEFI services and
> assist the boot up process.
> 
> Signed-off-by: Varad Gautam <varad.gautam@suse.com>
> ---
>  lib/efi.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 lib/efi.c
> 
> diff --git a/lib/efi.c b/lib/efi.c
> new file mode 100644
> index 0000000..9711354
> --- /dev/null
> +++ b/lib/efi.c
> @@ -0,0 +1,58 @@
> +#include <linux/uefi.h>

Please add at least an SPDX header.

> +
> +unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab);
> +efi_system_table_t *efi_system_table = NULL;
> +
> +static void efi_free_pool(void *ptr)
> +{
> +	efi_bs_call(free_pool, ptr);
> +}
> +
> +static efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> +{
> +	efi_memory_desc_t *m = NULL;
> +	efi_status_t status;
> +	unsigned long key = 0, map_size = 0, desc_size = 0;
> +
> +	status = efi_bs_call(get_memory_map, &map_size,
> +			     NULL, &key, &desc_size, NULL);
> +	if (status != EFI_BUFFER_TOO_SMALL || map_size == 0)
> +		goto out;
> +
> +	/* Pad map_size with additional descriptors so we don't need to
> +	 * retry. */

nit: please use Linux comment style

> +	map_size += 4 * desc_size;
> +	*map->buff_size = map_size;
> +	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
> +			     map_size, (void **)&m);
> +	if (status != EFI_SUCCESS)
> +		goto out;
> +
> +	/* Get the map. */
> +	status = efi_bs_call(get_memory_map, &map_size,
> +			     m, &key, &desc_size, NULL);
> +	if (status != EFI_SUCCESS) {
> +		efi_free_pool(m);
> +		goto out;
> +	}
> +
> +	*map->desc_size = desc_size;
> +	*map->map_size = map_size;
> +	*map->key_ptr = key;
> +out:
> +	*map->map = m;
> +	return status;
> +}
> +
> +static efi_status_t efi_exit_boot_services(void *handle,
> +					   struct efi_boot_memmap *map)
> +{
> +	return efi_bs_call(exit_boot_services, handle, *map->key_ptr);
> +}
> +
> +unsigned long __efiapi efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> +{
> +	efi_system_table = sys_tab;
> +
> +	return 0;
> +}
> -- 
> 2.33.0.259.gc128427fd7-goog
>

Otherwise

Reviewed-by: Andrew Jones <drjones@redhat.com>