Change in coreboot[master]: [UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: [UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
[UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init with cbmem instead of storing them to the SPI flash.
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/include/commonlib/cbmem_id.h M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 4 files changed, 21 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/1
diff --git a/src/commonlib/include/commonlib/cbmem_id.h b/src/commonlib/include/commonlib/cbmem_id.h index ac271a0..058a9d4 100644 --- a/src/commonlib/include/commonlib/cbmem_id.h +++ b/src/commonlib/include/commonlib/cbmem_id.h @@ -8,6 +8,7 @@ #define CBMEM_ID_ACPI_UCSI 0x55435349 #define CBMEM_ID_AFTER_CAR 0xc4787a93 #define CBMEM_ID_AGESA_RUNTIME 0x41474553 +#define CBMEM_ID_AGESA_MTRR 0xf08b4b9d #define CBMEM_ID_AMDMCT_MEMINFO 0x494D454E #define CBMEM_ID_CAR_GLOBALS 0xcac4e6a3 #define CBMEM_ID_CBTABLE 0x43425442 @@ -74,6 +75,7 @@ { CBMEM_ID_ACPI_GNVS, "ACPI GNVS " }, \ { CBMEM_ID_ACPI_UCSI, "ACPI UCSI " }, \ { CBMEM_ID_AGESA_RUNTIME, "AGESA RSVD " }, \ + { CBMEM_ID_AGESA_MTRR, "AGESA MTRR " }, \ { CBMEM_ID_AFTER_CAR, "AFTER CAR " }, \ { CBMEM_ID_AMDMCT_MEMINFO, "AMDMEM INFO" }, \ { CBMEM_ID_CAR_GLOBALS, "CAR GLOBALS" }, \ diff --git a/src/drivers/amd/agesa/oem_s3.c b/src/drivers/amd/agesa/oem_s3.c index e47daf3..0b37d3e 100644 --- a/src/drivers/amd/agesa/oem_s3.c +++ b/src/drivers/amd/agesa/oem_s3.c @@ -9,45 +9,24 @@ #include <AGESA.h> #include <northbridge/amd/agesa/agesa_helper.h>
-typedef enum { - S3DataTypeNonVolatile = 0, ///< NonVolatile Data Type - S3DataTypeMTRR ///< MTRR storage -} S3_DATA_TYPE; - /* The size needs to be 4k aligned, which is the sector size of most flashes. */ -#define S3_DATA_MTRR_SIZE 0x1000 #define S3_DATA_NONVOLATILE_SIZE 0x1000
-#if CONFIG(HAVE_ACPI_RESUME) && \ - (S3_DATA_MTRR_SIZE + S3_DATA_NONVOLATILE_SIZE) > CONFIG_S3_DATA_SIZE +#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(S3_DATA_TYPE S3DataType, uintptr_t *pos, uintptr_t *len) +static void get_s3nv_data(uintptr_t *pos, uintptr_t *len) { /* FIXME: Find file from CBFS. */ - u32 s3_data = CONFIG_S3_DATA_POS; - - switch (S3DataType) { - case S3DataTypeMTRR: - *pos = s3_data; - *len = S3_DATA_MTRR_SIZE; - break; - case S3DataTypeNonVolatile: - *pos = s3_data + S3_DATA_MTRR_SIZE; - *len = S3_DATA_NONVOLATILE_SIZE; - break; - default: - *pos = 0; - *len = 0; - break; - } + *pos = CONFIG_S3_DATA_POS; + *len= S3_DATA_NONVOLATILE_SIZE; }
AGESA_STATUS OemInitResume(AMD_S3_PARAMS *dataBlock) { uintptr_t pos, size; - get_s3nv_data(S3DataTypeNonVolatile, &pos, &size); + get_s3nv_data(&pos, &size);
u32 len = *(u32*)pos;
@@ -101,15 +80,12 @@ #endif }
-static u8 MTRRStorage[S3_DATA_MTRR_SIZE]; - AGESA_STATUS OemS3Save(AMD_S3_PARAMS *dataBlock) { - u32 MTRRStorageSize = 0; uintptr_t pos, size;
/* To be consumed in AmdInitResume. */ - get_s3nv_data(S3DataTypeNonVolatile, &pos, &size); + get_s3nv_data(&pos, &size); if (size && dataBlock->NvStorageSize) spi_SaveS3info(pos, size, dataBlock->NvStorage, dataBlock->NvStorageSize); @@ -127,24 +103,9 @@ }
/* Collect MTRR setup. */ - backup_mtrr(MTRRStorage, &MTRRStorageSize); - - /* To be consumed in restore_mtrr, CPU enumeration in ramstage. */ - get_s3nv_data(S3DataTypeMTRR, &pos, &size); - if (size && MTRRStorageSize) - spi_SaveS3info(pos, size, MTRRStorage, MTRRStorageSize); + backup_mtrr();
return AGESA_SUCCESS; }
#endif /* ENV_RAMSTAGE */ - -const void *OemS3Saved_MTRR_Storage(void) -{ - uintptr_t pos, size; - get_s3nv_data(S3DataTypeMTRR, &pos, &size); - if (!size) - return NULL; - - return (void *)(pos + sizeof(UINT32)); -} diff --git a/src/drivers/amd/agesa/s3_mtrr.c b/src/drivers/amd/agesa/s3_mtrr.c index 672eb20..be38327 100644 --- a/src/drivers/amd/agesa/s3_mtrr.c +++ b/src/drivers/amd/agesa/s3_mtrr.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <stdint.h> +#include <cbmem.h> #include <cpu/x86/msr.h> #include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> @@ -20,12 +21,16 @@ msr_t top_mem2; };
-void backup_mtrr(void *mtrr_store, u32 *mtrr_store_size) +void backup_mtrr(void) { msr_t syscfg_msr; - struct mtrr_backup *mtrr_save = (struct mtrr_backup *)mtrr_store; + struct mtrr_backup *mtrr_save; uint32_t i;
+ mtrr_save = cbmem_add(CBMEM_ID_AGESA_MTRR, sizeof(struct mtrr_backup)); + if (!mtrr_save) + return; + /* Enable access to AMD RdDram and WrDram extension bits */ syscfg_msr = rdmsr(SYSCFG_MSR); syscfg_msr.lo |= SYSCFG_MSR_MtrrFixDramModEn; @@ -51,16 +56,17 @@ mtrr_save->syscfg = rdmsr(SYSCFG_MSR); mtrr_save->top_mem = rdmsr(TOP_MEM); mtrr_save->top_mem2 = rdmsr(TOP_MEM2); - - *mtrr_store_size = sizeof(struct mtrr_backup); }
void restore_mtrr(void) { msr_t syscfg_msr; - struct mtrr_backup *mtrr_save = (struct mtrr_backup *)OemS3Saved_MTRR_Storage(); + struct mtrr_backup *mtrr_save = cbmem_find(CBMEM_ID_AGESA_MTRR); uint32_t i;
+ if (!mtrr_save) + return; + /* Enable access to AMD RdDram and WrDram extension bits */ syscfg_msr = rdmsr(SYSCFG_MSR); syscfg_msr.lo |= SYSCFG_MSR_MtrrFixDramModEn; diff --git a/src/northbridge/amd/agesa/agesa_helper.h b/src/northbridge/amd/agesa/agesa_helper.h index 120e74a..ef6085f 100644 --- a/src/northbridge/amd/agesa/agesa_helper.h +++ b/src/northbridge/amd/agesa/agesa_helper.h @@ -39,7 +39,7 @@ void recover_postcar_frame(struct postcar_frame *pcf, int s3resume);
void restore_mtrr(void); -void backup_mtrr(void *mtrr_store, u32 *mtrr_store_size); +void backup_mtrr(void); const void *OemS3Saved_MTRR_Storage(void);
#endif /* _AGESA_HELPER_H_ */
Hello Mike Banon, Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44294
to look at the new patch set (#2).
Change subject: [UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
[UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init in cbmem instead of storing them to the SPI flash.
This was probably done because historically this code run with late cbmem init (in ramstage).
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/include/commonlib/cbmem_id.h M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 4 files changed, 21 insertions(+), 52 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: [UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44294/2/src/drivers/amd/agesa/oem_s... File src/drivers/amd/agesa/oem_s3.c:
https://review.coreboot.org/c/coreboot/+/44294/2/src/drivers/amd/agesa/oem_s... PS2, Line 23: *len= S3_DATA_NONVOLATILE_SIZE; spaces required around that '=' (ctx:VxW)
Hello Mike Banon, build bot (Jenkins), Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44294
to look at the new patch set (#3).
Change subject: [UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
[UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init in cbmem instead of storing them to the SPI flash.
This was probably done because historically this code run with late cbmem init (in ramstage).
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/include/commonlib/cbmem_id.h M src/cpu/amd/agesa/Kconfig M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 5 files changed, 22 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: [UNTESTED]drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44294/3/src/drivers/amd/agesa/oem_s... File src/drivers/amd/agesa/oem_s3.c:
https://review.coreboot.org/c/coreboot/+/44294/3/src/drivers/amd/agesa/oem_s... PS3, Line 23: *len= S3_DATA_NONVOLATILE_SIZE; spaces required around that '=' (ctx:VxW)
Hello Mike Banon, build bot (Jenkins), Michał Żygowski,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44294
to look at the new patch set (#4).
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init in cbmem instead of storing them to the SPI flash.
This was probably done because historically this code run with late cbmem init (in ramstage).
Untested.
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/include/commonlib/cbmem_id.h M src/cpu/amd/agesa/Kconfig M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 5 files changed, 22 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
Patch Set 4:
(1 comment)
File src/drivers/amd/agesa/oem_s3.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134474): https://review.coreboot.org/c/coreboot/+/44294/comment/4f144e35_ad2ec67f PS4, Line 23: *len= S3_DATA_NONVOLATILE_SIZE; spaces required around that '=' (ctx:VxW)
Attention is currently required from: Kyösti Mälkki. Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Kyösti Mälkki,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44294
to look at the new patch set (#5).
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init in cbmem instead of storing them to the SPI flash.
This was probably done because historically this code run with late cbmem init (in ramstage).
Untested.
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h M src/cpu/amd/agesa/Kconfig M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 5 files changed, 22 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/5
Attention is currently required from: Arthur Heymans. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
Patch Set 5:
(1 comment)
File src/drivers/amd/agesa/oem_s3.c:
https://review.coreboot.org/c/coreboot/+/44294/comment/341e8ee5_b8c39152 PS5, Line 29: u32 s3_data = CONFIG_S3_DATA_POS; Layout of the s3nv file changes. This could break some fam16kb with ENABLE_MRC_CACHE=y and UPDATE_IMAGE=y, if s3nv contains data generated with older commit. I think it's fine to ignore this here if we have MRC_CACHE_DATA that will break such compatibility anyways.
Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
Patch Set 5:
(1 comment)
File src/drivers/amd/agesa/oem_s3.c:
https://review.coreboot.org/c/coreboot/+/44294/comment/f6a01afc_1698f8c4 PS5, Line 29: u32 s3_data = CONFIG_S3_DATA_POS;
Layout of the s3nv file changes. This could break some fam16kb with ENABLE_MRC_CACHE=y and UPDATE_IMAGE=y, if s3nv contains data generated with older commit. I think it's fine to ignore this here if we have MRC_CACHE_DATA that will break such compatibility anyways.
Right. The fmap changes already add incompatibility.
Attention is currently required from: Kyösti Mälkki. Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Kyösti Mälkki,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44294
to look at the new patch set (#6).
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init in cbmem instead of storing them to the SPI flash.
This was probably done because historically this code run with late cbmem init (in ramstage).
Untested.
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h M src/cpu/amd/agesa/Kconfig M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 5 files changed, 21 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/6
Attention is currently required from: Kyösti Mälkki. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44294 )
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
Patch Set 6:
(1 comment)
File src/drivers/amd/agesa/oem_s3.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-149198): https://review.coreboot.org/c/coreboot/+/44294/comment/80e02e84_aa03658d PS6, Line 23: *len= S3_DATA_NONVOLATILE_SIZE; spaces required around that '=' (ctx:VxW)
Attention is currently required from: Kyösti Mälkki. Hello Mike Banon, build bot (Jenkins), Michał Żygowski, Kyösti Mälkki,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44294
to look at the new patch set (#7).
Change subject: drivers/amd/agesa: Don't save regular boot MTRR to flash ......................................................................
drivers/amd/agesa: Don't save regular boot MTRR to flash
Save the regular boot MTRR's that are restored on the S3 path during the CPU init in cbmem instead of storing them to the SPI flash.
This was probably done because historically this code run with late cbmem init (in ramstage).
Untested.
Change-Id: Ia58e7cd1afb785ba0c379ba75ef6090b56cb9dc6 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h M src/cpu/amd/agesa/Kconfig M src/drivers/amd/agesa/oem_s3.c M src/drivers/amd/agesa/s3_mtrr.c M src/northbridge/amd/agesa/agesa_helper.h 5 files changed, 21 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/44294/7
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Kyösti Mälkki (Code Review)