EFI framebuffer support for ARM and arm64

This patch redeclares efi_boot_services_t::install_configuration_table as a function pointer so that the boot service is callable as a function.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 include/linux/efi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/efi.h +1 -1

index e747eb08b2be..8ba6eb7863aa 100644
@@ -283,7 +283,7 @@ typedef struct {
 	void *register_protocol_notify;
 	void *locate_handle;
 	void *locate_device_path;
-	void *install_configuration_table;
+	efi_status_t (*install_configuration_table)(efi_guid_t *, void *);
 	void *load_image;
 	void *start_image;
 	void *exit;

Signature · 1 line

1.9.1

Quoted 3 lines · Ard Biesheuvel

This patch redeclares efi_boot_services_t::install_configuration_table as a function pointer so that the boot service is callable as a function.

Quoted 4 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---
 include/linux/efi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Any chance you could fold this patch into the one that actually uses install_configuration_table() on ARM? That way it's clear to see where the users are.

Quoted 16 lines

include/linux/efi.h +1 -2

index e747eb08b2be..8ba6eb7863aa 100644
@@ -283,7 +283,7 @@ typedef struct {
      void *register_protocol_notify;
      void *locate_handle;
      void *locate_device_path;
-     void *install_configuration_table;
+     efi_status_t (*install_configuration_table)(efi_guid_t *, void *);
      void *load_image;
      void *start_image;
      void *exit;
--

1.9.1

The Graphics Output Protocol code executes in the stub, so create a generic version based on the x86 version in libstub so that we can move all archs to it in subsequent patches. The new source file gop.c is added to the libstub build for all architectures, but is not actually included in any of the final images, since this patch does not wire it up yet.

Note that the generic version uses slightly different ways of casting the protocol methods and some other variables to the correct types, since such method calls are not loosely typed on ARM and arm64 as they are on x86.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/efi.h            |   4 +-
 arch/arm64/include/asm/efi.h          |   4 +-
 arch/x86/boot/compressed/eboot.h      |  74 ----
 arch/x86/include/asm/efi.h            |   5 +
 drivers/firmware/efi/libstub/Makefile |   2 +-
 drivers/firmware/efi/libstub/gop.c    | 354 ++++++++++++++++++++
 include/linux/efi.h                   |  87 ++++-
 7 files changed, 451 insertions(+), 79 deletions(-)

The efifb quirks handling based on DMI identification of the platform is specific to x86, so move it to x86 arch code.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/include/asm/efi.h  |  2 ++
 arch/x86/kernel/sysfb_efi.c | 15 +++++++++++++++
 drivers/video/fbdev/efifb.c | 15 ++++-----------
 3 files changed, 21 insertions(+), 11 deletions(-)

arch/x86/include/asm/efi.h +2 -0

index 7d5187b66108..790adbc3a64a 100644
@@ -154,6 +154,8 @@ static inline bool efi_runtime_supported(void)
 extern struct console early_efi_console;
 extern void parse_efi_setup(u64 phys_addr, u32 data_len);
 
+extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
+
 #ifdef CONFIG_EFI_MIXED
 extern void efi_thunk_runtime_setup(void);
 extern efi_status_t efi_thunk_set_virtual_address_map(

arch/x86/kernel/sysfb_efi.c +15 -0

index b285d4e8c68e..e21a8a7ddcff 100644
@@ -68,6 +68,21 @@ struct efifb_dmi_info efifb_dmi_list[] = {
 	[M_UNKNOWN] = { NULL, 0, 0, 0, 0, OVERRIDE_NONE }
 };
 
+void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
+{
+	int i;
+
+	for (i = 0; i < M_UNKNOWN; i++) {
+		if (efifb_dmi_list[i].base != 0 &&
+		    !strcmp(opt, efifb_dmi_list[i].optname)) {
+			si->lfb_base = efifb_dmi_list[i].base;
+			si->lfb_linelength = efifb_dmi_list[i].stride;
+			si->lfb_width = efifb_dmi_list[i].width;
+			si->lfb_height = efifb_dmi_list[i].height;
+		}
+	}
+}
+
 #define choose_value(dmivalue, fwvalue, field, flags) ({	\
 		typeof(fwvalue) _ret_ = fwvalue;		\
 		if ((flags) & (field))				\

drivers/video/fbdev/efifb.c +4 -11

index 95d293b7445a..dd594369b8a6 100644
@@ -8,6 +8,7 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/efi.h>
 #include <linux/errno.h>
 #include <linux/fb.h>
 #include <linux/platform_device.h>

@@ -15,7 +16,7 @@
 #include <linux/dmi.h>
 #include <linux/pci.h>
 #include <video/vga.h>
-#include <asm/sysfb.h>
+#include <asm/efi.h>
 
 static bool request_mem_succeeded = false;
 

@@ -85,21 +86,13 @@ static struct fb_ops efifb_ops = {
 static int efifb_setup(char *options)
 {
 	char *this_opt;
-	int i;
 
 	if (options && *options) {
 		while ((this_opt = strsep(&options, ",")) != NULL) {
 			if (!*this_opt) continue;
 
-			for (i = 0; i < M_UNKNOWN; i++) {
-				if (efifb_dmi_list[i].base != 0 &&
-				    !strcmp(this_opt, efifb_dmi_list[i].optname)) {
-					screen_info.lfb_base = efifb_dmi_list[i].base;
-					screen_info.lfb_linelength = efifb_dmi_list[i].stride;
-					screen_info.lfb_width = efifb_dmi_list[i].width;
-					screen_info.lfb_height = efifb_dmi_list[i].height;
-				}
-			}
+			efifb_setup_from_dmi(&screen_info, this_opt);
+
 			if (!strncmp(this_opt, "base:", 5))
 				screen_info.lfb_base = simple_strtoul(this_opt+5, NULL, 0);
 			else if (!strncmp(this_opt, "stride:", 7))

Signature · 1 line

1.9.1

(Cc'ing efifb maintainer and sysfb_efi.c author)

Quoted 9 lines · Ard Biesheuvel

The efifb quirks handling based on DMI identification of the platform is specific to x86, so move it to x86 arch code.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/include/asm/efi.h  |  2 ++
 arch/x86/kernel/sysfb_efi.c | 15 +++++++++++++++
 drivers/video/fbdev/efifb.c | 15 ++++-----------
 3 files changed, 21 insertions(+), 11 deletions(-)

I've left the patch content intact below. This looks like a worthwhile change to me. David, Peter, any comments?

Quoted 88 lines

arch/x86/include/asm/efi.h +2 -0

index 7d5187b66108..790adbc3a64a 100644
@@ -154,6 +154,8 @@ static inline bool efi_runtime_supported(void)
 extern struct console early_efi_console;
 extern void parse_efi_setup(u64 phys_addr, u32 data_len);
 
+extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
+
 #ifdef CONFIG_EFI_MIXED
 extern void efi_thunk_runtime_setup(void);
 extern efi_status_t efi_thunk_set_virtual_address_map(

arch/x86/kernel/sysfb_efi.c +15 -0

index b285d4e8c68e..e21a8a7ddcff 100644
@@ -68,6 +68,21 @@ struct efifb_dmi_info efifb_dmi_list[] = {
 	[M_UNKNOWN] = { NULL, 0, 0, 0, 0, OVERRIDE_NONE }
 };
 
+void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
+{
+	int i;
+
+	for (i = 0; i < M_UNKNOWN; i++) {
+		if (efifb_dmi_list[i].base != 0 &&
+		    !strcmp(opt, efifb_dmi_list[i].optname)) {
+			si->lfb_base = efifb_dmi_list[i].base;
+			si->lfb_linelength = efifb_dmi_list[i].stride;
+			si->lfb_width = efifb_dmi_list[i].width;
+			si->lfb_height = efifb_dmi_list[i].height;
+		}
+	}
+}
+
 #define choose_value(dmivalue, fwvalue, field, flags) ({	\
 		typeof(fwvalue) _ret_ = fwvalue;		\
 		if ((flags) & (field))				\

drivers/video/fbdev/efifb.c +4 -11

index 95d293b7445a..dd594369b8a6 100644
@@ -8,6 +8,7 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/efi.h>
 #include <linux/errno.h>
 #include <linux/fb.h>
 #include <linux/platform_device.h>

@@ -15,7 +16,7 @@
 #include <linux/dmi.h>
 #include <linux/pci.h>
 #include <video/vga.h>
-#include <asm/sysfb.h>
+#include <asm/efi.h>
 
 static bool request_mem_succeeded = false;
 

@@ -85,21 +86,13 @@ static struct fb_ops efifb_ops = {
 static int efifb_setup(char *options)
 {
 	char *this_opt;
-	int i;
 
 	if (options && *options) {
 		while ((this_opt = strsep(&options, ",")) != NULL) {
 			if (!*this_opt) continue;
 
-			for (i = 0; i < M_UNKNOWN; i++) {
-				if (efifb_dmi_list[i].base != 0 &&
-				    !strcmp(this_opt, efifb_dmi_list[i].optname)) {
-					screen_info.lfb_base = efifb_dmi_list[i].base;
-					screen_info.lfb_linelength = efifb_dmi_list[i].stride;
-					screen_info.lfb_width = efifb_dmi_list[i].width;
-					screen_info.lfb_height = efifb_dmi_list[i].height;
-				}
-			}
+			efifb_setup_from_dmi(&screen_info, this_opt);
+
 			if (!strncmp(this_opt, "base:", 5))
 				screen_info.lfb_base = simple_strtoul(this_opt+5, NULL, 0);
 			else if (!strncmp(this_opt, "stride:", 7))

Signature · 1 line

1.9.1

Since efifb can only be built directly into the kernel, drop the module specific includes and definitions. Drop some other includes we don't need as well.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 drivers/video/fbdev/efifb.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/video/fbdev/efifb.c +1 -5

index dd594369b8a6..f4c045c0051c 100644
@@ -6,15 +6,12 @@
  *
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/efi.h>
 #include <linux/errno.h>
 #include <linux/fb.h>
 #include <linux/platform_device.h>
 #include <linux/screen_info.h>
-#include <linux/dmi.h>
-#include <linux/pci.h>
 #include <video/vga.h>
 #include <asm/efi.h>
 

@@ -331,5 +328,4 @@ static struct platform_driver efifb_driver = {
 	.remove = efifb_remove,
 };
 
-module_platform_driver(efifb_driver);
-MODULE_LICENSE("GPL");
+builtin_platform_driver(efifb_driver);

Signature · 1 line

1.9.1

Since efifb can only be built directly into the kernel, drop the module specific includes and definitions. Drop some other includes we don't need as well.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 drivers/video/fbdev/efifb.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/video/fbdev/efifb.c +1 -5

index dd594369b8a6..f4c045c0051c 100644
@@ -6,15 +6,12 @@
  *
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/efi.h>
 #include <linux/errno.h>
 #include <linux/fb.h>
 #include <linux/platform_device.h>
 #include <linux/screen_info.h>
-#include <linux/dmi.h>
-#include <linux/pci.h>
 #include <video/vga.h>
 #include <asm/efi.h>
 

@@ -331,5 +328,4 @@ static struct platform_driver efifb_driver = {
 	.remove = efifb_remove,
 };
 
-module_platform_driver(efifb_driver);
-MODULE_LICENSE("GPL");
+builtin_platform_driver(efifb_driver);

Signature · 1 line

1.9.1

drivers/firmware/efi/libstub/arm32-stub.c +31 -0

index 495ebd657e38..0fbe00f13186 100644
@@ -9,6 +9,44 @@
 #include <linux/efi.h>
 #include <asm/efi.h>
 
+static const efi_guid_t screen_info_guid = LINUX_ARM32_SCREEN_INFO_TABLE_GUID;
+
+struct screen_info *alloc_screen_info(efi_system_table_t *sys_table_arg)
+{
+	struct screen_info *si;
+	efi_status_t status;
+
+	/*
+	 * Unlike on arm64, where we can directly fill out the screen_info
+	 * structure from the stub, we need to allocate a buffer to hold
+	 * its contents while we hand over to the kernel proper from the
+	 * decompressor.
+	 */
+	status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+				sizeof(*si), (void **)&si);
+
+	if (status != EFI_SUCCESS)
+		return NULL;
+
+	status = efi_call_early(install_configuration_table,
+				(efi_guid_t *)&screen_info_guid, si);
+	if (status == EFI_SUCCESS)
+		return si;
+
+	efi_call_early(free_pool, si);
+	return NULL;
+}
+
+void free_screen_info(efi_system_table_t *sys_table_arg, struct screen_info *si)
+{
+	if (!si)

Quoted 46 lines · Ard Biesheuvel

drivers/firmware/efi/libstub/arm32-stub.c +38 -0

index 495ebd657e38..0fbe00f13186 100644
@@ -9,6 +9,44 @@
 #include <linux/efi.h>
 #include <asm/efi.h>
 
+static const efi_guid_t screen_info_guid = LINUX_ARM32_SCREEN_INFO_TABLE_GUID;
+
+struct screen_info *alloc_screen_info(efi_system_table_t *sys_table_arg)
+{
+     struct screen_info *si;
+     efi_status_t status;
+
+     /*
+      * Unlike on arm64, where we can directly fill out the screen_info
+      * structure from the stub, we need to allocate a buffer to hold
+      * its contents while we hand over to the kernel proper from the
+      * decompressor.
+      */
+     status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+                             sizeof(*si), (void **)&si);
+
+     if (status != EFI_SUCCESS)
+             return NULL;
+
+     status = efi_call_early(install_configuration_table,
+                             (efi_guid_t *)&screen_info_guid, si);
+     if (status == EFI_SUCCESS)
+             return si;
+
+     efi_call_early(free_pool, si);
+     return NULL;
+}
+
+void free_screen_info(efi_system_table_t *sys_table_arg, struct screen_info *si)
+{
+     if (!si)
+             return;
+
+     efi_call_early(install_configuration_table,
+                    (efi_guid_t *)&screen_info_guid, NULL);
+     efi_call_early(free_pool, si);
+}
+

drivers/firmware/efi/libstub/arm32-stub.c line 52

Quoted 3 lines

drivers/firmware/efi/libstub/arm32-stub.c +0 -0

 efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
                               unsigned long *image_addr,
                               unsigned long *image_size,

So screen_info_guid should probably not be a 'const': you have to cast it away anyway, adding artificial linebreaks and uglifying the code. It's also a bad practice to cast away const-ness, it hinders move-consts-to-readonly-sections efforts.

Otherwise the series looks mostly good to me. Time for new v4.6 merges is short, but if Matt acks it I can still take it into tip:efi/core.

Quoted 4 lines

So screen_info_guid should probably not be a 'const': you have to cast it away anyway, adding artificial linebreaks and uglifying the code. It's also a bad practice to cast away const-ness, it hinders move-consts-to-readonly-sections efforts.

The problem here is that the UEFI spec never uses const qualifiers in its APIs for by-ref parameters that are obviously never modified by the caller, such as these GUIDs. [...]

Quoted 8 lines · Ard Biesheuvel

Quoted 4 lines

So screen_info_guid should probably not be a 'const': you have to cast it away anyway, adding artificial linebreaks and uglifying the code. It's also a bad practice to cast away const-ness, it hinders move-consts-to-readonly-sections efforts.

The problem here is that the UEFI spec never uses const qualifiers in its APIs for by-ref parameters that are obviously never modified by the caller, such as these GUIDs. [...]

Ah, ok. Two related thoughts came up:

1)

While I was looking at this code and was asking myself why the EFI runtime is generally invoked via a relatively fragile, non-type-checking vararg construct.

Wouldn't you be better off by explicitly defining all the API variants, and then internally calling the EFI runtime?

That would neatly solve such const artifacts as well.

So instead of:

drivers/firmware/efi/libstub/arm32-stub.c +2 -0

+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+                               sizeof(*si), (void **)&si);

we could have something like:

        status = efi_early__allocate_pool(EFI_RUNTIME_SERVICES_DATA, sizeof(*si), &si);

        ...

        efi_early__free_pool(si);

i.e. it would look a lot more like a properly distributed, typed, structured family of C function APIs, instead of this single central bastard of an ioctl() interface.

There's over 100 invocations of the EFI runtime in the Linux kernel, I think it would be worth the effort. The wrapper inlines should be mostly trivial.

That would also add an opportunity to actually document most of these calls.

2)

Another suggestion: would it make sense to unify the 'EFI' and 'EFI early' calls - is there any deep reason why they are invoked via separate names? Why not use a single namespace:

        efi__allocate_pool()
        efi__free_pool()

and have a 'current EFI configuration' pointer internaly that can be switched from the early to the later variant during bootup. The various typed API wrappers would use this pointer.

Quoted 46 lines · Ingo Molnar

Quoted 8 lines · Ard Biesheuvel

Quoted 4 lines

So screen_info_guid should probably not be a 'const': you have to cast it away anyway, adding artificial linebreaks and uglifying the code. It's also a bad practice to cast away const-ness, it hinders move-consts-to-readonly-sections efforts.

The problem here is that the UEFI spec never uses const qualifiers in its APIs for by-ref parameters that are obviously never modified by the caller, such as these GUIDs. [...]

Ah, ok. Two related thoughts came up:

1)

While I was looking at this code and was asking myself why the EFI runtime is generally invoked via a relatively fragile, non-type-checking vararg construct.

Wouldn't you be better off by explicitly defining all the API variants, and then internally calling the EFI runtime?

That would neatly solve such const artifacts as well.

So instead of:

drivers/firmware/efi/libstub/arm32-stub.c +2 -0

+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+                               sizeof(*si), (void **)&si);

we could have something like:

        status = efi_early__allocate_pool(EFI_RUNTIME_SERVICES_DATA, sizeof(*si), &si);

        ...

        efi_early__free_pool(si);

i.e. it would look a lot more like a properly distributed, typed, structured family of C function APIs, instead of this single central bastard of an ioctl() interface.

There's over 100 invocations of the EFI runtime in the Linux kernel, I think it would be worth the effort. The wrapper inlines should be mostly trivial.

Quoted 2 lines

That would also add an opportunity to actually document most of these calls.

If only. The ARM and arm64 wrappers actually simply resolve to correctly typed calls. While I am not an expert on the x86 side of things, I think the vararg stuff is needed to be able to perform the thunking required to do function calls using the MS ABI for x86_64.

As far as documentation is concerned, all of these functions and protocol methods are documented in the UEFI spec, which is freely accessible. It may be somewhat redundant to have our own documentation for them.

Quoted 50 lines · Ard Biesheuvel

Quoted 48 lines · Ingo Molnar

Quoted 8 lines · Ard Biesheuvel

Quoted 4 lines

So screen_info_guid should probably not be a 'const': you have to cast it away anyway, adding artificial linebreaks and uglifying the code. It's also a bad practice to cast away const-ness, it hinders move-consts-to-readonly-sections efforts.

The problem here is that the UEFI spec never uses const qualifiers in its APIs for by-ref parameters that are obviously never modified by the caller, such as these GUIDs. [...]

Ah, ok. Two related thoughts came up:

1)

While I was looking at this code and was asking myself why the EFI runtime is generally invoked via a relatively fragile, non-type-checking vararg construct.

Wouldn't you be better off by explicitly defining all the API variants, and then internally calling the EFI runtime?

That would neatly solve such const artifacts as well.

So instead of:

drivers/firmware/efi/libstub/arm32-stub.c +2 -0

+       status = efi_call_early(allocate_pool, EFI_RUNTIME_SERVICES_DATA,
+                               sizeof(*si), (void **)&si);

we could have something like:

        status = efi_early__allocate_pool(EFI_RUNTIME_SERVICES_DATA, sizeof(*si), &si);

        ...

        efi_early__free_pool(si);

i.e. it would look a lot more like a properly distributed, typed, structured family of C function APIs, instead of this single central bastard of an ioctl() interface.

There's over 100 invocations of the EFI runtime in the Linux kernel, I think it would be worth the effort. The wrapper inlines should be mostly trivial.

That would also add an opportunity to actually document most of these calls.

Quoted 4 lines

If only. The ARM and arm64 wrappers actually simply resolve to correctly typed calls. While I am not an expert on the x86 side of things, I think the vararg stuff is needed to be able to perform the thunking required to do function calls using the MS ABI for x86_64.

So at least the efi_early calls seem to just be using a function pointer:

  arch/x86/include/asm/efi.h:     __efi_early()->call(__efi_early()->f, __VA_ARGS__);

Furthermore, my suggestion would work with arbitrarily structured thunking: my suggestion was to put a typed C layer in there - and the layer itself could then call the vararg construct internally. It's a C type demuxing, only a syntactic effort, it does not change any real call signature.

Quoted 4 lines

As far as documentation is concerned, all of these functions and protocol methods are documented in the UEFI spec, which is freely accessible. It may be somewhat redundant to have our own documentation for them.

I mean a properly split up typed C interface 'documents itself' to a large degree
- while with opaque varargs bugs can slip in more easily.

This is really an obvious argument ... it's the well known 'why ioctl()s are bad' API argument.

It has no bearing on this series obviously, it's a cleanup suggestion that could be done separately.

Indeed. I think the efi_early struct is a separate structure that is set up by the early boot code, and has a number of function pointers of EFI boot services copied into it. Matt should know the details.

Quoted 5 lines

Furthermore, my suggestion would work with arbitrarily structured thunking: my suggestion was to put a typed C layer in there - and the layer itself could then call the vararg construct internally. It's a C type demuxing, only a syntactic effort, it does not change any real call signature.

I would welcome any improvement in this regard. Happy to contribute as well.

Thank you. I am perfectly happy exercising some more caution here, though, considering that our past experiences involving reshuffling stub code for x86 were not a huge success (and broke Linus's laptop, among others')

I was very careful not to introduce cross-object data symbol references (which were the culprit before), and I tested both i386 and x86_64 to the extent feasible to me, but we'd need to confirm that those things are still fully correct before queuing this.

This enables the EFI stub side of handling the EFI Graphics Output Protocol. It involves invoking the generic GOP code to discover the GOP handle that also drives the console, and using its metadata to populate the screen_info structure.

For arm64, the stub and the kernel proper live in the same executable, so we can simply access screen_info directly.

For ARM, we need to allocate a struct screen_info in the stub, and install it as a configuration table so that the core kernel can go and fetch it to copy its contents into the real screen_info structure.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/efi.h                | 12 +++++++
 arch/arm/kernel/setup.c                   |  3 +-
 arch/arm64/include/asm/efi.h              |  3 ++
 arch/arm64/kernel/efi.c                   |  3 ++
 arch/arm64/kernel/image.h                 |  1 +
 drivers/firmware/efi/libstub/arm-stub.c   | 23 ++++++++++++
 drivers/firmware/efi/libstub/arm32-stub.c | 38 ++++++++++++++++++++
 7 files changed, 82 insertions(+), 1 deletion(-)

arch/arm/include/asm/efi.h +5 -0

index daebcfe6c35a..6329b5be1eca 100644
@@ -63,6 +63,18 @@ void efi_virtmap_unload(void);
 #define __efi_call_early(f, ...)	f(__VA_ARGS__)
 #define efi_is_64bit()			(0)
 
+struct screen_info *alloc_screen_info(efi_system_table_t *sys_table_arg);
+void free_screen_info(efi_system_table_t *sys_table, struct screen_info *si);
+
+/*
+ * This GUID is used to pass to the kernel proper the struct screen_info

Unlike on arm64, where we can simply access the screen_info struct directly, ARM requires an intermediate step to get the information discovered by the GOP code in the UEFI stub into the screen_info struct.

So retrieve the dedicated config table we invented for this purpose, and put its contents into the core kernel's copy of struct screen_info.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/kernel/efi.c           | 24 ++++++++++++++++++++
 drivers/firmware/efi/arm-init.c |  6 +++++
 2 files changed, 30 insertions(+)

drivers/firmware/efi/arm-init.c +0 -0

index eca9b4f826ee..689bf65f8ddf 100644
@@ -112,6 +112,12 @@ static int __init uefi_init(void)
 	retval = efi_config_parse_tables(config_tables, efi.systab->nr_tables,
 					 sizeof(efi_config_table_t), NULL);

Quoted 12 lines · Ard Biesheuvel

Unlike on arm64, where we can simply access the screen_info struct directly, ARM requires an intermediate step to get the information discovered by the GOP code in the UEFI stub into the screen_info struct.

So retrieve the dedicated config table we invented for this purpose, and put its contents into the core kernel's copy of struct screen_info.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/kernel/efi.c           | 24 ++++++++++++++++++++
 drivers/firmware/efi/arm-init.c |  6 +++++
 2 files changed, 30 insertions(+)

[...]

Quoted 7 lines

drivers/firmware/efi/arm-init.c +0 -0

index eca9b4f826ee..689bf65f8ddf 100644
@@ -112,6 +112,12 @@ static int __init uefi_init(void)
      retval = efi_config_parse_tables(config_tables, efi.systab->nr_tables,
                                       sizeof(efi_config_table_t), NULL);

drivers/firmware/efi/arm-init.c static int __init uefi_init(void) · line 123

Quoted 10 lines

drivers/firmware/efi/arm-init.c +6 -0

 
+     if (IS_ENABLED(CONFIG_ARM)) {
+             extern void efi_find_screen_info(efi_config_table_t *, u32);
+
+             efi_find_screen_info(config_tables, efi.systab->nr_tables);
+     }
+
      early_memunmap(config_tables, table_size);
 out:
      early_memunmap(efi.systab,  sizeof(efi_system_table_t));

It's unfortunate that this is the first example of putting arch-specific things in arm-init.c. Is there anyway we can move it out into arch/arm?

If not, because efi_find_screen_info() uses all standard data types and function calls, I wouldn't be opposed to just sticking this in efi_config_parse_tables() like we do for EFI_PROPERTIES_TABLE. It's certainly nice to have all config table code in one place.

Also, it would save ARM the effort of looping over the config tables twice.

This series adds support to ARM and arm64 for using the kernel's EFI framebuffer driver to drive EFI Graphics Output Protocol (GOP) based framebuffers.

This involves refactoring some of the existing x86 code so it can be reused by ARM and arm64, and wiring it up both in the UEFI stub and in the core kernel into the existing UEFI infrastructure for ARM and arm64.

Quoted 9 lines · Ard Biesheuvel

This series adds support to ARM and arm64 for using the kernel's EFI framebuffer driver to drive EFI Graphics Output Protocol (GOP) based framebuffers.

This involves refactoring some of the existing x86 code so it can be reused
by ARM and arm64, and wiring it up both in the UEFI stub and in the core kernel into the existing UEFI infrastructure for ARM and arm64.

Given that I have access to a variety of different ARM64 UEFI implementations, how would I go about testing this code? Do I need a specific UEFI implementation and are there any special command line arguments I should pass to the kernel?

(+ Laszlo)

Quoted 12 lines · Mark Langsdorf

Quoted 9 lines · Ard Biesheuvel

This series adds support to ARM and arm64 for using the kernel's EFI framebuffer driver to drive EFI Graphics Output Protocol (GOP) based framebuffers.

This involves refactoring some of the existing x86 code so it can be reused
by ARM and arm64, and wiring it up both in the UEFI stub and in the core kernel into the existing UEFI infrastructure for ARM and arm64.

Quoted 5 lines

Given that I have access to a variety of different ARM64 UEFI implementations, how would I go about testing this code? Do I need a specific UEFI implementation and are there any special command line arguments I should pass to the kernel?

I have tested this myself on QEMU and FVP Base, which are both software models. On bare metal, it would involve a system with a graphics card that the firmware knows how to drive, and I am honestly not sure if any such combinations currently exist, other than the ARM development platforms such as Juno which have an embedded-style (i.e., non-PCI) graphics controller that the firmware knows how to drive out of the box.

The code itself is smart enough to figure which (if any) of the available handles carrying the GOP protocol is the one that is attached to ConOut, and so there is nothing required to enable this other than building it into the kernel and running it on a system with a firmware supported screen attached.

Quoted 29 lines · Ard Biesheuvel

(+ Laszlo)

Quoted 12 lines · Mark Langsdorf

Quoted 9 lines · Ard Biesheuvel

This series adds support to ARM and arm64 for using the kernel's EFI framebuffer driver to drive EFI Graphics Output Protocol (GOP) based framebuffers.

This involves refactoring some of the existing x86 code so it can be reused
by ARM and arm64, and wiring it up both in the UEFI stub and in the core kernel into the existing UEFI infrastructure for ARM and arm64.

Quoted 5 lines

Given that I have access to a variety of different ARM64 UEFI implementations, how would I go about testing this code? Do I need a specific UEFI implementation and are there any special command line arguments I should pass to the kernel?

I have tested this myself on QEMU and FVP Base, which are both software models. On bare metal, it would involve a system with a graphics card that the firmware knows how to drive, and I am honestly not sure if any such combinations currently exist, other than the ARM development platforms such as Juno which have an embedded-style (i.e., non-PCI) graphics controller that the firmware knows how to drive out of the box.

Quoted 6 lines

The code itself is smart enough to figure which (if any) of the available handles carrying the GOP protocol is the one that is attached to ConOut, and so there is nothing required to enable this other than building it into the kernel and running it on a system with a firmware supported screen attached.

How does this deal with caches? Does the GOP driver access the frame buffer as cached or as uncached memory? I suppose it's always uncached?

I'm mostly curious because I'd like to implement support for GOP in U-Boot soon.