Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Angel Pons, Felix Held. Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63589 )
Change subject: soc/amd/common/block/cpu/*: Make ucode update more generic ......................................................................
Patch Set 3:
(4 comments)
File src/soc/amd/common/block/cpu/update_microcode.c:
https://review.coreboot.org/c/coreboot/+/63589/comment/0a2823d3_d0cc3400 PS2, Line 79: for
So you are dropping the for loop.... We need that for the zork/picasso/dali platforms. […]
For picasso soc, each microcode variant will be in its own file (cpu_microcode_8180.bin, cpu_microcode_8181.bin, and cpu_microcode_8201.bin). There are no longer multiple microcodes in a single file, so the for loop is not needed anymore. On a given variant (8180, 8181, or 8201), it will only look up its own microcode.
https://review.coreboot.org/c/coreboot/+/63589/comment/9eeb9cf3_59b23d41 PS2, Line 66: static const struct microcode *find_microcode(const struct microcode *ucode, uint16_t equivalent_processor_rev_id)
clang-format
Done
https://review.coreboot.org/c/coreboot/+/63589/comment/0b22f964_f1dc749b PS2, Line 83: 23
Do you need the size?
Tested a timeless build with and without the size, and they were identical. Good catch, size was not needed
https://review.coreboot.org/c/coreboot/+/63589/comment/2b3bcc69_1ecaa5f2 PS2, Line 86: snprintf
I would check the return code.
Leaving unresolved for now - I moved the code into the !cache_valid block so it only runs once.
If snprintf were to fail (it really should not here), then the cbfs_map would fail and printk below should catch it and print what happened. Same thing in the preload_microcode() function below. I'm not sure what is gained by checking the return value here.
I used https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thir... as a reference to load a file from CBFS based on an ID in the file name.
-
Fred Reitberger (Code Review)