coreboot

Does this patch detect your issue? https://review.coreboot.org/c/coreboot/+/73470

Maximilian Brune wrote in #note-1:

Does this patch detect your issue? https://review.coreboot.org/c/coreboot/+/73470

Unfortunately no, it seems that this does not cause an error, as ifdtool does not find the regions in FMAP. Why is that?

https://app.circleci.com/pipelines/github/gaspar-ilom/heads/37/workflows/a2223530-f5e9-4f28-be72-38f2f3e72e84/jobs/2705

Nov 15 18:25:27 printf "    IFDTOOL    validate IFD against FMAP\n"
Nov 15 18:25:27     IFDTOOL    validate IFD against FMAP
Nov 15 18:25:27 EOL_w541-maximized/util/ifdtool/ifdtool \
Nov 15 18:25:27          \
Nov 15 18:25:27         -t EOL_w541-maximized/coreboot.pre
Nov 15 18:25:27 Warning: No platform specified. Output may be incomplete
Nov 15 18:25:27 Warning: Not a single IFD region found in FMAP
Nov 15 18:25:27 File EOL_w541-maximized/coreboot.pre is 12582912 bytes

EDIT:
Looked into the code and apart from analyzing why regions could not be found I think we should also question whether it is the right call to just print an error instead of calling exit:

https://review.coreboot.org/c/coreboot/+/73470/13/util/ifdtool/ifdtool.c#1256

What does your build/fmap.fmd file look like?

Maximilian Brune wrote in #note-3:

What does your build/fmap.fmd file look like?

cat ./build/x86/coreboot-25.09/EOL_w541-maximized/fmap.fmd
# layout for firmware residing at top of 4GB address space
# +-------------+ <-- 4GB - ROM_SIZE / start of flash
# | unspecified |
# +-------------+ <-- 4GB - BIOS_SIZE
# | FMAP        |
# +-------------+ <-- 4GB - BIOS_SIZE + FMAP_SIZE
# | CBFS        |
# +-------------+ <-- 4GB / end of flash

FLASH 0x00c00000 {
	BIOS@131072 12451840 {
		
		RW_MRC_CACHE@0 0x10000
		
		
		
		
		FMAP@65536 0x200
		COREBOOT(CBFS)@66048 12385792
	}
}

gaspar ilom wrote in #note-4:

cat ./build/x86/coreboot-25.09/EOL_w541-maximized/fmap.fmd

# layout for firmware residing at top of 4GB address space
# +-------------+ <-- 4GB - ROM_SIZE / start of flash
# | unspecified |
# +-------------+ <-- 4GB - BIOS_SIZE
# | FMAP        |
# +-------------+ <-- 4GB - BIOS_SIZE + FMAP_SIZE
# | CBFS        |
# +-------------+ <-- 4GB / end of flash

FLASH 0x00c00000 {
	BIOS@131072 12451840 {
		
		RW_MRC_CACHE@0 0x10000
		
		
		
		
		FMAP@65536 0x200
		COREBOOT(CBFS)@66048 12385792
	}
}

there's something odd going on here, since the generated FMAP should have the IFD, ME, and GBE regions listed if you are including those blobs in the coreboot build.

gaspar ilom wrote in #note-2:

Maximilian Brune wrote in #note-1:

Does this patch detect your issue? https://review.coreboot.org/c/coreboot/+/73470

Unfortunately no, it seems that this does not cause an error, as ifdtool does not find the regions in FMAP. Why is that?

https://app.circleci.com/pipelines/github/gaspar-ilom/heads/37/workflows/a2223530-f5e9-4f28-be72-38f2f3e72e84/jobs/2705

Nov 15 18:25:27 printf "    IFDTOOL    validate IFD against FMAP\n"
Nov 15 18:25:27     IFDTOOL    validate IFD against FMAP
Nov 15 18:25:27 EOL_w541-maximized/util/ifdtool/ifdtool \
Nov 15 18:25:27          \
Nov 15 18:25:27         -t EOL_w541-maximized/coreboot.pre
Nov 15 18:25:27 Warning: No platform specified. Output may be incomplete
Nov 15 18:25:27 Warning: Not a single IFD region found in FMAP
Nov 15 18:25:27 File EOL_w541-maximized/coreboot.pre is 12582912 bytes

EDIT:
Looked into the code and apart from analyzing why regions could not be found I think we should also question whether it is the right call to just print an error instead of calling exit:

https://review.coreboot.org/c/coreboot/+/73470/13/util/ifdtool/ifdtool.c#1256

your manual invocation of ifdtool lacks the platform parameter, so all bets are off. But as above, since the IFD layout is not reflected in the FMAP as it should, then there is nothing to compare against.

here's what you should be seeing (plus the GBE region)

FLASH 0x1000000 {
        SI_DESC@0x0 0x1000
        SI_ME@0x1000 0x1FF000
        SI_BIOS@0xbb0000 0x450000 {

                RW_MRC_CACHE@0x0 0x10000
                SMMSTORE@0x10000 0x40000

                RO_VPD@0x50000 0x4000
                FMAP@0x54000 0x200
                COREBOOT(CBFS)@0x55000 0x3fb000
        }
}

Thanks Matt DeVillier!

there's something odd going on here, since the generated FMAP should have the IFD, ME, and GBE regions listed if you are including those blobs in the coreboot build.

The mentioned blobs are included. Here's my coreboot config: https://github.com/gaspar-ilom/heads/blob/try-cbfs-sanity-check-patch/config/coreboot-w541.config

your manual invocation of ifdtool lacks the platform parameter, so all bets are off. But as above, since the IFD layout is not reflected in the FMAP as it should, then there is nothing to compare against.

This was not a manual invocation but an excerpt from the logs when building heads with CircleCI. It seems that for haswell CONFIG_IFD_CHIPSET is not set, which means that some special handling does not apply in the main makefile: https://github.com/coreboot/coreboot/blob/main/Makefile.mk#L1087-L1096

This looks very much like the culprit to me or am I missing something? Question is: What would be the right way to make this work?

• Should we set CONFIG_IFD_CHIPSET for this board?
  • If so to what?
• Or should we strip the platform parameter from $(IFDTOOL) -p $(CONFIG_IFD_CHIPSET) -F $@ $< in the referenced code in case it is not there?
  • That would mean calling without the platform parameter if the ifd blob is used and the param is not explicitly selected. Maybe this is an option at least for some boards?

By the way, here's a manual invocation, that shows that it should work if this was stored in DEFAULT_FLASHMAP before the fmap.fmd target is built https://github.com/coreboot/coreboot/blob/main/Makefile.mk#L1231-L1246:

./build/x86/coreboot-25.09/util/ifdtool/ifdtool -p ifd2 -F w541_fmap.fmd blobs/w541/ifd.bin && cat w541_fmap.fmd 
File blobs/w541/ifd.bin is 4096 bytes
Wrote layout to w541_fmap.fmd
FLASH ##ROM_SIZE## {
	SI_DESC@0x0 0x1000
	SI_GBE@0x1000 0x2000
	SI_ME@0x3000 0x1E000
	SI_BIOS@##BIOS_BASE## ##BIOS_SIZE## {
		##CONSOLE_ENTRY##
		##MRC_CACHE_ENTRY##
		##SMMSTORE_ENTRY##
		##SPD_CACHE_ENTRY##
		##VPD_ENTRY##
		FMAP@##FMAP_BASE## ##FMAP_SIZE##
		COREBOOT(CBFS)@##CBFS_BASE## ##CBFS_SIZE##
	}
}

I didn't really test it yet, but this should cover it:
https://review.coreboot.org/c/coreboot/+/90107
Lets see if many boards fail the validation after jenkins is done.

Maximilian Brune wrote in #note-7:

I didn't really test it yet, but this should cover it:
https://review.coreboot.org/c/coreboot/+/90107
Lets see if many boards fail the validation after jenkins is done.

Thanks for your patch. Seems like passing no platform is not the correct approach. All the pre soc boards in heads fail the build with your patch: https://app.circleci.com/pipelines/github/gaspar-ilom/heads/43/workflows/40c359ba-cd32-4b40-8171-92ebc0a099c2 The reason is the generic fmap-template.fmd contains SI_PDR@0xFFF000 0x0 which causes an error:

FMAP       EOL_w541-maximized/util/cbfstool/fmaptool -h EOL_w541-maximized/fmap_config.h EOL_w541-maximized/fmap.fmd EOL_w541-maximized/fmap.fmap
E: Section 'SI_ME' too big
FATAL: Failed while processing provided descriptor
make[1]: *** [Makefile.mk:1262: EOL_w541-maximized/fmap.fmap] Error 4

cat ./build/x86/coreboot-25.09/EOL_w541-maximized/fmap-template.fmd
FLASH ##ROM_SIZE## {
        SI_DESC@0x0 0x1000
        SI_GBE@0x1000 0x2000
        SI_ME@0x3000 0x1E000
        SI_PDR@0xFFF000 0x0
        SI_BIOS@##BIOS_BASE## ##BIOS_SIZE## {
                ##CONSOLE_ENTRY##
                ##MRC_CACHE_ENTRY##
                ##SMMSTORE_ENTRY##
                ##SPD_CACHE_ENTRY##
                ##VPD_ENTRY##
                FMAP@##FMAP_BASE## ##FMAP_SIZE##
                COREBOOT(CBFS)@##CBFS_BASE## ##CBFS_SIZE##
        }
}

It is not replaced:

cat ./build/x86/coreboot-25.09/EOL_w541-maximized/fmap.fmd
FLASH 0x00c00000 {
	SI_DESC@0x0 0x1000
	SI_GBE@0x1000 0x2000
	SI_ME@0x3000 0x1E000
	SI_PDR@0xFFF000 0x0
	SI_BIOS@131072 12451840 {
		
		RW_MRC_CACHE@0 0x10000
		
		
		
		FMAP@65536 0x200
		COREBOOT(CBFS)@66048 12385792
	}
}

I tried building with hacking ifdtool_platform_param:=-p ifd2 in an else block for the W541 which creates an fmap file without SI_PDR but then the error changes to:

FMAP       EOL_w541-maximized/util/cbfstool/fmaptool -h EOL_w541-maximized/fmap_config.h EOL_w541-maximized/fmap.fmd EOL_w541-maximized/fmap.fmap
    MAKE       
E: Section 'SI_ME' too big
FATAL: Failed while processing provided descriptor
make[1]: *** [Makefile.mk:1262: EOL_w541-maximized/fmap.fmap] Error 4

Not sure how to move on from here.

I updated the change and it at least now passes jenkins.
Does it by any chance also fix your SI_ME too big issue?

Maximilian Brune wrote in #note-9:

I updated the change and it at least now passes jenkins.
Does it by any chance also fix your SI_ME too big issue?

Thanks for the update. SI_PDR is gone as expected but the SI_ME issue has remained as this excerpt from the logs shows:

    IFDTOOL     -F EOL_w541-maximized/fmap-template.fmd ../../../blobs/w541/ifd.bin
    CC         romstage/arch/x86/gdt_init.o
Warning: No platform specified. Output may be incomplete
File ../../../blobs/w541/ifd.bin is 4096 bytes
Wrote layout to EOL_w541-maximized/fmap-template.fmd
    CC            firmware/lib/cgptlib/cgptlib.o
    HOSTCC     util/sconfig/sconfig (link)
    HOSTCC     cbfstool/fmaptool (link)
    GEN        build.h
    MAKE       
    MAKE       
    MAKE       
    CC            firmware/lib/cgptlib/cgptlib_internal.o
    CC            firmware/lib/cgptlib/crc32.o
    CC            firmware/lib/gpt_misc.o
    CC            firmware/lib20/api_kernel.o
    CC            firmware/lib20/kernel.o
    RM            vboot_fw.a
    AR            vboot_fw.a
    SCONFIG    mainboard/lenovo/haswell/variants/w541/devicetree.cb
    FMAP       EOL_w541-maximized/util/cbfstool/fmaptool -h EOL_w541-maximized/fmap_config.h EOL_w541-maximized/fmap.fmd EOL_w541-maximized/fmap.fmap
E: Section 'SI_ME' too big
FATAL: Failed while processing provided descriptor
make[1]: *** [Makefile.mk:1260: EOL_w541-maximized/fmap.fmap] Error 4

Did you set CBFS_SIZE too high?

Maximilian Brune wrote in #note-11:

Did you set CBFS_SIZE too high?

Yes, the fix mentioned in the OP https://github.com/linuxboot/heads/pull/2025/commits/9b8b9cacdcea4f67c95dcec10b3153fc8c016940#diff-1146a022e2f557c41a3dc6d3b64c91c0891197c91163f30d80095f4d64ecab19 is not included so CBFS_SIZE is 0xBE4FFF where it should be 0xBDF000. However, this an incorrect size was also set for the sandy/ivy bridge boards (all 4+8MB SPI boards). The build only fails for the haswell boards as can be seen here, though: https://app.circleci.com/pipelines/circleci/MHXxJnuGL1oVD9jaDwEAGo/Kguuk4Rjpkd7hBpYBWhVuN/127/workflows/01a6f970-8d76-4e24-bb5c-b000109a4cbf

I have cherry-picked https://github.com/linuxboot/heads/pull/2025/commits/9b8b9cacdcea4f67c95dcec10b3153fc8c016940 to fix the size and build again. Build is happening here. Let's see if the boards build now: https://app.circleci.com/pipelines/github/gaspar-ilom/heads/58/workflows/94de47ac-a749-4dde-ac15-5b5e87341271

gaspar ilom wrote in #note-12:

Maximilian Brune wrote in #note-11:

Did you set CBFS_SIZE too high?

Yes, the fix mentioned in the OP https://github.com/linuxboot/heads/pull/2025/commits/9b8b9cacdcea4f67c95dcec10b3153fc8c016940#diff-1146a022e2f557c41a3dc6d3b64c91c0891197c91163f30d80095f4d64ecab19 is not included so CBFS_SIZE is 0xBE4FFF where it should be 0xBDF000. However, this an incorrect size was also set for the sandy/ivy bridge boards (all 4+8MB SPI boards). The build only fails for the haswell boards as can be seen here, though: https://app.circleci.com/pipelines/circleci/MHXxJnuGL1oVD9jaDwEAGo/Kguuk4Rjpkd7hBpYBWhVuN/127/workflows/01a6f970-8d76-4e24-bb5c-b000109a4cbf

I have cherry-picked https://github.com/linuxboot/heads/pull/2025/commits/9b8b9cacdcea4f67c95dcec10b3153fc8c016940 to fix the size and build again. Build is happening here. Let's see if the boards build now: https://app.circleci.com/pipelines/github/gaspar-ilom/heads/58/workflows/94de47ac-a749-4dde-ac15-5b5e87341271

I updated the link to the CI run as the original run failed (it is flaky) and the rerun succeeded: https://app.circleci.com/pipelines/github/gaspar-ilom/heads/58/workflows/94de47ac-a749-4dde-ac15-5b5e87341271

The result is that with the correct CBFS_SIZE the boards build successfully. So the main remaining issues now are:

• The error message could be improved to make it clear what is going wrong.
• The code for validating the IFD that is run by default with your original patch is never reached: https://review.coreboot.org/c/coreboot/+/73470 This is also why the error message is not as helpful as it could be.
• For some reason only older boards (Intel 4th Gen) pass validation even if CBFS_SIZE does not match the size in the IFD. This happens even when explicitly selecting VALIDATE_INTEL_DESCRIPTOR.

Here's another (force-pushed) run on my branch: https://app.circleci.com/pipelines/github/gaspar-ilom/heads/59/workflows/2ab3cc00-0fed-40a8-a325-ae6cb0b99c3c

• All boards with 4+8MB chips have a CBFS_SIZE greater than the bios region in the IFD
• T430 and W541 have VALIDATE_INTEL_DESCRIPTOR selected.
• Seems like both haswell boards (T440p and W541) fail with the SI_ME region error.
• However, older boars T430 and X230 both succeed. Although, both have an invalid CBFS_SIZE and t430 has VALIDATE_INTEL_DESCRIPTOR set

To me all of this seems really inconsistent.

Why does the t430 succeed with a too high CBFS_SIZE? and why does the t440p fail even though validation should not happen? What are the platform specific differences that cause this? sizes of the blobs paylod etc?