[PATCH] efi_loader: improve detection of ESP for storing UEFI variables

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 09.11.20 00:58, Paulo Alcantara wrote:
> The UEFI specification does not restrict on the number and location of
> ESPs in a system.  They are discovered as required by looking at the
> partition type, but firmware implementations are allowed to support
> ESPs which do not contain a valid partition type.

I guess you refer to chapter "13.3.3 Number and Location of System
Partitions" of the UEFI spec saying: "Further, UEFI implementations may
allow the use of conforming FAT partitions which do not use the ESP GUID."

Why should U-Boot support FAT partitions that are not of type FAT 0xef
and GPT partition that do not use the ESP GUID?

Best regards

Heinrich

>
> Besides checking for the partition type, for non-removable media check
> if /EFI directory exists, otherwise check if /EFI/BOOT/BOOT{ARCH}.EFI
> file exists as specified in UEFI 2.8 "13.3.1.3 Directory Structure".
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  lib/efi_loader/efi_disk.c | 60 +++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 22 deletions(-)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 7bd1ccec4501..2940a2edf2e8 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -335,6 +335,35 @@ static int efi_fs_exists(struct blk_desc *desc, int part)
>  	return 1;
>  }
>
> +/**
> + * efi_part_is_esp() - check if a partition is an EFI system partition
> + *
> + * @desc:	block device descriptor
> + * @part:	partition number
> + * Return:	true if a partition is an EFI system partition
> + *		false otherwise
> + */
> +static bool efi_part_is_esp(struct blk_desc *desc, int part)
> +{
> +	int ret;
> +	struct disk_partition info;
> +
> +	ret = part_get_info(desc, part, &info);
> +	if (ret)
> +		return false;
> +
> +	if (info.bootable & PART_EFI_SYSTEM_PARTITION)
> +		return true;
> +
> +	if (fs_set_blk_dev_with_part(desc, part))
> +		return false;
> +
> +	if (!desc->removable)
> +		return !!fs_exists("/EFI");
> +
> +	return !!fs_exists("/EFI/BOOT/" BOOTEFI_NAME);
> +}
> +
>  /**
>   * efi_disk_add_dev() - create a handle for a partition or disk
>   *
> @@ -436,21 +465,14 @@ static efi_status_t efi_disk_add_dev(
>  		*disk = diskobj;
>
>  	/* Store first EFI system partition */
> -	if (part && !efi_system_partition.if_type) {
> -		int r;
> -		struct disk_partition info;
> -
> -		r = part_get_info(desc, part, &info);
> -		if (r)
> -			return EFI_DEVICE_ERROR;
> -		if (info.bootable & PART_EFI_SYSTEM_PARTITION) {
> -			efi_system_partition.if_type = desc->if_type;
> -			efi_system_partition.devnum = desc->devnum;
> -			efi_system_partition.part = part;
> -			EFI_PRINT("EFI system partition: %s %d:%d\n",
> -				  blk_get_if_type_name(desc->if_type),
> -				  desc->devnum, part);
> -		}
> +	if (part && !efi_system_partition.if_type &&
> +	    efi_part_is_esp(desc, part)) {
> +		efi_system_partition.if_type = desc->if_type;
> +		efi_system_partition.devnum = desc->devnum;
> +		efi_system_partition.part = part;
> +		EFI_PRINT("EFI system partition: %s %d:%d\n",
> +			  blk_get_if_type_name(desc->if_type),
> +			  desc->devnum, part);
>  	}
>  	return EFI_SUCCESS;
>  }
> @@ -614,9 +636,7 @@ bool efi_disk_is_system_part(efi_handle_t handle)
>  {
>  	struct efi_handler *handler;
>  	struct efi_disk_obj *diskobj;
> -	struct disk_partition info;
>  	efi_status_t ret;
> -	int r;
>
>  	/* check if this is a block device */
>  	ret = efi_search_protocol(handle, &efi_block_io_guid, &handler);
> @@ -625,9 +645,5 @@ bool efi_disk_is_system_part(efi_handle_t handle)
>
>  	diskobj = container_of(handle, struct efi_disk_obj, header);
>
> -	r = part_get_info(diskobj->desc, diskobj->part, &info);
> -	if (r)
> -		return false;
> -
> -	return !!(info.bootable & PART_EFI_SYSTEM_PARTITION);
> +	return efi_part_is_esp(diskobj->desc, diskobj->part);
>  }
>