[PATCH 3/6] efi_loader: Implement EFI variable handling via OP-TEE

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

On 5/6/20 9:12 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 | 645 ++++++++++++++++++++++++++++++
>  3 files changed, 658 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..d4e206e22073
> --- /dev/null
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -0,0 +1,645 @@
> +// 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>
> +#include <mm_variable.h>
> +
> +#define PTA_STMM_CMDID_COMMUNICATE 0

This seems to be a part of the OP-TEE communication interface to SMM and
should be in an include with a comment describing the meaning of the
constant.

> +#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)

These two GUIDs are just the same by value. Looking at EFI_GUID() and
tee_optee_ta_uuid_to_octets() it seems that TEE is using big endian
GUIDs while UEFI uses low endian ones. I think this deserves a comment
in your code.

I would prefer to see the GUIDs in the includes. Anyway you copied from
MdeModulePkg/Include/Protocol/SmmVariable.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;
> +};
> +
> +int get_connection(struct mm_connection *conn)

This function is only used locally. Please, mark it as static.

Please, comment your functions according to
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

> +{
> +	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

%s/allocted/allocated/

> + * @dsize:		Buffer size
> + * Return:		status code

Please, be consistent in the capitalization of the argument descriptions.

> + */
> +static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
> +{
> +	ulong buf_size;
> +	efi_status_t ret;
> +	struct 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 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 MM_RET_SUCCESS:
> +		ret = EFI_SUCCESS;
> +		break;
> +
> +	case MM_RET_INVALID_PARAMS:
> +		ret = EFI_INVALID_PARAMETER;
> +		break;
> +
> +	case MM_RET_DENIED:
> +		ret = EFI_ACCESS_DENIED;
> +		break;
> +
> +	case 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

%s/cmonnucation/communication/

> + * it to OP-TEE
> + *
> + * @comm_buf:		Locally allocted communcation buffer

%s/allocted/allocated/

You can use spell-checking in vim with 'set spell'.

> + * @dsize:		Buffer size
> + * Return:		status code
> + */
> +static efi_status_t mm_communicate(u8 *comm_buf, efi_uintn_t dsize)
> +{
> +	efi_status_t ret;
> +	struct mm_communicate_header *mm_hdr;
> +	struct mm_variable_communicate_header *var_hdr;
> +
> +	dsize += MM_COMMUNICATE_HEADER_SIZE + MM_VARIABLE_COMMUNICATE_SIZE;
> +	mm_hdr = (struct mm_communicate_header *)comm_buf;
> +	var_hdr = (struct mm_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 mm_communicate_header *mm_hdr;
> +	struct mm_variable_communicate_header *var_hdr;
> +	u8 *comm_buf;
> +
> +	/* On the init function we initialize max_buffer_size with

%s/On the init/In the init/

> +	 * 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

%s/too long/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 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 mm_variable_communicate_header *)mm_hdr->data;
> +	var_hdr->function = func;
> +	if (dptr)
> +		*dptr = var_hdr->data;
> +	*ret = EFI_SUCCESS;
> +
> +	return comm_buf;
> +}
> +
> +/**
> + * Get variable payload size from StandAloneMM

get_max_payload() - get variable ....

> + *
> + * @size:    Size of the variable in storage
> + * Return:   status code
> + */
> +efi_status_t EFIAPI get_max_payload(efi_uintn_t *size)
> +{
> +	struct mm_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,
> +				MM_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 mm_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 comm buffer and initialize header */

%s/comm/communication/

> +	payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + tmp_dsize;
> +	comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
> +				MM_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 mm_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: "The size must be large enough to fit input
string supplied in VariableName buffer."

Further it is required to return EFI_INVALID_PARAMETER if the
"Null-terminator is not found in the first VariableNameSize bytes of the
input VariableName buffer."

Please, investigate if SMM takes care of the check or we should do it.

> +
> +	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);

Only out_name_size is relevant here.

> +	comm_buf = setup_mm_hdr((void **)&var_getnext, payload_size,
> +				MM_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 mm_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 comm buffer and initialize header */
> +	comm_buf = setup_mm_hdr((void **)&var_acc, payload_size,
> +				MM_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 mm_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,
> +				MM_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);
> +}

The code below for runtime seems to be duplicated from efi_variable.c.
Let's keep it like this for now. We can look for a better solution once
we investigate a runtime solution.

Overall the code looks good and clean (except for my nitpicky comments).

Thanks a lot for all the effort you invested here.

Best regards

Heinrich

> +
> +/**
> + * 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
> + * Returns:				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, MM_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;
> +}
>