* [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization
@ 2019-11-11 15:44 Andy Shevchenko
2019-11-11 17:59 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-11-11 15:44 UTC (permalink / raw)
To: linux-efi, mika.westerberg
Cc: Andy Shevchenko, Ard Biesheuvel, Alexander Graf, Matt Fleming
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 <ard.biesheuvel@linaro.org>
Cc: Alexander Graf <agraf@suse.de>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/firmware/efi/earlycon.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index c9a0efca17b0..ddb6c0ff2553 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -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);
}
--
2.24.0.rc1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization
2019-11-11 15:44 [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization Andy Shevchenko
@ 2019-11-11 17:59 ` Ard Biesheuvel
2019-11-12 12:57 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-11-11 17:59 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-efi, Mika Westerberg, Alexander Graf, Matt Fleming
On Mon, 11 Nov 2019 at 15:44, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> 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 <ard.biesheuvel@linaro.org>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi Andy,
Thanks for fixing this. One question below.
> ---
> drivers/firmware/efi/earlycon.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
> index c9a0efca17b0..ddb6c0ff2553 100644
> --- a/drivers/firmware/efi/earlycon.c
> +++ b/drivers/firmware/efi/earlycon.c
> @@ -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)
Will there be a user for this routine? If not, can we just drop it?
> +{
> + 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization
2019-11-11 17:59 ` Ard Biesheuvel
@ 2019-11-12 12:57 ` Andy Shevchenko
2019-11-12 12:59 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-11-12 12:57 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, Mika Westerberg, Alexander Graf, Matt Fleming
On Mon, Nov 11, 2019 at 05:59:42PM +0000, Ard Biesheuvel wrote:
> On Mon, 11 Nov 2019 at 15:44, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > 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.
> Thanks for fixing this. One question below.
>
> > +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)
>
> 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.
> > +{
> > + memunmap(efi_fb);
> > +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization
2019-11-12 12:57 ` Andy Shevchenko
@ 2019-11-12 12:59 ` Andy Shevchenko
2019-11-12 13:31 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-11-12 12:59 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, Mika Westerberg, Alexander Graf, Matt Fleming
On Tue, Nov 12, 2019 at 02:57:54PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 11, 2019 at 05:59:42PM +0000, Ard Biesheuvel wrote:
> > On Mon, 11 Nov 2019 at 15:44, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > 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.
>
> > Thanks for fixing this. One question below.
> >
> > > +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)
> >
> > 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).
I noted that I missed __exitcall() here. But will wait for your answer.
> 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.
>
> > > +{
> > > + memunmap(efi_fb);
> > > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization
2019-11-12 12:59 ` Andy Shevchenko
@ 2019-11-12 13:31 ` Ard Biesheuvel
2019-11-12 16:13 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-11-12 13:31 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-efi, Mika Westerberg, Alexander Graf, Matt Fleming
On Tue, 12 Nov 2019 at 12:59, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Nov 12, 2019 at 02:57:54PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 11, 2019 at 05:59:42PM +0000, Ard Biesheuvel wrote:
> > > On Mon, 11 Nov 2019 at 15:44, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > 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.
> >
> > > Thanks for fixing this. One question below.
> > >
> > > > +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)
> > >
> > > 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).
>
> I noted that I missed __exitcall() here. But will wait for your answer.
>
Ah ok, then it makes sense. Mind respinning with that added?
> > 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.
> >
> > > > +{
> > > > + memunmap(efi_fb);
> > > > +}
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization
2019-11-12 13:31 ` Ard Biesheuvel
@ 2019-11-12 16:13 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2019-11-12 16:13 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, Mika Westerberg, Alexander Graf, Matt Fleming
On Tue, Nov 12, 2019 at 01:31:11PM +0000, Ard Biesheuvel wrote:
> On Tue, 12 Nov 2019 at 12:59, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Nov 12, 2019 at 02:57:54PM +0200, Andy Shevchenko wrote:
> > > On Mon, Nov 11, 2019 at 05:59:42PM +0000, Ard Biesheuvel wrote:
> > > > On Mon, 11 Nov 2019 at 15:44, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > 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).
> >
> > 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!
> > > 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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-12 16:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-11 15:44 [PATCH v1] efi/earlycon: Remap entire framebuffer after page initialization Andy Shevchenko
2019-11-11 17:59 ` Ard Biesheuvel
2019-11-12 12:57 ` Andy Shevchenko
2019-11-12 12:59 ` Andy Shevchenko
2019-11-12 13:31 ` Ard Biesheuvel
2019-11-12 16:13 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.