From 52696946ab3f441496436ad7223cb2bd853c8beb Mon Sep 17 00:00:00 2001 From: Olivier Deprez Date: Thu, 16 Apr 2020 13:39:06 +0200 Subject: [PATCH 1/2] SPMD: code/comments cleanup As a follow-up to bdd2596d4, and related to SPM Dispatcher EL3 component and SPM Core S-EL2/S-EL1 component: update with cosmetic and coding rules changes. In addition: -Add Armv8.4-SecEL2 arch detection helper. -Add an SPMC context (on current core) get helper. -Return more meaningful error return codes. -Remove complexity in few spmd_smc_handler switch-cases. -Remove unused defines and structures from spmd_private.h Signed-off-by: Olivier Deprez Change-Id: I99e642450b0dafb19d3218a2f0e2d3107e8ca3fe --- include/arch/aarch64/arch_features.h | 8 +- include/plat/common/platform.h | 2 +- include/services/spm_core_manifest.h | 14 +- plat/common/plat_spmd_manifest.c | 105 +++++++------ services/std_svc/spmd/spmd_main.c | 221 ++++++++++++++------------- services/std_svc/spmd/spmd_private.h | 26 +--- 6 files changed, 194 insertions(+), 182 deletions(-) diff --git a/include/arch/aarch64/arch_features.h b/include/arch/aarch64/arch_features.h index 0491f48c6..9513e977d 100644 --- a/include/arch/aarch64/arch_features.h +++ b/include/arch/aarch64/arch_features.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Arm Limited. All rights reserved. + * Copyright (c) 2019-2020, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -52,4 +52,10 @@ static inline unsigned int get_armv8_5_mte_support(void) ID_AA64PFR1_EL1_MTE_MASK); } +static inline bool is_armv8_4_sel2_present(void) +{ + return ((read_id_aa64pfr0_el1() >> ID_AA64PFR0_SEL2_SHIFT) & + ID_AA64PFR0_SEL2_MASK) == 1ULL; +} + #endif /* ARCH_FEATURES_H */ diff --git a/include/plat/common/platform.h b/include/plat/common/platform.h index e4431d2f0..946a3b8b8 100644 --- a/include/plat/common/platform.h +++ b/include/plat/common/platform.h @@ -289,7 +289,7 @@ int plat_spm_sp_rd_load(struct sp_res_desc *rd, const void *ptr, size_t size); int plat_spm_sp_get_next_address(void **sp_base, size_t *sp_size, void **rd_base, size_t *rd_size); #if defined(SPD_spmd) -int plat_spm_core_manifest_load(spmc_manifest_sect_attribute_t *manifest, +int plat_spm_core_manifest_load(spmc_manifest_attribute_t *manifest, const void *ptr, size_t size); #endif diff --git a/include/services/spm_core_manifest.h b/include/services/spm_core_manifest.h index 71e6cfbe4..0c4363691 100644 --- a/include/services/spm_core_manifest.h +++ b/include/services/spm_core_manifest.h @@ -4,8 +4,8 @@ * SPDX-License-Identifier: BSD-3-Clause */ -#ifndef SPMC_MANIFEST_H -#define SPMC_MANIFEST_H +#ifndef SPM_CORE_MANIFEST_H +#define SPM_CORE_MANIFEST_H #include @@ -28,18 +28,18 @@ typedef struct spm_core_manifest_sect_attribute { uint32_t exec_state; /* - * Address of binary image containing SPM core in bytes (optional). + * Address of binary image containing SPM Core (optional). */ uint64_t load_address; /* * Offset from the base of the partition's binary image to the entry - * point of the partition. + * point of the partition (optional). */ uint64_t entrypoint; /* - * Size of binary image containing SPM core in bytes (mandatory). + * Size of binary image containing SPM Core in bytes (mandatory). */ uint32_t binary_size; @@ -48,6 +48,6 @@ typedef struct spm_core_manifest_sect_attribute { */ uint16_t spmc_id; -} spmc_manifest_sect_attribute_t; +} spmc_manifest_attribute_t; -#endif /* SPMC_MANIFEST_H */ +#endif /* SPM_CORE_MANIFEST_H */ diff --git a/plat/common/plat_spmd_manifest.c b/plat/common/plat_spmd_manifest.c index 8330356ae..a3e30e89c 100644 --- a/plat/common/plat_spmd_manifest.c +++ b/plat/common/plat_spmd_manifest.c @@ -5,65 +5,78 @@ */ #include +#include #include #include #include #include -#include #include #include +#define ATTRIBUTE_ROOT_NODE_STR "attribute" + /******************************************************************************* - * Attribute section handler + * SPMC attribute node parser ******************************************************************************/ -static int manifest_parse_attribute(spmc_manifest_sect_attribute_t *attr, +static int manifest_parse_attribute(spmc_manifest_attribute_t *attr, const void *fdt, int node) { uint32_t val32; - int rc = 0; + int rc; - assert(attr && fdt); + assert((attr != NULL) && (fdt != NULL)); rc = fdt_read_uint32(fdt, node, "maj_ver", &attr->major_version); - if (rc) { - ERROR("Missing SPCI major version in SPM core manifest.\n"); - return -ENOENT; + if (rc != 0) { + ERROR("Missing SPCI %s version in SPM Core manifest.\n", + "major"); + return rc; } rc = fdt_read_uint32(fdt, node, "min_ver", &attr->minor_version); - if (rc) { - ERROR("Missing SPCI minor version in SPM core manifest.\n"); - return -ENOENT; + if (rc != 0) { + ERROR("Missing SPCI %s version in SPM Core manifest.\n", + "minor"); + return rc; } rc = fdt_read_uint32(fdt, node, "spmc_id", &val32); - if (rc) { + if (rc != 0) { ERROR("Missing SPMC ID in manifest.\n"); - return -ENOENT; + return rc; } - attr->spmc_id = val32; + + attr->spmc_id = val32 & 0xffff; rc = fdt_read_uint32(fdt, node, "exec_state", &attr->exec_state); - if (rc) - NOTICE("Execution state not specified in SPM core manifest.\n"); + if (rc != 0) { + NOTICE("%s not specified in SPM Core manifest.\n", + "Execution state"); + } rc = fdt_read_uint32(fdt, node, "binary_size", &attr->binary_size); - if (rc) - NOTICE("Binary size not specified in SPM core manifest.\n"); + if (rc != 0) { + NOTICE("%s not specified in SPM Core manifest.\n", + "Binary size"); + } rc = fdt_read_uint64(fdt, node, "load_address", &attr->load_address); - if (rc) - NOTICE("Load address not specified in SPM core manifest.\n"); + if (rc != 0) { + NOTICE("%s not specified in SPM Core manifest.\n", + "Load address"); + } rc = fdt_read_uint64(fdt, node, "entrypoint", &attr->entrypoint); - if (rc) - NOTICE("Entrypoint not specified in SPM core manifest.\n"); + if (rc != 0) { + NOTICE("%s not specified in SPM Core manifest.\n", + "Entry point"); + } - VERBOSE("SPM core manifest attribute section:\n"); - VERBOSE(" version: %x.%x\n", attr->major_version, attr->minor_version); - VERBOSE(" spmc_id: %x\n", attr->spmc_id); + VERBOSE("SPM Core manifest attribute section:\n"); + VERBOSE(" version: %u.%u\n", attr->major_version, attr->minor_version); + VERBOSE(" spmc_id: 0x%x\n", attr->spmc_id); VERBOSE(" binary_size: 0x%x\n", attr->binary_size); VERBOSE(" load_address: 0x%llx\n", attr->load_address); VERBOSE(" entrypoint: 0x%llx\n", attr->entrypoint); @@ -74,53 +87,51 @@ static int manifest_parse_attribute(spmc_manifest_sect_attribute_t *attr, /******************************************************************************* * Root node handler ******************************************************************************/ -static int manifest_parse_root(spmc_manifest_sect_attribute_t *manifest, - const void *fdt, - int root) +static int manifest_parse_root(spmc_manifest_attribute_t *manifest, + const void *fdt, + int root) { int node; - char *str; - str = "attribute"; - node = fdt_subnode_offset_namelen(fdt, root, str, strlen(str)); + assert(manifest != NULL); + + node = fdt_subnode_offset_namelen(fdt, root, ATTRIBUTE_ROOT_NODE_STR, + sizeof(ATTRIBUTE_ROOT_NODE_STR) - 1); if (node < 0) { - ERROR("Root node doesn't contain subnode '%s'\n", str); - return -ENOENT; + ERROR("Root node doesn't contain subnode '%s'\n", + ATTRIBUTE_ROOT_NODE_STR); + return node; } return manifest_parse_attribute(manifest, fdt, node); } /******************************************************************************* - * Platform handler to parse a SPM core manifest. + * Platform handler to parse a SPM Core manifest. ******************************************************************************/ -int plat_spm_core_manifest_load(spmc_manifest_sect_attribute_t *manifest, +int plat_spm_core_manifest_load(spmc_manifest_attribute_t *manifest, const void *ptr, size_t size) { int rc; - int root_node; assert(manifest != NULL); assert(ptr != NULL); - INFO("Reading SPM core manifest at address %p\n", ptr); + INFO("Reading SPM Core manifest at address %p\n", ptr); rc = fdt_check_header(ptr); if (rc != 0) { - ERROR("Wrong format for SPM core manifest (%d).\n", rc); - return -EINVAL; + ERROR("Wrong format for SPM Core manifest (%d).\n", rc); + return rc; } - INFO("Reading SPM core manifest at address %p\n", ptr); - - root_node = fdt_node_offset_by_compatible(ptr, -1, + rc = fdt_node_offset_by_compatible(ptr, -1, "arm,spci-core-manifest-1.0"); - if (root_node < 0) { - ERROR("Unrecognized SPM core manifest\n"); - return -ENOENT; + if (rc < 0) { + ERROR("Unrecognized SPM Core manifest\n"); + return rc; } - INFO("Reading SPM core manifest at address %p\n", ptr); - return manifest_parse_root(manifest, ptr, root_node); + return manifest_parse_root(manifest, ptr, rc); } diff --git a/services/std_svc/spmd/spmd_main.c b/services/std_svc/spmd/spmd_main.c index a3e1a2d57..f6dbb97e3 100644 --- a/services/std_svc/spmd/spmd_main.c +++ b/services/std_svc/spmd/spmd_main.c @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -28,12 +29,12 @@ /******************************************************************************* * SPM Core context information. ******************************************************************************/ -spmd_spm_core_context_t spm_core_context[PLATFORM_CORE_COUNT]; +static spmd_spm_core_context_t spm_core_context[PLATFORM_CORE_COUNT]; /******************************************************************************* * SPM Core attribute information read from its manifest. ******************************************************************************/ -static spmc_manifest_sect_attribute_t spmc_attrs; +static spmc_manifest_attribute_t spmc_attrs; /******************************************************************************* * SPM Core entry point information. Discovered on the primary core and reused @@ -42,18 +43,34 @@ static spmc_manifest_sect_attribute_t spmc_attrs; static entry_point_info_t *spmc_ep_info; /******************************************************************************* - * Static function declaration. + * SPM Core context on current CPU get helper. ******************************************************************************/ -static int32_t spmd_init(void); -static int spmd_spmc_init(void *rd_base, size_t rd_size); -static uint64_t spmd_spci_error_return(void *handle, int error_code); -static uint64_t spmd_smc_forward(uint32_t smc_fid, bool secure_origin, - uint64_t x1, uint64_t x2, uint64_t x3, - uint64_t x4, void *handle); +spmd_spm_core_context_t *spmd_get_context(void) +{ + unsigned int linear_id = plat_my_core_pos(); + + return &spm_core_context[linear_id]; +} /******************************************************************************* - * This function takes an SP context pointer and performs a synchronous entry - * into it. + * Static function declaration. + ******************************************************************************/ +static int32_t spmd_init(void); +static int spmd_spmc_init(void *rd_base, + size_t rd_size); +static uint64_t spmd_spci_error_return(void *handle, + int error_code); +static uint64_t spmd_smc_forward(uint32_t smc_fid, + bool secure_origin, + uint64_t x1, + uint64_t x2, + uint64_t x3, + uint64_t x4, + void *handle); + +/******************************************************************************* + * This function takes an SPMC context pointer and performs a synchronous + * SPMC entry. ******************************************************************************/ uint64_t spmd_spm_core_sync_entry(spmd_spm_core_context_t *spmc_ctx) { @@ -83,14 +100,14 @@ uint64_t spmd_spm_core_sync_entry(spmd_spm_core_context_t *spmc_ctx) } /******************************************************************************* - * This function returns to the place where spm_sp_synchronous_entry() was + * This function returns to the place where spmd_spm_core_sync_entry() was * called originally. ******************************************************************************/ __dead2 void spmd_spm_core_sync_exit(uint64_t rc) { - spmd_spm_core_context_t *ctx = &spm_core_context[plat_my_core_pos()]; + spmd_spm_core_context_t *ctx = spmd_get_context(); - /* Get context of the SP in use by this CPU. */ + /* Get current CPU context from SPMC context */ assert(cm_get_context(SECURE) == &(ctx->cpu_ctx)); /* @@ -104,110 +121,98 @@ __dead2 void spmd_spm_core_sync_exit(uint64_t rc) } /******************************************************************************* - * Jump to the SPM core for the first time. + * Jump to the SPM Core for the first time. ******************************************************************************/ static int32_t spmd_init(void) { - uint64_t rc = 0; - spmd_spm_core_context_t *ctx = &spm_core_context[plat_my_core_pos()]; + spmd_spm_core_context_t *ctx = spmd_get_context(); + uint64_t rc; - INFO("SPM Core init start.\n"); + VERBOSE("SPM Core init start.\n"); ctx->state = SPMC_STATE_RESET; rc = spmd_spm_core_sync_entry(ctx); - if (rc) { + if (rc != 0ULL) { ERROR("SPMC initialisation failed 0x%llx\n", rc); - panic(); + return 0; } ctx->state = SPMC_STATE_IDLE; - INFO("SPM Core init end.\n"); + VERBOSE("SPM Core init end.\n"); return 1; } /******************************************************************************* - * Load SPMC manifest, init SPMC. + * Loads SPMC manifest and inits SPMC. ******************************************************************************/ static int spmd_spmc_init(void *rd_base, size_t rd_size) { - int rc; + spmd_spm_core_context_t *spm_ctx = spmd_get_context(); uint32_t ep_attr; - unsigned int linear_id = plat_my_core_pos(); - spmd_spm_core_context_t *spm_ctx = &spm_core_context[linear_id]; + int rc; - /* Load the SPM core manifest */ + /* Load the SPM Core manifest */ rc = plat_spm_core_manifest_load(&spmc_attrs, rd_base, rd_size); if (rc != 0) { - WARN("No or invalid SPM core manifest image provided by BL2 " - "boot loader. "); - return 1; + WARN("No or invalid SPM Core manifest image provided by BL2\n"); + return rc; } /* - * Ensure that the SPM core version is compatible with the SPM - * dispatcher version + * Ensure that the SPM Core version is compatible with the SPM + * Dispatcher version. */ if ((spmc_attrs.major_version != SPCI_VERSION_MAJOR) || (spmc_attrs.minor_version > SPCI_VERSION_MINOR)) { - WARN("Unsupported SPCI version (%x.%x) specified in SPM core " - "manifest image provided by BL2 boot loader.\n", + WARN("Unsupported SPCI version (%u.%u)\n", spmc_attrs.major_version, spmc_attrs.minor_version); - return 1; + return -EINVAL; } - INFO("SPCI version (%x.%x).\n", spmc_attrs.major_version, + VERBOSE("SPCI version (%u.%u).\n", spmc_attrs.major_version, spmc_attrs.minor_version); - INFO("SPM core run time EL%x.\n", + VERBOSE("SPM Core run time EL%x.\n", SPMD_SPM_AT_SEL2 ? MODE_EL2 : MODE_EL1); /* Validate the SPMC ID, Ensure high bit is set */ - if (!(spmc_attrs.spmc_id >> SPMC_SECURE_ID_SHIFT) & - SPMC_SECURE_ID_MASK) { - WARN("Invalid ID (0x%x) for SPMC.\n", - spmc_attrs.spmc_id); - return 1; + if (((spmc_attrs.spmc_id >> SPMC_SECURE_ID_SHIFT) & + SPMC_SECURE_ID_MASK) == 0U) { + WARN("Invalid ID (0x%x) for SPMC.\n", spmc_attrs.spmc_id); + return -EINVAL; } - INFO("SPMC ID %x.\n", spmc_attrs.spmc_id); - - /* Validate the SPM core execution state */ + /* Validate the SPM Core execution state */ if ((spmc_attrs.exec_state != MODE_RW_64) && (spmc_attrs.exec_state != MODE_RW_32)) { - WARN("Unsupported SPM core execution state %x specified in " - "manifest image provided by BL2 boot loader.\n", + WARN("Unsupported SPM Core execution state 0x%x.\n", spmc_attrs.exec_state); - return 1; + return -EINVAL; } - INFO("SPM core execution state %x.\n", spmc_attrs.exec_state); + VERBOSE("SPM Core execution state 0x%x.\n", spmc_attrs.exec_state); #if SPMD_SPM_AT_SEL2 /* Ensure manifest has not requested AArch32 state in S-EL2 */ if (spmc_attrs.exec_state == MODE_RW_32) { WARN("AArch32 state at S-EL2 is not supported.\n"); - return 1; + return -EINVAL; } /* * Check if S-EL2 is supported on this system if S-EL2 * is required for SPM */ - uint64_t sel2 = read_id_aa64pfr0_el1(); - - sel2 >>= ID_AA64PFR0_SEL2_SHIFT; - sel2 &= ID_AA64PFR0_SEL2_MASK; - - if (!sel2) { - WARN("SPM core run time S-EL2 is not supported."); - return 1; + if (!is_armv8_4_sel2_present()) { + WARN("SPM Core run time S-EL2 is not supported.\n"); + return -EINVAL; } #endif /* SPMD_SPM_AT_SEL2 */ /* Initialise an entrypoint to set up the CPU context */ ep_attr = SECURE | EP_ST_ENABLE; - if (read_sctlr_el3() & SCTLR_EE_BIT) { + if ((read_sctlr_el3() & SCTLR_EE_BIT) != 0ULL) { ep_attr |= EP_EE_BIG; } @@ -215,8 +220,8 @@ static int spmd_spmc_init(void *rd_base, size_t rd_size) assert(spmc_ep_info->pc == BL32_BASE); /* - * Populate SPSR for SPM core based upon validated parameters from the - * manifest + * Populate SPSR for SPM Core based upon validated parameters from the + * manifest. */ if (spmc_attrs.exec_state == MODE_RW_32) { spmc_ep_info->spsr = SPSR_MODE32(MODE32_svc, SPSR_T_ARM, @@ -236,22 +241,22 @@ static int spmd_spmc_init(void *rd_base, size_t rd_size) DISABLE_ALL_EXCEPTIONS); } - /* Initialise SPM core context with this entry point information */ + /* Initialise SPM Core context with this entry point information */ cm_setup_context(&spm_ctx->cpu_ctx, spmc_ep_info); /* Reuse PSCI affinity states to mark this SPMC context as off */ spm_ctx->state = AFF_STATE_OFF; - INFO("SPM core setup done.\n"); + INFO("SPM Core setup done.\n"); - /* Register init function for deferred init. */ + /* Register init function for deferred init. */ bl31_register_bl32_init(&spmd_init); return 0; } /******************************************************************************* - * Initialize context of SPM core. + * Initialize context of SPM Core. ******************************************************************************/ int spmd_setup(void) { @@ -262,26 +267,24 @@ int spmd_setup(void) uintptr_t rd_size_align; spmc_ep_info = bl31_plat_get_next_image_ep_info(SECURE); - if (!spmc_ep_info) { - WARN("No SPM core image provided by BL2 boot loader, Booting " - "device without SP initialization. SMC`s destined for SPM " - "core will return SMC_UNK\n"); - return 1; + if (spmc_ep_info == NULL) { + WARN("No SPM Core image provided by BL2 boot loader.\n"); + return -EINVAL; } /* Under no circumstances will this parameter be 0 */ - assert(spmc_ep_info->pc != 0U); + assert(spmc_ep_info->pc != 0ULL); /* * Check if BL32 ep_info has a reference to 'tos_fw_config'. This will - * be used as a manifest for the SPM core at the next lower EL/mode. + * be used as a manifest for the SPM Core at the next lower EL/mode. */ if (spmc_ep_info->args.arg0 == 0U || spmc_ep_info->args.arg2 == 0U) { ERROR("Invalid or absent SPM core manifest\n"); panic(); } - /* Obtain whereabouts of SPM core manifest */ + /* Obtain whereabouts of SPM Core manifest */ rd_base = (void *) spmc_ep_info->args.arg0; rd_size = spmc_ep_info->args.arg2; @@ -305,9 +308,7 @@ int spmd_setup(void) if (rc != 0) { int mmap_rc; - WARN("Booting device without SPM initialization. " - "SPCI SMCs destined for SPM core will return " - "ENOTSUPPORTED\n"); + WARN("Booting device without SPM initialization.\n"); mmap_rc = mmap_remove_dynamic_region(rd_base_align, rd_size_align); @@ -326,9 +327,13 @@ int spmd_setup(void) /******************************************************************************* * Forward SMC to the other security state ******************************************************************************/ -static uint64_t spmd_smc_forward(uint32_t smc_fid, bool secure_origin, - uint64_t x1, uint64_t x2, uint64_t x3, - uint64_t x4, void *handle) +static uint64_t spmd_smc_forward(uint32_t smc_fid, + bool secure_origin, + uint64_t x1, + uint64_t x2, + uint64_t x3, + uint64_t x4, + void *handle) { uint32_t secure_state_in = (secure_origin) ? SECURE : NON_SECURE; uint32_t secure_state_out = (!secure_origin) ? SECURE : NON_SECURE; @@ -367,19 +372,23 @@ static uint64_t spmd_spci_error_return(void *handle, int error_code) * This function handles all SMCs in the range reserved for SPCI. Each call is * either forwarded to the other security state or handled by the SPM dispatcher ******************************************************************************/ -uint64_t spmd_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, - uint64_t x3, uint64_t x4, void *cookie, void *handle, +uint64_t spmd_smc_handler(uint32_t smc_fid, + uint64_t x1, + uint64_t x2, + uint64_t x3, + uint64_t x4, + void *cookie, + void *handle, uint64_t flags) { - spmd_spm_core_context_t *ctx = &spm_core_context[plat_my_core_pos()]; + spmd_spm_core_context_t *ctx = spmd_get_context(); bool secure_origin; int32_t ret; /* Determine which security state this SMC originated from */ secure_origin = is_caller_secure(flags); - INFO("SPM: 0x%x, 0x%llx, 0x%llx, 0x%llx, 0x%llx, " - "0x%llx, 0x%llx, 0x%llx\n", + INFO("SPM: 0x%x 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx 0x%llx\n", smc_fid, x1, x2, x3, x4, SMC_GET_GP(handle, CTX_GPREG_X5), SMC_GET_GP(handle, CTX_GPREG_X6), SMC_GET_GP(handle, CTX_GPREG_X7)); @@ -388,7 +397,7 @@ uint64_t spmd_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, case SPCI_ERROR: /* * Check if this is the first invocation of this interface on - * this CPU. If so, then indicate that the SPM core initialised + * this CPU. If so, then indicate that the SPM Core initialised * unsuccessfully. */ if (secure_origin && (ctx->state == SPMC_STATE_RESET)) { @@ -402,9 +411,9 @@ uint64_t spmd_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, case SPCI_VERSION: /* * TODO: This is an optimization that the version information - * provided by the SPM core manifest is returned by the SPM + * provided by the SPM Core manifest is returned by the SPM * dispatcher. It might be a better idea to simply forward this - * call to the SPM core and wash our hands completely. + * call to the SPM Core and wash our hands completely. */ ret = MAKE_SPCI_VERSION(spmc_attrs.major_version, spmc_attrs.minor_version); @@ -416,7 +425,7 @@ uint64_t spmd_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, case SPCI_FEATURES: /* * This is an optional interface. Do the minimal checks and - * forward to SPM core which will handle it if implemented. + * forward to SPM Core which will handle it if implemented. */ /* @@ -428,42 +437,42 @@ uint64_t spmd_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, SPCI_ERROR_NOT_SUPPORTED); } - /* Forward SMC from Normal world to the SPM core */ + /* Forward SMC from Normal world to the SPM Core */ if (!secure_origin) { return spmd_smc_forward(smc_fid, secure_origin, x1, x2, x3, x4, handle); - } else { - /* - * Return success if call was from secure world i.e. all - * SPCI functions are supported. This is essentially a - * nop. - */ - SMC_RET8(handle, SPCI_SUCCESS_SMC32, x1, x2, x3, x4, - SMC_GET_GP(handle, CTX_GPREG_X5), - SMC_GET_GP(handle, CTX_GPREG_X6), - SMC_GET_GP(handle, CTX_GPREG_X7)); } + /* + * Return success if call was from secure world i.e. all + * SPCI functions are supported. This is essentially a + * nop. + */ + SMC_RET8(handle, SPCI_SUCCESS_SMC32, x1, x2, x3, x4, + SMC_GET_GP(handle, CTX_GPREG_X5), + SMC_GET_GP(handle, CTX_GPREG_X6), + SMC_GET_GP(handle, CTX_GPREG_X7)); + break; /* not reached */ case SPCI_ID_GET: /* * Returns the ID of the calling SPCI component. - */ + */ if (!secure_origin) { SMC_RET8(handle, SPCI_SUCCESS_SMC32, SPCI_TARGET_INFO_MBZ, SPCI_NS_ENDPOINT_ID, SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, SPCI_PARAM_MBZ); - } else { - SMC_RET8(handle, SPCI_SUCCESS_SMC32, - SPCI_TARGET_INFO_MBZ, spmc_attrs.spmc_id, - SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, - SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, - SPCI_PARAM_MBZ); } + SMC_RET8(handle, SPCI_SUCCESS_SMC32, + SPCI_TARGET_INFO_MBZ, spmc_attrs.spmc_id, + SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, + SPCI_PARAM_MBZ, SPCI_PARAM_MBZ, + SPCI_PARAM_MBZ); + break; /* not reached */ case SPCI_RX_RELEASE: @@ -513,7 +522,7 @@ uint64_t spmd_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, /* * Check if this is the first invocation of this interface on * this CPU from the Secure world. If so, then indicate that the - * SPM core initialised successfully. + * SPM Core initialised successfully. */ if (secure_origin && (ctx->state == SPMC_STATE_RESET)) { spmd_spm_core_sync_exit(0); diff --git a/services/std_svc/spmd/spmd_private.h b/services/std_svc/spmd/spmd_private.h index 0ad35c7c1..232031bb3 100644 --- a/services/std_svc/spmd/spmd_private.h +++ b/services/std_svc/spmd/spmd_private.h @@ -33,12 +33,6 @@ #include #include -/* - * Convert a function no. in a FID to a bit position. All function nos. are - * between 0 and 0x1f - */ -#define SPCI_FNO_TO_BIT_POS(_fid) (1 << ((_fid) & U(0x1f))) - typedef enum spmc_state { SPMC_STATE_RESET = 0, SPMC_STATE_IDLE @@ -60,21 +54,10 @@ typedef struct spmd_spm_core_context { #define SPCI_NS_ENDPOINT_ID U(0) /* Mask and shift to check valid secure SPCI Endpoint ID. */ -#define SPMC_SECURE_ID_MASK 0x1 -#define SPMC_SECURE_ID_SHIFT 15 +#define SPMC_SECURE_ID_MASK U(1) +#define SPMC_SECURE_ID_SHIFT U(15) -/* - * Data structure used by the SPM dispatcher (SPMD) in EL3 to track sequence of - * SPCI calls from lower ELs. - * - * next_smc_bit_map: Per-cpu bit map of SMCs from each world that are expected - * next. - */ -typedef struct spmd_spci_context { - uint32_t next_smc_bit_map[2]; -} spmd_spci_context_t; - -/* Functions used to enter/exit a Secure Partition synchronously */ +/* Functions used to enter/exit SPMC synchronously */ uint64_t spmd_spm_core_sync_entry(spmd_spm_core_context_t *ctx); __dead2 void spmd_spm_core_sync_exit(uint64_t rc); @@ -82,6 +65,9 @@ __dead2 void spmd_spm_core_sync_exit(uint64_t rc); uint64_t spmd_spm_core_enter(uint64_t *c_rt_ctx); void __dead2 spmd_spm_core_exit(uint64_t c_rt_ctx, uint64_t ret); +/* SPMC context on current CPU get helper */ +spmd_spm_core_context_t *spmd_get_context(void); + #endif /* __ASSEMBLER__ */ #endif /* SPMD_PRIVATE_H */ From 23d5ba86bd7a43e7abb634c328d7a85bbc877fb5 Mon Sep 17 00:00:00 2001 From: Olivier Deprez Date: Fri, 7 Feb 2020 15:44:43 +0100 Subject: [PATCH 2/2] SPMD: extract SPMC DTB header size from SPMD Currently BL2 passes TOS_FW_CONFIG address and size through registers to BL31. This corresponds to SPMC manifest load address and size. The SPMC manifest is mapped in BL31 by dynamic mapping. This patch removes BL2 changes from generic code (which were enclosed by SPD=spmd) and retrieves SPMC manifest size directly from within SPMD. The SPMC manifest load address is still passed through a register by generic code. Signed-off-by: Olivier Deprez Change-Id: I35c5abd95c616ae25677302f0b1d0c45c51c042f --- common/desc_image_load.c | 12 ------ include/plat/common/platform.h | 3 +- plat/common/plat_spmd_manifest.c | 68 ++++++++++++++++++++++++++----- services/std_svc/spmd/spmd_main.c | 62 +++++++--------------------- 4 files changed, 73 insertions(+), 72 deletions(-) diff --git a/common/desc_image_load.c b/common/desc_image_load.c index 47c80aa86..30b97e050 100644 --- a/common/desc_image_load.c +++ b/common/desc_image_load.c @@ -214,9 +214,6 @@ void populate_next_bl_params_config(bl_params_t *bl2_to_next_bl_params) { bl_params_node_t *params_node; unsigned int fw_config_id; -#ifdef SPD_spmd - uint32_t fw_config_size = 0; -#endif uintptr_t fw_config_base; bl_mem_params_node_t *mem_params; uintptr_t hw_config_base = 0; @@ -264,10 +261,6 @@ void populate_next_bl_params_config(bl_params_t *bl2_to_next_bl_params) mem_params = get_bl_mem_params_node(fw_config_id); if (mem_params != NULL) { fw_config_base = mem_params->image_info.image_base; -#ifdef SPD_spmd - fw_config_size = - mem_params->image_info.image_size; -#endif } } @@ -306,11 +299,6 @@ void populate_next_bl_params_config(bl_params_t *bl2_to_next_bl_params) if (params_node->ep_info->args.arg1 == 0U) params_node->ep_info->args.arg1 = hw_config_base; -#ifdef SPD_spmd - if (params_node->ep_info->args.arg2 == 0U) - params_node->ep_info->args.arg2 = - fw_config_size; -#endif } #ifdef SPD_opteed } diff --git a/include/plat/common/platform.h b/include/plat/common/platform.h index 946a3b8b8..b8ba14c57 100644 --- a/include/plat/common/platform.h +++ b/include/plat/common/platform.h @@ -290,8 +290,7 @@ int plat_spm_sp_get_next_address(void **sp_base, size_t *sp_size, void **rd_base, size_t *rd_size); #if defined(SPD_spmd) int plat_spm_core_manifest_load(spmc_manifest_attribute_t *manifest, - const void *ptr, - size_t size); + const void *pm_addr); #endif /******************************************************************************* * Mandatory BL image load functions(may be overridden). diff --git a/plat/common/plat_spmd_manifest.c b/plat/common/plat_spmd_manifest.c index a3e30e89c..109b001f0 100644 --- a/plat/common/plat_spmd_manifest.c +++ b/plat/common/plat_spmd_manifest.c @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -110,28 +111,75 @@ static int manifest_parse_root(spmc_manifest_attribute_t *manifest, * Platform handler to parse a SPM Core manifest. ******************************************************************************/ int plat_spm_core_manifest_load(spmc_manifest_attribute_t *manifest, - const void *ptr, - size_t size) + const void *pm_addr) { - int rc; + int rc, unmap_ret; + uintptr_t pm_base, pm_base_align; + size_t mapped_size; assert(manifest != NULL); - assert(ptr != NULL); + assert(pm_addr != NULL); - INFO("Reading SPM Core manifest at address %p\n", ptr); + /* + * Assume TOS_FW_CONFIG is not necessarily aligned to a page + * boundary, thus calculate the remaining space between SPMC + * manifest start address and upper page limit. + * + */ + pm_base = (uintptr_t)pm_addr; + pm_base_align = page_align(pm_base, UP); + mapped_size = pm_base_align - pm_base; - rc = fdt_check_header(ptr); + /* Check space within the page at least maps the FDT header */ + if (mapped_size < sizeof(struct fdt_header)) { + ERROR("Error while mapping SPM Core manifest.\n"); + return -EINVAL; + } + + /* Map first SPMC manifest page in the SPMD translation regime */ + pm_base_align = page_align(pm_base, DOWN); + rc = mmap_add_dynamic_region((unsigned long long)pm_base_align, + pm_base_align, + PAGE_SIZE, + MT_RO_DATA); if (rc != 0) { - ERROR("Wrong format for SPM Core manifest (%d).\n", rc); + ERROR("Error while mapping SPM Core manifest (%d).\n", rc); return rc; } - rc = fdt_node_offset_by_compatible(ptr, -1, + rc = fdt_check_header(pm_addr); + if (rc != 0) { + ERROR("Wrong format for SPM Core manifest (%d).\n", rc); + goto exit_unmap; + } + + /* Check SPMC manifest fits within the upper mapped page boundary */ + if (mapped_size < fdt_totalsize(pm_addr)) { + ERROR("SPM Core manifest too large.\n"); + rc = -EINVAL; + goto exit_unmap; + } + + VERBOSE("Reading SPM Core manifest at address %p\n", pm_addr); + + rc = fdt_node_offset_by_compatible(pm_addr, -1, "arm,spci-core-manifest-1.0"); if (rc < 0) { ERROR("Unrecognized SPM Core manifest\n"); - return rc; + goto exit_unmap; } - return manifest_parse_root(manifest, ptr, rc); + rc = manifest_parse_root(manifest, pm_addr, rc); + +exit_unmap: + unmap_ret = mmap_remove_dynamic_region(pm_base_align, PAGE_SIZE); + if (unmap_ret != 0) { + ERROR("Error while unmapping SPM Core manifest (%d).\n", + unmap_ret); + if (rc == 0) { + rc = unmap_ret; + } + } + + return rc; } diff --git a/services/std_svc/spmd/spmd_main.c b/services/std_svc/spmd/spmd_main.c index f6dbb97e3..501782ffb 100644 --- a/services/std_svc/spmd/spmd_main.c +++ b/services/std_svc/spmd/spmd_main.c @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -56,8 +55,7 @@ spmd_spm_core_context_t *spmd_get_context(void) * Static function declaration. ******************************************************************************/ static int32_t spmd_init(void); -static int spmd_spmc_init(void *rd_base, - size_t rd_size); +static int spmd_spmc_init(void *pm_addr); static uint64_t spmd_spci_error_return(void *handle, int error_code); static uint64_t spmd_smc_forward(uint32_t smc_fid, @@ -146,14 +144,14 @@ static int32_t spmd_init(void) /******************************************************************************* * Loads SPMC manifest and inits SPMC. ******************************************************************************/ -static int spmd_spmc_init(void *rd_base, size_t rd_size) +static int spmd_spmc_init(void *pm_addr) { spmd_spm_core_context_t *spm_ctx = spmd_get_context(); uint32_t ep_attr; int rc; /* Load the SPM Core manifest */ - rc = plat_spm_core_manifest_load(&spmc_attrs, rd_base, rd_size); + rc = plat_spm_core_manifest_load(&spmc_attrs, pm_addr); if (rc != 0) { WARN("No or invalid SPM Core manifest image provided by BL2\n"); return rc; @@ -170,7 +168,7 @@ static int spmd_spmc_init(void *rd_base, size_t rd_size) return -EINVAL; } - VERBOSE("SPCI version (%u.%u).\n", spmc_attrs.major_version, + VERBOSE("SPCI version (%u.%u)\n", spmc_attrs.major_version, spmc_attrs.minor_version); VERBOSE("SPM Core run time EL%x.\n", @@ -186,12 +184,13 @@ static int spmd_spmc_init(void *rd_base, size_t rd_size) /* Validate the SPM Core execution state */ if ((spmc_attrs.exec_state != MODE_RW_64) && (spmc_attrs.exec_state != MODE_RW_32)) { - WARN("Unsupported SPM Core execution state 0x%x.\n", + WARN("Unsupported %s%x.\n", "SPM Core execution state 0x", spmc_attrs.exec_state); return -EINVAL; } - VERBOSE("SPM Core execution state 0x%x.\n", spmc_attrs.exec_state); + VERBOSE("%s%x.\n", "SPM Core execution state 0x", + spmc_attrs.exec_state); #if SPMD_SPM_AT_SEL2 /* Ensure manifest has not requested AArch32 state in S-EL2 */ @@ -260,11 +259,8 @@ static int spmd_spmc_init(void *rd_base, size_t rd_size) ******************************************************************************/ int spmd_setup(void) { + void *spmc_manifest; int rc; - void *rd_base; - size_t rd_size; - uintptr_t rd_base_align; - uintptr_t rd_size_align; spmc_ep_info = bl31_plat_get_next_image_ep_info(SECURE); if (spmc_ep_info == NULL) { @@ -279,49 +275,19 @@ int spmd_setup(void) * Check if BL32 ep_info has a reference to 'tos_fw_config'. This will * be used as a manifest for the SPM Core at the next lower EL/mode. */ - if (spmc_ep_info->args.arg0 == 0U || spmc_ep_info->args.arg2 == 0U) { - ERROR("Invalid or absent SPM core manifest\n"); - panic(); - } - - /* Obtain whereabouts of SPM Core manifest */ - rd_base = (void *) spmc_ep_info->args.arg0; - rd_size = spmc_ep_info->args.arg2; - - rd_base_align = page_align((uintptr_t) rd_base, DOWN); - rd_size_align = page_align((uintptr_t) rd_size, UP); - - /* Map the manifest in the SPMD translation regime first */ - VERBOSE("SPM core manifest base : 0x%lx\n", rd_base_align); - VERBOSE("SPM core manifest size : 0x%lx\n", rd_size_align); - rc = mmap_add_dynamic_region((unsigned long long) rd_base_align, - (uintptr_t) rd_base_align, - rd_size_align, - MT_RO_DATA); - if (rc != 0) { - ERROR("Error while mapping SPM core manifest (%d).\n", rc); - panic(); + spmc_manifest = (void *)spmc_ep_info->args.arg0; + if (spmc_manifest == NULL) { + ERROR("Invalid or absent SPM Core manifest.\n"); + return -EINVAL; } /* Load manifest, init SPMC */ - rc = spmd_spmc_init(rd_base, rd_size); + rc = spmd_spmc_init(spmc_manifest); if (rc != 0) { - int mmap_rc; - WARN("Booting device without SPM initialization.\n"); - - mmap_rc = mmap_remove_dynamic_region(rd_base_align, - rd_size_align); - if (mmap_rc != 0) { - ERROR("Error while unmapping SPM core manifest (%d).\n", - mmap_rc); - panic(); - } - - return rc; } - return 0; + return rc; } /*******************************************************************************