public inbox for linux-efi@vger.kernel.org
 help / color / mirror / Atom feed
From: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
To: linux-efi@vger.kernel.org
Cc: "Hamza Mahfooz" <hamzamahfooz@linux.microsoft.com>,
	stable@vger.kernel.org, "Tyler Hicks" <code@tyhicks.com>,
	"Brian Nguyen" <nguyenbrian@microsoft.com>,
	"Jacob Pan" <panj@microsoft.com>,
	"Allen Pais" <apais@microsoft.com>,
	"Ard Biesheuvel" <ardb@kernel.org>,
	"Jonathan Marek" <jonathan@marek.ca>,
	"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
	"Kuppuswamy Sathyanarayanan"
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Jeremy Linton" <jeremy.linton@arm.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	"KONDO KAZUMA(近藤 和真)" <kazuma-kondo@nec.com>,
	"Kees Cook" <kees@kernel.org>,
	"Borislav Petkov (AMD)" <bp@alien8.de>,
	"Yuntao Wang" <ytcoode@gmail.com>,
	"Aditya Garg" <gargaditya08@live.com>,
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] efi: make the min and max mmap slack slots configurable
Date: Mon,  9 Dec 2024 11:24:34 -0500	[thread overview]
Message-ID: <20241209162449.48390-1-hamzamahfooz@linux.microsoft.com> (raw)

Recent platforms require more slack slots than the current value of
EFI_MMAP_NR_SLACK_SLOTS, otherwise they fail to boot. So, introduce
EFI_MIN_NR_MMAP_SLACK_SLOTS and EFI_MAX_NR_MMAP_SLACK_SLOTS
and use them to determine a number of slots that the platform
is willing to accept.

Cc: stable@vger.kernel.org
Cc: Tyler Hicks <code@tyhicks.com>
Tested-by: Brian Nguyen <nguyenbrian@microsoft.com>
Tested-by: Jacob Pan <panj@microsoft.com>
Reviewed-by: Allen Pais <apais@microsoft.com>
Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
---
 drivers/firmware/efi/Kconfig                  | 23 +++++++++++++++++
 .../firmware/efi/libstub/efi-stub-helper.c    |  2 +-
 drivers/firmware/efi/libstub/efistub.h        | 15 +----------
 drivers/firmware/efi/libstub/kaslr.c          |  2 +-
 drivers/firmware/efi/libstub/mem.c            | 25 +++++++++++++++----
 drivers/firmware/efi/libstub/randomalloc.c    |  2 +-
 drivers/firmware/efi/libstub/relocate.c       |  2 +-
 drivers/firmware/efi/libstub/x86-stub.c       |  8 +++---
 8 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index e312d731f4a3..7fedc271d543 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -155,6 +155,29 @@ config EFI_TEST
 	  Say Y here to enable the runtime services support via /dev/efi_test.
 	  If unsure, say N.
 
+#
+# An efi_boot_memmap is used by efi_get_memory_map() to return the
+# EFI memory map in a dynamically allocated buffer.
+#
+# The buffer allocated for the EFI memory map includes extra room for
+# a range of [EFI_MIN_NR_MMAP_SLACK_SLOTS, EFI_MAX_NR_MMAP_SLACK_SLOTS]
+# additional EFI memory descriptors. This facilitates the reuse of the
+# EFI memory map buffer when a second call to ExitBootServices() is
+# needed because of intervening changes to the EFI memory map. Other
+# related structures, e.g. x86 e820ext, need to factor in this headroom
+# requirement as well.
+#
+
+config EFI_MIN_NR_MMAP_SLACK_SLOTS
+	int
+	depends on EFI
+	default 8
+
+config EFI_MAX_NR_MMAP_SLACK_SLOTS
+	int
+	depends on EFI
+	default 64
+
 config EFI_DEV_PATH_PARSER
 	bool
 
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index c0c81ca4237e..adf2b0c0dd34 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -432,7 +432,7 @@ efi_status_t efi_exit_boot_services(void *handle, void *priv,
 	if (efi_disable_pci_dma)
 		efi_pci_disable_bridge_busmaster();
 
-	status = efi_get_memory_map(&map, true);
+	status = efi_get_memory_map(&map, true, NULL);
 	if (status != EFI_SUCCESS)
 		return status;
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 76e44c185f29..d86c6e13de5f 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -160,19 +160,6 @@ void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
  */
 #define EFI_100NSEC_PER_USEC	((u64)10)
 
-/*
- * An efi_boot_memmap is used by efi_get_memory_map() to return the
- * EFI memory map in a dynamically allocated buffer.
- *
- * The buffer allocated for the EFI memory map includes extra room for
- * a minimum of EFI_MMAP_NR_SLACK_SLOTS additional EFI memory descriptors.
- * This facilitates the reuse of the EFI memory map buffer when a second
- * call to ExitBootServices() is needed because of intervening changes to
- * the EFI memory map. Other related structures, e.g. x86 e820ext, need
- * to factor in this headroom requirement as well.
- */
-#define EFI_MMAP_NR_SLACK_SLOTS	8
-
 typedef struct efi_generic_dev_path efi_device_path_protocol_t;
 
 union efi_device_path_to_text_protocol {
@@ -1059,7 +1046,7 @@ void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_si
 char *efi_convert_cmdline(efi_loaded_image_t *image);
 
 efi_status_t efi_get_memory_map(struct efi_boot_memmap **map,
-				bool install_cfg_tbl);
+				bool install_cfg_tbl, unsigned int *n);
 
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max);
diff --git a/drivers/firmware/efi/libstub/kaslr.c b/drivers/firmware/efi/libstub/kaslr.c
index 6318c40bda38..06e7a1ef34ab 100644
--- a/drivers/firmware/efi/libstub/kaslr.c
+++ b/drivers/firmware/efi/libstub/kaslr.c
@@ -62,7 +62,7 @@ static bool check_image_region(u64 base, u64 size)
 	bool ret = false;
 	int map_offset;
 
-	status = efi_get_memory_map(&map, false);
+	status = efi_get_memory_map(&map, false, NULL);
 	if (status != EFI_SUCCESS)
 		return false;
 
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 4f1fa302234d..cab25183b790 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -13,32 +13,47 @@
  *			configuration table
  *
  * Retrieve the UEFI memory map. The allocated memory leaves room for
- * up to EFI_MMAP_NR_SLACK_SLOTS additional memory map entries.
+ * up to CONFIG_EFI_MAX_NR_MMAP_SLACK_SLOTS additional memory map entries.
  *
  * Return:	status code
  */
 efi_status_t efi_get_memory_map(struct efi_boot_memmap **map,
-				bool install_cfg_tbl)
+				bool install_cfg_tbl,
+				unsigned int *n)
 {
 	int memtype = install_cfg_tbl ? EFI_ACPI_RECLAIM_MEMORY
 				      : EFI_LOADER_DATA;
 	efi_guid_t tbl_guid = LINUX_EFI_BOOT_MEMMAP_GUID;
+	unsigned int nr = CONFIG_EFI_MIN_NR_MMAP_SLACK_SLOTS;
 	struct efi_boot_memmap *m, tmp;
 	efi_status_t status;
 	unsigned long size;
 
+	BUILD_BUG_ON(!is_power_of_2(CONFIG_EFI_MIN_NR_MMAP_SLACK_SLOTS) ||
+		     !is_power_of_2(CONFIG_EFI_MAX_NR_MMAP_SLACK_SLOTS) ||
+		     CONFIG_EFI_MIN_NR_MMAP_SLACK_SLOTS >=
+		     CONFIG_EFI_MAX_NR_MMAP_SLACK_SLOTS);
+
 	tmp.map_size = 0;
 	status = efi_bs_call(get_memory_map, &tmp.map_size, NULL, &tmp.map_key,
 			     &tmp.desc_size, &tmp.desc_ver);
 	if (status != EFI_BUFFER_TOO_SMALL)
 		return EFI_LOAD_ERROR;
 
-	size = tmp.map_size + tmp.desc_size * EFI_MMAP_NR_SLACK_SLOTS;
-	status = efi_bs_call(allocate_pool, memtype, sizeof(*m) + size,
-			     (void **)&m);
+	do {
+		size = tmp.map_size + tmp.desc_size * nr;
+		status = efi_bs_call(allocate_pool, memtype, sizeof(*m) + size,
+				     (void **)&m);
+		nr <<= 1;
+	} while (status == EFI_BUFFER_TOO_SMALL &&
+		 nr <= CONFIG_EFI_MAX_NR_MMAP_SLACK_SLOTS);
+
 	if (status != EFI_SUCCESS)
 		return status;
 
+	if (n)
+		*n = nr;
+
 	if (install_cfg_tbl) {
 		/*
 		 * Installing a configuration table might allocate memory, and
diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
index c41e7b2091cd..e80a65e7b87a 100644
--- a/drivers/firmware/efi/libstub/randomalloc.c
+++ b/drivers/firmware/efi/libstub/randomalloc.c
@@ -65,7 +65,7 @@ efi_status_t efi_random_alloc(unsigned long size,
 	efi_status_t status;
 	int map_offset;
 
-	status = efi_get_memory_map(&map, false);
+	status = efi_get_memory_map(&map, false, NULL);
 	if (status != EFI_SUCCESS)
 		return status;
 
diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
index d694bcfa1074..b7b0aad95ba4 100644
--- a/drivers/firmware/efi/libstub/relocate.c
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -28,7 +28,7 @@ efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
 	unsigned long nr_pages;
 	int i;
 
-	status = efi_get_memory_map(&map, false);
+	status = efi_get_memory_map(&map, false, NULL);
 	if (status != EFI_SUCCESS)
 		goto fail;
 
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 188c8000d245..cb14f0d2a3d9 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -740,15 +740,15 @@ static efi_status_t allocate_e820(struct boot_params *params,
 	struct efi_boot_memmap *map;
 	efi_status_t status;
 	__u32 nr_desc;
+	__u32 nr;
 
-	status = efi_get_memory_map(&map, false);
+	status = efi_get_memory_map(&map, false, &nr);
 	if (status != EFI_SUCCESS)
 		return status;
 
 	nr_desc = map->map_size / map->desc_size;
-	if (nr_desc > ARRAY_SIZE(params->e820_table) - EFI_MMAP_NR_SLACK_SLOTS) {
-		u32 nr_e820ext = nr_desc - ARRAY_SIZE(params->e820_table) +
-				 EFI_MMAP_NR_SLACK_SLOTS;
+	if (nr_desc > ARRAY_SIZE(params->e820_table) - nr) {
+		u32 nr_e820ext = nr_desc - ARRAY_SIZE(params->e820_table) + nr;
 
 		status = alloc_e820ext(nr_e820ext, e820ext, e820ext_size);
 	}
-- 
2.47.1


             reply	other threads:[~2024-12-09 16:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-09 16:24 Hamza Mahfooz [this message]
2024-12-09 16:40 ` [PATCH] efi: make the min and max mmap slack slots configurable Ard Biesheuvel
2024-12-09 17:02   ` Hamza Mahfooz
2024-12-09 18:00     ` Ard Biesheuvel
2024-12-09 18:03       ` Hamza Mahfooz
2024-12-11 23:12 ` kernel test robot

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=20241209162449.48390-1-hamzamahfooz@linux.microsoft.com \
    --to=hamzamahfooz@linux.microsoft.com \
    --cc=apais@microsoft.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=code@tyhicks.com \
    --cc=gargaditya08@live.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jeremy.linton@arm.com \
    --cc=jonathan@marek.ca \
    --cc=kazuma-kondo@nec.com \
    --cc=kees@kernel.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nguyenbrian@microsoft.com \
    --cc=panj@microsoft.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=stable@vger.kernel.org \
    --cc=ytcoode@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox