Heinrich,
On Sun, May 17, 2020 at 10:02:08AM +0200, Heinrich Schuchardt wrote:
> On 4/27/20 11:48 AM, AKASHI Takahiro wrote:
> > In this commit, skeleton functions for capsule-related API's are
> > added under CONFIG_EFI_UPDATE_CAPSULE configuration.
> > Detailed implementation for a specific capsule type will be added
> > in the succeeding patches.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> > ---
> > include/efi_api.h | 12 +++
> > include/efi_loader.h | 15 ++++
> > lib/efi_loader/Kconfig | 11 +++
> > lib/efi_loader/Makefile | 1 +
> > lib/efi_loader/efi_capsule.c | 153 +++++++++++++++++++++++++++++++++++
> > lib/efi_loader/efi_runtime.c | 104 ++++++++++++++----------
> > lib/efi_loader/efi_setup.c | 31 +++++--
> > 7 files changed, 276 insertions(+), 51 deletions(-)
> > create mode 100644 lib/efi_loader/efi_capsule.c
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 77d6bf2660b9..6fa3f4a887d2 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -217,6 +217,10 @@ enum efi_reset_type {
> > #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE 0x00020000
> > #define CAPSULE_FLAGS_INITIATE_RESET 0x00040000
> >
> > +#define EFI_CAPSULE_REPORT_GUID \
> > + EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
> > + 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
> > +
> > struct efi_capsule_header {
> > efi_guid_t capsule_guid;
> > u32 header_size;
> > @@ -224,6 +228,14 @@ struct efi_capsule_header {
> > u32 capsule_image_size;
> > } __packed;
> >
> > +struct efi_capsule_result_variable_header {
> > + u32 variable_total_size;
> > + u32 reserved;
> > + efi_guid_t capsule_guid;
> > + struct efi_time capsule_processed;
> > + efi_status_t capsule_status;
> > +} __packed;
> > +
> > #define EFI_RT_SUPPORTED_GET_TIME 0x0001
> > #define EFI_RT_SUPPORTED_SET_TIME 0x0002
> > #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME 0x0004
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d4510462d616..19ffc027c171 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -196,6 +196,8 @@ extern const efi_guid_t efi_guid_cert_type_pkcs7;
> >
> > /* GUID of RNG protocol */
> > extern const efi_guid_t efi_guid_rng_protocol;
> > +/* GUID of capsule update result */
> > +extern const efi_guid_t efi_guid_capsule_report;
> >
> > extern unsigned int __efi_runtime_start, __efi_runtime_stop;
> > extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
> > @@ -778,6 +780,19 @@ bool efi_image_parse(void *efi, size_t len, struct
> > efi_image_regions **regp,
> > WIN_CERTIFICATE **auth, size_t *auth_len);
> > #endif /* CONFIG_EFI_SECURE_BOOT */
>
> This line does not exist in upstream. Instead we have:
>
> bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
> WIN_CERTIFICATE **auth, size_t *auth_len);
>
> #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
>
> Am I missing patches? Or do you have to rebase your series?
The lines above come from my secure boot patch, while my capsule
patch is based on pre-v2020.07-rc1 in which my secure boot patch
has not been merged when I posted capsule patch.
> >
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +/* Capsule update */
> > +efi_status_t EFIAPI efi_update_capsule(
> > + struct efi_capsule_header **capsule_header_array,
> > + efi_uintn_t capsule_count,
> > + u64 scatter_gather_list);
> > +efi_status_t EFIAPI efi_query_capsule_caps(
> > + efi_capsule_header **capsule_header_array,
> > + efi_uintn_t capsule_count,
> > + u64 *maximum_capsule_size,
> > + u32 *reset_type);
> > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > +
> > #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 7cc2d940f848..e2b08251f26a 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -45,6 +45,17 @@ config EFI_SET_TIME
> > Provide the SetTime() runtime service at boottime. This service
> > can be used by an EFI application to adjust the real time clock.
> >
> > +config EFI_HAVE_CAPSULE_SUPPORT
> > + bool
> > +
> > +config EFI_RUNTIME_UPDATE_CAPSULE
> > + bool "UpdateCapsule() runtime service"
> > + default n
> > + select EFI_HAVE_CAPSULE_SUPPORT
> > + help
> > + Select this option if you want to use UpdateCapsule and
> > + QueryCapsuleCapabilities API's.
> > +
> > config EFI_DEVICE_PATH_TO_TEXT
> > bool "Device path to text protocol"
> > default y
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index eff3c25ec301..2ee1e683d9c5 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -22,6 +22,7 @@ endif
> > obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> > obj-y += efi_bootmgr.o
> > obj-y += efi_boottime.o
> > +obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> > obj-y += efi_console.o
> > obj-y += efi_device_path.o
> > obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > new file mode 100644
> > index 000000000000..fb104bb92a6c
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * EFI Capsule
> > + *
> > + * Copyright (c) 2018 Linaro Limited
> > + * Author: AKASHI Takahiro
> > + */
> > +
> > +#include <common.h>
> > +#include <efi_loader.h>
> > +#include <fs.h>
> > +#include <malloc.h>
> > +#include <sort.h>
> > +
> > +const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
> > +
> > +static __maybe_unused 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;
> > +}
> > +
>
> Missing function description
Okay, as I said somewhere else, I've forgot to add some function
descriptions in v1.
> > +static __maybe_unused
> > +void set_capsule_result(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 = variable_name16;
> > + 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)
> > + printf("EFI: creating %s failed\n", variable_name);
> > +}
> > +
> > +/*
> > + * Launch a capsule
>
> Please, stick to the style described in
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation
>
> > + */
> > +/**
> > + * efi_update_capsule() - process information from operating system
> > + *
> > + * This function implements the UpdateCapsule() runtime service.
> > + *
> > + * See the Unified Extensible Firmware Interface (UEFI) specification for
> > + * details.
> > + *
> > + * @capsule_header_array: pointer to array of virtual pointers
> > + * @capsule_count: number of pointers in capsule_header_array
> > + * @scatter_gather_list: pointer to arry of physical pointers
> > + * Return: status code
> > + */
> > +efi_status_t EFIAPI efi_update_capsule(
> > + struct efi_capsule_header **capsule_header_array,
> > + efi_uintn_t capsule_count,
> > + u64 scatter_gather_list)
> > +{
> > + struct efi_capsule_header *capsule;
> > + unsigned int i;
> > + efi_status_t ret;
> > +
> > + EFI_ENTRY("%p, %lu, %llu\n", capsule_header_array, capsule_count,
> > + scatter_gather_list);
> > +
> > + if (!capsule_count) {
> > + ret = EFI_INVALID_PARAMETER;
> > + goto out;
> > + }
> > +
> > + ret = EFI_SUCCESS;
> > + for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> > + i++, capsule = *(++capsule_header_array)) {
> > + }
> > +out:
> > + return EFI_EXIT(ret);
> > +}
> > +
> > +/**
> > + * efi_query_capsule_caps() - check if capsule is supported
> > + *
> > + * This function implements the QueryCapsuleCapabilities() runtime service.
> > + *
> > + * See the Unified Extensible Firmware Interface (UEFI) specification for
> > + * details.
> > + *
> > + * @capsule_header_array: pointer to array of virtual pointers
> > + * @capsule_count: number of pointers in capsule_header_array
> > + * @maximum_capsule_size: maximum capsule size
> > + * @reset_type: type of reset needed for capsule update
> > + * Return: status code
> > + */
> > +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)
> > +{
> > + struct efi_capsule_header *capsule __attribute__((unused));
> > + unsigned int i;
> > + efi_status_t ret;
> > +
> > + EFI_ENTRY("%p, %lu, %p, %p\n", capsule_header_array, capsule_count,
> > + maximum_capsule_size, reset_type);
> > +
> > + if (!maximum_capsule_size) {
> > + ret = EFI_INVALID_PARAMETER;
> > + goto out;
> > + }
> > +
> > + *maximum_capsule_size = U64_MAX;
> > + *reset_type = EFI_RESET_COLD;
> > +
> > + ret = EFI_SUCCESS;
> > + for (i = 0, capsule = *capsule_header_array; i < capsule_count;
> > + i++, capsule = *(++capsule_header_array)) {
> > + /* TODO */
> > + }
> > +out:
> > + return EFI_EXIT(ret);
> > +}
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index 6a25acbbcdf5..669e25a9589a 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -130,6 +130,11 @@ efi_status_t efi_init_runtime_supported(void)
> > #ifdef CONFIG_EFI_HAVE_RUNTIME_RESET
> > rt_table->runtime_services_supported |= EFI_RT_SUPPORTED_RESET_SYSTEM;
> > #endif
> > +#ifdef CONFIG_EFI_RUNTIME_UPDATE_CAPSULE
> > + rt_table->runtime_services_supported |=
> > + (EFI_RT_SUPPORTED_UPDATE_CAPSULE |
> > + EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES);
> > +#endif
> >
> > ret = efi_install_configuration_table(&efi_rt_properties_table_guid,
> > rt_table);
> > @@ -410,6 +415,50 @@ efi_status_t __weak __efi_runtime EFIAPI
> > efi_set_time(struct efi_time *time)
> > return EFI_UNSUPPORTED;
> > }
> >
> > +/**
> > + * efi_update_capsule_unsupported() - process information from operating
> > system
> > + *
> > + * This function implements the UpdateCapsule() runtime service.
> > + *
> > + * See the Unified Extensible Firmware Interface (UEFI) specification for
> > + * details.
>
> The description should differ from the description of efi_update_capsule().
I don't get your point.
This kind of description, like "See the UEFI ...," are often seen
in efi_boottime.c and others.
> > + *
> > + * @capsule_header_array: pointer to array of virtual pointers
> > + * @capsule_count: number of pointers in capsule_header_array
> > + * @scatter_gather_list: pointer to arry of physical pointers
> > + * Returns: status code
> > + */
> > +efi_status_t __efi_runtime EFIAPI efi_update_capsule_unsupported(
> > + struct efi_capsule_header **capsule_header_array,
> > + efi_uintn_t capsule_count,
> > + u64 scatter_gather_list)
> > +{
> > + return EFI_UNSUPPORTED;
> > +}
> > +
> > +/**
> > + * efi_query_capsule_caps_unsupported() - check if capsule is supported
> > + *
> > + * This function implements the QueryCapsuleCapabilities() runtime service.
> > + *
> > + * See the Unified Extensible Firmware Interface (UEFI) specification for
> > + * details.
>
> The description should differ from the description of
> efi_query_capsule_caps().
>
> > + *
> > + * @capsule_header_array: pointer to array of virtual pointers
> > + * @capsule_count: number of pointers in capsule_header_array
> > + * @maximum_capsule_size: maximum capsule size
> > + * @reset_type: type of reset needed for capsule update
> > + * Returns: status code
> > + */
> > +efi_status_t __efi_runtime EFIAPI efi_query_capsule_caps_unsupported(
> > + struct efi_capsule_header **capsule_header_array,
> > + efi_uintn_t capsule_count,
> > + u64 *maximum_capsule_size,
> > + u32 *reset_type)
> > +{
> > + return EFI_UNSUPPORTED;
> > +}
> > +
> > /**
> > * efi_is_runtime_service_pointer() - check if pointer points to runtime
> > table
> > *
> > @@ -433,6 +482,12 @@ void efi_runtime_detach(void)
> > efi_runtime_services.reset_system = efi_reset_system;
> > efi_runtime_services.get_time = efi_get_time;
> > efi_runtime_services.set_time = efi_set_time;
> > +#ifdef CONFIG_EFI_RUNTIME_UPDATE_CAPSULE
> > + /* won't support at runtime */
> > + efi_runtime_services.update_capsule = efi_update_capsule_unsupported;
> > + efi_runtime_services.query_capsule_caps =
> > + efi_query_capsule_caps_unsupported;
> > +#endif
> >
> > /* Update CRC32 */
> > efi_update_table_header_crc32(&efi_runtime_services.hdr);
> > @@ -837,50 +892,6 @@ static efi_status_t __efi_runtime EFIAPI
> > efi_unimplemented(void)
> > return EFI_UNSUPPORTED;
> > }
> >
> > -/**
> > - * efi_update_capsule() - process information from operating system
> > - *
> > - * This function implements the UpdateCapsule() runtime service.
> > - *
> > - * See the Unified Extensible Firmware Interface (UEFI) specification for
> > - * details.
> > - *
> > - * @capsule_header_array: pointer to array of virtual pointers
> > - * @capsule_count: number of pointers in capsule_header_array
> > - * @scatter_gather_list: pointer to arry of physical pointers
> > - * Returns: status code
> > - */
> > -efi_status_t __efi_runtime EFIAPI efi_update_capsule(
> > - struct efi_capsule_header **capsule_header_array,
> > - efi_uintn_t capsule_count,
> > - u64 scatter_gather_list)
> > -{
> > - return EFI_UNSUPPORTED;
> > -}
> > -
> > -/**
> > - * efi_query_capsule_caps() - check if capsule is supported
> > - *
> > - * This function implements the QueryCapsuleCapabilities() runtime service.
> > - *
> > - * See the Unified Extensible Firmware Interface (UEFI) specification for
> > - * details.
> > - *
> > - * @capsule_header_array: pointer to array of virtual pointers
> > - * @capsule_count: number of pointers in capsule_header_array
> > - * @maximum_capsule_size: maximum capsule size
> > - * @reset_type: type of reset needed for capsule update
> > - * Returns: status code
> > - */
> > -efi_status_t __efi_runtime 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;
> > -}
> > -
> > struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
> > .hdr = {
> > .signature = EFI_RUNTIME_SERVICES_SIGNATURE,
> > @@ -898,7 +909,12 @@ struct efi_runtime_services __efi_runtime_data
> > efi_runtime_services = {
> > .set_variable = efi_set_variable,
> > .get_next_high_mono_count = (void *)&efi_unimplemented,
> > .reset_system = &efi_reset_system_boottime,
> > +#ifdef CONFIG_EFI_RUNTIME_UPDATE_CAPSULE
> > .update_capsule = efi_update_capsule,
> > .query_capsule_caps = efi_query_capsule_caps,
> > +#else
> > + .update_capsule = efi_update_capsule_unsupported,
> > + .query_capsule_caps = efi_query_capsule_caps_unsupported,
> > +#endif
> > .query_variable_info = efi_query_variable_info,
> > };
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 1b648c84673a..8fe378bbfdfc 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -115,6 +115,29 @@ static efi_status_t efi_init_secure_boot(void)
> > }
> > #endif /* CONFIG_EFI_SECURE_BOOT */
> >
> > +/**
> > + * efi_init_os_indications() - indicate supported features for OS requests
> > + *
> > + * Set the OsIndicationsSupported variable.
> > + *
> > + * Return: status code
> > + */
> > +static efi_status_t efi_init_os_indications(void)
> > +{
> > + u64 os_indications_supported = 0;
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > + os_indications_supported |=
> > + EFI_OS_INDICATIONS_CAPSULE_RESULT_VAR_SUPPORTED;
> > +#endif
>
> Though the UEFI spec does not mention it I guess we will have to treat
> OsIndicationsSupported as read-only once we get that properly supported.
The only thing that we should do now is to export "efi_set_variable_common."
(I don't know if Ilias' StandaloneMM will accept READ_ONLY attribute.)
Please note that there is also an instance that should be read-only
in efi_init_secure_boot().
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > + return EFI_CALL(efi_set_variable(L"OsIndicationsSupported",
> > + &efi_global_variable_guid,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS,
> > + sizeof(os_indications_supported),
> > + &os_indications_supported));
> > +}
> > +
> > /**
> > * efi_init_obj_list() - Initialize and populate EFI object list
> > *
> > @@ -122,7 +145,6 @@ static efi_status_t efi_init_secure_boot(void)
> > */
> > efi_status_t efi_init_obj_list(void)
> > {
> > - u64 os_indications_supported = 0; /* None */
> > efi_status_t ret = EFI_SUCCESS;
> >
> > /* Initialize once only */
> > @@ -146,12 +168,7 @@ efi_status_t efi_init_obj_list(void)
> > goto out;
> >
> > /* Indicate supported features */
> > - ret = EFI_CALL(efi_set_variable(L"OsIndicationsSupported",
> > - &efi_global_variable_guid,
> > - EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > - EFI_VARIABLE_RUNTIME_ACCESS,
> > - sizeof(os_indications_supported),
> > - &os_indications_supported));
> > + ret = efi_init_os_indications();
> > if (ret != EFI_SUCCESS)
> > goto out;
> >
> >
>
