Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
Heinrich,
On Tue, Apr 14, 2020 at 09:41:51AM +0200, Heinrich Schuchardt wrote:
> On 2020-04-14 08:12, AKASHI Takahiro wrote:
> > On Tue, Apr 14, 2020 at 05:53:43AM +, Heinrich Schuchardt wrote:
> >> Am April 14, 2020 5:20:38 AM UTC schrieb AKASHI Takahiro
> >> :
> >>> 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
> >>> ---
> >>> 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
> >>> #include
> >>>
> >>> +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 = >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.)
>
> You can stick to your original design.
Thanks
> >
> >> 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.
>
> Do you have a better suggestion?
The root cause would be that there is no notion of "boot device"
in U-Boot. So I would suggest
1) we should add Kconfig option to specify it
just as we do for U-Boot environment (variables), and then
check if the partition has a system partition bit at boot time.
2) you would follow the similar way as I do in capsule support.
or
3) you would first examine what EDK2 does in this respect.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> >
> > -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
>
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
On 2020-04-14 08:12, AKASHI Takahiro wrote:
> On Tue, Apr 14, 2020 at 05:53:43AM +, Heinrich Schuchardt wrote:
>> Am April 14, 2020 5:20:38 AM UTC schrieb AKASHI Takahiro
>> :
>>> 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
>>> ---
>>> 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
>>> #include
>>>
>>> +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 = >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.)
You can stick to your original design.
>
>> 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.
Do you have a better suggestion?
Best regards
Heinrich
>
> -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, );
>>> + 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;
>>> +
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
On Tue, Apr 14, 2020 at 05:53:43AM +, Heinrich Schuchardt wrote:
> Am April 14, 2020 5:20:38 AM UTC schrieb AKASHI Takahiro
> :
> >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
> >> > >> ---
> >> > >> 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
> >> > >> #include
> >> > >>
> >> > >> +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 = >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, );
> >> > >> + 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
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
Am April 14, 2020 5:20:38 AM UTC schrieb AKASHI Takahiro
:
>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
>> > >> ---
>> > >> 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
>> > >> #include
>> > >>
>> > >> +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 = >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, );
>> > >> + 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
>> > >>
>> >
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
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
> > >> ---
> > >> 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
> > >> #include
> > >>
> > >> +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 = >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.
-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, );
> > >> +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
> > >>
> >
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
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
> >> ---
> >> 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
> >> #include
> >>
> >> +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 = >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."
-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, );
> >> + 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
> >>
>
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
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
>> ---
>> 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
>> #include
>>
>> +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 = >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?
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, );
>> +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
>>
Re: [PATCH v2 2/2] efi_loader: identify EFI system partition
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
> ---
> 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
> #include
>
> +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 = >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
- 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.
-Takahiro Akashi
> + if (part && !efi_system_partition.if_type) {
> + int r;
> + disk_partition_t info;
> +
> + r = part_get_info(desc, part, );
> + 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
>
