| Message ID | 1528637118-32739-15-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: efi: Fixes and enhancements to application and payload support | expand |
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? Alex
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. Regards, Bin
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? It might be quite handy to reuse UEFI block interfaces for example. It might also be useful to preserve (parts of?) the memory map so we don't overwrite random SMM code for example. Maybe I misunderstand the purpose of the UEFI U-Boot support though ;). Alex
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. > It might be quite handy to reuse UEFI block interfaces for example. It > might also be useful to preserve (parts of?) the memory map so we don't > overwrite random SMM code for example. > > Maybe I misunderstand the purpose of the UEFI U-Boot support though ;). Looks there is some misunderstanding around the EFI payload support. Regards, Bin
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? > >> It might be quite handy to reuse UEFI block interfaces for example. It >> might also be useful to preserve (parts of?) the memory map so we don't >> overwrite random SMM code for example. >> >> Maybe I misunderstand the purpose of the UEFI U-Boot support though ;). > Looks there is some misunderstanding around the EFI payload support. Yes, I have to admit that I never really cared enough about it to read its documentation though :). Alex
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. > >> >>> It might be quite handy to reuse UEFI block interfaces for example. It >>> might also be useful to preserve (parts of?) the memory map so we don't >>> overwrite random SMM code for example. >>> >>> Maybe I misunderstand the purpose of the UEFI U-Boot support though ;). >> >> Looks there is some misunderstanding around the EFI payload support. > > > Yes, I have to admit that I never really cared enough about it to read its > documentation though :). Yep, this sometimes confuses people :) Regards, Bin
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))
Hi Alex, On Mon, Jun 11, 2018 at 4:33 PM, Alexander Graf <agraf@suse.de> wrote: > 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. > Great, this way is smarter! > Something like the patch below? If yes, I'll resend it with proper > indentation :). > Yes, and I belive we will also need remove -DEFI_STUB in lib/efi/Makefile. > > 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)) > Regards, Bin
Hi, On 10 June 2018 at 05:25, Bin Meng <bmeng.cn@gmail.com> 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> > --- > > include/efi.h | 35 +++++++++++++++++++++++++++++++++++ > lib/efi/efi_stub.c | 15 +++++++++++++++ > 2 files changed, 50 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> Note (for Alex) that the stub can use boot services but the payload (U-Boot itself) cannot. It is a different mode from U-Boot as an EFI application. - Simon
diff --git a/include/efi.h b/include/efi.h index 98bddba..e08a913 100644 --- a/include/efi.h +++ b/include/efi.h @@ -242,6 +242,7 @@ struct efi_open_protocol_info_entry { enum efi_entry_t { EFIET_END, /* Signals this is the last (empty) entry */ EFIET_MEMORY_MAP, + EFIET_GOP_MODE, /* Number of entries */ EFIET_MEMORY_COUNT, @@ -298,6 +299,40 @@ struct efi_entry_memmap { struct efi_mem_desc desc[]; }; +/** + * struct efi_entry_gopmode - a GOP mode table passed to U-Boot + * + * @fb_base: EFI's framebuffer base address + * @fb_size: EFI's framebuffer size + * @info_size: GOP mode info structure size + * @info: Start address of the GOP mode info structure + */ +struct efi_entry_gopmode { + efi_physical_addr_t fb_base; + /* + * Not like the ones in 'struct efi_gop_mode' which are 'unsigned + * long', @fb_size and @info_size have to be 'u64' here. As the EFI + * stub codes may have different bit size from the U-Boot payload, + * using 'long' will cause mismatch between the producer (stub) and + * the consumer (payload). + */ + u64 fb_size; + u64 info_size; + /* + * We cannot directly use 'struct efi_gop_mode_info info[]' here as + * it causes compiler to complain: array type has incomplete element + * type 'struct efi_gop_mode_info'. + */ + struct /* efi_gop_mode_info */ { + u32 version; + u32 width; + u32 height; + u32 pixel_format; + u32 pixel_bitmask[4]; + u32 pixels_per_scanline; + } info[]; +}; + static inline struct efi_mem_desc *efi_get_next_mem_desc( struct efi_entry_memmap *map, struct efi_mem_desc *desc) { diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index 09023a2..4819373 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -274,6 +274,9 @@ efi_status_t efi_main(efi_handle_t image, struct efi_system_table *sys_table) struct efi_boot_services *boot = sys_table->boottime; struct efi_mem_desc *desc; struct efi_entry_memmap map; + struct efi_gop *gop; + struct efi_entry_gopmode mode; + efi_guid_t efi_gop_guid = EFI_GOP_GUID; efi_uintn_t key, desc_size, size; efi_status_t ret; u32 version; @@ -312,6 +315,18 @@ efi_status_t efi_main(efi_handle_t image, struct efi_system_table *sys_table) if (ret) return ret; + ret = boot->locate_protocol(&efi_gop_guid, NULL, (void **)&gop); + if (ret) { + puts(" GOP unavailable\n"); + } else { + mode.fb_base = gop->mode->fb_base; + mode.fb_size = gop->mode->fb_size; + mode.info_size = gop->mode->info_size; + add_entry_addr(priv, EFIET_GOP_MODE, &mode, sizeof(mode), + gop->mode->info, + sizeof(struct efi_gop_mode_info)); + } + ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version); if (ret) { printhex2(ret);
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> --- include/efi.h | 35 +++++++++++++++++++++++++++++++++++ lib/efi/efi_stub.c | 15 +++++++++++++++ 2 files changed, 50 insertions(+)