[systemd-devel] [PATCH 5/5] Auto detect both x64 and ia32 boot*.efi payloads

[Darren Hart]

On Wed, 2013-08-21 at 22:25 +0200, Kay Sievers wrote:
> On Wed, Aug 21, 2013 at 10:16 PM, Darren Hart <dvhart at linux.intel.com> wrote:
> > On Wed, 2013-08-21 at 21:31 +0200, Kay Sievers wrote:
> >> On Wed, Aug 21, 2013 at 9:26 PM, Darren Hart <dvhart at linux.intel.com> wrote:
> >> > On Wed, 2013-08-21 at 20:59 +0200, Kay Sievers wrote:
> >> >> On Wed, Aug 21, 2013 at 8:13 PM, Darren Hart <dvhart at linux.intel.com> wrote:
> >> >> > The EFI specification documents /EFI/BOOT/bootx64.efi for x86_64
> >> >> > machines and /EFI/BOOT/bootia32.efi for ia32 machines. Update the auto
> >> >> > detection to allow for both.
> >> >> >
> >> >> > Signed-off-by: Darren Hart <dvhart at linux.intel.com>
> >> >> > ---
> >> >> >  src/efi/gummiboot.c | 5 ++++-
> >> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/src/efi/gummiboot.c b/src/efi/gummiboot.c
> >> >> > index 9f2f31d..32e6f32 100644
> >> >> > --- a/src/efi/gummiboot.c
> >> >> > +++ b/src/efi/gummiboot.c
> >> >> > @@ -2199,7 +2199,10 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
> >> >> >          config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> >> >> >                                       L"auto-efi-shell", 's', L"EFI Shell", L"\\shellx64.efi");
> >> >> >          config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> >> >> > -                                     L"auto-efi-default", '\0', L"EFI Default Loader", L"\\EFI\\BOOT\\BOOTX64.EFI");
> >> >> > +                                     L"auto-efi-default-64", '\0', L"EFI Default Loader (x64)", L"\\EFI\\BOOT\\BOOTX64.EFI");
> >> >> > +        config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> >> >> > +                                     L"auto-efi-default-32", '\0', L"EFI Default Loader (ia32)", L"\\EFI\\BOOT\\BOOTIA32.EFI");
> >> >> > +
> >> >>
> >> >> Does it make sense to show both? Can EFI even start the non-native one?
> >> >>
> >> >> Shouldn't we just look for the "native" one, by composing the string
> >> >> at compile time?
> >> >
> >> > Hrm, good point. In my environment, I only have OR the other, and no
> >> > distros currently support IA32 EFI :-) But they should! So, yes, this
> >> > should be improved as you say. Should this just be an #ifdef/#else? on
> >> > the arch as is done for the ia32 tsc asm read function
> >>
> >> MACHINE_TYPE_NAME might just work. It's a define from the build system.
> >
> > OK, hoping you can save me some time here. I'm trying something like
> > this (I believe the filenames are case insensitive?):
> 
> Yeah, it's all on FAT, has no case, really.
> 
> > $ git diff
> > diff --git a/src/efi/gummiboot.c b/src/efi/gummiboot.c
> > index 32e6f32..edfab72 100644
> > --- a/src/efi/gummiboot.c
> > +++ b/src/efi/gummiboot.c
> > @@ -2196,12 +2196,16 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
> >          /* if we find some well-known loaders, add them to the end of the list */
> >          config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> >                                       L"auto-windows", 'w', L"Windows Boot Manager", L"\\EFI\\Microsoft\\Boot\\bootmgfw.efi");
> > +
> > +        s = PoolPrint(L"\\shell%s.efi", MACHINE_TYPE_NAME);
> >          config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> > -                                     L"auto-efi-shell", 's', L"EFI Shell", L"\\shellx64.efi");
> > -        config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> > -                                     L"auto-efi-default-64", '\0', L"EFI Default Loader (x64)", L"\\EFI\\BOOT\\BOOTX64.EFI");
> > +                                     L"auto-efi-shell", 's', L"EFI Shell", s);
> > +        FreePool(s);
> > +
> > +        s = PoolPrint(L"EFI Default Loader", L"\\EFI\\BOOT\\BOOT%s.EFI", MACHINE_TYPE_NAME);
> >          config_entry_add_loader_auto(&config, loaded_image->DeviceHandle, root_dir, loaded_image_path,
> > -                                     L"auto-efi-default-32", '\0', L"EFI Default Loader (ia32)", L"\\EFI\\BOOT\\BOOTIA32.EFI");
> > +                                     L"auto-efi-default", '\0', L"EFI Default Loader", s);
> > +        FreePool(s);
> >
> >          config_entry_add_osx(&config);
> >          efivar_set(L"LoaderEntriesAuto", config.entries_auto, FALSE);
> >
> >
> >
> > But the build complains about MACHINE_TYPE_NAME not being defined:
> >
> > $ make
> > make --no-print-directory all-am
> >   CC       src/efi/gummiboot.o
> > src/efi/gummiboot.c: In function ‘efi_main’:
> > src/efi/gummiboot.c:2200:41: error: ‘MACHINE_TYPE_NAME’ undeclared (first use in this function)
> >          s = PoolPrint(L"\\shell%s.efi", MACHINE_TYPE_NAME);
> >                                          ^
> > src/efi/gummiboot.c:2200:41: note: each undeclared identifier is reported only once for each function it appears in
> > make[1]: *** [src/efi/gummiboot.o] Error 1
> > make: *** [all] Error 2
> >
> >
> > I see it in the gummiboot_CFLAGS, what do I have to do to get that to
> > src/efi/gummiboot.c?
> 
> Patch attached. Does this work? Only compile-tested ...
> 
> Kay

Looks good and mostly matches what I ended up with (you modified
CPPFLAGS instead of just the CFLAGS, either seems to work). I'll include
this in my final testing once I sort out the other two patches and
include a testing report in the V2 submission.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel