Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Marshall Dawson, Zheng Bao, Felix Held. Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54901 )
Change subject: amdfwtool: Add a function to extract firmwares ......................................................................
Patch Set 6:
(8 comments)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/54901/comment/ca5a7dfb_4e4e92e8 PS6, Line 216: a
an
Done
File util/amdfwtool/extract.c:
https://review.coreboot.org/c/coreboot/+/54901/comment/7b84df08_6940dfc4 PS6, Line 61: void extract_psp_fws(uint8_t *buffer, psp_directory_table *ppsp, uint32_t image_base, uint32_t *psp2_offset)
line over 96 characters […]
Done
https://review.coreboot.org/c/coreboot/+/54901/comment/92a7e754_b6cf172b PS6, Line 68: psp_cookie = ppsp->header.cookie;
Since this is only used in if/else decisions for naming below, would it look cleaner to make the dec […]
Ack. Seems not good to take the filename construction out of loop.
https://review.coreboot.org/c/coreboot/+/54901/comment/4f089642_332e0a19 PS6, Line 77: 0x40 also use enum.
Done.
https://review.coreboot.org/c/coreboot/+/54901/comment/2b6b504b_a33b7528 PS6, Line 128: 0x63
enum. And same for below.
Done
https://review.coreboot.org/c/coreboot/+/54901/comment/4cdd0566_edba3d9b PS6, Line 190: image_fd
Need to close.
Done
https://review.coreboot.org/c/coreboot/+/54901/comment/dceed64e_aa9925fa PS6, Line 206: romsig_offsets[index] != 0xFFFFFFFF
Does this for() loop also need a check for not reading past the size of the buffer? For example, if […]
Done
https://review.coreboot.org/c/coreboot/+/54901/comment/5cb817e9_b125b19f PS6, Line 228: bhd_offset = *(uint32_t *)&buffer[romsig_offset + 0x28] - image_base;
Forgot to delete.
Done
-
Bao Zheng (Code Review)