Change in coreboot[master]: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893
This patch fixes random hang seeing during ramstage after including below MTRR range with CB:34893
MTRR Range: Start=0 End=1000000 (Size 1000000)
TEST=Able to build and boot CML-hatch and ICLRVP.
With this CL
MTRR Range: Start=99000000 End=9a000000 (Size 1000000) MTRR Range: Start=ff000000 End=0 (Size 1000000)
<Booted to OS>
Without this CL
MTRR Range: Start=99000000 End=9a000000 (Size 1000000) MTRR Range: Start=0 End=1000000 (Size 1000000) MTRR Range: Start=ff000000 End=0 (Size 1000000)
<Hang in ramstage>
Change-Id: I7553a5f4ab4cff4a2158c9b6e0f7058f5c028f99 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/apollolake/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/memmap.c 5 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/35160/1
diff --git a/src/soc/intel/apollolake/memmap.c b/src/soc/intel/apollolake/memmap.c index bda43bb..88236587 100644 --- a/src/soc/intel/apollolake/memmap.c +++ b/src/soc/intel/apollolake/memmap.c @@ -76,4 +76,8 @@ */ smm_region(&smm_base, &smm_size); postcar_frame_add_mtrr(pcf, smm_base, smm_size, MTRR_TYPE_WRBACK); + + pcf->skip_common_mtrr = 1; + /* Cache the memory-mapped boot media. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); } diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index f0c21d9..5192a88 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -280,4 +280,9 @@ printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); top_of_ram -= 16*MiB; postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); + + pcf->skip_common_mtrr = 1; + /* Cache the memory-mapped boot media. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); + } diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index f7b2e07..bb1504d 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -102,4 +102,8 @@ */ smm_region(&smm_base, &smm_size); postcar_frame_add_mtrr(pcf, smm_base, smm_size, MTRR_TYPE_WRBACK); + + pcf->skip_common_mtrr = 1; + /* Cache the memory-mapped boot media. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); } diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 71368c6..d179ea1 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -278,4 +278,8 @@ printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); top_of_ram -= 16*MiB; postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); + + pcf->skip_common_mtrr = 1; + /* Cache the memory-mapped boot media. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); } diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 4c3c58a..7d1662d 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -319,5 +319,9 @@ */ smm_region(&smm_base, &smm_size); postcar_frame_add_mtrr(pcf, smm_base, smm_size, MTRR_TYPE_WRBACK); + + pcf->skip_common_mtrr = 1; + /* Cache the memory-mapped boot media. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); } #endif
Subrata Banik has removed Vanny E from this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Removed reviewer Vanny E.
Kane Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 1:
I tested on my hatch. it's so far so good now. Quanta, pls help to verify thanks
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 1: Code-Review+1
Patch Set 1:
I tested on my hatch. it's so far so good now. Quanta, pls help to verify thanks
Tested on kindred, can pass more than 50 cycles. thanks.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 1:
Patch Set 1: Code-Review+1
Patch Set 1:
I tested on my hatch. it's so far so good now. Quanta, pls help to verify thanks
Tested on kindred, can pass more than 50 cycles. thanks.
Thanks David, will wait for Furquan confirmation
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Balaji Manigandan, Kane Chen, build bot (Jenkins), David Wu, Jamie Chen, Furquan Shaikh, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35160
to look at the new patch set (#2).
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893
This patch fixes random hang seeing during ramstage after including below MTRR range with CB:34893
MTRR Range: Start=0 End=1000000 (Size 1000000)
TEST=Able to build and boot CML-hatch and ICLRVP.
With this CL
MTRR Range: Start=99000000 End=9a000000 (Size 1000000) MTRR Range: Start=ff000000 End=0 (Size 1000000)
<Booted to OS>
Without this CL
MTRR Range: Start=99000000 End=9a000000 (Size 1000000) MTRR Range: Start=0 End=1000000 (Size 1000000) MTRR Range: Start=ff000000 End=0 (Size 1000000)
<Hang in ramstage>
Change-Id: I7553a5f4ab4cff4a2158c9b6e0f7058f5c028f99 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/apollolake/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/memmap.c 5 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/35160/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 2: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/35160/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35160/2//COMMIT_MSG@29 PS2, Line 29: <Hang in ramstage> I still don't completely understand why the hang is seen with the MTRR for 0->1000000. But, I don't want to block all boards which are broken right now because of this.
https://review.coreboot.org/c/coreboot/+/35160/2/src/soc/intel/cannonlake/me... File src/soc/intel/cannonlake/memmap.c:
https://review.coreboot.org/c/coreboot/+/35160/2/src/soc/intel/cannonlake/me... PS2, Line 284: pcf->skip_common_mtrr = 1; Can you add a comment here indicating why the common mtrr programming was skipped?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35160/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35160/2//COMMIT_MSG@29 PS2, Line 29: <Hang in ramstage>
I still don't completely understand why the hang is seen with the MTRR for 0->1000000. […]
I don't either, is there anything in low memory at that time?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 2:
Can we add the appropriate BUG= lines to the commit description please?
Furquan Shaikh has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Removed Code-Review+2 by Furquan Shaikh furquan@google.com
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 2:
Patch Set 2:
Can we add the appropriate BUG= lines to the commit description please?
do we still need this CL? CB:35161 likely to get merged ?
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Balaji Manigandan, Kane Chen, build bot (Jenkins), David Wu, Furquan Shaikh, Jamie Chen, Karthik Ramasubramanian, Vanny E, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35160
to look at the new patch set (#3).
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893
This patch fixes random hang seeing during ramstage after including below MTRR range with CB:34893
MTRR Range: Start=0 End=1000000 (Size 1000000)
BUG=b:136784418 TEST=Able to build and boot CML-hatch and ICLRVP.
With this CL
MTRR Range: Start=99000000 End=9a000000 (Size 1000000) MTRR Range: Start=ff000000 End=0 (Size 1000000)
<Booted to OS>
Without this CL
MTRR Range: Start=99000000 End=9a000000 (Size 1000000) MTRR Range: Start=0 End=1000000 (Size 1000000) MTRR Range: Start=ff000000 End=0 (Size 1000000)
<Hang in ramstage>
Change-Id: I7553a5f4ab4cff4a2158c9b6e0f7058f5c028f99 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/apollolake/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/memmap.c 5 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/35160/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Patch Set 3:
Patch Set 2:
Patch Set 2:
Can we add the appropriate BUG= lines to the commit description please?
do we still need this CL? CB:35161 likely to get merged ?
I believe this would not be required once above CL gets merged.
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35160 )
Change subject: soc/intel/{apl,cnl,dnv,icl,skl} : Fix random hang introduced by CB:34893 ......................................................................
Abandoned
CB:35161
-
Aaron Durbin (Code Review) -
David Wu (Code Review)
-
Furquan Shaikh (Code Review)
-
Kane Chen (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)