efi/earlycon: Remap entire framebuffer after page initialization
PATCH 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.
--- drivers/firmware/efi/earlycon.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
drivers/firmware/efi/earlycon.c +33 -0
@@ -17,14 +17,47 @@ 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 __init int early_efi_map_fb (void ) +{ +u32 size; + + size = screen_info.lfb_size; +if (pgprot_val (fb_prot) ==pgprot_val (PAGE_KERNEL )) + efi_fb =memremap (fb_base, size,MEMREMAP_WB ); +else + efi_fb =memremap (fb_base, size,MEMREMAP_WC ); + +return efi_fb ?0 : -ENOMEM ; +} +early_initcall (early_efi_map_fb); + +static __exit void early_efi_unmap_fb(void) +{ +memunmap (efi_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); }
Signature · 1 line
2.24.0.rc1
Quoted 13 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 whenkeep_bootconparameter 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 5 lines
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]>
Hi Andy,
Thanks for fixing this. One question below.
Quoted 26 lines
--- drivers/firmware/efi/earlycon.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)drivers/firmware/efi/earlycon.c +14
-0@@ -17,14 +17,47 @@ 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 __init int early_efi_map_fb (void ) +{ +u32 size; + + size = screen_info.lfb_size;
drivers/firmware/efi/earlycon.c static const struct font_desc *font; · line 43
Quoted 10 lines
drivers/firmware/efi/earlycon.c +10
-0+if (pgprot_val (fb_prot) ==pgprot_val (PAGE_KERNEL )) + efi_fb =memremap (fb_base, size,MEMREMAP_WB ); +else + efi_fb =memremap (fb_base, size,MEMREMAP_WC ); + +return efi_fb ?0 : -ENOMEM ; +} +early_initcall (early_efi_map_fb); + +static __exit void early_efi_unmap_fb(void)
Will there be a user for this routine? If not, can we just drop it?
Quoted 23 lines
drivers/firmware/efi/earlycon.c +9
-1+{ +memunmap (efi_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); } --2.24.0.rc1
Quoted 26 lines · Ard Biesheuvel
Quoted 9 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 whenkeep_bootconparameter is being provided.Quoted 3 lines
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.
Thanks for fixing this. One question below.
Quoted 9 lines
drivers/firmware/efi/earlycon.c +9
-0+static __init int early_efi_map_fb (void ) +{ +u32 size; + + size = screen_info.lfb_size; +if (pgprot_val (fb_prot) ==pgprot_val (PAGE_KERNEL )) + efi_fb =memremap (fb_base, size,MEMREMAP_WB ); +else + efi_fb =memremap (fb_base, size,MEMREMAP_WC );
Quoted 8 lines
drivers/firmware/efi/earlycon.c static const struct font_desc *font; · line 43
Quoted 6 lines
drivers/firmware/efi/earlycon.c +6
-0+ +return efi_fb ?0 : -ENOMEM ; +} +early_initcall (early_efi_map_fb); + +static __exit void early_efi_unmap_fb(void)Will there be a user for this routine? If not, can we just drop it?
The same question can be applied to the driver core part(s), e.g. deferred_probe_exit() in dd.c).
The above basically what Greg KH told people to do. While it is partially cargo cult here I can imagine that in some environments (virtual or kexec) somebody would like to get a picture of (post-mortem?) analysis where it would be helpful. Also code looks symmetrical in order to resource management. So, if you insist, I'll remove it, although I personally like my variant.
Quoted 3 lines
Quoted 3 lines
drivers/firmware/efi/earlycon.c +3
-0+{ +memunmap (efi_fb); +}
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 36 lines · Andy Shevchenko
Quoted 26 lines · Ard Biesheuvel
Quoted 9 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 whenkeep_bootconparameter is being provided.Quoted 3 lines
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.
Thanks for fixing this. One question below.
Quoted 9 lines
drivers/firmware/efi/earlycon.c +9
-0+static __init int early_efi_map_fb (void ) +{ +u32 size; + + size = screen_info.lfb_size; +if (pgprot_val (fb_prot) ==pgprot_val (PAGE_KERNEL )) + efi_fb =memremap (fb_base, size,MEMREMAP_WB ); +else + efi_fb =memremap (fb_base, size,MEMREMAP_WC );Quoted 8 lines
drivers/firmware/efi/earlycon.c static const struct font_desc *font; · line 43
Quoted 6 lines
drivers/firmware/efi/earlycon.c +6
-0+ +return efi_fb ?0 : -ENOMEM ; +} +early_initcall (early_efi_map_fb); + +static __exit void early_efi_unmap_fb(void)Will there be a user for this routine? If not, can we just drop it?
Quoted 2 lines
The same question can be applied to the driver core part(s), e.g.
deferred_probe_exit()indd.c).
I noted that I missed __exitcall() here. But will wait for your answer.
Quoted 15 lines
The above basically what Greg KH told people to do. While it is partially cargo cult here I can imagine that in some environments (virtual or kexec) somebody would like to get a picture of (post-mortem?) analysis where it would be helpful. Also code looks symmetrical in order to resource management. So, if you insist, I'll remove it, although I personally like my variant.
Quoted 3 lines
Quoted 3 lines
drivers/firmware/efi/earlycon.c +3
-0+{ +memunmap (efi_fb); +}Signature · 2 lines
With Best Regards, Andy Shevchenko
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 38 lines · Andy Shevchenko
Quoted 36 lines · Andy Shevchenko
Quoted 34 lines · Ard Biesheuvel
Quoted 9 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 whenkeep_bootconparameter is being provided.Quoted 3 lines
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.
Thanks for fixing this. One question below.
Quoted 5 lines
drivers/firmware/efi/earlycon.c +5
-0+static __init int early_efi_map_fb (void ) +{ +u32 size; + + size = screen_info.lfb_size;drivers/firmware/efi/earlycon.c static const struct font_desc *font; · line 43
Quoted 10 lines
drivers/firmware/efi/earlycon.c +10
-0+if (pgprot_val (fb_prot) ==pgprot_val (PAGE_KERNEL )) + efi_fb =memremap (fb_base, size,MEMREMAP_WB ); +else + efi_fb =memremap (fb_base, size,MEMREMAP_WC ); + +return efi_fb ?0 : -ENOMEM ; +} +early_initcall (early_efi_map_fb); + +static __exit void early_efi_unmap_fb(void)Will there be a user for this routine? If not, can we just drop it?
Quoted 5 lines
Quoted 2 lines
The same question can be applied to the driver core part(s), e.g.
deferred_probe_exit()indd.c).I noted that I missed
__exitcall()here. But will wait for your answer.
Ah ok, then it makes sense. Mind respinning with that added?
Quoted 21 lines
Quoted 6 lines
The above basically what Greg KH told people to do. While it is partially cargo cult here I can imagine that in some environments (virtual or kexec) somebody would like to get a picture of (post-mortem?) analysis where it would be helpful. Also code looks symmetrical in order to resource management. So, if you insist, I'll remove it, although I personally like my variant.
Quoted 9 lines
Quoted 3 lines
drivers/firmware/efi/earlycon.c static const struct font_desc *font; · line 46
Quoted 3 lines
drivers/firmware/efi/earlycon.c +3
-0+{ +memunmap (efi_fb); +}--
With Best Regards,
Andy Shevchenko--
With Best Regards,
Andy Shevchenko
Quoted 9 lines · Ard Biesheuvel
Quoted 7 lines · Andy Shevchenko
Quoted 6 lines · Andy Shevchenko
Quoted 4 lines · Ard Biesheuvel
On Mon, 11 Nov 2019 at 15:44, Andy Shevchenko
<[email protected]> wrote:Will there be a user for this routine? If not, can we just drop it?
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
The same question can be applied to the driver core part(s), e.g.
deferred_probe_exit()indd.c).I noted that I missed
__exitcall()here. But will wait for your answer.Ah ok, then it makes sense. Mind respinning with that added?
Done!
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
The above basically what Greg KH told people to do. While it is partially cargo cult here I can imagine that in some environments (virtual or kexec) somebody would like to get a picture of (post-mortem?) analysis where it would be helpful. Also code looks symmetrical in order to resource management. So, if you insist, I'll remove it, although I personally like my variant.
Signature · 2 lines
With Best Regards, Andy Shevchenko