Re: [PATCH v3] efi_loader: Trigger capsule updates with automatically generated boot options

[Heinrich Schuchardt]

Am 16. Dezember 2025 17:48:42 MEZ schrieb Ilias Apalodimas <ilias.apalodimas at linaro.org>:
>Hi Heinrich,
>
>[...]
>
>
>
>> >
>> >   /**
>> > - * device_is_present_and_system_part - check if a device exists
>> > + * get_esp_handle - check if a device exists and contains an ESP
>>
>> The logic looks ok. Yet, the descriptions need rework.
>>
>> "check if a device exists and contains an ESP":
>> This is not what this function does. It checks if a device has an ESP as
>> immediate child.
>
>Not only. If the provided DP is a pointing to a file in the ESP in
>will exit before trying to probe any of the disk children.
>
>>
>> >    *
>> >    * Check if a device pointed to by the device path, @dp, exists and is
>> > - * located in UEFI system partition.
>> > + * either an ESP or a disk containing an ESP.
>>
>> Nowhere do you check if the device is a disk. The end node of @dp could
>> be anything.
>>
>> An NVMe drive may have many namespaces, a SCSI drive many LUNs.
>>
>> The partition is not the child of the drive but of the namespace or the LUN.
>>
>> An NVMe(NSID,EUI) node in a device-path is for a namespace not for a
>> drive. Similarly an Scsi(PUN,LUN) node is for the LUN and not for the
>> drive (PUN).
>>
>> Please, provide an accurate function description.
>
>I am not sure I understand what you are expecting here.
>We treat disks and partitions as a 'udevice'. What the function does
>is look at the provided DP and get the associated handle for the
>device to look at the device children.
>I haven't looked closely into SCSI or NVMe device paths. Do you have
>any suggestions?
>
>>
>> >    *
>> >    * @dp              device path
>> > - * Return:   true - yes, false - no
>> > + * Return:   ESP handle or NULL
>> >    */
>> > -static bool device_is_present_and_system_part(struct efi_device_path *dp)
>> > +efi_handle_t get_esp_handle(struct efi_device_path *dp)
>> >   {
>> > -     efi_handle_t handle;
>> > +     efi_handle_t handle, dev_handle;
>> > +     struct udevice *child_dev;
>> >       struct efi_device_path *rem;
>> > +     efi_status_t ret;
>> >
>> >       /* Check device exists */
>> > -     handle = efi_dp_find_obj(dp, NULL, NULL);
>> > -     if (!handle)
>> > -             return false;
>> > +     dev_handle = efi_dp_find_obj(dp, NULL, NULL);
>> > +     if (!dev_handle)
>> > +             return NULL;
>> >
>> >       /* Check device is on system partition */
>>
>> Should this be
>> "Check if the device path points to an EFI system partition"?
>
>This checks if the DP is on a partition with the ESP installed.
>'device' is the udevice u-boot defines.
>I don't mind changing it.

I just dislike the inaccurate langage.

A device like mmc 0:0 cannot be *on a system partition*. The ESP GUID can be installed on the same handle as the device path dp.

>
>>
>> >       handle = efi_dp_find_obj(dp, &efi_system_partition_guid, &rem);
>> > -     if (!handle)
>> > -             return false;
>> > +     if (handle)
>> > +             return handle;
>> > +
>> > +     /* If it's a disk check its children */
>>
>> The object that you check may be anything, not necessarily a disk.

An EFI application can create an ESP partition device with any parent, e.g. Mem().

The function does not care what is the last node of dp.

"Check if any direct child of the dp handle is an ESP" would match what the loop does.


>
>efi_disk_create_raw() and efi_disk_create_part() are used to create
>the EFI tags. What else can it be?
>Shall I just remove the comment?
>
>Thanks
>/Ilias
>>
>> Best regards
>>
>> Heinrich
>>