From 82163aedc6f5787a7e9445ffc4602d1e07546f93 Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Sat, 12 Apr 2025 08:56:50 +0200 Subject: [PATCH] soc/amd/common/block/cpu/noncar: Move BSS and DATA out of PT_LOAD Currently .bss and .data are within the PT_LOAD area of the bootblock.elf and thus are placed and initialized at the correct spot when PSP loads the BIOS Reset Image into DRAM. On S3 resume PSP verifies that the "BIOS Reset Image" is unmodified before it hands over control to such. Due to the use of BSS and DATA within the BIOS Reset Image and the modifications of such at previous boot the verification always fails. This change moves '.bss' and '.data' out of the *first* PT_LOAD area and moves it into a separate data_segment also marked PT_LOAD. Since the second PT_LOAD is ignored by AMDCOMPRESS it doesn't end in the area being verified at S3 resume. Since '.data' is now part of a separate PT_LOAD a new region is inserted called '.datacopy' which is filled by using objcopy. In turn the assembly code in bootblock will memcpy '.datacopy' to '.data'. TEST: Can still boot on amd/birman+ and on up/squared. Change-Id: Id159ade3029060ce2ca6abcb723d5bdfe8841c3a Signed-off-by: Patrick Rudolph Reviewed-on: https://review.coreboot.org/c/coreboot/+/87305 Tested-by: build bot (Jenkins) Reviewed-by: Maximilian Brune --- src/cpu/x86/copy_data_section.inc | 7 ++- src/include/rules.h | 37 +++++++++++-- src/soc/amd/cezanne/Makefile.mk | 4 +- .../amd/common/block/cpu/noncar/Makefile.mk | 14 +++++ .../common/block/cpu/noncar/memlayout_x86.ld | 55 ++++++++++++++++++- src/soc/amd/common/block/cpu/noncar/pre_c.S | 3 + src/soc/amd/genoa_poc/Makefile.mk | 4 +- src/soc/amd/glinda/Makefile.mk | 4 +- src/soc/amd/mendocino/Makefile.mk | 4 +- src/soc/amd/phoenix/Makefile.mk | 4 +- src/soc/amd/picasso/Makefile.mk | 4 +- 11 files changed, 117 insertions(+), 23 deletions(-) diff --git a/src/cpu/x86/copy_data_section.inc b/src/cpu/x86/copy_data_section.inc index dccb8d3975..10e6f6c5d6 100644 --- a/src/cpu/x86/copy_data_section.inc +++ b/src/cpu/x86/copy_data_section.inc @@ -3,9 +3,10 @@ #if ENV_SEPARATE_DATA_AND_BSS /* - * Copy .data section content to Cache-As-Ram. - * This code can be included from 32 bits or 64 bits code. It also preserves - * registers. + * Copy .data section content from _data_load to it's linked + * address. Usually that's inside Cache-As-Ram. + * This code can be included from 32 bits or 64 bits code. + * It also preserves registers by using stack. */ copy_data_section: #if ENV_X86_64 diff --git a/src/include/rules.h b/src/include/rules.h index 1005058b48..673138bfe6 100644 --- a/src/include/rules.h +++ b/src/include/rules.h @@ -274,13 +274,38 @@ #if ENV_X86 /* Indicates memory layout is determined with arch/x86/car.ld. */ #define ENV_CACHE_AS_RAM (ENV_ROMSTAGE_OR_BEFORE && !CONFIG(RESET_VECTOR_IN_RAM)) -#else -#define ENV_CACHE_AS_RAM 0 -#endif -/* Indicates if the stage uses the _data and _bss regions defined in - * arch/x86/car.ld */ -#define ENV_SEPARATE_DATA_AND_BSS (ENV_CACHE_AS_RAM && (ENV_BOOTBLOCK || !CONFIG(NO_XIP_EARLY_STAGES))) +/* + * ENV_SEPARATE_DATA_AND_BSS: + * When not set .data and .bss are in the same PT_LOAD segment defined + * in program.ld. When set .data and .bss are in a different PT_LOAD segment, + * defined outside of program.ld. + * + * + * On Intel APL the bootblock is loaded into a 4KiB SRAM. + * There's no space for .data and .bss. + * Once the bootblock code has set up CAR it will use it for .data and .bss. + * The other stages are load into CAR and doesn't need separation. + * + * + * On Non CAR AMD platforms the bootblock is loaded into DRAM on cold boot. + * On S3 the loader verifies that the bootblock has the same hash as on cold boot. + * As it must not change .data and .bss are not part of the PT_LOAD segment and + * the assembly code must set up .bss and load .data. + * + * + * On other platforms that use Cache As RAM: + * By default CAR implies separate .data and .bss. + * The stages execute from memory mapped SPI flash and use CAR as heap. + */ +#define ENV_SEPARATE_DATA_AND_BSS (ENV_BOOTBLOCK || (ENV_CACHE_AS_RAM && !CONFIG(NO_XIP_EARLY_STAGES))) + +#else /* !ENV_X86 */ + +#define ENV_CACHE_AS_RAM 0 +#define ENV_SEPARATE_DATA_AND_BSS 0 + +#endif /* Currently ramstage has heap. */ #define ENV_HAS_HEAP_SECTION ENV_RAMSTAGE diff --git a/src/soc/amd/cezanne/Makefile.mk b/src/soc/amd/cezanne/Makefile.mk index 2747622a47..ee193e1bf2 100644 --- a/src/soc/amd/cezanne/Makefile.mk +++ b/src/soc/amd/cezanne/Makefile.mk @@ -115,7 +115,7 @@ PSP_APOB_BASE=$(CONFIG_PSP_APOB_DRAM_ADDRESS) # type = 0x62 PSP_BIOSBIN_FILE=$(obj)/amd_biospsp.img -PSP_ELF_FILE=$(objcbfs)/bootblock.elf +PSP_ELF_FILE=$(objcbfs)/bootblock_fixed_data.elf PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') PSP_BIOSBIN_DEST=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$3}') @@ -217,7 +217,7 @@ $(obj)/amdfw.rom: $(call strip_quotes, $(PSP_BIOSBIN_FILE)) \ $(DEP_FILES) \ $(AMDFWTOOL) \ $(obj)/fmap_config.h \ - $(objcbfs)/bootblock.elf # this target also creates the .map file + $(objcbfs)/bootblock_fixed_data.elf # this target also creates the .map file $(if $(PSP_APCB_FILES), ,$(error APCB_SOURCES is not set)) rm -f $@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" diff --git a/src/soc/amd/common/block/cpu/noncar/Makefile.mk b/src/soc/amd/common/block/cpu/noncar/Makefile.mk index f3ada6250f..80072b529d 100644 --- a/src/soc/amd/common/block/cpu/noncar/Makefile.mk +++ b/src/soc/amd/common/block/cpu/noncar/Makefile.mk @@ -14,4 +14,18 @@ romstage-y += cpu.c ramstage-y += memmap.c ramstage-y += mpinit.c +# See memlayout_x86.ld for details: +# AMDCOMPRESS only supports one PT_LOAD segment. +# 1. Copy .data section to .datacopy section +# Assembly will copy .datacopy to .data at runtime +# 2. Mark .data and .bss as NOLOAD +# Drops the second PT_LOAD segment from ELF +$(objcbfs)/bootblock_fixed_data.elf: $(objcbfs)/bootblock.elf + @printf " OBJCOPY $(notdir $(@))\n" + $(OBJCOPY_bootblock) --dump-section .data=$(objcbfs)/data.section $< + cp $< $@ + $(OBJCOPY_bootblock) --set-section-flags .datacopy=load \ + --set-section-flags .bss=noload --set-section-flags .data=noload $@ + $(OBJCOPY_bootblock) --update-section .datacopy=$(objcbfs)/data.section $@ + endif # CONFIG_SOC_AMD_COMMON_BLOCK_NONCAR diff --git a/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld b/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld index 3c596f4c20..1161e38448 100644 --- a/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld +++ b/src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld @@ -130,14 +130,64 @@ MEMORY } SECTIONS { - /* Page tables need to be at a 4K boundary so align the bootblock downwards */ + /* + * AMD's PSP will verify the integrity of the BIOS Reset Image at S3 resume, + * thus it must not change during execution. Having .data and .bss between + * .text and .reset breaks the integrity, as .bss or .data might get modified. + * + * Enable ENV_SEPARATE_DATA_AND_BSS for AMD's bootblock and mark as + * data_segment to place it in the second PT_LOAD area. AMDCOMPRESS will ignore + * the second PT_LOAD area and discard it. + */ + . = BOOTBLOCK_ADDR & ~(16 - 1); + .data . : { + _data = .; + *(.data); + *(.data.*); + *(.ldata); + *(.ldata.*); + *(.sdata); + *(.sdata.*); + . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + _edata = .; + RECORD_SIZE(data) + } : data_segment + + . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + + .bss . (NOLOAD) : { + _bss = .; + *(.bss) + *(.bss.*) + *(.lbss) + *(.lbss.*) + *(.sbss) + *(.sbss.*) + . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + _ebss = .; + RECORD_SIZE(bss) + } : data_segment + . = (BOOTBLOCK_TOP - PROGRAM_SZ) & ~(4096 - 1); _bootblock = .; INCLUDE "bootblock/lib/program.ld" - PROGRAM_SZ = SIZEOF(.text) + SIZEOF(.bss) + SIZEOF(.data); + /* + * Add free space for a copy of the data section. The assembly code will copy + * the section to .data at runtime when ENV_SEPARATE_DATA_AND_BSS is set. + * + * objcopy will be used during build to copy the .data to the .datacopy section. + * This ensures that only *one* loadable segment is being used as bootblock. + */ + .datacopy . : { + _datacopy = .; + . += SIZEOF(.data); + _edatacopy = .; + } : to_load = 0xff + _data_load = LOADADDR(.datacopy); + PROGRAM_SZ = SIZEOF(.text) + SIZEOF(.data); . = _X86_RESET_VECTOR - EARLYASM_SZ; . = ALIGN(16); BOOTBLOCK_TOP = .; @@ -159,6 +209,7 @@ SECTIONS { .reset (_X86_RESET_VECTOR) : { *(.reset); } >resetsection + .last_byte (BOOTBLOCK_END - 1) : { BYTE(0xff); } >resetsection diff --git a/src/soc/amd/common/block/cpu/noncar/pre_c.S b/src/soc/amd/common/block/cpu/noncar/pre_c.S index b75458815e..3dc3dd30f0 100644 --- a/src/soc/amd/common/block/cpu/noncar/pre_c.S +++ b/src/soc/amd/common/block/cpu/noncar/pre_c.S @@ -42,6 +42,9 @@ bootblock_pre_c_entry: movl $_eearlyram_stack, %esp + /* Copy .data section content */ +#include + /* Align the stack and keep aligned for call to bootblock_c_entry() */ and $0xfffffff0, %esp sub $8, %esp diff --git a/src/soc/amd/genoa_poc/Makefile.mk b/src/soc/amd/genoa_poc/Makefile.mk index 8a62852157..6e98378542 100644 --- a/src/soc/amd/genoa_poc/Makefile.mk +++ b/src/soc/amd/genoa_poc/Makefile.mk @@ -75,7 +75,7 @@ PSP_APOB_BASE=$(CONFIG_PSP_APOB_DRAM_ADDRESS) # type = 0x62 PSP_BIOSBIN_FILE=$(obj)/amd_biospsp.img -PSP_ELF_FILE=$(objcbfs)/bootblock.elf +PSP_ELF_FILE=$(objcbfs)/bootblock_fixed_data.elf PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') PSP_BIOSBIN_DEST=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$3}') @@ -134,7 +134,7 @@ $(obj)/amdfw.rom: $(call strip_quotes, $(PSP_BIOSBIN_FILE)) \ $(DEP_FILES) \ $(AMDFWTOOL) \ $(obj)/fmap_config.h \ - $(objcbfs)/bootblock.elf # this target also creates the .map file + $(objcbfs)/bootblock_fixed_data.elf # this target also creates the .map file $(if $(PSP_APCB_FILES), ,$(error APCB_SOURCES is not set)) rm -f $@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" diff --git a/src/soc/amd/glinda/Makefile.mk b/src/soc/amd/glinda/Makefile.mk index eb488d6a0f..f4c55c6e33 100644 --- a/src/soc/amd/glinda/Makefile.mk +++ b/src/soc/amd/glinda/Makefile.mk @@ -122,7 +122,7 @@ PSP_APOB_BASE=$(CONFIG_PSP_APOB_DRAM_ADDRESS) # type = 0x62 PSP_BIOSBIN_FILE=$(obj)/amd_biospsp.img -PSP_ELF_FILE=$(objcbfs)/bootblock.elf +PSP_ELF_FILE=$(objcbfs)/bootblock_fixed_data.elf PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') PSP_BIOSBIN_DEST=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$3}') @@ -237,7 +237,7 @@ $(obj)/amdfw.rom: $(call strip_quotes, $(PSP_BIOSBIN_FILE)) \ $(DEP_FILES) \ $(AMDFWTOOL) \ $(obj)/fmap_config.h \ - $(objcbfs)/bootblock.elf # this target also creates the .map file + $(objcbfs)/bootblock_fixed_data.elf # this target also creates the .map file rm -f $@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ diff --git a/src/soc/amd/mendocino/Makefile.mk b/src/soc/amd/mendocino/Makefile.mk index 06ac55bda4..b8fec5701c 100644 --- a/src/soc/amd/mendocino/Makefile.mk +++ b/src/soc/amd/mendocino/Makefile.mk @@ -124,7 +124,7 @@ PSP_APOB_BASE=$(CONFIG_PSP_APOB_DRAM_ADDRESS) # type = 0x62 PSP_BIOSBIN_FILE=$(obj)/amd_biospsp.img -PSP_ELF_FILE=$(objcbfs)/bootblock.elf +PSP_ELF_FILE=$(objcbfs)/bootblock_fixed_data.elf PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') PSP_BIOSBIN_DEST=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$3}') @@ -264,7 +264,7 @@ $(obj)/amdfw.rom: $(call strip_quotes, $(PSP_BIOSBIN_FILE)) \ $(DEP_FILES) \ $(AMDFWTOOL) \ $(obj)/fmap_config.h \ - $(objcbfs)/bootblock.elf # this target also creates the .map file + $(objcbfs)/bootblock_fixed_data.elf # this target also creates the .map file rm -f $@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ diff --git a/src/soc/amd/phoenix/Makefile.mk b/src/soc/amd/phoenix/Makefile.mk index 68cc935f75..21f10c56b9 100644 --- a/src/soc/amd/phoenix/Makefile.mk +++ b/src/soc/amd/phoenix/Makefile.mk @@ -128,7 +128,7 @@ PSP_APOB_BASE=$(CONFIG_PSP_APOB_DRAM_ADDRESS) # type = 0x62 PSP_BIOSBIN_FILE=$(obj)/amd_biospsp.img -PSP_ELF_FILE=$(objcbfs)/bootblock.elf +PSP_ELF_FILE=$(objcbfs)/bootblock_fixed_data.elf PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') PSP_BIOSBIN_DEST=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$3}') @@ -265,7 +265,7 @@ $(obj)/amdfw.rom: $(call strip_quotes, $(PSP_BIOSBIN_FILE)) \ $(DEP_FILES) \ $(AMDFWTOOL) \ $(obj)/fmap_config.h \ - $(objcbfs)/bootblock.elf # this target also creates the .map file + $(objcbfs)/bootblock_fixed_data.elf # this target also creates the .map file rm -f $@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n" $(AMDFWTOOL) \ diff --git a/src/soc/amd/picasso/Makefile.mk b/src/soc/amd/picasso/Makefile.mk index 7ba975f0e4..0b574db845 100644 --- a/src/soc/amd/picasso/Makefile.mk +++ b/src/soc/amd/picasso/Makefile.mk @@ -116,7 +116,7 @@ PSP_APOB_BASE=$(CONFIG_PSP_APOB_DRAM_ADDRESS) # type = 0x62 PSP_BIOSBIN_FILE=$(obj)/amd_biospsp.img -PSP_ELF_FILE=$(objcbfs)/bootblock.elf +PSP_ELF_FILE=$(objcbfs)/bootblock_fixed_data.elf PSP_BIOSBIN_SIZE=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$5}') PSP_BIOSBIN_DEST=$(shell $(READELF_bootblock) -Wl $(PSP_ELF_FILE) | grep LOAD | awk '{print $$3}') @@ -225,7 +225,7 @@ $(obj)/amdfw.rom: $(call strip_quotes, $(PSP_BIOSBIN_FILE)) \ $(DEP_FILES) \ $(AMDFWTOOL) \ $(obj)/fmap_config.h \ - $(objcbfs)/bootblock.elf # this target also creates the .map file + $(objcbfs)/bootblock_fixed_data.elf # this target also creates the .map file $(if $(PSP_APCB_FILES), ,$(error APCB_SOURCES is not set)) rm -f $@ @printf " AMDFWTOOL $(subst $(obj)/,,$(@))\n"