The following changes since commit 2f13437b8917627119d163d62f73e7a78a92303a:
Merge tag 'trace-v5.5-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace (2019-12-04 19:13:52 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-urgent
for you to fetch changes up to f74622f872e9cb59cabd89cd908ade976e2269b9:
efi/earlycon: Remap entire framebuffer after page initialization (2019-12-05 16:40:06 +0000)
----------------------------------------------------------------
Some EFI fixes for the v5.5 cycle:
- Ensure that EFI persistent memory reservations are safe from being
clobbered by the kexec userland tools by listing them in /proc/iomem
- Reinstate a EFIFB earlycon optimization that got lost when moving the
code from x86 earlyprintk
- Various fixes for logic bugs in the handling of graphics output by
the EFI stub.
----------------------------------------------------------------
Andy Shevchenko (1):
efi/earlycon: Remap entire framebuffer after page initialization
Ard Biesheuvel (1):
efi/memreserve: register reservations as 'reserved' in /proc/iomem
Arvind Sankar (4):
efi/gop: Return EFI_NOT_FOUND if there are no usable GOPs
efi/gop: Return EFI_SUCCESS if a usable GOP was found
efi/gop: Fix memory leak from __gop_query32/64
efi: fix type of unload field in efi_loaded_image_t
drivers/firmware/efi/earlycon.c | 40 +++++++++++++++++++
drivers/firmware/efi/efi.c | 28 ++++++++++++-
drivers/firmware/efi/libstub/gop.c | 80 +++++++++-----------------------------
include/linux/efi.h | 10 ++---
4 files changed, 90 insertions(+), 68 deletions(-)
From: Arvind Sankar <[email protected]>
If we don't find a usable instance of the Graphics Output Protocol
(GOP) because none of them have a framebuffer (i.e. they were all
PIXEL_BLT_ONLY), but all the efi calls succeeded, we will return
EFI_SUCCESS even though we didn't find a usable GOP.
Fix this by explicitly returning EFI_NOT_FOUND if no usable GOPs are
found, allowing the caller to probe for UGA instead.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/libstub/gop.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 0101ca4c13b1..08f3c1a2fb48 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -119,7 +119,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
u64 fb_base;
struct efi_pixel_bitmask pixel_info;
int pixel_format;
- efi_status_t status = EFI_NOT_FOUND;
+ efi_status_t status;
u32 *handles = (u32 *)(unsigned long)gop_handle;
int i;
@@ -175,7 +175,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
/* Did we find any GOPs? */
if (!first_gop)
- goto out;
+ return EFI_NOT_FOUND;
/* EFI framebuffer */
si->orig_video_isVGA = VIDEO_TYPE_EFI;
@@ -197,7 +197,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->lfb_size = si->lfb_linelength * si->lfb_height;
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
-out:
+
return status;
}
@@ -237,7 +237,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
u64 fb_base;
struct efi_pixel_bitmask pixel_info;
int pixel_format;
- efi_status_t status = EFI_NOT_FOUND;
+ efi_status_t status;
u64 *handles = (u64 *)(unsigned long)gop_handle;
int i;
@@ -293,7 +293,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
/* Did we find any GOPs? */
if (!first_gop)
- goto out;
+ return EFI_NOT_FOUND;
/* EFI framebuffer */
si->orig_video_isVGA = VIDEO_TYPE_EFI;
@@ -315,7 +315,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->lfb_size = si->lfb_linelength * si->lfb_height;
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
-out:
+
return status;
}
--
2.17.1
From: Arvind Sankar <[email protected]>
The unload field is a function pointer, so it should be u32 for 32-bit,
u64 for 64-bit. Add a prototype for it in the native efi_loaded_image_t
type. Also change type of parent_handle and device_handle from void* to
efi_handle_t for documentation purposes.
The unload method is not used, so no functional change.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
include/linux/efi.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 99dfea595c8c..aa54586db7a5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -824,7 +824,7 @@ typedef struct {
__aligned_u64 image_size;
unsigned int image_code_type;
unsigned int image_data_type;
- unsigned long unload;
+ u32 unload;
} efi_loaded_image_32_t;
typedef struct {
@@ -840,14 +840,14 @@ typedef struct {
__aligned_u64 image_size;
unsigned int image_code_type;
unsigned int image_data_type;
- unsigned long unload;
+ u64 unload;
} efi_loaded_image_64_t;
typedef struct {
u32 revision;
- void *parent_handle;
+ efi_handle_t parent_handle;
efi_system_table_t *system_table;
- void *device_handle;
+ efi_handle_t device_handle;
void *file_path;
void *reserved;
u32 load_options_size;
@@ -856,7 +856,7 @@ typedef struct {
__aligned_u64 image_size;
unsigned int image_code_type;
unsigned int image_data_type;
- unsigned long unload;
+ efi_status_t (*unload)(efi_handle_t image_handle);
} efi_loaded_image_t;
--
2.17.1
From: Andy Shevchenko <[email protected]>
When commit 69c1f396f25b
"efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation"
moved the x86 specific EFI earlyprintk implementation to a shared location,
it also tweaked the behaviour. In particular, it dropped a trick with full
framebuffer remapping after page initialization, leading to two regressions:
1) very slow scrolling after page initialization,
2) kernel hang when the 'keep_bootcon' command line argument is passed.
Putting the tweak back fixes #2 and mitigates #1, i.e., it limits the slow
behavior to the early boot stages, presumably due to eliminating heavy
map()/unmap() operations per each pixel line on the screen.
Fixes: 69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation")
Signed-off-by: Andy Shevchenko <[email protected]>
[ardb: ensure efifb is unmapped again unless keep_bootcon is in effect]
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/earlycon.c | 40 +++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index c9a0efca17b0..0bc4fe741415 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -13,18 +13,57 @@
#include <asm/early_ioremap.h>
+static const struct console *earlycon_console __initdata;
static const struct font_desc *font;
static u32 efi_x, efi_y;
static u64 fb_base;
static pgprot_t fb_prot;
+static void *efi_fb;
+
+/*
+ * efi earlycon needs to use early_memremap() to map the framebuffer.
+ * But early_memremap() is not usable for 'earlycon=efifb keep_bootcon',
+ * memremap() should be used instead. memremap() will be available after
+ * paging_init() which is earlier than initcall callbacks. Thus adding this
+ * early initcall function early_efi_map_fb() to map the whole efi framebuffer.
+ */
+static int __init efi_earlycon_remap_fb(void)
+{
+ /* bail if there is no bootconsole or it has been disabled already */
+ if (!earlycon_console || !(earlycon_console->flags & CON_ENABLED))
+ return 0;
+
+ if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL))
+ efi_fb = memremap(fb_base, screen_info.lfb_size, MEMREMAP_WB);
+ else
+ efi_fb = memremap(fb_base, screen_info.lfb_size, MEMREMAP_WC);
+
+ return efi_fb ? 0 : -ENOMEM;
+}
+early_initcall(efi_earlycon_remap_fb);
+
+static int __init efi_earlycon_unmap_fb(void)
+{
+ /* unmap the bootconsole fb unless keep_bootcon has left it enabled */
+ if (efi_fb && !(earlycon_console->flags & CON_ENABLED))
+ memunmap(efi_fb);
+ return 0;
+}
+late_initcall(efi_earlycon_unmap_fb);
static __ref void *efi_earlycon_map(unsigned long start, unsigned long len)
{
+ if (efi_fb)
+ return efi_fb + start;
+
return early_memremap_prot(fb_base + start, len, pgprot_val(fb_prot));
}
static __ref void efi_earlycon_unmap(void *addr, unsigned long len)
{
+ if (efi_fb)
+ return;
+
early_memunmap(addr, len);
}
@@ -201,6 +240,7 @@ static int __init efi_earlycon_setup(struct earlycon_device *device,
efi_earlycon_scroll_up();
device->con->write = efi_earlycon_write;
+ earlycon_console = device->con;
return 0;
}
EARLYCON_DECLARE(efifb, efi_earlycon_setup);
--
2.17.1
From: Arvind Sankar <[email protected]>
efi_graphics_output_protocol::query_mode() returns info in
callee-allocated memory which must be freed by the caller, which
we aren't doing.
We don't actually need to call query_mode in order to obtain the
info for the current graphics mode, which is already there in
gop->mode->info, so just access it directly in the setup_gop32/64
functions.
Also nothing uses the size of the info structure, so don't update the
passed-in size (which is the size of the gop_handle table in bytes)
unnecessarily.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/libstub/gop.c | 66 ++++++------------------------
1 file changed, 12 insertions(+), 54 deletions(-)
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 69b2b019a1d0..b7bf1e993b8b 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -83,30 +83,6 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
}
}
-static efi_status_t
-__gop_query32(efi_system_table_t *sys_table_arg,
- struct efi_graphics_output_protocol_32 *gop32,
- struct efi_graphics_output_mode_info **info,
- unsigned long *size, u64 *fb_base)
-{
- struct efi_graphics_output_protocol_mode_32 *mode;
- efi_graphics_output_protocol_query_mode query_mode;
- efi_status_t status;
- unsigned long m;
-
- m = gop32->mode;
- mode = (struct efi_graphics_output_protocol_mode_32 *)m;
- query_mode = (void *)(unsigned long)gop32->query_mode;
-
- status = __efi_call_early(query_mode, (void *)gop32, mode->mode, size,
- info);
- if (status != EFI_SUCCESS)
- return status;
-
- *fb_base = mode->frame_buffer_base;
- return status;
-}
-
static efi_status_t
setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
efi_guid_t *proto, unsigned long size, void **gop_handle)
@@ -128,6 +104,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
nr_gops = size / sizeof(u32);
for (i = 0; i < nr_gops; i++) {
+ struct efi_graphics_output_protocol_mode_32 *mode;
struct efi_graphics_output_mode_info *info = NULL;
efi_guid_t conout_proto = EFI_CONSOLE_OUT_DEVICE_GUID;
bool conout_found = false;
@@ -145,9 +122,11 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
if (status == EFI_SUCCESS)
conout_found = true;
- status = __gop_query32(sys_table_arg, gop32, &info, &size,
- ¤t_fb_base);
- if (status == EFI_SUCCESS && (!first_gop || conout_found) &&
+ mode = (void *)(unsigned long)gop32->mode;
+ info = (void *)(unsigned long)mode->info;
+ current_fb_base = mode->frame_buffer_base;
+
+ if ((!first_gop || conout_found) &&
info->pixel_format != PIXEL_BLT_ONLY) {
/*
* Systems that use the UEFI Console Splitter may
@@ -201,30 +180,6 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
return EFI_SUCCESS;
}
-static efi_status_t
-__gop_query64(efi_system_table_t *sys_table_arg,
- struct efi_graphics_output_protocol_64 *gop64,
- struct efi_graphics_output_mode_info **info,
- unsigned long *size, u64 *fb_base)
-{
- struct efi_graphics_output_protocol_mode_64 *mode;
- efi_graphics_output_protocol_query_mode query_mode;
- efi_status_t status;
- unsigned long m;
-
- m = gop64->mode;
- mode = (struct efi_graphics_output_protocol_mode_64 *)m;
- query_mode = (void *)(unsigned long)gop64->query_mode;
-
- status = __efi_call_early(query_mode, (void *)gop64, mode->mode, size,
- info);
- if (status != EFI_SUCCESS)
- return status;
-
- *fb_base = mode->frame_buffer_base;
- return status;
-}
-
static efi_status_t
setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
efi_guid_t *proto, unsigned long size, void **gop_handle)
@@ -246,6 +201,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
nr_gops = size / sizeof(u64);
for (i = 0; i < nr_gops; i++) {
+ struct efi_graphics_output_protocol_mode_64 *mode;
struct efi_graphics_output_mode_info *info = NULL;
efi_guid_t conout_proto = EFI_CONSOLE_OUT_DEVICE_GUID;
bool conout_found = false;
@@ -263,9 +219,11 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
if (status == EFI_SUCCESS)
conout_found = true;
- status = __gop_query64(sys_table_arg, gop64, &info, &size,
- ¤t_fb_base);
- if (status == EFI_SUCCESS && (!first_gop || conout_found) &&
+ mode = (void *)(unsigned long)gop64->mode;
+ info = (void *)(unsigned long)mode->info;
+ current_fb_base = mode->frame_buffer_base;
+
+ if ((!first_gop || conout_found) &&
info->pixel_format != PIXEL_BLT_ONLY) {
/*
* Systems that use the UEFI Console Splitter may
--
2.17.1
From: Arvind Sankar <[email protected]>
If we've found a usable instance of the Graphics Output Protocol
(GOP) with a framebuffer, it is possible that one of the later EFI
calls fails while checking if any support console output. In this
case status may be an EFI error code even though we found a usable
GOP.
Fix this by explicitly return EFI_SUCCESS if a usable GOP has been
located.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/libstub/gop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 08f3c1a2fb48..69b2b019a1d0 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -198,7 +198,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
- return status;
+ return EFI_SUCCESS;
}
static efi_status_t
@@ -316,7 +316,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
- return status;
+ return EFI_SUCCESS;
}
/*
--
2.17.1
The following commit has been merged into the efi/urgent branch of tip:
Commit-ID: b418d660bb9798d2249ac6a46c844389ef50b6a5
Gitweb: https://git.kernel.org/tip/b418d660bb9798d2249ac6a46c844389ef50b6a5
Author: Andy Shevchenko <[email protected]>
AuthorDate: Fri, 06 Dec 2019 16:55:42
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 08 Dec 2019 12:42:19 +01:00
efi/earlycon: Remap entire framebuffer after page initialization
When commit:
69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation")
moved the x86 specific EFI earlyprintk implementation to a shared location,
it also tweaked the behaviour. In particular, it dropped a trick with full
framebuffer remapping after page initialization, leading to two regressions:
1) very slow scrolling after page initialization,
2) kernel hang when the 'keep_bootcon' command line argument is passed.
Putting the tweak back fixes #2 and mitigates #1, i.e., it limits the slow
behavior to the early boot stages, presumably due to eliminating heavy
map()/unmap() operations per each pixel line on the screen.
[ ardb: ensure efifb is unmapped again unless keep_bootcon is in effect. ]
[ mingo: speling fixes. ]
Signed-off-by: Andy Shevchenko <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Cc: Arvind Sankar <[email protected]>
Cc: Bhupesh Sharma <[email protected]>
Cc: Masayoshi Mizuma <[email protected]>
Cc: [email protected]
Fixes: 69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation")
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/firmware/efi/earlycon.c | 40 ++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+)
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index c9a0efc..d4077db 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -13,18 +13,57 @@
#include <asm/early_ioremap.h>
+static const struct console *earlycon_console __initdata;
static const struct font_desc *font;
static u32 efi_x, efi_y;
static u64 fb_base;
static pgprot_t fb_prot;
+static void *efi_fb;
+
+/*
+ * EFI earlycon needs to use early_memremap() to map the framebuffer.
+ * But early_memremap() is not usable for 'earlycon=efifb keep_bootcon',
+ * memremap() should be used instead. memremap() will be available after
+ * paging_init() which is earlier than initcall callbacks. Thus adding this
+ * early initcall function early_efi_map_fb() to map the whole EFI framebuffer.
+ */
+static int __init efi_earlycon_remap_fb(void)
+{
+ /* bail if there is no bootconsole or it has been disabled already */
+ if (!earlycon_console || !(earlycon_console->flags & CON_ENABLED))
+ return 0;
+
+ if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL))
+ efi_fb = memremap(fb_base, screen_info.lfb_size, MEMREMAP_WB);
+ else
+ efi_fb = memremap(fb_base, screen_info.lfb_size, MEMREMAP_WC);
+
+ return efi_fb ? 0 : -ENOMEM;
+}
+early_initcall(efi_earlycon_remap_fb);
+
+static int __init efi_earlycon_unmap_fb(void)
+{
+ /* unmap the bootconsole fb unless keep_bootcon has left it enabled */
+ if (efi_fb && !(earlycon_console->flags & CON_ENABLED))
+ memunmap(efi_fb);
+ return 0;
+}
+late_initcall(efi_earlycon_unmap_fb);
static __ref void *efi_earlycon_map(unsigned long start, unsigned long len)
{
+ if (efi_fb)
+ return efi_fb + start;
+
return early_memremap_prot(fb_base + start, len, pgprot_val(fb_prot));
}
static __ref void efi_earlycon_unmap(void *addr, unsigned long len)
{
+ if (efi_fb)
+ return;
+
early_memunmap(addr, len);
}
@@ -201,6 +240,7 @@ static int __init efi_earlycon_setup(struct earlycon_device *device,
efi_earlycon_scroll_up();
device->con->write = efi_earlycon_write;
+ earlycon_console = device->con;
return 0;
}
EARLYCON_DECLARE(efifb, efi_earlycon_setup);
The following commit has been merged into the efi/urgent branch of tip:
Commit-ID: 9fa76ca7b8bdcdf51fc8c7b7b7a7bfc4eccceb58
Gitweb: https://git.kernel.org/tip/9fa76ca7b8bdcdf51fc8c7b7b7a7bfc4eccceb58
Author: Arvind Sankar <[email protected]>
AuthorDate: Fri, 06 Dec 2019 16:55:41
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 08 Dec 2019 12:42:19 +01:00
efi: Fix efi_loaded_image_t::unload type
The ::unload field is a function pointer, so it should be u32 for 32-bit,
u64 for 64-bit. Add a prototype for it in the native efi_loaded_image_t
type. Also change type of parent_handle and device_handle from void * to
efi_handle_t for documentation purposes.
The unload method is not used, so no functional change.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Bhupesh Sharma <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masayoshi Mizuma <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/efi.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 99dfea5..aa54586 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -824,7 +824,7 @@ typedef struct {
__aligned_u64 image_size;
unsigned int image_code_type;
unsigned int image_data_type;
- unsigned long unload;
+ u32 unload;
} efi_loaded_image_32_t;
typedef struct {
@@ -840,14 +840,14 @@ typedef struct {
__aligned_u64 image_size;
unsigned int image_code_type;
unsigned int image_data_type;
- unsigned long unload;
+ u64 unload;
} efi_loaded_image_64_t;
typedef struct {
u32 revision;
- void *parent_handle;
+ efi_handle_t parent_handle;
efi_system_table_t *system_table;
- void *device_handle;
+ efi_handle_t device_handle;
void *file_path;
void *reserved;
u32 load_options_size;
@@ -856,7 +856,7 @@ typedef struct {
__aligned_u64 image_size;
unsigned int image_code_type;
unsigned int image_data_type;
- unsigned long unload;
+ efi_status_t (*unload)(efi_handle_t image_handle);
} efi_loaded_image_t;
The following commit has been merged into the efi/urgent branch of tip:
Commit-ID: dbd89c303b4420f6cdb689fd398349fc83b059dd
Gitweb: https://git.kernel.org/tip/dbd89c303b4420f6cdb689fd398349fc83b059dd
Author: Arvind Sankar <[email protected]>
AuthorDate: Fri, 06 Dec 2019 16:55:39
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 08 Dec 2019 12:42:18 +01:00
efi/gop: Return EFI_SUCCESS if a usable GOP was found
If we've found a usable instance of the Graphics Output Protocol
(GOP) with a framebuffer, it is possible that one of the later EFI
calls fails while checking if any support console output. In this
case status may be an EFI error code even though we found a usable
GOP.
Fix this by explicitly return EFI_SUCCESS if a usable GOP has been
located.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Bhupesh Sharma <[email protected]>
Cc: Masayoshi Mizuma <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/firmware/efi/libstub/gop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 08f3c1a..69b2b01 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -198,7 +198,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
- return status;
+ return EFI_SUCCESS;
}
static efi_status_t
@@ -316,7 +316,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
- return status;
+ return EFI_SUCCESS;
}
/*
The following commit has been merged into the efi/urgent branch of tip:
Commit-ID: 6fc3cec30dfeee7d3c5db8154016aff9d65503c5
Gitweb: https://git.kernel.org/tip/6fc3cec30dfeee7d3c5db8154016aff9d65503c5
Author: Arvind Sankar <[email protected]>
AuthorDate: Fri, 06 Dec 2019 16:55:38
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 08 Dec 2019 12:42:18 +01:00
efi/gop: Return EFI_NOT_FOUND if there are no usable GOPs
If we don't find a usable instance of the Graphics Output Protocol
(GOP) because none of them have a framebuffer (i.e. they were all
PIXEL_BLT_ONLY), but all the EFI calls succeeded, we will return
EFI_SUCCESS even though we didn't find a usable GOP.
Fix this by explicitly returning EFI_NOT_FOUND if no usable GOPs are
found, allowing the caller to probe for UGA instead.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Bhupesh Sharma <[email protected]>
Cc: Masayoshi Mizuma <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/firmware/efi/libstub/gop.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 0101ca4..08f3c1a 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -119,7 +119,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
u64 fb_base;
struct efi_pixel_bitmask pixel_info;
int pixel_format;
- efi_status_t status = EFI_NOT_FOUND;
+ efi_status_t status;
u32 *handles = (u32 *)(unsigned long)gop_handle;
int i;
@@ -175,7 +175,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
/* Did we find any GOPs? */
if (!first_gop)
- goto out;
+ return EFI_NOT_FOUND;
/* EFI framebuffer */
si->orig_video_isVGA = VIDEO_TYPE_EFI;
@@ -197,7 +197,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->lfb_size = si->lfb_linelength * si->lfb_height;
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
-out:
+
return status;
}
@@ -237,7 +237,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
u64 fb_base;
struct efi_pixel_bitmask pixel_info;
int pixel_format;
- efi_status_t status = EFI_NOT_FOUND;
+ efi_status_t status;
u64 *handles = (u64 *)(unsigned long)gop_handle;
int i;
@@ -293,7 +293,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
/* Did we find any GOPs? */
if (!first_gop)
- goto out;
+ return EFI_NOT_FOUND;
/* EFI framebuffer */
si->orig_video_isVGA = VIDEO_TYPE_EFI;
@@ -315,7 +315,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
si->lfb_size = si->lfb_linelength * si->lfb_height;
si->capabilities |= VIDEO_CAPABILITY_SKIP_QUIRKS;
-out:
+
return status;
}
The following commit has been merged into the efi/urgent branch of tip:
Commit-ID: ff397be685e410a59c34b21ce0c55d4daa466bb7
Gitweb: https://git.kernel.org/tip/ff397be685e410a59c34b21ce0c55d4daa466bb7
Author: Arvind Sankar <[email protected]>
AuthorDate: Fri, 06 Dec 2019 16:55:40
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 08 Dec 2019 12:42:18 +01:00
efi/gop: Fix memory leak in __gop_query32/64()
efi_graphics_output_protocol::query_mode() returns info in
callee-allocated memory which must be freed by the caller, which
we aren't doing.
We don't actually need to call query_mode() in order to obtain the
info for the current graphics mode, which is already there in
gop->mode->info, so just access it directly in the setup_gop32/64()
functions.
Also nothing uses the size of the info structure, so don't update the
passed-in size (which is the size of the gop_handle table in bytes)
unnecessarily.
Signed-off-by: Arvind Sankar <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Bhupesh Sharma <[email protected]>
Cc: Masayoshi Mizuma <[email protected]>
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/firmware/efi/libstub/gop.c | 66 +++++------------------------
1 file changed, 12 insertions(+), 54 deletions(-)
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 69b2b01..b7bf1e9 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -84,30 +84,6 @@ setup_pixel_info(struct screen_info *si, u32 pixels_per_scan_line,
}
static efi_status_t
-__gop_query32(efi_system_table_t *sys_table_arg,
- struct efi_graphics_output_protocol_32 *gop32,
- struct efi_graphics_output_mode_info **info,
- unsigned long *size, u64 *fb_base)
-{
- struct efi_graphics_output_protocol_mode_32 *mode;
- efi_graphics_output_protocol_query_mode query_mode;
- efi_status_t status;
- unsigned long m;
-
- m = gop32->mode;
- mode = (struct efi_graphics_output_protocol_mode_32 *)m;
- query_mode = (void *)(unsigned long)gop32->query_mode;
-
- status = __efi_call_early(query_mode, (void *)gop32, mode->mode, size,
- info);
- if (status != EFI_SUCCESS)
- return status;
-
- *fb_base = mode->frame_buffer_base;
- return status;
-}
-
-static efi_status_t
setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
efi_guid_t *proto, unsigned long size, void **gop_handle)
{
@@ -128,6 +104,7 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
nr_gops = size / sizeof(u32);
for (i = 0; i < nr_gops; i++) {
+ struct efi_graphics_output_protocol_mode_32 *mode;
struct efi_graphics_output_mode_info *info = NULL;
efi_guid_t conout_proto = EFI_CONSOLE_OUT_DEVICE_GUID;
bool conout_found = false;
@@ -145,9 +122,11 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
if (status == EFI_SUCCESS)
conout_found = true;
- status = __gop_query32(sys_table_arg, gop32, &info, &size,
- ¤t_fb_base);
- if (status == EFI_SUCCESS && (!first_gop || conout_found) &&
+ mode = (void *)(unsigned long)gop32->mode;
+ info = (void *)(unsigned long)mode->info;
+ current_fb_base = mode->frame_buffer_base;
+
+ if ((!first_gop || conout_found) &&
info->pixel_format != PIXEL_BLT_ONLY) {
/*
* Systems that use the UEFI Console Splitter may
@@ -202,30 +181,6 @@ setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
}
static efi_status_t
-__gop_query64(efi_system_table_t *sys_table_arg,
- struct efi_graphics_output_protocol_64 *gop64,
- struct efi_graphics_output_mode_info **info,
- unsigned long *size, u64 *fb_base)
-{
- struct efi_graphics_output_protocol_mode_64 *mode;
- efi_graphics_output_protocol_query_mode query_mode;
- efi_status_t status;
- unsigned long m;
-
- m = gop64->mode;
- mode = (struct efi_graphics_output_protocol_mode_64 *)m;
- query_mode = (void *)(unsigned long)gop64->query_mode;
-
- status = __efi_call_early(query_mode, (void *)gop64, mode->mode, size,
- info);
- if (status != EFI_SUCCESS)
- return status;
-
- *fb_base = mode->frame_buffer_base;
- return status;
-}
-
-static efi_status_t
setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
efi_guid_t *proto, unsigned long size, void **gop_handle)
{
@@ -246,6 +201,7 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
nr_gops = size / sizeof(u64);
for (i = 0; i < nr_gops; i++) {
+ struct efi_graphics_output_protocol_mode_64 *mode;
struct efi_graphics_output_mode_info *info = NULL;
efi_guid_t conout_proto = EFI_CONSOLE_OUT_DEVICE_GUID;
bool conout_found = false;
@@ -263,9 +219,11 @@ setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
if (status == EFI_SUCCESS)
conout_found = true;
- status = __gop_query64(sys_table_arg, gop64, &info, &size,
- ¤t_fb_base);
- if (status == EFI_SUCCESS && (!first_gop || conout_found) &&
+ mode = (void *)(unsigned long)gop64->mode;
+ info = (void *)(unsigned long)mode->info;
+ current_fb_base = mode->frame_buffer_base;
+
+ if ((!first_gop || conout_found) &&
info->pixel_format != PIXEL_BLT_ONLY) {
/*
* Systems that use the UEFI Console Splitter may
On Fri, Dec 06, 2019 at 04:55:42PM +0000, Ard Biesheuvel wrote:
> From: Andy Shevchenko <[email protected]>
>
> When commit 69c1f396f25b
>
> "efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation"
>
> moved the x86 specific EFI earlyprintk implementation to a shared location,
> it also tweaked the behaviour. In particular, it dropped a trick with full
> framebuffer remapping after page initialization, leading to two regressions:
> 1) very slow scrolling after page initialization,
> 2) kernel hang when the 'keep_bootcon' command line argument is passed.
>
> Putting the tweak back fixes #2 and mitigates #1, i.e., it limits the slow
> behavior to the early boot stages, presumably due to eliminating heavy
> map()/unmap() operations per each pixel line on the screen.
>
Could the efi earlycon have an interaction with PCI resource allocation,
similar to what commit dcf8f5ce3165 ("drivers/fbdev/efifb: Allow BAR to
be moved instead of claiming it") fixed for efifb?
On Mon, 9 Dec 2019 at 20:12, Arvind Sankar <[email protected]> wrote:
>
> On Fri, Dec 06, 2019 at 04:55:42PM +0000, Ard Biesheuvel wrote:
> > From: Andy Shevchenko <[email protected]>
> >
> > When commit 69c1f396f25b
> >
> > "efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation"
> >
> > moved the x86 specific EFI earlyprintk implementation to a shared location,
> > it also tweaked the behaviour. In particular, it dropped a trick with full
> > framebuffer remapping after page initialization, leading to two regressions:
> > 1) very slow scrolling after page initialization,
> > 2) kernel hang when the 'keep_bootcon' command line argument is passed.
> >
> > Putting the tweak back fixes #2 and mitigates #1, i.e., it limits the slow
> > behavior to the early boot stages, presumably due to eliminating heavy
> > map()/unmap() operations per each pixel line on the screen.
> >
>
> Could the efi earlycon have an interaction with PCI resource allocation,
> similar to what commit dcf8f5ce3165 ("drivers/fbdev/efifb: Allow BAR to
> be moved instead of claiming it") fixed for efifb?
Yes. If the BAR gets moved, things will break. This is mostly an issue
for the keep_bootcon case, but that is documented as being a debug
feature specifically for addressing console initialization related
issues. Earlycon itself is also a debug feature, so if you hit the BAR
reallocation issue, you're simply out of luck. Note that this happens
rarely in practice, only on non-x86 systems where the firmware and the
kernel have very different policies regarding BAR allocation, and on
DT based systems, you can force the OS to honour the existing
allocation by using linux,pci-probe-only
On Mon, Dec 09, 2019 at 07:24:13PM +0000, Ard Biesheuvel wrote:
> On Mon, 9 Dec 2019 at 20:12, Arvind Sankar <[email protected]> wrote:
> >
> > On Fri, Dec 06, 2019 at 04:55:42PM +0000, Ard Biesheuvel wrote:
> > > From: Andy Shevchenko <[email protected]>
> > >
> > > When commit 69c1f396f25b
> > >
> > > "efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation"
> > >
> > > moved the x86 specific EFI earlyprintk implementation to a shared location,
> > > it also tweaked the behaviour. In particular, it dropped a trick with full
> > > framebuffer remapping after page initialization, leading to two regressions:
> > > 1) very slow scrolling after page initialization,
> > > 2) kernel hang when the 'keep_bootcon' command line argument is passed.
> > >
> > > Putting the tweak back fixes #2 and mitigates #1, i.e., it limits the slow
> > > behavior to the early boot stages, presumably due to eliminating heavy
> > > map()/unmap() operations per each pixel line on the screen.
> > >
> >
> > Could the efi earlycon have an interaction with PCI resource allocation,
> > similar to what commit dcf8f5ce3165 ("drivers/fbdev/efifb: Allow BAR to
> > be moved instead of claiming it") fixed for efifb?
>
> Yes. If the BAR gets moved, things will break. This is mostly an issue
> for the keep_bootcon case, but that is documented as being a debug
> feature specifically for addressing console initialization related
> issues. Earlycon itself is also a debug feature, so if you hit the BAR
> reallocation issue, you're simply out of luck. Note that this happens
> rarely in practice, only on non-x86 systems where the firmware and the
> kernel have very different policies regarding BAR allocation, and on
> DT based systems, you can force the OS to honour the existing
> allocation by using linux,pci-probe-only
Thanks. Another q -- I tried out the earlycon=efifb, and it seems like
it gets disabled (without keep_bootcon) as soon as dummycon takes over,
which is well before the real console.
DUMMY_CONSOLE is defined as
depends on VGA_CONSOLE!=y || SGI_NEWPORT_CONSOLE!=y
default y
so it seems like it will pretty much always be enabled, as it doesn't
seem likely that VGA_CONSOLE=y and SGI_NEWPORT_CONSOLE=y would ever be
true simultaneously.
Am I missing something or is this the way it's supposed to work? So
keep_bootcon seems almost necessary with the EFI boot console? Would a
patch to not disable boot console when dummycon is initialized, but wait
for a real console, be useful?
Thanks.
On Tue, 10 Dec 2019 at 21:05, Arvind Sankar <[email protected]> wrote:
>
> On Mon, Dec 09, 2019 at 07:24:13PM +0000, Ard Biesheuvel wrote:
> > On Mon, 9 Dec 2019 at 20:12, Arvind Sankar <[email protected]> wrote:
> > >
> > > On Fri, Dec 06, 2019 at 04:55:42PM +0000, Ard Biesheuvel wrote:
> > > > From: Andy Shevchenko <[email protected]>
> > > >
> > > > When commit 69c1f396f25b
> > > >
> > > > "efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation"
> > > >
> > > > moved the x86 specific EFI earlyprintk implementation to a shared location,
> > > > it also tweaked the behaviour. In particular, it dropped a trick with full
> > > > framebuffer remapping after page initialization, leading to two regressions:
> > > > 1) very slow scrolling after page initialization,
> > > > 2) kernel hang when the 'keep_bootcon' command line argument is passed.
> > > >
> > > > Putting the tweak back fixes #2 and mitigates #1, i.e., it limits the slow
> > > > behavior to the early boot stages, presumably due to eliminating heavy
> > > > map()/unmap() operations per each pixel line on the screen.
> > > >
> > >
> > > Could the efi earlycon have an interaction with PCI resource allocation,
> > > similar to what commit dcf8f5ce3165 ("drivers/fbdev/efifb: Allow BAR to
> > > be moved instead of claiming it") fixed for efifb?
> >
> > Yes. If the BAR gets moved, things will break. This is mostly an issue
> > for the keep_bootcon case, but that is documented as being a debug
> > feature specifically for addressing console initialization related
> > issues. Earlycon itself is also a debug feature, so if you hit the BAR
> > reallocation issue, you're simply out of luck. Note that this happens
> > rarely in practice, only on non-x86 systems where the firmware and the
> > kernel have very different policies regarding BAR allocation, and on
> > DT based systems, you can force the OS to honour the existing
> > allocation by using linux,pci-probe-only
>
> Thanks. Another q -- I tried out the earlycon=efifb, and it seems like
> it gets disabled (without keep_bootcon) as soon as dummycon takes over,
> which is well before the real console.
>
> DUMMY_CONSOLE is defined as
> depends on VGA_CONSOLE!=y || SGI_NEWPORT_CONSOLE!=y
> default y
>
> so it seems like it will pretty much always be enabled, as it doesn't
> seem likely that VGA_CONSOLE=y and SGI_NEWPORT_CONSOLE=y would ever be
> true simultaneously.
>
> Am I missing something or is this the way it's supposed to work? So
> keep_bootcon seems almost necessary with the EFI boot console? Would a
> patch to not disable boot console when dummycon is initialized, but wait
> for a real console, be useful?
>
Well spotted!
I have traced this down to [0] which combined various arch specific
definitions into one, and obviously chose the wrong boolean operator
for combining the conditions.
Patches welcome.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/drivers/video/console/Kconfig?id=31d2a7d36d6989c714b792ec00358ada24c039e7
On Wed, Dec 11, 2019 at 11:26:17AM +0000, Ard Biesheuvel wrote:
> On Tue, 10 Dec 2019 at 21:05, Arvind Sankar <[email protected]> wrote:
> > Thanks. Another q -- I tried out the earlycon=efifb, and it seems like
> > it gets disabled (without keep_bootcon) as soon as dummycon takes over,
> > which is well before the real console.
> >
> > DUMMY_CONSOLE is defined as
> > depends on VGA_CONSOLE!=y || SGI_NEWPORT_CONSOLE!=y
> > default y
> >
> > so it seems like it will pretty much always be enabled, as it doesn't
> > seem likely that VGA_CONSOLE=y and SGI_NEWPORT_CONSOLE=y would ever be
> > true simultaneously.
> >
> > Am I missing something or is this the way it's supposed to work? So
> > keep_bootcon seems almost necessary with the EFI boot console? Would a
> > patch to not disable boot console when dummycon is initialized, but wait
> > for a real console, be useful?
> >
>
> Well spotted!
>
> I have traced this down to [0] which combined various arch specific
> definitions into one, and obviously chose the wrong boolean operator
> for combining the conditions.
>
> Patches welcome.
>
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/drivers/video/console/Kconfig?id=31d2a7d36d6989c714b792ec00358ada24c039e7
Thanks! I was a little mistaken about what actually disables it though,
it seems to be when vt_console_driver gets registered, and at that point
the vt is still using dummy_con. I had thought dummy_con itself was a
console driver.