Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AMD AGESA, binaryPI: implement C bootblock ......................................................................
AMD AGESA, binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/00730F01/Makefile.inc M src/cpu/amd/pi/Kconfig M src/cpu/amd/pi/romstage.c M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S A src/drivers/amd/agesa/exit_car.S M src/drivers/amd/agesa/romstage.c M src/southbridge/amd/pi/hudson/Makefile.inc M src/southbridge/amd/pi/hudson/bootblock.c M src/southbridge/amd/pi/hudson/hudson.h 13 files changed, 157 insertions(+), 138 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/1
diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index d14eb40..65f2b44 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -30,6 +30,8 @@ select CBMEM_STAGE_CACHE if HAVE_ACPI_RESUME select SMM_ASEG select NO_FIXED_XIP_ROM_SIZE + select C_ENVIRONMENT_BOOTBLOCK +
if CPU_AMD_AGESA
@@ -49,6 +51,11 @@ hex default 0x10000
+config DCACHE_BSP_STACK_SIZE + hex + depends on C_ENVIRONMENT_BOOTBLOCK + default 0x4000 + config ENABLE_MRC_CACHE bool "Use cached memory configuration" default n diff --git a/src/cpu/amd/pi/00730F01/Makefile.inc b/src/cpu/amd/pi/00730F01/Makefile.inc index c4a92cf..aa0c05c 100644 --- a/src/cpu/amd/pi/00730F01/Makefile.inc +++ b/src/cpu/amd/pi/00730F01/Makefile.inc @@ -11,6 +11,8 @@ # GNU General Public License for more details. #
+bootblock-y += fixme.c + romstage-y += fixme.c
ramstage-y += fixme.c diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index b33302e..82dc9e8 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -29,6 +29,7 @@ select CAR_GLOBAL_MIGRATION if BINARYPI_LEGACY_WRAPPER select SMM_ASEG select NO_FIXED_XIP_ROM_SIZE + select C_ENVIRONMENT_BOOTBLOCK
if CPU_AMD_PI
@@ -51,6 +52,11 @@ hex default 0x10000
+config DCACHE_BSP_STACK_SIZE + hex + depends on C_ENVIRONMENT_BOOTBLOCK + default 0x4000 + config S3_DATA_POS hex default 0xFFFF0000 diff --git a/src/cpu/amd/pi/romstage.c b/src/cpu/amd/pi/romstage.c index cac5664..f060dc2 100644 --- a/src/cpu/amd/pi/romstage.c +++ b/src/cpu/amd/pi/romstage.c @@ -21,11 +21,6 @@ #include <northbridge/amd/agesa/agesa_helper.h> #include <northbridge/amd/agesa/state_machine.h>
-void asmlinkage early_all_cores(void) -{ - amd_initmmio(); -} - void *asmlinkage romstage_main(unsigned long bist) { int s3resume = 0; diff --git a/src/cpu/x86/lapic/Makefile.inc b/src/cpu/x86/lapic/Makefile.inc index 9454f8f..0d11478 100644 --- a/src/cpu/x86/lapic/Makefile.inc +++ b/src/cpu/x86/lapic/Makefile.inc @@ -1,6 +1,7 @@ ramstage-y += lapic.c ramstage-y += lapic_cpu_init.c ramstage-$(CONFIG_SMP) += secondary.S +bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c romstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c ramstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c postcar-$(CONFIG_UDELAY_LAPIC) += apic_timer.c diff --git a/src/drivers/amd/agesa/Makefile.inc b/src/drivers/amd/agesa/Makefile.inc index 4d5bd3e..be048c4 100644 --- a/src/drivers/amd/agesa/Makefile.inc +++ b/src/drivers/amd/agesa/Makefile.inc @@ -21,8 +21,8 @@
ramstage-y += state_machine.c
-cpu_incs-y += $(src)/drivers/amd/agesa/cache_as_ram.S -postcar-y += cache_as_ram.S +bootblock-y += cache_as_ram.S +postcar-y += exit_car.S
else
@@ -30,6 +30,8 @@
endif
+bootblock-y += bootblock.c + romstage-y += def_callouts.c romstage-y += eventlog.c
diff --git a/src/drivers/amd/agesa/bootblock.c b/src/drivers/amd/agesa/bootblock.c new file mode 100644 index 0000000..1a6bfdb --- /dev/null +++ b/src/drivers/amd/agesa/bootblock.c @@ -0,0 +1,24 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <bootblock_common.h> +#include <cpu/x86/lapic.h> +#include <northbridge/amd/agesa/agesa_helper.h> + +asmlinkage void bootblock_c_entry(uint64_t base_timestamp) +{ + amd_initmmio(); + enable_lapic(); + /* TSC cannot be relied upon. Override the TSC value passed in. */ + bootblock_main_with_basetime(timestamp_get()); +} \ No newline at end of file diff --git a/src/drivers/amd/agesa/cache_as_ram.S b/src/drivers/amd/agesa/cache_as_ram.S index dcb0c43..707615d 100644 --- a/src/drivers/amd/agesa/cache_as_ram.S +++ b/src/drivers/amd/agesa/cache_as_ram.S @@ -25,14 +25,14 @@ #include <cpu/x86/cache.h> #include <cpu/x86/post_code.h>
-.code32 -.globl _cache_as_ram_setup, _cache_as_ram_setup_end -.globl chipset_teardown_car +/* + * on entry: + * mm0: BIST (ignored) + * mm2_mm1: timestamp at bootblock_protected_mode_entry + */
-_cache_as_ram_setup: - - /* Preserve BIST. */ - movd %eax, %mm0 +.global bootblock_pre_c_entry +bootblock_pre_c_entry:
post_code(0xa0)
@@ -43,128 +43,35 @@ orl $(3 << 9), %eax movl %eax, %cr4
- post_code(0xa1)
AMD_ENABLE_STACK
- /* Align the stack. */ - and $0xFFFFFFF0, %esp - -#ifdef __x86_64__ - /* switch to 64 bit long mode */ - mov %esi, %ecx - add $0, %ecx # core number - xor %eax, %eax - lea (0x1000+0x23)(%ecx), %edi - mov %edi, (%ecx) - mov %eax, 4(%ecx) - - lea 0x1000(%ecx), %edi - movl $0x000000e3, 0x00(%edi) - movl %eax, 0x04(%edi) - movl $0x400000e3, 0x08(%edi) - movl %eax, 0x0c(%edi) - movl $0x800000e3, 0x10(%edi) - movl %eax, 0x14(%edi) - movl $0xc00000e3, 0x18(%edi) - movl %eax, 0x1c(%edi) - - # load ROM based identity mapped page tables - mov %ecx, %eax - mov %eax, %cr3 - - # enable PAE - mov %cr4, %eax - bts $5, %eax - mov %eax, %cr4 - - # enable long mode - mov $0xC0000080, %ecx + /* Setup bootblock stack on BSP */ + mov $0x1b, %ecx rdmsr - bts $8, %eax - wrmsr + test $256, %eax + jz stack_align
- # enable paging - mov %cr0, %eax - bts $31, %eax - mov %eax, %cr0 + mov $_ecar_stack, %esp
- # use call far to switch to 64-bit code segment - ljmp $0x18, $1f -1: +stack_align:
-#endif + /* Align the stack and keep aligned for call to bootblock_c_entry() */ + and $0xfffffff0, %esp + sub $8, %esp
- call early_all_cores + movd %mm2, %eax + pushl %eax /* tsc[63:32] */ + movd %mm1, %eax + pushl %eax /* tsc[31:0] */
- /* Must maintain 16-byte stack alignment here. */ - pushl $0x0 - pushl $0x0 - pushl $0x0 - movd %mm0, %eax /* bist */ - pushl %eax - call romstage_main +before_carstage: + post_code(0xa2)
-#if CONFIG(POSTCAR_STAGE) + call bootblock_c_entry + /* Never reached */
-/* We do not return. Execution continues with run_postcar_phase() - * calling to chipset_teardown_car below. - */ - jmp postcar_entry_failure - -chipset_teardown_car: - -/* - * Retrieve return address from stack as it will get trashed below if - * execution is utilizing the cache-as-ram stack. - */ - pop %esp - -#else - - movl %eax, %esp - -/* Register %esp is new stacktop for remaining of romstage. */ - -#endif - - /* Disable cache */ - movl %cr0, %eax - orl $CR0_CacheDisable, %eax - movl %eax, %cr0 - -/* Register %esp is preserved in AMD_DISABLE_STACK. */ - AMD_DISABLE_STACK - -#if CONFIG(POSTCAR_STAGE) - - jmp *%esp - -#else - - /* enable cache */ - movl %cr0, %eax - andl $0x9fffffff, %eax - movl %eax, %cr0 - - call romstage_after_car - -#endif - - /* Should never see this postcode */ - post_code(0xaf) - -stop: +.halt_forever: + post_code(POST_DEAD_CODE) hlt - jmp stop - -/* These are here for linking purposes. */ -.weak early_all_cores, romstage_main -early_all_cores: -romstage_main: -postcar_entry_failure: - /* Should never see this postcode */ - post_code(0xae) - jmp stop - -_cache_as_ram_setup_end: + jmp .halt_forever diff --git a/src/drivers/amd/agesa/exit_car.S b/src/drivers/amd/agesa/exit_car.S new file mode 100644 index 0000000..f9d056e --- /dev/null +++ b/src/drivers/amd/agesa/exit_car.S @@ -0,0 +1,37 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2011 Advanced Micro Devices, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <gcccar.inc> +#include <cpu/x86/cache.h> + +.code32 +.globl chipset_teardown_car + +chipset_teardown_car: + pop %esp + + /* Disable cache */ + movl %cr0, %eax + orl $CR0_CacheDisable, %eax + movl %eax, %cr0 + + AMD_DISABLE_STACK + + /* enable cache */ + movl %cr0, %eax + andl $(~(CR0_CD | CR0_NW)), %eax + movl %eax, %cr0 + + jmp *%esp diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c index 76a6ea4..bfdbda3 100644 --- a/src/drivers/amd/agesa/romstage.c +++ b/src/drivers/amd/agesa/romstage.c @@ -34,11 +34,6 @@ #error "LEGACY_WRAPPER code not supported" #endif
-void asmlinkage early_all_cores(void) -{ - amd_initmmio(); -} - void __weak platform_once(struct sysinfo *cb) { board_BeforeAgesa(cb); @@ -52,7 +47,7 @@ agesa_set_interface(cb); }
-void *asmlinkage romstage_main(unsigned long bist) +void asmlinkage car_stage_entry(void) { struct postcar_frame pcf; struct sysinfo romstage_state; @@ -75,9 +70,6 @@ printk(BIOS_DEBUG, "APIC %02d: CPU Family_Model = %08x\n", initial_apic_id, cpuid_eax(1));
- /* Halt if there was a built in self test failure */ - report_bist_failure(bist); - agesa_execute_state(cb, AMD_INIT_RESET);
agesa_execute_state(cb, AMD_INIT_EARLY); @@ -109,6 +101,5 @@ recover_postcar_frame(&pcf, cb->s3resume);
run_postcar_phase(&pcf); - /* We do not return. */ - return NULL; + /* We do not return. */ } diff --git a/src/southbridge/amd/pi/hudson/Makefile.inc b/src/southbridge/amd/pi/hudson/Makefile.inc index 4b4b138..6199b24 100644 --- a/src/southbridge/amd/pi/hudson/Makefile.inc +++ b/src/southbridge/amd/pi/hudson/Makefile.inc @@ -28,6 +28,8 @@ # #*****************************************************************************
+bootblock-y += bootblock.c +bootblock-y += early_setup.c bootblock-$(CONFIG_USBDEBUG) += enable_usbdebug.c
romstage-y += early_setup.c diff --git a/src/southbridge/amd/pi/hudson/bootblock.c b/src/southbridge/amd/pi/hudson/bootblock.c index f12cec8..3899783 100644 --- a/src/southbridge/amd/pi/hudson/bootblock.c +++ b/src/southbridge/amd/pi/hudson/bootblock.c @@ -13,8 +13,12 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <stdint.h> +#include <arch/io.h> #include <device/pci_ops.h> +#include <device/pci_ids.h> +#include <southbridge/amd/pi/hudson/hudson.h>
/* * Enable 4MB (LPC) ROM access at 0xFFC00000 - 0xFFFFFFFF. @@ -60,3 +64,40 @@ { hudson_enable_rom(); } + +void bootblock_soc_early_init(void) +{ + pci_devfn_t dev; + u32 data; + + bootblock_southbridge_init(); + hudson_lpc_decode(); + + dev = PCI_DEV(0, 0x14, 3); + data = pci_read_config32(dev, LPC_IO_OR_MEM_DECODE_ENABLE); + /* enable 0x2e/0x4e IO decoding for SuperIO */ + pci_write_config32(dev, LPC_IO_OR_MEM_DECODE_ENABLE, data | 3); + + /* + * Enable FCH to decode TPM associated Memory and IO regions for vboot + * + * Enable decoding of TPM cycles defined in TPM 1.2 spec + * Enable decoding of legacy TPM addresses: IO addresses 0x7f- + * 0x7e and 0xef-0xee. + */ + + data = pci_read_config32(dev, LPC_TRUSTED_PLATFORM_MODULE); + data |= TPM_12_EN | TPM_LEGACY_EN; + pci_write_config32(dev, LPC_TRUSTED_PLATFORM_MODULE, data); + + /* + * In Hudson RRG, PMIOxD2[5:4] is "Drive strength control for + * LpcClk[1:0]". This following register setting has been + * replicated in every reference design since Parmer, so it is + * believed to be required even though it is not documented in + * the SoC BKDGs. Without this setting, there is no serial + * output. + */ + outb(0xD2, 0xcd6); + outb(0x00, 0xcd7); +} diff --git a/src/southbridge/amd/pi/hudson/hudson.h b/src/southbridge/amd/pi/hudson/hudson.h index 9511a6a..08adb84 100644 --- a/src/southbridge/amd/pi/hudson/hudson.h +++ b/src/southbridge/amd/pi/hudson/hudson.h @@ -117,6 +117,10 @@ #define LPC_ALT_WIDEIO1_ENABLE BIT(2) #define LPC_ALT_WIDEIO0_ENABLE BIT(0)
+#define LPC_TRUSTED_PLATFORM_MODULE 0x7c +#define TPM_12_EN BIT(0) +#define TPM_LEGACY_EN BIT(2) + #define LPC_WIDEIO2_GENERIC_PORT 0x90
#define SPI_CNTRL0 0x00
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 7:
This change is ready for review.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 8:
This patchset works with apu2 C bootblock change. However still can not move to SOC_AMD_COMMON_BLOCK_CAR, thus added implementation to drivers/amd/agesa/cahce_as_ram.S .
Problems with SOC_AMD_COMMON_BLOCK_CAR: - either frozen at postcode A2 - or reset loop in AmdInitReset
Possible reasons: - FPU and SSE initialization missing in SOC_AMD_COMMON_BLOCK_CAR path - BSP stack setup to _ecar_stack missing in SOC_AMD_COMMON_BLOCK_CAR
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 8:
(1 comment)
Patch Set 8:
This patchset works with apu2 C bootblock change. However still can not move to SOC_AMD_COMMON_BLOCK_CAR, thus added implementation to drivers/amd/agesa/cahce_as_ram.S .
Problems with SOC_AMD_COMMON_BLOCK_CAR:
- either frozen at postcode A2
- or reset loop in AmdInitReset
Possible reasons:
- FPU and SSE initialization missing in SOC_AMD_COMMON_BLOCK_CAR path
- BSP stack setup to _ecar_stack missing in SOC_AMD_COMMON_BLOCK_CAR
you probably want to set %esp to _ecar_stack on SOC_AMD_COMMON_BLOCK_CAR too.
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 142: OSFXSR [ Is done in arch/x86/bootblock_crt0.S if CONFIG_SSE is set. You might want to extend it with OSXMMEXCPT if SSE2 is set?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 8:
(1 comment)
Patch Set 8:
(1 comment)
Patch Set 8:
This patchset works with apu2 C bootblock change. However still can not move to SOC_AMD_COMMON_BLOCK_CAR, thus added implementation to drivers/amd/agesa/cahce_as_ram.S .
Problems with SOC_AMD_COMMON_BLOCK_CAR:
- either frozen at postcode A2
- or reset loop in AmdInitReset
Possible reasons:
- FPU and SSE initialization missing in SOC_AMD_COMMON_BLOCK_CAR path
- BSP stack setup to _ecar_stack missing in SOC_AMD_COMMON_BLOCK_CAR
you probably want to set %esp to _ecar_stack on SOC_AMD_COMMON_BLOCK_CAR too.
I thought about it. I will send a patch outside the relation chain and ask for testing (I do not have stoneyridge TBH).
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 142: OSFXSR [
Is done in arch/x86/bootblock_crt0.S if CONFIG_SSE is set. […]
Right. Would it be feasible to add it to bootblock_crt0.S? And most important, why bootblock_crt0 doesn't clear the FPU emulation bit? In fact I have copied this comment from cpu/x86/fpu_enable.inc and I wonder how true this statement is with #UD
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#9).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c M src/southbridge/amd/pi/hudson/include/soc/iomap.h 11 files changed, 215 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... PS9, Line 59: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 114: #else Maybe just use a different file and let the Makefile do its job?
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 142: OSFXSR [
Right. Would it be feasible to add it to bootblock_crt0.S? And most important, why bootblock_crt0 doesn't clear the FPU emulation bit? In fact I have copied this comment from cpu/x86/fpu_enable.inc and I wonder how true this statement is with #UD
You can just set those bits in c code before calling AGESA if you want btw.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 142: OSFXSR [
Right. Would it be feasible to add it to bootblock_crt0. […]
I remember I had problems when I placed it later, will test more and maybe remove it if it ends up redundant
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
Patch Set 8:
This patchset works with apu2 C bootblock change. However still can not move to SOC_AMD_COMMON_BLOCK_CAR, thus added implementation to drivers/amd/agesa/cahce_as_ram.S .
Cache-As-Ram setups in assembly have always been CPU family specific, to have SOC_AMD_COMMON_BLOCK_CAR adds no value.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... PS9, Line 58: !boot_cpu() You already branch in assembly to avoid changing %esp. Maybe just have a different C entry point for AP's (without timestamps)?
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 150: x1b LAPIC_BASE_MSR
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 152: 256 LAPIC_BASE_MSR_BOOTSTRAP_PROCESSOR
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 159: %esp Where are the AP's stack? You probably want some linker symbol region to assert that that it does not collide with other CAR symbols.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 159: %esp
Where are the AP's stack? You probably want some linker symbol region to assert that that it does no […]
It can be found in src/vendorcode/amd/pi/<family>/binaryPI/gcccar.inc for each family: For example 00730F01: BSP_STACK_BASE_ADDR = 0x30000 /* Base address for primary cores stack */ BSP_STACK_SIZE = 0x10000 /* 64KB for BSP core */ CORE0_STACK_BASE_ADDR = 0x80000 /* Base address for primary cores stack */ CORE0_STACK_SIZE = 0x4000 /* 16KB for primary cores */ CORE1_STACK_BASE_ADDR = 0x40000 /* Base address for AP cores */ CORE1_STACK_SIZE = 0x1000 /* 4KB for each AP cores */
Procedure with stack alignment is exactly the same as for ROMCC_BOOTBLOCK: https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache...
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 159: %esp
It can be found in src/vendorcode/amd/pi/<family>/binaryPI/gcccar.inc for each family: For example 00730F01: BSP_STACK_BASE_ADDR = 0x30000 /* Base address for primary cores stack */ BSP_STACK_SIZE = 0x10000 /* 64KB for BSP core */ CORE0_STACK_BASE_ADDR = 0x80000 /* Base address for primary cores stack */ CORE0_STACK_SIZE = 0x4000 /* 16KB for primary cores */ CORE1_STACK_BASE_ADDR = 0x40000 /* Base address for AP cores */ CORE1_STACK_SIZE = 0x1000 /* 4KB for each AP cores */
Procedure with stack alignment is exactly the same as for ROMCC_BOOTBLOCK: https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache...
I'm a bit lost on the terminology, but I presume it's something like: BSP, (well BSP in LAPIC reg) CORE0: primary cores on different CPU nodes (different packages?) CORE1: all other AP's?
If that's the case only the AP stacks are not getting in the way of coreboot. BTW it's pretty clear in gcccar.inc why CAR is not coherent over AP's since MTRR's set up differ per core.
It might be worth mentioning here that %esp has one of the values you describe here?
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#10).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 10 files changed, 221 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36914/10/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/10/src/drivers/amd/agesa/boot... PS10, Line 59: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
https://review.coreboot.org/c/coreboot/+/36914/10/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/10/src/drivers/amd/agesa/cach... PS10, Line 150: /* trailing whitespace
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#11).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 10 files changed, 221 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/11
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/11/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/11/src/drivers/amd/agesa/boot... PS11, Line 59: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#12).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
TEST=boot PC Engines apu2 with C bootblock patch and launch Debian with Linux kernel 4.14.50
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 10 files changed, 221 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/12
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#13).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c A src/drivers/amd/agesa/c_bootblock_car.S M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 11 files changed, 222 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/13
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/13/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/13/src/drivers/amd/agesa/boot... PS13, Line 59: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 13:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 142: OSFXSR [
I remember I had problems when I placed it later, will test more and maybe remove it if it ends up r […]
I just noticed neither SSE nor SSE2 is selected for any AMD AGESA or binaryPI... Although it is supported. Probably these lines will not be required if I select it in Kconfig
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 150: x1b
LAPIC_BASE_MSR
Done
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 152: 256
LAPIC_BASE_MSR_BOOTSTRAP_PROCESSOR
Done
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 159: %esp
It can be found in src/vendorcode/amd/pi/<family>/binaryPI/gcccar.inc for each family: […]
Added an appropriate comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 13:
(2 comments)
Kyösti I should credit you for the BSP stack issue on C bootblock (CB:34883). Feel free to add your signoff to the commit, or I can do it if you want.
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 114: #else
Maybe just use a different file and let the Makefile do its job?
Done
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 142: OSFXSR [
I just noticed neither SSE nor SSE2 is selected for any AMD AGESA or binaryPI... […]
After selecting SSE2 in Kconfig, setting the FPU EM bit, OSFXSR and OSXMMEXCPT became obsolete, thus removed.
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#14).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c A src/drivers/amd/agesa/c_bootblock_car.S M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 11 files changed, 221 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/14
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 59: void (*ap_romstage_entry)(void) = function definition argument 'void' should also have an identifier name
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 14:
(10 comments)
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 25: #define SOC_EARLY_VMTRR_TEMPRAM 3 unused?
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 38: mtrr = (mtrr_cap.lo & MTRR_CAP_VCNT) - SOC_EARLY_VMTRR_FLASH; get_free_var_mtrr()? or is something else touching these MTRR's later on such that you want to use a specific one?
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 39: FLASH_BASE_ADDR OPTIMAL_CACHE_ROM_BASE
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 39: CONFIG_ROM_SIZE OPTIMAL_CACHE_ROM_SIZE
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 42: CONFIG_PI_AGESA_CAR_HEAP_BASE, : CONFIG_PI_AGESA_HEAP_SIZE You might want to do a buildtime assertion that CONFIG_PI_AGESA_HEAP_SIZE is a power of 2 and that CONFIG_PI_AGESA_CAR_HEAP_BASE is aligned to CONFIG_PI_AGESA_HEAP_SIZE. Not sure if that is already done somewhere.
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 46: msr_t msr = rdmsr(0x1B); : msr.lo |= 1 << 11; : wrmsr(0x1B, msr); just enable_lapic() here?
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 52: bootblock_c_entry I'd adda ap_bootblock_c_entry and call it from assembly to avoid branching twice on the LAPIC_BASE_MSR reads.
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/cach... PS14, Line 40: remove change?
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/roms... File src/drivers/amd/agesa/romstage.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/roms... PS14, Line 57: #if !CONFIG(ROMCC_BOOTBLOCK) : set_ap_entry_ptr(agesa_call); : #endif You can probably use C code instead of CPP. It should get garbage collected on the ROMCC_BOOTBLOCK path.
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/roms... PS14, Line 62: : #if CONFIG(ROMCC_BOOTBLOCK) : void *asmlinkage romstage_main(unsigned long bist) : #else : asmlinkage void car_stage_entry(void) : #endif you can avoid the ifdefs if car_stage_entry just calls romstage_main.
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#15).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c A src/drivers/amd/agesa/c_bootblock_car.S M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 11 files changed, 224 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/15
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/15/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/15/src/drivers/amd/agesa/boot... PS15, Line 58: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 15:
(7 comments)
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 25: #define SOC_EARLY_VMTRR_TEMPRAM 3
unused?
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 38: mtrr = (mtrr_cap.lo & MTRR_CAP_VCNT) - SOC_EARLY_VMTRR_FLASH;
get_free_var_mtrr()? or is something else touching these MTRR's later on such that you want to use a […]
These particular MTRRs are used by AGESA, so they should stay as they are.
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 39: CONFIG_ROM_SIZE
OPTIMAL_CACHE_ROM_SIZE
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 39: FLASH_BASE_ADDR
OPTIMAL_CACHE_ROM_BASE
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 42: CONFIG_PI_AGESA_CAR_HEAP_BASE, : CONFIG_PI_AGESA_HEAP_SIZE
You might want to do a buildtime assertion that CONFIG_PI_AGESA_HEAP_SIZE is a power of 2 and that C […]
Stoneyridge does not guard it either. These values are fixed and should not be modified/configured since they are related to AGESA
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 46: msr_t msr = rdmsr(0x1B); : msr.lo |= 1 << 11; : wrmsr(0x1B, msr);
just enable_lapic() here?
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 52: bootblock_c_entry
I'd adda ap_bootblock_c_entry and call it from assembly to avoid branching twice on the LAPIC_BASE_M […]
Done
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#16).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c A src/drivers/amd/agesa/c_bootblock_car.S M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/nb_util.c 11 files changed, 223 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/16
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/16/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/16/src/drivers/amd/agesa/boot... PS16, Line 58: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#17).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c A src/drivers/amd/agesa/c_bootblock_car.S M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Kconfig M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/bootblock.c A src/northbridge/amd/pi/nb_util.c 12 files changed, 242 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/17
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/boot... PS17, Line 32: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 17:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 38: mtrr = (mtrr_cap.lo & MTRR_CAP_VCNT) - SOC_EARLY_VMTRR_FLASH;
These particular MTRRs are used by AGESA, so they should stay as they are.
Looks another reason is that MTRR's are shared accross cores. You might want to copy that comment from stoneyridge code.
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 42: CONFIG_PI_AGESA_CAR_HEAP_BASE, : CONFIG_PI_AGESA_HEAP_SIZE
Stoneyridge does not guard it either. These values are fixed and should not be modified/configured since they are related to AGESA
Sure, but it's not because values are fixed by something else that you don't want a (compiletime) check to see if what you are doing makes sense.
https://review.coreboot.org/c/coreboot/+/36914/16/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/16/src/drivers/amd/agesa/boot... PS16, Line 58: void maybe gcc optimises things if you add the __noreturn flag?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 17:
(2 comments)
I think we can make this more review-friendly, let me try something.
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/boot... PS17, Line 25: /* TSC cannot be relied upon. Override the TSC value passed in. */ Not sure if this comment is true. At least for fam14 TSC rescales during raminit. But timestamp_get() here is just another rdtsc call now. So you might as well pass base_timestamp below.
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/cach... PS17, Line 39: /* Turn on OSFXSR [BIT9] and OSXMMEXCPT [BIT10] onto CR4 register */ Needless whitespace change,
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... PS18, Line 32: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... PS18, Line 16: #include <cpu/x86/lapic.h> : #include <cpu/x86/mtrr.h> : #include <cpu/amd/msr.h> why ?
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... PS18, Line 26: timestamp_get include <timestamp.h> ?
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/cach... PS18, Line 40: related?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
I wasn't really able to communicate in the review how I wanted to split this commit into several pieces. If you think we can make CB:37353 work, I would submit it with the Change-Id of this one.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
Patch Set 18:
I wasn't really able to communicate in the review how I wanted to split this commit into several pieces. If you think we can make CB:37353 work, I would submit it with the Change-Id of this one.
No problem. Should I abandon this one or will it break the chain?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... PS18, Line 16: #include <cpu/x86/lapic.h> : #include <cpu/x86/mtrr.h> : #include <cpu/amd/msr.h>
why ?
Leftovers from previous patchsets. To be removed
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
Patch Set 18:
Patch Set 18:
I wasn't really able to communicate in the review how I wanted to split this commit into several pieces. If you think we can make CB:37353 work, I would submit it with the Change-Id of this one.
No problem. Should I abandon this one or will it break the chain?
Don't abandon, I will reuse the change-id to keep all the reviews around. Iff CB:37353 works out.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
Patch Set 18:
Patch Set 18:
Patch Set 18:
I wasn't really able to communicate in the review how I wanted to split this commit into several pieces. If you think we can make CB:37353 work, I would submit it with the Change-Id of this one.
No problem. Should I abandon this one or will it break the chain?
Don't abandon, I will reuse the change-id to keep all the reviews around. Iff CB:37353 works out.
Pushed new patchset to CB:37353 and applied CB:36915 on top of that. Small fix and works now.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
Patch Set 18:
Patch Set 18:
Patch Set 18:
Patch Set 18:
I wasn't really able to communicate in the review how I wanted to split this commit into several pieces. If you think we can make CB:37353 work, I would submit it with the Change-Id of this one.
No problem. Should I abandon this one or will it break the chain?
Don't abandon, I will reuse the change-id to keep all the reviews around. Iff CB:37353 works out.
Pushed new patchset to CB:37353 and applied CB:36915 on top of that. Small fix and works now.
Excellent. Can you rebase locally to current master and check we don't get any boot regression from the commits that have landed meanwhile. I would like to push this rebased on current master right after CAR_GLOBAL removals land, and merge a few more from the series.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
Patch Set 18:
Patch Set 18:
Patch Set 18:
Patch Set 18:
Patch Set 18:
I wasn't really able to communicate in the review how I wanted to split this commit into several pieces. If you think we can make CB:37353 work, I would submit it with the Change-Id of this one.
No problem. Should I abandon this one or will it break the chain?
Don't abandon, I will reuse the change-id to keep all the reviews around. Iff CB:37353 works out.
Pushed new patchset to CB:37353 and applied CB:36915 on top of that. Small fix and works now.
Excellent. Can you rebase locally to current master and check we don't get any boot regression from the commits that have landed meanwhile. I would like to push this rebased on current master right after CAR_GLOBAL removals land, and merge a few more from the series.
Rebased locally on top of 6229cc93ff16a5a9a424a0323fd631c8b3e1c943 and works well.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... File src/northbridge/amd/pi/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... PS18, Line 32: Copy the stoneyridge comment here? I think it applies too.
/* * todo: AGESA currently writes variable MTRRs. Once that is * corrected, un-hardcode this MTRR. * * Be careful not to use get_free_var_mtrr/set_var_mtrr pairs * where all cores execute the path. Both cores within a compute * unit share MTRRs. Programming core0 has the appearance of * modifying core1 too. Using the pair again will create * duplicate copies. */
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... File src/northbridge/amd/pi/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... PS18, Line 32:
Copy the stoneyridge comment here? I think it applies too. […]
Sharing the MTRRs is SOC dependent.
It looked like this commit was creating one implementation of amd_initmmio() in bootblock, and keeping another one for other stages. That gets super confusing so we will definetly revisit this after a rebase.
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#19).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/bootblock.c A src/northbridge/amd/pi/nb_util.c 10 files changed, 163 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/19
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/19/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/19/src/drivers/amd/agesa/boot... PS19, Line 30: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 19:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... PS18, Line 16: #include <cpu/x86/lapic.h> : #include <cpu/x86/mtrr.h> : #include <cpu/amd/msr.h>
Leftovers from previous patchsets. […]
Done
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/boot... PS18, Line 26: timestamp_get
include <timestamp. […]
Done
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... File src/northbridge/amd/pi/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... PS18, Line 32:
Sharing the MTRRs is SOC dependent. […]
Since all the boards are disabled, I may remove the amd_initmmio present in fixme.c files elsewhere. The bootblock implementation required AGESA heap to be set as writeback for some reason.
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#20).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
TEST=boot PC Engines apu2 with C bootblock patch and launch Debian with Linux kernel 4.14.50
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/northbridge/amd/pi/Makefile.inc A src/northbridge/amd/pi/bootblock.c A src/northbridge/amd/pi/nb_util.c 10 files changed, 163 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/20
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 20:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/amd/pi/Kconfig File src/cpu/amd/pi/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/amd/pi/Kconfig@55 PS20, Line 55: config PI_AGESA_CAR_HEAP_BASE These will need some argumentation in the commit message.
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... File src/cpu/x86/lapic/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... PS20, Line 4: bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c We'll go with this for now, but some PI blobs manipulate/reset LAPIC so some SOCs may need to switch to TSC. I believe stoney already did.
https://review.coreboot.org/c/coreboot/+/36914/20/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/drivers/amd/agesa/boot... PS20, Line 17: #include <northbridge/amd/agesa/agesa_helper.h> amd_initmmio() is not really about agesa and would be better elsewhere.
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/agesa/... File src/northbridge/amd/agesa/agesa_helper.h:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/agesa/... PS20, Line 66: These will come from some amdblocks/ headerfile instead. Not really about AGESA.
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... File src/northbridge/amd/pi/nb_util.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... PS20, Line 19: void *get_ap_entry_ptr(void) Make the code in stoney/nb_util.c available under SOC_AMD_COMMON instead of forking a copy.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... File src/cpu/x86/lapic/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... PS20, Line 4: bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c
We'll go with this for now, but some PI blobs manipulate/reset LAPIC so some SOCs may need to switch […]
Wasn't TSC recalibrated around memory init somewhere? I thought it is not good to use TSC for these boards.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... File src/cpu/x86/lapic/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... PS20, Line 4: bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c
Wasn't TSC recalibrated around memory init somewhere? I thought it is not good to use TSC for these […]
The value rdstc() returns is scaled by 4. We may not have other options, TSC is typically higher resolution than LAPIC. I have topic branch on monotonic-timer we will evaluate the options there.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/21/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/21/src/drivers/amd/agesa/boot... PS21, Line 30: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 21:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... File src/cpu/x86/lapic/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/x86/lapic/Makefile... PS20, Line 4: bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c
The value rdstc() returns is scaled by 4. […]
Ack
https://review.coreboot.org/c/coreboot/+/36914/20/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/drivers/amd/agesa/boot... PS20, Line 17: #include <northbridge/amd/agesa/agesa_helper.h>
amd_initmmio() is not really about agesa and would be better elsewhere.
This could be a static function here. Noticed MMIO configuration MSR is the same across all families as well as rom caching MTRR. The MTRR set as writeback for AGESA heap also seems to be the same with an exception that the size may be different (can be solved with Kconfig option)
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/agesa/... File src/northbridge/amd/agesa/agesa_helper.h:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/agesa/... PS20, Line 66:
These will come from some amdblocks/ headerfile instead. Not really about AGESA.
Not anymore after your patches?
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... File src/northbridge/amd/pi/nb_util.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... PS20, Line 19: void *get_ap_entry_ptr(void)
Make the code in stoney/nb_util.c available under SOC_AMD_COMMON instead of forking a copy.
Not needed anymore I guess
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... File src/northbridge/amd/pi/nb_util.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... PS20, Line 19: void *get_ap_entry_ptr(void)
Not needed anymore I guess
Right, put these in biosram.[ch]
Hello Kyösti Mälkki, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#22).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 6 files changed, 117 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/22
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/22/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/22/src/drivers/amd/agesa/boot... PS22, Line 61: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/23/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/23/src/drivers/amd/agesa/boot... PS23, Line 61: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/24/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/24/src/drivers/amd/agesa/boot... PS24, Line 61: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 24:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36914/20/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/drivers/amd/agesa/boot... PS20, Line 17: #include <northbridge/amd/agesa/agesa_helper.h>
This could be a static function here. […]
Done
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/agesa/... File src/northbridge/amd/agesa/agesa_helper.h:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/agesa/... PS20, Line 66:
Not anymore after your patches?
Done
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... File src/northbridge/amd/pi/nb_util.c:
https://review.coreboot.org/c/coreboot/+/36914/20/src/northbridge/amd/pi/nb_... PS20, Line 19: void *get_ap_entry_ptr(void)
Right, put these in biosram. […]
Done in CB:37416
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 24:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36914/3/src/cpu/amd/agesa/Kconfig File src/cpu/amd/agesa/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/3/src/cpu/amd/agesa/Kconfig@5... PS3, Line 56: depends on C_ENVIRONMENT_BOOTBLOCK
Haven't checked. Did it as sanity check.
No occurrences except car.ld
https://review.coreboot.org/c/coreboot/+/36914/3/src/drivers/amd/agesa/exit_... File src/drivers/amd/agesa/exit_car.S:
https://review.coreboot.org/c/coreboot/+/36914/3/src/drivers/amd/agesa/exit_... PS3, Line 15: : #include <gcccar.inc> : #include <cpu/x86/cache.h> : : .code32 : .globl chipset_teardown_car : : chipset_teardown_car: : pop %esp : : /* Disable cache */ : movl %cr0, %eax : orl $CR0_CacheDisable, %eax : movl %eax, %cr0 : : AMD_DISABLE_STACK : : /* enable cache */ : movl %cr0, %eax : andl $(~(CR0_CD | CR0_NW)), %eax : movl %eax, %cr0 : : jmp *%esp
No problem
Done
https://review.coreboot.org/c/coreboot/+/36914/3/src/northbridge/amd/agesa/f... File src/northbridge/amd/agesa/family14/nb_util.c:
https://review.coreboot.org/c/coreboot/+/36914/3/src/northbridge/amd/agesa/f... PS3, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2018 Advanced Micro Devices : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ : : #include <device/pci_ops.h> : #include <northbridge/amd/agesa/agesa_helper.h> : : #define GNB_SCRATCH_REG 0x78 : : #define GNB_DEV PCI_DEV(0, 0, 0) : : void *get_ap_entry_ptr(void) : { : return (void *)pci_read_config32(GNB_DEV, GNB_SCRATCH_REG); : } : : void set_ap_entry_ptr(void *entry) : { : pci_write_config32(GNB_DEV, GNB_SCRATCH_REG, (uintptr_t)entry); : }
Okay, thank you for the info. Will try this approach. […]
Utilized BIOSRAM space in FCH
Kyösti Mälkki has uploaded a new patch set (#25) to the change originally created by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 125 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/25
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 25:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/25/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/25/src/drivers/amd/agesa/boot... PS25, Line 61: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 25: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/25/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/25/src/drivers/amd/agesa/boot... PS25, Line 61: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr();
function definition argument 'void' should also have an identifier name
not sure if we should care about this
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 25:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... PS9, Line 58: !boot_cpu()
You already branch in assembly to avoid changing %esp. […]
Done
Hello Kyösti Mälkki, Angel Pons, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#26).
Change subject: binaryPI: implement C bootblock ......................................................................
binaryPI: implement C bootblock
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 121 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/26
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 26:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/26/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/26/src/drivers/amd/agesa/boot... PS26, Line 57: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 26:
(10 comments)
https://review.coreboot.org/c/coreboot/+/36914/3/src/cpu/amd/agesa/Kconfig File src/cpu/amd/agesa/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/3/src/cpu/amd/agesa/Kconfig@3... PS3, Line 33: select C_ENVIRONMENT_BOOTBLOCK
Understood
All the binary PI boards have ROMCC_BOOTBLOCK selected or are disabled from build. So if the platform reaches C bootblock, it should have already the superio initialized properly.
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/amd/pi/Kconfig File src/cpu/amd/pi/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/20/src/cpu/amd/pi/Kconfig@55 PS20, Line 55: config PI_AGESA_CAR_HEAP_BASE
These will need some argumentation in the commit message.
After enabling SSE2 setting writeback MTRR for AGESA heap seems not to be necessary for C bootblock. Checked on both apu1 and apu2 C bootblock
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/boot... PS14, Line 42: CONFIG_PI_AGESA_CAR_HEAP_BASE, : CONFIG_PI_AGESA_HEAP_SIZE
Stoneyridge does not guard it either. […]
The config options occurred to be not unnecessary
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/boot... PS17, Line 25: /* TSC cannot be relied upon. Override the TSC value passed in. */
Not sure if this comment is true. At least for fam14 TSC rescales during raminit. […]
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/cach... PS14, Line 40:
remove change?
Done
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/17/src/drivers/amd/agesa/cach... PS17, Line 39: /* Turn on OSFXSR [BIT9] and OSXMMEXCPT [BIT10] onto CR4 register */
Needless whitespace change,
Done
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/cach... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/18/src/drivers/amd/agesa/cach... PS18, Line 40:
related?
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/roms... File src/drivers/amd/agesa/romstage.c:
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/roms... PS14, Line 57: #if !CONFIG(ROMCC_BOOTBLOCK) : set_ap_entry_ptr(agesa_call); : #endif
You can probably use C code instead of CPP. […]
Done
https://review.coreboot.org/c/coreboot/+/36914/14/src/drivers/amd/agesa/roms... PS14, Line 62: : #if CONFIG(ROMCC_BOOTBLOCK) : void *asmlinkage romstage_main(unsigned long bist) : #else : asmlinkage void car_stage_entry(void) : #endif
you can avoid the ifdefs if car_stage_entry just calls romstage_main.
Done
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... File src/northbridge/amd/pi/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/18/src/northbridge/amd/pi/boo... PS18, Line 32:
Since all the boards are disabled, I may remove the amd_initmmio present in fixme.c files elsewhere. […]
Only ROM caching is set via MTRRs. Removed the amd_initmmio from bootblock to not confuse things.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 26:
(3 comments)
Squash CB:37331 and CB:37450 with this one?
https://review.coreboot.org/c/coreboot/+/36914/26//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36914/26//COMMIT_MSG@7 PS26, Line 7: binaryPI: implement C bootblock AGESA, binaryPI: ....
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/agesa/Kconfig File src/cpu/amd/agesa/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/agesa/Kconfig@... PS26, Line 71: Unless we squash fam14-16 here, this is at wrong place.
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig File src/cpu/amd/pi/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig@61 PS26, Line 61: default 0x30000 The HEAP ones weren't necessary after all?
Hello Kyösti Mälkki, Angel Pons, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#27).
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
AGESA, binaryPI: implement C bootblock
Modify CAR setup to work in bootblock. Provide bootblock C file with necessary C bootblock functions. Additionally chache the ROM and set the MMCONF base before jumping to bootblock main.
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 125 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/27
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 27:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/27/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/27/src/drivers/amd/agesa/boot... PS27, Line 57: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 27:
(2 comments)
CB:37331 and CB:37450 can be abandoned?
https://review.coreboot.org/c/coreboot/+/36914/26//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36914/26//COMMIT_MSG@7 PS26, Line 7: binaryPI: implement C bootblock
AGESA, binaryPI: ....
Done
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/agesa/Kconfig File src/cpu/amd/agesa/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/agesa/Kconfig@... PS26, Line 71:
Unless we squash fam14-16 here, this is at wrong place.
Done
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 27:
(4 comments)
Patch Set 27:
(2 comments)
CB:37331 and CB:37450 can be abandoned?
Yes, please abandon CB:37450. Already abandoned CB:37331
https://review.coreboot.org/c/coreboot/+/36914/26//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36914/26//COMMIT_MSG@7 PS26, Line 7: binaryPI: implement C bootblock
AGESA, binaryPI: ....
Done
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/agesa/Kconfig File src/cpu/amd/agesa/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/agesa/Kconfig@... PS26, Line 71:
Unless we squash fam14-16 here, this is at wrong place.
Done
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig File src/cpu/amd/pi/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig@61 PS26, Line 61: default 0x30000
The HEAP ones weren't necessary after all?
It seems so. Forgot to remove here
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig@61 PS26, Line 61: default 0x30000
The HEAP ones weren't necessary after all?
Done
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 27:
I'll merge CB:37168 and CB:37329. CB:37449 needs review but could go in as well.
We'll delay CB:37429 like commented there.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 27:
Patch Set 27:
I'll merge CB:37168 and CB:37329. CB:37449 needs review but could go in as well.
We'll delay CB:37429 like commented there.
Sure, the SB changes aren't tied to this change and can be applied earlier. Already commented on CB:37449
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 27:
I am going to fix CB:37451, lets rebase things on that.
Kyösti Mälkki has uploaded a new patch set (#28) to the change originally created by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
AGESA, binaryPI: implement C bootblock
Modify CAR setup to work in bootblock. Provide bootblock C file with necessary C bootblock functions. Additionally chache the ROM and set the MMCONF base before jumping to bootblock main.
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 119 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/28
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 57: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 28:
(3 comments)
See comments in CB:37440, we may need to do couple more things in fam14 and binaryPI bootblocks.
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 23: static void init_mmio(void) The name is bad, enable_pci_mmconf() would be accurate. And this should be shared from amd/stoneyridge instead of a new copy.
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE, I think there was previous comment this fails when cores share MTRRs. That is, this creates unnecessary duplicate VAR MTRR usage. Also, I believe vendorcode side uses fixed indexes, so dynamic ones here seem like bad idea to me.
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 57: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr();
function definition argument 'void' should also have an identifier name
Do we need the cast here at all?
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 28:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 23: static void init_mmio(void)
The name is bad, enable_pci_mmconf() would be accurate. […]
Stoneyridge kept the amd_initmmio naming and does not carve out the MMIOCONF initialization from that function. Do you mean to create a separate patch to implement common enable_pci_mmconf() for AMD boards? Where exactly? AMD blocks?
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
I think there was previous comment this fails when cores share MTRRs. […]
Yes, we can hardcode the MTRR indexes. I saw they are the same for family14 and binaryPI 16h and probably are for the rest of families currently present in the tree.
Any idea how to avoid duplication?
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 57: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr();
Do we need the cast here at all?
The compiler doesn't complain and the boards boot, so I guess not.
Hello Kyösti Mälkki, Angel Pons, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#29).
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
AGESA, binaryPI: implement C bootblock
Modify CAR setup to work in bootblock. Provide bootblock C file with necessary C bootblock functions. Additionally chache the ROM and set the MMCONF base before jumping to bootblock main.
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 115 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/29
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 29:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/29/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/29/src/drivers/amd/agesa/boot... PS29, Line 53: void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 29:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 57: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr();
The compiler doesn't complain and the boards boot, so I guess not.
Done
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 29:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
Yes, we can hardcode the MTRR indexes. […]
I do not see how the previous implementation with amdlib and LibAmdMsrWrite could be better in this aspect than this. So AGESA also does duplicates?
Hello Kyösti Mälkki, Angel Pons, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36914
to look at the new patch set (#30).
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
AGESA, binaryPI: implement C bootblock
Modify CAR setup to work in bootblock. Provide bootblock C file with necessary C bootblock functions. Additionally chache the ROM and set the MMCONF base before jumping to bootblock main.
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 106 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36914/30
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/30/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/30/src/drivers/amd/agesa/boot... PS30, Line 44: void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 30:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 23: static void init_mmio(void)
Stoneyridge kept the amd_initmmio naming and does not carve out the MMIOCONF initialization from tha […]
Prepared a preceding patch that moves the stoneyridge implementation to the common AMD block.
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
I do not see how the previous implementation with amdlib and LibAmdMsrWrite could be better in this […]
Looped through all BKDGS for fam14h, fam15h and 16h until processors 30-3f. The MTRRs are shared across cores in the compute unit (i.e. 2 cores) for whole fam15h, so the statement about duplicates is true for the whole fam15h. fam14h and 16h processors 00-0f and 30-3f are not affected. Will try to solve this now.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
Looped through all BKDGS for fam14h, fam15h and 16h until processors 30-3f. […]
Two quick questions. I see in the CPU init procedures that siblings are handled separately with rdmsr_amd and wrmsr_amd instruction which seem to have weird value in EDX 0x9c5a203a. What is the purpose of these functions? Could it fix the issue with duplicates?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
Two quick questions. […]
The magic value should be in %edi, not %edx? It is to make CPU really respond to the rdmsr/wrmsr instruction. I guess at some point in time AMD wanted to keep those MSRs hidden. I did not check what CPU_ID_FEATURES_MSR and CPU_ID_EXT_FEATURES_MSR really do, but maybe they were not MTRR related. Other MSRs are manipulated with common wrmsr() without the magic.
As you can quess, most of this is copy-paste from early fam15 and we should not really trust existing code to be correct.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
The magic value should be in %edi, not %edx? It is to make CPU really respond to the rdmsr/wrmsr ins […]
Maybe %edx, I'm a noob when it comes to clobbers...
I have researched the AMD CPU architecture and cannot come with any idea how to differentiate between cores in computing units in given node. I also don't think it will be safe to rely on initial apic ids and just program MTRR on the even apic ids (assuming BSP is always 0, so taking odd ids is a no go). Ideas?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 31:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/31/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/31/src/drivers/amd/agesa/boot... PS31, Line 44: void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 31:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
Maybe %edx, I'm a noob when it comes to clobbers... […]
Somehow I was thinking this is calling get_free_var_mtrr() while it clearly is not doing that. I still think it is better to use the same MTRR as the original amd_initmmio() did. A separate change with argumentation is required to change that, but we really should have no need to change it.
Note that for amd/stoneyridge seems to have moved MTRR programming outside vendorcode so we cannot use it as a guide with our old bloba.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 31:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 40: set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE,
Somehow I was thinking this is calling get_free_var_mtrr() while it clearly is not doing that. […]
I have originated this code for amd_initmmio from fixme.c files. These settings are shared across all families in AGESA and binaryPI, i.e. ROM caching is configured with 7th MTRR base/mask pair for all of them.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 31:
IIRC you've also been doing work to begin relying on soc/amd/common. Did you give any thought to using common/block/car and remove the cache_as_ram.S file here? Also, it seems like we have these DCACHE_* symbols duplicated in many places. I wonder whether they could be consolidated into block/car/Kconfig.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 31:
Patch Set 31:
IIRC you've also been doing work to begin relying on soc/amd/common. Did you give any thought to using common/block/car and remove the cache_as_ram.S file here? Also, it seems like we have these DCACHE_* symbols duplicated in many places. I wonder whether they could be consolidated into block/car/Kconfig.
I decided against attempts for a common cache_as_ram.S after three months of silence on CB:34883 from AMD and Silverback in fixing their/your design. Nor do we have access to the documentation or the source of CAR teardown changes that were made for amd/stoneyridge and we are also stuck with the old binaryPI blobs.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 31:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 23: static void init_mmio(void)
Prepared a preceding patch that moves the stoneyridge implementation to the common AMD block.
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 32:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/32/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/32/src/drivers/amd/agesa/boot... PS32, Line 44: void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 33:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36914/33/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/33/src/drivers/amd/agesa/boot... PS33, Line 44: void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 33:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig File src/cpu/amd/pi/Kconfig:
https://review.coreboot.org/c/coreboot/+/36914/26/src/cpu/amd/pi/Kconfig@61 PS26, Line 61: default 0x30000
Done
Done
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/28/src/drivers/amd/agesa/boot... PS28, Line 23: static void init_mmio(void)
Done
Done
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 33: Code-Review+1
Are you all ready to go on this? It's looking pretty complete to me.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
Patch Set 33: Code-Review+2
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: AGESA, binaryPI: implement C bootblock ......................................................................
AGESA, binaryPI: implement C bootblock
Modify CAR setup to work in bootblock. Provide bootblock C file with necessary C bootblock functions. Additionally chache the ROM and set the MMCONF base before jumping to bootblock main.
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36914 Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/pi/Kconfig M src/cpu/x86/lapic/Makefile.inc M src/drivers/amd/agesa/Makefile.inc A src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/cache_as_ram.S M src/drivers/amd/agesa/romstage.c 7 files changed, 106 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Marshall Dawson: Looks good to me, but someone else must approve
diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index 5e1ff1d..2c8f9c5 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -49,6 +49,14 @@ hex default 0x10000
+config DCACHE_BSP_STACK_SIZE + hex + default 0x4000 + +config C_ENV_BOOTBLOCK_SIZE + hex + default 0x8000 + config ENABLE_MRC_CACHE bool "Use cached memory configuration" default n diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index 728c7b1..c534b4d 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -48,6 +48,14 @@ hex default 0x10000
+config DCACHE_BSP_STACK_SIZE + hex + default 0x4000 + +config C_ENV_BOOTBLOCK_SIZE + hex + default 0x8000 + endif # CPU_AMD_PI
source "src/cpu/amd/pi/00630F01/Kconfig" diff --git a/src/cpu/x86/lapic/Makefile.inc b/src/cpu/x86/lapic/Makefile.inc index 9454f8f..0d11478 100644 --- a/src/cpu/x86/lapic/Makefile.inc +++ b/src/cpu/x86/lapic/Makefile.inc @@ -1,6 +1,7 @@ ramstage-y += lapic.c ramstage-y += lapic_cpu_init.c ramstage-$(CONFIG_SMP) += secondary.S +bootblock-$(CONFIG_UDELAY_LAPIC) += apic_timer.c romstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c ramstage-$(CONFIG_UDELAY_LAPIC) += apic_timer.c postcar-$(CONFIG_UDELAY_LAPIC) += apic_timer.c diff --git a/src/drivers/amd/agesa/Makefile.inc b/src/drivers/amd/agesa/Makefile.inc index dfb385d..3c3c4fc 100644 --- a/src/drivers/amd/agesa/Makefile.inc +++ b/src/drivers/amd/agesa/Makefile.inc @@ -19,7 +19,13 @@
ramstage-y += state_machine.c
+ifneq ($(CONFIG_ROMCC_BOOTBLOCK),y) +bootblock-y += bootblock.c +bootblock-y += cache_as_ram.S +else cpu_incs-y += $(src)/drivers/amd/agesa/cache_as_ram.S +endif + postcar-y += exit_car.S
romstage-y += def_callouts.c diff --git a/src/drivers/amd/agesa/bootblock.c b/src/drivers/amd/agesa/bootblock.c new file mode 100644 index 0000000..3763b98 --- /dev/null +++ b/src/drivers/amd/agesa/bootblock.c @@ -0,0 +1,47 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <bootblock_common.h> +#include <halt.h> +#include <timestamp.h> +#include <amdblocks/amd_pci_mmconf.h> +#include <amdblocks/biosram.h> +#include <cpu/amd/msr.h> +#include <cpu/x86/mtrr.h> + +#define EARLY_VMTRR_FLASH 6 + +static void set_early_mtrrs(void) +{ + /* Cache the ROM to speed up booting */ + set_var_mtrr(EARLY_VMTRR_FLASH, OPTIMAL_CACHE_ROM_BASE, + OPTIMAL_CACHE_ROM_SIZE, MTRR_TYPE_WRPROT); +} + +asmlinkage void bootblock_c_entry(uint64_t base_timestamp) +{ + enable_pci_mmconf(); + set_early_mtrrs(); + + bootblock_main_with_basetime(base_timestamp); +} + +asmlinkage void ap_bootblock_c_entry(void) +{ + enable_pci_mmconf(); + set_early_mtrrs(); + + void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); + ap_romstage_entry(); /* execution does not return */ + halt(); +} diff --git a/src/drivers/amd/agesa/cache_as_ram.S b/src/drivers/amd/agesa/cache_as_ram.S index 4417e64..1034992 100644 --- a/src/drivers/amd/agesa/cache_as_ram.S +++ b/src/drivers/amd/agesa/cache_as_ram.S @@ -27,9 +27,17 @@
.code32 .globl _cache_as_ram_setup, _cache_as_ram_setup_end +.global bootblock_pre_c_entry
_cache_as_ram_setup:
+/* + * on entry: + * mm0: BIST (ignored) + * mm2_mm1: timestamp at bootblock_protected_mode_entry + */ +bootblock_pre_c_entry: + post_code(0xa0)
AMD_ENABLE_STACK @@ -51,8 +59,10 @@ and $0xfffffff0, %esp sub $8, %esp
- pushl $0 /* tsc[63:32] */ - pushl $0 /* tsc[31:0] */ + movd %mm2, %eax + pushl %eax /* tsc[63:32] */ + movd %mm1, %eax + pushl %eax /* tsc[31:0] */
post_code(0xa2)
diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c index 48a81c5..dbf8bd6 100644 --- a/src/drivers/amd/agesa/romstage.c +++ b/src/drivers/amd/agesa/romstage.c @@ -11,6 +11,7 @@ * GNU General Public License for more details. */
+#include <amdblocks/biosram.h> #include <arch/acpi.h> #include <arch/cpu.h> #include <arch/romstage.h> @@ -26,6 +27,8 @@ #include <northbridge/amd/agesa/agesa_helper.h> #include <northbridge/amd/agesa/state_machine.h>
+void __weak board_BeforeAgesa(struct sysinfo *cb) { } + void __weak platform_once(struct sysinfo *cb) { board_BeforeAgesa(cb); @@ -39,6 +42,11 @@ agesa_set_interface(cb); }
+/* APs will enter directly here from bootblock, bypassing verstage + * and potential fallback / normal bootflow detection. + */ +static void ap_romstage_main(void); + static void romstage_main(void) { struct postcar_frame pcf; @@ -48,13 +56,15 @@ int cbmem_initted = 0;
/* Enable PCI MMIO configuration. */ - amd_initmmio(); + if (CONFIG(ROMCC_BOOTBLOCK)) + amd_initmmio();
fill_sysinfo(cb);
if (initial_apic_id == 0) {
- timestamp_init(timestamp_get()); + if (CONFIG(ROMCC_BOOTBLOCK)) + timestamp_init(timestamp_get()); timestamp_add_now(TS_START_ROMSTAGE);
platform_once(cb); @@ -65,6 +75,9 @@ printk(BIOS_DEBUG, "APIC %02d: CPU Family_Model = %08x\n", initial_apic_id, cpuid_eax(1));
+ if (!CONFIG(ROMCC_BOOTBLOCK)) + set_ap_entry_ptr(ap_romstage_main); + agesa_execute_state(cb, AMD_INIT_RESET);
agesa_execute_state(cb, AMD_INIT_EARLY); @@ -105,7 +118,8 @@ struct sysinfo *cb = &romstage_state;
/* Enable PCI MMIO configuration. */ - amd_initmmio(); + if (CONFIG(ROMCC_BOOTBLOCK)) + amd_initmmio();
fill_sysinfo(cb);
@@ -117,6 +131,7 @@ halt(); }
+#if CONFIG(ROMCC_BOOTBLOCK) /* This wrapper enables easy transition away from ROMCC_BOOTBLOCK * keeping changes in cache_as_ram.S easy to manage. */ @@ -129,3 +144,9 @@ { ap_romstage_main(); } +#else +asmlinkage void car_stage_entry(void) +{ + romstage_main(); +} +#endif
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
build bot (Jenkins) (Code Review)
-
HAOUAS Elyes (Code Review)
-
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Michał Żygowski (Code Review)