SPM: Optimize stateless service logic
- Rename the variables to make more sense.
- Do not chain stateless services as they are never looked up.
- Fine-tune the bit definition of service flags.
Change-Id: Ie242ceefa0a9b43581d12963a92e59c6da3fa3af
Signed-off-by: Ken Liu <Ken.Liu@arm.com>
diff --git a/secure_fw/partitions/partition_load_info.template b/secure_fw/partitions/partition_load_info.template
index 94e5bed..16d9709 100644
--- a/secure_fw/partitions/partition_load_info.template
+++ b/secure_fw/partitions/partition_load_info.template
@@ -126,12 +126,12 @@
| SERVICE_FLAG_NS_ACCESSIBLE
{% endif %}
{% if manifest.psa_framework_version > 1.0 and service.connection_based is sameas false %}
- | SERVICE_FLAG_STATELESS
+ | SERVICE_FLAG_STATELESS | {{"%x"|format(service.stateless_handle_index)}}
{% endif %}
{% if service.version_policy %}
- | TFM_VERSION_POLICY_{{service.version_policy}},
+ | SERVICE_VERSION_POLICY_{{service.version_policy}},
{% else %}
- | TFM_VERSION_POLICY_STRICT,
+ | SERVICE_VERSION_POLICY_STRICT,
{% endif %}
{% if service.version %}
.version = {{service.version}},
diff --git a/secure_fw/partitions/tfm_service_list.inc.template b/secure_fw/partitions/tfm_service_list.inc.template
index ac0e74a..26acd16 100644
--- a/secure_fw/partitions/tfm_service_list.inc.template
+++ b/secure_fw/partitions/tfm_service_list.inc.template
@@ -46,17 +46,4 @@
{% endfor %}
};
-/* p_service field of tracking table will be populated in spm_init() */
-struct stateless_service_tracking_t stateless_service_ref[] = {
- {% for service in stateless_services %}
- {{'{'}}
- {% if service is not none %}
- .sid = {{service.sid}},
- {% else %}
- .sid = 0,
- {% endif %}
- {{'}'}},
- {% endfor %}
-};
-
#endif /* __TFM_SERVICE_LIST_INC__ */
diff --git a/secure_fw/spm/cmsis_psa/spm_ipc.c b/secure_fw/spm/cmsis_psa/spm_ipc.c
index e812d2c..ed3fe0d 100644
--- a/secure_fw/spm/cmsis_psa/spm_ipc.c
+++ b/secure_fw/spm/cmsis_psa/spm_ipc.c
@@ -37,7 +37,8 @@
#include "load/spm_load_api.h"
extern struct spm_partition_db_t g_spm_partition_db;
-static struct service_t *all_services;
+static struct service_t *connection_services_listhead;
+struct service_t *stateless_services_ref_tbl[STATIC_HANDLE_NUM_LIMIT];
/* Pools */
TFM_POOL_DECLARE(conn_handle_pool, sizeof(struct tfm_conn_handle_t),
@@ -343,12 +344,12 @@
struct service_t *tfm_spm_get_service_by_sid(uint32_t sid)
{
- struct service_t *p_serv = all_services;
+ struct service_t *p_serv = connection_services_listhead;
while (p_serv && p_serv->p_ldinf->sid != sid) {
p_serv = TO_CONTAINER(BI_LIST_NEXT_NODE(&p_serv->list),
struct service_t, list);
- if (p_serv == all_services)
+ if (p_serv == connection_services_listhead)
return NULL;
}
@@ -390,12 +391,12 @@
TFM_CORE_ASSERT(service);
switch (SERVICE_GET_VERSION_POLICY(service->p_ldinf->flags)) {
- case TFM_VERSION_POLICY_RELAXED:
+ case SERVICE_VERSION_POLICY_RELAXED:
if (version > service->p_ldinf->version) {
return SPM_ERROR_VERSION;
}
break;
- case TFM_VERSION_POLICY_STRICT:
+ case SERVICE_VERSION_POLICY_STRICT:
if (version != service->p_ldinf->version) {
return SPM_ERROR_VERSION;
}
@@ -663,7 +664,9 @@
break;
}
- load_services_assuredly(partition, &all_services);
+ load_services_assuredly(partition, &connection_services_listhead,
+ stateless_services_ref_tbl,
+ sizeof(stateless_services_ref_tbl));
p_cmninf = partition->p_ldinf;
diff --git a/secure_fw/spm/cmsis_psa/spm_ipc.h b/secure_fw/spm/cmsis_psa/spm_ipc.h
index b193b81..37e6f2d 100644
--- a/secure_fw/spm/cmsis_psa/spm_ipc.h
+++ b/secure_fw/spm/cmsis_psa/spm_ipc.h
@@ -102,16 +102,10 @@
/* RoT Service data */
struct service_t {
- const struct service_load_info_t *p_ldinf; /* Service static pointer */
- struct partition_t *partition; /* Owner of the service */
- struct bi_list_node_t handle_list; /* Service handle list */
- struct bi_list_node_t list; /* For list operation */
-};
-
-/* Stateless RoT service tracking array item type. Indexed by static handle */
-struct stateless_service_tracking_t {
- uint32_t sid; /* Service ID */
- struct service_t *p_service; /* Service instance */
+ const struct service_load_info_t *p_ldinf; /* Service load info */
+ struct partition_t *partition; /* Owner of the service */
+ struct bi_list_node_t handle_list; /* Service handle list */
+ struct bi_list_node_t list; /* For list operation */
};
/* RoT connection handle list */
diff --git a/secure_fw/spm/cmsis_psa/static_load.c b/secure_fw/spm/cmsis_psa/static_load.c
index e8316c3..8972603 100644
--- a/secure_fw/spm/cmsis_psa/static_load.c
+++ b/secure_fw/spm/cmsis_psa/static_load.c
@@ -104,14 +104,16 @@
}
void load_services_assuredly(struct partition_t *p_partition,
- struct service_t **list_head)
+ struct service_t **connection_services_listhead,
+ struct service_t **stateless_service_ref_tbl,
+ size_t ref_tbl_size)
{
- uint32_t i, j;
+ uint32_t i, serv_ldflags, hidx;
struct service_t *services;
const struct partition_load_info_t *p_ptldinf;
const struct service_load_info_t *p_servldinf;
- if (p_partition == NULL) {
+ if (!p_partition || !connection_services_listhead) {
tfm_core_panic();
}
@@ -128,28 +130,36 @@
services[i].p_ldinf = &p_servldinf[i];
services[i].partition = p_partition;
- /* Populate the p_service of stateless_service_ref[] */
- if (SERVICE_IS_STATELESS(p_servldinf[i].flags)) {
- for (j = 0; j < STATIC_HANDLE_NUM_LIMIT; j++) {
- if (stateless_service_ref[j].sid == p_servldinf[i].sid) {
- stateless_service_ref[j].p_service = &services[i];
- break;
- }
- }
- /* Stateless service not found in tracking table */
- if (j >= STATIC_HANDLE_NUM_LIMIT) {
- tfm_core_panic();
- }
- }
BI_LIST_INIT_NODE(&services[i].handle_list);
BI_LIST_INIT_NODE(&services[i].list);
- if (list_head) {
- if (*list_head) {
- BI_LIST_INSERT_AFTER(&(*list_head)->list, &services[i].list);
- } else {
- *list_head = &services[i];
+ /* Populate the stateless service reference table */
+ serv_ldflags = p_servldinf[i].flags;
+ if (SERVICE_IS_STATELESS(serv_ldflags)) {
+ if ((stateless_service_ref_tbl == NULL) ||
+ (ref_tbl_size == 0) ||
+ (ref_tbl_size !=
+ STATIC_HANDLE_NUM_LIMIT * sizeof(struct service_t *))) {
+ tfm_core_panic();
}
+
+ hidx = SERVICE_GET_STATELESS_HINDEX(serv_ldflags);
+
+ if ((hidx >= STATIC_HANDLE_NUM_LIMIT) ||
+ stateless_service_ref_tbl[hidx]) {
+ tfm_core_panic();
+ }
+ stateless_service_ref_tbl[hidx] = &services[i];
+
+ /* Skip chaining stateless services as they won't be looked-up. */
+ continue;
+ }
+
+ if (*connection_services_listhead) {
+ BI_LIST_INSERT_AFTER(&(*connection_services_listhead)->list,
+ &services[i].list);
+ } else {
+ *connection_services_listhead = &services[i];
}
}
}
diff --git a/secure_fw/spm/ffm/spm_psa_client_call.c b/secure_fw/spm/ffm/spm_psa_client_call.c
index 805e9f3..ad76b9e 100644
--- a/secure_fw/spm/ffm/spm_psa_client_call.c
+++ b/secure_fw/spm/ffm/spm_psa_client_call.c
@@ -16,10 +16,8 @@
#include "tfm_nspm.h"
#include "ffm/spm_error_base.h"
-#define GET_STATELESS_SERVICE(index) (stateless_service_ref[index].p_service)
-#define GET_STATELESS_SID(index) (stateless_service_ref[index].sid)
-
-extern struct stateless_service_tracking_t stateless_service_ref[];
+#define GET_STATELESS_SERVICE(index) (stateless_services_ref_tbl[index])
+extern const struct service_t *stateless_services_ref_tbl[];
uint32_t tfm_spm_client_psa_framework_version(void)
{
@@ -160,7 +158,7 @@
}
service = GET_STATELESS_SERVICE(index);
- sid = GET_STATELESS_SID(index);
+ sid = service->p_ldinf->sid;
/*
* It is a PROGRAMMER ERROR if the caller is not authorized to access
diff --git a/secure_fw/spm/include/load/service_defs.h b/secure_fw/spm/include/load/service_defs.h
index 3692846..0dac420 100644
--- a/secure_fw/spm/include/load/service_defs.h
+++ b/secure_fw/spm/include/load/service_defs.h
@@ -12,24 +12,28 @@
#include "psa/service.h"
/*
- * Service static data - flags
- * bit 7-0: version policy
+ * Service load data - flags
+ * bit 7-0: stateless handle index
* bit 8: 1 - NS accessible, 0 - NS not accessible
* bit 9: 1 - stateless, 0 - connection-based
+ * bit 10: 1 - strict version policy, 0 - relaxed version policy
*/
+#define SERVICE_FLAG_STATELESS_HINDEX_MASK (0xFF)
#define SERVICE_FLAG_NS_ACCESSIBLE (1U << 8)
#define SERVICE_FLAG_STATELESS (1U << 9)
-#define TFM_VERSION_POLICY_RELAXED (0x0)
-#define TFM_VERSION_POLICY_STRICT (0x1)
-#define SERVICE_VERSION_POLICY_MASK (0xFF)
+#define SERVICE_FLAG_VERSION_POLICY_BIT (1U << 10)
+#define SERVICE_VERSION_POLICY_RELAXED (0U << 10)
+#define SERVICE_VERSION_POLICY_STRICT (1U << 10)
+#define SERVICE_GET_STATELESS_HINDEX(flag) \
+ ((flag) & SERVICE_FLAG_STATELESS_HINDEX_MASK)
#define SERVICE_IS_NS_ACCESSIBLE(flag) \
((flag) & SERVICE_FLAG_NS_ACCESSIBLE)
#define SERVICE_IS_STATELESS(flag) \
((flag) & SERVICE_FLAG_STATELESS)
#define SERVICE_GET_VERSION_POLICY(flag) \
- ((flag) & SERVICE_VERSION_POLICY_MASK)
+ ((flag) & SERVICE_FLAG_VERSION_POLICY_BIT)
#define STRID_TO_STRING_PTR(strid) (const char *)(strid)
#define STRING_PTR_TO_STRID(str) (uintptr_t)(str)
diff --git a/secure_fw/spm/include/load/spm_load_api.h b/secure_fw/spm/include/load/spm_load_api.h
index a047608..d698fd7 100644
--- a/secure_fw/spm/include/load/spm_load_api.h
+++ b/secure_fw/spm/include/load/spm_load_api.h
@@ -44,11 +44,14 @@
/*
* Allocated numbers of service objects based on given partition.
- * Link services with 'list_head' if it is provided.
- * An 'assuredly' function, errors simply panic the system and never
+ * Link services with 'list_head' if it is provided. It also needs the
+ * stateless service reference table and whole table size for loading.
+ * As an 'assuredly' function, errors simply panic the system and never
* return.
*/
void load_services_assuredly(struct partition_t *p_partition,
- struct service_t **list_head);
+ struct service_t **connection_services_listhead,
+ struct service_t **stateless_service_ref_tbl,
+ size_t ref_tbl_size);
#endif /* __SPM_LOAD_API_H__ */
diff --git a/secure_fw/spm/include/tfm_platform_core_api.h b/secure_fw/spm/include/tfm_platform_core_api.h
index 0f0d759..f79190b 100644
--- a/secure_fw/spm/include/tfm_platform_core_api.h
+++ b/secure_fw/spm/include/tfm_platform_core_api.h
@@ -26,7 +26,7 @@
/**
* \brief Return whether a secure partition is privileged.
*
- * \param[in] partition_idx The index of the partition in the partition_db.
+ * \param[in] partition_idx The index of the partition in the partition_list.
*
* \return True if the partition is privileged, false otherwise.
*/