Hello Martin Roth, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to review the following change.
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
soc/amd/picasso: Begin adding FSP support
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easy inclusion into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Increase the size set to WB for ramstage, as ramstage outgrew the region.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 4 files changed, 44 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 840de12..3c27f15 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -61,6 +61,18 @@ select POSTCAR_CONSOLE select SSE2 select RTC + select PLATFORM_USES_FSP2_0 + select UDK_2015_BINDING + select ADD_FSP_BINARIES + select HAVE_CF9_RESET + +config FSP_DEBUG_ALL + bool "Enable all FSP debug support" + default y + select DISPLAY_HOBS + select DISPLAY_UPD_DATA + select DISPLAY_FSP_CALLS_AND_STATUS + select DISPLAY_FSP_HEADER
config VBOOT select VBOOT_SEPARATE_VERSTAGE diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index 8d49271..d94091d 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -26,6 +26,7 @@ #include <soc/pci_devs.h> #include <soc/southbridge.h> #include "chip.h" +#include <fsp/api.h>
/* Supplied by i2c.c */ extern struct device_operations picasso_i2c_mmio_ops; @@ -117,6 +118,8 @@
static void soc_init(void *chip_info) { + fsp_silicon_init(acpi_is_wakeup_s3()); + southbridge_init(chip_info); setup_bsp_ramtop(); } diff --git a/src/soc/amd/picasso/reset.c b/src/soc/amd/picasso/reset.c index 9841038..03cf306 100644 --- a/src/soc/amd/picasso/reset.c +++ b/src/soc/amd/picasso/reset.c @@ -22,6 +22,7 @@ #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> #include <amdblocks/reset.h> +#include <fsp/util.h>
void set_warm_reset_flag(void) { @@ -56,3 +57,17 @@ /* TODO: Would a warm_reset() suffice? */ do_cold_reset(); } + +void chipset_handle_reset(uint32_t status) +{ + switch (status) { + case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ + printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); + do_cold_reset(); + break; + default: + printk(BIOS_ERR, "unhandled reset type %x\n", status); + die("unknown reset type"); + break; + } +} diff --git a/src/soc/amd/picasso/romstage.c b/src/soc/amd/picasso/romstage.c index da4ed8d..2b2813d 100644 --- a/src/soc/amd/picasso/romstage.c +++ b/src/soc/amd/picasso/romstage.c @@ -37,6 +37,7 @@ #include <soc/northbridge.h> #include <soc/southbridge.h> #include <soc/romstage.h> +#include <fsp/api.h>
__weak void romstage_mainboard_early_init(void) {} __weak void romstage_mainboard_init(int s3_resume) {} @@ -80,6 +81,11 @@ timestamp_add(TS_START_ROMSTAGE, stage_start); }
+void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) +{ + // dummy +} + asmlinkage void car_stage_entry(void) { struct postcar_frame pcf; @@ -137,27 +143,26 @@ boot_count_increment();
post_code(0x49); + + /* fsp_memory_init() requires cbmem_top() before returning. Use TOM. + * todo: verify TOM < UMA region when UMA is below 4GB */ msr_t tom = rdmsr(TOP_MEM); tom.lo &= ~0xffffff; backup_top_of_low_cacheable(tom.lo);
- post_code(0x4a); - if (cbmem_recovery(s3_resume)) - printk(BIOS_CRIT, "Failed to recover cbmem\n"); - if (romstage_handoff_init(s3_resume)) - printk(BIOS_ERR, "Failed to set romstage handoff data\n"); + fsp_memory_init(s3_resume);
- post_code(0x4b); + post_code(0x4a); if (postcar_frame_init(&pcf, 1 * KiB)) die("Unable to initialize postcar frame.\n");
/* * We need to make sure ramstage will be run cached. At this point exact * location of ramstage in cbmem is not known. Instruct postcar to cache - * 16 megs under cbmem top which is a safe bet to cover ramstage. + * 32 megs under cbmem top which is a safe bet to cover ramstage. */ top_of_ram = (uintptr_t) cbmem_top(); - postcar_frame_add_mtrr(&pcf, top_of_ram - 16*MiB, 16*MiB, + postcar_frame_add_mtrr(&pcf, top_of_ram - 32*MiB, 32*MiB, MTRR_TYPE_WRBACK);
/* Cache the memory-mapped boot media. */ @@ -174,7 +179,7 @@ tseg_base = (uintptr_t)smm_base; postcar_frame_add_mtrr(&pcf, tseg_base, smm_size, MTRR_TYPE_WRBACK);
- post_code(0x4c); + post_code(0x4b); run_postcar_phase(&pcf);
post_code(0x50); /* Should never see this post code. */
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG@11 PS2, Line 11: e.g. in EDK II, I do not understand this part. Do you mean it needs EDK II sources?
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG@12 PS2, Line 12: legacy BIOS But that is unrelated to coreboot? What do you mean by “legacy BIOS”?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG@11 PS2, Line 11: e.g. in EDK II,
I do not understand this part. […]
All this entire paragraph is doing is establishing a premise. That is, it's impractical (at the very best) to attempt to port AGESA v9 source for use in coreboot. To do so would require something unacceptable, like somehow combining coreboot and EDK II, or modifying AGESA so dramatically that it wouldn't be supportable from a single codebase.
To answer your question, like Intel FSP, the supplier's reference code is prebuilt within a UDK, into a firmware volume. The External Architecture Spec is on Intel's site, however under the hood the reference code still relies on UEFI type stuff. It's built as libraries and drivers, has a PEI dispatcher to load/run modules, etc.
You've seen the AGESA source in vendorcode; that's v5 and AMD refers to v9 as a complete rewrite for UEFI. The FSP route was chosen because it's an established interface for using code written for a UEFI environment.
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG@12 PS2, Line 12: legacy BIOS
What do you mean by “legacy BIOS”?
Anything non-UEFI. That's how v5 was designed. Its interface was intended to minimize disruption for adding into firmware, "call these handful of functions at specifics times"...
I'm glad to reword this to something more descriptive if you'd like.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/reset.c File src/soc/amd/picasso/reset.c:
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/reset.c... PS2, Line 70: die("unknown reset type"); Does this die make sense? Maybe assert so we die if fatal asserts are turned on, and just do the cold reset if fatal asserts are disabled?
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/romstag... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/romstag... PS2, Line 148: todo: verify TOM < UMA region when UMA is below 4GB If this isn't handled by the end of the patch stack, could you file a bug to make sure it get handled.
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
soc/amd/picasso: Begin adding FSP support
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easy inclusion into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Increase the size set to WB for ramstage, as ramstage outgrew the region.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 4 files changed, 34 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/3
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/reset.c File src/soc/amd/picasso/reset.c:
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/reset.c... PS2, Line 70: die("unknown reset type");
Does this die make sense? Maybe assert so we die if fatal asserts are turned on, and just do the co […]
Sure. Also, it looks like there's no point handling a case for any _<num> unless we add that to the FSP. Cold and warm is already called from within the driver.
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/romstag... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/romstag... PS2, Line 148: todo: verify TOM < UMA region when UMA is below 4GB
If this isn't handled by the end of the patch stack, could you file a bug to make sure it get handle […]
Will file a bug due to uncertainty of when we'll be able to test it by moving UMA.
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#5).
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
soc/amd/picasso: Begin adding FSP support
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easy inclusion into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 4 files changed, 70 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/5
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#7).
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
soc/amd/picasso: Begin adding FSP support
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easy inclusion into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 104 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/7
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#8).
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
soc/amd/picasso: Begin adding FSP support
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easy inclusion into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 95 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/8
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#11).
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
soc/amd/picasso: Begin adding FSP support
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easy inclusion into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Determine the memory map from HOBs passed from AGESA, as reading the TOM register is misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/hybrid_romstage.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/southbridge.c 9 files changed, 120 insertions(+), 61 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/11
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Begin adding FSP support ......................................................................
Patch Set 12: Code-Review+1
Hello Richard Spiegel, build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#13).
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easili building into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * Update UPD data and add mainboard calls for this purpose. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Determine the memory map from HOBs passed from AGESA. Relying on the TOM register is misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/include/soc/romstage.h M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c M src/soc/amd/picasso/southbridge.c 11 files changed, 120 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/13
Hello Richard Spiegel, build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#14).
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easili building into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * Update UPD data and add mainboard calls for this purpose. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Determine the memory map from HOBs passed from AGESA. Relying on the TOM register is misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/include/soc/romstage.h M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c M src/soc/amd/picasso/southbridge.c 11 files changed, 120 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/14
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 14:
(3 comments)
Some nits
https://review.coreboot.org/c/coreboot/+/34423/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34423/14//COMMIT_MSG@12 PS14, Line 12: easili easily
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/reset.... File src/soc/amd/picasso/reset.c:
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/reset.... PS14, Line 65: printk(BIOS_ERR, "Error: unexpected call to %s(0x%08x). Doing cold reset.\n", : __func__, status); could we move this above the assert so that when we have fatal asserts turned on we git this output?
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/romsta... PS14, Line 114: alone*/ Add space between alone and */
Hello Richard Spiegel, build bot (Jenkins), Martin Roth, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#15).
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Unlike Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. in EDK II, and has no entry points for easily building into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * Update UPD data and add mainboard calls for this purpose. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Determine the memory map from HOBs passed from AGESA. Relying on the TOM register is misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/include/soc/romstage.h M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c M src/soc/amd/picasso/southbridge.c 11 files changed, 120 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/15
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 15:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34423/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34423/14//COMMIT_MSG@12 PS14, Line 12: easili
easily
Done
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/reset.... File src/soc/amd/picasso/reset.c:
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/reset.... PS14, Line 65: printk(BIOS_ERR, "Error: unexpected call to %s(0x%08x). Doing cold reset.\n", : __func__, status);
could we move this above the assert so that when we have fatal asserts turned on we git this output?
Done
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/14/src/soc/amd/picasso/romsta... PS14, Line 114: alone*/
Add space between alone and */
Done
Hello Richard Spiegel, build bot (Jenkins), Martin Roth, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#16).
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * Update UPD data and add mainboard calls for this purpose. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver does this step. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/include/soc/romstage.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 9 files changed, 93 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/16
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 16:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG@11 PS2, Line 11: e.g. in EDK II,
All this entire paragraph is doing is establishing a premise. […]
Done
https://review.coreboot.org/c/coreboot/+/34423/2//COMMIT_MSG@12 PS2, Line 12: legacy BIOS
What do you mean by “legacy BIOS”? […]
Done
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/reset.c File src/soc/amd/picasso/reset.c:
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/reset.c... PS2, Line 70: die("unknown reset type");
Sure. […]
Done
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/romstag... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/2/src/soc/amd/picasso/romstag... PS2, Line 148: todo: verify TOM < UMA region when UMA is below 4GB
Will file a bug due to uncertainty of when we'll be able to test it by moving UMA.
OBE. Using HOBs now instead of assuming we can rely on TOM.
Hello Richard Spiegel, build bot (Jenkins), Martin Roth, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34423
to look at the new patch set (#20).
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * Update UPD data and add mainboard calls for this purpose. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/include/soc/romstage.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 9 files changed, 164 insertions(+), 69 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/20
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 51: /* Set 0 - 640K only, 640K - 1M wasn't modified */ 'wasn' may be misspelled - perhaps 'was'?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 21:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 39: clear_agesa_mtrrs Wouldn't it be more clear if MTRR's are cleared in one function and restored (to some extend) in a different one?
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 83: TODO: make AGESA leave the MTRRs alone Looks like this has been a problem ever since AGESA has been integrated in coreboot...
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 91: mem_top - 16 * MiB needs to be aligned to its size.
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 103: tseg_base needs to be aligned to its size.
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 120: config != NULL Die earlier if that is not the case? That allows to reduce the indentation.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/22/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/22/src/soc/amd/picasso/romsta... PS22, Line 51: /* Set 0 - 640K only, 640K - 1M wasn't modified */ 'wasn' may be misspelled - perhaps 'was'?
Felix Held has uploaded a new patch set (#23) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * Update UPD data and add mainboard calls for this purpose. * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/include/soc/romstage.h M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 9 files changed, 165 insertions(+), 69 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/23
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/23/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/23/src/soc/amd/picasso/romsta... PS23, Line 52: /* Set 0 - 640K only, 640K - 1M wasn't modified */ 'wasn' may be misspelled - perhaps 'was'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/24/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/24/src/soc/amd/picasso/romsta... PS24, Line 52: /* Set 0 - 640K only, 640K - 1M wasn't modified */ 'wasn' may be misspelled - perhaps 'was'?
Raul Rangel has uploaded a new patch set (#26) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
BUG=b:147042464 TEST=Boot trembyle to payload
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 112 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/26
Raul Rangel has uploaded a new patch set (#27) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
BUG=b:147042464 TEST=Boot trembyle to payload
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 109 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/27
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 27:
(6 comments)
PTAL
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 39: clear_agesa_mtrrs
Wouldn't it be more clear if MTRR's are cleared in one function and restored (to some extend) in a d […]
Done
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 83: TODO: make AGESA leave the MTRRs alone
Looks like this has been a problem ever since AGESA has been integrated in coreboot...
Ack
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 91: mem_top - 16 * MiB
needs to be aligned to its size.
Removed all MTRR caching from this patch.
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 103: tseg_base
needs to be aligned to its size.
Ack
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 120: config != NULL
Die earlier if that is not the case? That allows to reduce the indentation.
Done
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 120: config != NULL
Die earlier if that is not the case? That allows to reduce the indentation.
Done
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 27: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... PS27, Line 34: /* Disable WB from to region 4GB - TOM2 */ I'm curious why we are doing this from a policy standpoint.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 27:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... PS27, Line 34: /* Disable WB from to region 4GB - TOM2 */
I'm curious why we are doing this from a policy standpoint.
AGESA has always set MTRRs and the default caching. We went through this with Stoney Ridge as well until it was removed from that version of AGESA. So the intent was to restore settings to a pre-AGESA state.
Maybe this could be in CB:40922 too. And rather than jamming it to the default setting, rewrite SYS_CFG if it was modified by AGESA (possibly certain bits).
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 27:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... PS27, Line 34: /* Disable WB from to region 4GB - TOM2 */
AGESA has always set MTRRs and the default caching. […]
We need ensure to AGESA/FSP doesn't manipulate things it shouldn't. MTRR and cacheablility resources are definitely something it should not be doing. We have a bug to fix that. And once that is done we can nuke all this code as coreboot decides the policy w/o having to fix up things after AGESA runs.
Raul Rangel has uploaded a new patch set (#28) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
BUG=b:147042464 TEST=Boot trembyle to payload
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 104 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/28
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Uploaded patch set 28.
Raul Rangel has uploaded a new patch set (#29) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
BUG=b:147042464 TEST=Boot trembyle to payload
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 104 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/29
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Uploaded patch set 29: New patch set was added with same tree, parent, and commit message as Patch Set 28.
Raul Rangel has uploaded a new patch set (#30) to the change originally created by Marshall Dawson. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
BUG=b:147042464 TEST=Boot trembyle to payload
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 104 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/34423/30
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Uploaded patch set 30: New patch set was added with same tree, parent, and commit message as Patch Set 29.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/27/src/soc/amd/picasso/romsta... PS27, Line 34: /* Disable WB from to region 4GB - TOM2 */
We need ensure to AGESA/FSP doesn't manipulate things it shouldn't. […]
I modified the parent CL so it saves the SYSCFG registers as well.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 30: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/30/src/soc/amd/picasso/Kconfi... File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/34423/30/src/soc/amd/picasso/Kconfi... PS30, Line 235: 0x40000 Just curious: Is this chosen based on analysis of how much heap and stack is used by FSP? Is it documented anywhere?
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/30/src/soc/amd/picasso/Kconfi... File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/34423/30/src/soc/amd/picasso/Kconfi... PS30, Line 235: 0x40000
Just curious: Is this chosen based on analysis of how much heap and stack is used by FSP? Is it docu […]
I'm not actually sure. I filed a bug: b/155501050.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34423/30/src/soc/amd/picasso/Kconfi... File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/34423/30/src/soc/amd/picasso/Kconfi... PS30, Line 235: 0x40000
I'm not actually sure. I filed a bug: b/155501050.
Thanks Raul!
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 30: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
soc/amd/picasso: Add FSP support for including AGESA
AMD has rewritten AGESA (now at v9) for direct inclusion into UEFI build environments. Therefore, unlike the previous Arch2008 (a.k.a. v5), it can't be built without additional source, e.g. by combining with EDK II, and it has no entry points for easily building it into a legacy BIOS.
AGESA in coreboot now relies on the FSP 2.0 framework published by Intel and uses the existing fsp2_0 driver.
* Add fsp_memory_init() to romstage.c. Although Picasso comes out of reset with DRAM alive, this call is added to maximize compatibility and facilitate internal development. Future work may look at removing it. AGESA reports the memory map to coreboot via HOBs returned from fsp_memory_init(). * AGESA currently sets up MTRRs, as in most older generations. Take ownership back immediately before running ramstage. * Remove cbmem initialization, as the FSP driver handles this. * Add chipset_handle_reset() for compatibility. * Top of memory is determined by the FSP driver checking the HOBs passed from AGESA. Note that relying on the TOM register happens to be misleading when UMA is below 4GB.
BUG=b:147042464 TEST=Boot trembyle to payload
Change-Id: Iecb3a3f2599a8ccbc168b1d26a0271f51b71dcf0 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34423 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/memmap.c M src/soc/amd/picasso/reset.c M src/soc/amd/picasso/romstage.c 6 files changed, 104 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 5996cc6..c807ad4 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -47,6 +47,10 @@ select HAVE_SMI_HANDLER select SSE2 select RTC + select PLATFORM_USES_FSP2_0 + select FSP_USES_CB_STACK + select UDK_2017_BINDING + select HAVE_CF9_RESET
config AMD_FP5 def_bool y if !AMD_FT5 @@ -225,6 +229,13 @@ hex default 0x800
+config FSP_TEMP_RAM_SIZE + hex + depends on FSP_USES_CB_STACK + default 0x40000 + help + The amount of coreboot-allocated heap and stack usage by the FSP. + menu "PSP Configuration Options"
config AMDFW_OUTSIDE_CBFS diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 40275ee..71604d1 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -73,6 +73,7 @@ CPPFLAGS_common += -I$(src)/soc/amd/picasso CPPFLAGS_common += -I$(src)/soc/amd/picasso/include CPPFLAGS_common += -I$(src)/soc/amd/picasso/acpi +CPPFLAGS_common += -I$(src)/vendorcode/amd/fsp/picasso
# ROMSIG Normally At ROMBASE + 0x20000 # Overridden by CONFIG_AMD_FWM_POSITION_INDEX diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index 201afb4..2ab9462 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -13,6 +13,7 @@ #include <soc/pci_devs.h> #include <soc/southbridge.h> #include "chip.h" +#include <fsp/api.h>
/* Supplied by i2c.c */ extern struct device_operations picasso_i2c_mmio_ops; @@ -99,6 +100,8 @@
static void soc_init(void *chip_info) { + fsp_silicon_init(acpi_is_wakeup_s3()); + southbridge_init(chip_info); setup_bsp_ramtop(); } diff --git a/src/soc/amd/picasso/memmap.c b/src/soc/amd/picasso/memmap.c index c6fd118..7b504af 100644 --- a/src/soc/amd/picasso/memmap.c +++ b/src/soc/amd/picasso/memmap.c @@ -6,37 +6,11 @@ #include <assert.h> #include <stdint.h> #include <console/console.h> -#include <cpu/x86/msr.h> #include <cpu/x86/smm.h> #include <cpu/amd/msr.h> -#include <cpu/amd/mtrr.h> -#include <cbmem.h> -#include <arch/bert_storage.h> -#include <soc/northbridge.h> -#include <soc/iomap.h> -#include <amdblocks/acpimmio.h> - -void *cbmem_top_chipset(void) -{ - msr_t tom = rdmsr(TOP_MEM); - - if (!tom.lo) - return 0; - - /* 8MB alignment to keep MTRR usage low */ - return (void *)ALIGN_DOWN(restore_top_of_low_cacheable() - - CONFIG_SMM_TSEG_SIZE, 8*MiB); -} - -static uintptr_t smm_region_start(void) -{ - return (uintptr_t)cbmem_top(); -} - -static size_t smm_region_size(void) -{ - return CONFIG_SMM_TSEG_SIZE; -} +#include <memrange.h> +#include <fsp/util.h> +#include <FspGuids.h>
/* * For data stored in TSEG, ensure TValid is clear so R/W access can reach @@ -63,9 +37,21 @@ void smm_region(uintptr_t *start, size_t *size) { static int once; + struct range_entry tseg; + int status;
- *start = smm_region_start(); - *size = smm_region_size(); + *start = 0; + *size = 0; + + status = fsp_find_range_hob(&tseg, AMD_FSP_TSEG_HOB_GUID.b); + + if (status < 0) { + printk(BIOS_ERR, "Error: unable to find TSEG HOB\n"); + return; + } + + *start = (uintptr_t)range_entry_base(&tseg); + *size = range_entry_size(&tseg);
if (!once) { clear_tvalid(); diff --git a/src/soc/amd/picasso/reset.c b/src/soc/amd/picasso/reset.c index dda08de..81a4cab 100644 --- a/src/soc/amd/picasso/reset.c +++ b/src/soc/amd/picasso/reset.c @@ -9,6 +9,8 @@ #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> #include <amdblocks/reset.h> +#include <fsp/util.h> +#include <assert.h>
void set_warm_reset_flag(void) { @@ -43,3 +45,11 @@ /* TODO: Would a warm_reset() suffice? */ do_cold_reset(); } + +void chipset_handle_reset(uint32_t status) +{ + printk(BIOS_ERR, "Error: unexpected call to %s(0x%08x). Doing cold reset.\n", + __func__, status); + assert(0); + do_cold_reset(); +} diff --git a/src/soc/amd/picasso/romstage.c b/src/soc/amd/picasso/romstage.c index bbbc891..329429e 100644 --- a/src/soc/amd/picasso/romstage.c +++ b/src/soc/amd/picasso/romstage.c @@ -4,8 +4,10 @@ #include <arch/cpu.h> #include <arch/romstage.h> #include <arch/acpi.h> +#include <cpu/x86/cache.h> #include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> +#include <console/uart.h> #include <cbmem.h> #include <commonlib/helpers.h> #include <console/console.h> @@ -13,15 +15,69 @@ #include <romstage_handoff.h> #include <elog.h> #include <soc/romstage.h> +#include <soc/mtrr.h> +#include "chip.h" +#include <fsp/api.h>
void __weak mainboard_romstage_entry_s3(int s3_resume) { /* By default, don't do anything */ }
+/* TODO(b/155426691): Make FSP AGESA leave MTRRs alone */ +static void clear_agesa_mtrrs(void) +{ + disable_cache(); + + picasso_restore_mtrrs(); + + enable_cache(); +} + +void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) +{ + FSP_M_CONFIG *mcfg = &mupd->FspmConfig; + const config_t *config = config_of_soc(); + + mcfg->pci_express_base_addr = CONFIG_MMCONF_BASE_ADDRESS; + mcfg->tseg_size = CONFIG_SMM_TSEG_SIZE; + mcfg->serial_port_base = uart_platform_base(CONFIG_UART_FOR_CONSOLE); + mcfg->serial_port_use_mmio = CONFIG(DRIVERS_UART_8250MEM); + mcfg->serial_port_stride = CONFIG(DRIVERS_UART_8250MEM_32) ? 4 : 1; + mcfg->serial_port_baudrate = get_uart_baudrate(); + mcfg->serial_port_refclk = uart_platform_refclk(); + + mcfg->system_config = config->system_config; + + if ((config->slow_ppt_limit) && + (config->fast_ppt_limit) && + (config->slow_ppt_time_constant) && + (config->stapm_time_constant)) { + mcfg->slow_ppt_limit = config->slow_ppt_limit; + mcfg->fast_ppt_limit = config->fast_ppt_limit; + mcfg->slow_ppt_time_constant = config->slow_ppt_time_constant; + mcfg->stapm_time_constant = config->stapm_time_constant; + } + + mcfg->sustained_power_limit = config->sustained_power_limit; + mcfg->prochot_l_deassertion_ramp_time = config->prochot_l_deassertion_ramp_time; + mcfg->thermctl_limit = config->thermctl_limit; + mcfg->psi0_current_limit = config->psi0_current_limit; + mcfg->psi0_soc_current_limit = config->psi0_soc_current_limit; + mcfg->vddcr_soc_voltage_margin = config->vddcr_soc_voltage_margin; + mcfg->vddcr_vdd_voltage_margin = config->vddcr_vdd_voltage_margin; + mcfg->vrm_maximum_current_limit = config->vrm_maximum_current_limit; + mcfg->vrm_soc_maximum_current_limit = config->vrm_soc_maximum_current_limit; + mcfg->vrm_current_limit = config->vrm_current_limit; + mcfg->vrm_soc_current_limit = config->vrm_soc_current_limit; + mcfg->sb_tsi_alert_comparator_mode_en = config->sb_tsi_alert_comparator_mode_en; + mcfg->core_dldo_bypass = config->core_dldo_bypass; + mcfg->min_soc_vid_offset = config->min_soc_vid_offset; + mcfg->aclk_dpm0_freq_400MHz = config->aclk_dpm0_freq_400MHz; +} + asmlinkage void car_stage_entry(void) { - uintptr_t top_of_mem; int s3_resume;
post_code(0x40); @@ -37,16 +93,15 @@ printk(BIOS_DEBUG, "Family_Model: %08x\n", val);
post_code(0x43); - top_of_mem = ALIGN_DOWN(rdmsr(TOP_MEM).lo, 8 * MiB); - backup_top_of_low_cacheable(top_of_mem); + picasso_save_mtrrs();
post_code(0x44); - if (cbmem_recovery(s3_resume)) - printk(BIOS_CRIT, "Failed to recover cbmem\n"); - if (romstage_handoff_init(s3_resume)) - printk(BIOS_ERR, "Failed to set romstage handoff data\n"); + fsp_memory_init(s3_resume);
post_code(0x45); + clear_agesa_mtrrs(); + + post_code(0x46); run_ramstage();
post_code(0x50); /* Should never see this post code. */
-
Aaron Durbin (Code Review) -
Arthur Heymans (Code Review)
-
build bot (Jenkins) (Code Review)
-
Felix Held (Code Review)
-
Furquan Shaikh (Code Review)
-
Marshall Dawson (Code Review)
-
Martin Roth (Code Review)
-
Paul Menzel (Code Review)
-
Raul Rangel (Code Review)
-
Richard Spiegel (Code Review)