Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44196 )
Change subject: mrc_cache: Update mrc_cache data in romstage ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44196/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44196/8//COMMIT_MSG@16 PS8, Line 16: Updating spi driver Kconfig to set : BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY=y if both : BOOT_DEVICE_SPI_FLASH_RW_NOMMAP=y and BOOT_DEVICE_SPI_FLASH=y to bring : the appropriate libraries into romstage.
Why? I think in general it's better to make the thing you normally want be the default and not push […]
My understanding is that the config BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY was added to identify platforms that support writing to SPI flash in early CAR stages. There are some legacy Intel platforms which when executing out of memory mapped SPI flash did not allow writing back to it. Hence, we cannot just set BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY for all Intel platforms. Most of these platforms predate my involvement in coreboot. So, I am not really sure which platforms have this limitation.
Shelley - when we talked earlier this week I thought it was just Haswell and Broadwell boards that you needed BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY option for(based on the new boards where you selected MRC_WRITE_NV_LATE). But from your latest patchset, it looks like there are many more platforms for which this config is being added. I don't think it is possible to test out each of these platforms unless some one in the community has one of the boards and can do the testing.
In order to move forward with minimal regression, I think we should add a new Kconfig (I know we really wanted to avoid this, but it seems necessary) MRC_STASH_TO_CBMEM which will be auto-selected if MRC_WRITE_NV_LATE or !BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY. Then, in mrc_cache.c you can write back to flash immediately if !MRC_STASH_TO_CBMEM, else save the MRC data to CBMEM.
And, in finalize_mrc, you will have to write back to flash if MRC_STASH_TO_CBMEM is set (No need to check for MRC_WRITE_NV_LATE since it auto-selects MRC_STASH_TO_CBMEM).
Thoughts?