[RFC 06/14] efi_loader: capsule: add capsule_on_disk support

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 3/18/20 9:55 AM, Heinrich Schuchardt wrote:
> On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
>> Capsule data can be loaded into the system either via UpdateCapsule
>> runtime service or files on a file system (of boot device).
>> The latter case is called "capsules on disk", and actual updates will
>> take place at the next boot time.
>>
>> In this commit, we will support capsule on disk mechanism.
>>
>> Please note that U-Boot itself has no notion of "boot device" and
>> all the capsule files to be executed will be identified only if they
>> are located in a specific directory on a device that is determined
>> by "BootXXXX" variables.
> 
> We have efi_set_bootdev() defining the boot device. So why do you refer
> to BootXXXX?
> 
> Please, add Sphinx style comments to the functions describing
> functionality and parameters.
> 
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> ---
>> ? include/efi_loader.h????????? |? 18 ++
>> ? lib/efi_loader/Kconfig??????? |?? 7 +
>> ? lib/efi_loader/efi_boottime.c |?? 3 +
>> ? lib/efi_loader/efi_capsule.c? | 548 ++++++++++++++++++++++++++++++++++
>> ? lib/efi_loader/efi_setup.c??? |?? 6 +
>> ? 5 files changed, 582 insertions(+)
>>
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index c3cb7735bf50..c701672e18db 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -178,6 +178,8 @@ extern const efi_guid_t
>> efi_guid_hii_config_routing_protocol;
>> ? extern const efi_guid_t efi_guid_hii_config_access_protocol;
>> ? extern const efi_guid_t efi_guid_hii_database_protocol;
>> ? extern const efi_guid_t efi_guid_hii_string_protocol;
>> +/* GUID of capsule update result */
>> +extern const efi_guid_t efi_guid_capsule_report;
>>
>> ? /* GUID of RNG protocol */
>> ? extern const efi_guid_t efi_guid_rng_protocol;
>> @@ -690,6 +692,22 @@ efi_status_t EFIAPI efi_query_capsule_caps(
>> ????????? u64 *maximum_capsule_size,
>> ????????? u32 *reset_type);
>>
>> +#ifdef CONFIG_EFI_CAPSULE_ON_DISK
>> +#define EFI_CAPSULE_DIR L"\\EFI\\UpdateCapsule\\"
>> +
>> +/* Hook at initialization */
>> +efi_status_t efi_launch_capsules(void);
>> +/* Notify ExitBootServices() is called */
>> +void efi_capsule_boot_exit_notify(void);
>> +#else
>> +static inline efi_status_t efi_launch_capsules(void)
>> +{
>> +??? return EFI_SUCCESS;
>> +}
>> +
>> +static inline efi_capsule_boot_exit_notify(void) {}
>> +#endif /* CONFIG_EFI_CAPSULE_ON_DISK */
>> +
>> ? #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
>>
>> ? /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub
>> it out */
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index 2ef6cb124f3a..95e10f7d981b 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -97,6 +97,13 @@ config EFI_CAPSULE_UPDATE
>> ??????? Select this option if you want to use capsule update feature,
>> ??????? including firmware updates and variable updates.
>>
>> +config EFI_CAPSULE_ON_DISK
>> +??? bool "Enable capsule-on-disk support"
>> +??? depends on EFI_CAPSULE_UPDATE
>> +??? default n
>> +??? help
>> +????? Select this option if you want to use capsule-on-disk feature.
>> +
>> ? config EFI_LOADER_BOUNCE_BUFFER
>> ????? bool "EFI Applications use bounce buffers for DMA operations"
>> ????? depends on ARM64
>> diff --git a/lib/efi_loader/efi_boottime.c
>> b/lib/efi_loader/efi_boottime.c
>> index 9860d5047502..c2a789b4f910 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -1981,6 +1981,9 @@ static efi_status_t EFIAPI
>> efi_exit_boot_services(efi_handle_t image_handle,
>> ????? /* Notify variable services */
>> ????? efi_variables_boot_exit_notify();
>>
>> +??? /* Notify capsule services */
>> +??? efi_capsule_boot_exit_notify();
>> +
>> ????? /* Remove all events except EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE */
>> ????? list_for_each_entry_safe(evt, next_event, &efi_events, link) {
>> ????????? if (evt->type != EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE)
>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
>> index d3f931910d10..f3e2a555a6b9 100644
>> --- a/lib/efi_loader/efi_capsule.c
>> +++ b/lib/efi_loader/efi_capsule.c
>> @@ -10,8 +10,14 @@
>> ? #include <efi_loader.h>
>> ? #include <fs.h>
>> ? #include <malloc.h>
>> +#include <mapmem.h>
>> ? #include <sort.h>
>>
>> +const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
>> +
>> +/* for file system access */
>> +static struct efi_file_handle *bootdev_root;
>> +
>> ? /*
>> ?? * Launch a capsule
>> ?? */
>> @@ -96,3 +102,545 @@ efi_status_t EFIAPI efi_query_capsule_caps(
>> ? out:
>> ????? return EFI_EXIT(ret);
>> ? }
>> +
>> +#ifdef CONFIG_EFI_CAPSULE_ON_DISK
>> +static void efi_capsule_result_variable(int num,
>> +??????????????????? struct efi_capsule_header *capsule,
>> +??????????????????? efi_status_t return_status)
>> +{
>> +??? char variable_name[12];
>> +??? u16 variable_name16[12], *p;
>> +??? struct efi_capsule_result_variable_header result;
>> +??? struct efi_time time;
>> +??? efi_status_t ret;
>> +
>> +??? sprintf(variable_name, "Capsule%04X", num);
>> +??? p = variableame16;
>> +??? utf8_utf16_strncpy(&p, variable_name, 11);
>> +??? result.variable_total_size = sizeof(result);
>> +??? result.capsule_guid = capsule->capsule_guid;
>> +??? ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL));
>> +??? if (ret == EFI_SUCCESS)
>> +??????? memcpy(&result.capsule_processed, &time, sizeof(time));
>> +??? else
>> +??????? memset(&result.capsule_processed, 0, sizeof(time));
>> +??? result.capsule_status = return_status;
>> +??? ret = EFI_CALL(efi_set_variable(variable_name16,
>> +??????????????????? &efi_guid_capsule_report,
>> +??????????????????? EFI_VARIABLE_NON_VOLATILE |
>> +??????????????????? EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> +??????????????????? EFI_VARIABLE_RUNTIME_ACCESS,
>> +??????????????????? sizeof(result), &result));
>> +??? if (ret)
>> +??????? EFI_PRINT("EFI Capsule: creating %s failed\n", variable_name);
> 
> I think this is an error that should always be reported to the user.
> Please, use printf().
> 
> After https://patchwork.ozlabs.org/patch/1245322/ is merged we can move
> to log().
> 
>> +}
>> +
>> +static efi_status_t get_dp_device(u16 *boot_var,
>> +????????????????? struct efi_device_path **device_dp)
>> +{
>> +??? void *buf = NULL;
>> +??? efi_uintn_t size;
>> +??? struct efi_load_option lo;
>> +??? struct efi_device_path *file_dp;
>> +??? efi_status_t ret;
>> +
>> +??? size = 0;
>> +??? ret = EFI_CALL(efi_get_variable(boot_var, &efi_global_variable_guid,
>> +??????????????????? NULL, &size, NULL));
>> +??? if (ret == EFI_BUFFER_TOO_SMALL) {
>> +??????? buf = malloc(size);
>> +??????? if (!buf)
>> +??????????? return EFI_OUT_OF_RESOURCES;
>> +??????? ret = EFI_CALL(efi_get_variable(boot_var,
>> +??????????????????????? &efi_global_variable_guid,
>> +??????????????????????? NULL, &size, buf));
>> +??? }
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return ret;
>> +
>> +??? efi_deserialize_load_option(&lo, buf);
>> +
>> +??? if (lo.attributes & LOAD_OPTION_ACTIVE) {
>> +??????? efi_dp_split_file_path(lo.file_path, device_dp, &file_dp);
>> +??????? efi_free_pool(file_dp);
>> +
>> +??????? ret = EFI_SUCCESS;
>> +??? } else {
>> +??????? ret = EFI_NOT_FOUND;
>> +??? }
>> +
>> +??? free(buf);
>> +
>> +??? return ret;
>> +}
>> +
>> +static bool device_is_present(struct efi_device_path *dp)
>> +{
>> +??? efi_handle_t handle;
>> +??? struct efi_handler *handler;
>> +??? efi_status_t ret;
>> +
>> +??? handle = efi_dp_find_obj(dp, NULL);
>> +??? if (!handle)
>> +??????? return false;
>> +
>> +??? /* check if this is a block device */
>> +??? ret = efi_search_protocol(handle, &efi_block_io_guid, &handler);
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return false;
>> +
>> +??? return true;
>> +}
>> +
>> +static efi_status_t find_boot_device(void)

Please, use the EFI system partition, cf.

[PATCH 0/2] efi_loader: detect EFI system partition
https://lists.denx.de/pipermail/u-boot/2020-March/403560.htm
https://patchwork.ozlabs.org/project/uboot/list/?series=165417

Best regards

Heinrich

>> +{
>> +??? char boot_var[9];
>> +??? u16 boot_var16[9], *p, bootnext, *boot_order = NULL;
>> +??? efi_uintn_t size;
>> +??? int i, num;
>> +??? struct efi_simple_file_system_protocol *volume;
>> +??? struct efi_device_path *boot_dev = NULL;
>> +??? efi_status_t ret;
>> +
>> +??? /* find active boot device in BootNext */
>> +??? bootnext = 0;
>> +??? size = sizeof(bootnext);
>> +??? ret = EFI_CALL(efi_get_variable(L"BootNext",
>> +??????????????????? (efi_guid_t *)&efi_global_variable_guid,
>> +??????????????????? NULL, &size, &bootnext));
>> +??? if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
>> +??????? /* BootNext does exist here */
>> +??????? if (ret == EFI_BUFFER_TOO_SMALL || size != sizeof(u16)) {
>> +??????????? printf("BootNext must be 16-bit integer\n");
>> +??????????? goto skip;
>> +??????? }
>> +??????? sprintf((char *)boot_var, "Boot%04X", bootnext);
>> +??????? p = boot_var16;
>> +??????? utf8_utf16_strcpy(&p, boot_var);
>> +
>> +??????? ret = get_dp_device(boot_var16, &boot_dev);
>> +??????? if (ret == EFI_SUCCESS) {
>> +??????????? if (device_is_present(boot_dev)) {
>> +??????????????? goto out;
>> +??????????? } else {
>> +??????????????? efi_free_pool(boot_dev);
>> +??????????????? boot_dev = NULL;
>> +??????????? }
>> +??????? }
>> +??? }
>> +
>> +skip:
>> +??? /* find active boot device in BootOrder */
>> +??? size = 0;
>> +??? ret = EFI_CALL(efi_get_variable(L"BootOrder",
>> &efi_global_variable_guid,
>> +??????????????????? NULL, &size, NULL));
>> +??? if (ret == EFI_BUFFER_TOO_SMALL) {
>> +??????? boot_order = malloc(size);
>> +??????? if (!boot_order) {
>> +??????????? ret = EFI_OUT_OF_RESOURCES;
>> +??????????? goto out;
>> +??????? }
>> +
>> +??????? ret = EFI_CALL(efi_get_variable(
>> +??????????????????? L"BootOrder", &efi_global_variable_guid,
>> +??????????????????? NULL, &size, boot_order));
>> +??? }
>> +??? if (ret != EFI_SUCCESS)
>> +??????? goto out;
>> +
>> +??? /* check in higher order */
>> +??? num = size / sizeof(u16);
>> +??? for (i = 0; i < num; i++) {
>> +??????? sprintf((char *)boot_var, "Boot%04X", boot_order[i]);
>> +??????? p = boot_var16;
>> +??????? utf8_utf16_strcpy(&p, boot_var);
>> +??????? ret = get_dp_device(boot_var16, &boot_dev);
>> +??????? if (ret != EFI_SUCCESS)
>> +??????????? continue;
>> +
>> +??????? if (device_is_present(boot_dev))
>> +??????????? break;
>> +
>> +??????? efi_free_pool(boot_dev);
>> +??????? boot_dev = NULL;
>> +??? }
>> +out:
>> +??? if (boot_dev) {
>> +??????? u16 *path_str;
>> +
>> +??????? path_str = efi_dp_str(boot_dev);
>> +??????? EFI_PRINT("EFI Capsule: bootdev is %ls\n", path_str);
>> +??????? efi_free_pool(path_str);
>> +
>> +??????? volume = efi_fs_from_path(boot_dev);
>> +??????? if (!volume)
>> +??????????? ret = EFI_DEVICE_ERROR;
>> +??????? else
>> +??????????? ret = EFI_CALL(volume->open_volume(volume,
>> +?????????????????????????????? &bootdev_root));
>> +??????? efi_free_pool(boot_dev);
>> +??? } else {
>> +??????? ret = EFI_NOT_FOUND;
>> +??? }
>> +??? free(boot_order);
>> +
>> +??? return ret;
>> +}
>> +
>> +/*
>> + * Traverse a capsule directory in boot device
>> + * Called by initialization code, and returns an array of capsule file
>> + * names in @files
>> + */
>> +static efi_status_t efi_capsule_scan_dir(u16 ***files, int *num)
>> +{
>> +??? struct efi_file_handle *dirh;
>> +??? struct efi_file_info *dirent;
>> +??? efi_uintn_t dirent_size, tmp_size;
>> +??? int count;
>> +??? u16 **tmp_files;
>> +??? efi_status_t ret;
>> +
>> +??? ret = find_boot_device();
>> +??? if (ret == EFI_NOT_FOUND) {
>> +??????? EFI_PRINT("EFI Capsule: bootdev is not set\n");
>> +??????? *num = 0;
>> +??????? return EFI_SUCCESS;
>> +??? } else if (ret != EFI_SUCCESS) {
>> +??????? return EFI_DEVICE_ERROR;
>> +??? }
>> +
>> +??? /* count capsule files */
>> +??? ret = EFI_CALL((*bootdev_root->open)(bootdev_root, &dirh,
>> +???????????????????????? EFI_CAPSULE_DIR,
>> +???????????????????????? EFI_FILE_MODE_READ, 0));
>> +??? if (ret != EFI_SUCCESS) {
>> +??????? *num = 0;
>> +??????? return EFI_SUCCESS;
>> +??? }
>> +
>> +??? dirent_size = 256;
>> +??? dirent = malloc(dirent_size);
>> +??? if (!dirent)
>> +??????? return EFI_OUT_OF_RESOURCES;
>> +
>> +??? count = 0;
>> +??? while (1) {
>> +??????? tmp_size = dirent_size;
>> +??????? ret = EFI_CALL((*dirh->read)(dirh, &tmp_size, dirent));
>> +??????? if (ret == EFI_BUFFER_TOO_SMALL) {
>> +??????????? dirent = realloc(dirent, tmp_size);
>> +??????????? if (!dirent) {
>> +??????????????? ret = EFI_OUT_OF_RESOURCES;
>> +??????????????? goto err;
>> +??????????? }
>> +??????????? dirent_size = tmp_size;
>> +??????????? ret = EFI_CALL((*dirh->read)(dirh, &tmp_size, dirent));
>> +??????? }
>> +??????? if (ret != EFI_SUCCESS)
>> +??????????? goto err;
>> +??????? if (!tmp_size)
>> +??????????? break;
>> +
>> +??????? if (!(dirent->attribute & EFI_FILE_DIRECTORY) &&
>> +??????????? u16_strcmp(dirent->file_name, L".") &&
>> +??????????? u16_strcmp(dirent->file_name, L".."))
>> +??????????? count++;
>> +??? }
>> +
>> +??? ret = EFI_CALL((*dirh->setpos)(dirh, 0));
>> +??? if (ret != EFI_SUCCESS)
>> +??????? goto err;
>> +
>> +??? /* make a list */
>> +??? tmp_files = malloc(count * sizeof(*files));
>> +??? if (!tmp_files) {
>> +??????? ret = EFI_OUT_OF_RESOURCES;
>> +??????? goto err;
>> +??? }
>> +
>> +??? count = 0;
>> +??? while (1) {
>> +??????? tmp_size = dirent_size;
>> +??????? ret = EFI_CALL((*dirh->read)(dirh, &tmp_size, dirent));
>> +??????? if (ret != EFI_SUCCESS)
>> +??????????? goto err;
>> +??????? if (!tmp_size)
>> +??????????? break;
>> +
>> +??????? if (!(dirent->attribute & EFI_FILE_DIRECTORY) &&
>> +??????????? u16_strcmp(dirent->file_name, L".") &&
>> +??????????? u16_strcmp(dirent->file_name, L".."))
>> +??????????? tmp_files[count++] = u16_strdup(dirent->file_name);
>> +??? }
>> +??? /* ignore an error */
>> +??? EFI_CALL((*dirh->close)(dirh));
>> +
>> +??? /* in ascii order */
>> +??? /* FIXME: u16 version of strcasecmp */
>> +??? qsort(tmp_files, count, sizeof(*tmp_files),
>> +????????? (int (*)(const void *, const void *))strcasecmp);
>> +??? *files = tmp_files;
>> +??? *num = count;
>> +??? ret = EFI_SUCCESS;
>> +err:
>> +??? free(dirent);
>> +
>> +??? return ret;
>> +}
>> +
>> +/*
>> + * Read in a capsule file
>> + */
>> +static efi_status_t efi_capsule_read_file(u16 *filename,
>> +????????????????????? struct efi_capsule_header **capsule)
>> +{
>> +??? struct efi_file_handle *dirh, *fh;
>> +??? struct efi_file_info *file_info = NULL;
>> +??? struct efi_capsule_header *buf = NULL;
>> +??? efi_uintn_t size;
>> +??? efi_status_t ret;
>> +
>> +??? ret = EFI_CALL((*bootdev_root->open)(bootdev_root, &dirh,
>> +???????????????????????? EFI_CAPSULE_DIR,
>> +???????????????????????? EFI_FILE_MODE_READ, 0));
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return ret;
>> +??? ret = EFI_CALL((*dirh->open)(dirh, &fh, filename,
>> +???????????????????? EFI_FILE_MODE_READ, 0));
>> +??? /* ignore an error */
>> +??? EFI_CALL((*dirh->close)(dirh));
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return ret;
>> +
>> +??? /* file size */
>> +??? size = 0;
>> +??? ret = EFI_CALL((*fh->getinfo)(fh, &efi_file_info_guid,
>> +????????????????????? &size, file_info));
>> +??? if (ret == EFI_BUFFER_TOO_SMALL) {
>> +??????? file_info = malloc(size);
>> +??????? if (!file_info) {
>> +??????????? ret = EFI_OUT_OF_RESOURCES;
>> +??????????? goto err;
>> +??????? }
>> +??????? ret = EFI_CALL((*fh->getinfo)(fh, &efi_file_info_guid,
>> +????????????????????????? &size, file_info));
>> +??? }
>> +??? if (ret != EFI_SUCCESS)
>> +??????? goto err;
>> +??? size = file_info->file_size;
>> +??? free(file_info);
>> +??? buf = malloc(size);
>> +??? if (!buf) {
>> +??????? ret = EFI_OUT_OF_RESOURCES;
>> +??????? goto err;
>> +??? }
>> +
>> +??? /* fetch data */
>> +??? ret = EFI_CALL((*fh->read)(fh, &size, buf));
>> +??? if (ret == EFI_SUCCESS) {
>> +??????? if (size >= buf->capsule_image_size) {
>> +??????????? *capsule = buf;
>> +??????? } else {
>> +??????????? free(buf);
>> +??????????? ret = EFI_INVALID_PARAMETER;
>> +??????? }
>> +??? } else {
>> +??????? free(buf);
>> +??? }
>> +err:
>> +??? EFI_CALL((*fh->close)(fh));
>> +
>> +??? return ret;
>> +}
>> +
>> +static efi_status_t efi_capsule_delete_file(u16 *filename)
>> +{
>> +??? struct efi_file_handle *dirh, *fh;
>> +??? efi_status_t ret;
>> +
>> +??? ret = EFI_CALL((*bootdev_root->open)(bootdev_root, &dirh,
>> +???????????????????????? EFI_CAPSULE_DIR,
>> +???????????????????????? EFI_FILE_MODE_READ, 0));
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return ret;
>> +??? ret = EFI_CALL((*dirh->open)(dirh, &fh, filename,
>> +???????????????????? EFI_FILE_MODE_READ, 0));
>> +??? /* ignore an error */
>> +??? EFI_CALL((*dirh->close)(dirh));
>> +
>> +??? ret = EFI_CALL((*fh->delete)(fh));
>> +
>> +??? return ret;
>> +}
>> +
>> +static void efi_capsule_scan_done(void)
>> +{
>> +??? EFI_CALL((*bootdev_root->close)(bootdev_root));
>> +??? bootdev_root = NULL;
>> +}
>> +
>> +efi_status_t __weak arch_efi_load_capsule_drivers(void)
>> +{
>> +??? return EFI_SUCCESS;
>> +}
>> +
>> +static int get_last_capsule(void)
>> +{
>> +??? u16 value16[11]; /* "CapsuleXXXX": non-null-terminated */
>> +??? char value[11], *p;
>> +??? efi_uintn_t size;
>> +??? unsigned long num = 0xffff;
>> +??? efi_status_t ret;
>> +
>> +??? size = sizeof(value16);
>> +??? ret = EFI_CALL(efi_get_variable(L"CapsuleLast",
>> +??????????????????? &efi_guid_capsule_report,
>> +??????????????????? NULL, &size, value16));
>> +??? if (ret != EFI_SUCCESS || u16_strncmp(value16, L"Capsule", 7))
>> +??????? goto err;
>> +
>> +??? p = value;
>> +??? utf16_utf8_strcpy(&p, value16);
>> +??? strict_strtoul(&value[7], 16, &num);
>> +err:
>> +??? return (int)num;
>> +}
>> +
>> +/*
>> + * Launch all the capsules in system at boot time
>> + *
>> + * Called by efi init code
>> + */
>> +efi_status_t efi_launch_capsules(void)
>> +{
>> +??? struct efi_capsule_header *capsule = NULL;
>> +??? u16 **files;
>> +??? int nfiles, num, i;
>> +??? char variable_name[12];
>> +??? u16 variable_name16[12], *p;
>> +??? efi_status_t ret;
>> +
>> +??? num = get_last_capsule();
>> +
>> +??? /* Load capsule drivers */
>> +??? ret = arch_efi_load_capsule_drivers();
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return ret;
>> +
>> +??? /*
>> +???? * Find capsules on disk.
>> +???? * All the capsules are collected at the beginning because
>> +???? * capsule files will be removed instantly.
>> +???? */
>> +??? nfiles = 0;
>> +??? files = NULL;
>> +??? ret = efi_capsule_scan_dir(&files, &nfiles);
>> +??? if (ret != EFI_SUCCESS)
>> +??????? return ret;
>> +??? if (!nfiles)
>> +??????? return EFI_SUCCESS;
>> +
>> +??? /* Launch capsules */
>> +??? for (i = 0, ++num; i < nfiles; i++, num++) {
>> +??????? EFI_PRINT("EFI Capsule from %ls ...\n", files[i]);
>> +??????? if (num > 0xffff)
>> +??????????? num = 0;
>> +??????? ret = efi_capsule_read_file(files[i], &capsule);
>> +??????? if (ret == EFI_SUCCESS) {
>> +??????????? ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0));
>> +??????????? if (ret != EFI_SUCCESS)
>> +??????????????? EFI_PRINT("EFI Capsule update failed at %ls\n",
>> +????????????????????? files[i]);
> 
> Isn't this an error that should always be presented to the user?
> 
>> +
>> +??????????? free(capsule);
>> +??????? } else {
>> +??????????? EFI_PRINT("EFI Capsule read failed\n");
> 
> Same here.
> 
>> +??????? }
>> +??????? /* create CapsuleXXXX */
>> +??????? efi_capsule_result_variable(num, capsule, ret);
>> +
>> +??????? /* delete a capsule either in case of success or failure */
>> +??????? ret = efi_capsule_delete_file(files[i]);
>> +??????? if (ret != EFI_SUCCESS)
>> +??????????? EFI_PRINT("EFI Capsule deletion of capsule failed at %ls\n",
>> +????????????????? files[i]);
> 
> Same here.
> 
>> +??? }
>> +??? efi_capsule_scan_done();
>> +
>> +??? for (i = 0; i < nfiles; i++)
>> +??????? free(files[i]);
>> +??? free(files);
>> +
>> +??? /* CapsuleMax */
>> +??? p = variable_name16;
>> +??? utf8_utf16_strncpy(&p, "CapsuleFFFF", 11);
>> +??? EFI_CALL(efi_set_variable(L"CapsuleMax", &efi_guid_capsule_report,
>> +????????????????? EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> +????????????????? EFI_VARIABLE_RUNTIME_ACCESS,
>> +????????????????? 22, variable_name16));
>> +
>> +??? /* CapsuleLast */
>> +??? sprintf(variable_name, "Capsule%04X", num - 1);
>> +??? p = variable_name16;
>> +??? utf8_utf16_strncpy(&p, variable_name, 11);
>> +??? EFI_CALL(efi_set_variable(L"CapsuleLast", &efi_guid_capsule_report,
>> +????????????????? EFI_VARIABLE_NON_VOLATILE |
>> +????????????????? EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> +????????????????? EFI_VARIABLE_RUNTIME_ACCESS,
>> +????????????????? 22, variable_name16));
>> +
>> +??? return ret;
>> +}
>> +
>> +/*
>> + * Dummy functions after ExitBootServices()
>> + */
>> +efi_status_t EFIAPI efi_update_capsule_runtime(
>> +??????? struct efi_capsule_header **capsule_header_array,
>> +??????? efi_uintn_t capsule_count,
>> +??????? u64 scatter_gather_list)
>> +{
>> +??? return EFI_UNSUPPORTED;
>> +}
>> +
>> +efi_status_t EFIAPI efi_query_capsule_caps_runtime(
>> +??????? struct efi_capsule_header **capsule_header_array,
>> +??????? efi_uintn_t capsule_count,
>> +??????? u64 *maximum_capsule_size,
>> +??????? u32 *reset_type)
>> +{
>> +??? return EFI_UNSUPPORTED;
>> +}
>> +
>> +/**
>> + * efi_capsule_boot_exit_notify() - notify ExitBootServices() is called
>> + */
>> +void efi_capsule_boot_exit_notify(void)
> 
> Shouldn't we put this into efi_runtime_detach() to reduce code size?
> 
> The rest looks good at first sight.
> 
> Best regards
> 
> Heinrich
> 
>> +{
>> +??? efi_runtime_services.update_capsule = efi_update_capsule_runtime;
>> +??? efi_runtime_services.query_capsule_caps =
>> +??????????????? efi_query_capsule_caps_runtime;
>> +??? efi_update_table_header_crc32(&efi_runtime_services.hdr);
>> +}
>> +#else
>> +/*
>> + * Dummy functions for runtime services
>> + */
>> +efi_status_t EFIAPI efi_update_capsule(
>> +??????? struct efi_capsule_header **capsule_header_array,
>> +??????? efi_uintn_t capsule_count,
>> +??????? u64 scatter_gather_list)
>> +{
>> +??? return EFI_UNSUPPORTED;
>> +}
>> +
>> +efi_status_t EFIAPI efi_query_capsule_caps(
>> +??????? struct efi_capsule_header **capsule_header_array,
>> +??????? efi_uintn_t capsule_count,
>> +??????? u64 *maximum_capsule_size,
>> +??????? u32 *reset_type)
>> +{
>> +??? return EFI_UNSUPPORTED;
>> +}
>> +#endif /* CONFIG_EFI_CAPSULE_ON_DISK */
>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>> index c485cad34022..309defc5e40d 100644
>> --- a/lib/efi_loader/efi_setup.c
>> +++ b/lib/efi_loader/efi_setup.c
>> @@ -96,6 +96,10 @@ static efi_status_t efi_init_os_indications(void)
>> ? #ifdef CONFIG_EFI_CAPSULE_UPDATE
>> ????? os_indications_supported |=
>> ????????????? EFI_OS_INDICATIONS_CAPSULE_RESULT_VAR_SUPPORTED;
>> +#endif
>> +#ifdef CONFIG_EFI_CAPSULE_ON_DISK
>> +??? os_indications_supported |=
>> +??????????? EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;
>> ? #endif
>> ????? return EFI_CALL(efi_set_variable(L"OsIndicationsSupported",
>> ?????????????????????? &efi_global_variable_guid,
>> @@ -196,6 +200,8 @@ efi_status_t efi_init_obj_list(void)
>> ????? if (ret != EFI_SUCCESS)
>> ????????? goto out;
>>
>> +??? /* Execute capsules after reboot */
>> +??? ret = efi_launch_capsules();
>> ? out:
>> ????? efi_obj_list_initialized = ret;
>> ????? return ret;
>>
>