Ting Shen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31123
Change subject: bootmem: add new memory type for BL31 ......................................................................
bootmem: add new memory type for BL31
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/include/bootmem.h M src/lib/bootmem.c 3 files changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/1
diff --git a/src/arch/arm64/arm_tf.c b/src/arch/arm64/arm_tf.c index b74f7be..3f1aa2a 100644 --- a/src/arch/arm64/arm_tf.c +++ b/src/arch/arm64/arm_tf.c @@ -50,7 +50,7 @@ if (prog_locate(&bl31)) die("BL31 not found");
- if (!selfload_check(&bl31, BM_MEM_RESERVED)) + if (!selfload_check(&bl31, BM_MEM_BL31)) die("BL31 load failed"); bl31_entry = prog_entry(&bl31);
diff --git a/src/include/bootmem.h b/src/include/bootmem.h index fb67b13..0e6ba50 100644 --- a/src/include/bootmem.h +++ b/src/include/bootmem.h @@ -37,6 +37,7 @@ BM_MEM_NVS, /* ACPI NVS Memory */ BM_MEM_UNUSABLE, /* Unusable address space */ BM_MEM_VENDOR_RSVD, /* Vendor Reserved */ + BM_MEM_BL31, /* BL31 exectuable */ BM_MEM_TABLE, /* Ram configuration tables are kept in */ /* Tags below this point are ignored for the OS table. */ BM_MEM_OS_CUTOFF = BM_MEM_TABLE, @@ -53,6 +54,7 @@ * Bootmem types match to LB_MEM tags, except for the following: * BM_MEM_RAMSTAGE : Translates to LB_MEM_RAM. * BM_MEM_PAYLOAD : Translates to LB_MEM_RAM. + * BM_MEM_BL31 : Translates to LB_MEM_RESERVED. */ void bootmem_write_memory_table(struct lb_memory *mem);
diff --git a/src/lib/bootmem.c b/src/lib/bootmem.c index a7b7dd9..7fa6855 100644 --- a/src/lib/bootmem.c +++ b/src/lib/bootmem.c @@ -59,6 +59,8 @@ return LB_MEM_UNUSABLE; case BM_MEM_VENDOR_RSVD: return LB_MEM_VENDOR_RSVD; + case BM_MEM_BL31: + return LB_MEM_RESERVED; case BM_MEM_TABLE: return LB_MEM_TABLE; default:
Hello Julius Werner, Hung-Te Lin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#2).
Change subject: bootmem: add new memory type for BL31 ......................................................................
bootmem: add new memory type for BL31
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/include/bootmem.h M src/lib/bootmem.c 3 files changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 2: Code-Review+1
Probably need to check if some existing boards doing BL31 need to change their layout or something.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31123/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31123/2//COMMIT_MSG@8 PS2, Line 8: Please elaborate, why this is needed.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 2:
That will break all platforms using BL31, wouldn't it ? You need to add a BM_MEM_BL31 region on all platforms first.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 2:
Right, you need to actually update all the arm64 memlayouts to include this area, and you should also remove the corresponding mmio_resource() lines from SoC. Otherwise this looks good.
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#3).
Change subject: bootmem: add new memory type for BL31 ......................................................................
bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/include/bootmem.h M src/lib/bootmem.c 3 files changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/3
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#5).
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
WIP: bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/include/bootmem.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/soc.c M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/soc.c 6 files changed, 17 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/5
Ting Shen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 6: Code-Review-1
(1 comment)
Patch Set 2:
Right, you need to actually update all the arm64 memlayouts to include this area, and you should also remove the corresponding mmio_resource() lines from SoC. Otherwise this looks good.
Is there any way to verify this? I can only test this on a mt8173 device I have, but each platform uses different approach to setup its reserved range.. (Or maybe it's not a good idea to break compatibility?)
https://review.coreboot.org/#/c/31123/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31123/2//COMMIT_MSG@8 PS2, Line 8:
Please elaborate, why this is needed.
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 6:
(3 comments)
Is there any way to verify this? I can only test this on a mt8173 device I have, but each platform uses different approach to setup its reserved range.. (Or maybe it's not a good idea to break compatibility?)
Review is enough... we aren't *that* afraid of breaking things in coreboot, and this change isn't crazy hard to grok. Stifling all innovation because people are too afraid of breaking old junk isn't great either.
https://review.coreboot.org/#/c/31123/6/src/soc/cavium/cn81xx/soc.c File src/soc/cavium/cn81xx/soc.c:
https://review.coreboot.org/#/c/31123/6/src/soc/cavium/cn81xx/soc.c@332 PS6, Line 332: BM_MEM_BL31); Rather than doing this in every SoC this should be added to arm64's bootmem_arch_ranges(). You can use DECLARE_OPTIONAL_REGION(bl31) and test for _bl31_size to be non-zero before adding it so that platform with some weird special requirements have an out.
https://review.coreboot.org/#/c/31123/6/src/soc/nvidia/tegra210/soc.c File src/soc/nvidia/tegra210/soc.c:
https://review.coreboot.org/#/c/31123/6/src/soc/nvidia/tegra210/soc.c@53 PS6, Line 53: bootmem_add_range(begin * KiB * KiB, size * KiB * KiB, type); This is one of those examples where it's probably a good idea to have the platform handle it separately and not put it in memlayout.
https://review.coreboot.org/#/c/31123/6/src/soc/rockchip/rk3399/soc.c File src/soc/rockchip/rk3399/soc.c:
https://review.coreboot.org/#/c/31123/6/src/soc/rockchip/rk3399/soc.c@41 PS6, Line 41: bootmem_add_range(TZRAM_BASE, TZRAM_SIZE, BM_MEM_BL31); FYI, the range here (that should now be hardcoded in memlayout) is [0x0:0x100000] (from https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch...).
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#7).
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
WIP: bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/arch/arm64/tables.c M src/include/bootmem.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/include/soc/memlayout.ld M src/soc/rockchip/rk3399/include/soc/memlayout.ld 8 files changed, 20 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/7
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 7:
Is this still WIP?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/#/c/31123/7/src/arch/arm64/tables.c File src/arch/arm64/tables.c:
https://review.coreboot.org/#/c/31123/7/src/arch/arm64/tables.c@27 PS7, Line 27: #define _bl31_size (_ebl31 - _bl31) Please put these three into <symbols.h>
https://review.coreboot.org/#/c/31123/7/src/include/bootmem.h File src/include/bootmem.h:
https://review.coreboot.org/#/c/31123/7/src/include/bootmem.h@40 PS7, Line 40: nit: maybe add "arm64" for clarification
https://review.coreboot.org/#/c/31123/7/src/soc/mediatek/mt8183/include/soc/... File src/soc/mediatek/mt8183/include/soc/memlayout.ld:
https://review.coreboot.org/#/c/31123/7/src/soc/mediatek/mt8183/include/soc/... PS7, Line 51: REGION(bl31 Please define a new BL31() macro for this in <arch/memlayout.h>
https://review.coreboot.org/#/c/31123/7/src/soc/nvidia/tegra210/include/soc/... File src/soc/nvidia/tegra210/include/soc/memlayout.ld:
https://review.coreboot.org/#/c/31123/7/src/soc/nvidia/tegra210/include/soc/... PS7, Line 50: CONFIG_TRUSTZONE_CARVEOUT_SIZE_MB * 1M, 0x1000) ...yeah, you can't do that because memlayout intentionally doesn't let you overlap regions. One solution would be to manually put
_ebl31 = _ettb; _bl31 = _ebl31 - CONFIG_TRUSTZONE_CARVEOUT_SIZE_MB * 1M;
in here. But that's pretty ugly, so maybe you should just treat tegra210 specially and give it its own bootmem_platform_add_ranges() as I mentioned before (without putting BL31 in memlayout).
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#8).
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
WIP: bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/tables.c M src/include/bootmem.h M src/include/symbols.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8173/include/soc/memlayout.ld M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/include/soc/memlayout.ld 12 files changed, 37 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/8
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/#/c/31123/8/src/arch/arm64/include/arch/memlayou... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/8/src/arch/arm64/include/arch/memlayou... PS8, Line 40: #define BL31(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop
https://review.coreboot.org/#/c/31123/8/src/arch/arm64/include/arch/memlayou... PS8, Line 40: #define BL31(addr, size) \ macros should not use a trailing semicolon
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#9).
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
WIP: bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/tables.c M src/include/bootmem.h M src/include/symbols.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8173/soc.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/include/soc/memlayout.ld 12 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/#/c/31123/9/src/arch/arm64/include/arch/memlayou... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/9/src/arch/arm64/include/arch/memlayou... PS9, Line 40: #define BL31(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop
https://review.coreboot.org/#/c/31123/9/src/arch/arm64/include/arch/memlayou... PS9, Line 40: #define BL31(addr, size) \ macros should not use a trailing semicolon
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#10).
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
WIP: bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/tables.c M src/include/bootmem.h M src/include/symbols.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8173/soc.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/include/soc/memlayout.ld 12 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/#/c/31123/10/src/arch/arm64/include/arch/memlayo... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/10/src/arch/arm64/include/arch/memlayo... PS10, Line 40: #define BL31(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop
https://review.coreboot.org/#/c/31123/10/src/arch/arm64/include/arch/memlayo... PS10, Line 40: #define BL31(addr, size) \ macros should not use a trailing semicolon
Ting Shen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: WIP: bootmem: add new memory type for BL31 ......................................................................
Patch Set 10:
(5 comments)
This is ready for review.
Hi Patrick, can you help verify this? Thanks.
https://review.coreboot.org/#/c/31123/7/src/arch/arm64/tables.c File src/arch/arm64/tables.c:
https://review.coreboot.org/#/c/31123/7/src/arch/arm64/tables.c@27 PS7, Line 27: #define _bl31_size (_ebl31 - _bl31)
Please put these three into <symbols. […]
Done
https://review.coreboot.org/#/c/31123/7/src/include/bootmem.h File src/include/bootmem.h:
https://review.coreboot.org/#/c/31123/7/src/include/bootmem.h@40 PS7, Line 40:
nit: maybe add "arm64" for clarification
Done
https://review.coreboot.org/#/c/31123/6/src/soc/cavium/cn81xx/soc.c File src/soc/cavium/cn81xx/soc.c:
https://review.coreboot.org/#/c/31123/6/src/soc/cavium/cn81xx/soc.c@332 PS6, Line 332: BM_MEM_BL31);
Rather than doing this in every SoC this should be added to arm64's bootmem_arch_ranges(). […]
Done
https://review.coreboot.org/#/c/31123/7/src/soc/mediatek/mt8183/include/soc/... File src/soc/mediatek/mt8183/include/soc/memlayout.ld:
https://review.coreboot.org/#/c/31123/7/src/soc/mediatek/mt8183/include/soc/... PS7, Line 51: REGION(bl31
Please define a new BL31() macro for this in <arch/memlayout. […]
Done
https://review.coreboot.org/#/c/31123/7/src/soc/nvidia/tegra210/include/soc/... File src/soc/nvidia/tegra210/include/soc/memlayout.ld:
https://review.coreboot.org/#/c/31123/7/src/soc/nvidia/tegra210/include/soc/... PS7, Line 50: CONFIG_TRUSTZONE_CARVEOUT_SIZE_MB * 1M, 0x1000)
...yeah, you can't do that because memlayout intentionally doesn't let you overlap regions. […]
Done
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#11).
Change subject: bootmem: add new memory type for BL31 ......................................................................
bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/tables.c M src/include/bootmem.h M src/include/symbols.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8173/soc.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/include/soc/memlayout.ld 12 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/11
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 11: Code-Review+1
(1 comment)
Tested on opencellular/elgon. Still able to load BL31.
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c File src/soc/nvidia/tegra210/soc.c:
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c@44 PS11, Line 44: bootmem_add_range(begin * KiB * KiB, size * KiB * KiB, BM_MEM_BL31); MiB
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 11: Code-Review+2
(3 comments)
https://review.coreboot.org/#/c/31123/11/src/arch/arm64/include/arch/memlayo... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/11/src/arch/arm64/include/arch/memlayo... PS11, Line 41: 1K nit: Did you base this number on anything? The BL31 linker script aligns start and end of the ELF to PAGE_SIZE, so might as well make it 4K. (In practice, the required alignment depends on the secure address range protection granularity, which is usually higher but entirely platform-dependent.)
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c File src/soc/nvidia/tegra210/soc.c:
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c@41 PS11, Line 41: 0 nit: CARVEOUT_TZ
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c@53 PS11, Line 53: for (i = 1; i < CARVEOUT_NUM; i++) { ...and here maybe (CARVEOUT_TZ + 1)?
Hello Patrick Rudolph, Julius Werner, Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31123
to look at the new patch set (#12).
Change subject: bootmem: add new memory type for BL31 ......................................................................
bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com --- M src/arch/arm64/arm_tf.c M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/tables.c M src/include/bootmem.h M src/include/symbols.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8173/soc.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/include/soc/memlayout.ld 12 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/31123/12
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/#/c/31123/12/src/arch/arm64/include/arch/memlayo... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/12/src/arch/arm64/include/arch/memlayo... PS12, Line 40: #define BL31(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop
https://review.coreboot.org/#/c/31123/12/src/arch/arm64/include/arch/memlayo... PS12, Line 40: #define BL31(addr, size) \ macros should not use a trailing semicolon
Ting Shen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 12:
(4 comments)
https://review.coreboot.org/#/c/31123/11/src/arch/arm64/include/arch/memlayo... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/11/src/arch/arm64/include/arch/memlayo... PS11, Line 41: 1K
nit: Did you base this number on anything? The BL31 linker script aligns start and end of the ELF to […]
Done. It was copied from cavium/cn81xx/include/soc/memlayout.ld.
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c File src/soc/nvidia/tegra210/soc.c:
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c@41 PS11, Line 41: 0
nit: CARVEOUT_TZ
Done
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c@44 PS11, Line 44: bootmem_add_range(begin * KiB * KiB, size * KiB * KiB, BM_MEM_BL31);
MiB
Done
https://review.coreboot.org/#/c/31123/11/src/soc/nvidia/tegra210/soc.c@53 PS11, Line 53: for (i = 1; i < CARVEOUT_NUM; i++) {
... […]
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/#/c/31123/11/src/arch/arm64/include/arch/memlayo... File src/arch/arm64/include/arch/memlayout.h:
https://review.coreboot.org/#/c/31123/11/src/arch/arm64/include/arch/memlayo... PS11, Line 41: 1K
Done. […]
PAGE_SIZE Sounds good, as the protection granularity is 1MiB on cn81xx
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
Patch Set 12: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31123 )
Change subject: bootmem: add new memory type for BL31 ......................................................................
bootmem: add new memory type for BL31
After CL:31122, we can finally define a memory type specific for BL31, to make sure BL31 is not loaded on other reserved area.
Change-Id: Idbd9a7fe4b12af23de1519892936d8d88a000e2c Signed-off-by: Ting Shen phoenixshen@google.com Reviewed-on: https://review.coreboot.org/c/31123 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/arch/arm64/arm_tf.c M src/arch/arm64/include/arch/memlayout.h M src/arch/arm64/tables.c M src/include/bootmem.h M src/include/symbols.h M src/lib/bootmem.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/cavium/cn81xx/soc.c M src/soc/mediatek/mt8173/soc.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra210/soc.c M src/soc/rockchip/rk3399/include/soc/memlayout.ld 12 files changed, 41 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/arch/arm64/arm_tf.c b/src/arch/arm64/arm_tf.c index c0526e0..3f1aa2a 100644 --- a/src/arch/arm64/arm_tf.c +++ b/src/arch/arm64/arm_tf.c @@ -50,7 +50,7 @@ if (prog_locate(&bl31)) die("BL31 not found");
- if (!selfload(&bl31)) + if (!selfload_check(&bl31, BM_MEM_BL31)) die("BL31 load failed"); bl31_entry = prog_entry(&bl31);
diff --git a/src/arch/arm64/include/arch/memlayout.h b/src/arch/arm64/include/arch/memlayout.h index 7fce9aa..a3fdd66 100644 --- a/src/arch/arm64/include/arch/memlayout.h +++ b/src/arch/arm64/include/arch/memlayout.h @@ -37,4 +37,8 @@ REGION(stack, addr, size, 16) \ _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.inc");
+#define BL31(addr, size) \ + REGION(bl31, addr, size, 4K) \ + _ = ASSERT(size % 4K == 0, "BL31 size must be divisible by 4K!"); + #endif /* __ARCH_MEMLAYOUT_H */ diff --git a/src/arch/arm64/tables.c b/src/arch/arm64/tables.c index f36679e..ec949fd 100644 --- a/src/arch/arm64/tables.c +++ b/src/arch/arm64/tables.c @@ -20,6 +20,8 @@ #include <boot/coreboot_tables.h> #include <symbols.h>
+DECLARE_OPTIONAL_REGION(bl31); + void arch_write_tables(uintptr_t coreboot_table) { } @@ -28,6 +30,9 @@ { bootmem_add_range((uintptr_t)_ttb, _ttb_size, BM_MEM_RAMSTAGE);
+ if (IS_ENABLED(CONFIG_ARM64_USE_ARM_TRUSTED_FIRMWARE) && _bl31_size > 0) + bootmem_add_range((uintptr_t)_bl31, _bl31_size, BM_MEM_BL31); + if (!IS_ENABLED(CONFIG_COMMON_CBFS_SPI_WRAPPER)) return; bootmem_add_range((uintptr_t)_postram_cbfs_cache, diff --git a/src/include/bootmem.h b/src/include/bootmem.h index 4652d08..c935cb9 100644 --- a/src/include/bootmem.h +++ b/src/include/bootmem.h @@ -37,6 +37,7 @@ BM_MEM_NVS, /* ACPI NVS Memory */ BM_MEM_UNUSABLE, /* Unusable address space */ BM_MEM_VENDOR_RSVD, /* Vendor Reserved */ + BM_MEM_BL31, /* Arm64 BL31 exectuable */ BM_MEM_TABLE, /* Ram configuration tables are kept in */ /* Tags below this point are ignored for the OS table. */ BM_MEM_OS_CUTOFF = BM_MEM_TABLE, @@ -53,6 +54,7 @@ * Bootmem types match to LB_MEM tags, except for the following: * BM_MEM_RAMSTAGE : Translates to LB_MEM_RAM. * BM_MEM_PAYLOAD : Translates to LB_MEM_RAM. + * BM_MEM_BL31 : Translates to LB_MEM_RESERVED. */ void bootmem_write_memory_table(struct lb_memory *mem);
diff --git a/src/include/symbols.h b/src/include/symbols.h index fc9ef21..abb9fbe 100644 --- a/src/include/symbols.h +++ b/src/include/symbols.h @@ -114,6 +114,10 @@ extern u8 _epdpt[]; #define _pdpt_size (_epdpt - _pdpt)
+extern u8 _bl31[]; +extern u8 _ebl31[]; +#define _bl31_size (_ebl31 - _bl31) + /* Put this into a .c file accessing a linker script region to mark that region * as "optional". If it is defined in memlayout.ld (or anywhere else), the * values from that definition will be used. If not, start, end and size will diff --git a/src/lib/bootmem.c b/src/lib/bootmem.c index c804df5..7cc8fff 100644 --- a/src/lib/bootmem.c +++ b/src/lib/bootmem.c @@ -59,6 +59,8 @@ return LB_MEM_UNUSABLE; case BM_MEM_VENDOR_RSVD: return LB_MEM_VENDOR_RSVD; + case BM_MEM_BL31: + return LB_MEM_RESERVED; case BM_MEM_TABLE: return LB_MEM_TABLE; default: @@ -142,6 +144,7 @@ { BM_MEM_NVS, "NVS" }, { BM_MEM_UNUSABLE, "UNUSABLE" }, { BM_MEM_VENDOR_RSVD, "VENDOR RESERVED" }, + { BM_MEM_BL31, "BL31" }, { BM_MEM_TABLE, "CONFIGURATION TABLES" }, { BM_MEM_RAMSTAGE, "RAMSTAGE" }, { BM_MEM_PAYLOAD, "PAYLOAD" }, diff --git a/src/soc/cavium/cn81xx/include/soc/memlayout.ld b/src/soc/cavium/cn81xx/include/soc/memlayout.ld index e3bf61f..2222617 100644 --- a/src/soc/cavium/cn81xx/include/soc/memlayout.ld +++ b/src/soc/cavium/cn81xx/include/soc/memlayout.ld @@ -22,7 +22,7 @@ { DRAM_START(0x00000000) /* Secure region 0 - 1MiB */ - REGION(bl31, 0, 0xe0000, 0x1000) + BL31(0, 0xe0000) REGION(sff8104, 0xe0000, 0x20000, 0x1000)
/* Insecure region 1MiB - TOP OF DRAM */ diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 2046d21..4b265d7 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -319,18 +319,11 @@ return 0; }
-extern u8 _bl31[]; -extern u8 _ebl31[]; extern u8 _sff8104[]; extern u8 _esff8104[];
void bootmem_platform_add_ranges(void) { - /* ATF reserved */ - bootmem_add_range((uintptr_t)_bl31, - ((uintptr_t)_ebl31 - (uintptr_t)_bl31), - BM_MEM_RESERVED); - bootmem_add_range((uintptr_t)_sff8104, ((uintptr_t)_esff8104 - (uintptr_t)_sff8104), BM_MEM_RESERVED); diff --git a/src/soc/mediatek/mt8173/soc.c b/src/soc/mediatek/mt8173/soc.c index 37ceb34..b5c805a 100644 --- a/src/soc/mediatek/mt8173/soc.c +++ b/src/soc/mediatek/mt8173/soc.c @@ -13,10 +13,16 @@ * GNU General Public License for more details. */
+#include <bootmem.h> #include <device/device.h> #include <symbols.h> #include <soc/emi.h>
+void bootmem_platform_add_ranges(void) +{ + bootmem_add_range(0x101000, 124 * KiB, BM_MEM_BL31); +} + static void soc_read_resources(struct device *dev) { ram_resource(dev, 0, (uintptr_t)_dram / KiB, sdram_size() / KiB); diff --git a/src/soc/mediatek/mt8183/include/soc/memlayout.ld b/src/soc/mediatek/mt8183/include/soc/memlayout.ld index 2a6d42d..a547083 100644 --- a/src/soc/mediatek/mt8183/include/soc/memlayout.ld +++ b/src/soc/mediatek/mt8183/include/soc/memlayout.ld @@ -47,4 +47,6 @@ DRAM_START(0x40000000) POSTRAM_CBFS_CACHE(0x40000000, 2M) RAMSTAGE(0x40200000, 256K) + + BL31(0x54600000, 0x60000) } diff --git a/src/soc/nvidia/tegra210/soc.c b/src/soc/nvidia/tegra210/soc.c index 71532be..619d27f 100644 --- a/src/soc/nvidia/tegra210/soc.c +++ b/src/soc/nvidia/tegra210/soc.c @@ -16,6 +16,7 @@
#include <arch/io.h> #include <arch/cache.h> +#include <bootmem.h> #include <bootmode.h> #include <bootstate.h> #include <console/console.h> @@ -33,13 +34,23 @@
#include "chip.h"
+void bootmem_platform_add_ranges(void) +{ + uintptr_t begin; + size_t size; + carveout_range(CARVEOUT_TZ, &begin, &size); + if (size == 0) + return; + bootmem_add_range(begin * MiB, size * MiB, BM_MEM_BL31); +} + static void soc_read_resources(struct device *dev) { unsigned long index = 0; int i; uintptr_t begin, end; size_t size;
- for (i = 0; i < CARVEOUT_NUM; i++) { + for (i = CARVEOUT_TZ + 1; i < CARVEOUT_NUM; i++) { carveout_range(i, &begin, &size); if (size == 0) continue; diff --git a/src/soc/rockchip/rk3399/include/soc/memlayout.ld b/src/soc/rockchip/rk3399/include/soc/memlayout.ld index e181a35..01e352f 100644 --- a/src/soc/rockchip/rk3399/include/soc/memlayout.ld +++ b/src/soc/rockchip/rk3399/include/soc/memlayout.ld @@ -19,6 +19,7 @@ SECTIONS { DRAM_START(0x00000000) + BL31(0, 0x100000) POSTRAM_CBFS_CACHE(0x00100000, 1M) RAMSTAGE(0x00300000, 256K) DMA_COHERENT(0x10000000, 2M)
-
build bot (Jenkins) (Code Review) -
Hung-Te Lin (Code Review)
-
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Ting Shen (Code Review)