[v3,1/2] efi: remove old SetVirtualAddressMap() arrangement

Commit Message

Marek Marczykowski-Górecki Oct. 12, 2019, 10:11 p.m. UTC

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

No functional change.

Signed-off-by: Marek Marczykowski-Górecki <[e-mail]>
Acked-by: Andrew Cooper <[e-mail]>
---
 xen/common/efi/boot.c | 20 --------------------
 1 file changed, 20 deletions(-)

Comments

Jan Beulich Oct. 23, 2019, 3:15 p.m. UTC | #1

On 13.10.2019 00:11, Marek Marczykowski-Górecki  wrote:
> @@ -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

This doesn't get re-instated in any way by patch 2. How come you
get away without? In any event this is perhaps the best example
of why I personally think it would have been better to change
things in place, rather than remove everything first. But for
some of the other change the question also arises of why they
don't need re-instating in one form or another.

Jan

Jan Beulich Oct. 23, 2019, 3:26 p.m. UTC | #2

On 13.10.2019 00:11, Marek Marczykowski-Górecki  wrote:
> @@ -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. */

This comment, btw, also wants either adjusting or removing.

Jan

Patch

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