Hello Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42888
to review the following change.
Change subject: soc/amd/picasso: Enable paging on S3 resume ......................................................................
soc/amd/picasso: Enable paging on S3 resume
This way we can prevent the BSP from accidentally stomping on OS memory.
BUG=b:159081993 TEST=S3 suspend and resume to OS. See paging enabled message. Ran suspend_stress_test for 20+ cycles.
Signed-off-by: Furquan Shaikh furquan@google.com Signed-off-by: Raul E Rangel rrangel@google.com Change-Id: I58fed6f297831b3cdc2758a5e7610388171d84d3 --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/bootblock/bootblock.c M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memlayout.ld A src/soc/amd/picasso/page_map_s3.txt 6 files changed, 64 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/42888/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 9860546..f9c653a 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -174,6 +174,12 @@ Sets the size of DRAM allocation for verstage in linker script if running as a separate stage on x86.
+config PAGE_TABLE_ADDR + hex + default 0x2B0000 + help + Sets the address in DRAM where page tables should be loaded. + config RAMBASE hex default 0x10000000 diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 39269e9..9d3bbd8 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -445,4 +445,15 @@
cpu_microcode_bins += $(wildcard 3rdparty/amd_blobs/picasso/PSP/UcodePatch_*.bin)
+$(obj)/pt_s3.c $(obj)/pdpt_s3.c: src/soc/amd/picasso/page_map_s3.txt $(DOTCONFIG) util/x86/x86_page_tables.go + go run util/x86/x86_page_tables.go --iomap_file=$< --metadata_base_address=$(CONFIG_PAGE_TABLE_ADDR) --pdpt_output_c_file=$(obj)/pdpt_s3.c --pt_output_c_file=$(obj)/pt_s3.c + +cbfs-files-y += pt_s3 +pt_s3-file := $(obj)/pt_s3.c:struct +pt_s3-type := raw + +cbfs-files-y += pdpt_s3 +pdpt_s3-file := $(obj)/pdpt_s3.c:struct +pdpt_s3-type := raw + endif # ($(CONFIG_SOC_AMD_PICASSO),y) diff --git a/src/soc/amd/picasso/bootblock/bootblock.c b/src/soc/amd/picasso/bootblock/bootblock.c index a3935cc..ec30c19 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -4,6 +4,7 @@ #include <symbols.h> #include <bootblock_common.h> #include <console/console.h> +#include <cpu/x86/pae.h> #include <cpu/x86/cache.h> #include <cpu/x86/msr.h> #include <cpu/amd/msr.h> @@ -104,6 +105,17 @@ wrmsr(S3_RESUME_EIP_MSR, s3_resume_entry); }
+static void enable_paging(void) +{ + paging_set_nxe(1); + paging_set_default_pat(); + if (acpi_is_wakeup_s3()) { + printk(BIOS_DEBUG, "Enabling S3 page tables\n"); + paging_enable_for_car("pdpt_s3", "pt_s3"); + } +} + + asmlinkage void bootblock_c_entry(uint64_t base_timestamp) { set_caching(); @@ -123,5 +135,7 @@ u32 val = cpuid_eax(1); printk(BIOS_DEBUG, "Family_Model: %08x\n", val);
+ enable_paging(); + fch_early_init(); } diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index 2e5fae5..2abe54e 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <bootstate.h> #include <console/console.h> +#include <cpu/x86/pae.h> #include <device/device.h> #include <device/pci.h> #include <drivers/i2c/designware/dw_i2c.h> @@ -173,9 +175,30 @@ southbridge_final(chip_info); }
+static void picasso_disable_paging(void *unused) +{ + printk(BIOS_INFO, "%s: Disable Paging\n", __func__); + paging_disable_pae(); +} + +static void picasso_os_entry(void *unused) +{ + picasso_disable_paging(NULL); +} + struct chip_operations soc_amd_picasso_ops = { CHIP_NAME("AMD Picasso SOC") .enable_dev = enable_dev, .init = soc_init, .final = soc_final }; + +BOOT_STATE_INIT_ENTRY( + BS_PAYLOAD_BOOT, + BS_ON_ENTRY, + picasso_disable_paging, NULL); + +BOOT_STATE_INIT_ENTRY( + BS_OS_RESUME, + BS_ON_ENTRY, + picasso_os_entry, NULL); diff --git a/src/soc/amd/picasso/memlayout.ld b/src/soc/amd/picasso/memlayout.ld index 6f43ba1..27c72a2 100644 --- a/src/soc/amd/picasso/memlayout.ld +++ b/src/soc/amd/picasso/memlayout.ld @@ -89,6 +89,9 @@ VERSTAGE(CONFIG_VERSTAGE_ADDR, CONFIG_VERSTAGE_SIZE) #endif
+ REGION(pagetables, CONFIG_PAGE_TABLE_ADDR, 4096 * 12, 8) + REGION(pdpt, ., 32, 32) + EARLY_RESERVED_DRAM_END(.)
RAMSTAGE(CONFIG_RAMBASE, 8M) diff --git a/src/soc/amd/picasso/page_map_s3.txt b/src/soc/amd/picasso/page_map_s3.txt new file mode 100644 index 0000000..a03796f --- /dev/null +++ b/src/soc/amd/picasso/page_map_s3.txt @@ -0,0 +1,7 @@ +0xd0000000, 0x100000000, UC, NX # All of MMIO +0xff000000, 0x100000000, WP, # memory-mapped SPI +0xcbd0b000, 0xd0000000, WB, # RAM for CBMEM and FSP +0x100000, 0x3BD000, WB, # RAM for lower memory usage +0xFF000, 0x100000, WB, # RAM for FSP AP Reset Vectors [0xFF000 - 0xFF1A4] +0xE0000, 0x100000, WB, # ACPI Reset Vector search area +0x30000, 0x40000, WB, # RAM for SIPI