ci: base: enable efi in DISTRO_FEATURES

[ricardosalveti]

Enable efi in DISTRO_FEATURES in order to enable efi support in systemd, which provides a better integration with systemd-boot, bootctl and efi runtime support.

[quic-vkraleti]

Can this be part of meta-qcom-distro layer?

[ndechesne]

yes, it should. However our CI, currently, does not use meta-qcom-distro, as it's tested independently of any distro, with poky.
we need to start thinking what CI we want to do with -distro as well, for sure.

[ricardosalveti]

It will also be part of the distro itself, here it is just because it is required for basic systemd EFI integration. This is just so we can validate EFI support just with this layer.

[lumag]

I think it's fine as long as it is a part of the CI config (rather than being set in one of the layer's conf files).

[koenkooi]

I've created a PR for meta-qcom-distro to enable the 'efi' DISTRO_FEATURE: qualcomm-linux/meta-qcom-distro#7

[ricardosalveti]

I've created a PR for meta-qcom-distro to enable the 'efi' DISTRO_FEATURE: quic-yocto/meta-qcom-distro#7

Thanks, we should have enabled here and there as well, at least for efi, as this directs how the integration is done with systemd.

[igoropaniuk]

A bit off-topic and unrelated, but I've noticed that systemd is the only recipe which expects efi in the DISTRO_FEATURES , others check MACHINE_FEATURES instead (including Unified kernel image (UKI) class), which IMO it doesn't make much sense to me and it should be a machine feature only. So for the sake of avoiding confusion and divergence does it make sense to address that OE-core (systemd recipe)?

$ grep -e "_FEATURES" --include=*.bb* -r ./ | grep -e efi
./poky/meta/recipes-core/systemd/systemd_256.7.bb:    ${@bb.utils.filter('DISTRO_FEATURES', 'acl audit efi ldconfig pam pni-names selinux smack usrmerge polkit seccomp', d)} \
./poky/meta/recipes-core/packagegroups/packagegroup-core-boot.bb:    ${@bb.utils.contains("MACHINE_FEATURES", "efi", "${EFI_PROVIDER} kernel", "", d)} \
./poky/meta/classes-recipe/live-vm-common.bbclass:EFI = "${@bb.utils.contains("MACHINE_FEATURES", "efi", "1", "0", d)}"
./poky/meta/classes-recipe/live-vm-common.bbclass:EFI_CLASS = "${@bb.utils.contains("MACHINE_FEATURES", "efi", "${EFI_PROVIDER}", "", d)}"
./poky/meta/classes-recipe/live-vm-common.bbclass:        pcbios = bb.utils.contains("MACHINE_FEATURES", "efi", "0", "1", d)
./poky/meta/classes-recipe/uki.bbclass:#     MACHINE_FEATURES:append = " efi"

[koenkooi]

@igoropaniuk if the systemd recipe would look at MACHINE_FEATURES, you'd get a different init system for each machine, which is why its features are under DISTRO control.
The recipes that look at MACHINE_FEATURES do that to ensure the devices actually boots, for systemd it's not as critical.

I guess it would be nice to have the systemd recipe emit a warning when the selected MACHINE has efi, but the DISTRO has not.

[igoropaniuk]

@koenkooi

well, systemd recipe looks for efi feature specifically and only for enabling build of systemd-boot efi loader, whose functionality depends on existence of ESP partition (booctrl install), efi variables support and exported EFI system tables by UEFI implementation (EDK2 in our case), and all of that are machine specific. What the point of building it, if it might not be functional on a target system.

[lumag]

@igoropaniuk I think it is a mistake, see https://git.openembedded.org/openembedded-core/diff/meta/recipes-core/systemd/systemd_232.bb?id=0a1427bf9aeeda6bee2cc0af8da4ea5fd90aef6f . Initally systemd recipe was using MACHINE_FEATURES to check for efi, but then it was changed to use DISTRO_FEATURES during mass-editing. So it well might be an error of rp.

[ricardosalveti]

I didn't check the history to see when that was changed, but by original thinking was towards not only MACHINE_FEATURES, but also connect that when systemd-boot is set as EFI_PROVIDER, as it would be better integrated there (but in theory it could also work fine with grub, something we have to check).

If it gets well integrated with both grub2 and systemd-boot, then it should really be moved back to MACHINE_FEATURES.

[ndechesne]

As @koenkooi said, then it's really confusing. A recipe that has a behavior that depends on MACHINE_FEATURES should be MACHINE_ARCH, not generic.. i don't see how it can be otherwise. There are several places in poky where we use MACHINE_FEATURES in a ARCH package.. I don't quite get it.

[ndechesne]

FWIW. Quick chat on OE IRC based on the discussion above:

<ndec> hey RP , we noticed that in 0a1427bf9aeeda6bee2cc0af8da4ea5fd90aef6f (in core), which seems to be a rework to use bb.utils.filter() we changed the code and we used to check for 'efi' in MACHINE_FEATURES, and that got converted in checking DISTRO_FEATURES. It seems odd (and buggy) that this change was made in such a hidden way.  
<ndec> i know it's old stuff.. but we were discussing with lumag on an adjacent topic and noticed that
<ndec> i then realized that some recipes use MACHINE_FEATURES ([matchbox-panel-2_2.12.bb](http://matchbox-panel-2_2.12.bb/) or [ovmf_git.bb](http://ovmf_git.bb/)) but they are not marked as MACHINE_ARCH, and that seemed odd to me too. I would expect using MACHINE_FEATURE to imply a MACHINE_ARCH package.
<RP> ndec: I'd agree that MACHINE_FEATURES implies MACHINE_ARCH
<RP> ndec: the efi change looks like a mistake/bug
<RP> Saur: ndec spotted a mistake in 0a1427bf9aeeda6bee2cc0af8da4ea5fd90aef6f from 7 years ago! :)
<Saur> RP: While the switch from `MACHINE_FEATURES` to `DISTRO_FEATURES` for `efi` does look like a bug, using `MACHINE_FEATURES` in the `systemd` recipe seems like a bigger bug to me...
<Saur> The question now is do people set `efi` in `DISTRO_FEATURES` to get `systemd` to do the right thing, or is noone using that feature?
<Saur> (We do not use EFI, so I have no idea how it is used in OE.)
<RP> Saur: those are good questions but at the very least it should have been spelt out in the commit message
<khem> ona different note efi is just not a machine feature though perhaps a combined features
<RP> I agree having systemd machine specific would be a problem
<RP> I think distro feature does make sense but it should have been mentioned in the commit message
<RP> Saur: it just makes a change for the bug to not be in one of my commits :)
<Saur> RP: It should, but I most likely missed it not realising someone would actually use `MACHINE_FEATURES` in `systemd` and just moved it along with al the other features....
<RP> Saur: right, I can imagine doing that myself

[ricardosalveti]

Yeah, makes sense, using MACHINE_FEATURES in systemd wouldn't be a good change, so we are back to DISTRO_FEATURES.

[igoropaniuk]

LGTM