Re: [meta-inte][rmc][PATCH v2 1/1] rmc: add support for alternative EFI bootloaders

[Todor Minchev <todor.minchev@linux.intel.com>]

On Thu, 2016-12-15 at 10:09 -0800, Cal Sullivan wrote:
> 
> On 12/13/2016 04:50 PM, Todor Minchev wrote:
> > 
> > On Tue, 2016-12-13 at 16:22 -0800, Jianxun Zhang wrote:
> > > 
> > > > 
> > > > On Dec 13, 2016, at 2:56 PM, Todor Minchev <todor.minchev@linux
> > > > .intel.com> wrote:
> > > > 
> > > > RMC was previously configured to work only with the systemd-
> > > > boot EFI
> > > > bootloader. With this commit we can specify alternative
> > > > bootloaders by
> > > > setting the RMC_BOOTLOADER variable in local.conf. If
> > > > RMC_BOOTLOADER is
> > > > not set systemd-boot will be used by default.
> > > > 
> > > > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > > > ---
> > > > Remove references to grub-efi and gummiboot from V1.
> > > > 
> > > > classes/{rmc-systemd-boot.bbclass => rmc-boot.bbclass} |  5
> > > > +++--
> > > > conf/machine/include/meta-intel.inc                    |  2 +-
> > > > documentation/rmc/README                               | 16
> > > > +++++++++++++---
> > > > 3 files changed, 17 insertions(+), 6 deletions(-)
> > > > rename classes/{rmc-systemd-boot.bbclass => rmc-boot.bbclass}
> > > > (73%)
> > > > 
> > > > diff --git a/classes/rmc-systemd-boot.bbclass b/classes/rmc-
> > > > boot.bbclass
> > > > similarity index 73%
> > > > rename from classes/rmc-systemd-boot.bbclass
> > > > rename to classes/rmc-boot.bbclass
> > > > index ad2cf10..a1f2093 100644
> > > > --- a/classes/rmc-systemd-boot.bbclass
> > > > +++ b/classes/rmc-boot.bbclass
> > > > @@ -1,9 +1,10 @@
> > > > -# rmc-systemd-boot bbclass
> > > > +# rmc-boot bbclass
> > > > # Deploy central RMC database file to ESP
> > > > 
> > > > IMAGE_INSTALL_append = " rmc"
> > > > +RMC_BOOTLOADER ?= "systemd-boot”
> > > Maybe this is what we could have now without bothering OE. It is
> > > better than the corrent code at the cost of another variable to
> > > user. I hope in the future we could get rid of  the dependency to
> > > EFI_PROVIDER (e.g. bz10084).
> > > 
> > > Also refer to my comment for the document change at the below.
> > > > 
> > > > -inherit systemd-boot
> > > > +inherit ${RMC_BOOTLOADER}
> > > > 
> > > > do_bootimg[depends] += "${MLPREFIX}rmc-db:do_deploy"
> > > > 
> > > > diff --git a/conf/machine/include/meta-intel.inc
> > > > b/conf/machine/include/meta-intel.inc
> > > > index c7555ce..fd0a792 100644
> > > > --- a/conf/machine/include/meta-intel.inc
> > > > +++ b/conf/machine/include/meta-intel.inc
> > > > @@ -32,7 +32,7 @@ MACHINE_ESSENTIAL_EXTRA_RDEPENDS_append = "${
> > > > @bb.utils.contains('MACHINE_FEATURE
> > > > # merge the microcode data in the final initrd image.
> > > > INITRD_LIVE_prepend = "${@bb.utils.contains('MACHINE_FEATURES',
> > > > 'intel-ucode', '${DEPLOY_DIR_IMAGE}/microcode.cpio ', '', d)}"
> > > > 
> > > > -EFI_PROVIDER ?= "rmc-systemd-boot"
> > > > +EFI_PROVIDER ?= "rmc-boot"
> > > > 
> > > > # Add general MACHINEOVERRIDE for meta-intel
> > > > MACHINEOVERRIDES =. "intel-x86-common:"
> > > > diff --git a/documentation/rmc/README
> > > > b/documentation/rmc/README
> > > > index 2427ffd..dbee6b6 100644
> > > > --- a/documentation/rmc/README
> > > > +++ b/documentation/rmc/README
> > > > @@ -165,14 +165,24 @@ steps still can override results from
> > > > this hook for boot entries and KBOOTPARAM.
> > > > 
> > > > Enable RMC Feature
> > > > -------------------------------------------------------------
> > > > -------------------
> > > > -To Enable RMC feature in build, add the below lines in a conf
> > > > file:
> > > > +To enable the RMC feature please add the following variables
> > > > to your local.conf.
> > > > +
> > > > DISTRO_FEATURES_append = " rmc"
> > > > -EFI_PROVIDER = "rmc-systemd-boot"
> > > > +EFI_PROVIDER = "rmc-boot"
> > > > +
> > > > +The default EFI bootloader used with RMC is systemd-boot. To
> > > > change the default
> > > > +bootloader please overwrite the RMC_BOOTLOADER variable in
> > > > your local.conf
> > > > 
> > > > Note:
> > > > Image could be still bootable if you only have either of two
> > > > lines, but RMC
> > > > feature won't be fully functional.
> > > > 
> > > > +To install only the RMC client with the systemd-boot
> > > > bootloader without
> > > > +including a default RMC database file, add the following lines
> > > > to your
> > > > +local.conf:
> > > > +
> > > > +EFI_PROVIDER = "systemd-boot"
> > > > +IMAGE_INSTALL_append = " rmc”
> > > I think this use case could confuse for user without much
> > > benefit. And actually they still can set EFI_PROVIDER to any
> > > available efi bootloaders to get this effect as long as it is not
> > > “rmc-boot”, right?
> > > Maybe we should just say “you won’t get rmc database deployed if
> > > you set EFI_PROVIDER to any values not rmc-boot."
> > Yes, this can be another way to phrase this.
> Is there going to be a v3 or is it sufficient as-is?

This is OK for now. I will reword the README after we add full RMC
support to grub-efi.
 
> 
> Thanks,
> Cal
> > 
> > 
> > > 
> > > > 
> > > > Examples
> > > > @@ -190,7 +200,7 @@ EXAMPLE 1: Support a new board type:
> > > > (1) enable the feature and do a build to get a live-boot image
> > > > by adding these
> > > >      lines in conf/local.conf:
> > > >      DISTRO_FEATURES_append = " rmc"
> > > > -    EFI_PROVIDER = "rmc-systemd-boot"
> > > > +    EFI_PROVIDER = "rmc-boot"
> > > > 
> > > > (2) flash the image to a USB stick and boot it on your board
> > > >