Re: [PATCH] UEFI: Don't pass boot services regions to SetVirtualAddressMap()

[Matt Fleming <matt.fleming@intel.com>]

On 06/02/2013 11:12 PM, Matthew Garrett wrote:
> We need to map boot services regions during startup in order to avoid
> firmware bugs, but we shouldn't be passing those regions to
> SetVirtualAddressMap(). Ensure that we're only passing regions that are
> marked as being mapped at runtime.
> 
> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> ---
>  arch/x86/platform/efi/efi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 63e167a..add0e37 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -922,6 +922,13 @@ void __init efi_enter_virtual_mode(void)
>  			va = efi_ioremap(md->phys_addr, size,
>  					 md->type, md->attribute);
>  
> +		if (!(md->attribute & EFI_MEMORY_RUNTIME)) {
> +			if (!va)
> +				pr_err("ioremap of 0x%llX failed!\n",
> +				       (unsigned long long)md->phys_addr);
> +			continue;
> +		}
> +
>  		md->virt_addr = (u64) (unsigned long) va;
>  
>  		if (!va) {
> 

Is there any point in calling efi_ioremap() for non-runtime regions in
this case? In other words, why not the following patch?

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 63e167a..a438ed3 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -903,9 +903,7 @@ void __init efi_enter_virtual_mode(void)
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
-		if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
-		    md->type != EFI_BOOT_SERVICES_CODE &&
-		    md->type != EFI_BOOT_SERVICES_DATA)
+		if (!(md->attribute & EFI_MEMORY_RUNTIME))
 			continue;
 
 		size = md->num_pages << EFI_PAGE_SHIFT;