[PATCH v2 1/4] boot: correct the default sequence of boot methods

[Heinrich Schuchardt]

On 04.04.24 17:18, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> Date: Thu,  4 Apr 2024 13:47:39 +0200
>>
>> The default sequence of boot methods is determined by alphabetical sorting
>> during linkage.
>>
>> * efi_mgr must run before efi to be UEFI compliant
>> * pxe should run as last resort
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> 
> This sound very much like you're working around a standard boot design
> flaw.
> 
> Who does the alphabetical sorting here?  Is that just a result of how
> the toolchain behaves?  If so, that sounds fragile to me and I think
> it would be better to make the order explicit in the code.

Thanks for reviewing.

Please, observe the SORT command in our linker script lines

     KEEP(*(SORT(__u_boot_list*)));

This ensures that linker generated lists are always alphabetically 
sorted. We rely on this in other places too. See 
part_driver_lookup_type() where GPT must be tested before FAT (commit 
96e5b03c8ab7 ("dm: part: Convert partition API use to linker lists").

Best regards

Heinrich

> 
> Cheers,
> 
> Mark
> 
>> ---
>> v2:
>> 	no change
>> ---
>>   boot/bootmeth_efi.c     | 2 +-
>>   boot/bootmeth_efi_mgr.c | 2 +-
>>   boot/bootmeth_pxe.c     | 2 +-
>>   test/boot/bootflow.c    | 2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
>> index c4eb331d69e..a46b6c9c805 100644
>> --- a/boot/bootmeth_efi.c
>> +++ b/boot/bootmeth_efi.c
>> @@ -489,7 +489,7 @@ static const struct udevice_id distro_efi_bootmeth_ids[] = {
>>   	{ }
>>   };
>>   
>> -U_BOOT_DRIVER(bootmeth_efi) = {
>> +U_BOOT_DRIVER(bootmeth_4efi) = {
>>   	.name		= "bootmeth_efi",
>>   	.id		= UCLASS_BOOTMETH,
>>   	.of_match	= distro_efi_bootmeth_ids,
>> diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
>> index ed29d7ef021..b7d429f2c3d 100644
>> --- a/boot/bootmeth_efi_mgr.c
>> +++ b/boot/bootmeth_efi_mgr.c
>> @@ -114,7 +114,7 @@ static const struct udevice_id efi_mgr_bootmeth_ids[] = {
>>   	{ }
>>   };
>>   
>> -U_BOOT_DRIVER(bootmeth_efi_mgr) = {
>> +U_BOOT_DRIVER(bootmeth_3efi_mgr) = {
>>   	.name		= "bootmeth_efi_mgr",
>>   	.id		= UCLASS_BOOTMETH,
>>   	.of_match	= efi_mgr_bootmeth_ids,
>> diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c
>> index 8d489a11aa4..70f693aa239 100644
>> --- a/boot/bootmeth_pxe.c
>> +++ b/boot/bootmeth_pxe.c
>> @@ -184,7 +184,7 @@ static const struct udevice_id extlinux_bootmeth_pxe_ids[] = {
>>   	{ }
>>   };
>>   
>> -U_BOOT_DRIVER(bootmeth_pxe) = {
>> +U_BOOT_DRIVER(bootmeth_zpxe) = {
>>   	.name		= "bootmeth_pxe",
>>   	.id		= UCLASS_BOOTMETH,
>>   	.of_match	= extlinux_bootmeth_pxe_ids,
>> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
>> index 4845b7121c8..e60e9309fa9 100644
>> --- a/test/boot/bootflow.c
>> +++ b/test/boot/bootflow.c
>> @@ -377,7 +377,7 @@ static int bootflow_system(struct unit_test_state *uts)
>>   	if (!IS_ENABLED(CONFIG_EFI_BOOTMGR))
>>   		return -EAGAIN;
>>   	ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
>> -	ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_efi_mgr),
>> +	ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_3efi_mgr),
>>   				"efi_mgr", 0, ofnode_null(), &dev));
>>   	ut_assertok(device_probe(dev));
>>   	sandbox_set_fake_efi_mgr_dev(dev, true);
>> -- 
>> 2.43.0
>>
>>