lkml.org 
[lkml]   [2017]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 08/10] efi/x86: Move EFI BGRT init code to early init code
2017-05-15, 16:37:40 +0800, Dave Young wrote:
> Hi,
>
> Thanks for the report.
> On 05/14/17 at 01:18am, Sabrina Dubroca wrote:
> > 2017-01-31, 13:21:40 +0000, Ard Biesheuvel wrote:
> > > From: Dave Young <dyoung@redhat.com>
> > >
> > > Before invoking the arch specific handler, efi_mem_reserve() reserves
> > > the given memory region through memblock.
> > >
> > > efi_bgrt_init() will call efi_mem_reserve() after mm_init(), at which
> > > time memblock is dead and should not be used anymore.
> > >
> > > The EFI BGRT code depends on ACPI initialization to get the BGRT ACPI
> > > table, so move parsing of the BGRT table to ACPI early boot code to
> > > ensure that efi_mem_reserve() in EFI BGRT code still use memblock safely.
> > >
> > > Signed-off-by: Dave Young <dyoung@redhat.com>
> > > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > Cc: Len Brown <lenb@kernel.org>
> > > Cc: linux-acpi@vger.kernel.org
> > > Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > I have a box that panics in early boot after this patch. The kernel
> > config is based on a Fedora 25 kernel + localmodconfig.
> >
> > BUG: unable to handle kernel paging request at ffffffffff240001
> > IP: efi_bgrt_init+0xdc/0x134
> > PGD 1ac0c067
> > PUD 1ac0e067
> > PMD 1aee9067
> > PTE 9380701800000163
> >
> > Oops: 0009 [#1] SMP
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc5-00116-g7b0a911 #19
> > Hardware name: Hewlett-Packard HP Z220 CMT Workstation/1790, BIOS K51 v01.02 05/03/2012
> > task: ffffffff9fc10500 task.stack: ffffffff9fc00000
> > RIP: 0010:efi_bgrt_init+0xdc/0x134
> > RSP: 0000:ffffffff9fc03d58 EFLAGS: 00010082
> > RAX: ffffffffff240001 RBX: 0000000000000000 RCX: 1380701800000006
> > RDX: 8000000000000163 RSI: 9380701800000163 RDI: 00000000000005be
> > RBP: ffffffff9fc03d70 R08: 1380701800001000 R09: 0000000000000002
> > R10: 000000000002d000 R11: ffff98a3dedd2fc6 R12: ffffffff9f9f22b6
> > R13: ffffffff9ff49480 R14: 0000000000000010 R15: 0000000000000000
> > FS: 0000000000000000(0000) GS:ffffffff9fd20000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: ffffffffff240001 CR3: 000000001ac09000 CR4: 00000000000406b0
> > Call Trace:
> > ? acpi_parse_ioapic+0x98/0x98
> > acpi_parse_bgrt+0x9/0xd
> > acpi_table_parse+0x7a/0xa9
> > acpi_boot_init+0x3c7/0x4f9
> > ? acpi_parse_x2apic+0x74/0x74
> > ? acpi_parse_x2apic_nmi+0x46/0x46
> > setup_arch+0xb4b/0xc6f
> > ? printk+0x52/0x6e
> > start_kernel+0xb2/0x47b
> > ? early_idt_handler_array+0x120/0x120
> > x86_64_start_reservations+0x24/0x26
> > x86_64_start_kernel+0xf7/0x11a
> > start_cpu+0x14/0x14
> > Code: 48 c7 c7 10 16 a0 9f e8 4e 94 40 ff eb 62 be 06 00 00 00 e8 f9 ff 00 00 48 85 c0 75 0e 48 c7 c7 40 16 a0 9f e8 31 94 40 ff eb 45 <66> 44 8b 20 be 06 00 00 00 48 89 c7 8b 58 02 e8 87 00 01 00 66
> > RIP: efi_bgrt_init+0xdc/0x134 RSP: ffffffff9fc03d58
> > CR2: ffffffffff240001
> > ---[ end trace f68728a0d3053b52 ]---
> > Kernel panic - not syncing: Attempted to kill the idle task!
> > ---[ end Kernel panic - not syncing: Attempted to kill the idle task!
> >
> >
> > That code is:
> >
> >
> > All code
> > ========
> > 0: 48 c7 c7 10 16 a0 9f mov $0xffffffff9fa01610,%rdi
> > 7: e8 4e 94 40 ff callq 0xffffffffff40945a
> > c: eb 62 jmp 0x70
> > e: be 06 00 00 00 mov $0x6,%esi
> > 13: e8 f9 ff 00 00 callq 0x10011
> > 18: 48 85 c0 test %rax,%rax
> > 1b: 75 0e jne 0x2b
> > 1d: 48 c7 c7 40 16 a0 9f mov $0xffffffff9fa01640,%rdi
> > 24: e8 31 94 40 ff callq 0xffffffffff40945a
> > 29: eb 45 jmp 0x70
> > 2b:* 66 44 8b 20 mov (%rax),%r12w <-- trapping instruction
> > 2f: be 06 00 00 00 mov $0x6,%esi
> > 34: 48 89 c7 mov %rax,%rdi
> > 37: 8b 58 02 mov 0x2(%rax),%ebx
> > 3a: e8 87 00 01 00 callq 0x100c6
> > 3f: 66 data16
> >
> > Code starting with the faulting instruction
> > ===========================================
> > 0: 66 44 8b 20 mov (%rax),%r12w
> > 4: be 06 00 00 00 mov $0x6,%esi
> > 9: 48 89 c7 mov %rax,%rdi
> > c: 8b 58 02 mov 0x2(%rax),%ebx
> > f: e8 87 00 01 00 callq 0x1009b
> > 14: 66 data16
> >
> >
> > which is just after the early_memremap() call.
> >
> > I enabled early_ioremap_debug and the last warning had:
> >
> > __early_ioremap(1380701800001000, 00001000) [1] => 00000001 + ffffffffff240000
>
> The phys addr looks odd..
>
> From the kernel log, I do not see any efi messages so can you check if
> you are booting with legacy mode or efi boot?

I don't have physical access to the machine, but even from a succesful
boot there's no efi message. No /sys/firmware/efi as well, and
efivarfs isn't registered despite it being compiled in
(on kernel 4.10.14-200.fc25.x86_64):

# mount -t efivarfs none /mnt/foo
mount: unknown filesystem type 'efivarfs'

So I suppose it's legacy mode and the
!efi_enabled(EFI_RUNTIME_SERVICES)
check kicking in.


> I suppose bgrt are efi only, if you are test with legacy boot it is odd
> that there is BGRT table populated.
>
> For debugging purpose maybe you can add some printk to dump the acpi
> table header in efi_bgrt_init function, just print the version, status,
> image_type, image_address.

Added:

pr_info("%s acpi_table_bgrt.version %hu\n", __func__, bgrt->version);
pr_info("%s acpi_table_bgrt.status %hhu\n", __func__, bgrt->status);
pr_info("%s acpi_table_bgrt.image_type %hhu\n", __func__, bgrt->image_type);
pr_info("%s acpi_table_bgrt.image_address %llx\n", __func__, bgrt->image_address);
print_hex_dump(KERN_INFO, "efi_bgrt_init acpi_table_bgrt", DUMP_PREFIX_OFFSET, 16, 1, bgrt, sizeof(*bgrt), false);

efi_bgrt: efi_bgrt_init acpi_table_bgrt.version 1
efi_bgrt: efi_bgrt_init acpi_table_bgrt.status 0
efi_bgrt: efi_bgrt_init acpi_table_bgrt.image_type 0
efi_bgrt: efi_bgrt_init acpi_table_bgrt.image_address 1380701800000001
efi_bgrt_init acpi_table_bgrt: 00000000: 42 47 52 54 3c 00 00 00 00 8b 48 50 51 4f 45 4d
efi_bgrt_init acpi_table_bgrt: 00000010: 53 4c 49 43 2d 57 4b 53 09 20 07 01 41 4d 49 20
efi_bgrt_init acpi_table_bgrt: 00000020: 13 00 01 00 01 00 00 00 01 00 00 00 18 70 80 13
efi_bgrt_init acpi_table_bgrt: 00000030: 00 00 00 00 ff 00 00 00


> If you can prove it is a non-efi boot, then maybe you can test below
> patch:

Yeah, that works. I guess that makes sense, since before this patch,
efi_bgrt_init() wasn't called on that box (because of the
EFI_RUNTIME_SERVICES check in start_kernel()).

Thanks!

> diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
> index 04ca876..b986e26 100644
> --- a/arch/x86/platform/efi/efi-bgrt.c
> +++ b/arch/x86/platform/efi/efi-bgrt.c
> @@ -36,6 +36,9 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
> if (acpi_disabled)
> return;
>
> + if (!efi_enabled(EFI_CONFIG_TABLES))
> + return;
> +
> if (table->length < sizeof(bgrt_tab)) {
> pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n",
> table->length, sizeof(bgrt_tab));
>

--
Sabrina

\
 
 \ /
  Last update: 2017-05-15 13:11    [W:0.067 / U:8.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site