coreboot-gerrit November 2016

coreboot-gerrit@coreboot.org

1 participants
1666 discussions

Patch set updated for coreboot: quick_ram_check: Remove reference to RAMBASE
by Kyösti Mälkki 08 Nov '16

08 Nov '16

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15239 -gerrit commit 7eef180bc4e2861442747680572e71d10262e0dd Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Fri Jun 17 23:31:42 2016 +0300 quick_ram_check: Remove reference to RAMBASE Change-Id: Ieb8f11690fb0e9b287d866be56010bb9adefd21d Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/lib/ramtest.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/lib/ramtest.c b/src/lib/ramtest.c index a29fa25..6d12060 100644 --- a/src/lib/ramtest.c +++ b/src/lib/ramtest.c @@ -181,32 +181,37 @@ int ram_check_noprint_nodie(unsigned long start, unsigned long stop) return failures; } -void quick_ram_check(void) +static void __quick_ram_check(uintptr_t dst) { int fail = 0; u32 backup; - backup = read_phys(CONFIG_RAMBASE); - write_phys(CONFIG_RAMBASE, 0x55555555); + backup = read_phys(dst); + write_phys(dst, 0x55555555); phys_memory_barrier(); - if (read_phys(CONFIG_RAMBASE) != 0x55555555) + if (read_phys(dst) != 0x55555555) fail=1; - write_phys(CONFIG_RAMBASE, 0xaaaaaaaa); + write_phys(dst, 0xaaaaaaaa); phys_memory_barrier(); - if (read_phys(CONFIG_RAMBASE) != 0xaaaaaaaa) + if (read_phys(dst) != 0xaaaaaaaa) fail=1; - write_phys(CONFIG_RAMBASE, 0x00000000); + write_phys(dst, 0x00000000); phys_memory_barrier(); - if (read_phys(CONFIG_RAMBASE) != 0x00000000) + if (read_phys(dst) != 0x00000000) fail=1; - write_phys(CONFIG_RAMBASE, 0xffffffff); + write_phys(dst, 0xffffffff); phys_memory_barrier(); - if (read_phys(CONFIG_RAMBASE) != 0xffffffff) + if (read_phys(dst) != 0xffffffff) fail=1; - write_phys(CONFIG_RAMBASE, backup); + write_phys(dst, backup); if (fail) { post_code(0xea); die("RAM INIT FAILURE!\n"); } phys_memory_barrier(); } + +void quick_ram_check(void) +{ + __quick_ram_check(0x100000); +}

1 0

Patch set updated for coreboot: ACPI S3: Remove HIGH_MEMORY_SAVE where possible
by Kyösti Mälkki 08 Nov '16

08 Nov '16

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15255 -gerrit commit 4a372246144beffa9b521a0f22a800ab9261aa99 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Tue Jun 28 07:38:46 2016 +0300 ACPI S3: Remove HIGH_MEMORY_SAVE where possible Add implementation to use actual requirements of ramstage size for S3 resume backup in CBMEM. The backup covers complete pages of 4 KiB. Only the required amount of low memory is backed up when ACPI_TINY_LOWMEM_BACKUP is selected for the platform. Enable this option for AGESA and binaryPI, other platforms (without RELOCATABLE_RAMSTAGE) currently keep their romstage ramstack in low memory for s3 resume path. Change-Id: Ide7ce013f3727c2928cdb00fbcc7e7e84e859ff1 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/Kconfig | 8 +++ src/arch/x86/acpi_s3.c | 133 +++++++++++++++++++++++++++++++----- src/arch/x86/include/arch/acpi.h | 12 ++-- src/cpu/amd/agesa/Kconfig | 1 + src/cpu/amd/car/post_cache_as_ram.c | 4 +- src/cpu/amd/pi/Kconfig | 1 + src/include/cbfs.h | 3 + src/include/program_loading.h | 4 ++ src/lib/cbfs.c | 12 ++++ src/lib/prog_loaders.c | 14 +++- 10 files changed, 166 insertions(+), 26 deletions(-) diff --git a/src/Kconfig b/src/Kconfig index d6af6eb..6623e0d 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -539,6 +539,14 @@ config HAVE_ACPI_RESUME bool default n +config ACPI_TINY_LOWMEM_BACKUP + bool + default n + +config ACPI_HUGE_LOWMEM_BACKUP + bool + default !ACPI_TINY_LOWMEM_BACKUP + config RESUME_PATH_SAME_AS_BOOT bool default y if ARCH_X86 diff --git a/src/arch/x86/acpi_s3.c b/src/arch/x86/acpi_s3.c index c327cbd..534ec67 100644 --- a/src/arch/x86/acpi_s3.c +++ b/src/arch/x86/acpi_s3.c @@ -19,8 +19,10 @@ #include <cbmem.h> #include <cpu/cpu.h> #include <timestamp.h> +#include <program_loading.h> #include <romstage_handoff.h> #include <rules.h> +#include <symbols.h> #if ENV_RAMSTAGE @@ -79,30 +81,121 @@ void acpi_fail_wakeup(void) } #endif /* ENV_RAMSTAGE */ +struct resume_backup { + uint64_t cbmem; + uint64_t lowmem; + uint64_t size; + uint8_t valid; +}; + +#define BACKUP_PAGE_SZ 4096 + +static int backup_create_or_update(struct resume_backup *backup_mem, + uintptr_t base, size_t size) +{ + uintptr_t top; + + if (IS_ENABLED(CONFIG_ACPI_HUGE_LOWMEM_BACKUP)) { + base = CONFIG_RAMBASE; + size = HIGH_MEMORY_SAVE; + } + + /* Align backup region to complete pages. */ + top = ALIGN_UP(base + size, BACKUP_PAGE_SZ); + base = ALIGN_DOWN(base, BACKUP_PAGE_SZ); + size = top - base; + + /* Cannot extend existing region, should not happen. */ + if (backup_mem && (backup_mem->size < size)) + return -1; + + /* Allocate backup with room for header. */ + if (!backup_mem) { + size_t header_sz = ALIGN_UP(sizeof(*backup_mem), BACKUP_PAGE_SZ); + backup_mem = cbmem_add(CBMEM_ID_RESUME, header_sz + size); + if (!backup_mem) + return -1; + + /* Container starts from boundary after header. */ + backup_mem->cbmem = (uintptr_t)backup_mem + header_sz; + } + + backup_mem->valid = 0; + backup_mem->lowmem = base; + backup_mem->size = size; + return 0; +} + +void *acpi_backup_container(uintptr_t base, size_t size) +{ + struct resume_backup *backup_mem = cbmem_find(CBMEM_ID_RESUME); + if (!backup_mem) + return NULL; + + if (!IS_ALIGNED(base, BACKUP_PAGE_SZ) || !IS_ALIGNED(size, BACKUP_PAGE_SZ)) + return NULL; + + if (backup_create_or_update(backup_mem, base, size) < 0) + return NULL; + + backup_mem->valid = 1; + return (void*)(uintptr_t)backup_mem->cbmem; +} + +void backup_ramstage_section(uintptr_t base, size_t size) +{ + struct resume_backup *backup_mem = cbmem_find(CBMEM_ID_RESUME); + if (!backup_mem) + return; + + /* Check that the backup is not done twice. */ + if (backup_mem->valid) + return; + + /* When we are called from ramstage loader, update header with + * properties of the ramstage we will load. + */ + if (backup_create_or_update(backup_mem, base, size) < 0) + return; + + /* Back up the OS-controlled memory where ramstage will be loaded. */ + memcpy((void*)(uintptr_t)backup_mem->cbmem, + (void*)(uintptr_t)backup_mem->lowmem, (size_t)backup_mem->size); + backup_mem->valid = 1; +} + +/* Make backup of low-memory region, relying on the base and size + * of the ramstage that was loaded before entry to ACPI S3. + * + * DEPRECATED + */ void acpi_prepare_for_resume(void) { - if (!HIGH_MEMORY_SAVE) + struct resume_backup *backup_mem = cbmem_find(CBMEM_ID_RESUME); + if (!backup_mem) return; /* Back up the OS-controlled memory where ramstage will be loaded. */ - void *src = (void *)CONFIG_RAMBASE; - void *dest = cbmem_find(CBMEM_ID_RESUME); - if (dest != NULL) - memcpy(dest, src, HIGH_MEMORY_SAVE); + memcpy((void*)(uintptr_t)backup_mem->cbmem, + (void*)(uintptr_t)backup_mem->lowmem, (size_t)backup_mem->size); + backup_mem->valid = 1; } +/* Let's prepare the ACPI S3 Resume area now already, so we can rely on + * it being there during reboot time. If this fails, ACPI resume will + * be disabled. We assume that ramstage does not change while in suspend, + * so base and size of the currently running ramstage are used + * for allocation. + */ void acpi_prepare_resume_backup(void) { if (!acpi_s3_resume_allowed()) return; - /* Let's prepare the ACPI S3 Resume area now already, so we can rely on - * it being there during reboot time. We don't need the pointer, nor - * the result right now. If it fails, ACPI resume will be disabled. - */ + if (IS_ENABLED(CONFIG_RELOCATABLE_RAMSTAGE)) + return; - if (HIGH_MEMORY_SAVE) - cbmem_add(CBMEM_ID_RESUME, HIGH_MEMORY_SAVE); + backup_create_or_update(NULL, (uintptr_t)_program, _program_size); } #define WAKEUP_BASE 0x600 @@ -115,17 +208,22 @@ extern unsigned int __wakeup_size; static void acpi_jump_to_wakeup(void *vector) { - uintptr_t acpi_backup_memory = 0; + uintptr_t source = 0, target = 0; + size_t size = 0; if (!acpi_s3_resume_allowed()) { printk(BIOS_WARNING, "ACPI: S3 resume not allowed.\n"); return; } - if (HIGH_MEMORY_SAVE) { - acpi_backup_memory = (uintptr_t)cbmem_find(CBMEM_ID_RESUME); - - if (!acpi_backup_memory) { + if (!IS_ENABLED(CONFIG_RELOCATABLE_RAMSTAGE)) { + struct resume_backup *backup_mem = cbmem_find(CBMEM_ID_RESUME); + if (backup_mem && backup_mem->valid) { + backup_mem->valid = 0; + target = backup_mem->lowmem; + source = backup_mem->cbmem; + size = backup_mem->size; + } else { printk(BIOS_WARNING, "ACPI: Backup memory missing. " "No S3 resume.\n"); return; @@ -137,8 +235,7 @@ static void acpi_jump_to_wakeup(void *vector) timestamp_add_now(TS_ACPI_WAKE_JUMP); - acpi_do_wakeup((uintptr_t)vector, acpi_backup_memory, CONFIG_RAMBASE, - HIGH_MEMORY_SAVE); + acpi_do_wakeup((uintptr_t)vector, source, target, size); } void __attribute__((weak)) mainboard_suspend_resume(void) diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index 7fc1c50..7c29648 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -25,12 +25,7 @@ #ifndef __ASM_ACPI_H #define __ASM_ACPI_H -#if IS_ENABLED(CONFIG_HAVE_ACPI_RESUME) && \ - ! IS_ENABLED(CONFIG_RELOCATABLE_RAMSTAGE) #define HIGH_MEMORY_SAVE (CONFIG_RAMTOP - CONFIG_RAMBASE) -#else -#define HIGH_MEMORY_SAVE 0 -#endif #if IS_ENABLED(CONFIG_ACPI_INTEL_HARDWARE_SLEEP_VALUES) /* @@ -688,6 +683,13 @@ static inline int acpi_s3_resume_allowed(void) return IS_ENABLED(CONFIG_HAVE_ACPI_RESUME); } +/* Return address in reserved memory where to backup low memory + * while platform resumes from S3 suspend. Caller is responsible of + * making a complete copy of the region base..base+size, with + * parameteres base and size that meet page alignment requirement. + */ +void *acpi_backup_container(uintptr_t base, size_t size); + #if IS_ENABLED(CONFIG_HAVE_ACPI_RESUME) extern int acpi_slp_type; diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index ae5e854..7fe24ce 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -31,6 +31,7 @@ config CPU_AMD_AGESA select UDELAY_LAPIC select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME + select ACPI_TINY_LOWMEM_BACKUP if CPU_AMD_AGESA diff --git a/src/cpu/amd/car/post_cache_as_ram.c b/src/cpu/amd/car/post_cache_as_ram.c index 19aa0a2..296adc9 100644 --- a/src/cpu/amd/car/post_cache_as_ram.c +++ b/src/cpu/amd/car/post_cache_as_ram.c @@ -69,7 +69,7 @@ static void prepare_romstage_ramstack(int s3resume) print_car_debug("Prepare CAR migration and stack regions..."); if (s3resume) { - void *resume_backup_memory = cbmem_find(CBMEM_ID_RESUME); + void *resume_backup_memory = acpi_backup_container(CONFIG_RAMBASE, HIGH_MEMORY_SAVE); if (resume_backup_memory) memcpy_(resume_backup_memory + HIGH_MEMORY_SAVE - backup_top, (void *)(CONFIG_RAMTOP - backup_top), backup_top); @@ -85,7 +85,7 @@ static void prepare_ramstage_region(int s3resume) print_car_debug("Prepare ramstage memory region..."); if (s3resume) { - void *resume_backup_memory = cbmem_find(CBMEM_ID_RESUME); + void *resume_backup_memory = acpi_backup_container(CONFIG_RAMBASE, HIGH_MEMORY_SAVE); if (resume_backup_memory) memcpy_(resume_backup_memory, (void *) CONFIG_RAMBASE, HIGH_MEMORY_SAVE - backup_top); diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index ed379ea..203fa03 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -28,6 +28,7 @@ config CPU_AMD_PI select UDELAY_LAPIC select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME + select ACPI_TINY_LOWMEM_BACKUP if CPU_AMD_PI diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 6063dd6..8538b3d 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -47,6 +47,9 @@ size_t cbfs_boot_load_struct(const char *name, void *buf, size_t buf_size); size_t cbfs_load_and_decompress(const struct region_device *rdev, size_t offset, size_t in_size, void *buffer, size_t buffer_size, uint32_t compression); +/* Return the size and fill base of the memory pstage will occupy after loaded. */ +size_t cbfs_prog_stage_section(struct prog *pstage, uintptr_t *base); + /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog); diff --git a/src/include/program_loading.h b/src/include/program_loading.h index 08687f3..416e2e9 100644 --- a/src/include/program_loading.h +++ b/src/include/program_loading.h @@ -173,6 +173,10 @@ uintptr_t romstage_ram_stack_base(size_t size, int src); uintptr_t romstage_ram_stack_top(void); uintptr_t romstage_ram_stack_bottom(void); +/* Backup OS memory to CBMEM_ID_RESUME on ACPI S3 resume path, + * if ramstage overwrites low memory. */ +void backup_ramstage_section(uintptr_t base, size_t size); + /*********************** * PAYLOAD LOADING * ***********************/ diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 5a2f63f..19737a4 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -187,6 +187,18 @@ size_t cbfs_boot_load_struct(const char *name, void *buf, size_t buf_size) buf, buf_size, compression_algo); } +size_t cbfs_prog_stage_section(struct prog *pstage, uintptr_t *base) +{ + struct cbfs_stage stage; + const struct region_device *fh = prog_rdev(pstage); + + if (rdev_readat(fh, &stage, 0, sizeof(stage)) != sizeof(stage)) + return 0; + + *base = (uintptr_t)stage.load; + return stage.memlen; +} + int cbfs_prog_stage_load(struct prog *pstage) { struct cbfs_stage stage; diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c index 09933ae..3a6f2e2 100644 --- a/src/lib/prog_loaders.c +++ b/src/lib/prog_loaders.c @@ -110,6 +110,18 @@ static int load_relocatable_ramstage(struct prog *ramstage) return rmodule_stage_load(&rmod_ram); } +static int load_nonrelocatable_ramstage(struct prog *ramstage) +{ + if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) { + uintptr_t base = 0; + size_t size = cbfs_prog_stage_section(ramstage, &base); + if (size) + backup_ramstage_section(base, size); + } + + return cbfs_prog_stage_load(ramstage); +} + void run_ramstage(void) { struct prog ramstage = @@ -135,7 +147,7 @@ void run_ramstage(void) if (IS_ENABLED(CONFIG_RELOCATABLE_RAMSTAGE)) { if (load_relocatable_ramstage(&ramstage)) goto fail; - } else if (cbfs_prog_stage_load(&ramstage)) + } else if (load_nonrelocatable_ramstage(&ramstage)) goto fail; stage_cache_add(STAGE_RAMSTAGE, &ramstage);

1 0

Patch set updated for coreboot: AGESA: Delay ACPI S3 backup until ramstage loader
by Kyösti Mälkki 08 Nov '16

08 Nov '16

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15470 -gerrit commit 4d4c058639cd6935a3e1fe0d6fdad6906c1ea128 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Mon Jun 27 12:14:39 2016 +0300 AGESA: Delay ACPI S3 backup until ramstage loader Change-Id: I59773161f22c1ec6a52050245f9ad3e6cc74a934 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/cpu/amd/agesa/s3_resume.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cpu/amd/agesa/s3_resume.c b/src/cpu/amd/agesa/s3_resume.c index e74c5b3..1e4aadb 100644 --- a/src/cpu/amd/agesa/s3_resume.c +++ b/src/cpu/amd/agesa/s3_resume.c @@ -91,9 +91,4 @@ void prepare_for_resume(void) printk(BIOS_DEBUG, "CAR disabled.\n"); set_resume_cache(); - /* - * Copy the system memory that is in the ramstage area to the - * reserved area. - */ - acpi_prepare_for_resume(); }

1 0

Patch set updated for coreboot: AMD binaryPI: Delay ACPI S3 backup until ramstage loader
by Kyösti Mälkki 08 Nov '16

08 Nov '16

Kyösti Mälkki (kyosti.malkki(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15471 -gerrit commit a5deee12a94268cc378f0d1d155cc234b05d0018 Author: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Date: Mon Jun 27 12:14:49 2016 +0300 AMD binaryPI: Delay ACPI S3 backup until ramstage loader Change-Id: I482cf93fe5dfab95817c87c32aad33df2e0a6439 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- src/cpu/amd/pi/s3_resume.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cpu/amd/pi/s3_resume.c b/src/cpu/amd/pi/s3_resume.c index 6426c97..338f921 100644 --- a/src/cpu/amd/pi/s3_resume.c +++ b/src/cpu/amd/pi/s3_resume.c @@ -289,10 +289,5 @@ void prepare_for_resume(void) printk(BIOS_DEBUG, "CAR disabled.\n"); set_resume_cache(); - /* - * Copy the system memory that is in the ramstage area to the - * reserved area. - */ - acpi_prepare_for_resume(); } #endif

1 0

New patch to review for coreboot: samsung/stumpy: enable power on after AC loss
by Matt DeVillier 07 Nov '16

07 Nov '16

Matt DeVillier (matt.devillier(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17267 -gerrit commit d5a4e854705e5a71de3df755acc1f9e4d9353411 Author: Matt DeVillier <matt.devillier(a)gmail.com> Date: Mon Nov 7 16:09:20 2016 -0600 samsung/stumpy: enable power on after AC loss use SIO EC settings from google/panther, which has same SIO chip Change-Id: I50332b80bd46dbaa9784e9c660f5d17b65152479 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- src/mainboard/samsung/stumpy/devicetree.cb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mainboard/samsung/stumpy/devicetree.cb b/src/mainboard/samsung/stumpy/devicetree.cb index adc15e6..34b8639 100644 --- a/src/mainboard/samsung/stumpy/devicetree.cb +++ b/src/mainboard/samsung/stumpy/devicetree.cb @@ -101,6 +101,10 @@ chip northbridge/intel/sandybridge device pnp 2e.4 on # Environment Controller io 0x60 = 0x700 io 0x62 = 0x710 + irq 0x70 = 0x09 + irq 0xf2 = 0x20 + irq 0xf4 = 0x0 + irq 0xfa = 0x12 end device pnp 2e.7 on # GPIO io 0x60 = 0x720

1 0

New patch to review for coreboot: samsung/stumpy: fix power LED operation
by Matt DeVillier 07 Nov '16

07 Nov '16

Matt DeVillier (matt.devillier(a)gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17266 -gerrit commit c2622efd98d55bc14b87a518610ca168248f8016 Author: Matt DeVillier <matt.devillier(a)gmail.com> Date: Mon Nov 7 16:43:03 2016 -0600 samsung/stumpy: fix power LED operation commit 80EF7B7 broke power LED operation when it incorrectly transferred values from the old function to the new. Restore the correct values. Change-Id: I99a38351bb52063fafa7436e6397a8da7fc1e952 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- src/mainboard/samsung/stumpy/romstage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mainboard/samsung/stumpy/romstage.c b/src/mainboard/samsung/stumpy/romstage.c index 1062b64..b2b21c4 100644 --- a/src/mainboard/samsung/stumpy/romstage.c +++ b/src/mainboard/samsung/stumpy/romstage.c @@ -144,8 +144,8 @@ static void setup_sio_gpios(void) /* * GPIO45 as LED_POWER# */ - it8772f_gpio_led(DUMMY_DEV, 4 /* set */, (0x1 << 5) /* select */, - 0x00 /* polarity: non-inverting */, 0x00 /* 0 = pulldown */, + it8772f_gpio_led(GPIO_DEV, 4 /* set */, (0x1 << 5) /* select */, + (0x1 << 5) /* polarity */, (0x1 << 5) /* 1 = pullup */, (0x1 << 5) /* output */, (0x1 << 5) /* 1 = Simple IO function */, SIO_GPIO_BLINK_GPIO45, IT8772F_GPIO_BLINK_FREQUENCY_1_HZ);

1 0

Patch set updated for coreboot: mrc: Add support for separate training cache in recovery mode
by Furquan Shaikh 07 Nov '16

07 Nov '16

Furquan Shaikh (furquan(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17242 -gerrit commit b974df35258ca192d2f0354e56b0b8ac81b4853f Author: Furquan Shaikh <furquan(a)chromium.org> Date: Sat Nov 5 23:57:02 2016 -0700 mrc: Add support for separate training cache in recovery mode 1. Re-factor MRC cache driver to properly select RW_MRC_CACHE or RECOVERY_MRC_CACHE based on the boot mode. - If normal mode boot, use RW_MRC_CACHE, if available. - If recovery mode boot: - Retrain memory if RECOVERY_MRC_CACHE not present, or recovery is requested explicity with retrain memory request. - Use RECOVERY_MRC_CACHE otherwise. 2. Protect RW and RECOVERY mrc caches in recovery and non-recovery boot modes. 3. Update training data in appropriate cache: - Use RW_MRC_CACHE if normal mode. - Use RECOVERY_MRC_CACHE if present in recovery mode. Else use RW_MRC_CACHE. 4. Add proper debug logs to indicate which training data cache is used at any point. BUG=chrome-os-partner:59352 BRANCH=None TEST=Verified that correct cache is used in both normal and recovery mode on reef. Change-Id: Ie79737a1450bd1ff71543e44a5a3e16950e70fb3 Signed-off-by: Furquan Shaikh <furquan(a)chromium.org> --- src/drivers/intel/fsp2_0/memory_init.c | 40 +++++---- src/include/elog.h | 1 + src/soc/intel/common/Kconfig | 4 + src/soc/intel/common/mrc_cache.c | 157 ++++++++++++++++++++++----------- src/soc/intel/common/mrc_cache.h | 6 ++ 5 files changed, 137 insertions(+), 71 deletions(-) diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index d0a22ce..3ea1897 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -55,13 +55,6 @@ static void save_memory_training_data(bool s3wake, uint32_t fsp_version) printk(BIOS_ERR, "Failed to stash MRC data\n"); } -/* - * On every trip to recovery, newly generated MRC data is stored with this - * version since it is not expected to be a legit version. This ensures that on - * next normal boot, memory re-training occurs and new MRC data is stored. - */ -#define MRC_DEAD_VERSION (0xdeaddead) - static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) { struct range_entry fsp_mem; @@ -89,10 +82,6 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) (uintptr_t)cbmem_find(CBMEM_ID_FSP_RESERVED_MEMORY)) die("Failed to accommodate FSP reserved memory request!\n"); - /* Now that CBMEM is up, save the list so ramstage can use it */ - if (vboot_recovery_mode_enabled()) - fsp_version = MRC_DEAD_VERSION; - save_memory_training_data(s3wake, fsp_version); /* Create romstage handof information */ @@ -103,26 +92,39 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) printk(BIOS_SPEW, "Romstage handoff structure not added!\n"); } +static const char *mrc_cache_get_region_name(void) +{ + /* In normal mode, always use DEFAULT_MRC_CACHE */ + if (!vboot_recovery_mode_enabled()) + return DEFAULT_MRC_CACHE; + + /* + * In recovery mode, force retraining by returning NULL if: + * 1. Recovery cache is not supported, or + * 2. Memory retrain switch is set. + */ + if (!IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE) || + vboot_recovery_mode_memory_retrain()) + return NULL; + + return RECOVERY_MRC_CACHE; +} + static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, bool s3wake, uint32_t fsp_version) { const struct mrc_saved_data *mrc_cache; + const char *name; arch_upd->NvsBufferPtr = NULL; if (!IS_ENABLED(CONFIG_CACHE_MRC_SETTINGS)) return; - /* Don't use saved training data when recovery mode is enabled. */ - if (vboot_recovery_mode_enabled()) { - printk(BIOS_SPEW, "Recovery mode. Not using MRC cache.\n"); - return; - } + name = mrc_cache_get_region_name(); - if (mrc_cache_get_current_with_version(&mrc_cache, fsp_version)) { - printk(BIOS_SPEW, "MRC cache was not found\n"); + if (mrc_cache_get_current_from_region(&mrc_cache, fsp_version, name)) return; - } /* MRC cache found */ arch_upd->NvsBufferPtr = (void *)mrc_cache->data; diff --git a/src/include/elog.h b/src/include/elog.h index 7fdcda7..fec9d09 100644 --- a/src/include/elog.h +++ b/src/include/elog.h @@ -152,6 +152,7 @@ struct elog_event_data_me_extended { /* Memory Cache Update */ #define ELOG_TYPE_MEM_CACHE_UPDATE 0xaa #define ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL 0 +#define ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY 1 #define ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS 0 #define ELOG_MEM_CACHE_UPDATE_STATUS_FAIL 1 struct elog_event_mem_cache_update { diff --git a/src/soc/intel/common/Kconfig b/src/soc/intel/common/Kconfig index affec55..5d8bf66 100644 --- a/src/soc/intel/common/Kconfig +++ b/src/soc/intel/common/Kconfig @@ -27,6 +27,10 @@ config MRC_SETTINGS_PROTECT bool "Enable protection on MRC settings" default n +config HAS_RECOVERY_MRC_CACHE + bool + default n + endif # CACHE_MRC_SETTINGS config DISPLAY_MTRRS diff --git a/src/soc/intel/common/mrc_cache.c b/src/soc/intel/common/mrc_cache.c index bef3ba2..63c68a0 100644 --- a/src/soc/intel/common/mrc_cache.c +++ b/src/soc/intel/common/mrc_cache.c @@ -36,12 +36,16 @@ struct mrc_data_region { }; /* common code */ -static int mrc_cache_get_region(struct mrc_data_region *region) +static int mrc_cache_get_region(const char *name, + struct mrc_data_region *region) { bool located_by_fmap = true; struct region_device rdev; - if (fmap_locate_area_as_rdev("RW_MRC_CACHE", &rdev)) + region->base = NULL; + region->size = 0; + + if (fmap_locate_area_as_rdev(name, &rdev)) located_by_fmap = false; /* CHROMEOS builds must get their MRC cache from FMAP. */ @@ -62,6 +66,38 @@ static int mrc_cache_get_region(struct mrc_data_region *region) return 0; } +/* Protect mrc region with a Protected Range Register */ +static int __protect_mrc_cache(const struct mrc_data_region *region, + const char *name) +{ + if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT)) + return 0; + + if (nvm_is_write_protected() <= 0) { + printk(BIOS_INFO, "MRC: NOT enabling PRR for %s.\n", name); + return 1; + } + + if (nvm_protect(region->base, region->size) < 0) { + printk(BIOS_ERR, "MRC: ERROR setting PRR for %s.\n", name); + return -1; + } + + printk(BIOS_INFO, "MRC: Enabled Protected Range on %s.\n", name); + return 0; +} + +static int protect_mrc_cache(const char *name) +{ + struct mrc_data_region region; + if (mrc_cache_get_region(name, &region) < 0) { + printk(BIOS_ERR, "MRC: Could not find region %s\n", name); + return -1; + } + + return __protect_mrc_cache(&region, name); +} + static int mrc_cache_in_region(const struct mrc_data_region *region, const struct mrc_saved_data *cache) { @@ -149,31 +185,56 @@ static int __mrc_cache_get_current(const struct mrc_data_region *region, return -1; if (verified_cache->version != version) { - printk(BIOS_DEBUG, "MRC cache version mismatch: %x vs %x\n", + printk(BIOS_DEBUG, "MRC: cache version mismatch: %x vs %x\n", verified_cache->version, version); return -1; } - printk(BIOS_DEBUG, "MRC cache slot %d @ %p\n", slot-1, verified_cache); + printk(BIOS_DEBUG, "MRC: cache slot %d @ %p\n", slot-1, verified_cache); return 0; } -int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache, - uint32_t version) +int mrc_cache_get_current_from_region(const struct mrc_saved_data **cache, + uint32_t version, + const char *region_name) { struct mrc_data_region region; - if (mrc_cache_get_region(&region) < 0) + if (!region_name) { + printk(BIOS_ERR, "MRC: Requires memory retraining.\n"); return -1; + } + + printk(BIOS_ERR, "MRC: Using data from %s\n", region_name); + + if (mrc_cache_get_region(region_name, &region) < 0) { + printk(BIOS_ERR, "MRC: Region %s not found. " + "Requires memory retraining.\n", region_name); + return -1; + } - return __mrc_cache_get_current(&region, cache, version); + if (__mrc_cache_get_current(&region, cache, version) < 0) { + printk(BIOS_ERR, "MRC: Valid slot not found in %s." + "Requires memory retraining.\n", region_name); + return -1; + } + + return 0; +} + +int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache, + uint32_t version) +{ + return mrc_cache_get_current_from_region(cache, version, + DEFAULT_MRC_CACHE); } int mrc_cache_get_current(const struct mrc_saved_data **cache) { return mrc_cache_get_current_with_version(cache, 0); } + /* Fill in mrc_saved_data structure with payload. */ static void mrc_cache_fill(struct mrc_saved_data *cache, const void *data, size_t size, uint32_t version) @@ -197,14 +258,14 @@ int mrc_cache_stash_data_with_version(const void *data, size_t size, cache = cbmem_add(CBMEM_ID_MRCDATA, cbmem_size); if (cache == NULL) { - printk(BIOS_ERR, "No space in cbmem for MRC data.\n"); + printk(BIOS_ERR, "MRC: No space in cbmem for training data.\n"); return -1; } /* Clear alignment padding bytes at end of data. */ memset(&cache->data[size], 0, cbmem_size - size - sizeof(*cache)); - printk(BIOS_DEBUG, "Relocate MRC DATA from %p to %p (%zu bytes)\n", + printk(BIOS_DEBUG, "MRC: Relocate data from %p to %p (%zu bytes)\n", data, cache, size); mrc_cache_fill(cache, data, size, version); @@ -263,25 +324,6 @@ mrc_cache_next_slot(const struct mrc_data_region *region, return next_slot; } -/* Protect RW_MRC_CACHE region with a Protected Range Register */ -static int protect_mrc_cache(const struct mrc_data_region *region) -{ -#if IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT) - if (nvm_is_write_protected() <= 0) { - printk(BIOS_INFO, "NOT enabling PRR for RW_MRC_CACHE region\n"); - return 1; - } - - if (nvm_protect(region->base, region->size) < 0) { - printk(BIOS_ERR, "ERROR setting PRR for RW_MRC_CACHE region\n"); - return -1; - } - - printk(BIOS_INFO, "Enabled Protected Range on RW_MRC_CACHE region\n"); -#endif - return 0; -} - static void log_event_cache_update(uint8_t slot, uint8_t status) { const int type = ELOG_TYPE_MEM_CACHE_UPDATE; @@ -300,23 +342,33 @@ static void update_mrc_cache(void *unused) const struct mrc_saved_data *current_saved; const struct mrc_saved_data *next_slot; struct mrc_data_region region; + const char *region_name = DEFAULT_MRC_CACHE; + uint8_t slot = ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL; - printk(BIOS_DEBUG, "Updating MRC cache data.\n"); + printk(BIOS_DEBUG, "MRC: Updating cache data.\n"); - current_boot = cbmem_find(CBMEM_ID_MRCDATA); - if (!current_boot) { - printk(BIOS_ERR, "No MRC cache in cbmem.\n"); - return; + if (vboot_recovery_mode_enabled() && + IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE)) { + region_name = RECOVERY_MRC_CACHE; + slot = ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY; } - if (mrc_cache_get_region(&region)) { - printk(BIOS_ERR, "Could not obtain MRC cache region.\n"); - return; + printk(BIOS_ERR, "MRC: Cache region selected - %s\n", region_name); + + if (mrc_cache_get_region(region_name, &region)) { + printk(BIOS_ERR, "MRC: Could not obtain cache region.\n"); + goto out; + } + + current_boot = cbmem_find(CBMEM_ID_MRCDATA); + if (!current_boot) { + printk(BIOS_ERR, "MRC: No cache in cbmem.\n"); + goto out; } if (!mrc_cache_valid(&region, current_boot)) { - printk(BIOS_ERR, "MRC cache data in cbmem invalid.\n"); - return; + printk(BIOS_ERR, "MRC: Cache data in cbmem invalid.\n"); + goto out; } current_saved = NULL; @@ -326,20 +378,20 @@ static void update_mrc_cache(void *unused) if (current_saved->size == current_boot->size && !memcmp(&current_saved->data[0], &current_boot->data[0], current_saved->size)) { - printk(BIOS_DEBUG, "MRC cache up to date.\n"); - protect_mrc_cache(&region); - return; + printk(BIOS_DEBUG, "MRC: Cache up to date.\n"); + goto out; } } next_slot = mrc_cache_next_slot(&region, current_saved); if (!mrc_slot_valid(&region, next_slot, current_boot)) { - printk(BIOS_DEBUG, "Slot @ %p is invalid.\n", next_slot); + printk(BIOS_DEBUG, "MRC: Slot @ %p is invalid.\n", next_slot); if (!nvm_is_erased(region.base, region.size)) { if (nvm_erase(region.base, region.size) < 0) { - printk(BIOS_DEBUG, "Failure erasing region.\n"); - return; + printk(BIOS_DEBUG, "MRC: Failure erasing " + "region %s.\n", region_name); + goto out; } } next_slot = region.base; @@ -347,16 +399,17 @@ static void update_mrc_cache(void *unused) if (nvm_write((void *)next_slot, current_boot, current_boot->size + sizeof(*current_boot))) { - printk(BIOS_DEBUG, "Failure writing MRC cache to %p.\n", - next_slot); - log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL, - ELOG_MEM_CACHE_UPDATE_STATUS_FAIL); + printk(BIOS_DEBUG, "MRC: Failure writing MRC cache to %s:%p\n", + region_name, next_slot); + log_event_cache_update(slot, ELOG_MEM_CACHE_UPDATE_STATUS_FAIL); } else { - log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL, - ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS); + log_event_cache_update(slot, + ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS); } - protect_mrc_cache(&region); +out: + protect_mrc_cache(RECOVERY_MRC_CACHE); + protect_mrc_cache(DEFAULT_MRC_CACHE); } BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY, update_mrc_cache, NULL); diff --git a/src/soc/intel/common/mrc_cache.h b/src/soc/intel/common/mrc_cache.h index 9b14c1e..8bbd4ed 100644 --- a/src/soc/intel/common/mrc_cache.h +++ b/src/soc/intel/common/mrc_cache.h @@ -19,6 +19,9 @@ #include <stddef.h> #include <stdint.h> +#define DEFAULT_MRC_CACHE "RW_MRC_CACHE" +#define RECOVERY_MRC_CACHE "RECOVERY_MRC_CACHE" + /* Wrapper object to save MRC information. */ struct mrc_saved_data { uint32_t signature; @@ -32,6 +35,9 @@ struct mrc_saved_data { int mrc_cache_get_current(const struct mrc_saved_data **cache); int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache, uint32_t version); +int mrc_cache_get_current_from_region(const struct mrc_saved_data **cache, + uint32_t version, + const char *region_name); /* Stash the resulting MRC data to be saved in non-volatile storage later. */ int mrc_cache_stash_data(const void *data, size_t size);

1 0

Patch set updated for coreboot: mainboard/google/reef: Add support for RECOVERY_MRC_CACHE
by Furquan Shaikh 07 Nov '16

07 Nov '16

Furquan Shaikh (furquan(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17243 -gerrit commit e19a02c396b7dadd755300c3a24421b8c5b0e867 Author: Furquan Shaikh <furquan(a)chromium.org> Date: Sun Nov 6 00:07:08 2016 -0700 mainboard/google/reef: Add support for RECOVERY_MRC_CACHE 1. Add RECOVERY_MRC_CACHE region to reef FMAP. 2. Implement helper function for getting event for recovery mode with memory retraining. 3. Select HAS_RECOVERY_MRC_CACHE. BUG=chrome-os-partner:59352 BRANCH=None TEST=Verified recovery mode behavior with and without memory training request on reef. Change-Id: I91abc9f8122f1aa3980c6372ab557e56a7a92730 Signed-off-by: Furquan Shaikh <furquan(a)chromium.org> --- src/mainboard/google/reef/Kconfig | 1 + src/mainboard/google/reef/chromeos.c | 15 +++++++++++++-- src/mainboard/google/reef/chromeos.fmd | 3 ++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/mainboard/google/reef/Kconfig b/src/mainboard/google/reef/Kconfig index 6045f47..bb7a531 100644 --- a/src/mainboard/google/reef/Kconfig +++ b/src/mainboard/google/reef/Kconfig @@ -33,6 +33,7 @@ config DRIVER_TPM_I2C_IRQ config CHROMEOS select LID_SWITCH if BASEBOARD_REEF_LAPTOP + select HAS_RECOVERY_MRC_CACHE config DRIVERS_I2C_DA7219 default y diff --git a/src/mainboard/google/reef/chromeos.c b/src/mainboard/google/reef/chromeos.c index 58c2cd8..506f9de 100644 --- a/src/mainboard/google/reef/chromeos.c +++ b/src/mainboard/google/reef/chromeos.c @@ -55,11 +55,22 @@ int get_recovery_mode_switch(void) EC_HOST_EVENT_MASK(EC_HOST_EVENT_KEYBOARD_RECOVERY)); } +int get_recovery_mode_retrain_switch(void) +{ + /* + * Check if the EC has posted the keyboard recovery event with memory + * retrain. + */ + return !!(google_chromeec_get_events_b() & + EC_HOST_EVENT_MASK(EC_HOST_EVENT_KEYBOARD_RECOVERY_HW_REINIT)); +} + int clear_recovery_mode_switch(void) { - /* Clear keyboard recovery event. */ + /* Clear all host event bits requesting recovery mode. */ return google_chromeec_clear_events_b( - EC_HOST_EVENT_MASK(EC_HOST_EVENT_KEYBOARD_RECOVERY)); + EC_HOST_EVENT_MASK(EC_HOST_EVENT_KEYBOARD_RECOVERY) | + EC_HOST_EVENT_MASK(EC_HOST_EVENT_KEYBOARD_RECOVERY_HW_REINIT)); } int get_write_protect_state(void) diff --git a/src/mainboard/google/reef/chromeos.fmd b/src/mainboard/google/reef/chromeos.fmd index d25a9b2..c6a413d 100644 --- a/src/mainboard/google/reef/chromeos.fmd +++ b/src/mainboard/google/reef/chromeos.fmd @@ -13,7 +13,8 @@ FLASH 16M { } } MISC_RW@0x400000 0x4a000 { - RW_MRC_CACHE@0x0 0x40000 + RECOVERY_MRC_CACHE@0x0 0x10000 + RW_MRC_CACHE@0x10000 0x30000 RW_ELOG@0x40000 0x4000 RW_SHARED@0x44000 0x4000 { SHARED_DATA@0x0 0x2000

1 0

Patch set updated for coreboot: vboot: Add support for recovery mode with forced memory retraining
by Furquan Shaikh 07 Nov '16

07 Nov '16

Furquan Shaikh (furquan(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17241 -gerrit commit 74f28d88bd3b626d68d688a159aaf49fcbcf0b3a Author: Furquan Shaikh <furquan(a)chromium.org> Date: Sat Nov 5 23:52:08 2016 -0700 vboot: Add support for recovery mode with forced memory retraining 1. Add new function vboot_recovery_mode_memory_retrain that indicates if recovery mode requires memory retraining to be performed. 2. Add helper function get_recovery_mode_retrain_switch to read memory retrain switch. This is provided as weak function which should be implemented by mainboard just like {get,clear}_recovery_mode_switch. BUG=chrome-os-partner:59352 BRANCH=None TEST=Verified behavior of recovery mode with forced memory retraining on reef Change-Id: I46c10fbf25bc100d9f562c36da3ac646c9dae7d1 Signed-off-by: Furquan Shaikh <furquan(a)chromium.org> --- src/include/bootmode.h | 1 + src/vboot/bootmode.c | 10 ++++++++++ src/vboot/vboot_common.h | 1 + 3 files changed, 12 insertions(+) diff --git a/src/include/bootmode.h b/src/include/bootmode.h index 21aa386..c807472 100644 --- a/src/include/bootmode.h +++ b/src/include/bootmode.h @@ -22,6 +22,7 @@ int get_write_protect_state(void); int get_sw_write_protect_state(void); int get_developer_mode_switch(void); int get_recovery_mode_switch(void); +int get_recovery_mode_retrain_switch(void); int clear_recovery_mode_switch(void); int get_wipeout_mode_switch(void); int get_lid_switch(void); diff --git a/src/vboot/bootmode.c b/src/vboot/bootmode.c index 12a4dc0..5bb7040 100644 --- a/src/vboot/bootmode.c +++ b/src/vboot/bootmode.c @@ -162,6 +162,16 @@ int vboot_recovery_mode_enabled(void) return !!vboot_check_recovery_request(); } +int __attribute__((weak)) get_recovery_mode_retrain_switch(void) +{ + return 0; +} + +int vboot_recovery_mode_memory_retrain(void) +{ + return get_recovery_mode_retrain_switch(); +} + int vboot_developer_mode_enabled(void) { if (!IS_ENABLED(CONFIG_VBOOT)) diff --git a/src/vboot/vboot_common.h b/src/vboot/vboot_common.h index 59fd44c..956b54c 100644 --- a/src/vboot/vboot_common.h +++ b/src/vboot/vboot_common.h @@ -107,5 +107,6 @@ void verstage_mainboard_init(void); /* Check boot modes */ int vboot_developer_mode_enabled(void); int vboot_recovery_mode_enabled(void); +int vboot_recovery_mode_memory_retrain(void); #endif /* __VBOOT_VBOOT_COMMON_H__ */

1 0

Patch set updated for coreboot: mrc: Add support for separate training cache in recovery mode
by Furquan Shaikh 07 Nov '16

07 Nov '16

Furquan Shaikh (furquan(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17242 -gerrit commit 159f75d35f5d0bfe9284ba4eaabd65868fbeeb66 Author: Furquan Shaikh <furquan(a)chromium.org> Date: Sat Nov 5 23:57:02 2016 -0700 mrc: Add support for separate training cache in recovery mode 1. Re-factor MRC cache driver to properly select RW_MRC_CACHE or RECOVERY_MRC_CACHE based on the boot mode. - If normal mode boot, use RW_MRC_CACHE, if available. - If recovery mode boot: - Retrain memory if RECOVERY_MRC_CACHE not present, or recovery is requested explicity with retrain memory request. - Use RECOVERY_MRC_CACHE otherwise. 2. Protect RW and RECOVERY mrc caches in recovery and non-recovery boot modes. 3. Update training data in appropriate cache: - Use RW_MRC_CACHE if normal mode. - Use RECOVERY_MRC_CACHE if present in recovery mode. Else use RW_MRC_CACHE. 4. Add proper debug logs to indicate which training data cache is used at any point. BUG=chrome-os-partner:59352 BRANCH=None TEST=Verified that correct cache is used in both normal and recovery mode on reef. Change-Id: Ie79737a1450bd1ff71543e44a5a3e16950e70fb3 Signed-off-by: Furquan Shaikh <furquan(a)chromium.org> --- src/drivers/intel/fsp2_0/memory_init.c | 40 +++++---- src/include/elog.h | 1 + src/soc/intel/common/Kconfig | 4 + src/soc/intel/common/mrc_cache.c | 155 ++++++++++++++++++++++----------- src/soc/intel/common/mrc_cache.h | 6 ++ 5 files changed, 136 insertions(+), 70 deletions(-) diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index d0a22ce..3ea1897 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -55,13 +55,6 @@ static void save_memory_training_data(bool s3wake, uint32_t fsp_version) printk(BIOS_ERR, "Failed to stash MRC data\n"); } -/* - * On every trip to recovery, newly generated MRC data is stored with this - * version since it is not expected to be a legit version. This ensures that on - * next normal boot, memory re-training occurs and new MRC data is stored. - */ -#define MRC_DEAD_VERSION (0xdeaddead) - static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) { struct range_entry fsp_mem; @@ -89,10 +82,6 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) (uintptr_t)cbmem_find(CBMEM_ID_FSP_RESERVED_MEMORY)) die("Failed to accommodate FSP reserved memory request!\n"); - /* Now that CBMEM is up, save the list so ramstage can use it */ - if (vboot_recovery_mode_enabled()) - fsp_version = MRC_DEAD_VERSION; - save_memory_training_data(s3wake, fsp_version); /* Create romstage handof information */ @@ -103,26 +92,39 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version) printk(BIOS_SPEW, "Romstage handoff structure not added!\n"); } +static const char *mrc_cache_get_region_name(void) +{ + /* In normal mode, always use DEFAULT_MRC_CACHE */ + if (!vboot_recovery_mode_enabled()) + return DEFAULT_MRC_CACHE; + + /* + * In recovery mode, force retraining by returning NULL if: + * 1. Recovery cache is not supported, or + * 2. Memory retrain switch is set. + */ + if (!IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE) || + vboot_recovery_mode_memory_retrain()) + return NULL; + + return RECOVERY_MRC_CACHE; +} + static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, bool s3wake, uint32_t fsp_version) { const struct mrc_saved_data *mrc_cache; + const char *name; arch_upd->NvsBufferPtr = NULL; if (!IS_ENABLED(CONFIG_CACHE_MRC_SETTINGS)) return; - /* Don't use saved training data when recovery mode is enabled. */ - if (vboot_recovery_mode_enabled()) { - printk(BIOS_SPEW, "Recovery mode. Not using MRC cache.\n"); - return; - } + name = mrc_cache_get_region_name(); - if (mrc_cache_get_current_with_version(&mrc_cache, fsp_version)) { - printk(BIOS_SPEW, "MRC cache was not found\n"); + if (mrc_cache_get_current_from_region(&mrc_cache, fsp_version, name)) return; - } /* MRC cache found */ arch_upd->NvsBufferPtr = (void *)mrc_cache->data; diff --git a/src/include/elog.h b/src/include/elog.h index 7fdcda7..fec9d09 100644 --- a/src/include/elog.h +++ b/src/include/elog.h @@ -152,6 +152,7 @@ struct elog_event_data_me_extended { /* Memory Cache Update */ #define ELOG_TYPE_MEM_CACHE_UPDATE 0xaa #define ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL 0 +#define ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY 1 #define ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS 0 #define ELOG_MEM_CACHE_UPDATE_STATUS_FAIL 1 struct elog_event_mem_cache_update { diff --git a/src/soc/intel/common/Kconfig b/src/soc/intel/common/Kconfig index affec55..5d8bf66 100644 --- a/src/soc/intel/common/Kconfig +++ b/src/soc/intel/common/Kconfig @@ -27,6 +27,10 @@ config MRC_SETTINGS_PROTECT bool "Enable protection on MRC settings" default n +config HAS_RECOVERY_MRC_CACHE + bool + default n + endif # CACHE_MRC_SETTINGS config DISPLAY_MTRRS diff --git a/src/soc/intel/common/mrc_cache.c b/src/soc/intel/common/mrc_cache.c index bef3ba2..a64b62b 100644 --- a/src/soc/intel/common/mrc_cache.c +++ b/src/soc/intel/common/mrc_cache.c @@ -36,12 +36,16 @@ struct mrc_data_region { }; /* common code */ -static int mrc_cache_get_region(struct mrc_data_region *region) +static int mrc_cache_get_region(const char *name, + struct mrc_data_region *region) { bool located_by_fmap = true; struct region_device rdev; - if (fmap_locate_area_as_rdev("RW_MRC_CACHE", &rdev)) + region->base = NULL; + region->size = 0; + + if (fmap_locate_area_as_rdev(name, &rdev)) located_by_fmap = false; /* CHROMEOS builds must get their MRC cache from FMAP. */ @@ -62,6 +66,38 @@ static int mrc_cache_get_region(struct mrc_data_region *region) return 0; } +/* Protect mrc region with a Protected Range Register */ +static int __protect_mrc_cache(const struct mrc_data_region *region, + const char *name) +{ + if (!IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT)) + return 0; + + if (nvm_is_write_protected() <= 0) { + printk(BIOS_INFO, "MRC: NOT enabling PRR for %s.\n", name); + return 1; + } + + if (nvm_protect(region->base, region->size) < 0) { + printk(BIOS_ERR, "MRC: ERROR setting PRR for %s.\n", name); + return -1; + } + + printk(BIOS_INFO, "MRC: Enabled Protected Range on %s.\n", name); + return 0; +} + +static int protect_mrc_cache(const char *name) +{ + struct mrc_data_region region; + if (mrc_cache_get_region(name, &region) < 0) { + printk(BIOS_ERR, "MRC: Could not find region %s\n", name); + return -1; + } + + return __protect_mrc_cache(&region, name); +} + static int mrc_cache_in_region(const struct mrc_data_region *region, const struct mrc_saved_data *cache) { @@ -149,31 +185,56 @@ static int __mrc_cache_get_current(const struct mrc_data_region *region, return -1; if (verified_cache->version != version) { - printk(BIOS_DEBUG, "MRC cache version mismatch: %x vs %x\n", + printk(BIOS_DEBUG, "MRC: cache version mismatch: %x vs %x\n", verified_cache->version, version); return -1; } - printk(BIOS_DEBUG, "MRC cache slot %d @ %p\n", slot-1, verified_cache); + printk(BIOS_DEBUG, "MRC: cache slot %d @ %p\n", slot-1, verified_cache); return 0; } -int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache, - uint32_t version) +int mrc_cache_get_current_from_region(const struct mrc_saved_data **cache, + uint32_t version, + const char *region_name) { struct mrc_data_region region; - if (mrc_cache_get_region(&region) < 0) + if (!region_name) { + printk(BIOS_ERR, "MRC: Requires memory retraining.\n"); return -1; + } - return __mrc_cache_get_current(&region, cache, version); + printk(BIOS_ERR, "MRC: Using data from %s\n", region_name); + + if (mrc_cache_get_region(region_name, &region) < 0) { + printk(BIOS_ERR, "MRC: Region %s not found. " + "Requires memory retraining.\n", region_name); + return -1; + } + + if (__mrc_cache_get_current(&region, cache, version) < 0) { + printk(BIOS_ERR, "MRC: Valid slot not found in %s." + "Requires memory retraining.\n", region_name); + return -1; + } + + return 0; +} + +int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache, + uint32_t version) +{ + return mrc_cache_get_current_from_region(cache, version, + DEFAULT_MRC_CACHE); } int mrc_cache_get_current(const struct mrc_saved_data **cache) { return mrc_cache_get_current_with_version(cache, 0); } + /* Fill in mrc_saved_data structure with payload. */ static void mrc_cache_fill(struct mrc_saved_data *cache, const void *data, size_t size, uint32_t version) @@ -197,14 +258,14 @@ int mrc_cache_stash_data_with_version(const void *data, size_t size, cache = cbmem_add(CBMEM_ID_MRCDATA, cbmem_size); if (cache == NULL) { - printk(BIOS_ERR, "No space in cbmem for MRC data.\n"); + printk(BIOS_ERR, "MRC: No space in cbmem for training data.\n"); return -1; } /* Clear alignment padding bytes at end of data. */ memset(&cache->data[size], 0, cbmem_size - size - sizeof(*cache)); - printk(BIOS_DEBUG, "Relocate MRC DATA from %p to %p (%zu bytes)\n", + printk(BIOS_DEBUG, "MRC: Relocate data from %p to %p (%zu bytes)\n", data, cache, size); mrc_cache_fill(cache, data, size, version); @@ -263,25 +324,6 @@ mrc_cache_next_slot(const struct mrc_data_region *region, return next_slot; } -/* Protect RW_MRC_CACHE region with a Protected Range Register */ -static int protect_mrc_cache(const struct mrc_data_region *region) -{ -#if IS_ENABLED(CONFIG_MRC_SETTINGS_PROTECT) - if (nvm_is_write_protected() <= 0) { - printk(BIOS_INFO, "NOT enabling PRR for RW_MRC_CACHE region\n"); - return 1; - } - - if (nvm_protect(region->base, region->size) < 0) { - printk(BIOS_ERR, "ERROR setting PRR for RW_MRC_CACHE region\n"); - return -1; - } - - printk(BIOS_INFO, "Enabled Protected Range on RW_MRC_CACHE region\n"); -#endif - return 0; -} - static void log_event_cache_update(uint8_t slot, uint8_t status) { const int type = ELOG_TYPE_MEM_CACHE_UPDATE; @@ -300,23 +342,33 @@ static void update_mrc_cache(void *unused) const struct mrc_saved_data *current_saved; const struct mrc_saved_data *next_slot; struct mrc_data_region region; + const char *region_name = DEFAULT_MRC_CACHE; + uint8_t slot = ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL; - printk(BIOS_DEBUG, "Updating MRC cache data.\n"); + printk(BIOS_DEBUG, "MRC: Updating cache data.\n"); - current_boot = cbmem_find(CBMEM_ID_MRCDATA); - if (!current_boot) { - printk(BIOS_ERR, "No MRC cache in cbmem.\n"); - return; + if (vboot_recovery_mode_enabled() && + IS_ENABLED(CONFIG_HAS_RECOVERY_MRC_CACHE)) { + region_name = RECOVERY_MRC_CACHE; + slot = ELOG_MEM_CACHE_UPDATE_SLOT_RECOVERY; } - if (mrc_cache_get_region(&region)) { - printk(BIOS_ERR, "Could not obtain MRC cache region.\n"); + printk(BIOS_ERR, "MRC: Cache region selected - %s\n", region_name); + + if (mrc_cache_get_region(region_name, &region)) { + printk(BIOS_ERR, "MRC: Could not obtain cache region.\n"); return; } + current_boot = cbmem_find(CBMEM_ID_MRCDATA); + if (!current_boot) { + printk(BIOS_ERR, "MRC: No cache in cbmem.\n"); + goto out; + } + if (!mrc_cache_valid(&region, current_boot)) { - printk(BIOS_ERR, "MRC cache data in cbmem invalid.\n"); - return; + printk(BIOS_ERR, "MRC: Cache data in cbmem invalid.\n"); + goto out; } current_saved = NULL; @@ -326,20 +378,20 @@ static void update_mrc_cache(void *unused) if (current_saved->size == current_boot->size && !memcmp(&current_saved->data[0], &current_boot->data[0], current_saved->size)) { - printk(BIOS_DEBUG, "MRC cache up to date.\n"); - protect_mrc_cache(&region); - return; + printk(BIOS_DEBUG, "MRC: Cache up to date.\n"); + goto out; } } next_slot = mrc_cache_next_slot(&region, current_saved); if (!mrc_slot_valid(&region, next_slot, current_boot)) { - printk(BIOS_DEBUG, "Slot @ %p is invalid.\n", next_slot); + printk(BIOS_DEBUG, "MRC: Slot @ %p is invalid.\n", next_slot); if (!nvm_is_erased(region.base, region.size)) { if (nvm_erase(region.base, region.size) < 0) { - printk(BIOS_DEBUG, "Failure erasing region.\n"); - return; + printk(BIOS_DEBUG, "MRC: Failure erasing " + "region %s.\n", region_name); + goto out; } } next_slot = region.base; @@ -347,16 +399,17 @@ static void update_mrc_cache(void *unused) if (nvm_write((void *)next_slot, current_boot, current_boot->size + sizeof(*current_boot))) { - printk(BIOS_DEBUG, "Failure writing MRC cache to %p.\n", - next_slot); - log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL, - ELOG_MEM_CACHE_UPDATE_STATUS_FAIL); + printk(BIOS_DEBUG, "MRC: Failure writing MRC cache to %s:%p\n", + region_name, next_slot); + log_event_cache_update(slot, ELOG_MEM_CACHE_UPDATE_STATUS_FAIL); } else { - log_event_cache_update(ELOG_MEM_CACHE_UPDATE_SLOT_NORMAL, - ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS); + log_event_cache_update(slot, + ELOG_MEM_CACHE_UPDATE_STATUS_SUCCESS); } - protect_mrc_cache(&region); +out: + protect_mrc_cache(RECOVERY_MRC_CACHE); + protect_mrc_cache(DEFAULT_MRC_CACHE); } BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY, update_mrc_cache, NULL); diff --git a/src/soc/intel/common/mrc_cache.h b/src/soc/intel/common/mrc_cache.h index 9b14c1e..8bbd4ed 100644 --- a/src/soc/intel/common/mrc_cache.h +++ b/src/soc/intel/common/mrc_cache.h @@ -19,6 +19,9 @@ #include <stddef.h> #include <stdint.h> +#define DEFAULT_MRC_CACHE "RW_MRC_CACHE" +#define RECOVERY_MRC_CACHE "RECOVERY_MRC_CACHE" + /* Wrapper object to save MRC information. */ struct mrc_saved_data { uint32_t signature; @@ -32,6 +35,9 @@ struct mrc_saved_data { int mrc_cache_get_current(const struct mrc_saved_data **cache); int mrc_cache_get_current_with_version(const struct mrc_saved_data **cache, uint32_t version); +int mrc_cache_get_current_from_region(const struct mrc_saved_data **cache, + uint32_t version, + const char *region_name); /* Stash the resulting MRC data to be saved in non-volatile storage later. */ int mrc_cache_stash_data(const void *data, size_t size);

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2016