[M] Change in coreboot[main]: Support CCB at a fixed SPI-flash offset

[Martin Roth (Code Review)]

Attention is currently required from: Simon Glass.

Martin Roth has posted comments on this change by Simon Glass. ( https://review.coreboot.org/c/coreboot/+/83293?usp=email )

Change subject: Support CCB at a fixed SPI-flash offset
......................................................................

Patch Set 2:

(7 comments)

File Documentation/technotes/ccb.md:

https://review.coreboot.org/c/coreboot/+/83293/comment/c2aacbf3_e58c6ca1?usp... :
PS2, Line 88: Care should be taken that
            : this region is in read-only flash, if preventing users from changing it is
            : important.
            : 
This is true for chromeos, but not every platform.

File Makefile.mk:

https://review.coreboot.org/c/coreboot/+/83293/comment/620f75e3_acae9ffd?usp... :
PS2, Line 1124: ifeq ($(CONFIG_CCB_FMAP),y)
Maybe print a warning or just error out if the CCB FMAP alignment is different than what's in the fmap file?

https://review.coreboot.org/c/coreboot/+/83293/comment/af163b22_f16cbe24?usp... :
PS2, Line 1259: ifneq ($(CONFIG_ARCH_X86),)
Make this the `else` of the above? Reverse the logic?

https://review.coreboot.org/c/coreboot/+/83293/comment/0708f996_0cac1ad4?usp... :
PS2, Line 1274: /usr/bin/printf
Maybe use `env printf` instead of the path?

File src/lib/bootblock.c:

https://review.coreboot.org/c/coreboot/+/83293/comment/cac741f2_b93c8b2c?usp... :
PS2, Line 63: or FMAP
Why does it need to be initialized late if it's at a known location in the SPI rom?  Should this be architecture dependent and different for SoCs that have memory mapped SPI ROMs vs any that don't?

File src/lib/ccb.c:

https://review.coreboot.org/c/coreboot/+/83293/comment/e8acd97d_8375d0d0?usp... :
PS2, Line 102: if (fmap_locate_area_as_rdev(CCB_REGION, rdev)) {
It seems to me that the advantage of using FMAP vs CBFS is that we can parse the fmap at build time and just get a direct address for the table so that we don't have to deal with finding the FMAP and then finding the CCB. This implementation seems no better than CBFS to me.

File util/cbfstool/cbfstool.c:

https://review.coreboot.org/c/coreboot/+/83293/comment/2755e319_d3807051?usp... :
PS2, Line 731: /* Now try FMAP */
Based on the Kconfig, we can't have CCB in more than one location. Why check more than the one that's specified in Kconfig? Doesn't that create a security hole?