On Tue, Apr 14, 2020 at 05:53:43AM +0000, Heinrich Schuchardt wrote:
> Am April 14, 2020 5:20:38 AM UTC schrieb AKASHI Takahiro
> <takahiro.aka...@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.g...@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.
Your commit message says,
"For capsule updates we need to identify the EFI system partition."
and then I made some counter comment.
So now you agreed with my comment, don't you?
(I need to confirm this to work on capsule patch.)
> For the storage of variables I still need this patch. I will adjust the
> commit message.
Even in this case, I believe that the first device detected in your logic
is not always a "valid" device for your purpose.
-Takahiro Akashi
>
> 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
> >> > >>
> >> >
>
