Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27369 )
Change subject: soc/intel/basecode: Add support for updating ucode loaded via FIT ......................................................................
Patch Set 46:
(5 comments)
https://review.coreboot.org/c/coreboot/+/27369/30/src/soc/intel/common/basec... File src/soc/intel/common/basecode/fw_update/ucode_update.c:
https://review.coreboot.org/c/coreboot/+/27369/30/src/soc/intel/common/basec... PS30, Line 33: define locate_staging_rw(rdev) \ : fmap_locate_area_as_rdev_rw(CONFIG_INTEL_TOP_SWAP_FIT_ENTRY_FMAP_REG, \ : rdev) : : #define locate_staging_ro(rdev) \ : fmap_locate_area_as_rdev(CONFIG_INTEL_TOP_SWAP_FIT_ENTRY_FMAP_REG, \ : rdev)
If it's only about the line length, how about giving these long […]
Done
https://review.coreboot.org/c/coreboot/+/27369/30/src/soc/intel/common/basec... PS30, Line 62: stage_microcode = rdev_mmap_full(&staging_rdev);
That requires us to allocate a buffer here. Wanted to avoid that hence the mmap.
Done
https://review.coreboot.org/c/coreboot/+/27369/30/src/soc/intel/common/basec... PS30, Line 118: check_and_update_ucode
Because the FIT table will be in RO. I don't think we need to check that.
Done
https://review.coreboot.org/c/coreboot/+/27369/30/src/soc/intel/common/basec... PS30, Line 130: if (ucode_update_rec_mode_enabled()) {
AFAIK, there is no clear line where coreboot ends. You could say, […]
Done
https://review.coreboot.org/c/coreboot/+/27369/30/src/soc/intel/common/basec... PS30, Line 179: * write does not finish and we end up not loading
you mean add an additional check for checksum at line 163 for checksum? […]
Done