All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway
@ 2021-12-20 14:14 xueliang.zhong
  2021-12-20 14:14 ` [PATCH 1/4] " xueliang.zhong
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
  To: meta-arm, Ross.Burton; +Cc: nd, Xueliang Zhong

From: Xueliang Zhong <xueliang.zhong@arm.com>

This patchset will add a macro to configure the volatile and
non volatile storage in SMM gateway. Few useful logs are
also added to the secure world.

Gowtham Suresh Kumar (3):
  arm-bsp/secure-partitions: corstone1000: Configure storage in SMM
    gateway
  arm-bsp/u-boot: corstone1000: Fix SCT failures
  arm-bsp/security: Revert set append write TS patch

Vishnu Banavath (1):
  arm-bsp/u-boot: populate ESRT table with image_info

 ...-Disable-DHCP-PING-config-parameters.patch |   31 +
 ...1000-Disable-set-get-of-NV-variables.patch |   48 +
 ...config-enable-CAPSULE_FIRMWARE_RAW-c.patch |   31 +
 ...ate-ESRT-table-if-EFI_ESRT-config-op.patch |   38 +
 ...-add-get_image_info-for-corstone1000.patch |  119 ++
 .../recipes-bsp/u-boot/u-boot_%.bbappend      |    5 +
 ...-logs-to-functions-in-SMM-gateway-SP.patch |  251 ++++
 .../0027-Configure-storage-size.patch         |   43 +
 ...d-uefi-variable-append-write-support.patch | 1220 +++++++++++++++++
 .../trusted-services/ts-corstone1000.inc      |    3 +
 10 files changed, 1789 insertions(+)
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0043-Disable-DHCP-PING-config-parameters.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0045-corstone1000-defconfig-enable-CAPSULE_FIRMWARE_RAW-c.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0046-efi_loader-populate-ESRT-table-if-EFI_ESRT-config-op.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0047-efi_firmware-add-get_image_info-for-corstone1000.patch
 create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
 create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch
 create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch

-- 
2.17.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway
  2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
@ 2021-12-20 14:14 ` xueliang.zhong
  2021-12-20 14:14 ` [PATCH 2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures xueliang.zhong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
  To: meta-arm, Ross.Burton; +Cc: nd, Gowtham Suresh Kumar

From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>

This patch will add a macro to configure the volatile and
non volatile storage in SMM gateway. Few useful logs are
also added to the secure world.

Change-Id: Ifdb405a09a9a72718df8b335b9f42509dd8c850c
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
---
 ...-logs-to-functions-in-SMM-gateway-SP.patch | 251 ++++++++++++++++++
 .../0027-Configure-storage-size.patch         |  43 +++
 .../trusted-services/ts-corstone1000.inc      |   2 +
 3 files changed, 296 insertions(+)
 create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
 create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch

diff --git a/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
new file mode 100644
index 0000000..1f46586
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0026-Add-logs-to-functions-in-SMM-gateway-SP.patch
@@ -0,0 +1,251 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 9dc09450180d2d35b61359399c1313a97016ad07 Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Thu, 16 Dec 2021 13:29:58 +0000
+Subject: [PATCH] Add logs to functions in SMM gateway SP
+
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+%% original patch: 0026-Logging.patch
+---
+ .../backend/uefi_variable_store.c             | 29 +++++++++++++++++--
+ .../provider/smm_variable_provider.c          |  7 +++--
+ 2 files changed, 32 insertions(+), 4 deletions(-)
+
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index ed50eaf..0c371e9 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -11,6 +11,7 @@
+ #include "uefi_variable_store.h"
+ #include "variable_index_iterator.h"
+ #include "variable_checker.h"
++#include <trace.h>
+ 
+ /* Private functions */
+ static void load_variable_index(
+@@ -151,12 +152,15 @@ void uefi_variable_store_set_storage_limits(
+ 	size_t total_capacity,
+ 	size_t max_variable_size)
+ {
++	EMSG("In func %s\n", __func__);
+ 	struct delegate_variable_store *delegate_store = select_delegate_store(
+ 		context,
+ 		attributes);
+ 
+ 	delegate_store->total_capacity = total_capacity;
+ 	delegate_store->max_variable_size = max_variable_size;
++	EMSG("In func %s total_capacity is %d\n", __func__, total_capacity);
++	EMSG("In func %s max_variable_size is %d\n", __func__, max_variable_size);
+ }
+ 
+ efi_status_t uefi_variable_store_set_variable(
+@@ -265,6 +269,7 @@ efi_status_t uefi_variable_store_get_variable(
+ 	size_t max_data_len,
+ 	size_t *total_length)
+ {
++	EMSG("In func %s\n", __func__);
+ 	efi_status_t status = check_name_terminator(var->Name, var->NameSize);
+ 	if (status != EFI_SUCCESS) return status;
+ 
+@@ -299,6 +304,7 @@ efi_status_t uefi_variable_store_get_next_variable_name(
+ 	size_t max_name_len,
+ 	size_t *total_length)
+ {
++	EMSG("In func %s\n", __func__);
+ 	efi_status_t status = check_name_terminator(cur->Name, cur->NameSize);
+ 	if (status != EFI_SUCCESS) return status;
+ 
+@@ -329,6 +335,8 @@ efi_status_t uefi_variable_store_query_variable_info(
+ 	struct uefi_variable_store *context,
+ 	SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO *var_info)
+ {
++
++	EMSG("In func %s\n", __func__);
+ 	struct delegate_variable_store *delegate_store = select_delegate_store(
+ 		context,
+ 		var_info->Attributes);
+@@ -337,13 +345,15 @@ efi_status_t uefi_variable_store_query_variable_info(
+ 		context,
+ 		var_info->Attributes,
+ 		delegate_store->storage_backend);
+-
++	EMSG("In func %s total_used is %d\n", __func__, total_used);
+ 	var_info->MaximumVariableSize = delegate_store->max_variable_size;
+ 	var_info->MaximumVariableStorageSize = delegate_store->total_capacity;
+ 	var_info->RemainingVariableStorageSize = (total_used < delegate_store->total_capacity) ?
+ 		delegate_store->total_capacity - total_used :
+ 		0;
+-
++	EMSG("In func %s var_info->MaximumVariableSize is %d\n", __func__, var_info->MaximumVariableSize);
++	EMSG("In func %s var_info->MaximumVariableStorageSize is %d\n", __func__, var_info->MaximumVariableStorageSize);
++	EMSG("In func %s var_info->RemainingVariableStorageSize is %d\n", __func__, var_info->RemainingVariableStorageSize);
+ 	return EFI_SUCCESS;
+ }
+ 
+@@ -358,6 +368,7 @@ efi_status_t uefi_variable_store_set_var_check_property(
+ 	struct uefi_variable_store *context,
+ 	const SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *property)
+ {
++	EMSG("In func %s\n", __func__);
+ 	efi_status_t status = check_name_terminator(property->Name, property->NameSize);
+ 	if (status != EFI_SUCCESS) return status;
+ 
+@@ -404,6 +415,7 @@ efi_status_t uefi_variable_store_get_var_check_property(
+ 	struct uefi_variable_store *context,
+ 	SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY *property)
+ {
++	EMSG("In func %s\n", __func__);
+ 	efi_status_t status = check_name_terminator(property->Name, property->NameSize);
+ 	if (status != EFI_SUCCESS) return status;
+ 
+@@ -430,6 +442,7 @@ efi_status_t uefi_variable_store_get_var_check_property(
+ static void load_variable_index(
+ 	struct uefi_variable_store *context)
+ {
++	EMSG("In func %s\n", __func__);
+ 	struct storage_backend *persistent_store = context->persistent_store.storage_backend;
+ 
+ 	if (persistent_store) {
+@@ -444,6 +457,7 @@ static void load_variable_index(
+ 			context->index_sync_buffer_size,
+ 			context->index_sync_buffer,
+ 			&data_len);
++		EMSG("In func %s get status is %d\n", __func__, psa_status);
+ 
+ 		if (psa_status == PSA_SUCCESS) {
+ 
+@@ -455,6 +469,7 @@ static void load_variable_index(
+ static efi_status_t sync_variable_index(
+ 	struct uefi_variable_store *context)
+ {
++	EMSG("In func %s\n", __func__);
+ 	efi_status_t status = EFI_SUCCESS;
+ 
+ 	/* Sync the varibale index to storage if anything is dirty */
+@@ -479,6 +494,7 @@ static efi_status_t sync_variable_index(
+ 				data_len,
+ 				context->index_sync_buffer,
+ 				PSA_STORAGE_FLAG_NONE);
++			EMSG("In func %s set status is %d\n", __func__, psa_status);
+ 
+ 			status = psa_to_efi_storage_status(psa_status);
+ 		}
+@@ -490,6 +506,7 @@ static efi_status_t sync_variable_index(
+ static efi_status_t check_capabilities(
+ 	const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var)
+ {
++	EMSG("In func %s\n", __func__);
+ 	efi_status_t status = EFI_SUCCESS;
+ 
+ 	/* Check if any unsupported variable attributes have been requested */
+@@ -551,6 +568,7 @@ static efi_status_t store_variable_data(
+ 	const struct variable_info *info,
+ 	const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var)
+ {
++	EMSG("In func %s\n", __func__);
+ 	psa_status_t psa_status = PSA_SUCCESS;
+ 	size_t data_len = var->DataSize;
+ 	const uint8_t *data = (const uint8_t*)var +
+@@ -599,6 +617,7 @@ static efi_status_t remove_variable_data(
+ 	struct uefi_variable_store *context,
+ 	const struct variable_info *info)
+ {
++	EMSG("In func %s\n", __func__);
+ 	psa_status_t psa_status = PSA_SUCCESS;
+ 
+ 	if (info->is_variable_set) {
+@@ -613,6 +632,7 @@ static efi_status_t remove_variable_data(
+ 				delegate_store->storage_backend->context,
+ 				context->owner_id,
+ 				info->metadata.uid);
++			EMSG("In func %s status is %d\n", __func__, psa_status);
+ 		}
+ 	}
+ 
+@@ -625,6 +645,7 @@ static efi_status_t load_variable_data(
+ 	SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var,
+ 	size_t max_data_len)
+ {
++	EMSG("In func %s\n", __func__);
+ 	psa_status_t psa_status = PSA_SUCCESS;
+ 	size_t data_len = 0;
+ 	uint8_t *data = (uint8_t*)var +
+@@ -644,6 +665,7 @@ static efi_status_t load_variable_data(
+ 			max_data_len,
+ 			data,
+ 			&data_len);
++		EMSG("In func %s get status is %d\n", __func__, psa_status);
+ 
+ 		var->DataSize = data_len;
+ 	}
+@@ -771,6 +793,7 @@ static void purge_orphan_index_entries(
+ 				context->owner_id,
+ 				info->metadata.uid,
+ 				&storage_info);
++			EMSG("In func %s get status is %d\n", __func__, psa_status);
+ 
+ 			if (psa_status != PSA_SUCCESS) {
+ 
+@@ -802,6 +825,7 @@ static size_t space_used(
+ 	uint32_t attributes,
+ 	struct storage_backend *storage_backend)
+ {
++	EMSG("In func %s\n", __func__);
+ 	if (!storage_backend) return 0;
+ 
+ 	size_t total_used = 0;
+@@ -823,6 +847,7 @@ static size_t space_used(
+ 				context->owner_id,
+ 				info->metadata.uid,
+ 				&storage_info);
++			EMSG("In func %s get status is %d\n", __func__, psa_status);
+ 
+ 			if (psa_status == PSA_SUCCESS) total_used += storage_info.size;
+ 		}
+diff --git a/components/service/smm_variable/provider/smm_variable_provider.c b/components/service/smm_variable/provider/smm_variable_provider.c
+index 52e68d0..1f362c1 100644
+--- a/components/service/smm_variable/provider/smm_variable_provider.c
++++ b/components/service/smm_variable/provider/smm_variable_provider.c
+@@ -9,6 +9,7 @@
+ #include <protocols/service/smm_variable/smm_variable_proto.h>
+ #include <protocols/rpc/common/packed-c/status.h>
+ #include "smm_variable_provider.h"
++#include <trace.h>
+ 
+ /* Service request handlers */
+ static rpc_status_t get_variable_handler(void *context, struct call_req *req);
+@@ -252,17 +253,18 @@ static rpc_status_t set_variable_handler(void *context, struct call_req* req)
+ 
+ static rpc_status_t query_variable_info_handler(void *context, struct call_req* req)
+ {
++	EMSG("In func %s \n", __func__);
+ 	efi_status_t efi_status = EFI_INVALID_PARAMETER;
+ 	struct smm_variable_provider *this_instance = (struct smm_variable_provider*)context;
+ 
+ 	const struct call_param_buf *req_buf = call_req_get_req_buf(req);
+-
++	EMSG("In func %s sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO) is %d\n", __func__, sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO));
+ 	if (req_buf->data_len >= sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO)) {
+ 
+ 		struct call_param_buf *resp_buf = call_req_get_resp_buf(req);
+ 
+ 		if (resp_buf->size >= req_buf->data_len) {
+-
++			
+ 			memmove(resp_buf->data, req_buf->data, req_buf->data_len);
+ 
+ 			efi_status = uefi_variable_store_query_variable_info(
+@@ -272,6 +274,7 @@ static rpc_status_t query_variable_info_handler(void *context, struct call_req*
+ 			if (efi_status == EFI_SUCCESS) {
+ 
+ 				resp_buf->data_len = sizeof(SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO);
++				EMSG("In func %s resp_buf->data_len is %d\n", __func__, resp_buf->data_len);
+ 			}
+ 		}
+ 		else {
+-- 
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch
new file mode 100644
index 0000000..764cfe6
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0027-Configure-storage-size.patch
@@ -0,0 +1,43 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 02746a26472f6aa7d57cfd5388823b0ec3c8a945 Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Thu, 16 Dec 2021 21:31:40 +0000
+Subject: [PATCH] Configure storage size
+
+---
+ .../service/smm_variable/backend/uefi_variable_store.c       | 5 +++--
+ 1 file changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index 0c371e9..b7cfff4 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -87,6 +87,7 @@ static efi_status_t check_name_terminator(
+  * may be overridden using uefi_variable_store_set_storage_limits()
+  */
+ #define DEFAULT_MAX_VARIABLE_SIZE			(2048)
++#define CONFIGURE_STORAGE_SIZE			    (50)
+ 
+ efi_status_t uefi_variable_store_init(
+ 	struct uefi_variable_store *context,
+@@ -100,13 +101,13 @@ efi_status_t uefi_variable_store_init(
+ 	/* Initialise persistent store defaults */
+ 	context->persistent_store.is_nv = true;
+ 	context->persistent_store.max_variable_size = DEFAULT_MAX_VARIABLE_SIZE;
+-	context->persistent_store.total_capacity = DEFAULT_MAX_VARIABLE_SIZE * max_variables;
++	context->persistent_store.total_capacity = CONFIGURE_STORAGE_SIZE * max_variables;
+ 	context->persistent_store.storage_backend = persistent_store;
+ 
+ 	/* Initialise volatile store defaults */
+ 	context->volatile_store.is_nv = false;
+ 	context->volatile_store.max_variable_size = DEFAULT_MAX_VARIABLE_SIZE;
+-	context->volatile_store.total_capacity = DEFAULT_MAX_VARIABLE_SIZE * max_variables;
++	context->volatile_store.total_capacity = CONFIGURE_STORAGE_SIZE * max_variables;
+ 	context->volatile_store.storage_backend = volatile_store;
+ 
+ 	context->owner_id = owner_id;
+-- 
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
index bb34f54..4a18586 100644
--- a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
+++ b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
@@ -36,6 +36,8 @@ SRC_URI:append = " \
                   file://0023-add-psa-ipc-crypto-backend.patch \
                   file://0024-Increase-SMM-gateway-UEFI-variable-macro-value.patch \
                   file://0025-Add-stub-capsule-update-service-components.patch \
+                  file://0026-Add-logs-to-functions-in-SMM-gateway-SP.patch \
+                  file://0027-Configure-storage-size.patch \
                   "
 
 SRC_URI_MBED = "git://github.com/ARMmbed/mbed-crypto.git;protocol=https;branch=development;name=mbed;destsuffix=git/mbedcrypto"
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures
  2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
  2021-12-20 14:14 ` [PATCH 1/4] " xueliang.zhong
@ 2021-12-20 14:14 ` xueliang.zhong
  2021-12-20 14:14 ` [PATCH 3/4] arm-bsp/security: Revert set append write TS patch xueliang.zhong
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
  To: meta-arm, Ross.Burton; +Cc: nd, Gowtham Suresh Kumar

From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>

This patch removes the CONFIG_CMD_DHCP and CONFIG_CMD_PING
config parameters from the defconfig. It also reverts the workaround
patch which disabled NV get and set on u-boot.

Change-Id: I80f41235dbca2e76003c28164b42f4403dadc499
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
---
 ...-Disable-DHCP-PING-config-parameters.patch | 31 ++++++++++++
 ...1000-Disable-set-get-of-NV-variables.patch | 48 +++++++++++++++++++
 .../recipes-bsp/u-boot/u-boot_%.bbappend      |  2 +
 3 files changed, 81 insertions(+)
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0043-Disable-DHCP-PING-config-parameters.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch

diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0043-Disable-DHCP-PING-config-parameters.patch b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0043-Disable-DHCP-PING-config-parameters.patch
new file mode 100644
index 0000000..e96e9a5
--- /dev/null
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0043-Disable-DHCP-PING-config-parameters.patch
@@ -0,0 +1,31 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From e6b420ce5d56fcc08aac2812ee5402686fa56fae Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Thu, 16 Dec 2021 17:25:29 +0000
+Subject: [PATCH] Disable DHCP, PING config parameters
+
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+---
+ configs/corstone1000_defconfig | 2 --
+ 1 file changed, 2 deletions(-)
+
+diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
+index 3c00e13ceb..e11ffbfc89 100644
+--- a/configs/corstone1000_defconfig
++++ b/configs/corstone1000_defconfig
+@@ -31,10 +31,8 @@ CONFIG_CMD_NVEDIT_EFI=y
+ CONFIG_CMD_USB=y
+ CONFIG_CMD_ITEST=y
+ # CONFIG_CMD_SETEXPR is not set
+-CONFIG_CMD_DHCP=y
+ # CONFIG_CMD_NFS is not set
+ CONFIG_CMD_MII=y
+-CONFIG_CMD_PING=y
+ CONFIG_CMD_CACHE=y
+ CONFIG_CMD_EFIDEBUG=y
+ CONFIG_CMD_FAT=y
+-- 
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch
new file mode 100644
index 0000000..cb3eb49
--- /dev/null
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch
@@ -0,0 +1,48 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 2a8d1b3fb89ae68d126e16f8346405df8fcc3ed6 Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Thu, 16 Dec 2021 19:36:18 +0000
+Subject: [PATCH] Revert "corstone1000: Disable set/get of NV variables"
+
+This reverts commit 5c77e9883ea29472c353d63c66a7f8ffd6ea367f.
+
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+---
+ lib/efi_loader/efi_setup.c | 12 ++----------
+ 1 file changed, 2 insertions(+), 10 deletions(-)
+
+diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
+index fcf2eae9cd..9e3399a28c 100644
+--- a/lib/efi_loader/efi_setup.c
++++ b/lib/efi_loader/efi_setup.c
+@@ -83,11 +83,7 @@ static efi_status_t efi_init_platform_lang(void)
+ 
+ 	ret = efi_set_variable_int(L"PlatformLang",
+ 				   &efi_global_variable_guid,
+-				   	/*
+-					* This is a temporary change until NV memory is accessible 
+-					* through OpenAmp. 
+-					*/
+-				   //EFI_VARIABLE_NON_VOLATILE | 
++				   EFI_VARIABLE_NON_VOLATILE |
+ 				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ 				   EFI_VARIABLE_RUNTIME_ACCESS,
+ 				   1 + strlen(lang), lang, false);
+@@ -214,11 +210,7 @@ static efi_status_t efi_clear_os_indications(void)
+ 		os_indications &=
+ 			~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;
+ 	ret = efi_set_variable_int(L"OsIndications", &efi_global_variable_guid,
+-				   	/*
+-					* This is a temporary change until NV memory is accessible 
+-					* through OpenAmp. 
+-					*/
+-				   //EFI_VARIABLE_NON_VOLATILE |
++				   EFI_VARIABLE_NON_VOLATILE |
+ 				   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ 				   EFI_VARIABLE_RUNTIME_ACCESS,
+ 				   sizeof(os_indications), &os_indications,
+-- 
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend b/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend
index 9b9a4ce..9d2db6a 100644
--- a/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend
@@ -52,6 +52,8 @@ SRC_URI:append:corstone1000 = " \
       file://0040-Return-proper-error-code-when-rx-buffer-is-larger.patch \
       file://0041-Use-correct-buffer-size.patch \
       file://0042-Update-comm_buf-when-EFI_BUFFER_TOO_SMALL.patch \
+      file://0043-Disable-DHCP-PING-config-parameters.patch \
+      file://0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch \
       "
 
 #
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] arm-bsp/security: Revert set append write TS patch
  2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
  2021-12-20 14:14 ` [PATCH 1/4] " xueliang.zhong
  2021-12-20 14:14 ` [PATCH 2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures xueliang.zhong
@ 2021-12-20 14:14 ` xueliang.zhong
  2021-12-20 14:14 ` [PATCH 4/4] arm-bsp/u-boot: populate ESRT table with image_info xueliang.zhong
  2021-12-20 22:58 ` [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway Jon Mason
  4 siblings, 0 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
  To: meta-arm, Ross.Burton; +Cc: nd, Gowtham Suresh Kumar

From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>

Change-Id: I6e8aea102ea24271097efe92f4eb820c68ff70d5
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
---
 ...d-uefi-variable-append-write-support.patch | 1220 +++++++++++++++++
 .../trusted-services/ts-corstone1000.inc      |    1 +
 2 files changed, 1221 insertions(+)
 create mode 100644 meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch

diff --git a/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch
new file mode 100644
index 0000000..b7efe19
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/trusted-services/secure-partitions/0028-Revert-Add-uefi-variable-append-write-support.patch
@@ -0,0 +1,1220 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+
+From 85df04f724f95218b57f78425966f0230d75c57e Mon Sep 17 00:00:00 2001
+From: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
+Date: Wed, 15 Dec 2021 17:23:25 +0000
+Subject: [PATCH] Revert "Add uefi variable append write support"
+
+This reverts commit e8758d9aff0eddae81a74b0191cd027bcdc92c04.
+---
+ .../backend/test/variable_index_tests.cpp     |  90 +++---
+ .../backend/test/variable_store_tests.cpp     |  72 +----
+ .../backend/uefi_variable_store.c             | 293 ++++++------------
+ .../smm_variable/backend/variable_index.c     |  95 ++++--
+ .../smm_variable/backend/variable_index.h     |  58 ++--
+ .../backend/variable_index_iterator.c         |   4 +-
+ .../backend/variable_index_iterator.h         |   2 +-
+ .../service/smm_variable_service_tests.cpp    |  48 ---
+ protocols/service/smm_variable/parameters.h   |   3 -
+ 9 files changed, 239 insertions(+), 426 deletions(-)
+
+diff --git a/components/service/smm_variable/backend/test/variable_index_tests.cpp b/components/service/smm_variable/backend/test/variable_index_tests.cpp
+index 8edc0e7..c8bacf9 100644
+--- a/components/service/smm_variable/backend/test/variable_index_tests.cpp
++++ b/components/service/smm_variable/backend/test/variable_index_tests.cpp
+@@ -69,37 +69,34 @@ TEST_GROUP(UefiVariableIndexTests)
+ 
+ 	void create_variables()
+ 	{
+-		struct variable_info *info = NULL;
++		const struct variable_info *info = NULL;
+ 
+-		info = variable_index_add_entry(
++		info = variable_index_add_variable(
+ 			&m_variable_index,
+ 			&guid_1,
+ 			name_1.size() * sizeof(int16_t),
+-			name_1.data());
+-		CHECK_TRUE(info);
+-		variable_index_set_variable(
+-			info,
++			name_1.data(),
+ 			EFI_VARIABLE_BOOTSERVICE_ACCESS);
+ 
+-		info = variable_index_add_entry(
++		CHECK_TRUE(info);
++
++		info = variable_index_add_variable(
+ 			&m_variable_index,
+ 			&guid_2,
+ 			name_2.size() * sizeof(int16_t),
+-			name_2.data());
+-		CHECK_TRUE(info);
+-		variable_index_set_variable(
+-			info,
++			name_2.data(),
+ 			EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS);
+ 
+-		info = variable_index_add_entry(
++		CHECK_TRUE(info);
++
++		info = variable_index_add_variable(
+ 			&m_variable_index,
+ 			&guid_1,
+ 			name_3.size() * sizeof(int16_t),
+-			name_3.data());
+-		CHECK_TRUE(info);
+-		variable_index_set_variable(
+-			info,
++			name_3.data(),
+ 			EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS);
++
++		CHECK_TRUE(info);
+ 	}
+ 
+ 	static const size_t MAX_VARIABLES = 10;
+@@ -114,7 +111,7 @@ TEST_GROUP(UefiVariableIndexTests)
+ 
+ TEST(UefiVariableIndexTests, emptyIndexOperations)
+ {
+-	struct variable_info *info = NULL;
++	const struct variable_info *info = NULL;
+ 
+ 	/* Expect not to find a variable */
+ 	info = variable_index_find(
+@@ -133,34 +130,36 @@ TEST(UefiVariableIndexTests, emptyIndexOperations)
+ 	POINTERS_EQUAL(NULL, info);
+ 
+ 	/* Remove should silently return */
+-	variable_index_clear_variable(
++	variable_index_remove_variable(
+ 		&m_variable_index,
+ 		info);
+ }
+ 
+ TEST(UefiVariableIndexTests, addWithOversizedName)
+ {
+-	struct variable_info *info = NULL;
++	const struct variable_info *info = NULL;
+ 	std::vector<int16_t> name;
+ 
+ 	name = to_variable_name(L"a long variable name that exceeds the length limit");
+ 
+-	info = variable_index_add_entry(
++	info = variable_index_add_variable(
+ 		&m_variable_index,
+ 		&guid_1,
+ 		name.size() * sizeof(int16_t),
+-		name.data());
++		name.data(),
++		EFI_VARIABLE_BOOTSERVICE_ACCESS);
+ 
+ 	/* Expect the add to fail because of an oversized name */
+ 	POINTERS_EQUAL(NULL, info);
+ 
+ 	name = to_variable_name(L"a long variable name that fits!");
+ 
+-	info = variable_index_add_entry(
++	info = variable_index_add_variable(
+ 		&m_variable_index,
+ 		&guid_1,
+ 		name.size() * sizeof(int16_t),
+-		name.data());
++		name.data(),
++		EFI_VARIABLE_BOOTSERVICE_ACCESS);
+ 
+ 	/* Expect the add succeed */
+ 	CHECK_TRUE(info);
+@@ -168,17 +167,18 @@ TEST(UefiVariableIndexTests, addWithOversizedName)
+ 
+ TEST(UefiVariableIndexTests, variableIndexFull)
+ {
+-	struct variable_info *info = NULL;
++	const struct variable_info *info = NULL;
+ 	EFI_GUID guid = guid_1;
+ 
+ 	/* Expect to be able to fill the index */
+ 	for (size_t i = 0; i < MAX_VARIABLES; ++i) {
+ 
+-		info = variable_index_add_entry(
++		info = variable_index_add_variable(
+ 			&m_variable_index,
+ 			&guid,
+ 			name_1.size() * sizeof(int16_t),
+-			name_1.data());
++			name_1.data(),
++			EFI_VARIABLE_BOOTSERVICE_ACCESS);
+ 
+ 		CHECK_TRUE(info);
+ 
+@@ -187,11 +187,12 @@ TEST(UefiVariableIndexTests, variableIndexFull)
+ 	}
+ 
+ 	/* Variable index should now be full */
+-	info = variable_index_add_entry(
++	info = variable_index_add_variable(
+ 		&m_variable_index,
+ 		&guid,
+ 		name_1.size() * sizeof(int16_t),
+-		name_1.data());
++		name_1.data(),
++		EFI_VARIABLE_BOOTSERVICE_ACCESS);
+ 
+ 	POINTERS_EQUAL(NULL, info);
+ }
+@@ -322,7 +323,7 @@ TEST(UefiVariableIndexTests, dumpBufferTooSmall)
+ TEST(UefiVariableIndexTests, removeVariable)
+ {
+ 	uint8_t buffer[MAX_VARIABLES * sizeof(struct variable_metadata)];
+-	struct variable_info *info = NULL;
++	const struct variable_info *info = NULL;
+ 
+ 	create_variables();
+ 
+@@ -333,7 +334,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ 		name_2.size() * sizeof(int16_t),
+ 		name_2.data());
+ 
+-	variable_index_clear_variable(
++	variable_index_remove_variable(
+ 		&m_variable_index,
+ 		info);
+ 
+@@ -351,7 +352,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ 		name_1.size() * sizeof(int16_t),
+ 		name_1.data());
+ 
+-	variable_index_clear_variable(
++	variable_index_remove_variable(
+ 		&m_variable_index,
+ 		info);
+ 
+@@ -369,7 +370,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ 		name_3.size() * sizeof(int16_t),
+ 		name_3.data());
+ 
+-	variable_index_clear_variable(
++	variable_index_remove_variable(
+ 		&m_variable_index,
+ 		info);
+ 
+@@ -394,7 +395,7 @@ TEST(UefiVariableIndexTests, removeVariable)
+ 
+ TEST(UefiVariableIndexTests, checkIterator)
+ {
+-	struct variable_info *info = NULL;
++	const struct variable_info *info = NULL;
+ 
+ 	create_variables();
+ 
+@@ -418,7 +419,7 @@ TEST(UefiVariableIndexTests, checkIterator)
+ 	UNSIGNED_LONGS_EQUAL(name_2.size() * sizeof(int16_t), info->metadata.name_size);
+ 	MEMCMP_EQUAL(name_2.data(), info->metadata.name, info->metadata.name_size);
+ 
+-	struct variable_info *info_to_remove = info;
++	const struct variable_info *info_to_remove = info;
+ 
+ 	variable_index_iterator_next(&iter);
+ 	CHECK_FALSE(variable_index_iterator_is_done(&iter));
+@@ -434,8 +435,7 @@ TEST(UefiVariableIndexTests, checkIterator)
+ 	CHECK_TRUE(variable_index_iterator_is_done(&iter));
+ 
+ 	/* Now remove the middle entry */
+-	variable_index_clear_variable(&m_variable_index, info_to_remove);
+-	variable_index_remove_unused_entry(&m_variable_index, info_to_remove);
++	variable_index_remove_variable(&m_variable_index, info_to_remove);
+ 
+ 	/* Iterate again but this time there should only be two entries */
+ 	variable_index_iterator_first(&iter, &m_variable_index);
+@@ -478,7 +478,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsExistingVar)
+ 	constraints.max_size = 100;
+ 
+ 	/* Set check constraints on one of the variables */
+-	struct variable_info *info = variable_index_find(
++	const struct variable_info *info = variable_index_find(
+ 		&m_variable_index,
+ 		&guid_2,
+ 		name_2.size() * sizeof(int16_t),
+@@ -488,7 +488,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsExistingVar)
+ 	CHECK_TRUE(info->is_variable_set);
+ 	CHECK_FALSE(info->is_constraints_set);
+ 
+-	variable_index_set_constraints(info, &constraints);
++	variable_index_update_constraints(info, &constraints);
+ 
+ 	CHECK_TRUE(info->is_constraints_set);
+ 	CHECK_TRUE(info->is_variable_set);
+@@ -496,7 +496,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsExistingVar)
+ 	/* Remove the variable but still expect the variable to be indexed
+ 	 * because of the set constraints.
+ 	 */
+-	variable_index_clear_variable(
++	variable_index_remove_variable(
+ 		&m_variable_index,
+ 		info);
+ 
+@@ -588,7 +588,7 @@ TEST(UefiVariableIndexTests, setCheckConstraintsNonExistingVar)
+ 	constraints.max_size = 100;
+ 
+ 	/* Initially expect no variable_info */
+-	struct variable_info *info = variable_index_find(
++	const struct variable_info *info = variable_index_find(
+ 		&m_variable_index,
+ 		&guid_2,
+ 		name_2.size() * sizeof(int16_t),
+@@ -597,19 +597,19 @@ TEST(UefiVariableIndexTests, setCheckConstraintsNonExistingVar)
+ 	CHECK_FALSE(info);
+ 
+ 	/* Adding the check constraints should result in an entry being added */
+-	info = variable_index_add_entry(
++	info = variable_index_add_constraints(
+ 		&m_variable_index,
+ 		&guid_2,
+ 		name_2.size() * sizeof(int16_t),
+-		name_2.data());
+-	CHECK_TRUE(info);
++		name_2.data(),
++		&constraints);
+ 
+-	variable_index_set_constraints(info, &constraints);
++	CHECK_TRUE(info);
+ 	CHECK_FALSE(info->is_variable_set);
+ 	CHECK_TRUE(info->is_constraints_set);
+ 
+ 	/* Updating the variable should cause the variable to be marked as set */
+-	variable_index_set_variable(info, EFI_VARIABLE_RUNTIME_ACCESS);
++	variable_index_update_variable(info, EFI_VARIABLE_RUNTIME_ACCESS);
+ 
+ 	CHECK_TRUE(info->is_variable_set);
+ 	CHECK_TRUE(info->is_constraints_set);
+diff --git a/components/service/smm_variable/backend/test/variable_store_tests.cpp b/components/service/smm_variable/backend/test/variable_store_tests.cpp
+index e90c106..235642e 100644
+--- a/components/service/smm_variable/backend/test/variable_store_tests.cpp
++++ b/components/service/smm_variable/backend/test/variable_store_tests.cpp
+@@ -305,37 +305,6 @@ TEST(UefiVariableStoreTests, setGetRoundtrip)
+ 	/* Expect got variable data to be the same as the set value */
+ 	UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
+ 	LONGS_EQUAL(0, input_data.compare(output_data));
+-
+-	/* Extend the variable using an append write */
+-	std::string input_data2 = " jumps over the lazy dog";
+-
+-	status = set_variable(var_name, input_data2, EFI_VARIABLE_APPEND_WRITE);
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+-	status = get_variable(var_name, output_data);
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+-	std::string expected_output = input_data + input_data2;
+-
+-	/* Expect the append write operation to have extended the variable */
+-	UNSIGNED_LONGLONGS_EQUAL(expected_output.size(), output_data.size());
+-	LONGS_EQUAL(0, expected_output.compare(output_data));
+-
+-	/* Expect query_variable_info to return consistent values */
+-	size_t max_variable_storage_size = 0;
+-	size_t remaining_variable_storage_size = 0;
+-	size_t max_variable_size = 0;
+-
+-	status = query_variable_info(
+-		0,
+-		&max_variable_storage_size,
+-		&remaining_variable_storage_size,
+-		&max_variable_size);
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+-	UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY, max_variable_storage_size);
+-	UNSIGNED_LONGLONGS_EQUAL(MAX_VARIABLE_SIZE, max_variable_size);
+-	UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY - expected_output.size(), remaining_variable_storage_size);
+ }
+ 
+ TEST(UefiVariableStoreTests, persistentSetGet)
+@@ -345,8 +314,7 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ 	std::string input_data = "quick brown fox";
+ 	std::string output_data;
+ 
+-	status = set_variable(var_name, input_data,
+-		EFI_VARIABLE_NON_VOLATILE);
++	status = set_variable(var_name, input_data, EFI_VARIABLE_NON_VOLATILE);
+ 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ 
+ 	status = get_variable(var_name, output_data);
+@@ -356,22 +324,6 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ 	UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
+ 	LONGS_EQUAL(0, input_data.compare(output_data));
+ 
+-	/* Extend the variable using an append write */
+-	std::string input_data2 = " jumps over the lazy dog";
+-
+-	status = set_variable(var_name, input_data2,
+-		EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_APPEND_WRITE);
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+-	status = get_variable(var_name, output_data);
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+-	std::string expected_output = input_data + input_data2;
+-
+-	/* Expect the append write operation to have extended the variable */
+-	UNSIGNED_LONGLONGS_EQUAL(expected_output.size(), output_data.size());
+-	LONGS_EQUAL(0, expected_output.compare(output_data));
+-
+ 	/* Expect the variable to survive a power cycle */
+ 	power_cycle();
+ 
+@@ -380,24 +332,8 @@ TEST(UefiVariableStoreTests, persistentSetGet)
+ 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ 
+ 	/* Still expect got variable data to be the same as the set value */
+-	UNSIGNED_LONGLONGS_EQUAL(expected_output.size(), output_data.size());
+-	LONGS_EQUAL(0, expected_output.compare(output_data));
+-
+-	/* Expect query_variable_info to return consistent values */
+-	size_t max_variable_storage_size = 0;
+-	size_t remaining_variable_storage_size = 0;
+-	size_t max_variable_size = 0;
+-
+-	status = query_variable_info(
+-		EFI_VARIABLE_NON_VOLATILE,
+-		&max_variable_storage_size,
+-		&remaining_variable_storage_size,
+-		&max_variable_size);
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+-
+-	UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY, max_variable_storage_size);
+-	UNSIGNED_LONGLONGS_EQUAL(MAX_VARIABLE_SIZE, max_variable_size);
+-	UNSIGNED_LONGLONGS_EQUAL(STORE_CAPACITY - expected_output.size(), remaining_variable_storage_size);
++	UNSIGNED_LONGLONGS_EQUAL(input_data.size(), output_data.size());
++	LONGS_EQUAL(0, input_data.compare(output_data));
+ }
+ 
+ TEST(UefiVariableStoreTests, removeVolatile)
+@@ -436,7 +372,7 @@ TEST(UefiVariableStoreTests, removePersistent)
+ 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ 
+ 	/* Remove by setting with zero data length */
+-	status = set_variable(var_name, std::string(), EFI_VARIABLE_NON_VOLATILE);
++	status = set_variable(var_name, std::string(), 0);
+ 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status);
+ 
+ 	/* Expect variable to no loger exist */
+diff --git a/components/service/smm_variable/backend/uefi_variable_store.c b/components/service/smm_variable/backend/uefi_variable_store.c
+index ed50eaf..d084e8d 100644
+--- a/components/service/smm_variable/backend/uefi_variable_store.c
++++ b/components/service/smm_variable/backend/uefi_variable_store.c
+@@ -46,20 +46,6 @@ static efi_status_t load_variable_data(
+ 	SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var,
+ 	size_t max_data_len);
+ 
+-static psa_status_t store_overwrite(
+-	struct delegate_variable_store *delegate_store,
+-	uint32_t client_id,
+-	uint64_t uid,
+-	size_t data_length,
+-	const void *data);
+-
+-static psa_status_t store_append_write(
+-	struct delegate_variable_store *delegate_store,
+-	uint32_t client_id,
+-	uint64_t uid,
+-	size_t data_length,
+-	const void *data);
+-
+ static void purge_orphan_index_entries(
+ 	struct uefi_variable_store *context);
+ 
+@@ -163,45 +149,40 @@ efi_status_t uefi_variable_store_set_variable(
+ 	struct uefi_variable_store *context,
+ 	const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var)
+ {
+-	bool should_sync_index = false;
+-
+-	/* Validate incoming request */
+ 	efi_status_t status = check_name_terminator(var->Name, var->NameSize);
+ 	if (status != EFI_SUCCESS) return status;
+ 
+ 	status = check_capabilities(var);
++	bool should_sync_index = false;
++
+ 	if (status != EFI_SUCCESS) return status;
+ 
+-	/* Find an existing entry in the variable index or add a new one */
+-	struct variable_info *info = variable_index_find(
++	/* Find in index */
++	const struct variable_info *info = variable_index_find(
+ 		&context->variable_index,
+ 		&var->Guid,
+ 		var->NameSize,
+ 		var->Name);
+ 
+-	if (!info) {
++	if (info) {
+ 
+-		info = variable_index_add_entry(
+-			&context->variable_index,
+-			&var->Guid,
+-			var->NameSize,
+-			var->Name);
++		/* Variable info already exists */
++		status = check_access_permitted_on_set(context, info, var);
+ 
+-		if (!info) return EFI_OUT_OF_RESOURCES;
+-	}
++		if (status == EFI_SUCCESS) {
+ 
+-	/* Control access */
+-	status = check_access_permitted_on_set(context, info, var);
++			should_sync_index =
++				(var->Attributes & EFI_VARIABLE_NON_VOLATILE) ||
++				(info->is_variable_set && (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE));
+ 
+-	if (status == EFI_SUCCESS) {
++			if (var->DataSize) {
+ 
+-		/* Access permitted */
+-		if (info->is_variable_set) {
+-
+-			/* It's a request to update to an existing variable */
+-			if (!(var->Attributes &
+-				(EFI_VARIABLE_APPEND_WRITE | EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS_MASK)) &&
+-				!var->DataSize) {
++				/* It's a set rather than a remove operation */
++				variable_index_update_variable(
++					info,
++					var->Attributes);
++			}
++			else {
+ 
+ 				/* It's a remove operation - for a remove, the variable
+ 				 * data must be removed from the storage backend before
+@@ -210,30 +191,31 @@ efi_status_t uefi_variable_store_set_variable(
+ 				 * the storage backend without a corresponding index entry.
+ 				 */
+ 				remove_variable_data(context, info);
+-				variable_index_clear_variable(&context->variable_index, info);
++				variable_index_remove_variable(&context->variable_index, info);
+ 
+-				should_sync_index = (var->Attributes & EFI_VARIABLE_NON_VOLATILE);
+-			}
+-			else {
+-
+-				/* It's a set operation where variable data is potentially
+-				 * being overwritten or extended.
+-				 */
+-				if ((var->Attributes & ~EFI_VARIABLE_APPEND_WRITE) != info->metadata.attributes) {
+-
+-					/* Modifying attributes is forbidden */
+-					return EFI_INVALID_PARAMETER;
+-				}
++				/* Variable info no longer valid */
++				info = NULL;
+ 			}
+ 		}
+ 		else {
+ 
+-			/*  It's a request to create a new variable */
+-			variable_index_set_variable(info, var->Attributes);
+-
+-			should_sync_index = (var->Attributes & EFI_VARIABLE_NON_VOLATILE);
++			/* Access forbidden */
++			info = NULL;
+ 		}
+ 	}
++	else if (var->DataSize) {
++
++		/* It's a new variable */
++		info = variable_index_add_variable(
++			&context->variable_index,
++			&var->Guid,
++			var->NameSize,
++			var->Name,
++			var->Attributes);
++
++		if (!info) status = EFI_OUT_OF_RESOURCES;
++		should_sync_index = info && (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE);
++	}
+ 
+ 	/* The order of these operations is important. For an update
+ 	 * or create operation, The variable index is always synchronized
+@@ -249,13 +231,11 @@ efi_status_t uefi_variable_store_set_variable(
+ 	}
+ 
+ 	/* Store any variable data to the storage backend */
+-	if (info->is_variable_set && (status == EFI_SUCCESS)) {
++	if (info && (status == EFI_SUCCESS)) {
+ 
+ 		status = store_variable_data(context, info, var);
+ 	}
+ 
+-	variable_index_remove_unused_entry(&context->variable_index, info);
+-
+ 	return status;
+ }
+ 
+@@ -361,41 +341,53 @@ efi_status_t uefi_variable_store_set_var_check_property(
+ 	efi_status_t status = check_name_terminator(property->Name, property->NameSize);
+ 	if (status != EFI_SUCCESS) return status;
+ 
+-	/* Find in index or create a new entry */
+-	struct variable_info *info = variable_index_find(
++	/* Find in index */
++	const struct variable_info *info = variable_index_find(
+ 		&context->variable_index,
+ 		&property->Guid,
+ 		property->NameSize,
+ 		property->Name);
+ 
+-	if (!info) {
++	if (info) {
+ 
+-		info = variable_index_add_entry(
+-			&context->variable_index,
+-			&property->Guid,
+-			property->NameSize,
+-			property->Name);
++		/* Applying check constraints to an existing variable that may have
++		 * constraints already set.  These could constrain the setting of
++		 * the constraints.
++		 */
++		struct variable_constraints constraints = info->check_constraints;
++
++		status = variable_checker_set_constraints(
++			&constraints,
++			info->is_constraints_set,
++			&property->VariableProperty);
++
++		if (status == EFI_SUCCESS) {
+ 
+-		if (!info) return EFI_OUT_OF_RESOURCES;
++			variable_index_update_constraints(info, &constraints);
++		}
+ 	}
++	else {
+ 
+-	/* Applying check constraints to an existing variable that may have
+-	 * constraints already set.  These could constrain the setting of
+-	 * the constraints.
+-	 */
+-	struct variable_constraints constraints = info->check_constraints;
++		/* Applying check constraints for a new variable */
++		struct variable_constraints constraints;
+ 
+-	status = variable_checker_set_constraints(
+-		&constraints,
+-		info->is_constraints_set,
+-		&property->VariableProperty);
++		status = variable_checker_set_constraints(
++			&constraints,
++			false,
++			&property->VariableProperty);
+ 
+-	if (status == EFI_SUCCESS) {
++		if (status == EFI_SUCCESS) {
+ 
+-		variable_index_set_constraints(info, &constraints);
+-	}
++			info = variable_index_add_constraints(
++				&context->variable_index,
++				&property->Guid,
++				property->NameSize,
++				property->Name,
++				&constraints);
+ 
+-	variable_index_remove_unused_entry(&context->variable_index, info);
++			if (!info) status = EFI_OUT_OF_RESOURCES;
++		}
++	}
+ 
+ 	return status;
+ }
+@@ -496,8 +488,7 @@ static efi_status_t check_capabilities(
+ 	if (var->Attributes & ~(
+ 		EFI_VARIABLE_NON_VOLATILE |
+ 		EFI_VARIABLE_BOOTSERVICE_ACCESS |
+-		EFI_VARIABLE_RUNTIME_ACCESS |
+-		EFI_VARIABLE_APPEND_WRITE)) {
++		EFI_VARIABLE_RUNTIME_ACCESS)) {
+ 
+ 		/* An unsupported attribute has been requested */
+ 		status = EFI_UNSUPPORTED;
+@@ -543,6 +534,17 @@ static efi_status_t check_access_permitted_on_set(
+ 			var->DataSize);
+ 	}
+ 
++	if ((status == EFI_SUCCESS) && var->DataSize) {
++
++		/* Restrict which attributes can be modified for an existing variable */
++		if ((var->Attributes & EFI_VARIABLE_NON_VOLATILE) !=
++			(info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE)) {
++
++			/* Don't permit change of storage class */
++			status = EFI_INVALID_PARAMETER;
++		}
++	}
++
+ 	return status;
+ }
+ 
+@@ -562,33 +564,20 @@ static efi_status_t store_variable_data(
+ 
+ 	if (delegate_store->storage_backend) {
+ 
+-		if (!(var->Attributes & EFI_VARIABLE_APPEND_WRITE)) {
+-
+-			/* Create or overwrite variable data */
+-			psa_status = store_overwrite(
+-				delegate_store,
+-				context->owner_id,
+-				info->metadata.uid,
+-				data_len,
+-				data);
+-		}
+-		else {
+-
+-			/* Append new data to existing variable data */
+-			psa_status = store_append_write(
+-				delegate_store,
+-				context->owner_id,
+-				info->metadata.uid,
+-				data_len,
+-				data);
+-		}
++		psa_status = delegate_store->storage_backend->interface->set(
++			delegate_store->storage_backend->context,
++			context->owner_id,
++			info->metadata.uid,
++			data_len,
++			data,
++			PSA_STORAGE_FLAG_NONE);
+ 	}
+ 
+ 	if ((psa_status != PSA_SUCCESS) && delegate_store->is_nv) {
+ 
+ 		/* A storage failure has occurred so attempt to fix any
+-		 * mismatch between the variable index and stored NV variables.
+-		 */
++		* mismatch between the variable index and stored NV variables.
++		*/
+ 		purge_orphan_index_entries(context);
+ 	}
+ 
+@@ -651,100 +640,6 @@ static efi_status_t load_variable_data(
+ 	return psa_to_efi_storage_status(psa_status);
+ }
+ 
+-static psa_status_t store_overwrite(
+-	struct delegate_variable_store *delegate_store,
+-	uint32_t client_id,
+-	uint64_t uid,
+-	size_t data_length,
+-	const void *data)
+-{
+-	/* Police maximum variable size limit */
+-	if (data_length > delegate_store->max_variable_size) return PSA_ERROR_INVALID_ARGUMENT;
+-
+-	psa_status_t psa_status = delegate_store->storage_backend->interface->set(
+-		delegate_store->storage_backend->context,
+-		client_id,
+-		uid,
+-		data_length,
+-		data,
+-		PSA_STORAGE_FLAG_NONE);
+-
+-	return psa_status;
+-}
+-
+-static psa_status_t store_append_write(
+-	struct delegate_variable_store *delegate_store,
+-	uint32_t client_id,
+-	uint64_t uid,
+-	size_t data_length,
+-	const void *data)
+-{
+-	struct psa_storage_info_t storage_info;
+-
+-	if (data_length == 0) return PSA_SUCCESS;
+-
+-	psa_status_t psa_status = delegate_store->storage_backend->interface->get_info(
+-		delegate_store->storage_backend->context,
+-		client_id,
+-		uid,
+-		&storage_info);
+-
+-	if (psa_status != PSA_SUCCESS) return psa_status;
+-
+-	/* Determine size of appended variable */
+-	size_t new_size = storage_info.size + data_length;
+-
+-	/* Defend against integer overflow */
+-	if (new_size < storage_info.size) return PSA_ERROR_INVALID_ARGUMENT;
+-
+-		/* Police maximum variable size limit */
+-	if (new_size > delegate_store->max_variable_size) return PSA_ERROR_INVALID_ARGUMENT;
+-
+-	/* Storage backend doesn't support an append operation so we need
+-	 * need to read the current variable data, extend it and write it back.
+-	 */
+-	uint8_t *rw_buf = malloc(new_size);
+-	if (!rw_buf) return PSA_ERROR_INSUFFICIENT_MEMORY;
+-
+-	size_t old_size = 0;
+-	psa_status = delegate_store->storage_backend->interface->get(
+-		delegate_store->storage_backend->context,
+-		client_id,
+-		uid,
+-		0,
+-		new_size,
+-		rw_buf,
+-		&old_size);
+-
+-	if (psa_status == PSA_SUCCESS) {
+-
+-		if ((old_size + data_length) <= new_size) {
+-
+-			/* Extend the variable data */
+-			memcpy(&rw_buf[old_size], data, data_length);
+-
+-			psa_status = delegate_store->storage_backend->interface->set(
+-				delegate_store->storage_backend->context,
+-				client_id,
+-				uid,
+-				old_size + data_length,
+-				rw_buf,
+-				storage_info.flags);
+-		}
+-		else {
+-
+-			/* There's a mismatch between the length obtained from
+-			 * get_info() and the subsequent length returned by get().
+-			 */
+-			psa_status = PSA_ERROR_STORAGE_FAILURE;
+-		}
+-	}
+-
+-	free(rw_buf);
+-
+-	return psa_status;
+-}
+-
+ static void purge_orphan_index_entries(
+ 	struct uefi_variable_store *context)
+ {
+@@ -759,7 +654,7 @@ static void purge_orphan_index_entries(
+ 	 */
+ 	while (!variable_index_iterator_is_done(&iter)) {
+ 
+-		struct variable_info *info = variable_index_iterator_current(&iter);
++		const struct variable_info *info = variable_index_iterator_current(&iter);
+ 
+ 		if (info->is_variable_set && (info->metadata.attributes & EFI_VARIABLE_NON_VOLATILE)) {
+ 
+@@ -775,7 +670,7 @@ static void purge_orphan_index_entries(
+ 			if (psa_status != PSA_SUCCESS) {
+ 
+ 				/* Detected a mismatch between the index and storage */
+-				variable_index_clear_variable(&context->variable_index, info);
++				variable_index_remove_variable(&context->variable_index, info);
+ 				any_orphans = true;
+ 			}
+ 		}
+diff --git a/components/service/smm_variable/backend/variable_index.c b/components/service/smm_variable/backend/variable_index.c
+index a8a5575..99d7c97 100644
+--- a/components/service/smm_variable/backend/variable_index.c
++++ b/components/service/smm_variable/backend/variable_index.c
+@@ -132,13 +132,13 @@ size_t variable_index_max_dump_size(
+ 	return sizeof(struct variable_metadata) * context->max_variables;
+ }
+ 
+-struct variable_info *variable_index_find(
+-	struct variable_index *context,
++const struct variable_info *variable_index_find(
++	const struct variable_index *context,
+ 	const EFI_GUID *guid,
+ 	size_t name_size,
+ 	const int16_t *name)
+ {
+-	struct variable_info *result = NULL;
++	const struct variable_info *result = NULL;
+ 	int pos = find_variable(context, guid, name_size, name);
+ 
+ 	if (pos >= 0) {
+@@ -149,13 +149,13 @@ struct variable_info *variable_index_find(
+ 	return result;
+ }
+ 
+-struct variable_info *variable_index_find_next(
++const struct variable_info *variable_index_find_next(
+ 	const struct variable_index *context,
+ 	const EFI_GUID *guid,
+ 	size_t name_size,
+ 	const int16_t *name)
+ {
+-	struct variable_info *result = NULL;
++	const struct variable_info *result = NULL;
+ 
+ 	if (name_size >= sizeof(int16_t)) {
+ 
+@@ -263,11 +263,12 @@ static struct variable_entry *add_entry(
+ 	return entry;
+ }
+ 
+-struct variable_info *variable_index_add_entry(
++const struct variable_info *variable_index_add_variable(
+ 	struct variable_index *context,
+ 	const EFI_GUID *guid,
+ 	size_t name_size,
+-	const int16_t *name)
++	const int16_t *name,
++	uint32_t attributes)
+ {
+ 	struct variable_info *info = NULL;
+ 	struct variable_entry *entry = add_entry(context, guid, name_size, name);
+@@ -275,41 +276,40 @@ struct variable_info *variable_index_add_entry(
+ 	if (entry) {
+ 
+ 		info = &entry->info;
++
++		info->metadata.attributes = attributes;
++		info->is_variable_set = true;
++
++		mark_dirty(entry);
+ 	}
+ 
+ 	return info;
+ }
+ 
+-void variable_index_remove_unused_entry(
++const struct variable_info *variable_index_add_constraints(
+ 	struct variable_index *context,
+-	struct variable_info *info)
++	const EFI_GUID *guid,
++	size_t name_size,
++	const int16_t *name,
++	const struct variable_constraints *constraints)
+ {
+-	if (info &&
+-		!info->is_constraints_set &&
+-		!info->is_variable_set) {
+-
+-		struct variable_entry *entry = containing_entry(info);
+-		entry->in_use = false;
++	struct variable_info *info = NULL;
++	struct variable_entry *entry = add_entry(context, guid, name_size, name);
+ 
+-		memset(info, 0, sizeof(struct variable_info));
+-	}
+-}
++	if (entry) {
+ 
+-void variable_index_set_variable(
+-	struct variable_info *info,
+-	uint32_t attributes)
+-{
+-	struct variable_entry *entry = containing_entry(info);
++		info = &entry->info;
+ 
+-	info->metadata.attributes = attributes;
+-	info->is_variable_set = true;
++		info->check_constraints = *constraints;
++		info->is_constraints_set = true;
++	}
+ 
+-	mark_dirty(entry);
++	return info;
+ }
+ 
+-void variable_index_clear_variable(
++void variable_index_remove_variable(
+ 	struct variable_index *context,
+-	struct variable_info *info)
++	const struct variable_info *info)
+ {
+ 	if (info) {
+ 
+@@ -318,17 +318,48 @@ void variable_index_clear_variable(
+ 
+ 		/* Mark variable as no longer set */
+ 		entry->info.is_variable_set = false;
++
++		/* Entry may still be needed if check constraints were set */
++		entry->in_use = info->is_constraints_set;
++
++		if (!entry->in_use) {
++
++			/* Entry not needed so wipe */
++			memset(&entry->info, 0, sizeof(struct variable_info));
++		}
+ 	}
+ }
+ 
+-void variable_index_set_constraints(
+-	struct variable_info *info,
++void variable_index_update_variable(
++	const struct variable_info *info,
++	uint32_t attributes)
++{
++	if (info) {
++
++		struct variable_info *modified_info = (struct variable_info*)info;
++		struct variable_entry *entry = containing_entry(modified_info);
++
++		if (!modified_info->is_variable_set ||
++			(attributes != modified_info->metadata.attributes)) {
++
++			/* The update changes the variable_info state */
++			modified_info->is_variable_set = true;
++			modified_info->metadata.attributes = attributes;
++			mark_dirty(entry);
++		}
++	}
++}
++
++void variable_index_update_constraints(
++	const struct variable_info *info,
+ 	const struct variable_constraints *constraints)
+ {
+ 	if (info) {
+ 
+-		info->check_constraints = *constraints;
+-		info->is_constraints_set = true;
++		struct variable_info *modified_info = (struct variable_info*)info;
++
++		modified_info->check_constraints = *constraints;
++		modified_info->is_constraints_set = true;
+ 	}
+ }
+ 
+diff --git a/components/service/smm_variable/backend/variable_index.h b/components/service/smm_variable/backend/variable_index.h
+index 63f42ab..e109d0d 100644
+--- a/components/service/smm_variable/backend/variable_index.h
++++ b/components/service/smm_variable/backend/variable_index.h
+@@ -119,8 +119,8 @@ size_t variable_index_max_dump_size(
+  *
+  * @return     Pointer to variable_info or NULL
+  */
+-struct variable_info *variable_index_find(
+-	struct variable_index *context,
++const struct variable_info *variable_index_find(
++	const struct variable_index *context,
+ 	const EFI_GUID *guid,
+ 	size_t name_size,
+ 	const int16_t *name);
+@@ -135,76 +135,78 @@ struct variable_info *variable_index_find(
+  *
+  * @return     Pointer to variable_info or NULL
+  */
+-struct variable_info *variable_index_find_next(
++const struct variable_info *variable_index_find_next(
+ 	const struct variable_index *context,
+ 	const EFI_GUID *guid,
+ 	size_t name_size,
+ 	const int16_t *name);
+ 
+ /**
+- * @brief      Add a new entry to the index
+- *
+- * An entry is needed either when a new variable is created or
+- * when variable constraints are set for a variable that doesn't
+- * yet exist.
++ * @brief      Add a new variable to the index
+  *
+  * @param[in]  context variable_index
+  * @param[in]  guid The variable's guid
+  * @param[in]  name_size The name parameter's size
+  * @param[in]  name The variable's name
++ * @param[in]  attributes The variable's attributes
+  *
+  * @return     Pointer to variable_info or NULL
+  */
+-struct variable_info *variable_index_add_entry(
++const struct variable_info *variable_index_add_variable(
+ 	struct variable_index *context,
+ 	const EFI_GUID *guid,
+ 	size_t name_size,
+-	const int16_t *name);
++	const int16_t *name,
++	uint32_t attributes);
+ 
+ /**
+- * @brief      Remove an unused entry from the index
++ * @brief      Remove a variable from the index
+  *
+- * Removes an entry if it is not in use.
++ * Removes a variable from the index if it exists.
+  *
+  * @param[in]  context variable_index
+  * @param[in]  info The variable info corresponding to the entry to remove
+  */
+-void variable_index_remove_unused_entry(
++void variable_index_remove_variable(
+ 	struct variable_index *context,
+-	struct variable_info *info);
++	const struct variable_info *info);
+ 
+ /**
+- * @brief      Set a variable to the index
+- *
+- * An entry for the variable must already exist.
++ * @brief      Update a variable that's already in the index
+  *
+  * @param[in]  info variable info
+  * @param[in]  attributes The variable's attributes
+  */
+-void variable_index_set_variable(
+-	struct variable_info *info,
++void variable_index_update_variable(
++	const struct variable_info *info,
+ 	uint32_t attributes);
+ 
+ /**
+- * @brief      Clear a variable from the index
+- *
+- * Clears a variable from the index
++ * @brief      Add a new check constraints object to the index
+  *
+  * @param[in]  context variable_index
+- * @param[in]  info The variable info corresponding to the variable to clear
++ * @param[in]  guid The variable's guid
++ * @param[in]  name_size The name parameter's size
++ * @param[in]  name The variable's name
++ * @param[in]  constraints The check constraints
++ *
++ * @return     Pointer to variable_info or NULL
+  */
+-void variable_index_clear_variable(
++const struct variable_info *variable_index_add_constraints(
+ 	struct variable_index *context,
+-	struct variable_info *info);
++	const EFI_GUID *guid,
++	size_t name_size,
++	const int16_t *name,
++	const struct variable_constraints *constraints);
+ 
+ /**
+- * @brief      Set a check constraints object associated with a variavle
++ * @brief      Update variable constraints that are already in the index
+  *
+  * @param[in]  info variable info
+  * @param[in]  constraints The check constraints
+  */
+-void variable_index_set_constraints(
+-	struct variable_info *info,
++void variable_index_update_constraints(
++	const struct variable_info *info,
+ 	const struct variable_constraints *constraints);
+ 
+ /**
+diff --git a/components/service/smm_variable/backend/variable_index_iterator.c b/components/service/smm_variable/backend/variable_index_iterator.c
+index 8f8fc74..7cc6dc7 100644
+--- a/components/service/smm_variable/backend/variable_index_iterator.c
++++ b/components/service/smm_variable/backend/variable_index_iterator.c
+@@ -31,10 +31,10 @@ bool variable_index_iterator_is_done(
+ 	return iter->current_pos >= iter->variable_index->max_variables;
+ }
+ 
+-struct variable_info *variable_index_iterator_current(
++const struct variable_info *variable_index_iterator_current(
+ 	const struct variable_index_iterator *iter)
+ {
+-	struct variable_info *current = NULL;
++	const struct variable_info *current = NULL;
+ 
+ 	if (!variable_index_iterator_is_done(iter)) {
+ 
+diff --git a/components/service/smm_variable/backend/variable_index_iterator.h b/components/service/smm_variable/backend/variable_index_iterator.h
+index 7ff77c5..f64a2c4 100644
+--- a/components/service/smm_variable/backend/variable_index_iterator.h
++++ b/components/service/smm_variable/backend/variable_index_iterator.h
+@@ -54,7 +54,7 @@ bool variable_index_iterator_is_done(
+  *
+  * @return     Pointer to variable_info or NULL
+  */
+-struct variable_info *variable_index_iterator_current(
++const struct variable_info *variable_index_iterator_current(
+ 	const struct variable_index_iterator *iter);
+ 
+ /**
+diff --git a/components/service/smm_variable/test/service/smm_variable_service_tests.cpp b/components/service/smm_variable/test/service/smm_variable_service_tests.cpp
+index 15556e9..38c08eb 100644
+--- a/components/service/smm_variable/test/service/smm_variable_service_tests.cpp
++++ b/components/service/smm_variable/test/service/smm_variable_service_tests.cpp
+@@ -249,30 +249,6 @@ TEST(SmmVariableServiceTests, setAndGet)
+ 	UNSIGNED_LONGS_EQUAL(set_data.size(), get_data.size());
+ 	LONGS_EQUAL(0, get_data.compare(set_data));
+ 
+-	/* Extend the variable using an append write */
+-	std::string append_data = " values added with append write";
+-
+-	efi_status = m_client->set_variable(
+-		m_common_guid,
+-		var_name,
+-		append_data,
+-		EFI_VARIABLE_APPEND_WRITE);
+-
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+-	efi_status = m_client->get_variable(
+-		m_common_guid,
+-		var_name,
+-		get_data);
+-
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+-	std::string appended_data = set_data + append_data;
+-
+-	/* Expect the append write operation to have extended the variable */
+-	UNSIGNED_LONGLONGS_EQUAL(appended_data.size(), get_data.size());
+-	LONGS_EQUAL(0, appended_data.compare(get_data));
+-
+ 	/* Expect remove to be permitted */
+ 	efi_status = m_client->remove_variable(m_common_guid, var_name);
+ 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+@@ -303,30 +279,6 @@ TEST(SmmVariableServiceTests, setAndGetNv)
+ 	UNSIGNED_LONGS_EQUAL(set_data.size(), get_data.size());
+ 	LONGS_EQUAL(0, get_data.compare(set_data));
+ 
+-	/* Extend the variable using an append write */
+-	std::string append_data = " values added with append write";
+-
+-	efi_status = m_client->set_variable(
+-		m_common_guid,
+-		var_name,
+-		append_data,
+-		EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_APPEND_WRITE);
+-
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+-	efi_status = m_client->get_variable(
+-		m_common_guid,
+-		var_name,
+-		get_data);
+-
+-	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+-
+-	std::string appended_data = set_data + append_data;
+-
+-	/* Expect the append write operation to have extended the variable */
+-	UNSIGNED_LONGLONGS_EQUAL(appended_data.size(), get_data.size());
+-	LONGS_EQUAL(0, appended_data.compare(get_data));
+-
+ 	/* Expect remove to be permitted */
+ 	efi_status = m_client->remove_variable(m_common_guid, var_name);
+ 	UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status);
+diff --git a/protocols/service/smm_variable/parameters.h b/protocols/service/smm_variable/parameters.h
+index 233f301..1f795a9 100644
+--- a/protocols/service/smm_variable/parameters.h
++++ b/protocols/service/smm_variable/parameters.h
+@@ -47,9 +47,6 @@ typedef struct {
+ 	 EFI_VARIABLE_HARDWARE_ERROR_RECORD | \
+ 	 EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
+ 	 EFI_VARIABLE_APPEND_WRITE)
+-#define	EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS_MASK \
+-	(EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \
+-	 EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)
+ 
+ /**
+  * Parameter structure for SetVariable and GetVariable.
+-- 
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
index 4a18586..9693430 100644
--- a/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
+++ b/meta-arm-bsp/recipes-security/trusted-services/ts-corstone1000.inc
@@ -38,6 +38,7 @@ SRC_URI:append = " \
                   file://0025-Add-stub-capsule-update-service-components.patch \
                   file://0026-Add-logs-to-functions-in-SMM-gateway-SP.patch \
                   file://0027-Configure-storage-size.patch \
+                  file://0028-Revert-Add-uefi-variable-append-write-support.patch \
                   "
 
 SRC_URI_MBED = "git://github.com/ARMmbed/mbed-crypto.git;protocol=https;branch=development;name=mbed;destsuffix=git/mbedcrypto"
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] arm-bsp/u-boot: populate ESRT table with image_info
  2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
                   ` (2 preceding siblings ...)
  2021-12-20 14:14 ` [PATCH 3/4] arm-bsp/security: Revert set append write TS patch xueliang.zhong
@ 2021-12-20 14:14 ` xueliang.zhong
  2021-12-20 22:58 ` [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway Jon Mason
  4 siblings, 0 replies; 6+ messages in thread
From: xueliang.zhong @ 2021-12-20 14:14 UTC (permalink / raw)
  To: meta-arm, Ross.Burton; +Cc: nd, Vishnu Banavath

From: Vishnu Banavath <vishnu.banavath@arm.com>

These changes are to support populating corstone1000 image_info
to ESRT table

Change-Id: I6e5cdd8a3477fbf3c480bf7a725198841ed79796
Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
---
 ...config-enable-CAPSULE_FIRMWARE_RAW-c.patch |  31 +++++
 ...ate-ESRT-table-if-EFI_ESRT-config-op.patch |  38 ++++++
 ...-add-get_image_info-for-corstone1000.patch | 119 ++++++++++++++++++
 .../recipes-bsp/u-boot/u-boot_%.bbappend      |   3 +
 4 files changed, 191 insertions(+)
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0045-corstone1000-defconfig-enable-CAPSULE_FIRMWARE_RAW-c.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0046-efi_loader-populate-ESRT-table-if-EFI_ESRT-config-op.patch
 create mode 100644 meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0047-efi_firmware-add-get_image_info-for-corstone1000.patch

diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0045-corstone1000-defconfig-enable-CAPSULE_FIRMWARE_RAW-c.patch b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0045-corstone1000-defconfig-enable-CAPSULE_FIRMWARE_RAW-c.patch
new file mode 100644
index 0000000..f9e7914
--- /dev/null
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0045-corstone1000-defconfig-enable-CAPSULE_FIRMWARE_RAW-c.patch
@@ -0,0 +1,31 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
+
+From 4820222f24901fbed8e0a87487603c0e06dfff5a Mon Sep 17 00:00:00 2001
+From: Vishnu Banavath <vishnu.banavath@arm.com>
+Date: Fri, 17 Dec 2021 19:46:52 +0000
+Subject: [PATCH 1/3] corstone1000/defconfig: enable CAPSULE_FIRMWARE_RAW
+ config option
+
+This change is to enable CAPSULE_FIRMWARE_RAW config option as
+we use RAW image on corstone1000 target. Also, disable
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT
+
+Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
+---
+ configs/corstone1000_defconfig | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
+index e11ffbfc89..d576ee1b67 100644
+--- a/configs/corstone1000_defconfig
++++ b/configs/corstone1000_defconfig
+@@ -65,3 +65,5 @@ CONFIG_EFI_SET_TIME=y
+ CONFIG_RTC_EMULATION=y
+ CONFIG_PSCI_RESET=y
+ CONFIG_DISTRO_DEFAULTS=y
++CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
++# CONFIG_EFI_CAPSULE_FIRMWARE_FIT is not set
+-- 
+2.25.1
+
diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0046-efi_loader-populate-ESRT-table-if-EFI_ESRT-config-op.patch b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0046-efi_loader-populate-ESRT-table-if-EFI_ESRT-config-op.patch
new file mode 100644
index 0000000..f29104d
--- /dev/null
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0046-efi_loader-populate-ESRT-table-if-EFI_ESRT-config-op.patch
@@ -0,0 +1,38 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
+
+From b9126620d74aed1a0ddd286e4c70344626dd72c3 Mon Sep 17 00:00:00 2001
+From: Vishnu Banavath <vishnu.banavath@arm.com>
+Date: Fri, 17 Dec 2021 19:49:02 +0000
+Subject: [PATCH 2/3] efi_loader: populate ESRT table if EFI_ESRT config option
+ is set
+
+This change is to call efi_esrt_populate function if CONFIG_EFI_ESRT
+is set. This will populte esrt table with firmware image info
+
+Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
+---
+ lib/efi_loader/efi_capsule.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
+index ec89865c98..00b3b21105 100644
+--- a/lib/efi_loader/efi_capsule.c
++++ b/lib/efi_loader/efi_capsule.c
+@@ -591,6 +591,13 @@ efi_status_t __efi_runtime EFIAPI efi_update_capsule(
+ 			ret = EFI_SUCCESS;
+ 		}
+ 
++		if (IS_ENABLED(CONFIG_EFI_ESRT)) {
++			/* Rebuild the ESRT to reflect any updated FW images. */
++			ret = efi_esrt_populate();
++	               if (ret != EFI_SUCCESS)
++				log_warning("EFI Capsule: failed to update ESRT\n");
++	       }
++
+ 		goto out;
+ 
+ #endif
+-- 
+2.25.1
+
diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0047-efi_firmware-add-get_image_info-for-corstone1000.patch b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0047-efi_firmware-add-get_image_info-for-corstone1000.patch
new file mode 100644
index 0000000..b401585
--- /dev/null
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot/corstone1000/0047-efi_firmware-add-get_image_info-for-corstone1000.patch
@@ -0,0 +1,119 @@
+Upstream-Status: Pending [Not submitted to upstream yet]
+Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
+
+From 6a3f1d425321545869f00295d9173e3a3519a6c6 Mon Sep 17 00:00:00 2001
+From: Vishnu Banavath <vishnu.banavath@arm.com>
+Date: Fri, 17 Dec 2021 19:50:25 +0000
+Subject: [PATCH] efi_firmware: add get_image_info for corstone1000
+
+This change is to populate get_image_info which eventually
+will be populated in ESRT table
+
+Signed-off-by: Vishnu Banavath <vishnu.banavath@arm.com>
+
+%% original patch: 0047-efi_firmware-add-get_image_info-for-corstone1000.patch
+
+diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
+index a1b88dbfc2..85fb5254eb 100644
+--- a/lib/efi_loader/efi_firmware.c
++++ b/lib/efi_loader/efi_firmware.c
+@@ -238,6 +238,7 @@ const efi_guid_t efi_firmware_image_type_uboot_fit =
+  *
+  * Return		status code
+  */
++
+ static
+ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
+ 	struct efi_firmware_management_protocol *this,
+@@ -329,6 +330,56 @@ const struct efi_firmware_management_protocol efi_fmp_fit = {
+ const efi_guid_t efi_firmware_image_type_uboot_raw =
+ 	EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
+ 
++#if CONFIG_IS_ENABLED(TARGET_CORSTONE1000)
++static efi_status_t efi_corstone1000_img_info_get (
++	efi_uintn_t *image_info_size,
++	struct efi_firmware_image_descriptor *image_info,
++	u32 *descriptor_version,
++	u8 *descriptor_count,
++	efi_uintn_t *descriptor_size,
++	u32 *package_version,
++	u16 **package_version_name,
++	const efi_guid_t *image_type)
++{
++	int i = 0;
++
++	*image_info_size = sizeof(*image_info);
++	*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
++	*descriptor_count = 1;//dfu_num;
++	*descriptor_size = sizeof(*image_info);
++	if (package_version)
++		*package_version = 0xffffffff; /* not supported */
++	if(package_version_name)
++		*package_version_name = NULL; /* not supported */
++
++	if(image_info == NULL) {
++		log_warning("image_info is null\n");
++		return EFI_BUFFER_TOO_SMALL;
++	}
++
++	image_info[i].image_index = i;
++	image_info[i].image_type_id = *image_type;
++	image_info[i].image_id = 0;
++	image_info[i].image_id_name = "wic";
++	image_info[i].version = 1;
++	image_info[i].version_name = NULL;
++	image_info[i].size = 0x1000;
++	image_info[i].attributes_supported = IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
++					     IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
++	image_info[i].attributes_setting = IMAGE_ATTRIBUTE_IMAGE_UPDATABLE;
++	/* Check if the capsule authentication is enabled */
++	if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
++		image_info[0].attributes_setting |=
++			IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
++	image_info[i].lowest_supported_image_version = 0;
++	image_info[i].last_attempt_version = 0;
++	image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
++	image_info[i].hardware_instance = 1;
++	image_info[i].dependencies = NULL;
++
++	return EFI_SUCCESS;
++}
++#endif
+ /**
+  * efi_firmware_raw_get_image_info - return information about the current
+ 				     firmware image
+@@ -373,12 +424,20 @@ efi_status_t EFIAPI efi_firmware_raw_get_image_info(
+ 	     !descriptor_size || !package_version || !package_version_name))
+ 		return EFI_EXIT(EFI_INVALID_PARAMETER);
+ 
+-	ret = efi_get_dfu_info(image_info_size, image_info,
++#if CONFIG_IS_ENABLED(TARGET_CORSTONE1000)
++	ret = efi_corstone1000_img_info_get(image_info_size, image_info,
+ 			       descriptor_version, descriptor_count,
+ 			       descriptor_size,
+ 			       package_version, package_version_name,
+ 			       &efi_firmware_image_type_uboot_raw);
++#else
+ 
++	ret = efi_get_dfu_info(image_info_size, image_info,
++			       descriptor_version, descriptor_count,
++			       descriptor_size,
++			       package_version, package_version_name,
++			       &efi_firmware_image_type_uboot_raw);
++#endif
+ 	return EFI_EXIT(ret);
+ }
+ 
+@@ -459,6 +518,9 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
+ 
+ 	}
+ 
++#if CONFIG_IS_ENABLED(TARGET_CORSTONE1000)
++	return EFI_EXIT(EFI_SUCCESS);
++#endif
+ 	if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
+ 			     NULL, NULL))
+ 		return EFI_EXIT(EFI_DEVICE_ERROR);
+-- 
+2.17.1
+
diff --git a/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend b/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend
index 9d2db6a..08ce270 100644
--- a/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend
+++ b/meta-arm-bsp/recipes-bsp/u-boot/u-boot_%.bbappend
@@ -54,6 +54,9 @@ SRC_URI:append:corstone1000 = " \
       file://0042-Update-comm_buf-when-EFI_BUFFER_TOO_SMALL.patch \
       file://0043-Disable-DHCP-PING-config-parameters.patch \
       file://0044-Revert-corstone1000-Disable-set-get-of-NV-variables.patch \
+      file://0045-corstone1000-defconfig-enable-CAPSULE_FIRMWARE_RAW-c.patch \
+      file://0046-efi_loader-populate-ESRT-table-if-EFI_ESRT-config-op.patch \
+      file://0047-efi_firmware-add-get_image_info-for-corstone1000.patch \
       "
 
 #
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway
  2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
                   ` (3 preceding siblings ...)
  2021-12-20 14:14 ` [PATCH 4/4] arm-bsp/u-boot: populate ESRT table with image_info xueliang.zhong
@ 2021-12-20 22:58 ` Jon Mason
  4 siblings, 0 replies; 6+ messages in thread
From: Jon Mason @ 2021-12-20 22:58 UTC (permalink / raw)
  To: meta-arm, Ross.Burton, xueliang.zhong; +Cc: nd

On Mon, 20 Dec 2021 14:14:14 +0000, xueliang.zhong@arm.com wrote:
> From: Xueliang Zhong <xueliang.zhong@arm.com>
> 
> This patchset will add a macro to configure the volatile and
> non volatile storage in SMM gateway. Few useful logs are
> also added to the secure world.
> 
> Gowtham Suresh Kumar (3):
>   arm-bsp/secure-partitions: corstone1000: Configure storage in SMM
>     gateway
>   arm-bsp/u-boot: corstone1000: Fix SCT failures
>   arm-bsp/security: Revert set append write TS patch
> 
> [...]

Applied, thanks!

[1/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway
      commit: 5f69e4a51bf67383e808b39d15f94ca239afd870
[2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures
      commit: 9ac90c6b25ba8a91e7b59e5a7db20e7c4c8cdef6
[3/4] arm-bsp/security: Revert set append write TS patch
      commit: 5661a1d61437bd9fe653ea43d9b34fd8908d2ecf
[4/4] arm-bsp/u-boot: populate ESRT table with image_info
      commit: 7afe7d50f1778777c62880c75da6024cd4fd79be

Best regards,
-- 
Jon Mason <jon.mason@arm.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-20 22:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-20 14:14 [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway xueliang.zhong
2021-12-20 14:14 ` [PATCH 1/4] " xueliang.zhong
2021-12-20 14:14 ` [PATCH 2/4] arm-bsp/u-boot: corstone1000: Fix SCT failures xueliang.zhong
2021-12-20 14:14 ` [PATCH 3/4] arm-bsp/security: Revert set append write TS patch xueliang.zhong
2021-12-20 14:14 ` [PATCH 4/4] arm-bsp/u-boot: populate ESRT table with image_info xueliang.zhong
2021-12-20 22:58 ` [PATCH 0/4] arm-bsp/secure-partitions: corstone1000: Configure storage in SMM gateway Jon Mason

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.