lkml.org 
[lkml]   [2020]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [TEST PATCH RFC] Revert the EFI leak fixes for now (was: Re: EFI boot crash regression (was: Re: 5.6-### doesn't boot))
    On Sat, Feb 1, 2020 at 1:45 AM Ingo Molnar <mingo@kernel.org> wrote:
    >
    >
    >
    > Jörg Otte wrote:
    >
    > > It's bisected.
    > > The first bad commit is :
    > > 1db91035d01aa8bfa2350c00ccb63d629b4041ad
    > > efi: Add tracking for dynamically allocated memmaps
    >
    > > Unfortunately I can not revert because of compile errors!
    > >
    > > In file included from /media/jojo/deftoshiba/kernel/linux/init/main.c:48:
    > > /media/jojo/deftoshiba/kernel/linux/include/linux/efi.h:975:1: error:
    > > version control conflict marker in file
    > > <<<<<<< HEAD
    >
    > So 1db91035d0 doesn't revert cleanly, because 484a418d0754 depends on it,
    > plus there a third commit (f0ef6523475f) that has a semantic dependency
    > on 1db91035d01a.
    >
    > But you can revert them all, if done in reverse chronological order:
    >
    > git revert f0ef6523475f # ("efi: Fix efi_memmap_alloc() leaks")
    > git revert 484a418d0754 # ("efi: Fix handling of multiple efi_fake_mem= entries")
    > git revert 1db91035d01a # ("efi: Add tracking for dynamically allocated memmaps")
    >
    > You can paste those three lines into a shell as-is, or you can apply the
    > patch below which is a combination of these three reverts.
    >
    > Jörg, can you confirm that doing these reverts fixes booting on your
    > system? If it does then a full dmesg from that kernel would be useful.
    >
    > FWIW I reviewed the bisected commit and didn't find the bug but I also
    > couldn't convince myself it's a 1:1 identity transformation and cleanup
    > of the existing logic.
    >
    > The size arithmethics transformation looks correct at first sight, but
    > the data->flags handling in particular looks rather interwoven.

    Agreed, but the only flags change that I couldn't convince myself was
    correct is this:

    diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
    index 59f7f6d60cf6..314b36ac2a08 100644
    --- a/arch/x86/platform/efi/efi.c
    +++ b/arch/x86/platform/efi/efi.c
    @@ -309,7 +309,7 @@ static void __init efi_clean_memmap(void)
    .desc_version = efi.memmap.desc_version,
    .desc_size = efi.memmap.desc_size,
    .size = data.desc_size * (efi.memmap.nr_map -
    n_removal),
    - .flags = 0,
    + .flags = efi.memmap.flags & EFI_MEMMAP_LATE,
    };

    pr_warn("Removing %d invalid memory map entries.\n", n_removal);
    ...but efi_clean_memmap() should "early".

    \
     
     \ /
      Last update: 2020-02-01 17:24    [W:3.029 / U:0.400 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site