Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37514 )
Change subject: soc/intel/skylake: Add option to control microcode update inclusion ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Sorry, I'm late. I'm not very happy about the prompts. "Board can ..." doesn't sound like a user decision. Also, some prompts are even for soldered down CPUs. This is nothing a user should be prompted for.
Also, we already had two means to override the default decision:
- `config CPU_MICROCODE_CBFS_EXTERNAL_BINS`, and
- mainboard makefiles can overwrite `cpu_microcode_bins`.
Now, if a board maintainer would choose to do the latter, it would override the prompted Kconfig settings :-/
Basically this is an attempt to make this process more user-friendly and remove the requirement to edit Kconfig (or other) files. In fact, modifying these settings is a user decision. It is only required to do this when you run into trouble because of a large payload e.g. I can understand your position as well but this hugely depends on the definition of what a user is. If you look at the feedback from Angel Pons you can see the request here was to have this as a menu item. So this is already in a conflict with your view on how this should work.
There was no need to edit files, we already have config CPU_MICROCODE_CBFS_EXTERNAL_BINS. What this patch adds is a mapping from CPU names/descriptions to CPUID signatures that was missing before. It's a nice addition, but I don't see how the added Kconfig options make it more user friendly. They don't even have a help text, no sign that they are related to the added microcode updates. Also, they only affect config CPU_MICROCODE_CBFS_DEFAULT_BINS, but show up no matter if that is selected.
And for 90% of the boards the added Kconfig prompts are not necessary because we know what is soldered to the board. We try to avoid Kconfig prompts that can result in bricks, you just added a bunch.
As you can imagine, it is not my intention to turn as many boards as possible into bricks. The intention is to make it easier for the user to select a subset of the microcode blobs available for this platform and easily remove the ones that don't apply.
You are absolutely right about the missing help and the missing dependency on CPU_MICROCODE_CBFS_DEFAULT_BINS this is absolutely confusing. The issue with CPU_MICROCODE_CBFS_EXTERNAL_BINS is that it is quite easy to pick the wrong one and brick the system that way.
Thinking about I was wondering if it would make more sense to reverse the logic. So if you have the CPU_MICROCODE_CBFS_DEFAULT_BINS you will be provided the option to remove SKYLAKE microcode e.g. This would make it easier to control from the mainboard level using a select and I think it would be more obvious to a user to understand what happens.
Please let me know what you think.