Change in coreboot[master]: mb/google/hatch: Preserve MRC training data across FW update
Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
mb/google/hatch: Preserve MRC training data across FW update
Enable RW_RESERVE so that we retain the memory training data upon a FW update.
BUG=b:142084637 BRANCH=None TEST=flash RW_SECTION_A, RW_SECTION_B, and WP_RO sections and make sure memory training doesn't occur on following bootup.
Change-Id: Ia5eb228b1f665a8371982544723dab3dfc40d401 Signed-off-by: Shelley Chen shchen@google.com --- M src/mainboard/google/hatch/chromeos.fmd 1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35803/1
diff --git a/src/mainboard/google/hatch/chromeos.fmd b/src/mainboard/google/hatch/chromeos.fmd index 393ac80..07c739c 100644 --- a/src/mainboard/google/hatch/chromeos.fmd +++ b/src/mainboard/google/hatch/chromeos.fmd @@ -19,9 +19,11 @@ RW_FWID_B@0x3dffc0 0x40 } RW_MISC@0x17c0000 0x40000 { - UNIFIED_MRC_CACHE@0x0 0x30000 { - RECOVERY_MRC_CACHE@0x0 0x10000 - RW_MRC_CACHE@0x10000 0x20000 + RW_PRESERVE(PRESERVE) { + UNIFIED_MRC_CACHE@0x0 0x30000 { + RECOVERY_MRC_CACHE@0x0 0x10000 + RW_MRC_CACHE@0x10000 0x20000 + } } RW_ELOG(PRESERVE)@0x30000 0x4000 RW_SHARED@0x34000 0x4000 {
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... PS1, Line 22: RW_PRESERVE nit: MRC_CACHE maybe? RW_PRESERVE doesn't have a lot of meaning
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 1: Code-Review+2
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... PS1, Line 22: RW_PRESERVE
nit: MRC_CACHE maybe? RW_PRESERVE doesn't have a lot of meaning
I modelled it after octopus here:
https://cs.corp.google.com/chromeos_public/src/third_party/coreboot/src/main...
I don't feel strongly about the name though...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... PS1, Line 22: RW_PRESERVE
I modelled it after octopus here: […]
We can just add PRESERVE property to UNIFIED_MRC_CACHE. RW_PRESERVE was a thing before (PRESERVE) property support was enabled.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG@7 PS1, Line 7: Preserve MRC training data across FW update I'm not sure why this would be desired over retraining. Would it be possible to state the reasons to do this? Thanks.
Hello Paul Fagerburg, Julius Werner, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35803
to look at the new patch set (#2).
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
mb/google/hatch: Preserve MRC training data across FW update
Enable RW_RESERVE so that we retain the memory training data upon a FW update.
BUG=b:142084637 BRANCH=None TEST=flash RW_SECTION_A, RW_SECTION_B, and WP_RO sections and make sure memory training doesn't occur on following bootup.
Change-Id: Ia5eb228b1f665a8371982544723dab3dfc40d401 Signed-off-by: Shelley Chen shchen@google.com --- M src/mainboard/google/hatch/chromeos.fmd 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35803/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35803/2//COMMIT_MSG@9 PS2, Line 9: Enable RW_RESERVE "Add PRESERVE to UNIFIED_MRC_CACHE" ?
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG@7 PS1, Line 7: Preserve MRC training data across FW update
I'm not sure why this would be desired over retraining. […]
The reason we're doing this is because after we do a FW update, it takes awhile to boot to kernel (like 15 seconds) due to memory training and people are getting confused if the device is actually booting or not and try to hard reset it at some point by holding the power button down.
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/35803/1/src/mainboard/google/hatch/... PS1, Line 22: RW_PRESERVE
We can just add PRESERVE property to UNIFIED_MRC_CACHE. […]
Done
Hello Paul Fagerburg, Julius Werner, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35803
to look at the new patch set (#3).
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
mb/google/hatch: Preserve MRC training data across FW update
Add PRESERVE to UNIFIED_MRC_CACHE so that we retain the memory training data upon a FW update.
BUG=b:142084637 BRANCH=None TEST=flash RW_SECTION_A, RW_SECTION_B, and WP_RO sections and make sure memory training doesn't occur on following bootup.
Change-Id: Ia5eb228b1f665a8371982544723dab3dfc40d401 Signed-off-by: Shelley Chen shchen@google.com --- M src/mainboard/google/hatch/chromeos.fmd 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35803/3
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35803/2//COMMIT_MSG@9 PS2, Line 9: Enable RW_RESERVE
"Add PRESERVE to UNIFIED_MRC_CACHE" ?
Oops. Thanks for catching that.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 3: Code-Review+2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 3: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG@7 PS1, Line 7: Preserve MRC training data across FW update
The reason we're doing this is because after we do a FW update, it takes awhile to boot to kernel (l […]
Fifteen seconds to do memory init? O_o
I can see why such a delay is unwanted: impatient users could wrongfully conclude their device is not booting anymore after a FW update. Could you please add this to the commit message? Thanks!
Hello Paul Fagerburg, Angel Pons, Julius Werner, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35803
to look at the new patch set (#4).
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
mb/google/hatch: Preserve MRC training data across FW update
Add PRESERVE to UNIFIED_MRC_CACHE so that we don't retain the memory training data upon a FW update unless we need to. We have had users complaining that a 15 second memory training upon update makes them believe that their device is not booting, thus many of them hard resetting before bootup.
BUG=b:142084637 BRANCH=None TEST=flash RW_SECTION_A, RW_SECTION_B, and WP_RO sections and make sure memory training doesn't occur on following bootup.
Change-Id: Ia5eb228b1f665a8371982544723dab3dfc40d401 Signed-off-by: Shelley Chen shchen@google.com --- M src/mainboard/google/hatch/chromeos.fmd 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35803/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Thanks!
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35803/1//COMMIT_MSG@7 PS1, Line 7: Preserve MRC training data across FW update
Fifteen seconds to do memory init? O_o […]
Done
Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35803 )
Change subject: mb/google/hatch: Preserve MRC training data across FW update ......................................................................
mb/google/hatch: Preserve MRC training data across FW update
Add PRESERVE to UNIFIED_MRC_CACHE so that we don't retain the memory training data upon a FW update unless we need to. We have had users complaining that a 15 second memory training upon update makes them believe that their device is not booting, thus many of them hard resetting before bootup.
BUG=b:142084637 BRANCH=None TEST=flash RW_SECTION_A, RW_SECTION_B, and WP_RO sections and make sure memory training doesn't occur on following bootup.
Change-Id: Ia5eb228b1f665a8371982544723dab3dfc40d401 Signed-off-by: Shelley Chen shchen@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35803 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/hatch/chromeos.fmd 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved Paul Fagerburg: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/chromeos.fmd b/src/mainboard/google/hatch/chromeos.fmd index 393ac80..45dbc81 100644 --- a/src/mainboard/google/hatch/chromeos.fmd +++ b/src/mainboard/google/hatch/chromeos.fmd @@ -19,7 +19,7 @@ RW_FWID_B@0x3dffc0 0x40 } RW_MISC@0x17c0000 0x40000 { - UNIFIED_MRC_CACHE@0x0 0x30000 { + UNIFIED_MRC_CACHE(PRESERVE)@0x0 0x30000 { RECOVERY_MRC_CACHE@0x0 0x10000 RW_MRC_CACHE@0x10000 0x20000 }
-
Angel Pons (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Fagerburg (Code Review)
-
Shelley Chen (Code Review)
-
Tim Wawrzynczak (Code Review)