Hi Angel,
Thanks for the response, and for all the work that's been done. I do appreciate what went into your patches.
I wish that it had been discussed more widely before you'd completed it though. This is always going to be an issue of doing something in private with the hope that coreboot will just accept what's been done. Of course, maybe the other way doesn't work well either. When I wrote up a proposal for postcodes, I was told to just implement it and then I could get feedback. I guess I don't know what the best way is.
The rest of my responses are inline. I'd still like to have a public meeting as I mentioned in my last email to see what we can work out regarding this issue.
Take care, Martin
Nov 16, 2023, 09:29 by th3fanbus@gmail.com:
Hi all,
I am writing things down as I think them, so I am sure this email will most likely turn out to be a hailstorm of scatterbrained ideas. Brace yourselves, here comes a wall o'text...
CFR is part of a very ambitious idea I came up several months ago to solve a specific problem with a general-purpose solution:
- Specific problem: one of 9elements' customers wants a "UEFI setup
menu" equivalent on a COMe module with coreboot+edk2, options' visibility and default value depend on runtime-only data
- General-purpose solution: payload-independent data structures in
cbtables (or somewhere else, if needed) that describe which options coreboot supports
Sure, having way to support a setup engine seems totally reasonable. I like the idea.
About the CFR (coreboot forms representation / cursed forms representation) name: I needed to name the folder in which I put the code I was writing. And because I was looking at UEFI stuff for inspiration when writing the code, the names of IFR (Internal Forms Representation) and VFR (Visual Forms Representation) were fresh in my brain-cache and influenced my decision... Even though CFR is separate from UEFI (although it borrows some names), if the CFR name is unnaceptably UEFI-sounding, I don't mind renaming the thing to something else as long as it's short and not easily confused with a different thing (something like "option table" wouldn't be great because it can be confused with the existing CMOS option table subsystem)...
Hmmm, maybe keep the CFR acronym but redefine it as "Configure Firmware at Runtime"? [This should address Martin's 1st objection.]
Why would we want to keep the CFR acronym and try to fit names to it? Is there something special about CFR?
What you're proposing here is data to be consumed by a setup engine. This actually doesn't have very much to do with the firmware configuration, as that would be the back-end that this proposal doesn't actually address (though I think that could be a problem).
With additional support for CMOS indexing, CFR could replace original CMOS option records in cbtables. However, boards like asus/p2b already struggle with flash chip space and would not benefit from using CFR anyway. What seems doable, however, is to generate both the cmos.{default,layout} files as well as the pre-CFR input data (the structures with the `sm_` prefix) from a single source of data (replacing the cmos.{default,layout} files, which would have been the source of data until then).
Sure, having a single format that supports any and all option back-ends sounds good to me too.
How would you propose to actually support that if there's no back-end information in any of this data? It seems like there's going to need to be some sort of method associated with this to tell where the data for each setting is going to be stored, at least for *some* back-ends, even if it's not needed for your current issue.
As Nico pointed out, most of the proposed code is about serialization, and is the result of "build to think" design thinking: it's much easier to notice shortcomings and other issues of a design when one has a working example to play with. Furthermore, the serialization needs to happen *somewhere*, no matter where the data to serialize comes from. However, I am fully aware that having to write option descriptors directly as C code is outright painful; a proper solution pretty much requires using a domain-specific language to generate the data to be serialized (not unlike SCONFIG and devicetrees, for instance).
Regarding the domain-specific language for the runtime-configurable options, my first thought was to use the devicetree. With chipset devicetrees, this would solve a major pain point that CMOS layouts have: redundancy. Every board needs to repeat option declarations and enumerations for chipset options, the only difference being the offset of the option in CMOS space (size should be the same no matter what). With runtime options in the devicetree, chipsets would describe the options and mainboards would "use" them as needed. There are options like `gfx_uma_size` (on Intel platforms, the amount of RAM reserved for the iGPU) that only matter in certain situations (e.g. `gfx_uma_size` does nothing when the iGPU is disabled); one could use devicetree information to show/hide these options as needed. One could even add a new device operation so that each device fills in CFR with its options. [This should address Martin's 3rd objection.]
One issue is that I'm not interested in just showing/hiding data as needed. For example, I don't want intel specific stuff to be present on AMD or ARM platforms, but hidden - that's a waste of space. We need to be able to customize what actually goes into the menu data and exclude pieces that aren't needed.
I'd look more to Kconfig than to devicetree for setting up a menu since that is, after all, what Kconfig does. I think we could probably use a DSL very similar to Kconfig to support everything that's needed here.
Even when only considering the specific problem (see above), generating CFR structures at build-time is out of the question: knowing which options are visible and what their default value should be is only possible after reading a "profile code" value from an EC-attached EEPROM. We already generate more complex structures at runtime (ACPI SSDTs, SMBIOS...) anyway. That being said, it is true that simpler use-cases (e.g. a regular desktop board, without custom shenanigans) could be made to work with build-time generated CFR structures, although conditional options would require patching the CFR structures manually or something else... [This should address Martin's 2nd objection.]
I have to disagree that generating the *structures* at build-time are out of the question. Maybe some of the data needs to be updated, but you can pull the data from the EC-attached EEPROM in your mainboard code, and update whatever fields need to be updated after copying things to CBMEM. You can even update the menu data directly in CBFS. That isn't something that every coreboot platform (or even most) will need to do. Please don't force a solution required for your single customized platform onto the entirety of coreboot. A mainboard-specific solution to the issue of reading the data and populating it where it needs to go seems totally reasonable to me.
Am I missing something?
About default values in CFR: they actually have a dual purpose. For regular options, they allow the user to reset options to their default value from the setup menu. For "volatile" options (i.e. options which are read-only and are not stored in the backend), the default value is the value of the option. These "volatile" options are used to pass data to the payload, e.g. serial numbers or the aforementioned profile code. This is convenient when using edk2 as payload, as the CFR deserializer outputs variables which can be accessed from anywhere. Note: edk2 deserializes the CFR structures into variables on every boot, but these variables are only displayed visually when entering the setup menu (Boot Manager).
So if you set a value of "Default" in the setup engine, and read the value in from the EEPROM on the next boot, does that not meet your requirement? I mean, that's what you're already proposing, though in the reverse order. You're reading the default value on *every* boot just in case it's needed by the setup engine. That seems wasteful to me.
Regarding enums being defined separately to allow for reuse, I like the idea. Why didn't I think of it before? It can also be useful for boolean options, which do not define any names for true/false (if memory serves right, edk2 code assumes `false ---> Disabled` and `true ---> Enabled`) and could benefit from having more specific wording in certain cases. It's true that one would have to resolve the enum indices on the deserializer side, but it shouldn't be too bad (the dependency/callback mechanism is likely more complicated, need to catch up with that part). [This should address Nico's comment about CMOS enums being more flexible.]
Regarding multi-language support, I haven't thought too much about it... Well, until now. To avoid repeating things, we could have a `translations` folder at the root of the coreboot repo containing one text file for each language. These files would need to use an encoding (UTF-8 maybe?) that works well with non-English languages. There would be some sort of "translation key" for each localized message, maybe a numeric ID or a descriptive string (e.g. `enums.power_on_after_fail.keep`). The option descriptors (e.g. stuff in the devicetree) would reference these "translation keys" accordingly. Then, the build-time "option processor" (if the options are part of the devicetree, then SCONFIG; else, a similar kind of program) would collect a list of all the needed "translation keys" for this image, and a separate program would take said list and generate a "string table" for each enabled language (behavior when translation incomplete TBD). These "string tables" would be in binary form, stored in CBFS (compressed), and coreboot would place them in cbtables at runtime. The payload (or whoever displays the options to the user) would pick the desired "string table" from cbtables and use it to display the localized options. The post-build "translation keys" can be numbers instead of variable-length strings for space efficiency reasons.
But we should first decide on which functionality we want to cover regarding i18n (internationalization). The above contemplates multi-language support with on-the-fly switching with no access to CBFS, which is the most extreme case. If accessing files from CBFS directly is possible, then it's not necessary to bloat cbtables with strings. If on-the-fly switching isn't necessary (changes take effect next boot), then coreboot only needs to place one "string table" into cbtables. If run-time language switching isn't necessary (language is chosen at build time), the "string table" in CBFS can be removed, and the build-time tools would instead place the localized strings in the to-be-compiled code.
I absolutely agree that we need to decide what we want first - that's what I think we're trying to do here. I'm not even pushing to include internationalization into the initial configuration, just saying that it's something we're probably going to want at some point, and we need to look at how we would support it when we need it.
I think it'd be better to have the setup engine support decompressing the language tables from CBFS, but I'd be fine if the setup engine required a reboot to change languages if needed. It's not like people are typically going to be switching them a lot.
I *think* I've addressed most questions, but I feel I've still missed something. If so, please remind me.with
Best regards, Angel
On Sat, Nov 11, 2023 at 3:09 AM Martin Roth via coreboot coreboot@coreboot.org wrote:
Since this is for a setup engine, I'd think we'd want multi-language support as well. That's going to be difficult to handle if we try to generate all the strings at runtime. I think that support needs to be fundamental to the design.
Martin
Nov 1, 2023 at 13:54 by coreboot@coreboot.org:
Hi Patrick, I put a bunch of comments into the reviews, but I thought I'd reply directly on the mailing list as well for anyone who isn't looking at the patches.
We discussed this at today's leadership meeting where I actually brought up the patches and discussed some of the things I'd seen.
I may not completely understand the intentions here, and I'd like to understand the requirements that led to this solution.
Really, I have 3 main objections:
The name - “Forms Representation” may be the single most UEFI phrase I've ever read. Use "Config Option Data", "Option Data Structures", "Menu Option Data", anything other than "Forms Representation". Is that maybe a mechanical translation from a different language, or does someone actually talk that way?
As I said, I don't understand your requirements, but this seems hugely more complex than it needs to be. I also think this entire structure should be generated at build time and not runtime. My personal opinion is that the default for any item shouldn't be something read from EC, SPI, wherever at runtime. Instead it should be a flag that says to update the (non-default) setting from someplace at runtime.
It should be to populate and modify the menus from any different level in the coreboot hierarchy - Arch, SoC, (Vendor), Mainboard. Right now, the menus seem to be very hardcoded, without a good way to insert items from any other place in the tree other than where the menu is created.
I guess I'd prefer to have something more like how Kconfig or devicetree work, taking text files from all of the different levels in coreboot, combining them into a tree, then generating the structures from that.
Unfortunately, Items 2 and 3 are completely different from your proposal, and would require a complete rewrite. Sorry about that.
Maybe we can set up a meeting time to discuss things between 9E and interested parties in the coreboot community?
Take care. Martin
Oct 31, 2023, 10:45 by patrick.rudolph@9elements.com:
Hello coreboot folks, At the time of writing coreboot is lacking good firmware menus or user-space applications to change options consumed by coreboot. While there's support for changing CMOS options in SeaBIOS or from within Linux on some x86 mainboards it's quite limited in user friendliness. While CMOS was a great thing some decades ago, it cannot compete with today's flash based non-volatile option store, like UEFI variables, SMMSTORE or VPD.
Our best engineers at 9elements came up with a possible solution: coreboot forms representation, see [1], [2], [3] CFR allows to have nice firmware menus in coreboot payloads as done in example for tianocore/EDK2 [5]. While it was designed with EDK2 in mind it's possible to use it on any payload with any variable store back-end (UEFI, SMMSTORE, VPD, CMOS, ...), as it's really about providing meta data about the options, not how to actually store it's value in a non-volatile fashion. The coreboot forms representation is provided to a payload in a new coreboot table and is generated at runtime by using C-style lookup tables, but in order to reduce code duplicates can be defined in common code as done in [4].
Please review and give feedback. Regards, Patrick
1: https://review.coreboot.org/c/coreboot/+/77882/ 2: https://review.coreboot.org/c/coreboot/+/74121/ 3: https://review.coreboot.org/c/coreboot/+/78506/ 4: https://review.coreboot.org/c/coreboot/+/78296/ 5: https://github.com/9elements/edk2/commit/f34f17996444ef811cc0223b4f875c45daa... _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
-
Martin Roth