Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
vboot: remove extraneous vboot_recovery_mode_memory_retrain
Just call get_recovery_mode_retrain_switch() directly.
BUG=b:124141368 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Icb88d6862db1782e0218276984e527638b21fd3a Signed-off-by: Joel Kitching kitching@google.com --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/bootmode.c M src/security/vboot/vboot_common.h M src/soc/mediatek/mt8183/memory.c 5 files changed, 4 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/39343/1
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 455dfa5..004b9e4 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -117,10 +117,10 @@ * 1. Recovery cache is not supported, or * 2. Memory retrain switch is set. */ - if (vboot_recovery_mode_enabled()) { + if (get_recovery_mode_retrain_switch()) { if (!CONFIG(HAS_RECOVERY_MRC_CACHE)) return; - if (vboot_recovery_mode_memory_retrain()) + if (get_recovery_mode_retrain_switch()) return; }
diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index 2881a6f..565feb8 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -516,7 +516,7 @@ /* Invalidate only on recovery mode with retraining enabled. */ if (!vboot_recovery_mode_enabled()) return; - if (!vboot_recovery_mode_memory_retrain()) + if (!get_recovery_mode_retrain_switch()) return;
if (fmap_locate_area_as_rdev_rw(name, &rdev) < 0) { diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c index 55c1e79..c20117e 100644 --- a/src/security/vboot/bootmode.c +++ b/src/security/vboot/bootmode.c @@ -75,8 +75,3 @@ */ BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY, do_clear_recovery_mode_switch, NULL); - -int vboot_recovery_mode_memory_retrain(void) -{ - return get_recovery_mode_retrain_switch(); -} diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 57f3475..8be9d2a 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -67,14 +67,12 @@ #if CONFIG(VBOOT) int vboot_developer_mode_enabled(void); int vboot_recovery_mode_enabled(void); -int vboot_recovery_mode_memory_retrain(void); int vboot_can_enable_udc(void); void vboot_run_logic(void); int vboot_locate_cbfs(struct region_device *rdev); #else /* !CONFIG_VBOOT */ static inline int vboot_developer_mode_enabled(void) { return 0; } static inline int vboot_recovery_mode_enabled(void) { return 0; } -static inline int vboot_recovery_mode_memory_retrain(void) { return 0; } /* If VBOOT is not enabled, we are okay enabling USB device controller (UDC). */ static inline int vboot_can_enable_udc(void) { return 1; } static inline void vboot_run_logic(void) {} diff --git a/src/soc/mediatek/mt8183/memory.c b/src/soc/mediatek/mt8183/memory.c index 78890ea..9b496fd 100644 --- a/src/soc/mediatek/mt8183/memory.c +++ b/src/soc/mediatek/mt8183/memory.c @@ -174,7 +174,7 @@ /* Load calibration params from flash and run fast calibration */ if (recovery_mode) { printk(BIOS_WARNING, "Skip loading cached calibration data\n"); - if (vboot_recovery_mode_memory_retrain() || + if (get_recovery_mode_retrain_switch() || vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) { printk(BIOS_WARNING, "Retrain memory in next boot\n"); /* Use 0xFF as erased flash data. */
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/1/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/39343/1/src/drivers/intel/fsp2_0/me... PS1, Line 120: if (get_recovery_mode_retrain_switch()) { Hmmm? I think you don't want to change this one.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 1: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/1/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/39343/1/src/drivers/intel/fsp2_0/me... PS1, Line 120: if (get_recovery_mode_retrain_switch()) {
Hmmm? I think you don't want to change this one.
That's right.
Hello build bot (Jenkins), Furquan Shaikh, Angel Pons, Julius Werner, Andrey Petrov, Patrick Rudolph, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39343
to look at the new patch set (#2).
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
vboot: remove extraneous vboot_recovery_mode_memory_retrain
Just call get_recovery_mode_retrain_switch() directly.
BUG=b:124141368 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Icb88d6862db1782e0218276984e527638b21fd3a Signed-off-by: Joel Kitching kitching@google.com --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/bootmode.c M src/security/vboot/vboot_common.h M src/soc/mediatek/mt8183/memory.c 5 files changed, 3 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/39343/2
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/1/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/39343/1/src/drivers/intel/fsp2_0/me... PS1, Line 120: if (get_recovery_mode_retrain_switch()) {
That's right.
Thanks for catching that! Fixed.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 2: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Angel Pons, Julius Werner, Andrey Petrov, Patrick Rudolph, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39343
to look at the new patch set (#3).
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
vboot: remove extraneous vboot_recovery_mode_memory_retrain
Just call get_recovery_mode_retrain_switch() directly.
BUG=b:124141368 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Icb88d6862db1782e0218276984e527638b21fd3a Signed-off-by: Joel Kitching kitching@google.com --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/bootmode.c M src/security/vboot/vboot_common.h M src/soc/mediatek/mt8183/memory.c 5 files changed, 3 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/39343/3
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 3:
Moved this to the bottom of the stack since it seems the least controversial.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/3/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/39343/3/src/drivers/intel/fsp2_0/me... PS3, Line 123: get_recovery_mode_retrain_switch Jenkins complains on this one
Hello build bot (Jenkins), Furquan Shaikh, Angel Pons, Julius Werner, Andrey Petrov, Patrick Rudolph, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39343
to look at the new patch set (#4).
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
vboot: remove extraneous vboot_recovery_mode_memory_retrain
Just call get_recovery_mode_retrain_switch() directly.
BUG=b:124141368 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Icb88d6862db1782e0218276984e527638b21fd3a Signed-off-by: Joel Kitching kitching@google.com --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/bootmode.c M src/security/vboot/vboot_common.h M src/soc/mediatek/mt8183/memory.c 5 files changed, 4 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/39343/4
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/3/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/39343/3/src/drivers/intel/fsp2_0/me... PS3, Line 123: get_recovery_mode_retrain_switch
Jenkins complains on this one
Thanks Angel. Added the necessary header.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/3/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/39343/3/src/drivers/intel/fsp2_0/me... PS3, Line 123: get_recovery_mode_retrain_switch
Thanks Angel. Added the necessary header.
Done, let's hope it builds now 😄
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
Woups.
https://review.coreboot.org/c/coreboot/+/39343/4/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/4/src/soc/mediatek/mt8183/mem... PS4, Line 177: get_recovery_mode_retrain_switch Same story on this one
Hello build bot (Jenkins), Furquan Shaikh, Angel Pons, Julius Werner, Andrey Petrov, Patrick Rudolph, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39343
to look at the new patch set (#5).
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
vboot: remove extraneous vboot_recovery_mode_memory_retrain
Just call get_recovery_mode_retrain_switch() directly.
BUG=b:124141368 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Icb88d6862db1782e0218276984e527638b21fd3a Signed-off-by: Joel Kitching kitching@google.com --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/bootmode.c M src/security/vboot/vboot_common.h M src/soc/mediatek/mt8183/memory.c 5 files changed, 5 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/39343/5
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/4/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/4/src/soc/mediatek/mt8183/mem... PS4, Line 177: get_recovery_mode_retrain_switch
Same story on this one
Oops -- thanks. I just checked and all files now definitely include bootmode.h.
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/4/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/4/src/soc/mediatek/mt8183/mem... PS4, Line 177: get_recovery_mode_retrain_switch
Oops -- thanks. […]
Done
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) { This is a somewhat tangential but still related question... why do we only handle this recovery reason in MT8183? Shouldn't this also work on Intel? I thought this was originally introduced for Intel anyway (CL:407656)?
I was thinking we should maybe have a helper that combines checking the manual switch and the recovery reason (which might then be called something like vboot_recovery_mode_memory_retrain(), so maybe worth keeping the helper and just changing its meaning), but then I noticed that all the other code doesn't actually handle this reason. Is that intentional or an oversight?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) {
why do we only handle this recovery reason in MT8183? Shouldn't this also work on Intel?
I had actually not seen this code closely until now. The original intent of VB2_RECOVERY_TRAIN_AND_REBOOT was to simply ensure that the recovery MRC cache has valid training data. On MT8183, it looks like it is being used as a trigger to wipe out the memory training data and force a retrain for recovery mode. That is the reason you don't see any special handling for Intel platforms for recovery mode.
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) {
why do we only handle this recovery reason in MT8183? Shouldn't this also work on Intel? […]
Here's the original CL. https://review.coreboot.org/c/coreboot/+/36251
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) {
Here's the original CL. […]
Ohhh... so it's literally just supposed to be a recovery boot that reboots automatically. I think I never quite understood how that was meant. It's probably wrong to have it on Kukui here then, too.
I thought we had a requirement to allow the system to automatically trigger a retrain from software, but I guess we don't? Or I guess you could always use flashrom to erase training data manually, too.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
vboot: remove extraneous vboot_recovery_mode_memory_retrain
Just call get_recovery_mode_retrain_switch() directly.
BUG=b:124141368 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Icb88d6862db1782e0218276984e527638b21fd3a Signed-off-by: Joel Kitching kitching@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39343 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/bootmode.c M src/security/vboot/vboot_common.h M src/soc/mediatek/mt8183/memory.c 5 files changed, 5 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 455dfa5..ad95dce 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -15,6 +15,7 @@ #include <security/vboot/antirollback.h> #include <arch/symbols.h> #include <assert.h> +#include <bootmode.h> #include <cbfs.h> #include <cbmem.h> #include <cf9_reset.h> @@ -120,7 +121,7 @@ if (vboot_recovery_mode_enabled()) { if (!CONFIG(HAS_RECOVERY_MRC_CACHE)) return; - if (vboot_recovery_mode_memory_retrain()) + if (get_recovery_mode_retrain_switch()) return; }
diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index d4a4aab..2287f27 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -521,7 +521,7 @@ /* Invalidate only on recovery mode with retraining enabled. */ if (!vboot_recovery_mode_enabled()) return; - if (!vboot_recovery_mode_memory_retrain()) + if (!get_recovery_mode_retrain_switch()) return;
if (fmap_locate_area_as_rdev_rw(name, &rdev) < 0) { diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c index 2363bf9..6cbb116 100644 --- a/src/security/vboot/bootmode.c +++ b/src/security/vboot/bootmode.c @@ -71,11 +71,6 @@ return 0; }
-int vboot_recovery_mode_memory_retrain(void) -{ - return get_recovery_mode_retrain_switch(); -} - #if CONFIG(VBOOT_NO_BOARD_SUPPORT) /** * TODO: Create flash protection interface which implements get_write_protect_state. diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 57f3475..8be9d2a 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -67,14 +67,12 @@ #if CONFIG(VBOOT) int vboot_developer_mode_enabled(void); int vboot_recovery_mode_enabled(void); -int vboot_recovery_mode_memory_retrain(void); int vboot_can_enable_udc(void); void vboot_run_logic(void); int vboot_locate_cbfs(struct region_device *rdev); #else /* !CONFIG_VBOOT */ static inline int vboot_developer_mode_enabled(void) { return 0; } static inline int vboot_recovery_mode_enabled(void) { return 0; } -static inline int vboot_recovery_mode_memory_retrain(void) { return 0; } /* If VBOOT is not enabled, we are okay enabling USB device controller (UDC). */ static inline int vboot_can_enable_udc(void) { return 1; } static inline void vboot_run_logic(void) {} diff --git a/src/soc/mediatek/mt8183/memory.c b/src/soc/mediatek/mt8183/memory.c index 78890ea..15ae9cb 100644 --- a/src/soc/mediatek/mt8183/memory.c +++ b/src/soc/mediatek/mt8183/memory.c @@ -14,6 +14,7 @@ */
#include <assert.h> +#include <bootmode.h> #include <cbfs.h> #include <console/console.h> #include <ip_checksum.h> @@ -174,7 +175,7 @@ /* Load calibration params from flash and run fast calibration */ if (recovery_mode) { printk(BIOS_WARNING, "Skip loading cached calibration data\n"); - if (vboot_recovery_mode_memory_retrain() || + if (get_recovery_mode_retrain_switch() || vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) { printk(BIOS_WARNING, "Retrain memory in next boot\n"); /* Use 0xFF as erased flash data. */
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) {
so it's literally just supposed to be a recovery boot that reboots automatically.
Correct.
It's probably wrong to have it on Kukui here then, too.
Yes, we shouldn't have it here on Kukui too.
I thought we had a requirement to allow the system to automatically trigger a retrain from software, but I guess we don't?
No, it was just for the factory flow to ensure that the recovery cache is populated before devices are shipped.
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) {
so it's literally just supposed to be a recovery boot that reboots automatically. […]
http://cs/chromeos_public/src/platform/factory/py/test/pytests/mrc_cache.py?...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39343 )
Change subject: vboot: remove extraneous vboot_recovery_mode_memory_retrain ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/39343/5/src/soc/mediatek/mt8183/mem... PS5, Line 179: vboot_check_recovery_request() == VB2_RECOVERY_TRAIN_AND_REBOOT) {
http://cs/chromeos_public/src/platform/factory/py/test/pytests/mrc_cache. […]
That test ensures recovery MRC cache gets generated on setting appropriate recovery reason. It was done to ensure that we do not miss this during firmware qualification.
-
Angel Pons (Code Review) -
Furquan Shaikh (Code Review)
-
Joel Kitching (Code Review)
-
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)