xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
To: xen-devel@lists.xenproject.org
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	"Tim Deegan" <tim@xen.org>, "Julien Grall" <julien.grall@arm.com>,
	"Jan Beulich" <jbeulich@suse.com>
Subject: [Xen-devel] [PATCH v3 2/2] xen/efi: optionally call SetVirtualAddressMap()
Date: Sun, 13 Oct 2019 00:11:39 +0200	[thread overview]
Message-ID: <aedda92afd26caac474870d44504074d3b2ff6d0.1570918263.git-series.marmarek@invisiblethingslab.com> (raw)
In-Reply-To: <cover.b80738fa53144bc6e5e32c048ccf4e4b95355844.1570918263.git-series.marmarek@invisiblethingslab.com>

Some UEFI implementations are not happy about running in 1:1 addressing,
but really virtual address space. Specifically, 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.
SetVirtualAddressMap() can be called only once, so it might make
future kexec EFI plumbing more complex or incompatible with this option.
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.
---
 xen/common/Kconfig    | 13 +++++++++++++
 xen/common/efi/boot.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 16829f6..4ad4534 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -88,6 +88,19 @@ config KEXEC
 
 	  If unsure, say Y.
 
+config SET_VIRTUAL_ADDRESS_MAP
+    bool "EFI: call SetVirtualAddressMap()" if EXPERT = "y"
+    default n
+    ---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. On the other hand, this call can be made only once, which might
+      be incompatible with future EFI support in Xen's kexec. Kexec ability
+      in the current form is not affected.
+
+      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..6eaabd4 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_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_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)
 {
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply	other threads:[~2019-10-12 22:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-12 22:11 [Xen-devel] [PATCH v3 0/2] Optionally call EFI SetVirtualAddressMap() Marek Marczykowski-Górecki
2019-10-12 22:11 ` [Xen-devel] [PATCH v3 1/2] efi: remove old SetVirtualAddressMap() arrangement Marek Marczykowski-Górecki
2019-10-23 15:15   ` Jan Beulich
2019-10-23 15:36     ` Marek Marczykowski-Górecki
2019-10-23 16:10       ` Jan Beulich
2019-10-23 16:38         ` Marek Marczykowski-Górecki
2019-10-23 15:26   ` Jan Beulich
2019-10-12 22:11 ` Marek Marczykowski-Górecki [this message]
2019-10-15 23:40   ` [Xen-devel] [PATCH v3 2/2] xen/efi: optionally call SetVirtualAddressMap() Andrew Cooper
2019-10-23 15:37   ` Jan Beulich
2019-10-23 16:07     ` Marek Marczykowski-Górecki
2019-10-23 16:13       ` Jan Beulich
2019-10-15 12:25 ` [Xen-devel] [PATCH v3 0/2] Optionally call EFI SetVirtualAddressMap() Jason Andryuk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aedda92afd26caac474870d44504074d3b2ff6d0.1570918263.git-series.marmarek@invisiblethingslab.com \
    --to=marmarek@invisiblethingslab.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).