RE: [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered

[Ghannam, Yazen]

> -----Original Message-----
> From: linux-kernel-owner@xxxxxxxxxxxxxxx <linux-kernel-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Ard Biesheuvel
> Sent: Saturday, February 2, 2019 3:41 AM
> To: linux-efi@xxxxxxxxxxxxxxx; Ingo Molnar <mingo@xxxxxxxxxx>; Thomas
> Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx>; Alexander Graf
> <agraf@xxxxxxx>; Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>; Borislav
> Petkov <bp@xxxxxxxxx>; Heinrich Schuchardt <xypron.glpk@xxxxxx>; Jeffrey
> Hugo <jhugo@xxxxxxxxxxxxxx>; Lee Jones <lee.jones@xxxxxxxxxx>; Leif
> Lindholm <leif.lindholm@xxxxxxxxxx>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Jones <pjones@xxxxxxxxxx>; Peter Zijlstra
> <peterz@xxxxxxxxxxxxx>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@xxxxxxxxx>
> Subject: [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it
> gets clobbered
> 
> The bitmap left in the framebuffer by the firmware is described by an
> ACPI table called "BGRT", which describes the size, pixel format and
> the address of a BMP image in memory. While the BGRT ACPI table is
> guaranteed to reside in a "ACPI reclaim" memory region, which is
> never touched by Linux. The BMP image, however, typically resides
> in EFI Boot Services Memory, which may have been overwritten by the
> time the BGRT discovery routine runs.
> 
> So instead, drop the handling from the ACPI init code, and call the
> BGRT parsing code immediately after going over the EFI configuration
> table array, at which time no memory has been touched yet except for
> the .data/.bss regions covered by the static kernel image.
> 
> Unfortunately, this involves a non-trivial amount of ACPI entry
> point and root table parsing, but we cannot rely on the normal
> ACPI infrastructure yet this early in the boot.
> 
> Also note that we cannot take the 'acpi_disabled' global variable
> into account, since it may not have assumed the correct value yet
> (on arm64, the default value is '1' which is overridden to '0' if
> no DT description has been made available by the firmware)
> 
> Cc: Peter Jones <pjones@xxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---

Hi Ard, et. al.,

I'm trying out tip/master and I find that my system panics early during boot. Reverting
this patch seems to resolve the issue. Please see the trace below.

I've started debugging, but I'm not familiar with this code. Please let me know if you
have any ideas or if there's anything you'd like me to try.

Thanks,
Yazen

[    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x0000000000000b40 bytes below 0x0000000000000000.
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc5-merged-bases+ #101
[    0.000000] Call Trace:
[    0.000000]  dump_stack+0x63/0x85
[    0.000000]  panic+0xfe/0x2a4
[    0.000000]  memblock_alloc_base+0x33/0x35
[    0.000000]  memblock_phys_alloc+0x10/0x12
[    0.000000]  efi_memmap_alloc+0x62/0x65
[    0.000000]  efi_arch_mem_reserve+0x10e/0x194
[    0.000000]  efi_mem_reserve+0x31/0x36
[    0.000000]  ? efi_mem_reserve+0x31/0x36
[    0.000000]  efi_bgrt_init+0x2c6/0x2e0
[    0.000000]  efi_config_parse_tables+0x1b2/0x1dd
[    0.000000]  efi_config_init+0x7b/0x9f
[    0.000000]  ? efi_config_init+0x7b/0x9f
[    0.000000]  efi_init+0x366/0x465
[    0.000000]  ? 0xffffffff87800000
[    0.000000]  setup_arch+0x42f/0xcc9
[    0.000000]  ? printk+0x52/0x6e
[    0.000000]  start_kernel+0x6c/0x516
[    0.000000]  x86_64_start_reservations+0x24/0x26
[    0.000000]  x86_64_start_kernel+0x74/0x77
[    0.000000]  secondary_startup_64+0xa4/0xb0
[    0.000000] ---[ end Kernel panic - not syncing: ERROR: Failed to allocate 0x0000000000000b40 bytes below 0x0000000000000000. ]---