Re: [PATCH] grub-install: check for arm-efi as a default target

[Steve McIntyre]

Hey Leif,

On Thu, Feb 21, 2019 at 11:41:10AM +0000, Leif Lindholm wrote:
>On Mon, Feb 11, 2019 at 02:42:34AM +0000, Steve McIntyre wrote:
>> Much like on x86, we can work out if the system is running on top of
>> EFI firmware. If so, return "arm-efi". If not, fall back to
>> "arm-uboot" as previously.
>
>Right, this clearly needs a fix.

Yup!

>> Heavily inspired by the existing code for x86.
>
>Mmm. I would much prefer if we could break out the efi test in a
>separate helper function. And clean it up while we're at it.

ACK. I was looking for a quick change, but I'll be less lazy. :-)

...

>>  const char *
>> +grub_install_get_default_arm_platform (void)
>> +{
>> +  /*
>> +     On Linux, we need the efivars kernel modules.
>> +     If no EFI is available this module just does nothing
>> +     besides a small hello and if we detect efi we'll load it
>> +     anyway later. So it should be safe to
>> +     try to load it here.
>> +   */
>> +  grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", 
>> NULL },
>> +                           NULL, NULL, "/dev/null");
>
>So, I guess this is a "safe" operation. But efivars isn't the
>interface that should be used these days - efivarfs is.
>The efivars library will always use efivarfs
>(mounted on /sys/firmware/efi/efivars) in preference to efivars
>(available through /sys/firmware/efi/vars).
>
>Since it's safe, we may leave it in for someone running bleeding edge
>grub on an ancient system, or just using a misconfigured kernel.
>But the comment is misleading. I would suggest changing it to
>something like:
>
>  /*
>      Linux uses efivarfs (mounted on /sys/firmware/efi/efivars)
>      to access the EFI variable store.
>      Some legacy systems may still use the deprecated efivars
>      interface (accessed through /sys/firmware/efi/vars).
>      Where both are present, libefivar will use the former in
>      preference, so attempting to load efivars will not interfere
>      with later operations.
>  */

ACK. Using your text. :-)

Patch v2 on its way in a mo.

-- 
Steve McIntyre, Cambridge, UK.                                address@hidden
We don't need no education.
We don't need no thought control.