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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 12 Nov 2019 at 12:59, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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
>
>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux