Hello Michał Żygowski,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/37353
to review the following change.
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
[ALTERNATIVE] binaryPI: implement C bootblock
Change-Id: I5d8888904425ed524e279c6c3223fbf07f2c4506 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, 160 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/37353/1
diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index 728c7b1..750ec4f 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -48,6 +48,18 @@ hex default 0x10000
+config DCACHE_BSP_STACK_SIZE + hex + default 0x4000 + +config PI_AGESA_CAR_HEAP_BASE + hex + default 0x10000000 + +config PI_AGESA_HEAP_SIZE + hex + default 0x30000 + 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..a886133 --- /dev/null +++ b/src/drivers/amd/agesa/bootblock.c @@ -0,0 +1,35 @@ +/* + * 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 <cpu/x86/lapic.h> +#include <cpu/x86/mtrr.h> +#include <cpu/amd/msr.h> +#include <northbridge/amd/agesa/agesa_helper.h> + +asmlinkage void bootblock_c_entry(uint64_t base_timestamp) +{ + amd_initmmio(); + + /* TSC cannot be relied upon. Override the TSC value passed in. */ + bootblock_main_with_basetime(timestamp_get()); +} + +asmlinkage void ap_bootblock_c_entry(void) +{ + amd_initmmio(); + void (*ap_romstage_entry)(void) = (void (*)(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 557c390..47b97b0 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..333ca03 100644 --- a/src/drivers/amd/agesa/romstage.c +++ b/src/drivers/amd/agesa/romstage.c @@ -39,6 +39,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 +53,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 +72,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 +115,8 @@ struct sysinfo *cb = &romstage_state;
/* Enable PCI MMIO configuration. */ - amd_initmmio(); + if (CONFIG(ROMCC_BOOTBLOCK)) + amd_initmmio();
fill_sysinfo(cb);
@@ -117,6 +128,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 +141,4 @@ { ap_romstage_main(); } +#endif diff --git a/src/northbridge/amd/agesa/agesa_helper.h b/src/northbridge/amd/agesa/agesa_helper.h index dcc3360..4a6374e 100644 --- a/src/northbridge/amd/agesa/agesa_helper.h +++ b/src/northbridge/amd/agesa/agesa_helper.h @@ -61,4 +61,7 @@ void backup_mtrr(void *mtrr_store, u32 *mtrr_store_size); const void *OemS3Saved_MTRR_Storage(void);
+void *get_ap_entry_ptr(void); +void set_ap_entry_ptr(void *entry); + #endif /* _AGESA_HELPER_H_ */ diff --git a/src/northbridge/amd/pi/Makefile.inc b/src/northbridge/amd/pi/Makefile.inc index ffafc60..1cedf1e 100644 --- a/src/northbridge/amd/pi/Makefile.inc +++ b/src/northbridge/amd/pi/Makefile.inc @@ -19,6 +19,11 @@ subdirs-$(CONFIG_NORTHBRIDGE_AMD_PI_00730F01) += 00730F01 subdirs-$(CONFIG_NORTHBRIDGE_AMD_PI_00660F01) += 00660F01
+bootblock-y += bootblock.c + +bootblock-y += nb_util.c +romstage-y += nb_util.c + romstage-y += ramtop.c postcar-y += ramtop.c ramstage-y += ramtop.c diff --git a/src/northbridge/amd/pi/bootblock.c b/src/northbridge/amd/pi/bootblock.c new file mode 100644 index 0000000..8031805 --- /dev/null +++ b/src/northbridge/amd/pi/bootblock.c @@ -0,0 +1,43 @@ +/* + * 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 <cpu/x86/lapic.h> +#include <cpu/x86/mtrr.h> +#include <cpu/amd/msr.h> +#include <northbridge/amd/agesa/agesa_helper.h> + +#define PI_AGESA_EARLY_VMTRR_FLASH 1 +#define PI_AGESA_EARLY_VMTRR_CAR_HEAP 2 + +void amd_initmmio(void) +{ + msr_t mmconf; + msr_t mtrr_cap = rdmsr(MTRR_CAP_MSR); + int mtrr; + + mmconf.hi = 0; + mmconf.lo = CONFIG_MMCONF_BASE_ADDRESS | MMIO_RANGE_EN + | fms(CONFIG_MMCONF_BUS_NUMBER) << MMIO_BUS_RANGE_SHIFT; + wrmsr(MMIO_CONF_BASE, mmconf); + + mtrr = (mtrr_cap.lo & MTRR_CAP_VCNT) - PI_AGESA_EARLY_VMTRR_FLASH; + set_var_mtrr(mtrr, OPTIMAL_CACHE_ROM_BASE, OPTIMAL_CACHE_ROM_SIZE, + MTRR_TYPE_WRPROT); + + mtrr = (mtrr_cap.lo & MTRR_CAP_VCNT) - PI_AGESA_EARLY_VMTRR_CAR_HEAP; + set_var_mtrr(mtrr, CONFIG_PI_AGESA_CAR_HEAP_BASE, + CONFIG_PI_AGESA_HEAP_SIZE, MTRR_TYPE_WRBACK); + + if (CONFIG(UDELAY_LAPIC)) + enable_lapic(); +} diff --git a/src/northbridge/amd/pi/nb_util.c b/src/northbridge/amd/pi/nb_util.c new file mode 100644 index 0000000..cfb9afc --- /dev/null +++ b/src/northbridge/amd/pi/nb_util.c @@ -0,0 +1,27 @@ +/* + * 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 <amdblocks/acpimmio.h> +#include <northbridge/amd/agesa/agesa_helper.h> + +#define BIOSRAM_AP_ENTRY 0xec /* 4 bytes */ + +void *get_ap_entry_ptr(void) +{ + return (void *)biosram_read32(BIOSRAM_AP_ENTRY); +} + +void set_ap_entry_ptr(void *entry) +{ + biosram_write32(BIOSRAM_AP_ENTRY, (uintptr_t)entry); +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37353/1/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37353/1/src/drivers/amd/agesa/bootb... PS1, Line 32: 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/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
Patch Set 1: Code-Review-2
No comments here, please. Will be resubmitted with change-id of CB:36914
Michał Żygowski has uploaded a new patch set (#2) to the change originally created by Kyösti Mälkki. ( https://review.coreboot.org/c/coreboot/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
[ALTERNATIVE] binaryPI: implement C bootblock
Change-Id: I5d8888904425ed524e279c6c3223fbf07f2c4506 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, 165 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/37353/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37353/2/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37353/2/src/drivers/amd/agesa/bootb... PS2, Line 32: void (*ap_romstage_entry)(void) = (void (*)(void))get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name
Michał Żygowski has uploaded a new patch set (#3) to the change originally created by Kyösti Mälkki. ( https://review.coreboot.org/c/coreboot/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
[ALTERNATIVE] binaryPI: implement C bootblock
Change-Id: I5d8888904425ed524e279c6c3223fbf07f2c4506 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/53/37353/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37353/3/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37353/3/src/drivers/amd/agesa/bootb... PS3, Line 30: 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/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37353/4/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/37353/4/src/drivers/amd/agesa/bootb... PS4, Line 30: 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 abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37353 )
Change subject: [ALTERNATIVE] binaryPI: implement C bootblock ......................................................................
Abandoned
Submitted with change-id of CB:36914
-
build bot (Jenkins) (Code Review) -
Kyösti Mälkki (Code Review)
-
Michał Żygowski (Code Review)