All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: mingo@redhat.com
Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Dave Young <dyoung@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Michael Weiser <michael@weiser.dinsnail.net>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
	kexec@lists.infradead.org, x86@kernel.org
Subject: [PATCH v3 0/4] efi: Fix handling of multiple efi_fake_mem= entries
Date: Wed, 01 Jan 2020 20:13:18 -0800	[thread overview]
Message-ID: <157793839827.977550.7845382457971215205.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)

Changes since v2 [1]:
- Move the efi_memmap_free() until efi_memmap_install() is committed to
  installing the new map. (Dave).

- Handle the case of a memblock allocated memmap being freed after the slab
  allocator is up. Just use memblock_free_late() for that case rather
  than warn. (Prompted by Dave's feedback on how many successful
  efi_memmap_free() calls occur during a boot).

- Not changed was anything additional related to Dave's concern about
  efi_fake_mem= being applied to overlapping entries. I tested
  "efi_fake_mem=4G@9G:0x40000,4G@12G:0x40000" which triggers the second
  entry to overwrite the first as well as another entry. The result is
  reasonable and functional for what is otherwise garbage input:

  efi: mem53: [Conventional Memory|   |  |SP|  |  |  |  |  |   |WB|WT|WC|UC] range=[0x240000000-0x2ffffffff] (3072MB)
  efi: mem54: [Conventional Memory|   |  |SP|  |  |  |  |  |   |WB|WT|WC|UC] range=[0x300000000-0x33fffffff] (1024MB)
  efi: mem55: [Conventional Memory|   |  |SP|  |  |  |  |  |   |WB|WT|WC|UC] range=[0x340000000-0x3ffffffff] (3072MB)
  efi: mem56: [Conventional Memory|   |  |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x400000000-0x43fffffff] (1024MB)

  # cat /proc/iomem  | grep Sof
  240000000-3ffffffff : Soft Reserved

[1]: http://lore.kernel.org/r/157782985777.367056.14741265874314204783.stgit@dwillia2-desk3.amr.corp.intel.com

---

While testing an upcoming patchset to enhance the "soft reservation"
implementation it started crashing when rebased on v5.5-rc3. This
uncovered a few bugs in the efi_fake_mem= handling and
efi_memmap_alloc() leaks.

---

Copied from patch4:

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.

---

Dan Williams (4):
      efi: Add a flags parameter to efi_memory_map
      efi: Add tracking for dynamically allocated memmaps
      efi: Fix efi_memmap_alloc() leaks
      efi: Fix handling of multiple efi_fake_mem= entries


 arch/x86/platform/efi/efi.c     |    2 +
 arch/x86/platform/efi/quirks.c  |   11 ++++---
 drivers/firmware/efi/fake_mem.c |   37 +++++++++++++------------
 drivers/firmware/efi/memmap.c   |   58 ++++++++++++++++++++++++++++++---------
 include/linux/efi.h             |   13 +++++++--
 5 files changed, 81 insertions(+), 40 deletions(-)

             reply	other threads:[~2020-01-02  4:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-02  4:13 Dan Williams [this message]
2020-01-02  4:13 ` [PATCH v3 1/4] efi: Add a flags parameter to efi_memory_map Dan Williams
2020-01-02  4:13 ` [PATCH v3 2/4] efi: Add tracking for dynamically allocated memmaps Dan Williams
2020-01-02  9:02   ` Ard Biesheuvel
2020-01-06 19:05     ` Dan Williams
2020-01-02  4:13 ` [PATCH v3 3/4] efi: Fix efi_memmap_alloc() leaks Dan Williams
2020-01-02  4:13 ` [PATCH v3 4/4] efi: Fix handling of multiple efi_fake_mem= entries Dan Williams

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=157793839827.977550.7845382457971215205.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: link
Be 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.