diff mbox series

[v4,2/3] xen/efi: optionally call SetVirtualAddressMap()

Message ID db4793665d03513d06ffd6b3d4a52c8a699ccb0b.1571888583.git-series.marmarek@invisiblethingslab.com (mailing list archive)
State New, archived
Headers show
Series Optionally call EFI SetVirtualAddressMap() | expand

Commit Message

Marek Marczykowski-Górecki Oct. 24, 2019, 3:45 a.m. UTC
Some UEFI implementations are not happy about lack of
SetVirtualAddressMap() call. Likely abuse the address map change
notification to do things beyond the necessary ConvertPointer() calls.
Specifically, wihtout the SetVirtualAddressMap() call, some access
EfiBootServices{Code,Data}, or even totally unmapped areas. Example
crash of GetVariable() call on Thinkpad W540:

    Xen call trace:
       [<0000000000000080>] 0000000000000080
       [<8c2b0398e0000daa>] 8c2b0398e0000daa

    Pagetable walk from ffffffff858483a1:
       L4[0x1ff] = 0000000000000000 ffffffffffffffff

    ****************************************
    Panic on CPU 0:
    FATAL PAGE FAULT
    [error_code=0002]
    Faulting linear address: ffffffff858483a1
    ****************************************

Fix this by calling SetVirtualAddressMap() runtime service, giving it
1:1 map for areas marked as needed during runtime. The address space in
which EFI runtime services are called is unchanged, but UEFI view of it
may be.
Since it's fairly late in Xen 4.13 development cycle, disable it
by default and hide behind EXPERT.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes in v2:
 - call  SetVirtualAddressMap() before adjusting efi pointers; especially
   efi_memmap at this point still needs to use physical address, not a
   directmap one
Changes in v3:
 - clarify impact (or rather: lack of it) on kexec, drop !KEXEC
   dependency.
Changes in v4:
 - update commit message
 - adjust comment
 - rename config option to add EFI_ prefix
---
 xen/common/Kconfig    | 10 ++++++++++
 xen/common/efi/boot.c | 33 ++++++++++++++++++++++++++++++---
 2 files changed, 40 insertions(+), 3 deletions(-)

Comments

Jan Beulich Oct. 25, 2019, 2:27 p.m. UTC | #1
On 24.10.2019 05:45, Marek Marczykowski-Górecki wrote:
> Some UEFI implementations are not happy about lack of
> SetVirtualAddressMap() call. Likely abuse the address map change
> notification to do things beyond the necessary ConvertPointer() calls.
> Specifically, wihtout the SetVirtualAddressMap() call, some access
> EfiBootServices{Code,Data}, or even totally unmapped areas. Example
> crash of GetVariable() call on Thinkpad W540:
> 
>     Xen call trace:
>        [<0000000000000080>] 0000000000000080
>        [<8c2b0398e0000daa>] 8c2b0398e0000daa
> 
>     Pagetable walk from ffffffff858483a1:
>        L4[0x1ff] = 0000000000000000 ffffffffffffffff
> 
>     ****************************************
>     Panic on CPU 0:
>     FATAL PAGE FAULT
>     [error_code=0002]
>     Faulting linear address: ffffffff858483a1
>     ****************************************
> 
> Fix this by calling SetVirtualAddressMap() runtime service, giving it
> 1:1 map for areas marked as needed during runtime. The address space in
> which EFI runtime services are called is unchanged, but UEFI view of it
> may be.
> Since it's fairly late in Xen 4.13 development cycle, disable it
> by default and hide behind EXPERT.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff mbox series

Patch

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 16829f6..549a7d5 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -88,6 +88,16 @@  config KEXEC
 
 	  If unsure, say Y.
 
+config EFI_SET_VIRTUAL_ADDRESS_MAP
+    bool "EFI: call SetVirtualAddressMap()" if EXPERT = "y"
+    ---help---
+      Call EFI SetVirtualAddressMap() runtime service to setup memory map for
+      further runtime services. According to UEFI spec, it isn't strictly
+      necessary, but many UEFI implementations misbehave when this call is
+      missing.
+
+      If unsure, say N.
+
 config XENOPROF
 	def_bool y
 	prompt "Xen Oprofile Support" if EXPERT = "y"
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index cddf3de..9debc5b 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1056,11 +1056,17 @@  static void __init efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop
         efi_arch_video_init(gop, info_size, mode_info);
 }
 
+#define INVALID_VIRTUAL_ADDRESS (0xBAAADUL << \
+                                 (EFI_PAGE_SHIFT + BITS_PER_LONG - 32))
+
 static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 {
     EFI_STATUS status;
     UINTN info_size = 0, map_key;
     bool retry;
+#ifdef CONFIG_EFI_SET_VIRTUAL_ADDRESS_MAP
+    unsigned int i;
+#endif
 
     efi_bs->GetMemoryMap(&info_size, NULL, &map_key,
                          &efi_mdesc_size, &mdesc_ver);
@@ -1094,6 +1100,26 @@  static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste
     if ( EFI_ERROR(status) )
         PrintErrMesg(L"Cannot exit boot services", status);
 
+#ifdef CONFIG_EFI_SET_VIRTUAL_ADDRESS_MAP
+    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
+    {
+        EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
+
+        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
+            desc->VirtualStart = desc->PhysicalStart;
+        else
+            desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
+    }
+    status = efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
+                                          mdesc_ver, efi_memmap);
+    if ( status != EFI_SUCCESS )
+    {
+        printk(XENLOG_ERR "EFI: SetVirtualAddressMap() failed (%#lx), disabling runtime services\n",
+               status);
+        __clear_bit(EFI_RS, &efi_flags);
+    }
+#endif
+
     /* Adjust pointers into EFI. */
     efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START;
     efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
@@ -1460,8 +1486,6 @@  static bool __init rt_range_valid(unsigned long smfn, unsigned long emfn)
     return true;
 }
 
-#define INVALID_VIRTUAL_ADDRESS (0xBAAADUL << \
-                                 (EFI_PAGE_SHIFT + BITS_PER_LONG - 32))
 
 void __init efi_init_memory(void)
 {
@@ -1576,7 +1600,10 @@  void __init efi_init_memory(void)
         return;
     }
 
-    /* Set up 1:1 page tables to do runtime calls in "physical" mode. */
+    /*
+     * Set up 1:1 page tables for runtime calls. See SetVirtualAddressMap() in
+     * efi_exit_boot().
+     */
     efi_l4_pgtable = alloc_xen_pagetable();
     BUG_ON(!efi_l4_pgtable);
     clear_page(efi_l4_pgtable);