* [PATCH 0/6 v2] EFI variable support via OP-TEE
@ 2020-05-11 18:13 Ilias Apalodimas
2020-05-11 18:13 ` [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication Ilias Apalodimas
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-11 18:13 UTC (permalink / raw)
To: u-boot
Hi!
This is the v2 of the patchset adding EFI variable support via OP-TEE
originally posted here [1]
Changes since v1:
* patch #2:
- Fix Copyright issues
- Merge the include files in mm_communication.h
- Rename some variables and follow EDK2 naming
- Added proper documentation on struct definitions
- Add missing defines (unused but good to have there)
- Use flexible array members on structs and replace data[1] with data[]
* patch #3:
- Adjust efi_variable_tee.c to header file changes and fix typos
* patch #4:
- Remove EFI_HANDLE_WIDTH on printing
- Rephrase 'efidebug query' help message
* patch #5
- Move mm_communication.h maintenership
* patch #6
- Heinrich's suggestion, on the help file, was a lot cleaner and easier
to understand. Using it as-is
[1] https://lists.denx.de/pipermail/u-boot/2020-May/410772.html
Ilias Apalodimas (4):
efi_loader: Implement EFI variable handling via OP-TEE
cmd: efidebug: Add support for querying UEFI variable storage
MAINTAINERS: Add maintainer for EFI variables via OP-TEE
doc: uefi.rst: Add OP-TEE variable storage config options
Sughosh Ganu (1):
efi_loader: Add headers for EDK2 StandAloneMM communication
MAINTAINERS | 6 +
cmd/efidebug.c | 44 +-
doc/uefi/uefi.rst | 17 +
include/mm_communication.h | 207 ++++++++++
lib/efi_loader/Kconfig | 9 +
lib/efi_loader/Makefile | 4 +
lib/efi_loader/efi_variable_tee.c | 643 ++++++++++++++++++++++++++++++
7 files changed, 929 insertions(+), 1 deletion(-)
create mode 100644 include/mm_communication.h
create mode 100644 lib/efi_loader/efi_variable_tee.c
--
2.26.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication
2020-05-11 18:13 [PATCH 0/6 v2] EFI variable support via OP-TEE Ilias Apalodimas
@ 2020-05-11 18:13 ` Ilias Apalodimas
2020-05-11 19:39 ` Heinrich Schuchardt
2020-05-11 18:14 ` [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE Ilias Apalodimas
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-11 18:13 UTC (permalink / raw)
To: u-boot
From: Sughosh Ganu <sughosh.ganu@linaro.org>
In Arm devices OP-TEE has the ability to run StandAloneMM (from EDK2)
in a separate partition and handle UEFI variables.
A following patch introduces this functionality.
Add the headers needed for OP-TEE <--> StandAloneMM communication
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
include/mm_communication.h | 207 +++++++++++++++++++++++++++++++++++++
1 file changed, 207 insertions(+)
create mode 100644 include/mm_communication.h
diff --git a/include/mm_communication.h b/include/mm_communication.h
new file mode 100644
index 000000000000..b9bfbe4cf0a1
--- /dev/null
+++ b/include/mm_communication.h
@@ -0,0 +1,207 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Headers for EFI variable service via StandAloneMM, EDK2 application running
+ * in OP-TEE
+ *
+ * Copyright (c) 2017, Intel Corporation. All rights reserved.
+ * Copyright (C) 2020 Linaro Ltd. <sughosh.ganu@linaro.org>
+ * Copyright (C) 2020 Linaro Ltd. <ilias.apalodimas@linaro.org>
+ */
+
+#ifndef _MM_VARIABLE_H_
+#define _MM_VARIABLE_H_
+
+#include <part_efi.h>
+
+/*
+ * Interface to the pseudo TA, which provides a communication channel with
+ * the StandaloneMM Secure Partition (StMM) running at S-EL0
+ */
+
+#define PTA_STMM_CMDID_COMMUNICATE 0
+
+/* OP-TEE is using big endian GUIDs while UEFI uses little endian ones */
+#define PTA_STMM_UUID { 0xed32d533, 0x99e6, 0x4209, {\
+ 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7 } }
+
+#define EFI_MM_VARIABLE_GUID \
+ EFI_GUID(0xed32d533, 0x99e6, 0x4209, \
+ 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7)
+
+/* Defined in EDK2 MdePkg/Include/Protocol/MmCommunication.h */
+
+/**
+ * struct efi_mm_communicate_header - Header used for SMM variable communication
+
+ * @header_guid: header use for disambiguation of content
+ * @message_len: length of the message. Does not include the size of the
+ * header
+ * @data: payload of the message
+ *
+ * Defined in EDK2 as EFI_MM_COMMUNICATE_HEADER
+ * To avoid confusion in interpreting frames, the communication buffer should
+ * always begin with efi_mm_communicate_header
+ */
+struct efi_mm_communicate_header {
+ efi_guid_t header_guid;
+ size_t message_len;
+ u8 data[];
+};
+
+#define MM_COMMUNICATE_HEADER_SIZE \
+ (sizeof(struct efi_mm_communicate_header))
+
+/* Defined in EDK2 ArmPkg/Include/IndustryStandard/ArmStdSmc.h */
+
+/* MM return error codes */
+#define ARM_SMC_MM_RET_SUCCESS 0
+#define ARM_SMC_MM_RET_NOT_SUPPORTED -1
+#define ARM_SMC_MM_RET_INVALID_PARAMS -2
+#define ARM_SMC_MM_RET_DENIED -3
+#define ARM_SMC_MM_RET_NO_MEMORY -4
+
+/* Defined in EDK2 MdeModulePkg/Include/Guid/SmmVariableCommon.h */
+
+#define SMM_VARIABLE_FUNCTION_GET_VARIABLE 1
+/*
+ * The payload for this function is
+ * SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME.
+ */
+#define SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME 2
+/*
+ * The payload for this function is SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE.
+ */
+#define SMM_VARIABLE_FUNCTION_SET_VARIABLE 3
+/*
+ * The payload for this function is
+ * SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO.
+ */
+#define SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO 4
+/*
+ * It is a notify event, no extra payload for this function.
+ */
+#define SMM_VARIABLE_FUNCTION_READY_TO_BOOT 5
+/*
+ * It is a notify event, no extra payload for this function.
+ */
+#define SMM_VARIABLE_FUNCTION_EXIT_BOOT_SERVICE 6
+/*
+ * The payload for this function is VARIABLE_INFO_ENTRY.
+ * The GUID in EFI_SMM_COMMUNICATE_HEADER is gEfiSmmVariableProtocolGuid.
+ */
+#define SMM_VARIABLE_FUNCTION_GET_STATISTICS 7
+/*
+ * The payload for this function is SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE
+ */
+#define SMM_VARIABLE_FUNCTION_LOCK_VARIABLE 8
+
+#define SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_SET 9
+
+#define SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET 10
+
+#define SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE 11
+/*
+ * The payload for this function is
+ * SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
+ */
+#define SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT 12
+
+#define SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE 13
+/*
+ * The payload for this function is
+ * SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO
+ */
+#define SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO 14
+
+/**
+ * struct smm_variable_communicate_header - Used for SMM variable communication
+
+ * @function: function to call in Smm.
+ * @ret_status: return status
+ * @data: payload
+ *
+ * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_HEADER
+ */
+struct smm_variable_communicate_header {
+ efi_uintn_t function;
+ efi_status_t ret_status;
+ u8 data[];
+};
+
+#define MM_VARIABLE_COMMUNICATE_SIZE \
+ (sizeof(struct smm_variable_communicate_header))
+
+/**
+ * struct smm_variable_access - Used to communicate with StMM by
+ * SetVariable and GetVariable.
+
+ * @function: vendor GUID to call in Smm
+ * @data_size: size of EFI variable data
+ * @name_size: size of EFI name
+ * @attr: attributes
+ * @name: variable name
+ *
+ * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE
+ *
+ */
+struct smm_variable_access {
+ efi_guid_t guid;
+ efi_uintn_t data_size;
+ efi_uintn_t name_size;
+ u32 attr;
+ u16 name[];
+};
+
+#define MM_VARIABLE_ACCESS_HEADER_SIZE \
+ (sizeof(struct smm_variable_access))
+/**
+ * struct smm_variable_payload_size - Used to get the max allowed
+ * payload used in StMM.
+ *
+ * @size: size to fill in
+ *
+ * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
+ *
+ */
+struct smm_variable_payload_size {
+ efi_uintn_t size;
+};
+
+/**
+ * struct smm_variable_getnext - Used to communicate with StMM for
+ * GetNextVariableName.
+ *
+ * @size: vendor GUID
+ * @name_size: size of the name of the variable
+ * @name: variable name
+ *
+ * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
+ */
+struct smm_variable_getnext {
+ efi_guid_t guid;
+ efi_uintn_t name_size;
+ u16 name[];
+};
+
+#define MM_VARIABLE_GET_NEXT_HEADER_SIZE \
+ (sizeof(struct smm_variable_getnext))
+
+/**
+ * struct smm_variable_query_info - Used to communicate with StMM for
+ * QueryVariableInfo.
+ *
+ * @max_variable_storage: max available storage
+ * @remaining_variable_storage: remaining available storage
+ * @max_variable_size: max variable supported size
+ * @attr: attributes to query storage for
+ *
+ * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
+ */
+struct smm_variable_query_info {
+ u64 max_variable_storage;
+ u64 remaining_variable_storage;
+ u64 max_variable_size;
+ u32 attr;
+};
+
+#endif /* _MM_VARIABLE_H_ */
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE
2020-05-11 18:13 [PATCH 0/6 v2] EFI variable support via OP-TEE Ilias Apalodimas
2020-05-11 18:13 ` [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication Ilias Apalodimas
@ 2020-05-11 18:14 ` Ilias Apalodimas
2020-05-13 6:14 ` Heinrich Schuchardt
2020-05-15 11:55 ` Heinrich Schuchardt
2020-05-11 18:14 ` [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage Ilias Apalodimas
` (2 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-11 18:14 UTC (permalink / raw)
To: u-boot
In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
StandAloneMM is responsible for the UEFI variable support. In
combination with OP-TEE and it's U-Boot supplicant, variables are
authenticated/validated in secure world and stored on an RPMB partition.
So let's add a new config option in U-Boot implementing the necessary
calls to OP-TEE for the variable management.
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Pipat Methavanitpong <pipat1010@gmail.com>
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
lib/efi_loader/Kconfig | 9 +
lib/efi_loader/Makefile | 4 +
lib/efi_loader/efi_variable_tee.c | 643 ++++++++++++++++++++++++++++++
3 files changed, 656 insertions(+)
create mode 100644 lib/efi_loader/efi_variable_tee.c
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 1cfa24ffcf72..e385cd0b9dae 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -164,4 +164,13 @@ config EFI_SECURE_BOOT
it is signed with a trusted key. To do that, you need to install,
at least, PK, KEK and db.
+config EFI_MM_COMM_TEE
+ bool "UEFI variables storage service via OP-TEE"
+ depends on SUPPORT_EMMC_RPMB
+ default n
+ help
+ If OP-TEE is present and running StandAloneMM dispatch all UEFI variable
+ related operations to that. The application will verify, authenticate and
+ store the variables on an RPMB
+
endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index eff3c25ec301..dba652121eab 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -34,7 +34,11 @@ 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
+ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
+obj-y += efi_variable_tee.o
+else
obj-y += efi_variable.o
+endif
obj-y += efi_watchdog.o
obj-$(CONFIG_LCD) += efi_gop.o
obj-$(CONFIG_DM_VIDEO) += efi_gop.o
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
new file mode 100644
index 000000000000..f8857bb81e72
--- /dev/null
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -0,0 +1,643 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * EFI variable service via OP-TEE
+ *
+ * Copyright (C) 2019 Linaro Ltd. <sughosh.ganu@linaro.org>
+ * Copyright (C) 2019 Linaro Ltd. <ilias.apalodimas@linaro.org>
+ */
+
+#include <common.h>
+#include <efi.h>
+#include <efi_api.h>
+#include <efi_loader.h>
+#include <tee.h>
+#include <malloc.h>
+#include <mm_communication.h>
+
+static efi_uintn_t max_buffer_size; /* comm + var + func + data */
+static efi_uintn_t max_payload_size; /* func + data */
+
+struct mm_connection {
+ struct udevice *tee;
+ u32 session;
+};
+
+/**
+ * get_connection() - Retrieve OP-TEE session for a specific UUID.
+ *
+ * @conn: session buffer to fill
+ * Return: status code
+ */
+static int get_connection(struct mm_connection *conn)
+{
+ static const struct tee_optee_ta_uuid uuid = PTA_STMM_UUID;
+ struct udevice *tee = NULL;
+ struct tee_open_session_arg arg;
+ int rc;
+
+ tee = tee_find_device(tee, NULL, NULL, NULL);
+ if (!tee)
+ return -ENODEV;
+
+ memset(&arg, 0, sizeof(arg));
+ tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
+ rc = tee_open_session(tee, &arg, 0, NULL);
+ if (!rc) {
+ conn->tee = tee;
+ conn->session = arg.session;
+ }
+
+ return rc;
+}
+
+/**
+ * optee_mm_communicate() - Pass a buffer to StandaloneMM running in OP-TEE
+ *
+ * @comm_buf: locally allocted communcation buffer
+ * @dsize: buffer size
+ * Return: status code
+ */
+static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
+{
+ ulong buf_size;
+ efi_status_t ret;
+ struct efi_mm_communicate_header *mm_hdr;
+ struct mm_connection conn = { NULL, 0 };
+ struct tee_invoke_arg arg;
+ struct tee_param param[2];
+ struct tee_shm *shm = NULL;
+ int rc;
+
+ if (!comm_buf)
+ return EFI_INVALID_PARAMETER;
+
+ mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
+ buf_size = mm_hdr->message_len + sizeof(efi_guid_t) + sizeof(size_t);
+
+ if (dsize != buf_size)
+ return EFI_INVALID_PARAMETER;
+
+ rc = get_connection(&conn);
+ if (rc) {
+ log_err("Unable to open OP-TEE session (err=%d)\n", rc);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {
+ log_err("Unable to register shared memory\n");
+ return EFI_UNSUPPORTED;
+ }
+
+ memset(&arg, 0, sizeof(arg));
+ arg.func = PTA_STMM_CMDID_COMMUNICATE;
+ arg.session = conn.session;
+
+ memset(param, 0, sizeof(param));
+ param[0].attr = TEE_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ param[0].u.memref.size = buf_size;
+ param[0].u.memref.shm = shm;
+ param[1].attr = TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT;
+
+ rc = tee_invoke_func(conn.tee, &arg, 2, param);
+ if (rc)
+ return EFI_INVALID_PARAMETER;
+ tee_shm_free(shm);
+ tee_close_session(conn.tee, conn.session);
+
+ switch (param[1].u.value.a) {
+ case ARM_SMC_MM_RET_SUCCESS:
+ ret = EFI_SUCCESS;
+ break;
+
+ case ARM_SMC_MM_RET_INVALID_PARAMS:
+ ret = EFI_INVALID_PARAMETER;
+ break;
+
+ case ARM_SMC_MM_RET_DENIED:
+ ret = EFI_ACCESS_DENIED;
+ break;
+
+ case ARM_SMC_MM_RET_NO_MEMORY:
+ ret = EFI_OUT_OF_RESOURCES;
+ break;
+
+ default:
+ ret = EFI_ACCESS_DENIED;
+ }
+
+ return ret;
+}
+
+/**
+ * mm_communicate() - Adjust the cmonnucation buffer to StandAlonneMM and send
+ * it to OP-TEE
+ *
+ * @comm_buf: locally allocted communcation buffer
+ * @dsize: buffer size
+ * Return: status code
+ */
+static efi_status_t mm_communicate(u8 *comm_buf, efi_uintn_t dsize)
+{
+ efi_status_t ret;
+ struct efi_mm_communicate_header *mm_hdr;
+ struct smm_variable_communicate_header *var_hdr;
+
+ dsize += MM_COMMUNICATE_HEADER_SIZE + MM_VARIABLE_COMMUNICATE_SIZE;
+ mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
+ var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data;
+
+ ret = optee_mm_communicate(comm_buf, dsize);
+ if (ret != EFI_SUCCESS) {
+ log_err("%s failed!\n", __func__);
+ return ret;
+ }
+
+ return var_hdr->ret_status;
+}
+
+/**
+ * setup_mm_hdr() - Allocate a buffer for StandAloneMM and initialize the
+ * header data.
+ *
+ * @dptr: pointer address of the corresponding StandAloneMM
+ * function
+ * @payload_size: buffer size
+ * @func: standAloneMM function number
+ * @ret: EFI return code
+ * Return: buffer or NULL
+ */
+static u8 *setup_mm_hdr(void **dptr, efi_uintn_t payload_size,
+ efi_uintn_t func, efi_status_t *ret)
+{
+ const efi_guid_t mm_var_guid = EFI_MM_VARIABLE_GUID;
+ struct efi_mm_communicate_header *mm_hdr;
+ struct smm_variable_communicate_header *var_hdr;
+ u8 *comm_buf;
+
+ /* In the init function we initialize max_buffer_size with
+ * get_max_payload(). So skip the test if max_buffer_size is initialized
+ * StandAloneMM will perform similar checks and drop the buffer if it's
+ * too long
+ */
+ if (max_buffer_size && max_buffer_size <
+ (MM_COMMUNICATE_HEADER_SIZE +
+ MM_VARIABLE_COMMUNICATE_SIZE +
+ payload_size)) {
+ *ret = EFI_INVALID_PARAMETER;
+ return NULL;
+ }
+
+ comm_buf = calloc(1, MM_COMMUNICATE_HEADER_SIZE +
+ MM_VARIABLE_COMMUNICATE_SIZE +
+ payload_size);
+ if (!comm_buf) {
+ *ret = EFI_OUT_OF_RESOURCES;
+ return NULL;
+ }
+
+ mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
+ guidcpy(&mm_hdr->header_guid, &mm_var_guid);
+ mm_hdr->message_len = MM_VARIABLE_COMMUNICATE_SIZE + payload_size;
+
+ var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data;
+ var_hdr->function = func;
+ if (dptr)
+ *dptr = var_hdr->data;
+ *ret = EFI_SUCCESS;
+
+ return comm_buf;
+}
+
+/**
+ * get_max_payload() - Get variable payload size from StandAloneMM.
+ *
+ * @size: size of the variable in storage
+ * Return: status code
+ */
+efi_status_t EFIAPI get_max_payload(efi_uintn_t *size)
+{
+ struct smm_variable_payload_size *var_payload = NULL;
+ efi_uintn_t payload_size;
+ efi_status_t ret;
+ u8 *comm_buf;
+
+ if (!size)
+ return EFI_INVALID_PARAMETER;
+
+ payload_size = sizeof(*var_payload);
+ comm_buf = setup_mm_hdr((void **)&var_payload, payload_size,
+ SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE, &ret);
+ if (!comm_buf)
+ return EFI_EXIT(ret);
+
+ ret = mm_communicate(comm_buf, payload_size);
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ *size = var_payload->size;
+
+out:
+ free(comm_buf);
+ 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)
+{
+ struct smm_variable_access *var_acc;
+ efi_uintn_t payload_size;
+ efi_uintn_t name_size;
+ efi_uintn_t tmp_dsize;
+ efi_status_t ret;
+ u8 *comm_buf;
+
+ EFI_ENTRY("\"%ls\" %pUl %p %p %p", name, guid, attr, data_size, data);
+
+ if (!name || !guid || !data_size)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+ /* Check payload size */
+ name_size = u16_strsize(name);
+ if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+ /* Trim output buffer size */
+ tmp_dsize = *data_size;
+ if (name_size + tmp_dsize >
+ max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) {
+ tmp_dsize = max_payload_size -
+ MM_VARIABLE_ACCESS_HEADER_SIZE -
+ name_size;
+ }
+
+ /* Get communication buffer and initialize header */
+ payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + tmp_dsize;
+ comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
+ SMM_VARIABLE_FUNCTION_GET_VARIABLE, &ret);
+ if (!comm_buf)
+ return EFI_EXIT(ret);
+
+ /* Fill in contents */
+ guidcpy(&var_acc->guid, guid);
+ 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);
+
+ /* Communicate */
+ ret = mm_communicate(comm_buf, payload_size);
+ if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
+ /* Update with reported data size for trimmed case */
+ *data_size = var_acc->data_size;
+ }
+ if (ret != EFI_SUCCESS)
+ goto done;
+
+ if (attr)
+ *attr = var_acc->attr;
+ if (data)
+ memcpy(data, (u8 *)var_acc->name + var_acc->name_size,
+ var_acc->data_size);
+ else
+ ret = EFI_INVALID_PARAMETER;
+
+done:
+ free(comm_buf);
+ return EFI_EXIT(ret);
+}
+
+/**
+ * efi_get_next_variable_name() - enumerate the current variable names
+ *
+ * @variable_name_size: size of variable_name buffer in byte
+ * @variable_name: name of uefi variable's name in u16
+ * @guid: vendor's guid
+ *
+ * This function implements the GetNextVariableName service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * Return: status code
+ */
+efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
+ u16 *variable_name,
+ efi_guid_t *guid)
+{
+ struct smm_variable_getnext *var_getnext;
+ efi_uintn_t payload_size;
+ efi_uintn_t tmp_dsize;
+ efi_uintn_t name_size;
+ efi_status_t ret;
+ efi_uintn_t out_name_size;
+ efi_uintn_t in_name_size;
+ u8 *comm_buf;
+
+ EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, guid);
+
+ if (!variable_name_size || !variable_name || !guid)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+ out_name_size = *variable_name_size;
+ in_name_size = u16_strsize(variable_name);
+
+ name_size = u16_strsize(variable_name);
+ if (name_size > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+ /* Trim output buffer size */
+ tmp_dsize = *variable_name_size;
+ if (name_size + tmp_dsize >
+ max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) {
+ tmp_dsize = max_payload_size -
+ MM_VARIABLE_GET_NEXT_HEADER_SIZE -
+ name_size;
+ }
+
+ payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE +
+ max(out_name_size, in_name_size);
+ comm_buf = setup_mm_hdr((void **)&var_getnext, payload_size,
+ SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME,
+ &ret);
+ if (!comm_buf)
+ return EFI_EXIT(ret);
+
+ /* Fill in contents */
+ guidcpy(&var_getnext->guid, guid);
+ var_getnext->name_size = out_name_size;
+ memcpy(var_getnext->name, variable_name, in_name_size);
+ if (out_name_size > in_name_size) {
+ memset((u8 *)var_getnext->name + in_name_size, 0x0,
+ out_name_size - in_name_size);
+ }
+
+ /* Communicate */
+ ret = mm_communicate(comm_buf, payload_size);
+ if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
+ /* Update with reported data size for trimmed case */
+ *variable_name_size = var_getnext->name_size;
+ }
+ if (ret != EFI_SUCCESS)
+ goto done;
+
+ guidcpy(guid, &var_getnext->guid);
+ memcpy(variable_name, (u8 *)var_getnext->name,
+ var_getnext->name_size);
+
+done:
+ free(comm_buf);
+ 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)
+{
+ struct smm_variable_access *var_acc;
+ efi_uintn_t payload_size;
+ efi_uintn_t name_size;
+ efi_status_t ret;
+ u8 *comm_buf;
+
+ EFI_ENTRY("\"%ls\" %pUl %x %zu %p", name, guid, attr, data_size, data);
+
+ if (!name || name[0] == 0 || !guid)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+ if (data_size > 0 && !data)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+ /* Check payload size */
+ name_size = u16_strsize(name);
+ payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size;
+ if (payload_size > max_payload_size)
+ return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+ /* Get communication buffer and initialize header */
+ comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
+ SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret);
+ if (!comm_buf)
+ return EFI_EXIT(ret);
+
+ /* Fill in contents */
+ guidcpy(&var_acc->guid, guid);
+ var_acc->data_size = data_size;
+ var_acc->name_size = name_size;
+ var_acc->attr = attr;
+ memcpy(var_acc->name, name, name_size);
+ memcpy((u8 *)var_acc->name + name_size, data, data_size);
+
+ /* Communicate */
+ ret = mm_communicate(comm_buf, payload_size);
+ free(comm_buf);
+
+ return EFI_EXIT(ret);
+}
+
+/**
+ * efi_query_variable_info() - get information about EFI variables
+ *
+ * This function implements the QueryVariableInfo() runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @attributes: bitmask to select variables to be
+ * queried
+ * @maximum_variable_storage_size: maximum size of storage area for the
+ * selected variable types
+ * @remaining_variable_storage_size: remaining size of storage are for the
+ * selected variable types
+ * @maximum_variable_size: maximum size of a variable of the
+ * selected type
+ * Returns: status code
+ */
+efi_status_t EFIAPI __efi_runtime
+efi_query_variable_info(u32 attributes, u64 *max_variable_storage_size,
+ u64 *remain_variable_storage_size,
+ u64 *max_variable_size)
+{
+ struct smm_variable_query_info *mm_query_info;
+ efi_uintn_t payload_size;
+ efi_status_t ret;
+ u8 *comm_buf;
+
+ EFI_ENTRY("%x %p %p %p", attributes, max_variable_storage_size,
+ remain_variable_storage_size, max_variable_size);
+
+ payload_size = sizeof(*mm_query_info);
+ comm_buf = setup_mm_hdr((void **)&mm_query_info, payload_size,
+ SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO,
+ &ret);
+ if (!comm_buf)
+ return EFI_EXIT(ret);
+
+ mm_query_info->attr = attributes;
+ ret = mm_communicate(comm_buf, payload_size);
+ if (ret != EFI_SUCCESS)
+ goto out;
+ *max_variable_storage_size = mm_query_info->max_variable_storage;
+ *remain_variable_storage_size =
+ mm_query_info->remaining_variable_storage;
+ *max_variable_size = mm_query_info->max_variable_size;
+
+out:
+ free(comm_buf);
+ return EFI_EXIT(ret);
+}
+
+/**
+ * efi_get_variable_runtime() - runtime implementation of GetVariable()
+ *
+ * @variable_name: name of the variable
+ * @guid: 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
+ */
+static efi_status_t __efi_runtime EFIAPI
+efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
+ u32 *attributes, efi_uintn_t *data_size, void *data)
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * efi_get_next_variable_name_runtime() - runtime implementation of
+ * GetNextVariable()
+ *
+ * @variable_name_size: size of variable_name buffer in byte
+ * @variable_name: name of uefi variable's name in u16
+ * @guid: vendor's guid
+ * Return: status code
+ */
+static efi_status_t __efi_runtime EFIAPI
+efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
+ u16 *variable_name, efi_guid_t *guid)
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * efi_query_variable_info() - get information about EFI variables
+ *
+ * This function implements the QueryVariableInfo() runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @attributes: bitmask to select variables to be
+ * queried
+ * @maximum_variable_storage_size: maximum size of storage area for the
+ * selected variable types
+ * @remaining_variable_storage_size: remaining size of storage are for the
+ * selected variable types
+ * @maximum_variable_size: maximum size of a variable of the
+ * selected type
+ * Return: status code
+ */
+efi_status_t EFIAPI __efi_runtime
+efi_query_variable_info_runtime(u32 attributes, u64 *max_variable_storage_size,
+ u64 *remain_variable_storage_size,
+ u64 *max_variable_size)
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * efi_set_variable_runtime() - runtime implementation of SetVariable()
+ *
+ * @variable_name: name of the variable
+ * @guid: 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
+ */
+static efi_status_t __efi_runtime EFIAPI
+efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
+ u32 attributes, efi_uintn_t data_size,
+ const void *data)
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ * efi_variables_boot_exit_notify() - notify ExitBootServices() is called
+ */
+void efi_variables_boot_exit_notify(void)
+{
+ u8 *comm_buf;
+ efi_status_t ret;
+
+ comm_buf = setup_mm_hdr(NULL, 0,
+ SMM_VARIABLE_FUNCTION_EXIT_BOOT_SERVICE, &ret);
+ if (comm_buf)
+ ret = mm_communicate(comm_buf, 0);
+ else
+ ret = EFI_NOT_FOUND;
+
+ if (ret != EFI_SUCCESS)
+ log_err("Unable to notify StMM for ExitBootServices\n");
+ free(comm_buf);
+
+ /* Update runtime service table */
+ efi_runtime_services.query_variable_info =
+ efi_query_variable_info_runtime;
+ efi_runtime_services.get_variable = efi_get_variable_runtime;
+ efi_runtime_services.get_next_variable_name =
+ efi_get_next_variable_name_runtime;
+ efi_runtime_services.set_variable = efi_set_variable_runtime;
+ efi_update_table_header_crc32(&efi_runtime_services.hdr);
+}
+
+/**
+ * efi_init_variables() - initialize variable services
+ *
+ * Return: status code
+ */
+efi_status_t efi_init_variables(void)
+{
+ efi_status_t ret;
+
+ ret = get_max_payload(&max_payload_size);
+ if (ret != EFI_SUCCESS)
+ return ret;
+
+ max_buffer_size = MM_COMMUNICATE_HEADER_SIZE +
+ MM_VARIABLE_COMMUNICATE_SIZE +
+ max_payload_size;
+
+ return EFI_SUCCESS;
+}
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage
2020-05-11 18:13 [PATCH 0/6 v2] EFI variable support via OP-TEE Ilias Apalodimas
2020-05-11 18:13 ` [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication Ilias Apalodimas
2020-05-11 18:14 ` [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE Ilias Apalodimas
@ 2020-05-11 18:14 ` Ilias Apalodimas
2020-05-11 18:54 ` Heinrich Schuchardt
2020-05-11 18:14 ` [PATCH 4/5 v2] MAINTAINERS: Add maintainer for EFI variables via OP-TEE Ilias Apalodimas
2020-05-11 18:14 ` [PATCH 5/5 v2] doc: uefi.rst: Add OP-TEE variable storage config options Ilias Apalodimas
4 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-11 18:14 UTC (permalink / raw)
To: u-boot
With the previous patches that use OP-TEE and StandAloneMM for UEFI
variable storage we've added functionality for efi_query_variable_info.
So let's add the relevant command to efidebug and retrieve information
about the container used to store UEFI variables
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
cmd/efidebug.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index d8a76d78a388..a3980772c934 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1160,6 +1160,44 @@ static int do_efi_test(cmd_tbl_t *cmdtp, int flag,
return cp->cmd(cmdtp, flag, argc, argv);
}
+/**
+ * do_efi_query_info() - QueryVariableInfo EFI service
+ *
+ * @cmdtp: Command table
+ * @flag: Command flag
+ * @argc: Number of arguments
+ * @argv: Argument array
+ * Return: CMD_RET_SUCCESS on success,
+ * CMD_RET_USAGE or CMD_RET_FAILURE on failure
+ *
+ * Implement efidebug "test" sub-command.
+ */
+
+static int do_efi_query_info(cmd_tbl_t *cmdtp, int flag,
+ int argc, char * const argv[])
+{
+ efi_status_t ret;
+ u32 attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS |
+ EFI_VARIABLE_NON_VOLATILE;
+ u64 max_variable_storage_size;
+ u64 remain_variable_storage_size;
+ u64 max_variable_size;
+
+ ret = EFI_CALL(efi_query_variable_info(attr,
+ &max_variable_storage_size,
+ &remain_variable_storage_size,
+ &max_variable_size));
+ if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
+ printf("Max storage size %llu\n", max_variable_storage_size);
+ printf("Remaining storage size %llu\n", remain_variable_storage_size);
+ printf("Max variable size %llu\n", max_variable_size);
+
+ return CMD_RET_SUCCESS;
+}
+
static cmd_tbl_t cmd_efidebug_sub[] = {
U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
@@ -1176,6 +1214,8 @@ static cmd_tbl_t cmd_efidebug_sub[] = {
"", ""),
U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_efi_test,
"", ""),
+ U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
+ "", ""),
};
/**
@@ -1247,7 +1287,9 @@ static char efidebug_help_text[] =
"efidebug tables\n"
" - show UEFI configuration tables\n"
"efidebug test bootmgr\n"
- " - run simple bootmgr for test\n";
+ " - run simple bootmgr for test\n"
+ "efidebug query\n"
+ " - show size of UEFI variables store\n";
#endif
U_BOOT_CMD(
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5 v2] MAINTAINERS: Add maintainer for EFI variables via OP-TEE
2020-05-11 18:13 [PATCH 0/6 v2] EFI variable support via OP-TEE Ilias Apalodimas
` (2 preceding siblings ...)
2020-05-11 18:14 ` [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage Ilias Apalodimas
@ 2020-05-11 18:14 ` Ilias Apalodimas
2020-05-11 18:39 ` Heinrich Schuchardt
2020-05-11 18:14 ` [PATCH 5/5 v2] doc: uefi.rst: Add OP-TEE variable storage config options Ilias Apalodimas
4 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-11 18:14 UTC (permalink / raw)
To: u-boot
Add myself as maintainer for the OP-TEE related UEFI variable storage
and add the headers file on the existing EFI list
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ec59ce8b8802..0c38890be09c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -635,6 +635,12 @@ F: cmd/efidebug.c
F: cmd/nvedit_efi.c
F: tools/file2include.c
+EFI VARIABLES VIA OP-TEE
+M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
+S: Maintained
+F: lib/efi_loader/efi_variable_tee.c
+F: include/mm_communication.h
+
ENVIRONMENT
M: Joe Hershberger <joe.hershberger@ni.com>
R: Wolfgang Denk <wd@denx.de>
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5 v2] doc: uefi.rst: Add OP-TEE variable storage config options
2020-05-11 18:13 [PATCH 0/6 v2] EFI variable support via OP-TEE Ilias Apalodimas
` (3 preceding siblings ...)
2020-05-11 18:14 ` [PATCH 4/5 v2] MAINTAINERS: Add maintainer for EFI variables via OP-TEE Ilias Apalodimas
@ 2020-05-11 18:14 ` Ilias Apalodimas
2020-05-11 18:38 ` Heinrich Schuchardt
4 siblings, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-11 18:14 UTC (permalink / raw)
To: u-boot
If OP-TEE is compiled with an EDK2 application running in secure world
it can process and store UEFI variables in an RPMB.
Add documentation for the config options enabling this
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
doc/uefi/uefi.rst | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index 4fda00d68721..03d6fd0c6aa8 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -188,6 +188,23 @@ on the sandbox
cd <U-Boot source directory>
pytest.py test/py/tests/test_efi_secboot/test_signed.py --bd sandbox
+Using OP-TEE for EFI variables
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Instead of implementing UEFI variable services inside U-Boot they can
+also be provided in the secure world by a module for OP-TEE[1]. The
+interface between U-Boot and OP-TEE for variable services is enabled by
+CONFIG_EFI_MM_COMM_TEE=y.
+
+Tianocore EDK II's standalone management mode driver for variables can
+be linked to OP-TEE for this purpose. This module uses the Replay
+Protected Memory Block (RPMB) of an eMMC device for persisting
+non-volatile variables. When calling the variable services via the
+OP-TEE API U-Boot's OP-TEE supplicant relays calls to the RPMB driver
+which has to be enabled via CONFIG_SUPPORT_EMMC_RPMB=y.
+
+[1] https://optee.readthedocs.io/ - OP-TEE documentation
+
Executing the boot manager
~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5 v2] doc: uefi.rst: Add OP-TEE variable storage config options
2020-05-11 18:14 ` [PATCH 5/5 v2] doc: uefi.rst: Add OP-TEE variable storage config options Ilias Apalodimas
@ 2020-05-11 18:38 ` Heinrich Schuchardt
0 siblings, 0 replies; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-05-11 18:38 UTC (permalink / raw)
To: u-boot
On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> If OP-TEE is compiled with an EDK2 application running in secure world
> it can process and store UEFI variables in an RPMB.
> Add documentation for the config options enabling this
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> doc/uefi/uefi.rst | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
> index 4fda00d68721..03d6fd0c6aa8 100644
> --- a/doc/uefi/uefi.rst
> +++ b/doc/uefi/uefi.rst
> @@ -188,6 +188,23 @@ on the sandbox
> cd <U-Boot source directory>
> pytest.py test/py/tests/test_efi_secboot/test_signed.py --bd sandbox
>
> +Using OP-TEE for EFI variables
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Instead of implementing UEFI variable services inside U-Boot they can
> +also be provided in the secure world by a module for OP-TEE[1]. The
> +interface between U-Boot and OP-TEE for variable services is enabled by
> +CONFIG_EFI_MM_COMM_TEE=y.
> +
> +Tianocore EDK II's standalone management mode driver for variables can
> +be linked to OP-TEE for this purpose. This module uses the Replay
> +Protected Memory Block (RPMB) of an eMMC device for persisting
> +non-volatile variables. When calling the variable services via the
> +OP-TEE API U-Boot's OP-TEE supplicant relays calls to the RPMB driver
> +which has to be enabled via CONFIG_SUPPORT_EMMC_RPMB=y.
> +
> +[1] https://optee.readthedocs.io/ - OP-TEE documentation
> +
> Executing the boot manager
> ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/5 v2] MAINTAINERS: Add maintainer for EFI variables via OP-TEE
2020-05-11 18:14 ` [PATCH 4/5 v2] MAINTAINERS: Add maintainer for EFI variables via OP-TEE Ilias Apalodimas
@ 2020-05-11 18:39 ` Heinrich Schuchardt
0 siblings, 0 replies; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-05-11 18:39 UTC (permalink / raw)
To: u-boot
On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> Add myself as maintainer for the OP-TEE related UEFI variable storage
> and add the headers file on the existing EFI list
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> MAINTAINERS | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ec59ce8b8802..0c38890be09c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -635,6 +635,12 @@ F: cmd/efidebug.c
> F: cmd/nvedit_efi.c
> F: tools/file2include.c
>
> +EFI VARIABLES VIA OP-TEE
> +M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> +S: Maintained
> +F: lib/efi_loader/efi_variable_tee.c
> +F: include/mm_communication.h
> +
> ENVIRONMENT
> M: Joe Hershberger <joe.hershberger@ni.com>
> R: Wolfgang Denk <wd@denx.de>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage
2020-05-11 18:14 ` [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage Ilias Apalodimas
@ 2020-05-11 18:54 ` Heinrich Schuchardt
2020-05-12 4:02 ` Ilias Apalodimas
0 siblings, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-05-11 18:54 UTC (permalink / raw)
To: u-boot
On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> With the previous patches that use OP-TEE and StandAloneMM for UEFI
> variable storage we've added functionality for efi_query_variable_info.
> So let's add the relevant command to efidebug and retrieve information
> about the container used to store UEFI variables
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
For now attributes (e.g. EFI_VARIABLE_NON_VOLATILE) cannot be passed to
the 'efidebug query' sub-command instead a fixed value is used. We can
add attributes on the command line later.
> ---
> cmd/efidebug.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index d8a76d78a388..a3980772c934 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -1160,6 +1160,44 @@ static int do_efi_test(cmd_tbl_t *cmdtp, int flag,
> return cp->cmd(cmdtp, flag, argc, argv);
> }
>
> +/**
> + * do_efi_query_info() - QueryVariableInfo EFI service
> + *
> + * @cmdtp: Command table
> + * @flag: Command flag
> + * @argc: Number of arguments
> + * @argv: Argument array
> + * Return: CMD_RET_SUCCESS on success,
> + * CMD_RET_USAGE or CMD_RET_FAILURE on failure
> + *
> + * Implement efidebug "test" sub-command.
> + */
> +
> +static int do_efi_query_info(cmd_tbl_t *cmdtp, int flag,
> + int argc, char * const argv[])
> +{
> + efi_status_t ret;
> + u32 attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS |
> + EFI_VARIABLE_NON_VOLATILE;
As we do not support variables at runtime currently shouldn't we remove
EFI_VARIABLE_RUNTIME_ACCESS from the default value? I could do that when
merging.
Otherwise:
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> + u64 max_variable_storage_size;
> + u64 remain_variable_storage_size;
> + u64 max_variable_size;
> +
> + ret = EFI_CALL(efi_query_variable_info(attr,
> + &max_variable_storage_size,
> + &remain_variable_storage_size,
> + &max_variable_size));
> + if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> + printf("Max storage size %llu\n", max_variable_storage_size);
> + printf("Remaining storage size %llu\n", remain_variable_storage_size);
> + printf("Max variable size %llu\n", max_variable_size);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> static cmd_tbl_t cmd_efidebug_sub[] = {
> U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
> U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
> @@ -1176,6 +1214,8 @@ static cmd_tbl_t cmd_efidebug_sub[] = {
> "", ""),
> U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_efi_test,
> "", ""),
> + U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
> + "", ""),
> };
>
> /**
> @@ -1247,7 +1287,9 @@ static char efidebug_help_text[] =
> "efidebug tables\n"
> " - show UEFI configuration tables\n"
> "efidebug test bootmgr\n"
> - " - run simple bootmgr for test\n";
> + " - run simple bootmgr for test\n"
> + "efidebug query\n"
> + " - show size of UEFI variables store\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication
2020-05-11 18:13 ` [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication Ilias Apalodimas
@ 2020-05-11 19:39 ` Heinrich Schuchardt
2020-05-12 4:15 ` Ilias Apalodimas
2020-05-12 4:34 ` Ilias Apalodimas
0 siblings, 2 replies; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-05-11 19:39 UTC (permalink / raw)
To: u-boot
On 5/11/20 8:13 PM, Ilias Apalodimas wrote:
> From: Sughosh Ganu <sughosh.ganu@linaro.org>
>
> In Arm devices OP-TEE has the ability to run StandAloneMM (from EDK2)
> in a separate partition and handle UEFI variables.
> A following patch introduces this functionality.
>
> Add the headers needed for OP-TEE <--> StandAloneMM communication
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> include/mm_communication.h | 207 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 207 insertions(+)
> create mode 100644 include/mm_communication.h
>
> diff --git a/include/mm_communication.h b/include/mm_communication.h
> new file mode 100644
> index 000000000000..b9bfbe4cf0a1
> --- /dev/null
> +++ b/include/mm_communication.h
> @@ -0,0 +1,207 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Headers for EFI variable service via StandAloneMM, EDK2 application running
> + * in OP-TEE
> + *
> + * Copyright (c) 2017, Intel Corporation. All rights reserved.
> + * Copyright (C) 2020 Linaro Ltd. <sughosh.ganu@linaro.org>
> + * Copyright (C) 2020 Linaro Ltd. <ilias.apalodimas@linaro.org>
> + */
> +
> +#ifndef _MM_VARIABLE_H_
> +#define _MM_VARIABLE_H_
> +
> +#include <part_efi.h>
> +
> +/*
> + * Interface to the pseudo TA, which provides a communication channel with
U-Boot developers might not know the OP-TEE terms. So I would tend to
avoid abbreviations at least in the first reference.
%s/pseudo TA/Pseudo Trusted Application/
> + * the StandaloneMM Secure Partition (StMM) running at S-EL0
What does MM stand for? Management Mode?
> + */
> +
> +#define PTA_STMM_CMDID_COMMUNICATE 0
> +
> +/* OP-TEE is using big endian GUIDs while UEFI uses little endian ones */
> +#define PTA_STMM_UUID { 0xed32d533, 0x99e6, 0x4209, {\
> + 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7 } }
> +
> +#define EFI_MM_VARIABLE_GUID \
> + EFI_GUID(0xed32d533, 0x99e6, 0x4209, \
> + 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7)
> +
> +/* Defined in EDK2 MdePkg/Include/Protocol/MmCommunication.h */
> +
> +/**
> + * struct efi_mm_communicate_header - Header used for SMM variable communication
> +
> + * @header_guid: header use for disambiguation of content
> + * @message_len: length of the message. Does not include the size of the
> + * header
> + * @data: payload of the message
> + *
> + * Defined in EDK2 as EFI_MM_COMMUNICATE_HEADER
> + * To avoid confusion in interpreting frames, the communication buffer should
> + * always begin with efi_mm_communicate_header
%s/efi_mm_communicate_header/efi_mm_communicate_header./
> + */
> +struct efi_mm_communicate_header {
> + efi_guid_t header_guid;
> + size_t message_len;
> + u8 data[];
> +};
> +
> +#define MM_COMMUNICATE_HEADER_SIZE \
> + (sizeof(struct efi_mm_communicate_header))
> +
> +/* Defined in EDK2 ArmPkg/Include/IndustryStandard/ArmStdSmc.h */
> +
> +/* MM return error codes */
> +#define ARM_SMC_MM_RET_SUCCESS 0
> +#define ARM_SMC_MM_RET_NOT_SUPPORTED -1
> +#define ARM_SMC_MM_RET_INVALID_PARAMS -2
> +#define ARM_SMC_MM_RET_DENIED -3
> +#define ARM_SMC_MM_RET_NO_MEMORY -4
> +
> +/* Defined in EDK2 MdeModulePkg/Include/Guid/SmmVariableCommon.h */
> +
> +#define SMM_VARIABLE_FUNCTION_GET_VARIABLE 1
> +/*
> + * The payload for this function is
> + * SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME.
> + */
> +#define SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME 2
> +/*
> + * The payload for this function is SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE.
> + */
> +#define SMM_VARIABLE_FUNCTION_SET_VARIABLE 3
> +/*
> + * The payload for this function is
> + * SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO.
> + */
> +#define SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO 4
> +/*
> + * It is a notify event, no extra payload for this function.
> + */
> +#define SMM_VARIABLE_FUNCTION_READY_TO_BOOT 5
> +/*
> + * It is a notify event, no extra payload for this function.
> + */
> +#define SMM_VARIABLE_FUNCTION_EXIT_BOOT_SERVICE 6
> +/*
> + * The payload for this function is VARIABLE_INFO_ENTRY.
> + * The GUID in EFI_SMM_COMMUNICATE_HEADER is gEfiSmmVariableProtocolGuid.
> + */
> +#define SMM_VARIABLE_FUNCTION_GET_STATISTICS 7
> +/*
> + * The payload for this function is SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE
> + */
> +#define SMM_VARIABLE_FUNCTION_LOCK_VARIABLE 8
> +
> +#define SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_SET 9
> +
> +#define SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET 10
> +
> +#define SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE 11
> +/*
> + * The payload for this function is
> + * SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> + */
> +#define SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT 12
> +
> +#define SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE 13
> +/*
> + * The payload for this function is
> + * SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO
> + */
> +#define SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO 14
> +
> +/**
> + * struct smm_variable_communicate_header - Used for SMM variable communication
> +
> + * @function: function to call in Smm.
> + * @ret_status: return status
> + * @data: payload
> + *
> + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_HEADER
> + */
> +struct smm_variable_communicate_header {
> + efi_uintn_t function;
> + efi_status_t ret_status;
> + u8 data[];
> +};
> +
> +#define MM_VARIABLE_COMMUNICATE_SIZE \
> + (sizeof(struct smm_variable_communicate_header))
> +
> +/**
> + * struct smm_variable_access - Used to communicate with StMM by
> + * SetVariable and GetVariable.
> +
> + * @function: vendor GUID to call in Smm
> + * @data_size: size of EFI variable data
> + * @name_size: size of EFI name
> + * @attr: attributes
> + * @name: variable name
> + *
> + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE
> + *
> + */
> +struct smm_variable_access {
> + efi_guid_t guid;
> + efi_uintn_t data_size;
> + efi_uintn_t name_size;
> + u32 attr;
> + u16 name[];
> +};
> +
> +#define MM_VARIABLE_ACCESS_HEADER_SIZE \
> + (sizeof(struct smm_variable_access))
> +/**
> + * struct smm_variable_payload_size - Used to get the max allowed
> + * payload used in StMM.
> + *
> + * @size: size to fill in
> + *
> + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
%s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE./
> + *
> + */
> +struct smm_variable_payload_size {
> + efi_uintn_t size;
> +};
> +
> +/**
> + * struct smm_variable_getnext - Used to communicate with StMM for
> + * GetNextVariableName.
> + *
> + * @size: vendor GUID
> + * @name_size: size of the name of the variable
> + * @name: variable name
> + *
> + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
%s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME./
Too much copy and paste ;)
> + */
> +struct smm_variable_getnext {
> + efi_guid_t guid;
> + efi_uintn_t name_size;
> + u16 name[];
> +};
> +
> +#define MM_VARIABLE_GET_NEXT_HEADER_SIZE \
> + (sizeof(struct smm_variable_getnext))
> +
> +/**
> + * struct smm_variable_query_info - Used to communicate with StMM for
> + * QueryVariableInfo.
> + *
> + * @max_variable_storage: max available storage
> + * @remaining_variable_storage: remaining available storage
> + * @max_variable_size: max variable supported size
> + * @attr: attributes to query storage for
> + *
> + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
%s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO./
I hope I caught all comment errors. Please, recheck.
Otherwise:
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +struct smm_variable_query_info {
> + u64 max_variable_storage;
> + u64 remaining_variable_storage;
> + u64 max_variable_size;
> + u32 attr;
> +};
> +
> +#endif /* _MM_VARIABLE_H_ */
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage
2020-05-11 18:54 ` Heinrich Schuchardt
@ 2020-05-12 4:02 ` Ilias Apalodimas
0 siblings, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-12 4:02 UTC (permalink / raw)
To: u-boot
On Mon, May 11, 2020 at 08:54:04PM +0200, Heinrich Schuchardt wrote:
> On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> > With the previous patches that use OP-TEE and StandAloneMM for UEFI
> > variable storage we've added functionality for efi_query_variable_info.
> > So let's add the relevant command to efidebug and retrieve information
> > about the container used to store UEFI variables
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> For now attributes (e.g. EFI_VARIABLE_NON_VOLATILE) cannot be passed to
> the 'efidebug query' sub-command instead a fixed value is used. We can
> add attributes on the command line later.
Good point, I'll add it on the commit message for v3
>
> > ---
> > cmd/efidebug.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> > index d8a76d78a388..a3980772c934 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -1160,6 +1160,44 @@ static int do_efi_test(cmd_tbl_t *cmdtp, int flag,
> > return cp->cmd(cmdtp, flag, argc, argv);
> > }
> >
> > +/**
> > + * do_efi_query_info() - QueryVariableInfo EFI service
> > + *
> > + * @cmdtp: Command table
> > + * @flag: Command flag
> > + * @argc: Number of arguments
> > + * @argv: Argument array
> > + * Return: CMD_RET_SUCCESS on success,
> > + * CMD_RET_USAGE or CMD_RET_FAILURE on failure
> > + *
> > + * Implement efidebug "test" sub-command.
> > + */
> > +
> > +static int do_efi_query_info(cmd_tbl_t *cmdtp, int flag,
> > + int argc, char * const argv[])
> > +{
> > + efi_status_t ret;
> > + u32 attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS |
> > + EFI_VARIABLE_NON_VOLATILE;
>
> As we do not support variables at runtime currently shouldn't we remove
> EFI_VARIABLE_RUNTIME_ACCESS from the default value? I could do that when
> merging.
We can still add variables marked as runtime in U-Boot's cmd line although we
don't support those in Linux. So I'd keep that flag on the defaults.
>
> Otherwise:
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
Regards
/Ilias
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication
2020-05-11 19:39 ` Heinrich Schuchardt
@ 2020-05-12 4:15 ` Ilias Apalodimas
2020-05-12 4:34 ` Ilias Apalodimas
1 sibling, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-12 4:15 UTC (permalink / raw)
To: u-boot
Hi Heinrich
On Mon, May 11, 2020 at 09:39:51PM +0200, Heinrich Schuchardt wrote:
> On 5/11/20 8:13 PM, Ilias Apalodimas wrote:
> > +
[...]
> > +/*
> > + * Interface to the pseudo TA, which provides a communication channel with
>
> U-Boot developers might not know the OP-TEE terms. So I would tend to
> avoid abbreviations at least in the first reference.
>
> %s/pseudo TA/Pseudo Trusted Application/
>
> > + * the StandaloneMM Secure Partition (StMM) running at S-EL0
>
> What does MM stand for? Management Mode?
>
Yes
> > + */
> > +
> > +#define PTA_STMM_CMDID_COMMUNICATE 0
> > +
> > + 0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7 } }
[...]
> > +
> > +#define EFI_MM_VARIABLE_GUID \
> > + *
> > + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
>
> %s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME./
>
> Too much copy and paste ;)
>
Indeed! Thanks for cathcing those
> > + */
> > +struct smm_variable_getnext {
> > + efi_guid_t guid;
> > + efi_uintn_t name_size;
> > + u16 name[];
> > +};
> > +
> > +#define MM_VARIABLE_GET_NEXT_HEADER_SIZE \
> > + (sizeof(struct smm_variable_getnext))
> > +
> > +/**
> > + * struct smm_variable_query_info - Used to communicate with StMM for
> > + * QueryVariableInfo.
> > + *
> > + * @max_variable_storage: max available storage
> > + * @remaining_variable_storage: remaining available storage
> > + * @max_variable_size: max variable supported size
> > + * @attr: attributes to query storage for
> > + *
> > + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
>
> %s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO./
>
> I hope I caught all comment errors. Please, recheck.
Ok will do
>
> Otherwise:
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
Thanks!
/Ilias
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication
2020-05-11 19:39 ` Heinrich Schuchardt
2020-05-12 4:15 ` Ilias Apalodimas
@ 2020-05-12 4:34 ` Ilias Apalodimas
1 sibling, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-12 4:34 UTC (permalink / raw)
To: u-boot
>
> %s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE./
>
> > + * @size: vendor GUID
[...]
> > + * @name_size: size of the name of the variable
> > + * @name: variable name
> > + *
> > + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
>
> %s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME./
>
> Too much copy and paste ;)
>
> > + */
> > + * @attr: attributes to query storage for
[...]
> > + *
> > + * Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
>
> %s/SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE/SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO./
>
> I hope I caught all comment errors. Please, recheck.
>
There were 3-4 more c/p trainwrecks in there. I'll post a v3 once you are done
with your testing.
Thanks
/Ilias
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE
2020-05-11 18:14 ` [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE Ilias Apalodimas
@ 2020-05-13 6:14 ` Heinrich Schuchardt
2020-05-13 8:10 ` Ilias Apalodimas
2020-05-15 11:55 ` Heinrich Schuchardt
1 sibling, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-05-13 6:14 UTC (permalink / raw)
To: u-boot
On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
> StandAloneMM is responsible for the UEFI variable support. In
> combination with OP-TEE and it's U-Boot supplicant, variables are
> authenticated/validated in secure world and stored on an RPMB partition.
>
> So let's add a new config option in U-Boot implementing the necessary
> calls to OP-TEE for the variable management.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Pipat Methavanitpong <pipat1010@gmail.com>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> lib/efi_loader/Kconfig | 9 +
> lib/efi_loader/Makefile | 4 +
> lib/efi_loader/efi_variable_tee.c | 643 ++++++++++++++++++++++++++++++
> 3 files changed, 656 insertions(+)
> create mode 100644 lib/efi_loader/efi_variable_tee.c
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 1cfa24ffcf72..e385cd0b9dae 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -164,4 +164,13 @@ config EFI_SECURE_BOOT
> it is signed with a trusted key. To do that, you need to install,
> at least, PK, KEK and db.
>
> +config EFI_MM_COMM_TEE
> + bool "UEFI variables storage service via OP-TEE"
> + depends on SUPPORT_EMMC_RPMB
> + default n
> + help
> + If OP-TEE is present and running StandAloneMM dispatch all UEFI variable
> + related operations to that. The application will verify, authenticate and
> + store the variables on an RPMB
> +
> endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index eff3c25ec301..dba652121eab 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -34,7 +34,11 @@ 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
> +ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
> +obj-y += efi_variable_tee.o
> +else
> obj-y += efi_variable.o
> +endif
> obj-y += efi_watchdog.o
> obj-$(CONFIG_LCD) += efi_gop.o
> obj-$(CONFIG_DM_VIDEO) += efi_gop.o
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> new file mode 100644
> index 000000000000..f8857bb81e72
> --- /dev/null
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -0,0 +1,643 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * EFI variable service via OP-TEE
> + *
> + * Copyright (C) 2019 Linaro Ltd. <sughosh.ganu@linaro.org>
> + * Copyright (C) 2019 Linaro Ltd. <ilias.apalodimas@linaro.org>
> + */
> +
> +#include <common.h>
> +#include <efi.h>
> +#include <efi_api.h>
> +#include <efi_loader.h>
> +#include <tee.h>
> +#include <malloc.h>
> +#include <mm_communication.h>
> +
> +static efi_uintn_t max_buffer_size; /* comm + var + func + data */
> +static efi_uintn_t max_payload_size; /* func + data */
> +
> +struct mm_connection {
> + struct udevice *tee;
> + u32 session;
> +};
> +
> +/**
> + * get_connection() - Retrieve OP-TEE session for a specific UUID.
> + *
> + * @conn: session buffer to fill
> + * Return: status code
> + */
> +static int get_connection(struct mm_connection *conn)
> +{
> + static const struct tee_optee_ta_uuid uuid = PTA_STMM_UUID;
> + struct udevice *tee = NULL;
> + struct tee_open_session_arg arg;
> + int rc;
> +
> + tee = tee_find_device(tee, NULL, NULL, NULL);
> + if (!tee)
> + return -ENODEV;
> +
> + memset(&arg, 0, sizeof(arg));
> + tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
> + rc = tee_open_session(tee, &arg, 0, NULL);
> + if (!rc) {
> + conn->tee = tee;
> + conn->session = arg.session;
> + }
> +
> + return rc;
> +}
> +
> +/**
> + * optee_mm_communicate() - Pass a buffer to StandaloneMM running in OP-TEE
> + *
> + * @comm_buf: locally allocted communcation buffer
> + * @dsize: buffer size
> + * Return: status code
> + */
> +static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
> +{
> + ulong buf_size;
> + efi_status_t ret;
> + struct efi_mm_communicate_header *mm_hdr;
> + struct mm_connection conn = { NULL, 0 };
> + struct tee_invoke_arg arg;
> + struct tee_param param[2];
> + struct tee_shm *shm = NULL;
> + int rc;
> +
> + if (!comm_buf)
> + return EFI_INVALID_PARAMETER;
> +
> + mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
> + buf_size = mm_hdr->message_len + sizeof(efi_guid_t) + sizeof(size_t);
> +
> + if (dsize != buf_size)
> + return EFI_INVALID_PARAMETER;
> +
> + rc = get_connection(&conn);
> + if (rc) {
> + log_err("Unable to open OP-TEE session (err=%d)\n", rc);
> + return EFI_UNSUPPORTED;
> + }
> +
> + if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {
> + log_err("Unable to register shared memory\n");
> + return EFI_UNSUPPORTED;
> + }
> +
> + memset(&arg, 0, sizeof(arg));
> + arg.func = PTA_STMM_CMDID_COMMUNICATE;
> + arg.session = conn.session;
> +
> + memset(param, 0, sizeof(param));
> + param[0].attr = TEE_PARAM_ATTR_TYPE_MEMREF_INOUT;
> + param[0].u.memref.size = buf_size;
> + param[0].u.memref.shm = shm;
> + param[1].attr = TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> +
> + rc = tee_invoke_func(conn.tee, &arg, 2, param);
> + if (rc)
> + return EFI_INVALID_PARAMETER;
> + tee_shm_free(shm);
> + tee_close_session(conn.tee, conn.session);
> +
> + switch (param[1].u.value.a) {
> + case ARM_SMC_MM_RET_SUCCESS:
> + ret = EFI_SUCCESS;
> + break;
> +
> + case ARM_SMC_MM_RET_INVALID_PARAMS:
> + ret = EFI_INVALID_PARAMETER;
> + break;
> +
> + case ARM_SMC_MM_RET_DENIED:
> + ret = EFI_ACCESS_DENIED;
> + break;
> +
> + case ARM_SMC_MM_RET_NO_MEMORY:
> + ret = EFI_OUT_OF_RESOURCES;
> + break;
> +
> + default:
> + ret = EFI_ACCESS_DENIED;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * mm_communicate() - Adjust the cmonnucation buffer to StandAlonneMM and send
> + * it to OP-TEE
> + *
> + * @comm_buf: locally allocted communcation buffer
> + * @dsize: buffer size
> + * Return: status code
> + */
> +static efi_status_t mm_communicate(u8 *comm_buf, efi_uintn_t dsize)
> +{
> + efi_status_t ret;
> + struct efi_mm_communicate_header *mm_hdr;
> + struct smm_variable_communicate_header *var_hdr;
> +
> + dsize += MM_COMMUNICATE_HEADER_SIZE + MM_VARIABLE_COMMUNICATE_SIZE;
> + mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
> + var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data;
> +
> + ret = optee_mm_communicate(comm_buf, dsize);
> + if (ret != EFI_SUCCESS) {
> + log_err("%s failed!\n", __func__);
> + return ret;
> + }
> +
> + return var_hdr->ret_status;
> +}
> +
> +/**
> + * setup_mm_hdr() - Allocate a buffer for StandAloneMM and initialize the
> + * header data.
> + *
> + * @dptr: pointer address of the corresponding StandAloneMM
> + * function
> + * @payload_size: buffer size
> + * @func: standAloneMM function number
> + * @ret: EFI return code
> + * Return: buffer or NULL
> + */
> +static u8 *setup_mm_hdr(void **dptr, efi_uintn_t payload_size,
> + efi_uintn_t func, efi_status_t *ret)
> +{
> + const efi_guid_t mm_var_guid = EFI_MM_VARIABLE_GUID;
> + struct efi_mm_communicate_header *mm_hdr;
> + struct smm_variable_communicate_header *var_hdr;
> + u8 *comm_buf;
> +
> + /* In the init function we initialize max_buffer_size with
> + * get_max_payload(). So skip the test if max_buffer_size is initialized
> + * StandAloneMM will perform similar checks and drop the buffer if it's
> + * too long
> + */
> + if (max_buffer_size && max_buffer_size <
> + (MM_COMMUNICATE_HEADER_SIZE +
> + MM_VARIABLE_COMMUNICATE_SIZE +
> + payload_size)) {
> + *ret = EFI_INVALID_PARAMETER;
> + return NULL;
> + }
> +
> + comm_buf = calloc(1, MM_COMMUNICATE_HEADER_SIZE +
> + MM_VARIABLE_COMMUNICATE_SIZE +
> + payload_size);
> + if (!comm_buf) {
> + *ret = EFI_OUT_OF_RESOURCES;
> + return NULL;
> + }
> +
> + mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
> + guidcpy(&mm_hdr->header_guid, &mm_var_guid);
> + mm_hdr->message_len = MM_VARIABLE_COMMUNICATE_SIZE + payload_size;
> +
> + var_hdr = (struct smm_variable_communicate_header *)mm_hdr->data;
> + var_hdr->function = func;
> + if (dptr)
> + *dptr = var_hdr->data;
> + *ret = EFI_SUCCESS;
> +
> + return comm_buf;
> +}
> +
> +/**
> + * get_max_payload() - Get variable payload size from StandAloneMM.
> + *
> + * @size: size of the variable in storage
> + * Return: status code
> + */
> +efi_status_t EFIAPI get_max_payload(efi_uintn_t *size)
> +{
> + struct smm_variable_payload_size *var_payload = NULL;
> + efi_uintn_t payload_size;
> + efi_status_t ret;
> + u8 *comm_buf;
> +
> + if (!size)
> + return EFI_INVALID_PARAMETER;
> +
> + payload_size = sizeof(*var_payload);
> + comm_buf = setup_mm_hdr((void **)&var_payload, payload_size,
> + SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE, &ret);
> + if (!comm_buf)
> + return EFI_EXIT(ret);
> +
> + ret = mm_communicate(comm_buf, payload_size);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + *size = var_payload->size;
> +
> +out:
> + free(comm_buf);
> + 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)
> +{
> + struct smm_variable_access *var_acc;
> + efi_uintn_t payload_size;
> + efi_uintn_t name_size;
> + efi_uintn_t tmp_dsize;
> + efi_status_t ret;
> + u8 *comm_buf;
> +
> + EFI_ENTRY("\"%ls\" %pUl %p %p %p", name, guid, attr, data_size, data);
> +
> + if (!name || !guid || !data_size)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + /* Check payload size */
> + name_size = u16_strsize(name);
> + if (name_size > max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + /* Trim output buffer size */
> + tmp_dsize = *data_size;
> + if (name_size + tmp_dsize >
> + max_payload_size - MM_VARIABLE_ACCESS_HEADER_SIZE) {
> + tmp_dsize = max_payload_size -
> + MM_VARIABLE_ACCESS_HEADER_SIZE -
> + name_size;
> + }
> +
> + /* Get communication buffer and initialize header */
> + payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + tmp_dsize;
> + comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
> + SMM_VARIABLE_FUNCTION_GET_VARIABLE, &ret);
> + if (!comm_buf)
> + return EFI_EXIT(ret);
> +
> + /* Fill in contents */
> + guidcpy(&var_acc->guid, guid);
> + 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);
> +
> + /* Communicate */
> + ret = mm_communicate(comm_buf, payload_size);
> + if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
> + /* Update with reported data size for trimmed case */
> + *data_size = var_acc->data_size;
> + }
> + if (ret != EFI_SUCCESS)
> + goto done;
> +
> + if (attr)
> + *attr = var_acc->attr;
> + if (data)
> + memcpy(data, (u8 *)var_acc->name + var_acc->name_size,
> + var_acc->data_size);
> + else
> + ret = EFI_INVALID_PARAMETER;
> +
> +done:
> + free(comm_buf);
> + return EFI_EXIT(ret);
> +}
> +
> +/**
> + * efi_get_next_variable_name() - enumerate the current variable names
> + *
> + * @variable_name_size: size of variable_name buffer in byte
> + * @variable_name: name of uefi variable's name in u16
> + * @guid: vendor's guid
> + *
> + * This function implements the GetNextVariableName service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * Return: status code
> + */
> +efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
> + u16 *variable_name,
> + efi_guid_t *guid)
> +{
> + struct smm_variable_getnext *var_getnext;
> + efi_uintn_t payload_size;
> + efi_uintn_t tmp_dsize;
> + efi_uintn_t name_size;
> + efi_status_t ret;
> + efi_uintn_t out_name_size;
> + efi_uintn_t in_name_size;
> + u8 *comm_buf;
> +
> + EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, guid);
> +
> + if (!variable_name_size || !variable_name || !guid)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + out_name_size = *variable_name_size;
> + in_name_size = u16_strsize(variable_name);
The UEFI spec requires that EFI_INVALID_PARAMETER should be returned if
there is no '\0' character in the first *variable_name_size words of
variable_name. I think we should add this test here instead of using
max(out_name_size, in_name_size) later in the code.
You are currently calling EFI_EXIT() in many places. Depending on the
level of code optimizations done by the compiler and the debug settings
this may need to unnecessary code size. I suggest to use a single exit
point in each of the functions, e.g.
if (out_name_size > in_name_size) {
ret = EFI_INVALID_PARAMETER;
goto out;
}
...
out:
EFI_EXIT(ret);
}
Best regards
Heinrich
> +
> + name_size = u16_strsize(variable_name);
> + if (name_size > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + /* Trim output buffer size */
> + tmp_dsize = *variable_name_size;
> + if (name_size + tmp_dsize >
> + max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) {
> + tmp_dsize = max_payload_size -
> + MM_VARIABLE_GET_NEXT_HEADER_SIZE -
> + name_size;
> + }
> +
> + payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE +
> + max(out_name_size, in_name_size);
> + comm_buf = setup_mm_hdr((void **)&var_getnext, payload_size,
> + SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME,
> + &ret);
> + if (!comm_buf)
> + return EFI_EXIT(ret);
> +
> + /* Fill in contents */
> + guidcpy(&var_getnext->guid, guid);
> + var_getnext->name_size = out_name_size;
> + memcpy(var_getnext->name, variable_name, in_name_size);
> + if (out_name_size > in_name_size) {
> + memset((u8 *)var_getnext->name + in_name_size, 0x0,
> + out_name_size - in_name_size);
> + }
> +
> + /* Communicate */
> + ret = mm_communicate(comm_buf, payload_size);
> + if (ret == EFI_SUCCESS || ret == EFI_BUFFER_TOO_SMALL) {
> + /* Update with reported data size for trimmed case */
> + *variable_name_size = var_getnext->name_size;
> + }
> + if (ret != EFI_SUCCESS)
> + goto done;
> +
> + guidcpy(guid, &var_getnext->guid);
> + memcpy(variable_name, (u8 *)var_getnext->name,
> + var_getnext->name_size);
> +
> +done:
> + free(comm_buf);
> + 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)
> +{
> + struct smm_variable_access *var_acc;
> + efi_uintn_t payload_size;
> + efi_uintn_t name_size;
> + efi_status_t ret;
> + u8 *comm_buf;
> +
> + EFI_ENTRY("\"%ls\" %pUl %x %zu %p", name, guid, attr, data_size, data);
> +
> + if (!name || name[0] == 0 || !guid)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> + if (data_size > 0 && !data)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + /* Check payload size */
> + name_size = u16_strsize(name);
> + payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + data_size;
> + if (payload_size > max_payload_size)
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> + /* Get communication buffer and initialize header */
> + comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
> + SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret);
> + if (!comm_buf)
> + return EFI_EXIT(ret);
> +
> + /* Fill in contents */
> + guidcpy(&var_acc->guid, guid);
> + var_acc->data_size = data_size;
> + var_acc->name_size = name_size;
> + var_acc->attr = attr;
> + memcpy(var_acc->name, name, name_size);
> + memcpy((u8 *)var_acc->name + name_size, data, data_size);
> +
> + /* Communicate */
> + ret = mm_communicate(comm_buf, payload_size);
> + free(comm_buf);
> +
> + return EFI_EXIT(ret);
> +}
> +
> +/**
> + * efi_query_variable_info() - get information about EFI variables
> + *
> + * This function implements the QueryVariableInfo() runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @attributes: bitmask to select variables to be
> + * queried
> + * @maximum_variable_storage_size: maximum size of storage area for the
> + * selected variable types
> + * @remaining_variable_storage_size: remaining size of storage are for the
> + * selected variable types
> + * @maximum_variable_size: maximum size of a variable of the
> + * selected type
> + * Returns: status code
> + */
> +efi_status_t EFIAPI __efi_runtime
> +efi_query_variable_info(u32 attributes, u64 *max_variable_storage_size,
> + u64 *remain_variable_storage_size,
> + u64 *max_variable_size)
> +{
> + struct smm_variable_query_info *mm_query_info;
> + efi_uintn_t payload_size;
> + efi_status_t ret;
> + u8 *comm_buf;
> +
> + EFI_ENTRY("%x %p %p %p", attributes, max_variable_storage_size,
> + remain_variable_storage_size, max_variable_size);
> +
> + payload_size = sizeof(*mm_query_info);
> + comm_buf = setup_mm_hdr((void **)&mm_query_info, payload_size,
> + SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO,
> + &ret);
> + if (!comm_buf)
> + return EFI_EXIT(ret);
> +
> + mm_query_info->attr = attributes;
> + ret = mm_communicate(comm_buf, payload_size);
> + if (ret != EFI_SUCCESS)
> + goto out;
> + *max_variable_storage_size = mm_query_info->max_variable_storage;
> + *remain_variable_storage_size =
> + mm_query_info->remaining_variable_storage;
> + *max_variable_size = mm_query_info->max_variable_size;
> +
> +out:
> + free(comm_buf);
> + return EFI_EXIT(ret);
> +}
> +
> +/**
> + * efi_get_variable_runtime() - runtime implementation of GetVariable()
> + *
> + * @variable_name: name of the variable
> + * @guid: 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
> + */
> +static efi_status_t __efi_runtime EFIAPI
> +efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> + u32 *attributes, efi_uintn_t *data_size, void *data)
> +{
> + return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * efi_get_next_variable_name_runtime() - runtime implementation of
> + * GetNextVariable()
> + *
> + * @variable_name_size: size of variable_name buffer in byte
> + * @variable_name: name of uefi variable's name in u16
> + * @guid: vendor's guid
> + * Return: status code
> + */
> +static efi_status_t __efi_runtime EFIAPI
> +efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
> + u16 *variable_name, efi_guid_t *guid)
> +{
> + return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * efi_query_variable_info() - get information about EFI variables
> + *
> + * This function implements the QueryVariableInfo() runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @attributes: bitmask to select variables to be
> + * queried
> + * @maximum_variable_storage_size: maximum size of storage area for the
> + * selected variable types
> + * @remaining_variable_storage_size: remaining size of storage are for the
> + * selected variable types
> + * @maximum_variable_size: maximum size of a variable of the
> + * selected type
> + * Return: status code
> + */
> +efi_status_t EFIAPI __efi_runtime
> +efi_query_variable_info_runtime(u32 attributes, u64 *max_variable_storage_size,
> + u64 *remain_variable_storage_size,
> + u64 *max_variable_size)
> +{
> + return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * efi_set_variable_runtime() - runtime implementation of SetVariable()
> + *
> + * @variable_name: name of the variable
> + * @guid: 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
> + */
> +static efi_status_t __efi_runtime EFIAPI
> +efi_set_variable_runtime(u16 *variable_name, const efi_guid_t *guid,
> + u32 attributes, efi_uintn_t data_size,
> + const void *data)
> +{
> + return EFI_UNSUPPORTED;
> +}
> +
> +/**
> + * efi_variables_boot_exit_notify() - notify ExitBootServices() is called
> + */
> +void efi_variables_boot_exit_notify(void)
> +{
> + u8 *comm_buf;
> + efi_status_t ret;
> +
> + comm_buf = setup_mm_hdr(NULL, 0,
> + SMM_VARIABLE_FUNCTION_EXIT_BOOT_SERVICE, &ret);
> + if (comm_buf)
> + ret = mm_communicate(comm_buf, 0);
> + else
> + ret = EFI_NOT_FOUND;
> +
> + if (ret != EFI_SUCCESS)
> + log_err("Unable to notify StMM for ExitBootServices\n");
> + free(comm_buf);
> +
> + /* Update runtime service table */
> + efi_runtime_services.query_variable_info =
> + efi_query_variable_info_runtime;
> + efi_runtime_services.get_variable = efi_get_variable_runtime;
> + efi_runtime_services.get_next_variable_name =
> + efi_get_next_variable_name_runtime;
> + efi_runtime_services.set_variable = efi_set_variable_runtime;
> + efi_update_table_header_crc32(&efi_runtime_services.hdr);
> +}
> +
> +/**
> + * efi_init_variables() - initialize variable services
> + *
> + * Return: status code
> + */
> +efi_status_t efi_init_variables(void)
> +{
> + efi_status_t ret;
> +
> + ret = get_max_payload(&max_payload_size);
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
> + max_buffer_size = MM_COMMUNICATE_HEADER_SIZE +
> + MM_VARIABLE_COMMUNICATE_SIZE +
> + max_payload_size;
> +
> + return EFI_SUCCESS;
> +}
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE
2020-05-13 6:14 ` Heinrich Schuchardt
@ 2020-05-13 8:10 ` Ilias Apalodimas
0 siblings, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-13 8:10 UTC (permalink / raw)
To: u-boot
On Wed, May 13, 2020 at 08:14:19AM +0200, Heinrich Schuchardt wrote:
> On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> > In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
> > StandAloneMM is responsible for the UEFI variable support. In
> > +
[...]
> > + EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, guid);
> > +
> > + if (!variable_name_size || !variable_name || !guid)
> > + return EFI_EXIT(EFI_INVALID_PARAMETER);
> > +
> > + out_name_size = *variable_name_size;
> > + in_name_size = u16_strsize(variable_name);
>
> The UEFI spec requires that EFI_INVALID_PARAMETER should be returned if
> there is no '\0' character in the first *variable_name_size words of
> variable_name. I think we should add this test here instead of using
> max(out_name_size, in_name_size) later in the code.
Ok I'll have a look
>
> You are currently calling EFI_EXIT() in many places. Depending on the
> level of code optimizations done by the compiler and the debug settings
> this may need to unnecessary code size. I suggest to use a single exit
> point in each of the functions, e.g.
>
> if (out_name_size > in_name_size) {
> ret = EFI_INVALID_PARAMETER;
> goto out;
> }
> ...
> out:
> EFI_EXIT(ret);
Fair enough, most of U-Boot is coded that way anyway, might as well have a
common approach.
I'll post a v3 with the changes, so you can do your testing directly in that
Regards
/Ilias
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE
2020-05-11 18:14 ` [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE Ilias Apalodimas
2020-05-13 6:14 ` Heinrich Schuchardt
@ 2020-05-15 11:55 ` Heinrich Schuchardt
2020-05-15 12:08 ` Ilias Apalodimas
1 sibling, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2020-05-15 11:55 UTC (permalink / raw)
To: u-boot
On 11.05.20 20:14, Ilias Apalodimas wrote:
> In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
> StandAloneMM is responsible for the UEFI variable support. In
> combination with OP-TEE and it's U-Boot supplicant, variables are
> authenticated/validated in secure world and stored on an RPMB partition.
>
> So let's add a new config option in U-Boot implementing the necessary
> calls to OP-TEE for the variable management.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Pipat Methavanitpong <pipat1010@gmail.com>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> lib/efi_loader/Kconfig | 9 +
> lib/efi_loader/Makefile | 4 +
> lib/efi_loader/efi_variable_tee.c | 643 ++++++++++++++++++++++++++++++
> 3 files changed, 656 insertions(+)
> create mode 100644 lib/efi_loader/efi_variable_tee.c
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 1cfa24ffcf72..e385cd0b9dae 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -164,4 +164,13 @@ config EFI_SECURE_BOOT
> it is signed with a trusted key. To do that, you need to install,
> at least, PK, KEK and db.
>
> +config EFI_MM_COMM_TEE
> + bool "UEFI variables storage service via OP-TEE"
> + depends on SUPPORT_EMMC_RPMB
> + default n
> + help
> + If OP-TEE is present and running StandAloneMM dispatch all UEFI variable
> + related operations to that. The application will verify, authenticate and
> + store the variables on an RPMB
> +
> endif
Dear Ilias,
at least CONFIG_TEE=y is needed as dependency. Otherwise compiling fails.
If OP-TEE is not found:
=> bootefi hello
Unable to open OP-TEE session (err=-19)
mm_communicate failed!
Error: Cannot initialize UEFI sub-system, r = 3
I think it could be allowable to boot without variable support unless we
are in secure boot audit mode. So if CONFIG_EFI_SECURE_BOOT=n, maybe we
should be less strict. I guess it will end up in weighing user
friendliness against complexity. What are your ideas?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE
2020-05-15 11:55 ` Heinrich Schuchardt
@ 2020-05-15 12:08 ` Ilias Apalodimas
0 siblings, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2020-05-15 12:08 UTC (permalink / raw)
To: u-boot
On Fri, May 15, 2020 at 01:55:35PM +0200, Heinrich Schuchardt wrote:
> On 11.05.20 20:14, Ilias Apalodimas wrote:
> > In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
> > StandAloneMM is responsible for the UEFI variable support. In
> > combination with OP-TEE and it's U-Boot supplicant, variables are
> > authenticated/validated in secure world and stored on an RPMB partition.
> >
> > So let's add a new config option in U-Boot implementing the necessary
> > calls to OP-TEE for the variable management.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Signed-off-by: Pipat Methavanitpong <pipat1010@gmail.com>
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > lib/efi_loader/Kconfig | 9 +
> > lib/efi_loader/Makefile | 4 +
> > lib/efi_loader/efi_variable_tee.c | 643 ++++++++++++++++++++++++++++++
> > 3 files changed, 656 insertions(+)
> > create mode 100644 lib/efi_loader/efi_variable_tee.c
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 1cfa24ffcf72..e385cd0b9dae 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -164,4 +164,13 @@ config EFI_SECURE_BOOT
> > it is signed with a trusted key. To do that, you need to install,
> > at least, PK, KEK and db.
> >
> > +config EFI_MM_COMM_TEE
> > + bool "UEFI variables storage service via OP-TEE"
> > + depends on SUPPORT_EMMC_RPMB
> > + default n
> > + help
> > + If OP-TEE is present and running StandAloneMM dispatch all UEFI variable
> > + related operations to that. The application will verify, authenticate and
> > + store the variables on an RPMB
> > +
> > endif
>
> Dear Ilias,
>
> at least CONFIG_TEE=y is needed as dependency. Otherwise compiling fails.
Ok I'll add that
>
> If OP-TEE is not found:
>
> => bootefi hello
> Unable to open OP-TEE session (err=-19)
> mm_communicate failed!
> Error: Cannot initialize UEFI sub-system, r = 3
>
> I think it could be allowable to boot without variable support unless we
> are in secure boot audit mode. So if CONFIG_EFI_SECURE_BOOT=n, maybe we
> should be less strict. I guess it will end up in weighing user
> friendliness against complexity. What are your ideas?
I don't have any strong opinions on that tbh. But I think i'd choose the
stricter approach. If it's a UEFI boot/whatever, variables must be initialized
and accessible.
Regards
/Ilias
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-05-15 12:08 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11 18:13 [PATCH 0/6 v2] EFI variable support via OP-TEE Ilias Apalodimas
2020-05-11 18:13 ` [PATCH 1/5 v2] efi_loader: Add headers for EDK2 StandAloneMM communication Ilias Apalodimas
2020-05-11 19:39 ` Heinrich Schuchardt
2020-05-12 4:15 ` Ilias Apalodimas
2020-05-12 4:34 ` Ilias Apalodimas
2020-05-11 18:14 ` [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via OP-TEE Ilias Apalodimas
2020-05-13 6:14 ` Heinrich Schuchardt
2020-05-13 8:10 ` Ilias Apalodimas
2020-05-15 11:55 ` Heinrich Schuchardt
2020-05-15 12:08 ` Ilias Apalodimas
2020-05-11 18:14 ` [PATCH 3/5 v2] cmd: efidebug: Add support for querying UEFI variable storage Ilias Apalodimas
2020-05-11 18:54 ` Heinrich Schuchardt
2020-05-12 4:02 ` Ilias Apalodimas
2020-05-11 18:14 ` [PATCH 4/5 v2] MAINTAINERS: Add maintainer for EFI variables via OP-TEE Ilias Apalodimas
2020-05-11 18:39 ` Heinrich Schuchardt
2020-05-11 18:14 ` [PATCH 5/5 v2] doc: uefi.rst: Add OP-TEE variable storage config options Ilias Apalodimas
2020-05-11 18:38 ` Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.