Change in coreboot[master]: soc/amd/picasso: solve MTRRs only from 4GiB and below

[Aaron Durbin (Code Review)]

Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

soc/amd/picasso: solve MTRRs only from 4GiB and below

Use x86_setup_mtrrs_with_detect_no_above_4gb() to only
solve the MTRR solution for memory up to 4GiB. This assumes
4GiB to TOM2 is marked as writeback in sys_cfg MSR.

BUG=b:155426691

Signed-off-by: Aaron Durbin 
Change-Id: Ib8358b614682f6a97278f3a60b5ada5e607965af
---
M src/soc/amd/picasso/cpu.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/41898/1

diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c
index 55f9014..c1d598e 100644
--- a/src/soc/amd/picasso/cpu.c
+++ b/src/soc/amd/picasso/cpu.c
@@ -36,7 +36,7 @@
  */
 static void pre_mp_init(void)
 {
-	x86_setup_mtrrs_with_detect();
+	x86_setup_mtrrs_with_detect_no_above_4gb();
    x86_mtrr_check();
 }

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 1: Code-Review-2

Need to fix up quite a few things before this can land, but keeping this here for now as it will be part of the final solution.

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 2: -Code-Review

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4: Code-Review+2

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4:

I cherry-picked the 3 MTRR changes and the Mandolin support on top of current coreboot master and rebuilt FSP, but first ramstage seems to crash (just stops and bootblock is rung again) and the second time it failed with "FspMemoryInit returned 0x80000003".
Without the 3 patches, but with the freshly built FSP coreboot hangs when running the VBIOS, so I'm reasonably sure that I'm running with the MTRR changes on the FSP side

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4: Code-Review-1

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4: Code-Review+1
> Patch Set 4:
>
> I cherry-picked the 3 MTRR changes and the Mandolin support on top of current coreboot master and rebuilt FSP, but first ramstage seems to crash (just stops and bootblock is rung again) and the second time it failed with "FspMemoryInit returned 0x80000003".
> Without the 3 patches, but with the freshly built FSP coreboot hangs when running the VBIOS, so I'm reasonably sure that I'm running with the MTRR changes on the FSP side

It sounded like there was something goofed with yesterday's CQ ordering and many people were seeing problems.  So hopefully if you give it another try it'll be OK.  I did a repo sync Saturday morning and checked it on Mandolin.  I rebased these 3 changes on top of 524962 and cherry-picked CB:33772.  I'm not seeing the problems you're reporting.  Also FWIW I verified the MTRRs are unmodified during the two FSP calls.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4:
> It sounded like there was something goofed with yesterday's CQ ordering and many people were seeing problems.  So hopefully if you give it another try it'll be OK.  I did a repo sync Saturday morning and checked it on Mandolin.  I rebased these 3 changes on top of 524962 and cherry-picked CB:33772.  I'm not seeing the problems you're reporting.  Also FWIW I verified the MTRRs are unmodified during the two FSP calls.

Ok, I'll retry that later today with a fresh FSP repo checkout. Only had 1 new MTRR related change in my FSP repo checkout when I built that yesterday; might be the cause of the issues

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4:
> Patch Set 4:
> > It sounded like there was something goofed with yesterday's CQ ordering and many people were seeing problems.  So hopefully if you give it another try it'll be OK.  I did a repo sync Saturday morning and checked it on Mandolin.  I rebased these 3 changes on top of 524962 and cherry-picked CB:33772.  I'm not seeing the problems you're reporting.  Also FWIW I verified the MTRRs are unmodified during the two FSP calls.
>
> Ok, I'll retry that later today with a fresh FSP repo checkout. Only had 1 new MTRR related change in my FSP repo checkout when I built that yesterday; might be the cause of the issues

Felix, were you able to verify things? Yes, in the chromium repos I messed up and didn't incorporate the new cq-depend syntax which caused things to land unatomically.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

Patch Set 4: Code-Review+2

reverting an unrelated fsp change fixed things for me

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41898 )

Change subject: soc/amd/picasso: solve MTRRs only from 4GiB and below
......................................................................

soc/amd/picasso: solve MTRRs only from 4GiB and below

Use x86_setup_mtrrs_with_detect_no_above_4gb() to only
solve the MTRR solution for memory up to 4GiB. This assumes
4GiB to TOM2 is marked as writeback in sys_cfg MSR.

BUG=b:155426691

Signed-off-by: Aaron Durbin 
Change-Id: Ib8358b614682f6a97278f3a60b5ada5e607965af
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41898
Reviewed-by: Raul Rangel 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Marshall Dawson 
Reviewed-by: Felix Held 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/picasso/cpu.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Held: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved
  Marshall Dawson: Looks good to me, but someone else must approve
  Raul Rangel: Looks good to me, approved

diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c
index de9a2fa..96edd70 100644
--- a/src/soc/amd/picasso/cpu.c
+++ b/src/soc/amd/picasso/cpu.c
@@ -36,7 +36,7 @@
  */
 static void pre_mp_init(void)
 {
-	x86_setup_mtrrs_with_detect();
+	x86_setup_mtrrs_with_detect_no_above_4gb();
    x86_mtrr_check();
 }