[PATCH v2 1/1] efi_loader: prepare for read only OP-TEE variables

We currently have two implementations of UEFI variables:
* variables provided via an OP-TEE module * variables stored in the U-Boot environment
Read only variables are up to now only implemented in the U-Boot environment implementation.
Provide a common interface for both implementations that allows handling read-only variables.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: add missing efi_variable.h consider attributes==NULL in efi_variable_get() --- include/efi_variable.h | 40 +++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_variable.c | 171 ++++++++------------------- lib/efi_loader/efi_variable_common.c | 79 +++++++++++++ lib/efi_loader/efi_variable_tee.c | 75 ++++-------- 5 files changed, 188 insertions(+), 178 deletions(-) create mode 100644 include/efi_variable.h create mode 100644 lib/efi_loader/efi_variable_common.c
diff --git a/include/efi_variable.h b/include/efi_variable.h new file mode 100644 index 0000000000..784dbd9cf4 --- /dev/null +++ b/include/efi_variable.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020, Heinrich Schuchardt xypron.glpk@gmx.de + */ + +#ifndef _EFI_VARIABLE_H +#define _EFI_VARIABLE_H + +#define EFI_VARIABLE_READ_ONLY BIT(31) + +/** + * efi_get_variable() - retrieve value of a UEFI variable + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer to which the variable value is copied + * @data: buffer to which the variable value is copied + * Return: status code + */ +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, + void *data); + +/** + * efi_set_variable() - set value of a UEFI variable + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer with the variable value + * @data: buffer with the variable value + * @ro_check: check the read only read only bit in attributes + * Return: status code + */ +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check); + +#endif diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 57c7e66ea0..16b93ef7f0 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -35,6 +35,7 @@ obj-y += efi_root_node.o obj-y += efi_runtime.o obj-y += efi_setup.o obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o +obj-y += efi_variable_common.o ifeq ($(CONFIG_EFI_MM_COMM_TEE),y) obj-y += efi_variable_tee.o else diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index e097670e28..85df1427bc 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -7,6 +7,7 @@
#include <common.h> #include <efi_loader.h> +#include <efi_variable.h> #include <env.h> #include <env_internal.h> #include <hexdump.h> @@ -15,7 +16,6 @@ #include <search.h> #include <uuid.h> #include <crypto/pkcs7_parser.h> -#include <linux/bitops.h> #include <linux/compat.h> #include <u-boot/crc.h>
@@ -30,20 +30,6 @@ static bool efi_secure_boot; static int efi_secure_mode; static u8 efi_vendor_keys;
-#define READ_ONLY BIT(31) - -static efi_status_t efi_get_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 *attributes, - efi_uintn_t *data_size, void *data); - -static efi_status_t efi_set_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 attributes, - efi_uintn_t data_size, - const void *data, - bool ro_check); - /* * Mapping between EFI variables and u-boot variables: * @@ -154,7 +140,7 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep) str++;
if ((s = prefix(str, "ro"))) { - attr |= READ_ONLY; + attr |= EFI_VARIABLE_READ_ONLY; } else if ((s = prefix(str, "nv"))) { attr |= EFI_VARIABLE_NON_VOLATILE; } else if ((s = prefix(str, "boot"))) { @@ -202,29 +188,29 @@ static efi_status_t efi_set_secure_state(int sec_boot, int setup_mode,
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | - READ_ONLY; - ret = efi_set_variable_common(L"SecureBoot", &efi_global_variable_guid, - attributes, sizeof(sec_boot), &sec_boot, - false); + EFI_VARIABLE_READ_ONLY; + ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid, + attributes, sizeof(sec_boot), &sec_boot, + false); if (ret != EFI_SUCCESS) goto err;
- ret = efi_set_variable_common(L"SetupMode", &efi_global_variable_guid, - attributes, sizeof(setup_mode), - &setup_mode, false); + ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid, + attributes, sizeof(setup_mode), + &setup_mode, false); if (ret != EFI_SUCCESS) goto err;
- ret = efi_set_variable_common(L"AuditMode", &efi_global_variable_guid, - attributes, sizeof(audit_mode), - &audit_mode, false); + ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid, + attributes, sizeof(audit_mode), + &audit_mode, false); if (ret != EFI_SUCCESS) goto err;
- ret = efi_set_variable_common(L"DeployedMode", - &efi_global_variable_guid, attributes, - sizeof(deployed_mode), &deployed_mode, - false); + ret = efi_set_variable_int(L"DeployedMode", + &efi_global_variable_guid, attributes, + sizeof(deployed_mode), &deployed_mode, + false); err: return ret; } @@ -234,7 +220,7 @@ err: * @mode: new state * * Depending on @mode, secure boot related variables are updated. - * Those variables are *read-only* for users, efi_set_variable_common() + * Those variables are *read-only* for users, efi_set_variable_int() * is called here. * * Return: status code @@ -252,10 +238,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
efi_secure_boot = true; } else if (mode == EFI_MODE_AUDIT) { - ret = efi_set_variable_common(L"PK", &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS, - 0, NULL, false); + ret = efi_set_variable_int(L"PK", &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + 0, NULL, false); if (ret != EFI_SUCCESS) goto err;
@@ -307,8 +293,8 @@ static efi_status_t efi_init_secure_state(void) */
size = 0; - ret = efi_get_variable_common(L"PK", &efi_global_variable_guid, - NULL, &size, NULL); + ret = efi_get_variable_int(L"PK", &efi_global_variable_guid, + NULL, &size, NULL); if (ret == EFI_BUFFER_TOO_SMALL) { if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) mode = EFI_MODE_USER; @@ -325,13 +311,13 @@ static efi_status_t efi_init_secure_state(void)
ret = efi_transfer_secure_state(mode); if (ret == EFI_SUCCESS) - ret = efi_set_variable_common(L"VendorKeys", - &efi_global_variable_guid, - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS | - READ_ONLY, - sizeof(efi_vendor_keys), - &efi_vendor_keys, false); + ret = efi_set_variable_int(L"VendorKeys", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_READ_ONLY, + sizeof(efi_vendor_keys), + &efi_vendor_keys, false);
err: return ret; @@ -593,10 +579,9 @@ static efi_status_t efi_variable_authenticate(u16 *variable, } #endif /* CONFIG_EFI_SECURE_BOOT */
-static efi_status_t efi_get_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 *attributes, - efi_uintn_t *data_size, void *data) +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, + void *data) { char *native_name; efi_status_t ret; @@ -679,35 +664,6 @@ out: return ret; }
-/** - * efi_efi_get_variable() - retrieve value of a UEFI variable - * - * This function implements the GetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @variable_name: name of the variable - * @vendor: vendor GUID - * @attributes: attributes of the variable - * @data_size: size of the buffer to which the variable value is copied - * @data: buffer to which the variable value is copied - * Return: status code - */ -efi_status_t EFIAPI efi_get_variable(u16 *variable_name, - const efi_guid_t *vendor, u32 *attributes, - efi_uintn_t *data_size, void *data) -{ - efi_status_t ret; - - EFI_ENTRY(""%ls" %pUl %p %p %p", variable_name, vendor, attributes, - data_size, data); - - ret = efi_get_variable_common(variable_name, vendor, attributes, - data_size, data); - return EFI_EXIT(ret); -} - static char *efi_variables_list; static char *efi_cur_variable;
@@ -871,12 +827,9 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, return EFI_EXIT(ret); }
-static efi_status_t efi_set_variable_common(u16 *variable_name, - const efi_guid_t *vendor, - u32 attributes, - efi_uintn_t data_size, - const void *data, - bool ro_check) +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check) { char *native_name = NULL, *old_data = NULL, *val = NULL, *s; efi_uintn_t old_size; @@ -899,15 +852,15 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* check if a variable exists */ old_size = 0; attr = 0; - ret = efi_get_variable_common(variable_name, vendor, &attr, - &old_size, NULL); + ret = efi_get_variable_int(variable_name, vendor, &attr, + &old_size, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes);
/* check attributes */ if (old_size) { - if (ro_check && (attr & READ_ONLY)) { + if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) { ret = EFI_WRITE_PROTECTED; goto err; } @@ -915,8 +868,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* attributes won't be changed */ if (!delete && ((ro_check && attr != attributes) || - (!ro_check && ((attr & ~(u32)READ_ONLY) - != (attributes & ~(u32)READ_ONLY))))) { + (!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY) + != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { ret = EFI_INVALID_PARAMETER; goto err; } @@ -990,8 +943,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, ret = EFI_OUT_OF_RESOURCES; goto err; } - ret = efi_get_variable_common(variable_name, vendor, - &attr, &old_size, old_data); + ret = efi_get_variable_int(variable_name, vendor, + &attr, &old_size, old_data); if (ret != EFI_SUCCESS) goto err; } else { @@ -1011,7 +964,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* * store attributes */ - attributes &= (READ_ONLY | + attributes &= (EFI_VARIABLE_READ_ONLY | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | @@ -1020,7 +973,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, while (attributes) { attr = 1 << (ffs(attributes) - 1);
- if (attr == READ_ONLY) { + if (attr == EFI_VARIABLE_READ_ONLY) { s += sprintf(s, "ro"); } else if (attr == EFI_VARIABLE_NON_VOLATILE) { s += sprintf(s, "nv"); @@ -1074,12 +1027,12 @@ out: /* update VendorKeys */ if (vendor_keys_modified & efi_vendor_keys) { efi_vendor_keys = 0; - ret = efi_set_variable_common( + ret = efi_set_variable_int( L"VendorKeys", &efi_global_variable_guid, EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS - | READ_ONLY, + | EFI_VARIABLE_READ_ONLY, sizeof(efi_vendor_keys), &efi_vendor_keys, false); @@ -1096,36 +1049,6 @@ err: return ret; }
-/** - * efi_set_variable() - set value of a UEFI variable - * - * This function implements the SetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @variable_name: name of the variable - * @vendor: vendor GUID - * @attributes: attributes of the variable - * @data_size: size of the buffer with the variable value - * @data: buffer with the variable value - * Return: status code - */ -efi_status_t EFIAPI efi_set_variable(u16 *variable_name, - const efi_guid_t *vendor, u32 attributes, - efi_uintn_t data_size, const void *data) -{ - EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, - data_size, data); - - /* READ_ONLY bit is not part of API */ - attributes &= ~(u32)READ_ONLY; - - return EFI_EXIT(efi_set_variable_common(variable_name, vendor, - attributes, data_size, data, - true)); -} - /** * efi_query_variable_info() - get information about EFI variables * diff --git a/lib/efi_loader/efi_variable_common.c b/lib/efi_loader/efi_variable_common.c new file mode 100644 index 0000000000..e6a39fceac --- /dev/null +++ b/lib/efi_loader/efi_variable_common.c @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * UEFI runtime variable services + * + * Copyright (c) 2020, Heinrich Schuchardt xypron.glpk@gmx.de + */ + +#include <common.h> +#include <efi_loader.h> +#include <efi_variable.h> +#include <linux/bitops.h> + +/** + * efi_efi_get_variable() - retrieve value of a UEFI variable + * + * This function implements the GetVariable runtime service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer to which the variable value is copied + * @data: buffer to which the variable value is copied + * Return: status code + */ +efi_status_t EFIAPI efi_get_variable(u16 *variable_name, + const efi_guid_t *vendor, u32 *attributes, + efi_uintn_t *data_size, void *data) +{ + efi_status_t ret; + + EFI_ENTRY(""%ls" %pUl %p %p %p", variable_name, vendor, attributes, + data_size, data); + + ret = efi_get_variable_int(variable_name, vendor, attributes, + data_size, data); + + /* Remove EFI_VARIABLE_READ_ONLY flag */ + if (attributes) + *attributes &= EFI_VARIABLE_MASK; + + return EFI_EXIT(ret); +} + +/** + * efi_set_variable() - set value of a UEFI variable + * + * This function implements the SetVariable runtime service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * @variable_name: name of the variable + * @vendor: vendor GUID + * @attributes: attributes of the variable + * @data_size: size of the buffer with the variable value + * @data: buffer with the variable value + * Return: status code + */ +efi_status_t EFIAPI efi_set_variable(u16 *variable_name, + const efi_guid_t *vendor, u32 attributes, + efi_uintn_t data_size, const void *data) +{ + efi_status_t ret; + + EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes, + data_size, data); + + /* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */ + if (attributes & ~(u32)EFI_VARIABLE_MASK) + ret = EFI_INVALID_PARAMETER; + else + ret = efi_set_variable_int(variable_name, vendor, attributes, + data_size, data, true); + + return EFI_EXIT(ret); +} diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index cacc76e23d..2513878c82 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -10,6 +10,7 @@ #include <efi.h> #include <efi_api.h> #include <efi_loader.h> +#include <efi_variable.h> #include <tee.h> #include <malloc.h> #include <mm_communication.h> @@ -243,24 +244,9 @@ out: return ret; }
-/** - * efi_get_variable() - retrieve value of a UEFI variable - * - * This function implements the GetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @name: name of the variable - * @guid: vendor GUID - * @attr: attributes of the variable - * @data_size: size of the buffer to which the variable value is copied - * @data: buffer to which the variable value is copied - * Return: status code - */ -efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, - u32 *attr, efi_uintn_t *data_size, - void *data) +efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 *attributes, efi_uintn_t *data_size, + void *data) { struct smm_variable_access *var_acc; efi_uintn_t payload_size; @@ -269,15 +255,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, u8 *comm_buf = NULL; efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %p %p %p", name, guid, attr, data_size, data); - - if (!name || !guid || !data_size) { + if (!variable_name || !vendor || !data_size) { ret = EFI_INVALID_PARAMETER; goto out; }
/* Check payload size */ - name_size = u16_strsize(name); + name_size = u16_strsize(variable_name); if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) { ret = EFI_INVALID_PARAMETER; goto out; @@ -300,11 +284,11 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, goto out;
/* Fill in contents */ - guidcpy(&var_acc->guid, guid); + guidcpy(&var_acc->guid, vendor); var_acc->data_size = tmp_dsize; var_acc->name_size = name_size; - var_acc->attr = attr ? *attr : 0; - memcpy(var_acc->name, name, name_size); + var_acc->attr = attributes ? *attributes : 0; + memcpy(var_acc->name, variable_name, name_size);
/* Communicate */ ret = mm_communicate(comm_buf, payload_size); @@ -315,8 +299,8 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, if (ret != EFI_SUCCESS) goto out;
- if (attr) - *attr = var_acc->attr; + if (attributes) + *attributes = var_acc->attr; if (data) memcpy(data, (u8 *)var_acc->name + var_acc->name_size, var_acc->data_size); @@ -325,7 +309,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
out: free(comm_buf); - return EFI_EXIT(ret); + return ret; }
/** @@ -417,24 +401,9 @@ out: return EFI_EXIT(ret); }
-/** - * efi_set_variable() - set value of a UEFI variable - * - * This function implements the SetVariable runtime service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * @name: name of the variable - * @guid: vendor GUID - * @attr: attributes of the variable - * @data_size: size of the buffer with the variable value - * @data: buffer with the variable value - * Return: status code - */ -efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, - u32 attr, efi_uintn_t data_size, - const void *data) +efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, + u32 attributes, efi_uintn_t data_size, + const void *data, bool ro_check) { struct smm_variable_access *var_acc; efi_uintn_t payload_size; @@ -442,9 +411,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, u8 *comm_buf = NULL; efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %x %zu %p", name, guid, attr, data_size, data); - - if (!name || name[0] == 0 || !guid) { + if (!variable_name || variable_name[0] == 0 || !vendor) { ret = EFI_INVALID_PARAMETER; goto out; } @@ -454,7 +421,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, }
/* Check payload size */ - name_size = u16_strsize(name); + name_size = u16_strsize(variable_name); payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size; if (payload_size > max_payload_size) { ret = EFI_INVALID_PARAMETER; @@ -468,11 +435,11 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, goto out;
/* Fill in contents */ - guidcpy(&var_acc->guid, guid); + guidcpy(&var_acc->guid, vendor); var_acc->data_size = data_size; var_acc->name_size = name_size; - var_acc->attr = attr; - memcpy(var_acc->name, name, name_size); + var_acc->attr = attributes; + memcpy(var_acc->name, variable_name, name_size); memcpy((u8 *)var_acc->name + name_size, data, data_size);
/* Communicate */ @@ -480,7 +447,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
out: free(comm_buf); - return EFI_EXIT(ret); + return ret; }
/** -- 2.27.0

On Mon, Jun 22, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
We currently have two implementations of UEFI variables:
- variables provided via an OP-TEE module
- variables stored in the U-Boot environment
Read only variables are up to now only implemented in the U-Boot environment implementation.
Provide a common interface for both implementations that allows handling read-only variables.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: add missing efi_variable.h consider attributes==NULL in efi_variable_get()
include/efi_variable.h | 40 +++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_variable.c | 171 ++++++++------------------- lib/efi_loader/efi_variable_common.c | 79 +++++++++++++ lib/efi_loader/efi_variable_tee.c | 75 ++++-------- 5 files changed, 188 insertions(+), 178 deletions(-) create mode 100644 include/efi_variable.h create mode 100644 lib/efi_loader/efi_variable_common.c
diff --git a/include/efi_variable.h b/include/efi_variable.h new file mode 100644 index 0000000000..784dbd9cf4 --- /dev/null +++ b/include/efi_variable.h
I think that all the stuff here should be put in efi_loader.h. I don't see any benefit of having a separate header.
@@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright (c) 2020, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#ifndef _EFI_VARIABLE_H +#define _EFI_VARIABLE_H
+#define EFI_VARIABLE_READ_ONLY BIT(31)
This is not part of UEFI specification. Having the same prefix, EFI_VARIABLE_, as other attributes can be confusing.
-Takahiro Akashi
+/**
- efi_get_variable() - retrieve value of a UEFI variable
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer to which the variable value is copied
- @data: buffer to which the variable value is copied
- Return: status code
- */
+efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
u32 *attributes, efi_uintn_t *data_size,
void *data);
+/**
- efi_set_variable() - set value of a UEFI variable
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer with the variable value
- @data: buffer with the variable value
- @ro_check: check the read only read only bit in attributes
- Return: status code
- */
+efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
u32 attributes, efi_uintn_t data_size,
const void *data, bool ro_check);
+#endif diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 57c7e66ea0..16b93ef7f0 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -35,6 +35,7 @@ obj-y += efi_root_node.o obj-y += efi_runtime.o obj-y += efi_setup.o obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o +obj-y += efi_variable_common.o ifeq ($(CONFIG_EFI_MM_COMM_TEE),y) obj-y += efi_variable_tee.o else diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index e097670e28..85df1427bc 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -7,6 +7,7 @@
#include <common.h> #include <efi_loader.h> +#include <efi_variable.h> #include <env.h> #include <env_internal.h> #include <hexdump.h> @@ -15,7 +16,6 @@ #include <search.h> #include <uuid.h> #include <crypto/pkcs7_parser.h> -#include <linux/bitops.h> #include <linux/compat.h> #include <u-boot/crc.h>
@@ -30,20 +30,6 @@ static bool efi_secure_boot; static int efi_secure_mode; static u8 efi_vendor_keys;
-#define READ_ONLY BIT(31)
-static efi_status_t efi_get_variable_common(u16 *variable_name,
const efi_guid_t *vendor,
u32 *attributes,
efi_uintn_t *data_size, void *data);
-static efi_status_t efi_set_variable_common(u16 *variable_name,
const efi_guid_t *vendor,
u32 attributes,
efi_uintn_t data_size,
const void *data,
bool ro_check);
/*
- Mapping between EFI variables and u-boot variables:
@@ -154,7 +140,7 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep) str++;
if ((s = prefix(str, "ro"))) {
attr |= READ_ONLY;
} else if ((s = prefix(str, "nv"))) { attr |= EFI_VARIABLE_NON_VOLATILE; } else if ((s = prefix(str, "boot"))) {attr |= EFI_VARIABLE_READ_ONLY;
@@ -202,29 +188,29 @@ static efi_status_t efi_set_secure_state(int sec_boot, int setup_mode,
attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS |
READ_ONLY;
- ret = efi_set_variable_common(L"SecureBoot", &efi_global_variable_guid,
attributes, sizeof(sec_boot), &sec_boot,
false);
EFI_VARIABLE_READ_ONLY;
- ret = efi_set_variable_int(L"SecureBoot", &efi_global_variable_guid,
attributes, sizeof(sec_boot), &sec_boot,
if (ret != EFI_SUCCESS) goto err;false);
- ret = efi_set_variable_common(L"SetupMode", &efi_global_variable_guid,
attributes, sizeof(setup_mode),
&setup_mode, false);
- ret = efi_set_variable_int(L"SetupMode", &efi_global_variable_guid,
attributes, sizeof(setup_mode),
if (ret != EFI_SUCCESS) goto err;&setup_mode, false);
- ret = efi_set_variable_common(L"AuditMode", &efi_global_variable_guid,
attributes, sizeof(audit_mode),
&audit_mode, false);
- ret = efi_set_variable_int(L"AuditMode", &efi_global_variable_guid,
attributes, sizeof(audit_mode),
if (ret != EFI_SUCCESS) goto err;&audit_mode, false);
- ret = efi_set_variable_common(L"DeployedMode",
&efi_global_variable_guid, attributes,
sizeof(deployed_mode), &deployed_mode,
false);
- ret = efi_set_variable_int(L"DeployedMode",
&efi_global_variable_guid, attributes,
sizeof(deployed_mode), &deployed_mode,
false);
err: return ret; } @@ -234,7 +220,7 @@ err:
- @mode: new state
- Depending on @mode, secure boot related variables are updated.
- Those variables are *read-only* for users, efi_set_variable_common()
- Those variables are *read-only* for users, efi_set_variable_int()
- is called here.
- Return: status code
@@ -252,10 +238,10 @@ static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
efi_secure_boot = true;
} else if (mode == EFI_MODE_AUDIT) {
ret = efi_set_variable_common(L"PK", &efi_global_variable_guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS,
0, NULL, false);
ret = efi_set_variable_int(L"PK", &efi_global_variable_guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS,
if (ret != EFI_SUCCESS) goto err;0, NULL, false);
@@ -307,8 +293,8 @@ static efi_status_t efi_init_secure_state(void) */
size = 0;
- ret = efi_get_variable_common(L"PK", &efi_global_variable_guid,
NULL, &size, NULL);
- ret = efi_get_variable_int(L"PK", &efi_global_variable_guid,
if (ret == EFI_BUFFER_TOO_SMALL) { if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) mode = EFI_MODE_USER;NULL, &size, NULL);
@@ -325,13 +311,13 @@ static efi_status_t efi_init_secure_state(void)
ret = efi_transfer_secure_state(mode); if (ret == EFI_SUCCESS)
ret = efi_set_variable_common(L"VendorKeys",
&efi_global_variable_guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS |
READ_ONLY,
sizeof(efi_vendor_keys),
&efi_vendor_keys, false);
ret = efi_set_variable_int(L"VendorKeys",
&efi_global_variable_guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS |
EFI_VARIABLE_READ_ONLY,
sizeof(efi_vendor_keys),
&efi_vendor_keys, false);
err: return ret; @@ -593,10 +579,9 @@ static efi_status_t efi_variable_authenticate(u16 *variable, } #endif /* CONFIG_EFI_SECURE_BOOT */
-static efi_status_t efi_get_variable_common(u16 *variable_name,
const efi_guid_t *vendor,
u32 *attributes,
efi_uintn_t *data_size, void *data)
+efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
u32 *attributes, efi_uintn_t *data_size,
void *data)
{ char *native_name; efi_status_t ret; @@ -679,35 +664,6 @@ out: return ret; }
-/**
- efi_efi_get_variable() - retrieve value of a UEFI variable
- This function implements the GetVariable runtime service.
- See the Unified Extensible Firmware Interface (UEFI) specification for
- details.
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer to which the variable value is copied
- @data: buffer to which the variable value is copied
- Return: status code
- */
-efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
const efi_guid_t *vendor, u32 *attributes,
efi_uintn_t *data_size, void *data)
-{
- efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %p %p %p", variable_name, vendor, attributes,
data_size, data);
- ret = efi_get_variable_common(variable_name, vendor, attributes,
data_size, data);
- return EFI_EXIT(ret);
-}
static char *efi_variables_list; static char *efi_cur_variable;
@@ -871,12 +827,9 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, return EFI_EXIT(ret); }
-static efi_status_t efi_set_variable_common(u16 *variable_name,
const efi_guid_t *vendor,
u32 attributes,
efi_uintn_t data_size,
const void *data,
bool ro_check)
+efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
u32 attributes, efi_uintn_t data_size,
const void *data, bool ro_check)
{ char *native_name = NULL, *old_data = NULL, *val = NULL, *s; efi_uintn_t old_size; @@ -899,15 +852,15 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* check if a variable exists */ old_size = 0; attr = 0;
- ret = efi_get_variable_common(variable_name, vendor, &attr,
&old_size, NULL);
ret = efi_get_variable_int(variable_name, vendor, &attr,
&old_size, NULL);
append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes);
/* check attributes */ if (old_size) {
if (ro_check && (attr & READ_ONLY)) {
}if (ro_check && (attr & EFI_VARIABLE_READ_ONLY)) { ret = EFI_WRITE_PROTECTED; goto err;
@@ -915,8 +868,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* attributes won't be changed */ if (!delete && ((ro_check && attr != attributes) ||
(!ro_check && ((attr & ~(u32)READ_ONLY)
!= (attributes & ~(u32)READ_ONLY))))) {
(!ro_check && ((attr & ~(u32)EFI_VARIABLE_READ_ONLY)
}!= (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { ret = EFI_INVALID_PARAMETER; goto err;
@@ -990,8 +943,8 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, ret = EFI_OUT_OF_RESOURCES; goto err; }
ret = efi_get_variable_common(variable_name, vendor,
&attr, &old_size, old_data);
ret = efi_get_variable_int(variable_name, vendor,
if (ret != EFI_SUCCESS) goto err; } else {&attr, &old_size, old_data);
@@ -1011,7 +964,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, /* * store attributes */
- attributes &= (READ_ONLY |
- attributes &= (EFI_VARIABLE_READ_ONLY | EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS |
@@ -1020,7 +973,7 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, while (attributes) { attr = 1 << (ffs(attributes) - 1);
if (attr == READ_ONLY) {
} else if (attr == EFI_VARIABLE_NON_VOLATILE) { s += sprintf(s, "nv");if (attr == EFI_VARIABLE_READ_ONLY) { s += sprintf(s, "ro");
@@ -1074,12 +1027,12 @@ out: /* update VendorKeys */ if (vendor_keys_modified & efi_vendor_keys) { efi_vendor_keys = 0;
ret = efi_set_variable_common(
ret = efi_set_variable_int( L"VendorKeys", &efi_global_variable_guid, EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS
| READ_ONLY,
| EFI_VARIABLE_READ_ONLY, sizeof(efi_vendor_keys), &efi_vendor_keys, false);
@@ -1096,36 +1049,6 @@ err: return ret; }
-/**
- efi_set_variable() - set value of a UEFI variable
- This function implements the SetVariable runtime service.
- See the Unified Extensible Firmware Interface (UEFI) specification for
- details.
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer with the variable value
- @data: buffer with the variable value
- Return: status code
- */
-efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
const efi_guid_t *vendor, u32 attributes,
efi_uintn_t data_size, const void *data)
-{
- EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes,
data_size, data);
- /* READ_ONLY bit is not part of API */
- attributes &= ~(u32)READ_ONLY;
- return EFI_EXIT(efi_set_variable_common(variable_name, vendor,
attributes, data_size, data,
true));
-}
/**
- efi_query_variable_info() - get information about EFI variables
diff --git a/lib/efi_loader/efi_variable_common.c b/lib/efi_loader/efi_variable_common.c new file mode 100644 index 0000000000..e6a39fceac --- /dev/null +++ b/lib/efi_loader/efi_variable_common.c @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- UEFI runtime variable services
- Copyright (c) 2020, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <efi_loader.h> +#include <efi_variable.h> +#include <linux/bitops.h>
+/**
- efi_efi_get_variable() - retrieve value of a UEFI variable
- This function implements the GetVariable runtime service.
- See the Unified Extensible Firmware Interface (UEFI) specification for
- details.
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer to which the variable value is copied
- @data: buffer to which the variable value is copied
- Return: status code
- */
+efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
const efi_guid_t *vendor, u32 *attributes,
efi_uintn_t *data_size, void *data)
+{
- efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %p %p %p", variable_name, vendor, attributes,
data_size, data);
- ret = efi_get_variable_int(variable_name, vendor, attributes,
data_size, data);
- /* Remove EFI_VARIABLE_READ_ONLY flag */
- if (attributes)
*attributes &= EFI_VARIABLE_MASK;
- return EFI_EXIT(ret);
+}
+/**
- efi_set_variable() - set value of a UEFI variable
- This function implements the SetVariable runtime service.
- See the Unified Extensible Firmware Interface (UEFI) specification for
- details.
- @variable_name: name of the variable
- @vendor: vendor GUID
- @attributes: attributes of the variable
- @data_size: size of the buffer with the variable value
- @data: buffer with the variable value
- Return: status code
- */
+efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
const efi_guid_t *vendor, u32 attributes,
efi_uintn_t data_size, const void *data)
+{
- efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes,
data_size, data);
- /* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */
- if (attributes & ~(u32)EFI_VARIABLE_MASK)
ret = EFI_INVALID_PARAMETER;
- else
ret = efi_set_variable_int(variable_name, vendor, attributes,
data_size, data, true);
- return EFI_EXIT(ret);
+} diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index cacc76e23d..2513878c82 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -10,6 +10,7 @@ #include <efi.h> #include <efi_api.h> #include <efi_loader.h> +#include <efi_variable.h> #include <tee.h> #include <malloc.h> #include <mm_communication.h> @@ -243,24 +244,9 @@ out: return ret; }
-/**
- efi_get_variable() - retrieve value of a UEFI variable
- This function implements the GetVariable runtime service.
- See the Unified Extensible Firmware Interface (UEFI) specification for
- details.
- @name: name of the variable
- @guid: vendor GUID
- @attr: attributes of the variable
- @data_size: size of the buffer to which the variable value is copied
- @data: buffer to which the variable value is copied
- Return: status code
- */
-efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
u32 *attr, efi_uintn_t *data_size,
void *data)
+efi_status_t efi_get_variable_int(u16 *variable_name, const efi_guid_t *vendor,
u32 *attributes, efi_uintn_t *data_size,
void *data)
{ struct smm_variable_access *var_acc; efi_uintn_t payload_size; @@ -269,15 +255,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, u8 *comm_buf = NULL; efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %p %p %p", name, guid, attr, data_size, data);
- if (!name || !guid || !data_size) {
if (!variable_name || !vendor || !data_size) { ret = EFI_INVALID_PARAMETER; goto out; }
/* Check payload size */
- name_size = u16_strsize(name);
- name_size = u16_strsize(variable_name); if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) { ret = EFI_INVALID_PARAMETER; goto out;
@@ -300,11 +284,11 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, goto out;
/* Fill in contents */
- guidcpy(&var_acc->guid, guid);
- guidcpy(&var_acc->guid, vendor); var_acc->data_size = tmp_dsize; var_acc->name_size = name_size;
- var_acc->attr = attr ? *attr : 0;
- memcpy(var_acc->name, name, name_size);
var_acc->attr = attributes ? *attributes : 0;
memcpy(var_acc->name, variable_name, name_size);
/* Communicate */ ret = mm_communicate(comm_buf, payload_size);
@@ -315,8 +299,8 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid, if (ret != EFI_SUCCESS) goto out;
- if (attr)
*attr = var_acc->attr;
- if (attributes)
if (data) memcpy(data, (u8 *)var_acc->name + var_acc->name_size, var_acc->data_size);*attributes = var_acc->attr;
@@ -325,7 +309,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *name, const efi_guid_t *guid,
out: free(comm_buf);
- return EFI_EXIT(ret);
- return ret;
}
/** @@ -417,24 +401,9 @@ out: return EFI_EXIT(ret); }
-/**
- efi_set_variable() - set value of a UEFI variable
- This function implements the SetVariable runtime service.
- See the Unified Extensible Firmware Interface (UEFI) specification for
- details.
- @name: name of the variable
- @guid: vendor GUID
- @attr: attributes of the variable
- @data_size: size of the buffer with the variable value
- @data: buffer with the variable value
- Return: status code
- */
-efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
u32 attr, efi_uintn_t data_size,
const void *data)
+efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor,
u32 attributes, efi_uintn_t data_size,
const void *data, bool ro_check)
{ struct smm_variable_access *var_acc; efi_uintn_t payload_size; @@ -442,9 +411,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, u8 *comm_buf = NULL; efi_status_t ret;
- EFI_ENTRY(""%ls" %pUl %x %zu %p", name, guid, attr, data_size, data);
- if (!name || name[0] == 0 || !guid) {
- if (!variable_name || variable_name[0] == 0 || !vendor) { ret = EFI_INVALID_PARAMETER; goto out; }
@@ -454,7 +421,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, }
/* Check payload size */
- name_size = u16_strsize(name);
- name_size = u16_strsize(variable_name); payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size; if (payload_size > max_payload_size) { ret = EFI_INVALID_PARAMETER;
@@ -468,11 +435,11 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid, goto out;
/* Fill in contents */
- guidcpy(&var_acc->guid, guid);
- guidcpy(&var_acc->guid, vendor); var_acc->data_size = data_size; var_acc->name_size = name_size;
- var_acc->attr = attr;
- memcpy(var_acc->name, name, name_size);
var_acc->attr = attributes;
memcpy(var_acc->name, variable_name, name_size); memcpy((u8 *)var_acc->name + name_size, data, data_size);
/* Communicate */
@@ -480,7 +447,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *name, const efi_guid_t *guid,
out: free(comm_buf);
- return EFI_EXIT(ret);
- return ret;
}
/**
2.27.0

On 6/23/20 1:44 AM, AKASHI Takahiro wrote:
On Mon, Jun 22, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
We currently have two implementations of UEFI variables:
- variables provided via an OP-TEE module
- variables stored in the U-Boot environment
Read only variables are up to now only implemented in the U-Boot environment implementation.
Provide a common interface for both implementations that allows handling read-only variables.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: add missing efi_variable.h consider attributes==NULL in efi_variable_get()
include/efi_variable.h | 40 +++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_variable.c | 171 ++++++++------------------- lib/efi_loader/efi_variable_common.c | 79 +++++++++++++ lib/efi_loader/efi_variable_tee.c | 75 ++++-------- 5 files changed, 188 insertions(+), 178 deletions(-) create mode 100644 include/efi_variable.h create mode 100644 lib/efi_loader/efi_variable_common.c
diff --git a/include/efi_variable.h b/include/efi_variable.h new file mode 100644 index 0000000000..784dbd9cf4 --- /dev/null +++ b/include/efi_variable.h
I think that all the stuff here should be put in efi_loader.h. I don't see any benefit of having a separate header.
This is more or less a question of taste. My motivation is:
* efi_loader.h is rather large (805 lines). * Other variable functions will be added. * The functions defined here are used only in very few places while efi_loader.h is included in 57 files.
Best regards
Heinrich

On Wed, Jun 24, 2020 at 07:51:42AM +0200, Heinrich Schuchardt wrote:
On 6/23/20 1:44 AM, AKASHI Takahiro wrote:
On Mon, Jun 22, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
We currently have two implementations of UEFI variables:
- variables provided via an OP-TEE module
- variables stored in the U-Boot environment
Read only variables are up to now only implemented in the U-Boot environment implementation.
Provide a common interface for both implementations that allows handling read-only variables.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: add missing efi_variable.h consider attributes==NULL in efi_variable_get()
include/efi_variable.h | 40 +++++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_variable.c | 171 ++++++++------------------- lib/efi_loader/efi_variable_common.c | 79 +++++++++++++ lib/efi_loader/efi_variable_tee.c | 75 ++++-------- 5 files changed, 188 insertions(+), 178 deletions(-) create mode 100644 include/efi_variable.h create mode 100644 lib/efi_loader/efi_variable_common.c
diff --git a/include/efi_variable.h b/include/efi_variable.h new file mode 100644 index 0000000000..784dbd9cf4 --- /dev/null +++ b/include/efi_variable.h
I think that all the stuff here should be put in efi_loader.h. I don't see any benefit of having a separate header.
This is more or less a question of taste. My motivation is:
I can agree, but at the same time, I don't like such an ad-hoc confusing approach. I think that you should have a firm discipline.
- efi_loader.h is rather large (805 lines).
- Other variable functions will be added.
- The functions defined here are used only in very few places while efi_loader.h is included in 57 files.
If we allow this, we will have a number of small headers, which will contradict a notion of efi_loader.h.
-Takahiro Akashi
Best regards
Heinrich
participants (2)
-
AKASHI Takahiro
-
Heinrich Schuchardt