From: Dan Williams <dan.j.williams@intel.com> To: mingo@redhat.com Cc: Dave Young <dyoung@redhat.com>, Taku Izumi <izumi.taku@jp.fujitsu.com>, Michael Weiser <michael@weiser.dinsnail.net>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>, linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, kexec@lists.infradead.org, x86@kernel.org Subject: [PATCH v2 4/4] efi: Fix handling of multiple efi_fake_mem= entries Date: Tue, 31 Dec 2019 14:04:38 -0800 [thread overview] Message-ID: <157782987865.367056.15199592105978588123.stgit@dwillia2-desk3.amr.corp.intel.com> (raw) In-Reply-To: <157782985777.367056.14741265874314204783.stgit@dwillia2-desk3.amr.corp.intel.com> Dave noticed that when specifying multiple efi_fake_mem= entries only the last entry was successfully being reflected in the efi memory map. This is due to the fact that the efi_memmap_insert() is being called multiple times, but on successive invocations the insertion should be applied to the last new memmap rather than the original map at efi_fake_memmap() entry. Rework efi_fake_memmap() to install the new memory map after each efi_fake_mem= entry is parsed. This also fixes an issue in efi_fake_memmap() that caused it to litter emtpy entries into the end of the efi memory map. The empty entry causes efi_memmap_insert() to attempt more memmap splits / copies than efi_memmap_split_count() accounted for when sizing the new map. BUG: unable to handle page fault for address: ffffffffff281000 [..] RIP: 0010:efi_memmap_insert+0x11d/0x191 [..] Call Trace: ? bgrt_init+0xbe/0xbe ? efi_arch_mem_reserve+0x1cb/0x228 ? acpi_parse_bgrt+0xa/0xd ? acpi_table_parse+0x86/0xb8 ? acpi_boot_init+0x494/0x4e3 ? acpi_parse_x2apic+0x87/0x87 ? setup_acpi_sci+0xa2/0xa2 ? setup_arch+0x8db/0x9e1 ? start_kernel+0x6a/0x547 ? secondary_startup_64+0xb6/0xc0 Commit af1648984828 "x86/efi: Update e820 with reserved EFI boot services data to fix kexec breakage" is listed in Fixes: since it introduces more occurrences where efi_memmap_insert() is invoked after an efi_fake_mem= configuration has been parsed. Previously the side effects of vestigial empty entries were benign, but with commit af1648984828 that follow-on efi_memmap_insert() invocation triggers the above crash signature. Fixes: 0f96a99dab36 ("efi: Add 'efi_fake_mem' boot option") Fixes: af1648984828 ("x86/efi: Update e820 with reserved EFI boot services...") Link: https://lore.kernel.org/r/20191231014630.GA24942@dhcp-128-65.nay.redhat.com Reported-by: Dave Young <dyoung@redhat.com> Cc: Taku Izumi <izumi.taku@jp.fujitsu.com> Cc: Michael Weiser <michael@weiser.dinsnail.net> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@kernel.org> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/firmware/efi/fake_mem.c | 32 +++++++++++++++++--------------- drivers/firmware/efi/memmap.c | 2 +- include/linux/efi.h | 2 ++ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c index 7e53e5520548..68d752d8af21 100644 --- a/drivers/firmware/efi/fake_mem.c +++ b/drivers/firmware/efi/fake_mem.c @@ -34,26 +34,17 @@ static int __init cmp_fake_mem(const void *x1, const void *x2) return 0; } -void __init efi_fake_memmap(void) +static void __init efi_fake_range(struct efi_mem_range *efi_range) { int new_nr_map = efi.memmap.nr_map; efi_memory_desc_t *md; phys_addr_t new_memmap_phy; unsigned long flags = 0; void *new_memmap; - int i; - - if (!efi_enabled(EFI_MEMMAP) || !nr_fake_mem) - return; /* count up the number of EFI memory descriptor */ - for (i = 0; i < nr_fake_mem; i++) { - for_each_efi_memory_desc(md) { - struct range *r = &efi_fake_mems[i].range; - - new_nr_map += efi_memmap_split_count(md, r); - } - } + for_each_efi_memory_desc(md) + new_nr_map += efi_memmap_split_count(md, &efi_range->range); /* allocate memory for new EFI memmap */ new_memmap_phy = efi_memmap_alloc(new_nr_map, &flags); @@ -64,17 +55,28 @@ void __init efi_fake_memmap(void) new_memmap = early_memremap(new_memmap_phy, efi.memmap.desc_size * new_nr_map); if (!new_memmap) { - memblock_free(new_memmap_phy, efi.memmap.desc_size * new_nr_map); + __efi_memmap_free(new_memmap_phy, + efi.memmap.desc_size * new_nr_map, flags); return; } - for (i = 0; i < nr_fake_mem; i++) - efi_memmap_insert(&efi.memmap, new_memmap, &efi_fake_mems[i]); + efi_memmap_insert(&efi.memmap, new_memmap, efi_range); /* swap into new EFI memmap */ early_memunmap(new_memmap, efi.memmap.desc_size * new_nr_map); efi_memmap_install(new_memmap_phy, new_nr_map, flags); +} + +void __init efi_fake_memmap(void) +{ + int i; + + if (!efi_enabled(EFI_MEMMAP) || !nr_fake_mem) + return; + + for (i = 0; i < nr_fake_mem; i++) + efi_fake_range(&efi_fake_mems[i]); /* print new EFI memmap */ efi_print_memmap(); diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c index 188ab3cd5c52..de66c2a0e8f8 100644 --- a/drivers/firmware/efi/memmap.c +++ b/drivers/firmware/efi/memmap.c @@ -29,7 +29,7 @@ static phys_addr_t __init __efi_memmap_alloc_late(unsigned long size) return PFN_PHYS(page_to_pfn(p)); } -static void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags) +void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags) { if (WARN_ON(slab_is_available() && (flags & EFI_MEMMAP_MEMBLOCK))) return; diff --git a/include/linux/efi.h b/include/linux/efi.h index fa2668a992ae..6ae31e064321 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1061,6 +1061,8 @@ extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr); extern phys_addr_t __init efi_memmap_alloc(unsigned int num_entries, unsigned long *flags); +extern void __efi_memmap_free(u64 phys, unsigned long size, + unsigned long flags); extern int __init efi_memmap_init_early(struct efi_memory_map_data *data); extern int __init efi_memmap_init_late(phys_addr_t addr, unsigned long size); extern void __init efi_memmap_unmap(void);
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com> To: mingo@redhat.com Cc: Michael Weiser <michael@weiser.dinsnail.net>, linux-efi@vger.kernel.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>, x86@kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Taku Izumi <izumi.taku@jp.fujitsu.com>, Thomas Gleixner <tglx@linutronix.de>, Dave Young <dyoung@redhat.com>, Ingo Molnar <mingo@kernel.org> Subject: [PATCH v2 4/4] efi: Fix handling of multiple efi_fake_mem= entries Date: Tue, 31 Dec 2019 14:04:38 -0800 [thread overview] Message-ID: <157782987865.367056.15199592105978588123.stgit@dwillia2-desk3.amr.corp.intel.com> (raw) In-Reply-To: <157782985777.367056.14741265874314204783.stgit@dwillia2-desk3.amr.corp.intel.com> Dave noticed that when specifying multiple efi_fake_mem= entries only the last entry was successfully being reflected in the efi memory map. This is due to the fact that the efi_memmap_insert() is being called multiple times, but on successive invocations the insertion should be applied to the last new memmap rather than the original map at efi_fake_memmap() entry. Rework efi_fake_memmap() to install the new memory map after each efi_fake_mem= entry is parsed. This also fixes an issue in efi_fake_memmap() that caused it to litter emtpy entries into the end of the efi memory map. The empty entry causes efi_memmap_insert() to attempt more memmap splits / copies than efi_memmap_split_count() accounted for when sizing the new map. BUG: unable to handle page fault for address: ffffffffff281000 [..] RIP: 0010:efi_memmap_insert+0x11d/0x191 [..] Call Trace: ? bgrt_init+0xbe/0xbe ? efi_arch_mem_reserve+0x1cb/0x228 ? acpi_parse_bgrt+0xa/0xd ? acpi_table_parse+0x86/0xb8 ? acpi_boot_init+0x494/0x4e3 ? acpi_parse_x2apic+0x87/0x87 ? setup_acpi_sci+0xa2/0xa2 ? setup_arch+0x8db/0x9e1 ? start_kernel+0x6a/0x547 ? secondary_startup_64+0xb6/0xc0 Commit af1648984828 "x86/efi: Update e820 with reserved EFI boot services data to fix kexec breakage" is listed in Fixes: since it introduces more occurrences where efi_memmap_insert() is invoked after an efi_fake_mem= configuration has been parsed. Previously the side effects of vestigial empty entries were benign, but with commit af1648984828 that follow-on efi_memmap_insert() invocation triggers the above crash signature. Fixes: 0f96a99dab36 ("efi: Add 'efi_fake_mem' boot option") Fixes: af1648984828 ("x86/efi: Update e820 with reserved EFI boot services...") Link: https://lore.kernel.org/r/20191231014630.GA24942@dhcp-128-65.nay.redhat.com Reported-by: Dave Young <dyoung@redhat.com> Cc: Taku Izumi <izumi.taku@jp.fujitsu.com> Cc: Michael Weiser <michael@weiser.dinsnail.net> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@kernel.org> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/firmware/efi/fake_mem.c | 32 +++++++++++++++++--------------- drivers/firmware/efi/memmap.c | 2 +- include/linux/efi.h | 2 ++ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c index 7e53e5520548..68d752d8af21 100644 --- a/drivers/firmware/efi/fake_mem.c +++ b/drivers/firmware/efi/fake_mem.c @@ -34,26 +34,17 @@ static int __init cmp_fake_mem(const void *x1, const void *x2) return 0; } -void __init efi_fake_memmap(void) +static void __init efi_fake_range(struct efi_mem_range *efi_range) { int new_nr_map = efi.memmap.nr_map; efi_memory_desc_t *md; phys_addr_t new_memmap_phy; unsigned long flags = 0; void *new_memmap; - int i; - - if (!efi_enabled(EFI_MEMMAP) || !nr_fake_mem) - return; /* count up the number of EFI memory descriptor */ - for (i = 0; i < nr_fake_mem; i++) { - for_each_efi_memory_desc(md) { - struct range *r = &efi_fake_mems[i].range; - - new_nr_map += efi_memmap_split_count(md, r); - } - } + for_each_efi_memory_desc(md) + new_nr_map += efi_memmap_split_count(md, &efi_range->range); /* allocate memory for new EFI memmap */ new_memmap_phy = efi_memmap_alloc(new_nr_map, &flags); @@ -64,17 +55,28 @@ void __init efi_fake_memmap(void) new_memmap = early_memremap(new_memmap_phy, efi.memmap.desc_size * new_nr_map); if (!new_memmap) { - memblock_free(new_memmap_phy, efi.memmap.desc_size * new_nr_map); + __efi_memmap_free(new_memmap_phy, + efi.memmap.desc_size * new_nr_map, flags); return; } - for (i = 0; i < nr_fake_mem; i++) - efi_memmap_insert(&efi.memmap, new_memmap, &efi_fake_mems[i]); + efi_memmap_insert(&efi.memmap, new_memmap, efi_range); /* swap into new EFI memmap */ early_memunmap(new_memmap, efi.memmap.desc_size * new_nr_map); efi_memmap_install(new_memmap_phy, new_nr_map, flags); +} + +void __init efi_fake_memmap(void) +{ + int i; + + if (!efi_enabled(EFI_MEMMAP) || !nr_fake_mem) + return; + + for (i = 0; i < nr_fake_mem; i++) + efi_fake_range(&efi_fake_mems[i]); /* print new EFI memmap */ efi_print_memmap(); diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c index 188ab3cd5c52..de66c2a0e8f8 100644 --- a/drivers/firmware/efi/memmap.c +++ b/drivers/firmware/efi/memmap.c @@ -29,7 +29,7 @@ static phys_addr_t __init __efi_memmap_alloc_late(unsigned long size) return PFN_PHYS(page_to_pfn(p)); } -static void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags) +void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags) { if (WARN_ON(slab_is_available() && (flags & EFI_MEMMAP_MEMBLOCK))) return; diff --git a/include/linux/efi.h b/include/linux/efi.h index fa2668a992ae..6ae31e064321 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1061,6 +1061,8 @@ extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr); extern phys_addr_t __init efi_memmap_alloc(unsigned int num_entries, unsigned long *flags); +extern void __efi_memmap_free(u64 phys, unsigned long size, + unsigned long flags); extern int __init efi_memmap_init_early(struct efi_memory_map_data *data); extern int __init efi_memmap_init_late(phys_addr_t addr, unsigned long size); extern void __init efi_memmap_unmap(void); _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
next prev parent reply other threads:[~2019-12-31 22:20 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-31 22:04 [PATCH v2 0/4] efi: Fix handling of multiple efi_fake_mem= entries Dan Williams 2019-12-31 22:04 ` Dan Williams 2019-12-31 22:04 ` [PATCH v2 1/4] efi: Add a flags parameter to efi_memory_map Dan Williams 2019-12-31 22:04 ` Dan Williams 2019-12-31 22:04 ` [PATCH v2 2/4] efi: Add tracking for dynamically allocated memmaps Dan Williams 2019-12-31 22:04 ` Dan Williams 2019-12-31 22:04 ` [PATCH v2 3/4] efi: Fix efi_memmap_alloc() leaks Dan Williams 2019-12-31 22:04 ` Dan Williams 2020-01-01 3:35 ` Dave Young 2020-01-01 4:52 ` Dan Williams 2019-12-31 22:04 ` Dan Williams [this message] 2019-12-31 22:04 ` [PATCH v2 4/4] efi: Fix handling of multiple efi_fake_mem= entries Dan Williams 2020-01-01 4:51 ` Dave Young 2020-01-01 5:04 ` Dan Williams 2020-01-01 6:15 ` Dave Young 2020-01-01 6:20 ` Dave Young 2020-01-01 18:36 ` Dan Williams 2020-01-02 2:21 ` Dave Young
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=157782987865.367056.15199592105978588123.stgit@dwillia2-desk3.amr.corp.intel.com \ --to=dan.j.williams@intel.com \ --cc=ard.biesheuvel@linaro.org \ --cc=dyoung@redhat.com \ --cc=izumi.taku@jp.fujitsu.com \ --cc=kexec@lists.infradead.org \ --cc=linux-efi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=michael@weiser.dinsnail.net \ --cc=mingo@kernel.org \ --cc=mingo@redhat.com \ --cc=tglx@linutronix.de \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.