Re: [PATCH 0/3] Fix ACPI BGRT support for images located in EFI boot services memory

Matt Fleming
2012-09-04T20:11:14Z

On Tue, 2012-09-04 at 10:59 -0700, Josh Triplett wrote:
> On Tue, Sep 04, 2012 at 03:27:20PM +0100, Matt Fleming wrote:
> > On Thu, 2012-08-30 at 14:28 -0700, Josh Triplett wrote:
> > > The ACPI BGRT lets the OS access the BIOS logo image and its position on the
> > > screen at boot time, allowing it to maintain that image on the screen until
> > > ready to display something else, making boot more seamless. This series fixes
> > > support for accessing the boot logo image via the BGRT when the BIOS stores it
> > > in EFI boot services memory, as recommended by the ACPI 5.0 spec. Linux needs
> > > to copy the image out of boot services memory before reclaiming boot services
> > > memory.
> > >
> > > The first patch refactors EFI initialization to defer freeing boot services
> > > memory until later in the boot process, after we have ACPI available. The
> > > second patch adds a helper function to look up existing EFI boot services
> > > mappings, to avoid re-mapping them. The third patch moves BGRT initialization
> > > to before the reclamation of boot services memory, copies the logo at that
> > > point, and reworks the existing BGRT driver to use that existing copy.
> >
> > Since we always end up doing a copy anyway, is there no way we could
> > just copy the boot logo *without* deferring freeing the boot services
> > code, e.g. move the copy before we do SetVirtualAddressMap()?
>
> Unfortunately not. We need enough of ACPI available to go read the
> BGRT to know what to copy, so we need to defer freeing boot services
> code until after we initialize ACPI (and thus everything ACPI needs,
> which includes EFI since ACPI looks for root tables there).

Ah, right. It was also pointed out to me offline that some drivers have
been known to access boot services data even after
SetVirtualAddressMap(), so this deferring shouldn't be a problem.

> > I wouldn't be surprised if some implementations got really cranky if
> > we accessed boot services data after we installed a new virtual memory
> > map.
>
> Note that I've carefully accessed the boot services data *through* the
> new virtual memory map, which should work fine.
>
> > Besides, if we can avoid moving the efi_free_boot_services() call we can
> > avoid littering init/main.c with more #ifdef CONFIG_X86 blocks.
>
> Those seem easy enough to convert into appropriate always-available
> stubs, if you'd like. And I could move efi_free_boot_services() inside
> efi_late_init(), too, keeping it an internal implementation detail of
> EFI initialization. Would that help?

Yeah, that would seem like a good solution.