remove UEFI reserved regions from the linear mapping

In an upcoming patch, we will wire up page_is_ram() to the UEFI memory map, so make sure the EFI_MEMMAP bit accurately reflects whether the UEFI memory map is available via its permanent mapping.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/kernel/efi.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

arch/arm64/kernel/efi.c +1 -9

index 852eb927ea85..11b59e9a5954 100644
@@ -191,8 +191,6 @@ static __init void reserve_regions(void)
 		if (efi_enabled(EFI_DBG))
 			pr_cont("\n");
 	}
-
-	set_bit(EFI_MEMMAP, &efi.flags);
 }
 
 void __init efi_init(void)

@@ -291,13 +289,6 @@ static int __init arm64_enable_runtime_services(void)
 		return -1;
 	}
 
-	if (efi_runtime_disabled()) {
-		pr_info("EFI runtime services will be disabled.\n");
-		return -1;
-	}
-
-	pr_info("Remapping and enabling EFI services.\n");
-
 	mapsize = memmap.map_end - memmap.map;
 	memmap.map = (__force void *)ioremap_cache((phys_addr_t)memmap.phys_map,
 						   mapsize);

@@ -307,6 +298,14 @@ static int __init arm64_enable_runtime_services(void)
 	}
 	memmap.map_end = memmap.map + mapsize;
 	efi.memmap = &memmap;
+	set_bit(EFI_MEMMAP, &efi.flags);

This patch overrides the __weak default implementation of page_is_ram(), which uses string comparisons to find entries called 'System RAM' in /proc/iomem. Since we used the contents of memblock to create those entries in the first place, let's use memblock directly.

Also, since the UEFI memory map may describe regions backed by RAM that are not in memblock (i.e., reserved regions that were removed from the linear mapping), check the pfn against the UEFI memory map as well.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/mm/mmu.c | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)

Quoted 9 lines · Ard Biesheuvel

This patch overrides the __weak default implementation of page_is_ram(), which uses string comparisons to find entries called 'System RAM' in /proc/iomem. Since we used the contents of memblock to create those entries in the first place, let's use memblock directly.

Also, since the UEFI memory map may describe regions backed by RAM that are not in memblock (i.e., reserved regions that were removed from the linear mapping), check the pfn against the UEFI memory map as well.

Quoted 4 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/mm/mmu.c | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)

Am I correct in thinking that the purpose of this series is just to placate acpi_os_ioremap() on arm64, and its use of page_is_ram()?

While there aren't many users of page_is_ram() right now, I can see how in the future if new users are added they'd be extremely confused to find that page_is_ram(pfn) returns true but 'pfn' isn't accessible by the kernel proper.

Wouldn't it make more sense to teach acpi_os_ioremap() about these special reserved regions outside of page_is_ram()?

Quoted 10 lines · Matt Fleming

Quoted 9 lines · Ard Biesheuvel

This patch overrides the __weak default implementation of page_is_ram(), which uses string comparisons to find entries called 'System RAM' in /proc/iomem. Since we used the contents of memblock to create those entries in the first place, let's use memblock directly.

Also, since the UEFI memory map may describe regions backed by RAM that are not in memblock (i.e., reserved regions that were removed from the linear mapping), check the pfn against the UEFI memory map as well.

Quoted 8 lines

Quoted 4 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/mm/mmu.c | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)

Am I correct in thinking that the purpose of this series is just to placate acpi_os_ioremap() on arm64, and its use of page_is_ram()?

That is currently the primary user, but we need this information for other purposes as well. One example is /dev/mem, which is used for both devices and memory (for instance, tools like dmidecode rely heavily on it). When using it to access a memory region that we punched out of the linear mapping, we should typically not map it as a device, since unaligned accesses cause faults in that case.

In summary, it would be nice if we could preserve the 'is ram" annotation for regions that are not covered by the linear mapping.

Quoted 5 lines

While there aren't many users of page_is_ram() right now, I can see how in the future if new users are added they'd be extremely confused to find that page_is_ram(pfn) returns true but 'pfn' isn't accessible by the kernel proper.

Well, who knows. page_is_ram() is poorly documented, and so is the 'System RAM' iomem annotation that its default implementation relies on.

Quoted 36 lines · Ard Biesheuvel

Quoted 10 lines · Matt Fleming

Quoted 9 lines · Ard Biesheuvel

This patch overrides the __weak default implementation of page_is_ram(), which uses string comparisons to find entries called 'System RAM' in /proc/iomem. Since we used the contents of memblock to create those entries in the first place, let's use memblock directly.

Also, since the UEFI memory map may describe regions backed by RAM that are not in memblock (i.e., reserved regions that were removed from the linear mapping), check the pfn against the UEFI memory map as well.

Quoted 8 lines

Quoted 4 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/mm/mmu.c | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)

Am I correct in thinking that the purpose of this series is just to placate acpi_os_ioremap() on arm64, and its use of page_is_ram()?

That is currently the primary user, but we need this information for other purposes as well. One example is /dev/mem, which is used for both devices and memory (for instance, tools like dmidecode rely heavily on it). When using it to access a memory region that we punched out of the linear mapping, we should typically not map it as a device, since unaligned accesses cause faults in that case.

In summary, it would be nice if we could preserve the 'is ram" annotation for regions that are not covered by the linear mapping.

Quoted 5 lines

While there aren't many users of page_is_ram() right now, I can see how in the future if new users are added they'd be extremely confused to find that page_is_ram(pfn) returns true but 'pfn' isn't accessible by the kernel proper.

Quoted 3 lines

Well, who knows. page_is_ram() is poorly documented, and so is the 'System RAM' iomem annotation that its default implementation relies on.

Sorry if this is a bit of a derailment, but perhaps now is a good opportunity to introduce something like:

#ifndef page_is_linear_mapped
#define page_is_linear_mapped page_is_ram
#endif

With documentation as to the semantic difference, and a conversion of existing users.

This is yet another approach to solving the issues around removing RAM regions known to UEFI from the linear mapping while preserving the record of the fact that these regions are backed by memory.

The previous approach added a memblock flag called MEMBLOCK_NOMAP to keep track of RAM regions that should be removed from the linear mapping.

The primary motivation for the new approach is the observation that there is only a single use case that requires this, which is acpi_os_ioremap(). Since ACPI implies UEFI on arm64 platforms, and since acpi_os_ioremap() uses page_is_ram() internally (which is a __weak generic function), we can simply reimplement page_is_ram() to take the UEFI memory map into account if we are booted via UEFI.

Once we have a page_is_ram() implementation in place that will return true even for RAM that is known to UEFI but not covered by the linear mapping, we can remove all UEFI reserved and runtime regions from the linear mapping as well.

As is obvious from the diffstat, this is the approach with the least impact, both in terms of number of changes and in terms of the locality of the changes. If we end up needing this information for other reasons (e.g., /dev/mem access to /reserved-memory subnodes with the nomap property on !EFI systems), we can always revisit this, but for now, I think this approach is the most suitable.

Patch #1 slightly reorders the UEFI runtime services initialization routines so that the EFI_MEMMAP flag is only set if the permanent mapping of the UEFI memory map is in place.

Quoted 7 lines · Ard Biesheuvel

This is yet another approach to solving the issues around removing RAM regions known to UEFI from the linear mapping while preserving the record of the fact that these regions are backed by memory.

The previous approach added a memblock flag called MEMBLOCK_NOMAP to keep track of RAM regions that should be removed from the linear mapping.

Quoted 6 lines

The primary motivation for the new approach is the observation that there is only a single use case that requires this, which is acpi_os_ioremap(). Since ACPI implies UEFI on arm64 platforms, and since acpi_os_ioremap() uses page_is_ram() internally (which is a __weak generic function), we can simply reimplement page_is_ram() to take the UEFI memory map into account if we are booted via UEFI.

Just to check, is the above the only reason for this new approach? Or were there other issues with the MEMBLOCK_NOMAP approach other than the diffstat?

I quite liked the MEMBLOCK_NOMAP approach as it looked reusable.

Quoted 4 lines

Once we have a page_is_ram() implementation in place that will return true even for RAM that is known to UEFI but not covered by the linear mapping, we can remove all UEFI reserved and runtime regions from the linear mapping as well.

I take it there aren't any lurking instances of page_is_ram() used to test if something exists in the linear mapping?

Quoted 6 lines

As is obvious from the diffstat, this is the approach with the least impact, both in terms of number of changes and in terms of the locality of the changes. If we end up needing this information for other reasons (e.g., /dev/mem access to /reserved-memory subnodes with the nomap property on !EFI systems), we can always revisit this, but for now, I think this approach is the most suitable.

Quoted 3 lines

Patch #1 slightly reorders the UEFI runtime services initialization routines so that the EFI_MEMMAP flag is only set if the permanent mapping of the UEFI memory map is in place.

This also means that the memory map is mapped even with EFI runtime support disabled, but I guess that's not a big problem.

As a side thought, it would be nice if we could memremap_ro the system table and memory map in future to prevent potential corruption, given they have fixed VAs and are always mapped.

Quoted 19 lines · Mark Rutland

Quoted 7 lines · Ard Biesheuvel

This is yet another approach to solving the issues around removing RAM regions known to UEFI from the linear mapping while preserving the record of the fact that these regions are backed by memory.

The previous approach added a memblock flag called MEMBLOCK_NOMAP to keep track of RAM regions that should be removed from the linear mapping.

Quoted 6 lines

The primary motivation for the new approach is the observation that there is only a single use case that requires this, which is acpi_os_ioremap(). Since ACPI implies UEFI on arm64 platforms, and since acpi_os_ioremap() uses page_is_ram() internally (which is a __weak generic function), we can simply reimplement page_is_ram() to take the UEFI memory map into account if we are booted via UEFI.

Just to check, is the above the only reason for this new approach? Or were there other issues with the MEMBLOCK_NOMAP approach other than the diffstat?

Quoted 2 lines

I quite liked the MEMBLOCK_NOMAP approach as it looked reusable.

I think the MEMBLOCK_NOMAP approach is sound, but it is harder to prove that there are no corner cases that behave incorrectly.

Quoted 3 lines

I take it there aren't any lurking instances of page_is_ram() used to test if something exists in the linear mapping?

Well, first of all, the linear mapping only covers lowmem, so that in itself would not be a portable use. In general, pfn_valid() would be the correct test for that (possibly combined with PageHighmem())

page_is_ram() and the 'System RAM' iomem region are so poorly defined or documented that we may be better off just removing it in the first place and replace it with something meaningful.

Quoted 7 lines

Quoted 3 lines

Patch #1 slightly reorders the UEFI runtime services initialization routines so that the EFI_MEMMAP flag is only set if the permanent mapping of the UEFI memory map is in place.

This also means that the memory map is mapped even with EFI runtime support disabled, but I guess that's not a big problem.

Yes. You need that anyway if you are going to rely on it even when the runtime services are disabled.

Quoted 4 lines

As a side thought, it would be nice if we could memremap_ro the system table and memory map in future to prevent potential corruption, given they have fixed VAs and are always mapped.

I agree, and I already have some local patches using early_memremap_ro() for the EFI init code.

memremap_ro() does not actually exist yet, but I intend to propose MEMREMAP_RO and MEMREMAP_NX flags to Dan Williams's memremap() work once I get around to it.

Quoted 2 lines · Ard Biesheuvel

Quoted 1 line · Mark Rutland

On Thu, Oct 29, 2015 at 02:40:56PM +0100, Ard Biesheuvel wrote:

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Patch #1 slightly reorders the UEFI runtime services initialization routines so that the EFI_MEMMAP flag is only set if the permanent mapping of the UEFI memory map is in place.

Quoted 6 lines

Quoted 3 lines

This also means that the memory map is mapped even with EFI runtime support disabled, but I guess that's not a big problem.

Yes. You need that anyway if you are going to rely on it even when the runtime services are disabled.

Not with the MEMBLOCK_NOMAP approach.

I don't have a strong case for caring about that; I only imagine that being a problem if you're trying to track down extremely nasty memory corruption / bad pointer bugs and want the bare minimum VA space mapped. Even then the impact is minimal.

Quoted 7 lines

Quoted 4 lines

As a side thought, it would be nice if we could memremap_ro the system table and memory map in future to prevent potential corruption, given they have fixed VAs and are always mapped.

I agree, and I already have some local patches using early_memremap_ro() for the EFI init code.

Ah, nice!

Quoted 3 lines

memremap_ro() does not actually exist yet, but I intend to propose MEMREMAP_RO and MEMREMAP_NX flags to Dan Williams's memremap() work once I get around to it.

That sounds good; I would certainly be in favour of that.

For some reason I thought the memremap arch changes had gone in for v4.3, but I see that's not the case. I'll take a look around.