Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39327 )
Change subject: cbfs: Move more stuff into cbfs_boot_lookup() ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/39327/3/src/lib/cbfs.c File src/lib/cbfs.c:
https://review.coreboot.org/c/coreboot/+/39327/3/src/lib/cbfs.c@37 PS3, Line 37: err == CB_CBFS_NOT_FOUND) {
same line like before?
I honestly don't know what the deal is with the line lengths right now... when it was voted on they said they would reformat the whole codebase to 96 and now a year or whatever it was later we're still stuck on this ugly mix of mostly 80 and a little 96 everywhere. :(
Anyway, for now my policy has always been to keep changes to files at 80 where the rest of the file is still at 80 for consistency, and write whole new files with 96. Not sure what everyone else is doing anymore.
https://review.coreboot.org/c/coreboot/+/39327/3/src/lib/cbfs.c@62 PS3, Line 62: const struct region_device *bootdev = boot_device_ro();
Why is this boot_device_ro()? Below you are using rdev_relative_offset() with bootdev and the result […]
I'm using the assumption here that boot_device_ro() is always the root rdev for anything cbfs_get_boot_device() could return. I'm using the same assumption in https://review.coreboot.org/c/coreboot/+/39304/4/src/lib/cbfs.c#96 -- oh okay, I see you didn't like it there either. ;)
I think it's fine though, because both for the RO and RW use case in cbfs_get_boot_device() we eventually end up making a fmap_locate_area_as_rdev() (which calls boot_device_ro_subregion()). Everything in the CBFS stack is just about reading, we never write, it never calls fmap_locate_area_as_rdev_rw(). So even for the split boot device platforms this should be fine.
I have added some comments to document this in the declarations of struct cbfs_boot_device and boot_device_ro(), does that help?
https://review.coreboot.org/c/coreboot/+/39327/3/src/security/tpm/tspi/crtm.... File src/security/tpm/tspi/crtm.h:
https://review.coreboot.org/c/coreboot/+/39327/3/src/security/tpm/tspi/crtm.... PS3, Line 48: struct region_device *rde
const
Done
https://review.coreboot.org/c/coreboot/+/39327/3/src/security/tpm/tspi/crtm.... File src/security/tpm/tspi/crtm.c:
https://review.coreboot.org/c/coreboot/+/39327/3/src/security/tpm/tspi/crtm.... PS3, Line 109: rdev
variable naming conflict.
Whoops, sorry, looks like I left this half-finished somehow. Done.