Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [PATCH v1 0/2] Optionally call EFI SetVirtualAddressMap()
@ 2019-10-09  0:48 Marek Marczykowski-Górecki
  2019-10-09  0:48 ` [Xen-devel] [PATCH v1 1/2] efi: remove old SetVirtualAddressMap() arrangement Marek Marczykowski-Górecki
  2019-10-09  0:48 ` [Xen-devel] [PATCH v1 2/2] xen/efi: optionally call SetVirtualAddressMap() Marek Marczykowski-Górecki
  0 siblings, 2 replies; 3+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-10-09  0:48 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Stefano Stabellini, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, Tim Deegan, Julien Grall,
	Jan Beulich

Workaround buggy UEFI accessing boot services memory after ExitBootServices().
Patches discussed here:
https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00701.html

Cc: Juergen Gross <jgross@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>

Marek Marczykowski-Górecki (2):
  efi: remove old SetVirtualAddressMap() arrangement
  xen/efi: optionally call SetVirtualAddressMap()

 xen/common/Kconfig    | 13 ++++++++++++-
 xen/common/efi/boot.c | 48 +++++++++++++++++++++++---------------------
 2 files changed, 39 insertions(+), 22 deletions(-)

base-commit: 7a4e6711114905b3cbbe48e81c3222361a7f3579
-- 
git-series 0.9.1

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Xen-devel] [PATCH v1 1/2] efi: remove old SetVirtualAddressMap() arrangement
  2019-10-09  0:48 [Xen-devel] [PATCH v1 0/2] Optionally call EFI SetVirtualAddressMap() Marek Marczykowski-Górecki
@ 2019-10-09  0:48 ` Marek Marczykowski-Górecki
  2019-10-09  0:48 ` [Xen-devel] [PATCH v1 2/2] xen/efi: optionally call SetVirtualAddressMap() Marek Marczykowski-Górecki
  1 sibling, 0 replies; 3+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-10-09  0:48 UTC (permalink / raw)
  To: xen-devel; +Cc: Marek Marczykowski-Górecki, Jan Beulich

Remove unused (#ifdef-ed out) code. Reviving it in its current shape
won't fly because:
 - SetVirtualAddressMap() needs to be mapped with 1:1 mapping, which
   isn't the case at this time
 - it uses directmap, which is going away soon
 - it uses directmap, which is mapped with NX, breaking EfiRuntimeServicesCode

No functional change.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 xen/common/efi/boot.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 7919378..cddf3de 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -29,9 +29,6 @@
 #undef __ASSEMBLY__
 #endif
 
-/* Using SetVirtualAddressMap() is incompatible with kexec: */
-#undef USE_SET_VIRTUAL_ADDRESS_MAP
-
 #define EFI_REVISION(major, minor) (((major) << 16) | (minor))
 
 #define SMBIOS3_TABLE_GUID \
@@ -1099,9 +1096,6 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste
 
     /* Adjust pointers into EFI. */
     efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START;
-#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
-    efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START;
-#endif
     efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
 }
@@ -1422,7 +1416,6 @@ static int __init parse_efi_param(const char *s)
 }
 custom_param("efi", parse_efi_param);
 
-#ifndef USE_SET_VIRTUAL_ADDRESS_MAP
 static __init void copy_mapping(unsigned long mfn, unsigned long end,
                                 bool (*is_valid)(unsigned long smfn,
                                                  unsigned long emfn))
@@ -1466,7 +1459,6 @@ static bool __init rt_range_valid(unsigned long smfn, unsigned long emfn)
 {
     return true;
 }
-#endif
 
 #define INVALID_VIRTUAL_ADDRESS (0xBAAADUL << \
                                  (EFI_PAGE_SHIFT + BITS_PER_LONG - 32))
@@ -1474,13 +1466,11 @@ static bool __init rt_range_valid(unsigned long smfn, unsigned long emfn)
 void __init efi_init_memory(void)
 {
     unsigned int i;
-#ifndef USE_SET_VIRTUAL_ADDRESS_MAP
     struct rt_extra {
         struct rt_extra *next;
         unsigned long smfn, emfn;
         unsigned int prot;
     } *extra, *extra_head = NULL;
-#endif
 
     free_ebmalloc_unused_mem();
 
@@ -1563,7 +1553,6 @@ void __init efi_init_memory(void)
                 printk(XENLOG_ERR "Could not map MFNs %#lx-%#lx\n",
                        smfn, emfn - 1);
         }
-#ifndef USE_SET_VIRTUAL_ADDRESS_MAP
         else if ( !((desc->PhysicalStart + len - 1) >> (VADDR_BITS - 1)) &&
                   (extra = xmalloc(struct rt_extra)) != NULL )
         {
@@ -1574,12 +1563,8 @@ void __init efi_init_memory(void)
             extra_head = extra;
             desc->VirtualStart = desc->PhysicalStart;
         }
-#endif
         else
         {
-#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
-            /* XXX allocate e.g. down from FIXADDR_START */
-#endif
             printk(XENLOG_ERR "No mapping for MFNs %#lx-%#lx\n",
                    smfn, emfn - 1);
         }
@@ -1591,10 +1576,6 @@ void __init efi_init_memory(void)
         return;
     }
 
-#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
-    efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
-                                 mdesc_ver, efi_memmap);
-#else
     /* Set up 1:1 page tables to do runtime calls in "physical" mode. */
     efi_l4_pgtable = alloc_xen_pagetable();
     BUG_ON(!efi_l4_pgtable);
@@ -1680,6 +1661,5 @@ void __init efi_init_memory(void)
     for ( i = l4_table_offset(HYPERVISOR_VIRT_START);
           i < l4_table_offset(DIRECTMAP_VIRT_END); ++i )
         efi_l4_pgtable[i] = idle_pg_table[i];
-#endif
 }
 #endif
-- 
git-series 0.9.1

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Xen-devel] [PATCH v1 2/2] xen/efi: optionally call SetVirtualAddressMap()
  2019-10-09  0:48 [Xen-devel] [PATCH v1 0/2] Optionally call EFI SetVirtualAddressMap() Marek Marczykowski-Górecki
  2019-10-09  0:48 ` [Xen-devel] [PATCH v1 1/2] efi: remove old SetVirtualAddressMap() arrangement Marek Marczykowski-Górecki
@ 2019-10-09  0:48 ` Marek Marczykowski-Górecki
  1 sibling, 0 replies; 3+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-10-09  0:48 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, Tim Deegan, Julien Grall,
	Jan Beulich

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, hence it's incompatible
with kexec. For this reason, make it an optional feature, depending on
!KEXEC. And to not inflate number of supported configurations, hide it
behind EXPERT.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 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..fe98f8a 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
+    depends on !KEXEC
+    ---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 makes
+      it incompatible with kexec (kexec-ing this Xen from other Xen or Linux).
+
+      If unsuser, 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..5c187db 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);
@@ -1098,6 +1104,26 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste
     efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START;
     efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
+
+#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
 }
 
 static int __init __maybe_unused set_color(u32 mask, int bpp, u8 *pos, u8 *sz)
@@ -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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09  0:48 [Xen-devel] [PATCH v1 0/2] Optionally call EFI SetVirtualAddressMap() Marek Marczykowski-Górecki
2019-10-09  0:48 ` [Xen-devel] [PATCH v1 1/2] efi: remove old SetVirtualAddressMap() arrangement Marek Marczykowski-Górecki
2019-10-09  0:48 ` [Xen-devel] [PATCH v1 2/2] xen/efi: optionally call SetVirtualAddressMap() Marek Marczykowski-Górecki

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox