Skip to contentSkip to conversation

efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

PATCH New Ard Biesheuvel 5 participants 7 messages Mar 30, 2016 – Apr 16, 2016 EFI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

Hi Matt,

Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please? (assuming no objections from any of the cc'ees)

Thanks,
Ard.

----------8<--------------
Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices regions as well as the kernel's copy of the UEFI memory map to set the MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the kernel direct mapping, and from being covered by a struct page. For the RuntimeServices regions, this is an obvious win, since the contents of these regions have significance to the firmware executable code itself, and are mapped in the EFI page tables using attributes that are described in the UEFI memory map, and which may differ from the attributes we use for mapping system RAM. It also prevents the contents from being modified inadvertently, since the EFI page tables are only live during runtime service invocations.

None of these concerns apply to the allocation that covers the UEFI memory map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on the region did allow us to use ioremap_cache() to map it both on arm64 and on ARM, since the latter does not allow ioremap_cache() to be used on regions that are covered by a struct page.

The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe, but in the mean time, it has been reported that commit 4dffbfc48d65 causes a regression on 64k granule kernels. This is due to the fact that, given the 64 KB page size, the region that we end up removing from the kernel direct mapping is rounded up to 64 KB, and this 64 KB page frame may be shared with the initrd when booting via GRUB (which does not align its EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash the kernel as soon as it tries to access the initrd.

Since the issue is specific to arm64, revert back to memblock_reserve()'ing the UEFI memory map when running on arm64. This is a temporary fix for v4.5 and v4.6, and will be superseded in the v4.7 timeframe when we will be able to move back to memblock_reserve() unconditionally.

Fixes 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
---
 drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
drivers/firmware/efi/arm-init.c +15 -3
index aa1f743152a2..8714f8c271ba 100644
@@ -203,7 +203,19 @@ void __init efi_init(void)
 
 	reserve_regions();
 	early_memunmap(memmap.map, params.mmap_size);
-	memblock_mark_nomap(params.mmap & PAGE_MASK,
-			    PAGE_ALIGN(params.mmap_size +
-				       (params.mmap & ~PAGE_MASK)));
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * ARM currently does not allow ioremap_cache() to be called on
+		 * memory regions that are covered by struct page. So remove the
+		 * UEFI memory map from the linear mapping.
+		 */
+		memblock_mark_nomap(params.mmap & PAGE_MASK,
+				    PAGE_ALIGN(params.mmap_size +
+					       (params.mmap & ~PAGE_MASK)));
+	} else {
+		memblock_reserve(params.mmap & PAGE_MASK,
+				 PAGE_ALIGN(params.mmap_size +
+					    (params.mmap & ~PAGE_MASK)));
+	}
 }
Signature · 1 line
2.5.0
#1 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

Quoted 4 lines · Ard Biesheuvel

Hi Matt,

Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please? (assuming no objections from any of the cc'ees)

FWIW:

Yes, this is a temporary hack, but it's small, fixes a real issue and you're already working on a proper solution anyway.

Will

Quoted 77 lines

----------8<--------------
Commit 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP") updated the mapping logic of both the RuntimeServices regions as well as the kernel's copy of the UEFI memory map to set the MEMBLOCK_NOMAP flag, which causes these regions to be omitted from the kernel direct mapping, and from being covered by a struct page. For the RuntimeServices regions, this is an obvious win, since the contents of these regions have significance to the firmware executable code itself, and are mapped in the EFI page tables using attributes that are described in the UEFI memory map, and which may differ from the attributes we use for mapping system RAM. It also prevents the contents from being modified inadvertently, since the EFI page tables are only live during runtime service invocations.

None of these concerns apply to the allocation that covers the UEFI memory map, since it is entirely owned by the kernel. Setting the MEMBLOCK_NOMAP on the region did allow us to use ioremap_cache() to map it both on arm64 and on ARM, since the latter does not allow ioremap_cache() to be used on regions that are covered by a struct page.

The ioremap_cache() on ARM restriction will be lifted in the v4.7 timeframe, but in the mean time, it has been reported that commit 4dffbfc48d65 causes a regression on 64k granule kernels. This is due to the fact that, given the 64 KB page size, the region that we end up removing from the kernel direct mapping is rounded up to 64 KB, and this 64 KB page frame may be shared with the initrd when booting via GRUB (which does not align its EFI_LOADER_DATA allocations to 64 KB like the stub does). This will crash the kernel as soon as it tries to access the initrd.

Since the issue is specific to arm64, revert back to memblock_reserve()'ing the UEFI memory map when running on arm64. This is a temporary fix for v4.5 and v4.6, and will be superseded in the v4.7 timeframe when we will be able to move back to memblock_reserve() unconditionally.

Fixes 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")
---
 drivers/firmware/efi/arm-init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
drivers/firmware/efi/arm-init.c +15 -3
index aa1f743152a2..8714f8c271ba 100644
@@ -203,7 +203,19 @@ void __init efi_init(void)
 
 	reserve_regions();
 	early_memunmap(memmap.map, params.mmap_size);
-	memblock_mark_nomap(params.mmap & PAGE_MASK,
-			    PAGE_ALIGN(params.mmap_size +
-				       (params.mmap & ~PAGE_MASK)));
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * ARM currently does not allow ioremap_cache() to be called on
+		 * memory regions that are covered by struct page. So remove the
+		 * UEFI memory map from the linear mapping.
+		 */
+		memblock_mark_nomap(params.mmap & PAGE_MASK,
+				    PAGE_ALIGN(params.mmap_size +
+					       (params.mmap & ~PAGE_MASK)));
+	} else {
+		memblock_reserve(params.mmap & PAGE_MASK,
+				 PAGE_ALIGN(params.mmap_size +
+					    (params.mmap & ~PAGE_MASK)));
+	}
 }
Signature · 7 lines
2.5.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#2 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

In reply to Ard Biesheuvel
Quoted 4 lines · Ard Biesheuvel

Hi Matt,

Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please? (assuming no objections from any of the cc'ees)

Yep, I've picked this up. I'll go out to tip tomorrow.

#3 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

Quoted 8 lines · Matt Fleming
Quoted 4 lines · Ard Biesheuvel

Hi Matt,

Could we queue this as a fix for v4.6 with a cc:stable for v4.5, please? (assuming no objections from any of the cc'ees)

Yep, I've picked this up. I'll go out to tip tomorrow.

Hello,
Is this going to make it in for 4.6?
I ran into this issue the other day, and can confirm that Ard's patch is helpful.

Cheers,

Signature · 1 line
Steve
Quoted 4 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#4 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

Quoted 5 lines · Steve Capper

Hello,
Is this going to make it in for 4.6?
I ran into this issue the other day, and can confirm that Ard's patch is helpful.

Good question.

Ingo did you send the patch in the last EFI urgent pull request to Linus yet? I can find it in tip/efi-urgent-for-linus but not in Linus' tree.

#5 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

Quoted 7 lines · Matt Fleming
Quoted 5 lines · Steve Capper

Hello,
Is this going to make it in for 4.6?
I ran into this issue the other day, and can confirm that Ard's patch is helpful.

Quoted 5 lines

Good question.

Ingo did you send the patch in the last EFI urgent pull request to Linus yet? I can find it in tip/efi-urgent-for-linus but not in Linus' tree.

I just sent it to Linus.

It got delayed a bit because it had an x86/urgent dependency that had to be sent upstream first. (In the future you can avoid such delays by basing EFI urgent fixes on an upstream -rc kernel, not x86/urgent.)

Thanks,

Ingo

#6 lore raw

PATCH 1/1 efi/arm64: don't apply MEMBLOCK_NOMAP to UEFI memory map mapping

Quoted 6 lines · Ingo Molnar

I just sent it to Linus.

It got delayed a bit because it had an x86/urgent dependency that had to be sent upstream first. (In the future you can avoid such delays by basing EFI urgent fixes on an upstream -rc kernel, not x86/urgent.)

Noted. I'll do this in future, thanks Ingo.