Change in coreboot[master]: [UNTESTED]cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44295 )
Change subject: [UNTESTED]cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
[UNTESTED]cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Most of the code is copied from soc/amd/common/block/s3/s3_resume.c
The current default MRC_CACHE FMAP region is 64K instead of 8K. One could make this configurable in the future.
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c 3 files changed, 38 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/1
diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index 499cc5b..f407139 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -18,6 +18,7 @@ select SMM_ASEG select SSE2 select ACPI_NO_SMI_GNVS + select CACHE_MRC_SETTINGS
if CPU_AMD_AGESA
@@ -53,14 +54,6 @@ Try to restore memory training results from non-volatile memory.
-config S3_DATA_POS - hex - default 0xFFFF0000 - -config S3_DATA_SIZE - int - default 8192 - endif # CPU_AMD_AGESA
source "src/cpu/amd/agesa/family14/Kconfig" diff --git a/src/cpu/amd/agesa/Makefile.inc b/src/cpu/amd/agesa/Makefile.inc index 14067e1..1f9cca5 100644 --- a/src/cpu/amd/agesa/Makefile.inc +++ b/src/cpu/amd/agesa/Makefile.inc @@ -3,18 +3,3 @@ subdirs-$(CONFIG_CPU_AMD_AGESA_FAMILY14) += family14 subdirs-$(CONFIG_CPU_AMD_AGESA_FAMILY15_TN) += family15tn subdirs-$(CONFIG_CPU_AMD_AGESA_FAMILY16_KB) += family16kb - -ifeq ($(CONFIG_HAVE_ACPI_RESUME), y) - -$(obj)/coreboot_s3nv.rom: $(obj)/config.h - echo " S3 NVRAM $(CONFIG_S3_DATA_POS) (S3 storage area)" - # force C locale, so cygwin awk doesn't try to interpret the 0xff below as UTF-8 (or worse) - printf %d $(CONFIG_S3_DATA_SIZE) | LC_ALL=C awk '{for (i=0; i<$$1; i++) {printf "%c", 255}}' > $@.tmp - mv $@.tmp $@ - -cbfs-files-y += s3nv -s3nv-file := $(obj)/coreboot_s3nv.rom -s3nv-position := $(CONFIG_S3_DATA_POS) -s3nv-type := raw - -endif # CONFIG_HAVE_ACPI_RESUME == y diff --git a/src/drivers/amd/agesa/oem_s3.c b/src/drivers/amd/agesa/oem_s3.c index 0b37d3e..8d32bae 100644 --- a/src/drivers/amd/agesa/oem_s3.c +++ b/src/drivers/amd/agesa/oem_s3.c @@ -1,41 +1,53 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <spi-generic.h> -#include <spi_flash.h> #include <string.h> +#include <device/mmio.h> #include <cbmem.h> #include <console/console.h> +#include <mrc_cache.h> +#include <reset.h> #include <northbridge/amd/agesa/state_machine.h> #include <AGESA.h> #include <northbridge/amd/agesa/agesa_helper.h>
-/* The size needs to be 4k aligned, which is the sector size of most flashes. */ -#define S3_DATA_NONVOLATILE_SIZE 0x1000 +/* Training data versioning is not supported or tracked. */ +#define DEFAULT_MRC_VERSION 0
-#if CONFIG(HAVE_ACPI_RESUME) && S3_DATA_NONVOLATILE_SIZE > CONFIG_S3_DATA_SIZE -#error "Please increase the value of S3_DATA_SIZE" -#endif - -static void get_s3nv_data(uintptr_t *pos, uintptr_t *len) +static void __noreturn reboot_from_resume(const char *message) { - /* FIXME: Find file from CBFS. */ - *pos = CONFIG_S3_DATA_POS; - *len= S3_DATA_NONVOLATILE_SIZE; + printk(BIOS_ERR, "%s", message); +// set_pm1cnt_s5(); TODO?? + board_reset(); }
AGESA_STATUS OemInitResume(AMD_S3_PARAMS *dataBlock) { - uintptr_t pos, size; - get_s3nv_data(&pos, &size); + void *base; + size_t size; + int i; + uint32_t erased = 0xffffffff; + struct region_device rdev;
- u32 len = *(u32*)pos; + if (mrc_cache_get_current(MRC_TRAINING_DATA, DEFAULT_MRC_VERSION, + &rdev)) + reboot_from_resume("mrc_cache_get_current error, rebooting.\n");
- /* Test for uninitialized s3nv data in SPI. */ - if (len == 0 || len == (u32)-1ULL) - return AGESA_FATAL; + base = rdev_mmap_full(&rdev); + size = region_device_sz(&rdev); + if (!base || !size) + reboot_from_resume("Error: S3 NV data not found, rebooting.\n");
- dataBlock->NvStorageSize = len; - dataBlock->NvStorage = (void *) (pos + sizeof(u32)); + /* Read 16 bytes to infer if the NV has been erased from flash. */ + for (i = 0; i < 4; i++) + erased &= read32((uint32_t *)base + i); + if (erased == 0xffffffff) + reboot_from_resume("Error: S3 NV data invalid, rebooting.\n"); + + dataBlock->NvStorage = base; + dataBlock->NvStorageSize = size; + printk(BIOS_SPEW, "S3 NV data @%p, 0x%0zx bytes\n", + dataBlock->NvStorage, (size_t)dataBlock->NvStorageSize); + return AGESA_SUCCESS; }
@@ -56,44 +68,13 @@ return AGESA_SUCCESS; }
-#if ENV_RAMSTAGE - -static int spi_SaveS3info(u32 pos, u32 size, u8 *buf, u32 len) -{ -#if CONFIG(SPI_FLASH) - struct spi_flash flash; - - spi_init(); - if (spi_flash_probe(0, 0, &flash)) - return -1; - - spi_flash_volatile_group_begin(&flash); - - spi_flash_erase(&flash, pos, size); - spi_flash_write(&flash, pos, sizeof(len), &len); - spi_flash_write(&flash, pos + sizeof(len), len, buf); - - spi_flash_volatile_group_end(&flash); - return 0; -#else - return -1; -#endif -} - AGESA_STATUS OemS3Save(AMD_S3_PARAMS *dataBlock) { - uintptr_t pos, size; - - /* To be consumed in AmdInitResume. */ - get_s3nv_data(&pos, &size); - if (size && dataBlock->NvStorageSize) - spi_SaveS3info(pos, size, dataBlock->NvStorage, - dataBlock->NvStorageSize); - else - printk(BIOS_EMERG, - "Error: Cannot store memory training results in SPI.\n" - "Error: S3 resume will not be possible.\n" - ); + if (mrc_cache_stash_data(MRC_TRAINING_DATA, DEFAULT_MRC_VERSION, + dataBlock->NvStorage, dataBlock->NvStorageSize) < 0) { + printk(BIOS_ERR, "Failed to stash MRC data\n"); + return AGESA_CRITICAL; + }
/* To be consumed in AmdS3LateRestore. */ char *heap = cbmem_add(CBMEM_ID_RESUME_SCRATCH, HIGH_MEMORY_SCRATCH); @@ -107,5 +88,3 @@
return AGESA_SUCCESS; } - -#endif /* ENV_RAMSTAGE */
Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44295
to look at the new patch set (#3).
Change subject: [UNTESTED]cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
[UNTESTED]cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Most of the code is copied from soc/amd/common/block/s3/s3_resume.c
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c 3 files changed, 43 insertions(+), 75 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/3
Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44295
to look at the new patch set (#4).
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Use the common code to save data for fast boot or S3 resume. An notable improvement that comes with this, is that the same 4K page is not rewritten all the time. This prolongs the hardware's life.
UNTESTED.
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c 3 files changed, 22 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/4
Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44295
to look at the new patch set (#5).
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Use the common code to save data for fast boot or S3 resume. An notable improvement that comes with this, is that the same 4K page is not rewritten all the time. This prolongs the hardware's life.
UNTESTED.
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c 3 files changed, 20 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/5
Attention is currently required from: Kyösti Mälkki. Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Martin L Roth, Kyösti Mälkki,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44295
to look at the new patch set (#7).
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Use the common code to save data for fast boot or S3 resume. An notable improvement that comes with this, is that the same 4K page is not rewritten all the time. This prolongs the hardware's life.
UNTESTED.
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c M src/southbridge/amd/agesa/hudson/Kconfig 4 files changed, 21 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/7
Attention is currently required from: Kyösti Mälkki. Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Martin L Roth, Kyösti Mälkki,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44295
to look at the new patch set (#8).
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Use the common code to save data for fast boot or S3 resume. An notable improvement that comes with this, is that the same 4K page is not rewritten all the time. This prolongs the hardware's life.
UNTESTED.
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c M src/southbridge/amd/agesa/hudson/Kconfig M src/southbridge/amd/cimx/sb800/Kconfig 5 files changed, 22 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/8
Attention is currently required from: Arthur Heymans. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44295 )
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/44295/comment/680c1d7d_25548fd1 PS8, Line 11: is not rewritten all the time. This prolongs the hardware's life. AMDFW_OUTSIDE_CBFS option may be obsolete or unused, expect for ChromeOS and stoneyridge -combination. Adding FMAP regions could introduce a conflict with HUDSON_FWM_POSITION in some configuration combinations.
Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44295 )
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/44295/comment/3d83befe_fd57372c PS8, Line 11: is not rewritten all the time. This prolongs the hardware's life.
AMDFW_OUTSIDE_CBFS option may be obsolete or unused, expect for ChromeOS and stoneyridge -combination. Adding FMAP regions could introduce a conflict with HUDSON_FWM_POSITION in some configuration combinations.
Isnt stoneyridge handled in soc/amd instead of here? Stoneyridge already use MRC_CACHE fmap afaict
Attention is currently required from: Marshall Dawson, Arthur Heymans, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44295 )
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/44295/comment/efe9d11a_caa78161 PS8, Line 11: is not rewritten all the time. This prolongs the hardware's life.
AMDFW_OUTSIDE_CBFS option may be obsolete or unused, expect for ChromeOS and stoneyridge -combinat […]
CB:35853 https://ticket.coreboot.org/issues/224
What I meant was that instead of analyzing if AMDFW_OUTSIDE_CBFS together with FMAP causes problems, we might be able to drop it entirely. But some Kconfigs that are unused in upstream repo are used with ChromeOS configs.
Attention is currently required from: Marshall Dawson, Arthur Heymans, Felix Held. Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Martin L Roth, Marshall Dawson, Kyösti Mälkki, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44295
to look at the new patch set (#10).
Change subject: cpu/amd/agesa: Use common MRC_CACHE code to save S3 data ......................................................................
cpu/amd/agesa: Use common MRC_CACHE code to save S3 data
Use the common code to save data for fast boot or S3 resume. An notable improvement that comes with this, is that the same 4K page is not rewritten all the time. This prolongs the hardware's life.
UNTESTED.
Change-Id: I0f4f36dcead52a6c550fb5e606772e0a99029872 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/Makefile.inc M src/drivers/amd/agesa/oem_s3.c M src/southbridge/amd/agesa/hudson/Kconfig M src/southbridge/amd/cimx/sb800/Kconfig 5 files changed, 22 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/44295/10
-
Arthur Heymans (Code Review) -
Kyösti Mälkki (Code Review)