[U-Boot] [PATCH 14/18] efi: stub: Pass EFI GOP information to U-Boot payload

[Alexander Graf <agraf@suse.de>]

On 06/11/2018 09:44 AM, Bin Meng wrote:
> Hi Alex,
>
> On Mon, Jun 11, 2018 at 3:34 PM, Alexander Graf <agraf@suse.de> wrote:
>> On 06/11/2018 08:01 AM, Bin Meng wrote:
>>> Hi Alex,
>>>
>>> On Mon, Jun 11, 2018 at 1:52 PM, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>> On 11.06.18 01:29, Bin Meng wrote:
>>>>> On Mon, Jun 11, 2018 at 3:16 AM, Alexander Graf <agraf@suse.de> wrote:
>>>>>>
>>>>>> On 10.06.18 15:25, Bin Meng wrote:
>>>>>>> If UEFI BIOS has the graphics output protocol (GOP), let's pass its
>>>>>>> information to U-Boot payload so that U-Boot can utilize it (eg:
>>>>>>> an EFI framebuffer driver).
>>>>>>>
>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>> Why can't the FB drive determine all of this on its own and just fail
>>>>>> probe if no GOP protocol can be found?
>>>>>>
>>>>> It cannot. Once U-Boot payload is running, the boot services are gone.
>>>>> There is no way to determine the GOP protocol.
>>>> Interesting. Is there a particular reason you're not preserving boot
>>>> services?
>>>>
>>> This is EFI payload support with CONFIG_EFI_STUB. Preserving boot
>>> services is EFI application, with CONFIG_EFI_APP. For example, see
>>> serial_efi.c which is the serial driver that uses EFI's boot services
>>> to output characters on the serial port.
>>
>> Oh, I see. That makes sense now.
>>
>> Do people actually need CONFIG_EFI_STUB then?
> I think you wanted to say: Do people actually need CONFIG_EFI_APP
> then? CONFIG_EFI_STUB is more useful than CONFIG_EFI_APP. The
> application support is very limited.
>
> As the README.u-boot_on_efi says:
>
> Running U-Boot on EFI is useful in several situations:
>
> - You have EFI running on a board but U-Boot does not natively support it
> fully yet. You can boot into U-Boot from EFI and use that until U-Boot is
> fully ported
>
> - You need to use an EFI implementation (e.g. UEFI) because your vendor
> requires it in order to provide support
>
> - You plan to use coreboot to boot into U-Boot but coreboot support does
> not currently exist for your platform. In the meantime you can use U-Boot
> on EFI and then move to U-Boot on coreboot when ready
>
> - You use EFI but want to experiment with a simpler alternative like U-Boot
>
> Right now, I have one Intel SkyLake board, and before I get a native
> U-Boot up and running as the 1st stage bootloader on this board, I can
> run U-Boot as the EFI payload to experiment something, which is very
> handy.

Oh, I see. So it's mostly used as bringup aid. In that case I agree that 
the stub bit makes a lot of sense.

The one thing that really bites us with the stub and different bitness 
is when you want to use efi_api.h again from within U-Boot. The obvious 
example is a 32bit U-Boot built as 64bit payload, allowing 32bit UEFI 
applications to run inside.

Do you think we could just agree on efi.h to not consume any stub config 
options and just determine things from its build environment instead? 
That way it automatically adapts according to the -m32/-m64 build flag 
for the stub and we would not need to worry about the stub in generic code.

Something like the patch below? If yes, I'll resend it with proper 
indentation :).


Alex


diff --git a/include/efi.h b/include/efi.h
index 98bddbac1a..5e1e8ac78c 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -19,12 +19,12 @@
  #include <linux/string.h>
  #include <linux/types.h>

-#if CONFIG_EFI_STUB_64BIT || (!defined(CONFIG_EFI_STUB) && 
defined(__x86_64__))
-/* EFI uses the Microsoft ABI which is not the default for GCC */
+/* EFI on x86_64 uses the Microsoft ABI which is not the default for GCC */
+#ifdef __x86_64__
  #define EFIAPI __attribute__((ms_abi))
  #else
  #define EFIAPI asmlinkage
-#endif
+#endif /* __x86_64__ */

  struct efi_device_path;

@@ -32,16 +32,7 @@ typedef struct {
      u8 b[16];
  } efi_guid_t;

-#define EFI_BITS_PER_LONG    BITS_PER_LONG
-
-/*
- * With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64. EFI_STUB is set
- * in lib/efi/Makefile, when building the stub.
- */
-#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
-#undef EFI_BITS_PER_LONG
-#define EFI_BITS_PER_LONG    64
-#endif
+#define EFI_BITS_PER_LONG    (sizeof(long) * 8)

  /* Bit mask for EFI status code with error */
  #define EFI_ERROR_MASK (1UL << (EFI_BITS_PER_LONG - 1))