Inject Heads version string into coreboot LOCALVERSION... #859
Conversation
|
this is mostly a POC at the moment, we can adjust as desired from here |
|
@MrChromebox away from computer but ci build failed. Will check |
|
@tlaurion probably a few dependencies on the literal coreboot.rom that I missed, looks like one for flash-x230. I'll clean it up later tonight or tomorrow |
|
@MrChromebox @tlaurion I built and flashed the rom image for x230. Hwids log looks good: |
@Asiderr PrivacyBeast and Nitropad will have some problems going that path, if the sole version of the ROM board version (LOCAL_VERSION = ProductName -> Heads-x230-hotp-v0.2.0-917-g19f0e65) alone isn't enough for fwupd to determine firmware upgrade applicable, since x230 ProductName SEEMS to be considered in the equation. So to speak, a coreboot BiosVersion (CBET4000 Heads-x230-hotp-v0.2.0-917-g19f0e65) SHOULD serve alone to differentiate a BiosVersion to be applicable for fwupd, without considering ProductName. There is more then 30 different ProductName variants for x230 boards (including x230t), while Heads supports all of them without internal differences. If some differences applies between boards, they would be covered in different $BOARD in Heads parlance, to support more or less features, rquiring more or less SPI freed (available CBFS space), also differentiating how flashrom should deal with rom upgrades in the board config (should that board flash only the BIOS ifd region or flash the whole 12mb flash region?) since we are not producers of the boards themselves and do not know prior of the fact which ProductName the Heads board configuration supports (and we sell), which will vary from machine to machines supported by the same board config (x230-htop, x230, x230-external-flash, etc) Is that a problem? (impacts NitroPads from Nitrokey also : @jans23) |
|
pondering this a bit, thinking that we should probably drop setting the LOCALVERSION from the coreboot configs, since it could be confusing to have a value there that's overwritten. What would probably make the most sense is to only set LOCALVERSION if it doesn't already exist in the config -- so default to injecting $(HEADS_GIT_VERSION) but still allow the user to be override via coreboot config. thoughts? |
@agreed that that would confuse users to have a value and not understand easily where that comes from. Would be better to not set LOCAL_VERSION into coreboot config but inject it prior of building coreboot.
I do not think $(HEADS_GIT_VERSION) is enough. @MrChromebox I restate the need to have something like Heads-$BOARD-$TAG-$$(HEADS_GIT_VERSION-$dirty as proposed previously, while @Asiderr input is mandatory to have something FWUPD will be able to deal with to compare versions, to not go further before #859 (comment) is understood and consensus is obtained on proper versioning for Heads, dealt with by FWUPD. Else my understanding is that coreboot will need to be patched with custom vendors for Insurgo and Nitrokey and others and really complexifying board and coreboot configs.
Short future is Heads having versioning from releases tags again, so artifacts are injected by CIs and hashes confirmed for reproducibility by different docker image building the same commit. So an artifact should clearly differenciate that we are talking about:
That's at least my 2 cents until @Asiderr gives his input, since the real importance here is how FWUPD and LVFS will deal with firmware upgrade calls and storage and differenciate them. |
will drop and modify to inject only if not present in board's coreboot config
@tlaurion |
@MrChromebox awesome! |
I already added $(BOARD) to the output filenames, just not to LOCALVERSION :) |
@MrChromebox i'm confused here. Board name, as explained and showed in @Asiderr output, shows that LocalVersion SHOULD have this information else #859 (comment) Here, we do not know if boardname is x230, x230-hotp-verification, x230-external-flash etc. And that would be totally different firmwares and functionalities, and should differentiate which firmware is to be the upgrade to current locally deployed firmware. Not the ProductName, BaseboardProduct. Short:
Hope this clarifies the use case and the reasoning behind all of this. |
IMO, we should patch coreboot to not overwrite the product name, unless doing so breaks something on the OS side. All non-thinkpad boards properly have the board name set by coreboot and can be identified that way |
|
@MrChromebox I tried to reach you over slack to clarify incomprehension and reduce issue cluttering. |
MrChromebox
force-pushed
the
inject_heads_version
branch
from
c3aa537
to
0cd0684
Compare
|
@tlaurion @MrChromebox From the fwupd wrapper point of view, a user is passing the device flag, which is equivalent to the BOARD variable. It's compared with the update id, which specifies update type. The whole update process is based on the metadata file, which describes the update. This is an example of the metadata that I’ve used during the development stage: IMHO we want to place an update type and a device name in the |
|
@Asiderr the question remains, how does the proper device flag get passed to fwupd? If the OS is doing that, then it needs to be pulled from somewhere, and the SMBIOS ProductName seems like a reasonable place to do so (vs embedded in the BIOS VERSION and having to extract from a longer string) |
MrChromebox
force-pushed
the
inject_heads_version
branch
2 times, most recently
from
ecb678e
to
dda913c
Compare
|
@MrChromebox @tlaurion For now, this depends only on the user choice - fwupd looks for the updates for the fwupd will look for x230-hotp updates. If the update exists, fwupd will download the ROM image and will place it in the |
I already talked with @jans23 about this topic. As I always try to upstream our additions, our NitroPad builds differ only in a few things to default heads builds. If it is necessary to drop these differences for a fwupd support, we would probably just go with a "standard" heads build. Anything useful/important should be included in osresearch/heads anyway. That is to say, anything necessary to change would probably be fine for us. |
|
@MrChromebox : (Tip: search in screen for |
|
@tlaurion I'm confused as to why it would be trying to build linuxboot at all |
|
Ok. I'll use x230-flash build output to compare master and this PR failed builds. In 1: In 2: in 1 Makefile: in 2 Makefile: in 1 boards/x230-flash/x230-flash.config: in 2 boards/x230-flash/x230-flash.config: |
|
@MrChromebox : On x230 board, there is no specification for all, so the upper Makefile specifies the location of the image. |
|
Seems like the |
|
@MrChromebox applied on top of this branch: Passes.... |
Produces: |
Will be injected as part of the build using $(HEADS_GIT_VERSION) Signed-off-by: Matt DeVillier <[email protected]>
Needed for fwupd to handle board updates Signed-off-by: Matt DeVillier <[email protected]>
Signed-off-by: Matt DeVillier <[email protected]>
MrChromebox
force-pushed
the
inject_heads_version
branch
from
dda913c
to
9fb1649
Compare
makes builds uniquely identifiable based on board and version. Signed-off-by: Matt DeVillier <[email protected]>
MrChromebox
force-pushed
the
inject_heads_version
branch
from
9fb1649
to
7d3c048
Compare
|
@tlaurion seems like we came to same conclusion but slightly different implementation |
PR linuxboot#859 should have included this after linuxboot#858 was merged, but was missed Signed-off-by: Matt DeVillier <[email protected]>
PR #859 should have included this after #858 was merged, but was missed Signed-off-by: Matt DeVillier <[email protected]>
|
Note because I din't know myself: |
and produce coreboot output file in format:
heads-$(BOARD)-$(VERSION)[-dirty].rom
Signed-off-by: Matt DeVillier [email protected]