[PATCH 1/2] efi: check EFI_MEMORY_DESCRIPTOR version explicitly

[PATCH 1/2] efi: check EFI_MEMORY_DESCRIPTOR version explicitly

[Ard Biesheuvel]

Our efi_memory_desc_t type is based on EFI_MEMORY_DESCRIPTOR version 1 in
the UEFI spec. No version updates are expected, but since we are about to
introduce support for new firmware tables that use the same descriptor
type, it makes sense to at least warn if we encounter other versions.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/x86/platform/efi/efi.c     | 4 ++++
 drivers/firmware/efi/arm-init.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 88d2fb2cb3ef..dde46cd78b8f 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -211,6 +211,10 @@ int __init efi_memblock_x86_reserve_range(void)
 	efi.memmap.desc_size	= e->efi_memdesc_size;
 	efi.memmap.desc_version	= e->efi_memdesc_version;
 
+	WARN(efi.memmap.desc_version != 1,
+	     "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
+	     efi.memmap.desc_version);
+
 	memblock_reserve(pmap, efi.memmap.nr_map * efi.memmap.desc_size);
 
 	return 0;
diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 93e46bfd6c23..aafaac3c02d5 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -198,6 +198,10 @@ void __init efi_init(void)
 	efi.memmap.desc_size = params.desc_size;
 	efi.memmap.desc_version = params.desc_ver;
 
+	WARN(efi.memmap.desc_version != 1,
+	     "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
+	      efi.memmap.desc_version);
+
 	if (uefi_init() < 0)
 		return;
 
-- 
2.5.0

[PATCH 2/2] efi/arm*: use memremap() to create the persistent memmap mapping

[Ard Biesheuvel]

Instead of using ioremap_cache(), which is slightly inappropriate for
mapping firmware tables, and is not even allowed on ARM for mapping
regions that are covered by a struct page, use memremap(), which was
invented for this purpose, and will also reuse the existing kernel
direct mapping if the requested region is covered by it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/efi/arm-runtime.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 98fe93a9813b..17ccf0a8787a 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -111,8 +111,7 @@ static int __init arm_enable_runtime_services(void)
 
 	mapsize = efi.memmap.map_end - efi.memmap.map;
 
-	efi.memmap.map = (__force void *)ioremap_cache(efi.memmap.phys_map,
-						       mapsize);
+	efi.memmap.map = memremap(efi.memmap.phys_map, mapsize, MEMREMAP_WB);
 	if (!efi.memmap.map) {
 		pr_err("Failed to remap EFI memory map\n");
 		return -ENOMEM;
-- 
2.5.0

Re: [PATCH 2/2] efi/arm*: use memremap() to create the persistent memmap mapping

[Ard Biesheuvel]

On 13 April 2016 at 17:37, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Instead of using ioremap_cache(), which is slightly inappropriate for
> mapping firmware tables, and is not even allowed on ARM for mapping
> regions that are covered by a struct page, use memremap(), which was
> invented for this purpose, and will also reuse the existing kernel
> direct mapping if the requested region is covered by it.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/firmware/efi/arm-runtime.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> index 98fe93a9813b..17ccf0a8787a 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -111,8 +111,7 @@ static int __init arm_enable_runtime_services(void)
>
>         mapsize = efi.memmap.map_end - efi.memmap.map;
>
> -       efi.memmap.map = (__force void *)ioremap_cache(efi.memmap.phys_map,
> -                                                      mapsize);
> +       efi.memmap.map = memremap(efi.memmap.phys_map, mapsize, MEMREMAP_WB);
>         if (!efi.memmap.map) {
>                 pr_err("Failed to remap EFI memory map\n");
>                 return -ENOMEM;

Actually, this patch does not depend on the ARM memremap changes being
merged: before those changes, memremap() unconditionally falls back to
ioremap_cache() for 'System RAM' so this patch is essentially a nop in
that case. This is guaranteed to work due to the fact that we [still]
use MEMBLOCK_NOMAP for mapping the memory map on ARM.

The memory attributes table and GOP changes do depend on it, however,
since the configuration tables it accesses are likely to be covered by
a struct page, in which case ARM disallows the ioremap_cache() (and
thus the use of memremap() until it is updated to deal with this
situation explicitly)

The other change that depends on it is dropping the MEMBLOCK_NOMAP for
ARM as well (as we did in the arm64 patch that is currently queued up
for v4.6-rc)

Re: [PATCH 2/2] efi/arm*: use memremap() to create the persistent memmap mapping

[Matt Fleming]

On Thu, 14 Apr, at 10:43:45AM, Ard Biesheuvel wrote:
> 
> Actually, this patch does not depend on the ARM memremap changes being
> merged: before those changes, memremap() unconditionally falls back to
> ioremap_cache() for 'System RAM' so this patch is essentially a nop in
> that case. This is guaranteed to work due to the fact that we [still]
> use MEMBLOCK_NOMAP for mapping the memory map on ARM.
 
OK, these two look good to me, so I'm going to queue them up in 'next'
chronologically before your EFI Memory Attributes table patches.

> The memory attributes table and GOP changes do depend on it, however,
> since the configuration tables it accesses are likely to be covered by
> a struct page, in which case ARM disallows the ioremap_cache() (and
> thus the use of memremap() until it is updated to deal with this
> situation explicitly)
> 
> The other change that depends on it is dropping the MEMBLOCK_NOMAP for
> ARM as well (as we did in the arm64 patch that is currently queued up
> for v4.6-rc)

Noted. Thanks Ard.