[U-Boot] [PATCH 4/6] x86: efi: Add EFI loader support for x86

[Alexander Graf]

On 10/08/2016 14:56, Simon Glass wrote:
> Hi Alex,
> 
> On 10 August 2016 at 05:49, Alexander Graf <agraf at suse.de> wrote:
>> On 08/07/2016 01:23 AM, Simon Glass wrote:
>>>
>>> Add the required pieces to support the EFI loader on x86.
>>>
>>> Since U-Boot only builds for 32-bit on x86, only a 32-bit EFI application
>>> is supported. If a 64-bit kernel must be booted, U-Boot supports this
>>> directly using FIT (see doc/uImage.FIT/kernel.its). U-Boot can act as a
>>> payload for both 32-bit and 64-bit EFI.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> ---
>>>
>>>   arch/x86/cpu/u-boot.lds       | 36 +++++++++++++++++++++++++++++++++++-
>>>   arch/x86/lib/Makefile         |  1 +
>>>   arch/x86/lib/sections.c       | 12 ++++++++++++
>>>   include/efi.h                 |  3 ++-
>>>   lib/efi_loader/efi_boottime.c |  9 +++++++++
>>>   lib/efi_loader/efi_runtime.c  |  4 ++++
>>>   6 files changed, 63 insertions(+), 2 deletions(-)
>>>   create mode 100644 arch/x86/lib/sections.c
>>>
>>> diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
>>> index 36f59ea..cca536b 100644
>>> --- a/arch/x86/cpu/u-boot.lds
>>> +++ b/arch/x86/cpu/u-boot.lds
>>> @@ -28,7 +28,10 @@ SECTIONS
>>>         }
>>>         . = ALIGN(4);
>>> -       .rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
>>> +       .rodata : {
>>> +               *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*)))
>>> +               KEEP(*(.rodata.efi.init));
>>> +       }
>>>         . = ALIGN(4);
>>>         .data : { *(.data*) }
>>> @@ -40,6 +43,37 @@ SECTIONS
>>>         .got : { *(.got*) }
>>>         . = ALIGN(4);
>>> +
>>> +       .__efi_runtime_start : {
>>> +               *(.__efi_runtime_start)
>>> +       }
>>> +
>>> +       .efi_runtime : {
>>> +               *(efi_runtime_text)
>>> +               *(efi_runtime_data)
>>> +       }
>>> +
>>> +       .__efi_runtime_stop : {
>>> +               *(.__efi_runtime_stop)
>>> +       }
>>> +
>>> +       .efi_runtime_rel_start :
>>> +       {
>>> +               *(.__efi_runtime_rel_start)
>>> +       }
>>> +
>>> +       .efi_runtime_rel : {
>>> +               *(.relefi_runtime_text)
>>> +               *(.relefi_runtime_data)
>>> +       }
>>> +
>>> +       .efi_runtime_rel_stop :
>>> +       {
>>> +               *(.__efi_runtime_rel_stop)
>>> +       }
>>> +
>>> +       . = ALIGN(4);
>>> +
>>>         __data_end = .;
>>>         __init_end = .;
>>>   diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
>>> index e17f0bb..e46e7f1 100644
>>> --- a/arch/x86/lib/Makefile
>>> +++ b/arch/x86/lib/Makefile
>>> @@ -28,6 +28,7 @@ obj-y += pirq_routing.o
>>>   obj-y += relocate.o
>>>   obj-y += physmem.o
>>>   obj-$(CONFIG_X86_RAMTEST) += ramtest.o
>>> +obj-y  += sections.o
>>>   obj-y += sfi.o
>>>   obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
>>>   obj-y += string.o
>>> diff --git a/arch/x86/lib/sections.c b/arch/x86/lib/sections.c
>>> new file mode 100644
>>> index 0000000..6455e0f
>>> --- /dev/null
>>> +++ b/arch/x86/lib/sections.c
>>> @@ -0,0 +1,12 @@
>>> +/*
>>> + * Copyright 2013 Albert ARIBAUD <albert.u.boot at aribaud.net>
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +char __efi_runtime_start[0]
>>> __attribute__((section(".__efi_runtime_start")));
>>> +char __efi_runtime_stop[0]
>>> __attribute__((section(".__efi_runtime_stop")));
>>> +char __efi_runtime_rel_start[0]
>>> +               __attribute__((section(".__efi_runtime_rel_start")));
>>> +char __efi_runtime_rel_stop[0]
>>> +               __attribute__((section(".__efi_runtime_rel_stop")));
>>> diff --git a/include/efi.h b/include/efi.h
>>> index 1dbc3b7..47b351f 100644
>>> --- a/include/efi.h
>>> +++ b/include/efi.h
>>> @@ -15,6 +15,7 @@
>>>   #ifndef _EFI_H
>>>   #define _EFI_H
>>>   +#include <linux/linkage.h>
>>>   #include <linux/string.h>
>>>   #include <linux/types.h>
>>>   @@ -22,7 +23,7 @@
>>>   /* EFI uses the Microsoft ABI which is not the default for GCC */
>>>   #define EFIAPI __attribute__((ms_abi))
>>>   #else
>>> -#define EFIAPI
>>> +#define EFIAPI asmlinkage
>>
>>
>> Ah, this should trigger what I mentioned in my previous email. Since
>> linkage.h is now included in efi.h, we no longer have to include the header
>> in bootefi.c explicitly. How about you just move this part of the patch
>> between patches 1/6 and 2/6?
> 
> OK will take a look.
> 
>>
>>
>>>   #endif
>>>     struct efi_device_path;
>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>> index 798b566..e027bd3 100644
>>> --- a/lib/efi_loader/efi_boottime.c
>>> +++ b/lib/efi_loader/efi_boottime.c
>>> @@ -39,6 +39,7 @@ static bool efi_is_direct_boot = true;
>>>    */
>>>   static struct efi_configuration_table EFI_RUNTIME_DATA
>>> efi_conf_table[1];
>>>   +#ifdef CONFIG_ARM
>>>   /*
>>>    * The "gd" pointer lives in a register on ARM and AArch64 that we
>>> declare
>>>    * fixed when compiling U-Boot. However, the payload does not know about
>>> that
>>> @@ -46,16 +47,20 @@ static struct efi_configuration_table EFI_RUNTIME_DATA
>>> efi_conf_table[1];
>>>    * EFI callback entry/exit.
>>>    */
>>>   static volatile void *efi_gd, *app_gd;
>>> +#endif
>>
>>
>> So is fs reserved for firmware use on x86?
> 
> No I don't think so. The correct approach would be to save and restore
> the FS register, but it seems to work this way, so I haven't taken it
> any further.

Can you make sure you save/restore %fs on x86 in the next round then?
Finding bugs like these is very hard once the code is in.

> This series was just an attempt to get the EFI loader working on x86.

Yes, and it's very very very much appreciated :).

> Since we have the 32/64-bit limitation on x86 and U-Boot runs in
> 32-bit at present, it's not going to be very useful yet IMO. But it is
> a start.

I agree. It's a really good start.


Alex