Fix the inconsistencies in bl1_tbbr_image_descs[]
This patch fixes inconsistencies in bl1_tbbr_image_descs[]
and miscellaneous fixes in Firmware Update code.
Following are the changes:
* As part of the original FWU changes, a `copied_size`
field was added to `image_info_t`. This was a subtle binary
compatibility break because it changed the size of the
`bl31_params_t` struct, which could cause problems if
somebody used different versions of BL2 or BL31, one with
the old `image_info_t` and one with the new version.
This patch put the `copied_size` within the `image_desc_t`.
* EXECUTABLE flag is now stored in `ep_info.h.attr` in place
of `image_info.h.attr`, associating it to an entrypoint.
* The `image_info.image_base` is only relevant for secure
images that are copied from non-secure memory into secure
memory. This patch removes initializing `image_base` for
non secure images in the bl1_tbbr_image_descs[].
* A new macro `SET_STATIC_PARAM_HEAD` is added for populating
bl1_tbbr_image_descs[].ep_info/image_info.h members statically.
The version, image_type and image attributes are now
populated using this new macro.
* Added PLAT_ARM_NVM_BASE and PLAT_ARM_NVM_SIZE to avoid direct
usage of V2M_FLASH0_XXX in plat/arm/common/arm_bl1_fwu.c.
* Refactoring of code/macros related to SECURE and EXECUTABLE flags.
NOTE: PLATFORM PORTS THAT RELY ON THE SIZE OF `image_info_t`
OR USE the "EXECUTABLE" BIT WITHIN `image_info_t.h.attr`
OR USE THEIR OWN `image_desc_t` ARRAY IN BL1, MAY BE
BROKEN BY THIS CHANGE. THIS IS CONSIDERED UNLIKELY.
Change-Id: Id4e5989af7bf0ed263d19d3751939da1169b561d
diff --git a/bl1/bl1_context_mgmt.c b/bl1/bl1_context_mgmt.c
index 6355190..bd40608 100644
--- a/bl1/bl1_context_mgmt.c
+++ b/bl1/bl1_context_mgmt.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -74,7 +74,7 @@
next_bl_ep = &image_desc->ep_info;
/* Get the image security state. */
- security_state = GET_SEC_STATE(next_bl_ep->h.attr);
+ security_state = GET_SECURITY_STATE(next_bl_ep->h.attr);
/* Setup the Secure/Non-Secure context if not done already. */
if (!cm_get_context(security_state))
diff --git a/bl1/bl1_fwu.c b/bl1/bl1_fwu.c
index 80ce831..f333805 100644
--- a/bl1/bl1_fwu.c
+++ b/bl1/bl1_fwu.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -135,8 +135,8 @@
}
/* Only Normal world is allowed to copy a Secure image. */
- if ((GET_SEC_STATE(flags) == SECURE) ||
- (GET_SEC_STATE(image_desc->ep_info.h.attr) == NON_SECURE)) {
+ if ((GET_SECURITY_STATE(flags) == SECURE) ||
+ (GET_SECURITY_STATE(image_desc->ep_info.h.attr) == NON_SECURE)) {
WARN("BL1-FWU: Copy not allowed for Non-Secure "
"image from Secure-world\n");
return -EPERM;
@@ -156,10 +156,10 @@
* If last block is more than expected then
* clip the block to the required image size.
*/
- if (image_desc->image_info.copied_size + block_size >
+ if (image_desc->copied_size + block_size >
image_desc->image_info.image_size) {
block_size = image_desc->image_info.image_size -
- image_desc->image_info.copied_size;
+ image_desc->copied_size;
WARN("BL1-FWU: Copy argument block_size > remaining image size."
" Clipping block_size\n");
}
@@ -173,13 +173,13 @@
INFO("BL1-FWU: Continuing image copy in blocks\n");
/* Copy image for given block size. */
- base_addr += image_desc->image_info.copied_size;
- image_desc->image_info.copied_size += block_size;
+ base_addr += image_desc->copied_size;
+ image_desc->copied_size += block_size;
memcpy((void *)base_addr, (const void *)image_src, block_size);
flush_dcache_range(base_addr, block_size);
/* Update the state if last block. */
- if (image_desc->image_info.copied_size ==
+ if (image_desc->copied_size ==
image_desc->image_info.image_size) {
image_desc->state = IMAGE_STATE_COPIED;
INFO("BL1-FWU: Image copy in blocks completed\n");
@@ -234,7 +234,7 @@
INFO("BL1-FWU: Started image copy in blocks\n");
}
- image_desc->image_info.copied_size = block_size;
+ image_desc->copied_size = block_size;
}
return 0;
@@ -257,14 +257,14 @@
if (!image_desc)
return -EPERM;
- if (GET_SEC_STATE(flags) == SECURE) {
+ if (GET_SECURITY_STATE(flags) == SECURE) {
if (image_desc->state != IMAGE_STATE_RESET) {
WARN("BL1-FWU: Authentication from secure world "
"while in invalid state\n");
return -EPERM;
}
} else {
- if (GET_SEC_STATE(image_desc->ep_info.h.attr) == SECURE) {
+ if (GET_SECURITY_STATE(image_desc->ep_info.h.attr) == SECURE) {
if (image_desc->state != IMAGE_STATE_COPIED) {
WARN("BL1-FWU: Authentication of secure image "
"from non-secure world while not in copied state\n");
@@ -369,10 +369,10 @@
* Image is NOT in AUTHENTICATED state.
*/
if ((!image_desc) ||
- (GET_SEC_STATE(flags) == SECURE) ||
- (GET_SEC_STATE(image_desc->ep_info.h.attr) == NON_SECURE) ||
- (GET_EXEC_STATE(image_desc->image_info.h.attr) == NON_EXECUTABLE) ||
- (image_desc->state != IMAGE_STATE_AUTHENTICATED)) {
+ (GET_SECURITY_STATE(flags) == SECURE) ||
+ (GET_SECURITY_STATE(image_desc->ep_info.h.attr) == NON_SECURE) ||
+ (EP_GET_EXE(image_desc->ep_info.h.attr) == NON_EXECUTABLE) ||
+ (image_desc->state != IMAGE_STATE_AUTHENTICATED)) {
WARN("BL1-FWU: Execution not allowed due to invalid state/args\n");
return -EPERM;
}
@@ -402,7 +402,7 @@
{
image_desc_t *image_desc;
unsigned int resume_sec_state;
- unsigned int caller_sec_state = GET_SEC_STATE(flags);
+ unsigned int caller_sec_state = GET_SECURITY_STATE(flags);
/* Get the image descriptor for last executed secure image id. */
image_desc = bl1_plat_get_image_desc(sec_exec_image_id);
@@ -417,8 +417,8 @@
assert(image_desc);
}
- assert(GET_SEC_STATE(image_desc->ep_info.h.attr) == SECURE);
- assert(GET_EXEC_STATE(image_desc->image_info.h.attr) == EXECUTABLE);
+ assert(GET_SECURITY_STATE(image_desc->ep_info.h.attr) == SECURE);
+ assert(EP_GET_EXE(image_desc->ep_info.h.attr) == EXECUTABLE);
if (caller_sec_state == SECURE) {
assert(image_desc->state == IMAGE_STATE_EXECUTED);
@@ -458,7 +458,7 @@
image_desc_t *image_desc;
/* Make sure caller is from the secure world */
- if (GET_SEC_STATE(flags) == NON_SECURE) {
+ if (GET_SECURITY_STATE(flags) == NON_SECURE) {
WARN("BL1-FWU: Image done not allowed from normal world\n");
return -EPERM;
}
@@ -468,8 +468,8 @@
/* image_desc must correspond to a valid secure executing image */
assert(image_desc);
- assert(GET_SEC_STATE(image_desc->ep_info.h.attr) == SECURE);
- assert(GET_EXEC_STATE(image_desc->image_info.h.attr) == EXECUTABLE);
+ assert(GET_SECURITY_STATE(image_desc->ep_info.h.attr) == SECURE);
+ assert(EP_GET_EXE(image_desc->ep_info.h.attr) == EXECUTABLE);
assert(image_desc->state == IMAGE_STATE_EXECUTED);
/* Update the flags. */
diff --git a/bl1/tbbr/tbbr_img_desc.c b/bl1/tbbr/tbbr_img_desc.c
index 42de851..7651f1c 100644
--- a/bl1/tbbr/tbbr_img_desc.c
+++ b/bl1/tbbr/tbbr_img_desc.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -35,42 +35,46 @@
image_desc_t bl1_tbbr_image_descs[] = {
{
.image_id = FWU_CERT_ID,
- .image_info.h.attr = SET_EXEC_STATE(NON_EXECUTABLE),
+ SET_STATIC_PARAM_HEAD(image_info, PARAM_IMAGE_BINARY,
+ VERSION_1, image_info_t, 0),
.image_info.image_base = BL2_BASE,
- .ep_info.h.attr = SET_SEC_STATE(SECURE),
+ SET_STATIC_PARAM_HEAD(ep_info, PARAM_IMAGE_BINARY,
+ VERSION_1, entry_point_info_t, SECURE),
},
#if NS_BL1U_BASE
{
.image_id = NS_BL1U_IMAGE_ID,
- .image_info.h.attr = SET_EXEC_STATE(EXECUTABLE),
- .image_info.image_base = NS_BL1U_BASE,
- .ep_info.h.attr = SET_SEC_STATE(NON_SECURE),
+ SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,
+ VERSION_1, entry_point_info_t, NON_SECURE | EXECUTABLE),
.ep_info.pc = NS_BL1U_BASE,
},
#endif
#if SCP_BL2U_BASE
{
.image_id = SCP_BL2U_IMAGE_ID,
- .image_info.h.attr = SET_EXEC_STATE(NON_EXECUTABLE),
+ SET_STATIC_PARAM_HEAD(image_info, PARAM_IMAGE_BINARY,
+ VERSION_1, image_info_t, 0),
.image_info.image_base = SCP_BL2U_BASE,
- .ep_info.h.attr = SET_SEC_STATE(SECURE),
+ SET_STATIC_PARAM_HEAD(ep_info, PARAM_IMAGE_BINARY,
+ VERSION_1, entry_point_info_t, SECURE),
},
#endif
#if BL2U_BASE
{
.image_id = BL2U_IMAGE_ID,
- .image_info.h.attr = SET_EXEC_STATE(EXECUTABLE),
+ SET_STATIC_PARAM_HEAD(image_info, PARAM_EP,
+ VERSION_1, image_info_t, 0),
.image_info.image_base = BL2U_BASE,
- .ep_info.h.attr = SET_SEC_STATE(SECURE),
+ SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,
+ VERSION_1, entry_point_info_t, SECURE | EXECUTABLE),
.ep_info.pc = BL2U_BASE,
},
#endif
#if NS_BL2U_BASE
{
.image_id = NS_BL2U_IMAGE_ID,
- .image_info.h.attr = SET_EXEC_STATE(NON_EXECUTABLE),
- .image_info.image_base = NS_BL2U_BASE,
- .ep_info.h.attr = SET_SEC_STATE(NON_SECURE),
+ SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,
+ VERSION_1, entry_point_info_t, NON_SECURE),
},
#endif
BL2_IMAGE_DESC,
diff --git a/include/common/bl_common.h b/include/common/bl_common.h
index e5e6717..f13dc31 100644
--- a/include/common/bl_common.h
+++ b/include/common/bl_common.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013-2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -53,28 +53,12 @@
#define ENTRY_POINT_INFO_ARGS_OFFSET 0x18
/* The following are used to set/get image attributes. */
-#define EXECUTABLE (0x1)
-#define NON_EXECUTABLE (0x0)
-#define PARAM_EP_EXECUTE_MASK (0x1)
-#define PARAM_EP_EXECUTE_SHIFT (0x1)
#define PARAM_EP_SECURITY_MASK (0x1)
-#define PARAM_EP_SECURITY_SHIFT (0x0)
#define GET_SECURITY_STATE(x) (x & PARAM_EP_SECURITY_MASK)
#define SET_SECURITY_STATE(x, security) \
((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security))
-#define GET_EXEC_STATE(x) \
- (((x) >> PARAM_EP_EXECUTE_SHIFT) & PARAM_EP_EXECUTE_MASK)
-
-#define SET_EXEC_STATE(x) \
- (((x) & PARAM_EP_EXECUTE_MASK) << PARAM_EP_EXECUTE_SHIFT)
-
-#define GET_SEC_STATE(x) \
- (((x) >> PARAM_EP_SECURITY_SHIFT) & PARAM_EP_SECURITY_MASK)
-
-#define SET_SEC_STATE(x) \
- (((x) & PARAM_EP_SECURITY_MASK) << PARAM_EP_SECURITY_SHIFT)
/*
* The following are used for image state attributes.
@@ -99,11 +83,17 @@
#define EP_GET_ST(x) (x & EP_ST_MASK)
#define EP_SET_ST(x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee))
-#define PARAM_EP 0x01
-#define PARAM_IMAGE_BINARY 0x02
-#define PARAM_BL31 0x03
+#define EP_EXE_MASK 0x8
+#define NON_EXECUTABLE 0x0
+#define EXECUTABLE 0x8
+#define EP_GET_EXE(x) (x & EP_EXE_MASK)
+#define EP_SET_EXE(x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee))
-#define VERSION_1 0x01
+#define PARAM_EP 0x01
+#define PARAM_IMAGE_BINARY 0x02
+#define PARAM_BL31 0x03
+
+#define VERSION_1 0x01
#define INVALID_IMAGE_ID (0xFFFFFFFF)
@@ -114,6 +104,14 @@
(_p)->h.attr = (uint32_t)(_attr) ; \
} while (0)
+/* Following is used for populating structure members statically. */
+#define SET_STATIC_PARAM_HEAD(_p, _type, _ver, _p_type, _attr) \
+ ._p.h.type = (uint8_t)(_type), \
+ ._p.h.version = (uint8_t)(_ver), \
+ ._p.h.size = (uint16_t)sizeof(_p_type), \
+ ._p.h.attr = (uint32_t)(_attr)
+
+
/*******************************************************************************
* Constants to indicate type of exception to the common exception handler.
******************************************************************************/
@@ -224,7 +222,6 @@
param_header_t h;
uintptr_t image_base; /* physical address of base of image */
uint32_t image_size; /* bytes read from image file */
- uint32_t copied_size; /* image size copied in blocks */
} image_info_t;
/*****************************************************************************
@@ -238,6 +235,7 @@
* Refer IMAGE_STATE_XXX defined above.
*/
unsigned int state;
+ uint32_t copied_size; /* image size copied in blocks */
image_info_t image_info;
entry_point_info_t ep_info;
} image_desc_t;
diff --git a/include/plat/arm/board/common/board_arm_def.h b/include/plat/arm/board/common/board_arm_def.h
index db2a8df..70f6b5b 100644
--- a/include/plat/arm/board/common/board_arm_def.h
+++ b/include/plat/arm/board/common/board_arm_def.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -143,5 +143,8 @@
#define PLAT_ARM_FIP_BASE V2M_FLASH0_BASE
#define PLAT_ARM_FIP_MAX_SIZE V2M_FLASH0_SIZE
+#define PLAT_ARM_NVM_BASE V2M_FLASH0_BASE
+#define PLAT_ARM_NVM_SIZE V2M_FLASH0_SIZE
+
#endif /* __BOARD_ARM_DEF_H__ */
diff --git a/include/plat/arm/common/arm_def.h b/include/plat/arm/common/arm_def.h
index 8d75363..95cb648 100644
--- a/include/plat/arm/common/arm_def.h
+++ b/include/plat/arm/common/arm_def.h
@@ -299,7 +299,7 @@
#define BL2U_BASE BL2_BASE
#define BL2U_LIMIT BL31_BASE
#define NS_BL2U_BASE ARM_NS_DRAM1_BASE
-#define NS_BL1U_BASE (V2M_FLASH0_BASE + 0x03EB8000)
+#define NS_BL1U_BASE (PLAT_ARM_NVM_BASE + 0x03EB8000)
/*
* ID of the secure physical generic timer interrupt used by the TSP.
diff --git a/include/plat/common/common_def.h b/include/plat/common/common_def.h
index 916720c..9fac9fa 100644
--- a/include/plat/common/common_def.h
+++ b/include/plat/common/common_def.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -72,11 +72,12 @@
#define BL2_IMAGE_DESC { \
.image_id = BL2_IMAGE_ID, \
- .image_info.h.version = VERSION_1, \
- .image_info.h.attr = SET_EXEC_STATE(EXECUTABLE),\
+ SET_STATIC_PARAM_HEAD(image_info, PARAM_EP, \
+ VERSION_1, image_info_t, 0), \
.image_info.image_base = BL2_BASE, \
- .ep_info.h.attr = SET_SEC_STATE(SECURE), \
- .ep_info.pc = BL2_BASE \
+ SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP, \
+ VERSION_1, entry_point_info_t, SECURE | EXECUTABLE),\
+ .ep_info.pc = BL2_BASE, \
}
#endif /* __COMMON_DEF_H__ */
diff --git a/plat/arm/common/arm_bl1_fwu.c b/plat/arm/common/arm_bl1_fwu.c
index 9a0d93a..2a18d34 100644
--- a/plat/arm/common/arm_bl1_fwu.c
+++ b/plat/arm/common/arm_bl1_fwu.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -33,11 +33,12 @@
#include <debug.h>
#include <errno.h>
#include <plat_arm.h>
+#include <platform_def.h>
#include <tbbr_img_desc.h>
/* Struct to keep track of usable memory */
-typedef struct bl1_mem_info{
+typedef struct bl1_mem_info {
uintptr_t mem_base;
unsigned int mem_size;
} bl1_mem_info_t;
@@ -58,8 +59,8 @@
.mem_size = ARM_NS_DRAM1_SIZE
},
{
- .mem_base = V2M_FLASH0_BASE,
- .mem_size = V2M_FLASH0_SIZE
+ .mem_base = PLAT_ARM_NVM_BASE,
+ .mem_size = PLAT_ARM_NVM_SIZE
},
{
.mem_size = 0
@@ -79,7 +80,7 @@
/*
* Check the given image source and size.
*/
- if (GET_SEC_STATE(flags) == SECURE)
+ if (GET_SECURITY_STATE(flags) == SECURE)
mmap = fwu_addr_map_secure;
else
mmap = fwu_addr_map_non_secure;
diff --git a/plat/arm/common/arm_common.mk b/plat/arm/common/arm_common.mk
index 2647f04..425e0d3 100644
--- a/plat/arm/common/arm_common.mk
+++ b/plat/arm/common/arm_common.mk
@@ -129,8 +129,8 @@
PLAT_INCLUDES += -Iinclude/bl1/tbbr
- BL1_SOURCES += ${AUTH_SOURCES} \
- bl1/tbbr/tbbr_img_desc.c \
+ BL1_SOURCES += ${AUTH_SOURCES} \
+ bl1/tbbr/tbbr_img_desc.c \
plat/arm/common/arm_bl1_fwu.c
BL2_SOURCES += ${AUTH_SOURCES}
