[PATCH v2 18/19] x86/efi: Do not abuse RUNTIME bit to mark boot regions as reserved

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@xxxxxxxxxx>

efi_reserve_boot_regions() marks all EFI boot services memory regions as
memblock_reserve()'d temporarily, so that they can be mapped in the EFI
page tables during the call to the SetVirtualAddressMap() runtime
service.

This means it has to take care to distinguish between regions that are
entirely unused from regions that are already covered by some prior
reservations, either by the kernel itself via memblock, or via the
firmware or bootloader via the E820 map.

For this reason, it only memblock_reserve()'s boot services regions that
are not covered by any prior memblock reservation. Otherwise, it will
set the EFI_MEMORY_RUNTIME flag for the region, which indicates to the
freeing code that runs later that the region must remain reserved.

It also sets the EFI_MEMORY_RUNTIME flag for the region if it covers any
E820 region that is not E820_RAM, so that -again- the entire region
remains reserved indefinitely.

This is inefficient, and abusing the EFI_MEMORY_RUNTIME flag for this is
not great either. It would be better to respect the actual memblock or
E820 reservations instead, which is feasible now that the freeing code
takes the MEMBLOCK_RSRV_KERN flag into account.

So drop the EFI_MEMORY_RUNTIME hack, and instead, respect existing
memblock reservations by upgrading them to MEMBLOCK_RSRV_KERN
reservations. Take E820 reservations into account by cross-referencing
them with the EFI and memblock reservations when actually returning the
pages back to the page allocator.

Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
---
 arch/x86/platform/efi/quirks.c | 29 ++++++--------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index bc9dfe7925aa..8f2dc477eee0 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -298,26 +298,13 @@ void __init efi_reserve_boot_services(void)
 		 */
 		if (!already_reserved) {
 			memblock_reserve(start, size);
-
+		} else {
 			/*
-			 * If we are the first to reserve the region, no
-			 * one else cares about it. We own it and can
-			 * free it later.
+			 * Mark existing reservations as MEMBLOCK_RSRV_KERN so
+			 * they will be respected by efi_free_boot_services().
 			 */
-			if (can_free_region(start, size))
-				continue;
+			memblock_reserved_mark_kern(start, size);
 		}
-
-		/*
-		 * We don't own the region. We must not free it.
-		 *
-		 * Setting this bit for a boot services region really
-		 * doesn't make sense as far as the firmware is
-		 * concerned, but it does provide us with a way to tag
-		 * those regions that must not be paired with
-		 * memblock_free_late().
-		 */
-		md->attribute |= EFI_MEMORY_RUNTIME;
 	}
 }
 
@@ -392,6 +379,9 @@ efi_free_unreserved_subregions(u64 range_start, u64 range_end)
 		if (start >= end)
 			continue;
 
+		if (!can_free_region(start, end - start))
+			continue;
+
 		free_reserved_area(phys_to_virt(start),
 				   phys_to_virt(end), -1, NULL);
 		freed += (end - start);
@@ -428,9 +418,8 @@ static int __init efi_free_boot_services(void)
 		if (md_start >= md_end)
 			continue;
 
-		if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
-		    (md->type == EFI_BOOT_SERVICES_CODE ||
-		     md->type == EFI_BOOT_SERVICES_DATA)) {
+		if (md->type == EFI_BOOT_SERVICES_CODE ||
+		    md->type == EFI_BOOT_SERVICES_DATA) {
 			u64 f = efi_free_unreserved_subregions(md_start, md_end);
 
 			/*
-- 
2.53.0.851.ga537e3e6e9-goog