Skip to contentSkip to conversation

efi/earlycon: Remap entire framebuffer after page initialization

PATCH In review Andy Shevchenko 2 participants 4 messages Nov 15, 2019 – Nov 18, 2019 EFI

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

PATCH v3 1/1 efi/earlycon: Remap entire framebuffer after page initialization

When the commit 69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation") moved x86 specific EFI earlyprintk implementation to shared location it also tweaked the behaviour. In particular it dropped a trick with full framebuffer remapping after page initialization. This lead to two regressions:
1) very slow scrolling after page initialization;
2) kernel hang when keep_bootcon parameter is being provided.

Returning the trick back fixes #2 and mitigates, i.e. reduces the window when slowness appears, #1 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")
Cc Ard Biesheuvel <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Matt Fleming <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>

---
v3:
- assign size in efi_earlycon_setup()
- check base and size before mapping in early_efi_map_fb()

 drivers/firmware/efi/earlycon.c | 37 +++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
drivers/firmware/efi/earlycon.c +37 -0
index c9a0efca17b0..60b5350c6105 100644
@@ -16,15 +16,50 @@
 static const struct font_desc *font;
 static u32 efi_x, efi_y;
 static u64 fb_base;
+static u32 fb_size;
 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 early_efi_map_fb(void)
+{
+	if (!fb_base || !fb_size)
+		return NULL;
+
+	if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL))
+		efi_fb = memremap(fb_base, fb_size, MEMREMAP_WB);
+	else
+		efi_fb = memremap(fb_base, fb_size, MEMREMAP_WC);
+
+	return efi_fb ? 0 : -ENOMEM;
+}
+early_initcall(early_efi_map_fb);
+
+static void __exit early_efi_unmap_fb(void)
+{
+	memunmap(efi_fb);
+}
+__exitcall(early_efi_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);
 }
 

@@ -176,6 +211,8 @@ static int __init efi_earlycon_setup(struct earlycon_device *device,
 	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
 		fb_base |= (u64)screen_info.ext_lfb_base << 32;
 
+	fb_size = screen_info.lfb_size;
+
 	if (opt && !strcmp(opt, "ram"))
 		fb_prot = PAGE_KERNEL;
 	else
Signature · 1 line
2.24.0
#1 lore raw
Quoted 39 lines · Andy Shevchenko

When the commit 69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation") moved x86 specific EFI earlyprintk implementation to shared location it also tweaked the behaviour. In particular it dropped a trick with full framebuffer remapping after page initialization. This lead to two regressions:
1) very slow scrolling after page initialization;
2) kernel hang when keep_bootcon parameter is being provided.

Returning the trick back fixes #2 and mitigates, i.e. reduces the window when slowness appears, #1 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")
Cc Ard Biesheuvel <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Matt Fleming <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>

---
v3:
- assign size in efi_earlycon_setup()
- check base and size before mapping in early_efi_map_fb()

 drivers/firmware/efi/earlycon.c | 37 +++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
drivers/firmware/efi/earlycon.c +5 -0
index c9a0efca17b0..60b5350c6105 100644
@@ -16,15 +16,50 @@
 static const struct font_desc *font;
 static u32 efi_x, efi_y;
 static u64 fb_base;
+static u32 fb_size;
 static pgprot_t fb_prot;
+static void *efi_fb;
+
+/*
+ * efi earlycon needs to use early_memremap() to map the framebuffer.

drivers/firmware/efi/earlycon.c line 34

Quoted 10 lines
drivers/firmware/efi/earlycon.c +10 -0
+ * 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 early_efi_map_fb(void)
+{
+       if (!fb_base || !fb_size)
+               return NULL;
+

'return 0' please.

I'll test this on actual arm64 hardware somewhere next week, and queue it if it doesn't break anything (no need to resend for the above)

Quoted 43 lines
drivers/firmware/efi/earlycon.c +22 -1
+       if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL))
+               efi_fb = memremap(fb_base, fb_size, MEMREMAP_WB);
+       else
+               efi_fb = memremap(fb_base, fb_size, MEMREMAP_WC);
+
+       return efi_fb ? 0 : -ENOMEM;
+}
+early_initcall(early_efi_map_fb);
+
+static void __exit early_efi_unmap_fb(void)
+{
+       memunmap(efi_fb);
+}
+__exitcall(early_efi_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);
 }
 

@@ -176,6 +211,8 @@ static int __init efi_earlycon_setup(struct earlycon_device *device,
        if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
                fb_base |= (u64)screen_info.ext_lfb_base << 32;
 
+       fb_size = screen_info.lfb_size;
+
        if (opt && !strcmp(opt, "ram"))
                fb_prot = PAGE_KERNEL;
        else
--

2.24.0

author
#2 lore raw
Quoted 15 lines · Ard Biesheuvel
Quoted 12 lines · Andy Shevchenko

When the commit 69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation") moved x86 specific EFI earlyprintk implementation to shared location it also tweaked the behaviour. In particular it dropped a trick with full framebuffer remapping after page initialization. This lead to two regressions:
1) very slow scrolling after page initialization;
2) kernel hang when keep_bootcon parameter is being provided.

Returning the trick back fixes #2 and mitigates, i.e. reduces the window when slowness appears, #1 presumably due to eliminating heavy map()/unmap() operations per each pixel line on the screen.

Quoted 14 lines

drivers/firmware/efi/earlycon.c line 34

Quoted 12 lines
drivers/firmware/efi/earlycon.c +12 -0
+/*
+ * 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 early_efi_map_fb(void)
+{
+       if (!fb_base || !fb_size)
+               return NULL;
+

'return 0' please.

Ah, sorry. Though I think we rather return -ENOVEV here.

Quoted 2 lines

I'll test this on actual arm64 hardware somewhere next week, and queue it if it doesn't break anything (no need to resend for the above)

Thanks!

Quoted 8 lines
Quoted 8 lines
drivers/firmware/efi/earlycon.c +8 -0
+       if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL))
+               efi_fb = memremap(fb_base, fb_size, MEMREMAP_WB);
+       else
+               efi_fb = memremap(fb_base, fb_size, MEMREMAP_WC);
+
+       return efi_fb ? 0 : -ENOMEM;
+}
+early_initcall(early_efi_map_fb);
Signature · 2 lines
With Best Regards,
Andy Shevchenko
Tested-by
#3 lore raw
Quoted 32 lines · Andy Shevchenko
Quoted 15 lines · Ard Biesheuvel
Quoted 12 lines · Andy Shevchenko

When the commit 69c1f396f25b ("efi/x86: Convert x86 EFI earlyprintk into generic earlycon implementation") moved x86 specific EFI earlyprintk implementation to shared location it also tweaked the behaviour. In particular it dropped a trick with full framebuffer remapping after page initialization. This lead to two regressions:
1) very slow scrolling after page initialization;
2) kernel hang when keep_bootcon parameter is being provided.

Returning the trick back fixes #2 and mitigates, i.e. reduces the window when slowness appears, #1 presumably due to eliminating heavy map()/unmap() operations per each pixel line on the screen.

Quoted 14 lines

drivers/firmware/efi/earlycon.c line 34

Quoted 12 lines
drivers/firmware/efi/earlycon.c +12 -0
+/*
+ * 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 early_efi_map_fb(void)
+{
+       if (!fb_base || !fb_size)
+               return NULL;
+

'return 0' please.

Quoted 2 lines

Ah, sorry. Though I think we rather return -ENOVEV here.

fb_base and fb_size will only be set if earlycon=efifb is passed, so this is not an error condition, and so returning an error is incorrect.

Quoted 5 lines
Quoted 2 lines

I'll test this on actual arm64 hardware somewhere next week, and queue it if it doesn't break anything (no need to resend for the above)

Thanks!

Works fine for me

Tested-by Ard Biesheuvel <[email protected]> # arm64
Quoted 14 lines
Quoted 8 lines

drivers/firmware/efi/earlycon.c line 42

Quoted 8 lines
drivers/firmware/efi/earlycon.c +8 -0
+       if (pgprot_val(fb_prot) == pgprot_val(PAGE_KERNEL))
+               efi_fb = memremap(fb_base, fb_size, MEMREMAP_WB);
+       else
+               efi_fb = memremap(fb_base, fb_size, MEMREMAP_WC);
+
+       return efi_fb ? 0 : -ENOMEM;
+}
+early_initcall(early_efi_map_fb);

--
With Best Regards,
Andy Shevchenko