[PATCH v2 2/2] efi_loader: identify EFI system partition

[Heinrich Schuchardt]

Am April 14, 2020 5:20:38 AM UTC schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
>Heinrich,
>
>On Mon, Apr 06, 2020 at 02:31:35PM +0900, AKASHI Takahiro wrote:
>> On Mon, Apr 06, 2020 at 07:12:56AM +0200, Heinrich Schuchardt wrote:
>> > On 4/6/20 6:21 AM, AKASHI Takahiro wrote:
>> > > Heinrich,
>> > >
>> > > On Sun, Apr 05, 2020 at 11:28:18AM +0200, Heinrich Schuchardt
>wrote:
>> > >> For capsule updates we need to identify the EFI system
>partition.
>> > >
>> > > Right, but
>> > >
>> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> > >> ---
>> > >> v2:
>> > >> 	no change
>> > >> ---
>> > >>  include/efi_loader.h      |  7 +++++++
>> > >>  lib/efi_loader/efi_disk.c | 20 ++++++++++++++++++++
>> > >>  2 files changed, 27 insertions(+)
>> > >>
>> > >> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> > >> index 3f2792892f..4a45033476 100644
>> > >> --- a/include/efi_loader.h
>> > >> +++ b/include/efi_loader.h
>> > >> @@ -45,6 +45,13 @@ static inline void *guidcpy(void *dst, const
>void *src)
>> > >>  /* Root node */
>> > >>  extern efi_handle_t efi_root;
>> > >>
>> > >> +/* EFI system partition */
>> > >> +extern struct efi_system_partition {
>> > >> +	enum if_type if_type;
>> > >> +	int devnum;
>> > >> +	u8 part;
>> > >> +} efi_system_partition;
>> > >> +
>> > >>  int __efi_entry_check(void);
>> > >>  int __efi_exit_check(void);
>> > >>  const char *__efi_nesting(void);
>> > >> diff --git a/lib/efi_loader/efi_disk.c
>b/lib/efi_loader/efi_disk.c
>> > >> index fc0682bc48..2f752a5e99 100644
>> > >> --- a/lib/efi_loader/efi_disk.c
>> > >> +++ b/lib/efi_loader/efi_disk.c
>> > >> @@ -13,6 +13,8 @@
>> > >>  #include <part.h>
>> > >>  #include <malloc.h>
>> > >>
>> > >> +struct efi_system_partition efi_system_partition;
>> > >> +
>> > >>  const efi_guid_t efi_block_io_guid =
>EFI_BLOCK_IO_PROTOCOL_GUID;
>> > >>
>> > >>  /**
>> > >> @@ -372,6 +374,24 @@ static efi_status_t efi_disk_add_dev(
>> > >>  	diskobj->ops.media = &diskobj->media;
>> > >>  	if (disk)
>> > >>  		*disk = diskobj;
>> > >> +
>> > >> +	/* Store first EFI system partition */
>> > >
>> > > I don't think that the policy, first comes first serves as system
>> > > partition, is a right decision as
>> > > - the order of device probe on U-Boot is indeterministic, and
>> > 
>> > Indeterministic would mean that on two runs with the same media
>provided
>> > you will get different results. I cannot see any source for such
>> > randomness in the U-Boot code. In dm_init_and_scan() the device
>tree is
>> > scanned and drivers and bound in the sequence of occurrence in the
>> > device tree.
>> > 
>> > > - there can be several partitions that hold a system partition
>bit.
>> > >   You may have OS installed on eMMC, but also may have bootable
>DVD
>> > >   on the system.
>> > 
>> > This is a similar logic like finding the relevant boot.scr script
>to run.
>> > 
>> > What would be the alternative?
>> 
>> I think that most UEFI systems have ability for user to specify
>> "boot order."
>
>Any comment?
>The discussion and your patch will have some impact on
>my efi capsule patch.

Concerning capsules the spec says we should use the boot device. So my patch doesn't help you there.

For the storage of variables I still need this patch. I will adjust the commit message.

Best regards

Heinrich

>
>-Takahiro Akashi
>
>> -Takahiro Akashi
>> > 
>> > Definition via Kconfig would mean that a Linux distribution like
>Debian
>> > would have to provide a separate U-Boot build for each boot medium
>that
>> > a user might possibly use (eMMC, SD-card, USB, NVME, SCSI).
>> > 
>> > Best regards
>> > 
>> > Heinrich
>> > 
>> > >
>> > > -Takahiro Akashi
>> > >
>> > >> +	if (part && !efi_system_partition.if_type) {
>> > >> +		int r;
>> > >> +		disk_partition_t 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);
>> > >> +		}
>> > >> +	}
>> > >>  	return EFI_SUCCESS;
>> > >>  }
>> > >>
>> > >> --
>> > >> 2.25.1
>> > >>
>> >