Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the flash map layout

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

mb/intel/jasperlake_rvp: Re-organize the flash map layout

More space is required in the COREBOOT CBFS to accommodate some features.
Currently no alternate firmware is stuffed into RW_LEGACY CBFS and has
~1 MB of unused space. Borrow some space from RW_LEGACY CBFS and extend
the RO_SECTION. Even within RO_SECTION, GBB requires only 12 KiB. So
adjust the GBB region accordingly and extend the COREBOOT CBFS.

BUG=b:162159386
TEST=Build the JSLRVP mainboard.

Change-Id: Ia8bb381c31ddf76f3211f9d4ac5c8c18c27834b7
Signed-off-by: Karthikeyan Ramasubramanain 
---
M src/mainboard/intel/jasperlake_rvp/chromeos.fmd
1 file changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44283/1

diff --git a/src/mainboard/intel/jasperlake_rvp/chromeos.fmd b/src/mainboard/intel/jasperlake_rvp/chromeos.fmd
index 827e448..05f4592 100644
--- a/src/mainboard/intel/jasperlake_rvp/chromeos.fmd
+++ b/src/mainboard/intel/jasperlake_rvp/chromeos.fmd
@@ -28,15 +28,15 @@
    		RW_VPD(PRESERVE)@0x28000 0x2000
    		RW_NVRAM(PRESERVE)@0x2a000 0x6000
    	}
-		RW_LEGACY(CBFS)@0x5d0000 0x100000
-		WP_RO@0x6d0000 0x330000 {
+		RW_LEGACY(CBFS)@0x5d0000 0x30000
+		WP_RO@0x600000 0x400000 {
    		RO_VPD(PRESERVE)@0x0 0x4000
-			RO_SECTION@0x4000 0x32c000 {
+			RO_SECTION@0x4000 0x3fc000 {
    			FMAP@0x0 0x800
    			RO_FRID@0x800 0x40
    			RO_FRID_PAD@0x840 0x7c0
-				GBB@0x1000 0xef000
-				COREBOOT(CBFS)@0xf0000 0x23c000
+				GBB@0x1000 0x3000
+				COREBOOT(CBFS)@0x4000 0x3f8000
    		}
    	}
    }

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

Patch Set 2:

This CL has got nothing to do with the build failure in havlor(pasted below). Is it possible to re-run the build bot. May be there is some flakiness?

CC         romstage/mainboard/google/volteer/variants/halvor/memory.o
src/mainboard/google/volteer/variants/halvor/memory.c:57:27: error: conflicting types for 'variant_memory_params'
 const struct lpddr4x_cfg *variant_memory_params(void)
                           ^~~~~~~~~~~~~~~~~~~~~
In file included from src/mainboard/google/volteer/variants/halvor/memory.c:3:
src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h:21:30: note: previous declaration of 'variant_memory_params' was here
 const struct ddr_memory_cfg *variant_memory_params(void);
                              ^~~~~~~~~~~~~~~~~~~~~
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
make[1]: *** [Makefile:362: /cb-build/coreboot-gerrit.0/default/GOOGLE_HALVOR/romstage/mainboard/google/volteer/variants/halvor/memory.o] Error 1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

Patch Set 2: Code-Review+2
> Patch Set 2:
>
> This CL has got nothing to do with the build failure in havlor(pasted below). Is it possible to re-run the build bot. May be there is some flakiness?
> CC         romstage/mainboard/google/volteer/variants/halvor/memory.o
>
> src/mainboard/google/volteer/variants/halvor/memory.c:57:27: error: conflicting types for 'variant_memory_params'
>  const struct lpddr4x_cfg *variant_memory_params(void)
>                            ^~~~~~~~~~~~~~~~~~~~~
> In file included from src/mainboard/google/volteer/variants/halvor/memory.c:3:
> src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h:21:30: note: previous declaration of 'variant_memory_params' was here
>  const struct ddr_memory_cfg *variant_memory_params(void);
>                               ^~~~~~~~~~~~~~~~~~~~~
> cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:362: /cb-build/coreboot-gerrit.0/default/GOOGLE_HALVOR/romstage/mainboard/google/volteer/variants/halvor/memory.o] Error 1

That error is definitely unrelated. Two commits were submitted, CB:44249 which changed the signature of `variant_memory_params`, and CB:43988 which introduced a new instance using the old signature. This broke master. Once CB:43076 was submitted, Jenkins was happy again.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44283/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44283/2//COMMIT_MSG@7 
PS2, Line 7: flash map layout
FMAP layout

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

Patch Set 2: Code-Review+2

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44283/2/src/mainboard/intel/jasperl... 
File src/mainboard/intel/jasperlake_rvp/chromeos.fmd:

https://review.coreboot.org/c/coreboot/+/44283/2/src/mainboard/intel/jasperl... 
PS2, Line 31: 0x30000
if required, we can further cut on this. current allocation would not be enough to fit the alt-fw anyways.

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the flash map layout
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44283/2/src/mainboard/intel/jasperl... 
File src/mainboard/intel/jasperlake_rvp/chromeos.fmd:

https://review.coreboot.org/c/coreboot/+/44283/2/src/mainboard/intel/jasperl... 
PS2, Line 31: 0x30000
> if required, we can further cut on this. […]

Currently this re-org meets the needs. We can re-visit later if required.

Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the FMAP layout

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Justin TerAvest, Angel Pons, Aamir Bohra, Divagar Mohandass, Karthikeyan Ramasubramanian,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44283

to look at the new patch set (#3).

Change subject: mb/intel/jasperlake_rvp: Re-organize the FMAP layout
......................................................................

mb/intel/jasperlake_rvp: Re-organize the FMAP layout

More space is required in the COREBOOT CBFS to accommodate some features.
Currently no alternate firmware is stuffed into RW_LEGACY CBFS and has
~1 MB of unused space. Borrow some space from RW_LEGACY CBFS and extend
the RO_SECTION. Even within RO_SECTION, GBB requires only 12 KiB. So
adjust the GBB region accordingly and extend the COREBOOT CBFS.

BUG=b:162159386
TEST=Build the JSLRVP mainboard.

Change-Id: Ia8bb381c31ddf76f3211f9d4ac5c8c18c27834b7
Signed-off-by: Karthikeyan Ramasubramanain 
---
M src/mainboard/intel/jasperlake_rvp/chromeos.fmd
1 file changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44283/3

Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the FMAP layout

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the FMAP layout
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44283/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44283/2//COMMIT_MSG@7 
PS2, Line 7: flash map layout
> FMAP layout

Done

Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the FMAP layout

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the FMAP layout
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/44283/2/src/mainboard/intel/jasperl... 
File src/mainboard/intel/jasperlake_rvp/chromeos.fmd:

https://review.coreboot.org/c/coreboot/+/44283/2/src/mainboard/intel/jasperl... 
PS2, Line 31: 0x30000
> Currently this re-org meets the needs. We can re-visit later if required.

ok.

Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the FMAP layout

[Justin TerAvest (Code Review)]

Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the FMAP layout
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the FMAP layout

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the FMAP layout
......................................................................

Patch Set 3:

Since I started a discussion on FMAP changes in CB:43790 a short discussion why this CL is still okay whereas 43790 may run into failure states:

We never update RW_LEGACY (even though the chromeos updater has _some_ provisions for that, I'm not sure we ever activated them) and so there are no changes during updates to any regions affected here (RW_LEGACY, RO_* and the FMAP itself). The only way to change these is to rewrite the entire flash which is safe.

Change in coreboot[master]: mb/intel/jasperlake_rvp: Re-organize the FMAP layout

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44283 )

Change subject: mb/intel/jasperlake_rvp: Re-organize the FMAP layout
......................................................................

mb/intel/jasperlake_rvp: Re-organize the FMAP layout

More space is required in the COREBOOT CBFS to accommodate some features.
Currently no alternate firmware is stuffed into RW_LEGACY CBFS and has
~1 MB of unused space. Borrow some space from RW_LEGACY CBFS and extend
the RO_SECTION. Even within RO_SECTION, GBB requires only 12 KiB. So
adjust the GBB region accordingly and extend the COREBOOT CBFS.

BUG=b:162159386
TEST=Build the JSLRVP mainboard.

Change-Id: Ia8bb381c31ddf76f3211f9d4ac5c8c18c27834b7
Signed-off-by: Karthikeyan Ramasubramanain 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44283
Tested-by: build bot (Jenkins) 
Reviewed-by: Aamir Bohra 
Reviewed-by: Justin TerAvest 
---
M src/mainboard/intel/jasperlake_rvp/chromeos.fmd
1 file changed, 5 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aamir Bohra: Looks good to me, approved
  Justin TerAvest: Looks good to me, approved

diff --git a/src/mainboard/intel/jasperlake_rvp/chromeos.fmd b/src/mainboard/intel/jasperlake_rvp/chromeos.fmd
index 827e448..05f4592 100644
--- a/src/mainboard/intel/jasperlake_rvp/chromeos.fmd
+++ b/src/mainboard/intel/jasperlake_rvp/chromeos.fmd
@@ -28,15 +28,15 @@
    		RW_VPD(PRESERVE)@0x28000 0x2000
    		RW_NVRAM(PRESERVE)@0x2a000 0x6000
    	}
-		RW_LEGACY(CBFS)@0x5d0000 0x100000
-		WP_RO@0x6d0000 0x330000 {
+		RW_LEGACY(CBFS)@0x5d0000 0x30000
+		WP_RO@0x600000 0x400000 {
    		RO_VPD(PRESERVE)@0x0 0x4000
-			RO_SECTION@0x4000 0x32c000 {
+			RO_SECTION@0x4000 0x3fc000 {
    			FMAP@0x0 0x800
    			RO_FRID@0x800 0x40
    			RO_FRID_PAD@0x840 0x7c0
-				GBB@0x1000 0xef000
-				COREBOOT(CBFS)@0xf0000 0x23c000
+				GBB@0x1000 0x3000
+				COREBOOT(CBFS)@0x4000 0x3f8000
    		}
    	}
    }