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

2019-02-05 Thread Ard Biesheuvel
On Tue, 5 Feb 2019 at 19:07, Ghannam, Yazen  wrote:
>
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org  > ow...@vger.kernel.org> On Behalf Of Ard Biesheuvel
> > Sent: Saturday, February 2, 2019 3:41 AM
> > To: linux-efi@vger.kernel.org; Ingo Molnar ; Thomas
> > Gleixner 
> > Cc: Ard Biesheuvel ; 
> > linux-ker...@vger.kernel.org;
> > AKASHI Takahiro ; Alexander Graf
> > ; Bjorn Andersson ; Borislav
> > Petkov ; Heinrich Schuchardt ; Jeffrey
> > Hugo ; Lee Jones ; Leif
> > Lindholm ; Linus Torvalds  > foundation.org>; Peter Jones ; Peter Zijlstra
> > ; Sai Praneeth Prakhya
> > 
> > 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 
> > Signed-off-by: Ard Biesheuvel 
> > ---
>
> 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.
>

Hi Yazen,

Thanks for the report, you are the second person to flag this issue,
so in the mean time, I have asked Ingo to drop it from the efi/core
queue, and so the patch will be gone from -next as soon as it
refreshes.

I'll cc you on the updated version of this patch once I get around to
looking into it, which will probably be around early next week.

Thanks,
Ard.


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


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

2019-02-05 Thread Ghannam, Yazen
> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  ow...@vger.kernel.org> On Behalf Of Ard Biesheuvel
> Sent: Saturday, February 2, 2019 3:41 AM
> To: linux-efi@vger.kernel.org; Ingo Molnar ; Thomas
> Gleixner 
> Cc: Ard Biesheuvel ; linux-ker...@vger.kernel.org;
> AKASHI Takahiro ; Alexander Graf
> ; Bjorn Andersson ; Borislav
> Petkov ; Heinrich Schuchardt ; Jeffrey
> Hugo ; Lee Jones ; Leif
> Lindholm ; Linus Torvalds  foundation.org>; Peter Jones ; Peter Zijlstra
> ; Sai Praneeth Prakhya
> 
> 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 
> Signed-off-by: Ard Biesheuvel 
> ---

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