Re: [PATCH v5 8/8] util/grub.d: Activate bli module on EFI

[Daniel Kiper]

On Tue, Apr 11, 2023 at 05:51:42PM +0200, Oliver Steffen wrote:
> On Tue, Apr 11, 2023 at 4:10 PM Oliver Steffen <osteffen@redhat.com> wrote:
>      On Wed, Apr 5, 2023 at 5:57 PM Daniel Kiper <dkiper@net-space.pl>
>      wrote:
>           On Thu, Mar 30, 2023 at 01:18:13PM +0200, Oliver Steffen
>           wrote:
>           > Add a new configuration drop-in file that loads the bli
>           module and runs
>           > the command if booting on the EFI platform.
>           >
>           > Signed-off-by: Oliver Steffen <osteffen@redhat.com>
>           > ---
>           >  Makefile.util.def     |  6 ++++++
>           >  util/grub.d/25_bli.in | 25 +++++++++++++++++++++++++
>           >  2 files changed, 31 insertions(+)
>           >  create mode 100644 util/grub.d/25_bli.in
>           >
>           > diff --git a/Makefile.util.def b/Makefile.util.def
>           > index beaef1168..d90e59b94 100644
>           > --- a/Makefile.util.def
>           > +++ b/Makefile.util.def
>           > @@ -510,6 +510,12 @@ script = {
>           >    condition = COND_HOST_LINUX;
>           >  };
>           >
>           > +script = {
>           > +  name = '25_bli';
>           > +  common = util/grub.d/25_bli.in;
>           > +  installdir = grubconf;
>           > +};
>           > +
>           >  script = {
>           >    name = '30_os-prober';
>           >    common = util/grub.d/30_os-prober.in;
>           > diff --git a/util/grub.d/25_bli.in b/util/grub.d/
>           25_bli.in
>           > new file mode 100644
>           > index 000000000..ba0f466f0
>           > --- /dev/null
>           > +++ b/util/grub.d/25_bli.in
>           > @@ -0,0 +1,25 @@
>           > +#!/usr/bin/sh
>           > +set -e
>           > +
>           > +# grub-mkconfig helper script.
>           > +# Copyright (C) 2023  Free Software Foundation, Inc.
>           > +#
>           > +# GRUB is free software: you can redistribute it and/or
>           modify
>           > +# it under the terms of the GNU General Public License
>           as published by
>           > +# the Free Software Foundation, either version 3 of the
>           License, or
>           > +# (at your option) any later version.
>           > +#
>           > +# GRUB is distributed in the hope that it will be
>           useful,
>           > +# but WITHOUT ANY WARRANTY; without even the implied
>           warranty of
>           > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. 
>           See the
>           > +# GNU General Public License for more details.
>           > +#
>           > +# You should have received a copy of the GNU General
>           Public License
>           > +# along with GRUB.  If not, see <http://www.gnu.org/
>           licenses/>.
>           > +
>           > +cat << EOF
>           > +if [ "\$grub_platform" = "efi" ]; then
>           > +  insmod bli
>           > +  bli
>
>           When I see this I question myself: do we really need a bli
>           command?
>           Why do not simplify things and do what is needed when
>           module is loaded?
>
>
>      We could do everything from the module init function.  But then this
>      module
>      does not provide a command anymore and probably should not live in
>      grub-core/commands.  Where should this go?

We do not have separate place for such modules. Everything goes to the
grub-core/commands. E.g. the tpm module does not provide any commands
and is in the grub-core/commands directory.

> Same for the documentation for it.  The manual does not have a "modules"
> section.

It does not need to be a part of "commands" section. I would just create
a separate section for bli module. I expect it will be expanded in the
future.

Daniel