From 52696946ab3f441496436ad7223cb2bd853c8beb Mon Sep 17 00:00:00 2001 From: Olivier Deprez Date: Thu, 16 Apr 2020 13:39:06 +0200 Subject: 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 --- services/std_svc/spmd/spmd_main.c | 219 ++++++++++++++++++----------------- services/std_svc/spmd/spmd_private.h | 26 +---- 2 files changed, 120 insertions(+), 125 deletions(-) (limited to 'services') 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 @@ -41,19 +42,35 @@ static spmc_manifest_sect_attribute_t spmc_attrs; ******************************************************************************/ static entry_point_info_t *spmc_ep_info; +/******************************************************************************* + * SPM Core context on current CPU get helper. + ******************************************************************************/ +spmd_spm_core_context_t *spmd_get_context(void) +{ + unsigned int linear_id = plat_my_core_pos(); + + return &spm_core_context[linear_id]; +} + /******************************************************************************* * 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); +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 SP context pointer and performs a synchronous entry - * into it. + * 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 - -/* - * 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; +#define SPMC_SECURE_ID_MASK U(1) +#define SPMC_SECURE_ID_SHIFT U(15) -/* 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 */ -- cgit v1.2.3