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/+/56773 )
Change subject: amdfwtool: Add support for AMD's BIOS A/B recovery feature ......................................................................
Patch Set 40:
(7 comments)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/56773/comment/31372a62_cb27e440 PS32, Line 237:
not directly related to this patch: would be good to have the . […]
Done. https://review.coreboot.org/c/coreboot/+/59382
https://review.coreboot.org/c/coreboot/+/56773/comment/de0c88b9_03450ab6 PS32, Line 237:
not directly related to this patch: would be good to have the . […]
Done
https://review.coreboot.org/c/coreboot/+/56773/comment/94ae06b1_15c4a04b PS32, Line 271: { .type = AMD_FW_PSP_SMU_FIRMWARE, .subprog = 1, .level = PSP_BOTH }, : { .type = AMD_FW_PSP_SMU_FIRMWARE2, .subprog = 1, .level = PSP_BOTH },
not directly related to this patch: these two entries seem to be duplicates of entries above. […]
Done
https://review.coreboot.org/c/coreboot/+/56773/comment/addb00f0_77f13147 PS32, Line 804: if (count > MAX_PSP_ENTRIES) { : fprintf(stderr, "Error: PSP entries exceed max allowed items\n"); : free(ctx->rom); : exit(1); : }
this block should be replaced with a call to assert_fw_entry right before the pspdir->entries[count] […]
Done. Also in https://review.coreboot.org/c/coreboot/+/59382
https://review.coreboot.org/c/coreboot/+/56773/comment/7479d9b1_10d5fbc5 PS32, Line 1751: 0x1000
TABLE_ALIGNMENT
Done
https://review.coreboot.org/c/coreboot/+/56773/comment/76b01117_c04637c6 PS32, Line 1754: 0x1000
TABLE_ALIGNMENT
Done
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/56773/comment/d178f64d_ca45e45b PS36, Line 264: { .type = AMD_ABL0, .level = PSP_BOTH | PSP_LVL2_AB }, : { .type = AMD_ABL1, .level = PSP_BOTH }, : { .type = AMD_ABL2, .level = PSP_BOTH }, : { .type = AMD_ABL3, .level = PSP_BOTH }, : { .type = AMD_ABL4, .level = PSP_BOTH }, : { .type = AMD_ABL5, .level = PSP_BOTH }, : { .type = AMD_ABL6, .level = PSP_BOTH }, : { .type = AMD_ABL7, .level = PSP_BOTH },
from the documentation i'd assume that those should all be the same. […]
Done
-
Bao Zheng (Code Review)