[meta-intel] [PATCHv2 7/7] rmc: Use DISTRO_FEATURES as feature switch

[Tom Zanussi]

On 07/19/2016 01:53 PM, Jianxun Zhang wrote:
> 
>> On Jul 19, 2016, at 11:07 AM, Tom Zanussi <tom.zanussi at linux.intel.com> wrote:
>>
>> On 07/18/2016 08:00 PM, Jianxun Zhang wrote:
>>> From: Saul Wold <sgw at linux.intel.com>
>>>
>>> This refactoring is based on Saul Wold's initial attempt
>>> to have a more appropriate switch than using EFI_PROVIDER,
>>> to capture the whole RMC feature.
>>>
>>> Now put DISTRO_FEATURES_append = " rmc" in a conf file to
>>> enable RMC feature.
>>>
>>> RMC patches in systemd-boot, gnu-efi and EFI installer are
>>> not effective in build unless the feature is enabled.
>>>
>>> Signed-off-by: Jianxun Zhang <jianxun.zhang at linux.intel.com>
>>> ---
>>> conf/layer.conf          | 16 ++++++++++++++++
>>> documentation/README.rmc |  5 ++++-
>>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/conf/layer.conf b/conf/layer.conf
>>> index d8e5000..8e22186 100644
>>> --- a/conf/layer.conf
>>> +++ b/conf/layer.conf
>>> @@ -15,3 +15,19 @@ LICENSE_PATH += "${LAYERDIR}/common/custom-licenses"
>>> # This should only be incremented on significant changes that will
>>> # cause compatibility issues with other layers
>>> LAYERVERSION_intel = "3"
>>> +
>>> +
>>> +# Exclude RMC patches unless RMC Feature is eanbled
>>> +RMC_BBMASK := "${LAYERDIR}/common/recipes-bsp/systemd-boot/systemd-boot.*\.bbappend \
>>> +               ${LAYERDIR}/common/recipes-core/initrdscripts/initramfs-live-install-efi.*\.bbappend \
>>> +               ${LAYERDIR}/common/recipes-bsp/gnu-efi/gnu-efi.*\.bbappend"
>>> +
>>> +BBMASK += "${RMC_BBMASK}"
>>> +
>>> +BBMASK_remove = "${@bb.utils.contains('DISTRO_FEATURES', 'rmc', '${RMC_BBMASK}', '', d)}"
>>> +
>>> +# Override EFI_PROVIDER when RMC Feature is enabled
>>> +
>>> +EFI_PROVIDER_rmc_bootloader = "rmc-systemd-boot"
>>> +
>>> +OVERRIDES_append  = ":${@bb.utils.contains('DISTRO_FEATURES', 'rmc', 'rmc_bootloader', '', d)}"
>>> diff --git a/documentation/README.rmc b/documentation/README.rmc
>>> index eb31d22..d00b5ef 100644
>>> --- a/documentation/README.rmc
>>> +++ b/documentation/README.rmc
>>> @@ -153,7 +153,10 @@ up first, so it must be “INSTALLER.CONFIG”.
>>> Enable RMC Feature
>>> --------------------------------------------------------------------------------
>>> To Enable RMC feature in build, add the below line in a conf file:
>>> -EFI_PROVIDER="rmc-systemd-boot"
>>> +DISTRO_FEATURES_append = " rmc"
>>> +
>>> +Note: To ensure its whole functionality, RMC Feature overrides any bootloader
>>> +selected in EFI_PROVIDER with its own bootloader.
>>>
>>
>> It's ok to do this for a first implementation, but it needs to be made
>> very clear to the user that this isn't how it's supposed to work and is
>> just a temporary state of affairs.  i.e. the RMC feature isn't meant ot
>> override a bootloader, just augment it despite what the initial
>> implementation does.
>>
> 
> Tom,
> Installer and bootloader are key parts in RMC feature. The point to group pieces together with a distro switch is to treat them as a whole. That means we do want RMC to override any settings if we care the functionality of this feature. Otherwise, we lose the point to have a feature switch. 
> 
> But what’s to be improved is RMC should provide user a way to specify a RMC-supported bootloader once we add 2nd or more bootloaders with RMC support. This won’t change our argument on overriding however, regard to a single or a set of supported bootloader to replace others.
> 

>From a user's perspective, it seems simple - if I want to use
systemd-boot, I just say

EFI_PROVIDER = "systemd-boot"

If I specify that I want to use the rmc support with that, I add

DISTRO_FEATURES_append = " rmc" according to your latest scheme, in
order to do that.

If I don't add the rmc DISTRO_FEATURES_append, I just get the same
bootloader but no rmc support.

If I specify

EFI_PROVIDER = "grub-efi"

and

DISTRO_FEATURES_append = " rmc"

I'd expect rmc support to be added, if the bootloader supports it.  If
not, no harm done, I just get the bootloader as I always have.

That would be the same no matter how many bootloaders there are, so it's
completely extensible.

Not sure why you need to even say anything about overriding bootloaders
or how you happen to implement it now.

Tom


> I will check how distro features in OE to handle this case. (overriding, warning, failing build, etc)
> 
> Or we should tell user our implementation with systemd-boot is as a reference. They actually can have their bootloader, installer and other pieces, grouped in their way, as another “feature" based on RMC. Do you think this twist in statement could help?
> 
> Before we have a settlement, I will put this in V3 “This is a temporary solution so far. We are investigating a better way to let RMC notify user it can’t be full functional in a conflicted configuration than simply overriding any setting.”
> 
> Thanks
> 
>> Thanks,
>>
>> Tom
>