[v6,10/15] efi: switch to new APIs in EFI code

Commit Message

Hongyan Xia April 24, 2020, 2:09 p.m. UTC

From: Wei Liu <[e-mail]>

Signed-off-by: Wei Liu <[e-mail]>
Signed-off-by: Hongyan Xia <[e-mail]>
---
 xen/arch/x86/efi/runtime.h | 10 +++++++--
 xen/common/efi/boot.c      | 46 ++++++++++++++++++++++++++------------
 xen/common/efi/efi.h       |  3 ++-
 xen/common/efi/runtime.c   |  8 +++----
 4 files changed, 46 insertions(+), 21 deletions(-)

Comments

Jan Beulich April 30, 2020, 12:55 p.m. UTC | #1

On 24.04.2020 16:09, Hongyan Xia wrote:
> --- a/xen/arch/x86/efi/runtime.h
> +++ b/xen/arch/x86/efi/runtime.h
> @@ -1,12 +1,18 @@
> +#include <xen/domain_page.h>
> +#include <xen/mm.h>
>  #include <asm/atomic.h>
>  #include <asm/mc146818rtc.h>
>  
>  #ifndef COMPAT
> -l4_pgentry_t *__read_mostly efi_l4_pgtable;
> +mfn_t __read_mostly efi_l4_mfn = INVALID_MFN_INITIALIZER;
>  
>  void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e)
>  {
> -    if ( efi_l4_pgtable )
> +    if ( !mfn_eq(efi_l4_mfn, INVALID_MFN) )
> +    {
> +        l4_pgentry_t *efi_l4_pgtable = map_domain_page(efi_l4_mfn);
>          l4e_write(efi_l4_pgtable + l4idx, l4e);

Blank line between declaration(s) and statement(s) please.

Also, while I realize the choice of name of the local variable
is (presumably) to avoid further code churn, I think it isn't
really suitable for a local variable (also elsewhere below).

> @@ -1489,6 +1493,7 @@ static bool __init rt_range_valid(unsigned long smfn, unsigned long emfn)
>  void __init efi_init_memory(void)
>  {
>      unsigned int i;
> +    l4_pgentry_t *efi_l4_pgtable;
>      struct rt_extra {
>          struct rt_extra *next;
>          unsigned long smfn, emfn;
> @@ -1603,8 +1608,9 @@ void __init efi_init_memory(void)
>       * 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);
> +    efi_l4_mfn = alloc_xen_pagetable_new();
> +    BUG_ON(mfn_eq(efi_l4_mfn, INVALID_MFN));
> +    efi_l4_pgtable = map_domain_page(efi_l4_mfn);
>      clear_page(efi_l4_pgtable);
>  
>      copy_mapping(0, max_page, ram_range_valid);

Why don't you pass the already mapped L4 table into this function,
rather than mapping the same page a 2nd time there?

> @@ -1681,11 +1693,17 @@ void __init efi_init_memory(void)
>              extra_head = extra->next;
>              xfree(extra);
>          }
> +
> +        unmap_domain_page(l1t);
> +        unmap_domain_page(pl2e);
> +        unmap_domain_page(pl3e);

All three should be pulled further up, each to the earliest
possible place (and then using the uppercase version of the
construct as suitable).

Jan

Patch

diff --git a/xen/arch/x86/efi/runtime.h b/xen/arch/x86/efi/runtime.h
index d9eb8f5c27..d2483645c6 100644
--- a/xen/arch/x86/efi/runtime.h
+++ b/xen/arch/x86/efi/runtime.h
@@ -1,12 +1,18 @@ 
+#include <xen/domain_page.h>
+#include <xen/mm.h>
 #include <asm/atomic.h>
 #include <asm/mc146818rtc.h>
 
 #ifndef COMPAT
-l4_pgentry_t *__read_mostly efi_l4_pgtable;
+mfn_t __read_mostly efi_l4_mfn = INVALID_MFN_INITIALIZER;
 
 void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e)
 {
-    if ( efi_l4_pgtable )
+    if ( !mfn_eq(efi_l4_mfn, INVALID_MFN) )
+    {
+        l4_pgentry_t *efi_l4_pgtable = map_domain_page(efi_l4_mfn);
         l4e_write(efi_l4_pgtable + l4idx, l4e);
+        unmap_domain_page(efi_l4_pgtable);
+    }
 }
 #endif
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 8e96ef8de2..715217d2a9 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -6,6 +6,7 @@ 
 #include <xen/compile.h>
 #include <xen/ctype.h>
 #include <xen/dmi.h>
+#include <xen/domain_page.h>
 #include <xen/init.h>
 #include <xen/keyhandler.h>
 #include <xen/lib.h>
@@ -1442,6 +1443,7 @@  static __init void copy_mapping(unsigned long mfn, unsigned long end,
                                                  unsigned long emfn))
 {
     unsigned long next;
+    l4_pgentry_t *efi_l4_pgtable = map_domain_page(efi_l4_mfn);
 
     for ( ; mfn < end; mfn = next )
     {
@@ -1469,6 +1471,8 @@  static __init void copy_mapping(unsigned long mfn, unsigned long end,
         unmap_domain_page(l3src);
         unmap_domain_page(l3dst);
     }
+
+    unmap_domain_page(efi_l4_pgtable);
 }
 
 static bool __init ram_range_valid(unsigned long smfn, unsigned long emfn)
@@ -1489,6 +1493,7 @@  static bool __init rt_range_valid(unsigned long smfn, unsigned long emfn)
 void __init efi_init_memory(void)
 {
     unsigned int i;
+    l4_pgentry_t *efi_l4_pgtable;
     struct rt_extra {
         struct rt_extra *next;
         unsigned long smfn, emfn;
@@ -1603,8 +1608,9 @@  void __init efi_init_memory(void)
      * 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);
+    efi_l4_mfn = alloc_xen_pagetable_new();
+    BUG_ON(mfn_eq(efi_l4_mfn, INVALID_MFN));
+    efi_l4_pgtable = map_domain_page(efi_l4_mfn);
     clear_page(efi_l4_pgtable);
 
     copy_mapping(0, max_page, ram_range_valid);
@@ -1637,39 +1643,45 @@  void __init efi_init_memory(void)
 
         if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
         {
-            pl3e = alloc_xen_pagetable();
-            BUG_ON(!pl3e);
+            mfn_t l3mfn = alloc_xen_pagetable_new();
+
+            BUG_ON(mfn_eq(l3mfn, INVALID_MFN));
+            pl3e = map_domain_page(l3mfn);
             clear_page(pl3e);
             efi_l4_pgtable[l4_table_offset(addr)] =
-                l4e_from_paddr(virt_to_maddr(pl3e), __PAGE_HYPERVISOR);
+                l4e_from_mfn(l3mfn, __PAGE_HYPERVISOR);
         }
         else
-            pl3e = l4e_to_l3e(l4e);
+            pl3e = map_l3t_from_l4e(l4e);
         pl3e += l3_table_offset(addr);
         if ( !(l3e_get_flags(*pl3e) & _PAGE_PRESENT) )
         {
-            pl2e = alloc_xen_pagetable();
-            BUG_ON(!pl2e);
+            mfn_t l2mfn = alloc_xen_pagetable_new();
+
+            BUG_ON(mfn_eq(l2mfn, INVALID_MFN));
+            pl2e = map_domain_page(l2mfn);
             clear_page(pl2e);
-            *pl3e = l3e_from_paddr(virt_to_maddr(pl2e), __PAGE_HYPERVISOR);
+            *pl3e = l3e_from_mfn(l2mfn, __PAGE_HYPERVISOR);
         }
         else
         {
             BUG_ON(l3e_get_flags(*pl3e) & _PAGE_PSE);
-            pl2e = l3e_to_l2e(*pl3e);
+            pl2e = map_l2t_from_l3e(*pl3e);
         }
         pl2e += l2_table_offset(addr);
         if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
         {
-            l1t = alloc_xen_pagetable();
-            BUG_ON(!l1t);
+            mfn_t l1mfn = alloc_xen_pagetable_new();
+
+            BUG_ON(mfn_eq(l1mfn, INVALID_MFN));
+            l1t = map_domain_page(l1mfn);
             clear_page(l1t);
-            *pl2e = l2e_from_paddr(virt_to_maddr(l1t), __PAGE_HYPERVISOR);
+            *pl2e = l2e_from_mfn(l1mfn, __PAGE_HYPERVISOR);
         }
         else
         {
             BUG_ON(l2e_get_flags(*pl2e) & _PAGE_PSE);
-            l1t = l2e_to_l1e(*pl2e);
+            l1t = map_l1t_from_l2e(*pl2e);
         }
         for ( i = l1_table_offset(addr);
               i < L1_PAGETABLE_ENTRIES && extra->smfn < extra->emfn;
@@ -1681,11 +1693,17 @@  void __init efi_init_memory(void)
             extra_head = extra->next;
             xfree(extra);
         }
+
+        unmap_domain_page(l1t);
+        unmap_domain_page(pl2e);
+        unmap_domain_page(pl3e);
     }
 
     /* Insert Xen mappings. */
     for ( i = l4_table_offset(HYPERVISOR_VIRT_START);
           i < l4_table_offset(DIRECTMAP_VIRT_END); ++i )
         efi_l4_pgtable[i] = idle_pg_table[i];
+
+    unmap_domain_page(efi_l4_pgtable);
 }
 #endif
diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
index 2e38d05f3d..e364bae3e0 100644
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -6,6 +6,7 @@ 
 #include <efi/eficapsule.h>
 #include <efi/efiapi.h>
 #include <xen/efi.h>
+#include <xen/mm.h>
 #include <xen/spinlock.h>
 #include <asm/page.h>
 
@@ -29,7 +30,7 @@  extern UINTN efi_memmap_size, efi_mdesc_size;
 extern void *efi_memmap;
 
 #ifdef CONFIG_X86
-extern l4_pgentry_t *efi_l4_pgtable;
+extern mfn_t efi_l4_mfn;
 #endif
 
 extern const struct efi_pci_rom *efi_pci_roms;
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 95367694b5..375b94229e 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -85,7 +85,7 @@  struct efi_rs_state efi_rs_enter(void)
     static const u32 mxcsr = MXCSR_DEFAULT;
     struct efi_rs_state state = { .cr3 = 0 };
 
-    if ( !efi_l4_pgtable )
+    if ( mfn_eq(efi_l4_mfn, INVALID_MFN) )
         return state;
 
     state.cr3 = read_cr3();
@@ -111,7 +111,7 @@  struct efi_rs_state efi_rs_enter(void)
         lgdt(&gdt_desc);
     }
 
-    switch_cr3_cr4(virt_to_maddr(efi_l4_pgtable), read_cr4());
+    switch_cr3_cr4(mfn_to_maddr(efi_l4_mfn), read_cr4());
 
     return state;
 }
@@ -140,9 +140,9 @@  void efi_rs_leave(struct efi_rs_state *state)
 
 bool efi_rs_using_pgtables(void)
 {
-    return efi_l4_pgtable &&
+    return !mfn_eq(efi_l4_mfn, INVALID_MFN) &&
            (smp_processor_id() == efi_rs_on_cpu) &&
-           (read_cr3() == virt_to_maddr(efi_l4_pgtable));
+           (read_cr3() == mfn_to_maddr(efi_l4_mfn));
 }
 
 unsigned long efi_get_time(void)