[PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Ard Biesheuvel]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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 <andriy.shevchenko@linux.intel.com>
[ardb: ensure efifb is unmapped again unless keep_bootcon is in effect]
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 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

Re: [PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Ard Biesheuvel]

On Thu, 5 Dec 2019 at 16:42, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> 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 <andriy.shevchenko@linux.intel.com>
> [ardb: ensure efifb is unmapped again unless keep_bootcon is in effect]
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  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));
>  }
>

Actually, now I'm not so sure we really need the early map/unmap for
every line. If we switch from an early mapping to an ordinary mapping
at early_initcall() time, we can just map it once, and tear it down
when we do the ordinary memremap()


>  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
>

Re: [PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Ard Biesheuvel]

On Thu, 5 Dec 2019 at 17:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 5 Dec 2019 at 16:42, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > 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 <andriy.shevchenko@linux.intel.com>
> > [ardb: ensure efifb is unmapped again unless keep_bootcon is in effect]
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  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));
> >  }
> >
>
> Actually, now I'm not so sure we really need the early map/unmap for
> every line. If we switch from an early mapping to an ordinary mapping
> at early_initcall() time, we can just map it once, and tear it down
> when we do the ordinary memremap()
>

Never mind ... early_memremap() cannot map the entire framebuffer in one go.

>
> >  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
> >

Re: [PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Andy Shevchenko]

On Thu, Dec 05, 2019 at 04:42:48PM +0000, Ard Biesheuvel wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> 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 <andriy.shevchenko@linux.intel.com>
> [ardb: ensure efifb is unmapped again unless keep_bootcon is in effect]
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  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))

Isn't efi_fb test superfluous here?

> +		memunmap(efi_fb);

> +	return 0;
> +}
> +late_initcall(efi_earlycon_unmap_fb);

I still think we need __exitcall() to clean up the stuff in any case.

>  
>  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
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Ard Biesheuvel]

On Thu, 5 Dec 2019 at 18:56, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Dec 05, 2019 at 04:42:48PM +0000, Ard Biesheuvel wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > 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 <andriy.shevchenko@linux.intel.com>
> > [ardb: ensure efifb is unmapped again unless keep_bootcon is in effect]
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  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))
>
> Isn't efi_fb test superfluous here?
>

Well, it could be NULL so it is not superfluous. The initcall() is
always executed, even if you are not using earlycon=efifb at all.

> > +             memunmap(efi_fb);
>
> > +     return 0;
> > +}
> > +late_initcall(efi_earlycon_unmap_fb);
>
> I still think we need __exitcall() to clean up the stuff in any case.
>

Why? As far as I can tell, exitcalls go straight into a /DISCARD/
section, and I really don't see the point of carrying dead code in
this file.

> >
> >  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
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Andy Shevchenko]

On Thu, Dec 05, 2019 at 07:02:56PM +0000, Ard Biesheuvel wrote:
> On Thu, 5 Dec 2019 at 18:56, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Dec 05, 2019 at 04:42:48PM +0000, Ard Biesheuvel wrote:

> > > +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))
> >
> > Isn't efi_fb test superfluous here?
> >
> 
> Well, it could be NULL so it is not superfluous. The initcall() is
> always executed, even if you are not using earlycon=efifb at all.

My point is that memunmap() is no-op for NULL pointer, isn't it?

> > > +             memunmap(efi_fb);
> >
> > > +     return 0;
> > > +}
> > > +late_initcall(efi_earlycon_unmap_fb);

> > I still think we need __exitcall() to clean up the stuff in any case.
> >
> 
> Why? As far as I can tell, exitcalls go straight into a /DISCARD/
> section, and I really don't see the point of carrying dead code in
> this file.

Okay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] efi/earlycon: Remap entire framebuffer after page initialization

[Ard Biesheuvel]

On Thu, 5 Dec 2019 at 19:49, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Dec 05, 2019 at 07:02:56PM +0000, Ard Biesheuvel wrote:
> > On Thu, 5 Dec 2019 at 18:56, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, Dec 05, 2019 at 04:42:48PM +0000, Ard Biesheuvel wrote:
>
> > > > +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))
> > >
> > > Isn't efi_fb test superfluous here?
> > >
> >
> > Well, it could be NULL so it is not superfluous. The initcall() is
> > always executed, even if you are not using earlycon=efifb at all.
>
> My point is that memunmap() is no-op for NULL pointer, isn't it?
>

Right. So I could check for earlycon_console != NULL instead, since
that is implied by efi_fb != NULL, but it could be NULL otherwise, in
which case I should not dereference it to get at the flags. But I need
two checks in any case, so I think this is OK.

> > > > +             memunmap(efi_fb);
> > >
> > > > +     return 0;
> > > > +}
> > > > +late_initcall(efi_earlycon_unmap_fb);
>
> > > I still think we need __exitcall() to clean up the stuff in any case.
> > >
> >
> > Why? As far as I can tell, exitcalls go straight into a /DISCARD/
> > section, and I really don't see the point of carrying dead code in
> > this file.
>
> Okay.
>

Thanks,