Change in coreboot[master]: program layout: align to 64 bytes when Boot Guard is enabled
Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43401 )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
program layout: align to 64 bytes when Boot Guard is enabled
FIT entries for IBB segmnets have 64 byte granularity. Enforce the 64 bytes alignment on all programs and FIT table to ensure the entires for IBB will be added correctly.
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I3ce09e24d716fe9845104205bc934bd2a0efc9cb --- M src/arch/x86/memlayout.ld M src/cpu/intel/fit/fit.S M src/lib/program.ld 3 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/43401/1
diff --git a/src/arch/x86/memlayout.ld b/src/arch/x86/memlayout.ld index 3659cc9..605ed24 100644 --- a/src/arch/x86/memlayout.ld +++ b/src/arch/x86/memlayout.ld @@ -47,3 +47,8 @@ #include <cpu/intel/fit/fit.ld> #endif #endif /* ENV_BOOTBLOCK */ + +/* Each program stage needs to be 64 bytes aligned to be added as FIT entry */ +#if CONFIG_INTEL_BOOTGUARD +. = ALIGN(64); +#endif diff --git a/src/cpu/intel/fit/fit.S b/src/cpu/intel/fit/fit.S index 3b7396c..037918a 100644 --- a/src/cpu/intel/fit/fit.S +++ b/src/cpu/intel/fit/fit.S @@ -9,7 +9,15 @@ .previous
.section .text +#if CONFIG(INTEL_BOOTGUARD) +/* + * FIT must be excluded from IBB with BootGuard. As the IBB segments have 64 + * byte granularity, align the FIT to 64 bytes. + */ +.align 64 +#else .align 16 +#endif .global fit_table .global fit_table_end fit_table: @@ -28,5 +36,12 @@ /* Checksum byte - must add to zero. */ .byte 0x7d .fill CONFIG_CPU_INTEL_NUM_FIT_ENTRIES*16 +#if CONFIG(INTEL_BOOTGUARD) +/* + * Just in case the FIT does not end on 64 byte aligned address, maintain the + * 64 byte boundaries of IBB segments. + */ +.align 64 +#endif fit_table_end: .previous diff --git a/src/lib/program.ld b/src/lib/program.ld index 88a3126..ee105e3 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -48,6 +48,10 @@ *(.rodata); *(.rodata.*); . = ALIGN(ARCH_POINTER_ALIGN_SIZE); +#if CONFIG_INTEL_BOOTGUARD +/* Each program stage needs to be 64 bytes aligned to be added as FIT entry */ + . = ALIGN(64); +#endif _etext = .; } : to_load
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43401 )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld@5... PS1, Line 51: /* Each program stage needs to be 64 bytes aligned to be added as FIT entry */ I don't immediately see any SECTIONS declarations following the ALIGN(64) below. Can you improve the commit message with the actual symbols names this affects/fixes.
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld@5... PS1, Line 52: #if CONFIG_INTEL_BOOTGUARD CONFIG(INTEL_BOOTGUARD) ?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43401 )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld@5... PS1, Line 51: /* Each program stage needs to be 64 bytes aligned to be added as FIT entry */
I don't immediately see any SECTIONS declarations following the ALIGN(64) below. […]
It should align the end of the file to 64 bytes
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43401 )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld@5... PS1, Line 53: . = ALIGN(64); If CPU_INTEL_FIRMWARE_INTERFACE_TABLE is select the program counter is modified. Which stage(s) are you wantig to be 64 byte aligned?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43401 )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld@5... PS1, Line 53: . = ALIGN(64);
If CPU_INTEL_FIRMWARE_INTERFACE_TABLE is select the program counter is modified. […]
These should be aligned for sure: bootblock, verstage and romstage
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43401 )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/43401/1/src/arch/x86/memlayout.ld@5... PS1, Line 53: . = ALIGN(64);
These should be aligned for sure: bootblock, verstage and romstage
Well, you want the .text aligned to 64 bytes. The cache as ram variables just so happen to be below the text segment for intel devices w/ memory mapped spi. I'd suggest adding the correct alignment to the .text sections.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43401?usp=email )
Change subject: program layout: align to 64 bytes when Boot Guard is enabled ......................................................................
Abandoned
-
Aaron Durbin (Code Review) -
Kyösti Mälkki (Code Review)
-
Michał Żygowski (Code Review)
-
Stefan Reinauer (Code Review)