insomniac has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42029 )
Change subject: drivers/vpd: Add a function that reads BIOS version from a VPD variable ......................................................................
Patch Set 6:
Patch Set 4:
Oh... I think I may have figured out what these two patches are planning to do.
To identify the version of a static firmware image (file), currently (if no vboot) there's no easy way. But if we put version in VPD, then you have the 'vpd' utility that can read (even set) from firmware. So they may have put the vpd stuff in the build procedure for that purpose.
That is correct: in our build process the firmware version is set after the original build. The reason is that we create release candidates with non-stable version numbers a coreboot-dirty-$HASH is fine for an RC). Then we test extensively the firmware image, and if everything is good we assign a "stable" version number (with a different numbering scheme from the non-stable ones).
But the VPD by design (although not strictly enforced) was expected to be empty in build procedure, and only provisioned when being manufactured. Moreover, if you use the reference updater (futility update), VPD will be preserved when updating BIOS, which means you'll not get the right version string.
IMHO a more correct implementation is:
If you use FMAP, define a version section explicitly, and fill the section in your build procedure where you can still access CONFIG_LOCAL_VERSION.
If FMAP is not an option, create a file like 'coreboot_version' and put that into CBFS may be easier, so you can still extract and read using cbfstool.
After reading your explanation on VPD, I strongly agree with your proposal #2: CBFS sounds like the right place where this information should go. Another reason to use VPD was that these variables are easily accessible via Linux's /sys/firmware/vpd , so no vpd tool is necessary. I was wondering if we should have a similar driver for Linux to access CBFS read-only, or if this is a bad idea.