Change in ...coreboot[master]: google/kukui: Revise FMAP layout for larger CBFS

[Hung-Te Lin (Code Review)]

Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31035

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

google/kukui: Revise FMAP layout for larger CBFS

Kukui with vboot enabled will build with `detachable_ui`, which needs
larger space in CBFS for more complicated assets. So we need to revise
FMAP sections:

- Bootblock only needs <= 32K.
- GBB can be much smaller since assets moved from GBB to CBFS.
- COREBOOT(CBFS) should take all space left.
- FMAP is re-ordered (with the cost of less efficient in bsearch) so CBFS can
  get larger continuous space

BUG=b:123202015
TEST=Builds and boots on Kukui P1

Change-Id: I22cff99dca8c396c5897c3f6631721af40f3ffbd
Signed-off-by: Hung-Te Lin 
---
M src/mainboard/google/kukui/chromeos.fmd
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/31035/1

diff --git a/src/mainboard/google/kukui/chromeos.fmd b/src/mainboard/google/kukui/chromeos.fmd
index 1602f80..c563e08 100644
--- a/src/mainboard/google/kukui/chromeos.fmd
+++ b/src/mainboard/google/kukui/chromeos.fmd
@@ -1,10 +1,10 @@
 FLASH@0x0 0x800000 {
    WP_RO@0x0 0x200000 {
    	RO_SECTION@0x0 0x1f0000 {
-			BOOTBLOCK@0 128K
-			COREBOOT(CBFS)@0x20000 0xe0000
-			FMAP@0x100000 0x1000
-			GBB@0x101000 0xeef00
+			BOOTBLOCK@0 32K
+			FMAP@0x8000 0x1000
+			COREBOOT(CBFS)@0x9000 0x1e4000
+			GBB@0x1ed000 0x2f00
    		RO_FRID@0x1eff00 0x100
    	}
    	RO_VPD@0x1f0000 0x10000

[Hung-Te Lin (Code Review)]

Hello Julius Werner, You-Cheng Syu, build bot (Jenkins),

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

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

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

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

google/kukui: Revise FMAP layout for larger CBFS

Kukui with vboot enabled will build with `detachable_ui`, which needs
larger space in CBFS for more complicated assets. So we need to revise
FMAP sections:

- Bootblock only needs <= 32K.
- GBB can be much smaller since assets moved from GBB to CBFS.
- FMAP is re-ordered (with the cost of less efficient in bsearch) so CBFS can
  get larger continuous space
- COREBOOT(CBFS) should take all space left, and keep offset (so we
  don't need to update bootblock in EC)

BUG=b:123202015
TEST=Builds and boots on Kukui P1

Change-Id: I22cff99dca8c396c5897c3f6631721af40f3ffbd
Signed-off-by: Hung-Te Lin 
---
M src/mainboard/google/kukui/chromeos.fmd
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/31035/2

[Hung-Te Lin (Code Review)]

Hello Julius Werner, You-Cheng Syu, build bot (Jenkins),

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

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

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

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

google/kukui: Revise FMAP layout for larger CBFS

Kukui with vboot enabled will build with `detachable_ui`, which needs
larger space in CBFS for more complicated assets. So we need to revise
FMAP sections:

- BOOTBLOCK (not really used) only needs <= 32K.
- GBB can be much smaller since assets moved from GBB to CBFS.
- FMAP is re-ordered (with the cost of less efficient in bsearch) so CBFS can
  get larger continuous space.
- COREBOOT(CBFS) should take all space left.

Since FMAP and Coreboot has changed location, the system will need to
reflash EC (which contains the new bootblock) as well.

BUG=b:123202015
TEST=Builds and boots on Kukui P1

Change-Id: I22cff99dca8c396c5897c3f6631721af40f3ffbd
Signed-off-by: Hung-Te Lin 
---
M src/mainboard/google/kukui/chromeos.fmd
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/31035/3

[Hung-Te Lin (Code Review)]

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31035 )

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

Patch Set 3:

ready for review.

[Hung-Te Lin (Code Review)]

Hello Julius Werner, You-Cheng Syu, build bot (Jenkins),

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

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

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

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

google/kukui: Revise FMAP layout for larger CBFS

Kukui with vboot enabled will build with `detachable_ui`, which needs
larger space in CBFS for more complicated assets. So we need to revise
FMAP sections:

- BOOTBLOCK (not really used) only needs <= 32K.
- GBB can be much smaller since assets moved from GBB to CBFS.
- FMAP is re-ordered (with the cost of less efficient in bsearch) so CBFS can
  get larger continuous space.
- COREBOOT(CBFS) should take all space left.

Since FMAP and COREBOOT have changed location, the system will need to
reflash EC (which contains the new bootblock) as well.

BUG=b:123202015
TEST=Builds and boots on Kukui P1

Change-Id: I22cff99dca8c396c5897c3f6631721af40f3ffbd
Signed-off-by: Hung-Te Lin 
---
M src/mainboard/google/kukui/chromeos.fmd
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/31035/4

[You-Cheng Syu (Code Review)]

You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31035 )

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

Patch Set 4: Code-Review+1

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31035 )

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

Patch Set 4: Code-Review+2

(1 comment)

https://review.coreboot.org/#/c/31035/4/src/mainboard/google/kukui/chromeos.... 
File src/mainboard/google/kukui/chromeos.fmd:

https://review.coreboot.org/#/c/31035/4/src/mainboard/google/kukui/chromeos.... 
PS4, Line 6: 0x1e4000
BTW, you can leave values out that can be inferred (like the size here) if you just want it to fill all available space. In fact, you could write this whole section as just:

WP_RO 2M {
   RO_SECTION {
     BOOTBLOCK 32K
     FMAP 4K
     COREBOOT(CBFS)
     GBB 0x2f00
     RO_FRID 0x100
   }
   RO_VPD 64K
 }

It's a bit of a matter of taste whether you find that harder or easier to read, but it does make it easier to play around with the sizes (and harder to accidentally leave some empty space somewhere).

[Hung-Te Lin (Code Review)]

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31035 )

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/31035/4/src/mainboard/google/kukui/chromeos.... 
File src/mainboard/google/kukui/chromeos.fmd:

https://review.coreboot.org/#/c/31035/4/src/mainboard/google/kukui/chromeos.... 
PS4, Line 6: 0x1e4000
> BTW, you can leave values out that can be inferred (like the size here) if you just want it to fill  […]

Thanks, but I'd prefer to keep current form, which helps us to make sure all addressed were properly aligned, and also easier to debug if we have to code the address to somewhere in config.

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31035 )

Change subject: google/kukui: Revise FMAP layout for larger CBFS
......................................................................

google/kukui: Revise FMAP layout for larger CBFS

Kukui with vboot enabled will build with `detachable_ui`, which needs
larger space in CBFS for more complicated assets. So we need to revise
FMAP sections:

- BOOTBLOCK (not really used) only needs <= 32K.
- GBB can be much smaller since assets moved from GBB to CBFS.
- FMAP is re-ordered (with the cost of less efficient in bsearch) so CBFS can
  get larger continuous space.
- COREBOOT(CBFS) should take all space left.

Since FMAP and COREBOOT have changed location, the system will need to
reflash EC (which contains the new bootblock) as well.

BUG=b:123202015
TEST=Builds and boots on Kukui P1

Change-Id: I22cff99dca8c396c5897c3f6631721af40f3ffbd
Signed-off-by: Hung-Te Lin 
Reviewed-on: https://review.coreboot.org/c/31035
Tested-by: build bot (Jenkins) 
Reviewed-by: You-Cheng Syu 
Reviewed-by: Julius Werner 
---
M src/mainboard/google/kukui/chromeos.fmd
1 file changed, 4 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  You-Cheng Syu: Looks good to me, but someone else must approve

diff --git a/src/mainboard/google/kukui/chromeos.fmd b/src/mainboard/google/kukui/chromeos.fmd
index 1602f80..c563e08 100644
--- a/src/mainboard/google/kukui/chromeos.fmd
+++ b/src/mainboard/google/kukui/chromeos.fmd
@@ -1,10 +1,10 @@
 FLASH@0x0 0x800000 {
    WP_RO@0x0 0x200000 {
    	RO_SECTION@0x0 0x1f0000 {
-			BOOTBLOCK@0 128K
-			COREBOOT(CBFS)@0x20000 0xe0000
-			FMAP@0x100000 0x1000
-			GBB@0x101000 0xeef00
+			BOOTBLOCK@0 32K
+			FMAP@0x8000 0x1000
+			COREBOOT(CBFS)@0x9000 0x1e4000
+			GBB@0x1ed000 0x2f00
    		RO_FRID@0x1eff00 0x100
    	}
    	RO_VPD@0x1f0000 0x10000