From 2fb5312f61a7de8b7a70e1639199c4f14a10b6f9 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Fri, 26 Mar 2021 00:34:17 -0500 Subject: [PATCH 1/5] plat: ti: k3: board: lite: Increase SRAM size to account for additional table We actually have additional table entries than what we accounted for in our size. MAX_XLAT_TABLES is 8, but really we could be using upto 10 depending on the platform. So, we need an extra 8K space in. This gets exposed with DEBUG=1 and assert checks trigger, which for some reason completely escaped testing previously. ASSERT: lib/xlat_tables_v2/xlat_tables_core.c:97 BACKTRACE: START: assert Signed-off-by: Nishanth Menon Change-Id: I5c5d04440ef1fccfaf2317066f3abbc0ec645903 --- plat/ti/k3/board/lite/include/board_def.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plat/ti/k3/board/lite/include/board_def.h b/plat/ti/k3/board/lite/include/board_def.h index 7c7ea62c1..b363bea22 100644 --- a/plat/ti/k3/board/lite/include/board_def.h +++ b/plat/ti/k3/board/lite/include/board_def.h @@ -22,7 +22,7 @@ * a single cluster of 4 processor. */ #define SEC_SRAM_BASE 0x70000000 /* Base of SRAM */ -#define SEC_SRAM_SIZE 0x0001a000 /* 104k */ +#define SEC_SRAM_SIZE 0x0001c000 /* 112k */ #define PLAT_MAX_OFF_STATE U(2) #define PLAT_MAX_RET_STATE U(1) From c9f887d8b4327491ecb30cf3a6a293894a69c8e8 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Fri, 26 Mar 2021 00:34:17 -0500 Subject: [PATCH 2/5] plat: ti: k3: platform_def.h: Define the correct number of max table entries Since we are using static xlat tables, we need to account for exact count of table entries we are actually using. peripherals usart, gic, gtc, sec_proxy_rt, scfg and data account for 6 entries and are constant, however, we also need to account for: bl31 full range, codebase, ro_data as additional 3 region With USE_COHERENT_MEM we do add in 1 extra region as well. This implies that we will have upto 9 or 10 regions based on USE_COHERENT_MEM usage. Vs we currently define 8 regions. This gets exposed with DEBUG=1 and assert checks trigger, which for some reason completely escaped testing previously. ASSERT: lib/xlat_tables_v2/xlat_tables_core.c:97 BACKTRACE: START: assert Signed-off-by: Nishanth Menon Change-Id: I962cdfc779b4eb3b914fe1c46023d50bc289e6bc --- plat/ti/k3/include/platform_def.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plat/ti/k3/include/platform_def.h b/plat/ti/k3/include/platform_def.h index f12fb0b21..81a383a72 100644 --- a/plat/ti/k3/include/platform_def.h +++ b/plat/ti/k3/include/platform_def.h @@ -60,7 +60,11 @@ * used, choose the smallest value needed to map the required virtual addresses * for each BL stage. */ -#define MAX_XLAT_TABLES 8 +#if USE_COHERENT_MEM +#define MAX_XLAT_TABLES 10 +#else +#define MAX_XLAT_TABLES 9 +#endif /* * Defines the maximum number of regions that are allocated by the translation From a2b56476bb341148b508d38550435af4176a25e4 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Fri, 26 Mar 2021 00:34:17 -0500 Subject: [PATCH 3/5] plat: ti: k3: common: bl31_setup: Use BL31_SIZE instead of computing We compute BL31_END - BL31_START on the fly, which is basically BL31_SIZE. Lets just use the BL31_SIZE directly so that we dont complicate PIE relocations when actual address is +ve and -ve offsets relative to link address. Signed-off-by: Nishanth Menon Change-Id: I5e14906381d2d059163800d39798eb39c42da4ec --- plat/ti/k3/common/k3_bl31_setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plat/ti/k3/common/k3_bl31_setup.c b/plat/ti/k3/common/k3_bl31_setup.c index ac4e60e4d..457c95dd6 100644 --- a/plat/ti/k3/common/k3_bl31_setup.c +++ b/plat/ti/k3/common/k3_bl31_setup.c @@ -101,7 +101,7 @@ void bl31_early_platform_setup2(u_register_t arg0, u_register_t arg1, void bl31_plat_arch_setup(void) { const mmap_region_t bl_regions[] = { - MAP_REGION_FLAT(BL31_START, BL31_END - BL31_START, MT_MEMORY | MT_RW | MT_SECURE), + MAP_REGION_FLAT(BL31_START, BL31_SIZE, MT_MEMORY | MT_RW | MT_SECURE), MAP_REGION_FLAT(BL_CODE_BASE, BL_CODE_END - BL_CODE_BASE, MT_CODE | MT_RO | MT_SECURE), MAP_REGION_FLAT(BL_RO_DATA_BASE, BL_RO_DATA_END - BL_RO_DATA_BASE, MT_RO_DATA | MT_RO | MT_SECURE), #if USE_COHERENT_MEM From f5872a00478dc99dc8199da03ef62fd297e7d110 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Fri, 26 Mar 2021 02:01:38 -0500 Subject: [PATCH 4/5] plat: ti: k3: board: Lets cast our macros Lets cast our macros to the right types and reduce a few MISRA warnings. Signed-off-by: Nishanth Menon Change-Id: I0dc06072713fe7c9440eca0635094c5f3ceb7f1c --- plat/ti/k3/board/generic/include/board_def.h | 10 +++++----- plat/ti/k3/board/lite/include/board_def.h | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/plat/ti/k3/board/generic/include/board_def.h b/plat/ti/k3/board/generic/include/board_def.h index 0d451167e..61753fb6b 100644 --- a/plat/ti/k3/board/generic/include/board_def.h +++ b/plat/ti/k3/board/generic/include/board_def.h @@ -19,14 +19,14 @@ * This RAM will be used for the bootloader including code, bss, and stacks. * It may need to be increased if BL31 grows in size. */ -#define SEC_SRAM_BASE 0x70000000 /* Base of MSMC SRAM */ -#define SEC_SRAM_SIZE 0x00020000 /* 128k */ +#define SEC_SRAM_BASE UL(0x70000000) /* Base of MSMC SRAM */ +#define SEC_SRAM_SIZE UL(0x00020000) /* 128k */ #define PLAT_MAX_OFF_STATE U(2) #define PLAT_MAX_RET_STATE U(1) -#define PLAT_PROC_START_ID 32 -#define PLAT_PROC_DEVICE_START_ID 202 -#define PLAT_CLUSTER_DEVICE_START_ID 198 +#define PLAT_PROC_START_ID U(32) +#define PLAT_PROC_DEVICE_START_ID U(202) +#define PLAT_CLUSTER_DEVICE_START_ID U(198) #endif /* BOARD_DEF_H */ diff --git a/plat/ti/k3/board/lite/include/board_def.h b/plat/ti/k3/board/lite/include/board_def.h index b363bea22..19587569c 100644 --- a/plat/ti/k3/board/lite/include/board_def.h +++ b/plat/ti/k3/board/lite/include/board_def.h @@ -21,14 +21,14 @@ * Current computation assumes data structures necessary for GIC and ARM for * a single cluster of 4 processor. */ -#define SEC_SRAM_BASE 0x70000000 /* Base of SRAM */ -#define SEC_SRAM_SIZE 0x0001c000 /* 112k */ +#define SEC_SRAM_BASE UL(0x70000000) /* Base of SRAM */ +#define SEC_SRAM_SIZE UL(0x0001c000) /* 112k */ #define PLAT_MAX_OFF_STATE U(2) #define PLAT_MAX_RET_STATE U(1) -#define PLAT_PROC_START_ID 32 -#define PLAT_PROC_DEVICE_START_ID 135 -#define PLAT_CLUSTER_DEVICE_START_ID 134 +#define PLAT_PROC_START_ID U(32) +#define PLAT_PROC_DEVICE_START_ID U(135) +#define PLAT_CLUSTER_DEVICE_START_ID U(134) #endif /* BOARD_DEF_H */ From 3dd87efb2e63249c7896dcae5324e1303bfc7b40 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Fri, 26 Mar 2021 02:06:40 -0500 Subject: [PATCH 5/5] plat: ti: k3: board: Let explicitly map our SEC_SRAM_BASE to 0x0 ENABLE_PIE (position independent executable) is default on K3 platform to handle variant RAM configurations in the system. This, unfortunately does cause confusion while reading the code, so, lets make things explicit by selecting 0x0 as the "SEC_SRAM_BASE" out of which we compute the BL31_BASE depending on usage. Lets also document a warning while at it to help folks copying code over to a custom K3 platform and optimizing size by disabling PIE to modify the defaults. Signed-off-by: Nishanth Menon Change-Id: I8e67a9210e907e266ff6a78ba4d02e3259bb2b21 --- plat/ti/k3/board/generic/include/board_def.h | 13 ++++++++++++- plat/ti/k3/board/lite/include/board_def.h | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/plat/ti/k3/board/generic/include/board_def.h b/plat/ti/k3/board/generic/include/board_def.h index 61753fb6b..4ff687cd8 100644 --- a/plat/ti/k3/board/generic/include/board_def.h +++ b/plat/ti/k3/board/generic/include/board_def.h @@ -18,8 +18,19 @@ /* * This RAM will be used for the bootloader including code, bss, and stacks. * It may need to be increased if BL31 grows in size. + * + * The link addresses are determined by SEC_SRAM_BASE + offset. + * When ENABLE_PIE is set, the TF images can be loaded anywhere, so + * SEC_SRAM_BASE is really arbitrary. + * + * When ENABLE_PIE is unset, SEC_SRAM_BASE should be chosen so that + * it matches to the physical address where BL31 is loaded, that is, + * SEC_SRAM_BASE should be the base address of the RAM region. + * + * Lets make things explicit by mapping SRAM_BASE to 0x0 since ENABLE_PIE is + * defined as default for our platform. */ -#define SEC_SRAM_BASE UL(0x70000000) /* Base of MSMC SRAM */ +#define SEC_SRAM_BASE UL(0x00000000) /* PIE remapped on fly */ #define SEC_SRAM_SIZE UL(0x00020000) /* 128k */ #define PLAT_MAX_OFF_STATE U(2) diff --git a/plat/ti/k3/board/lite/include/board_def.h b/plat/ti/k3/board/lite/include/board_def.h index 19587569c..18b7f4206 100644 --- a/plat/ti/k3/board/lite/include/board_def.h +++ b/plat/ti/k3/board/lite/include/board_def.h @@ -20,8 +20,19 @@ * It may need to be increased if BL31 grows in size. * Current computation assumes data structures necessary for GIC and ARM for * a single cluster of 4 processor. + * + * The link addresses are determined by SEC_SRAM_BASE + offset. + * When ENABLE_PIE is set, the TF images can be loaded anywhere, so + * SEC_SRAM_BASE is really arbitrary. + * + * When ENABLE_PIE is unset, SEC_SRAM_BASE should be chosen so that + * it matches to the physical address where BL31 is loaded, that is, + * SEC_SRAM_BASE should be the base address of the RAM region. + * + * Lets make things explicit by mapping SRAM_BASE to 0x0 since ENABLE_PIE is + * defined as default for our platform. */ -#define SEC_SRAM_BASE UL(0x70000000) /* Base of SRAM */ +#define SEC_SRAM_BASE UL(0x00000000) /* PIE remapped on fly */ #define SEC_SRAM_SIZE UL(0x0001c000) /* 112k */ #define PLAT_MAX_OFF_STATE U(2)