Hello Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42889
to review the following change.
Change subject: RFC: soc/amd/picasso: Enable paging for boot ......................................................................
RFC: soc/amd/picasso: Enable paging for boot
This is WIP because I think we should dynamically map the 0-4KiB page when we need to write to that range. Otherwise dereferencing a null pointer is valid.
BUG=b:159081993 TEST=Boot trembyle to the OS.
Signed-off-by: Furquan Shaikh furquan@google.com Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: I5d027a71cd32f6240fa004d813513b84a1ecfd26 --- M src/arch/x86/ebda.c M src/arch/x86/tables.c M src/device/oprom/realmode/x86.c M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/bootblock/bootblock.c A src/soc/amd/picasso/page_map.txt 6 files changed, 65 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/42889/1
diff --git a/src/arch/x86/ebda.c b/src/arch/x86/ebda.c index ea2d9d3..9c54359 100644 --- a/src/arch/x86/ebda.c +++ b/src/arch/x86/ebda.c @@ -5,6 +5,7 @@ #include <arch/ebda.h> #include <commonlib/endian.h> #include <console/console.h> +#include <cpu/x86/pae.h>
static void *get_ebda_start(void) { @@ -29,7 +30,13 @@ ebda_kb = ebda_size >> 10; ebda = get_ebda_start();
- /* clear BIOS DATA AREA */ + /* + * We disable paging because the BDA is located between 0 - 4KiB. + * We don't want to map a page in this region because otherwise + * 0 will become a valid memory address + */ + paging_disable_pae(); + zero_n(X86_BDA_BASE, X86_BDA_SIZE);
/* Avoid unaligned write16() since it's undefined behavior */ @@ -39,6 +46,8 @@ /* Set up EBDA */ zero_n(ebda, ebda_size); write_le16(ebda, ebda_kb); + + paging_enable_pae(); }
void setup_default_ebda(void) diff --git a/src/arch/x86/tables.c b/src/arch/x86/tables.c index 492674c..3e2f19a 100644 --- a/src/arch/x86/tables.c +++ b/src/arch/x86/tables.c @@ -11,6 +11,7 @@ #include <string.h> #include <cbmem.h> #include <smbios.h> +#include <cpu/x86/pae.h>
static unsigned long write_pirq_table(unsigned long rom_table_end) { @@ -200,8 +201,17 @@ if (CONFIG(GENERATE_SMBIOS_TABLES)) rom_table_end = write_smbios_table(rom_table_end);
+ /* + * We disable paging because the forwarding table is located + * between 0 - 4KiB. We don't want to map a page in this region because + * otherwise 0 will become a valid memory address + */ + paging_disable_pae(); + sz = write_coreboot_forwarding_table(forwarding_table, coreboot_table);
+ paging_enable_pae(); + forwarding_table += sz; /* Align up to page boundary for historical consistency. */ forwarding_table = ALIGN_UP(forwarding_table, 4*KiB); diff --git a/src/device/oprom/realmode/x86.c b/src/device/oprom/realmode/x86.c index 5215934..27bdf27 100644 --- a/src/device/oprom/realmode/x86.c +++ b/src/device/oprom/realmode/x86.c @@ -12,6 +12,7 @@ #include <pc80/i8254.h> #include <string.h> #include <vbe.h> +#include <cpu/x86/pae.h>
/* we use x86emu's register file representation */ #include <x86emu/regs.h> @@ -303,6 +304,8 @@ { printk(BIOS_DEBUG, "VBE: Getting information about VESA mode %04x\n", mi->video_mode); + + paging_disable_pae(); char *buffer = PTR_TO_REAL_MODE(__realmode_buffer); u16 buffer_seg = (((unsigned long)buffer) >> 4) & 0xff00; u16 buffer_adr = ((unsigned long)buffer) & 0xffff; @@ -312,6 +315,9 @@ die("\nError: In %s function\n", __func__); memcpy(mi->mode_info_block, buffer, sizeof(mi->mode_info_block)); mode_info_valid = 1; + + paging_enable_pae(); + return 0; }
@@ -322,10 +328,12 @@ mi->video_mode |= (1 << 14); // request clearing of framebuffer mi->video_mode &= ~(1 << 15); + paging_disable_pae(); X86_EAX = realmode_interrupt(0x10, VESA_SET_MODE, mi->video_mode, 0x0000, 0x0000, 0x0000, 0x0000); if (vbe_check_for_failure(X86_AH)) die("\nError: In %s function\n", __func__); + paging_enable_pae(); return 0; }
@@ -356,10 +364,13 @@ void vbe_textmode_console(void) { delay(2); + + paging_disable_pae(); X86_EAX = realmode_interrupt(0x10, 0x0003, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000); if (vbe_check_for_failure(X86_AH)) die("\nError: In %s function\n", __func__); + paging_enable_pae(); }
int fill_lb_framebuffer(struct lb_framebuffer *framebuffer) @@ -409,6 +420,13 @@ /* Set up C interrupt handlers */ setup_interrupt_handlers();
+ /* + * We disable paging because the IDT and Option ROM are located between + * 0 - 4KiB. We don't want to map a page in this region because + * otherwise 0 will become a valid memory address. + */ + paging_disable_pae(); + /* Set up real-mode IDT */ setup_realmode_idt();
@@ -419,6 +437,9 @@ /* TODO ES:DI Pointer to System BIOS PnP Installation Check Structure */ /* Option ROM entry point is at OPROM start + 3 */ realmode_call(addr + 0x0003, num_dev, 0xffff, 0x0000, 0xffff, 0x0, 0x0); + + paging_enable_pae(); + printk(BIOS_DEBUG, "... Option ROM returned.\n");
#if CONFIG(FRAMEBUFFER_SET_VESA_MODE) diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 9d3bbd8..5b0be65 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -445,6 +445,17 @@
cpu_microcode_bins += $(wildcard 3rdparty/amd_blobs/picasso/PSP/UcodePatch_*.bin)
+$(obj)/pt.c $(obj)/pdpt.c: src/soc/amd/picasso/page_map.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.c --pt_output_c_file=$(obj)/pt.c + +cbfs-files-y += pt +pt-file := $(obj)/pt.c:struct +pt-type := raw + +cbfs-files-y += pdpt +pdpt-file := $(obj)/pdpt.c:struct +pdpt-type := raw + $(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
diff --git a/src/soc/amd/picasso/bootblock/bootblock.c b/src/soc/amd/picasso/bootblock/bootblock.c index ec30c19..b82f99f 100644 --- a/src/soc/amd/picasso/bootblock/bootblock.c +++ b/src/soc/amd/picasso/bootblock/bootblock.c @@ -112,6 +112,9 @@ if (acpi_is_wakeup_s3()) { printk(BIOS_DEBUG, "Enabling S3 page tables\n"); paging_enable_for_car("pdpt_s3", "pt_s3"); + } else { + printk(BIOS_DEBUG, "Enabling page tables\n"); + paging_enable_for_car("pdpt", "pt"); } }
diff --git a/src/soc/amd/picasso/page_map.txt b/src/soc/amd/picasso/page_map.txt new file mode 100644 index 0000000..0fd224b --- /dev/null +++ b/src/soc/amd/picasso/page_map.txt @@ -0,0 +1,10 @@ +0xd0000000, 0x100000000, UC, NX # All of MMIO +0xff000000, 0x100000000, WP, # memory-mapped SPI +0xcbd0b000, 0xd0000000, WB, # RAM for CBMEM and FSP +0x30000000, 0x3023f000, WB, # RAM for payload +0x100000, 0x3BD000, WB, # RAM for lower memory usage +0xc0000, 0x100000, WB, # RAM for option ROM + EBDA [0xf6000 - 0xf6400] + IDT [0x0xff065 + 0xff84d + 0xff841 + 0xfec59 + 0xfe739 + 0xff859 + 0xfe82e + 0xfefd2 + 0xffe6e] +0xa0000, 0xc0000, UC, NX # VGA +0x30000, 0x40000, WB, # RAM for SIPI +0x1000, 0x2000, WB, # RAM for IDT stub code [0x1000 - 0x1900] +# 0x0, 0x1000, WB, # RAM for BDA [0x400 - 0x600] + CB forwarding table [0x500 - 0x1000]