[S] Change in coreboot[main]: cbfstool: Add hash to more than one region

[Julius Werner (Code Review)]

Attention is currently required from: Bao Zheng, Felix Held, Marshall Dawson, Zheng Bao.

Julius Werner has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85468?usp=email )

Change subject: cbfstool: Add hash to more than one region
......................................................................

Patch Set 12:

(1 comment)

Patchset:

PS1:
> Done

Sorry, I don't understand the "Done". Did you explain this somewhere else? Did I miss it?

You seem to be trying to add a big new boot path selection feature with tendrils deep into the core CBFS code here. I think it might be good if you could write down a high-level explanation of how that's supposed to work and what problem you're trying to solve with it, so that the rest of us can understand if and why this is the best solution for that. (Maybe it would even be a good candidate for the new feature request process that Werner suggested recently.)

From what I can guess from your code, it seems that you're trying to have two separate CBFSes, and the system is supposed to normally boot from the one in the COREBOOT section, but if it fails to do that due to a CBFS verification error, it is supposed to set some non-volatile state bit in the PSP that tells it to boot from the COREBOOTB section on the next boot instead? This basically seems like the same thing that you would get if you enable vboot with only a recovery and an "A" slot. Can you explain what is different about your solution that it needs to be implemented separately?

For features that touch core coreboot code (like the CBFS code), I think it is important that we try to only have a single implementation for each functionality, rather than each vendor trying to add their own thing that only differs in minor details. If 10 SoC vendors all wanted to implement their own A/B selection scheme in cbfs.c, the file would become unmaintainable. That's why we've been trying to standardize on CBFS verification and vboot to serve all of coreboot's verification and boot path selection needs, and expand them with more features and options to be able to meet everyone's use case. If you have a new use case that the existing code doesn't work for, maybe we should talk about where you could expand it and add more options to meed your needs, rather than adding a completely separate and disconnected thing on the side.