On Thu, Nov 12, 2015 at 03:40:21PM +0000, Matt Fleming wrote:
> This change is a prerequisite for pending patches that switch to a
> dedicated EFI page table, instead of using 'trampoline_pgd' which
> shares PGD entries with 'swapper_pg_dir'. The pending patches make it
> impossible to dereference the runtime service function pointer without
> first switching %cr3.
> 
> It's true that we now have duplicated switching code in
> efi_call_virt() and efi_call_phys_{prolog,epilog}() but we are
> sacrificing code duplication for a little more clarity and the ease of
> writing the page table switching code in C instead of asm.
> 
> Cc: Borislav Petkov <[email protected]>
> Cc: Sai Praneeth Prakhya <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Dave Jones <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Denys Vlasenko <[email protected]>,
> Cc: Stephen Smalley <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>
> ---
>  arch/x86/include/asm/efi.h          | 25 +++++++++++++++++++++
>  arch/x86/platform/efi/efi_64.c      | 24 ++++++++++-----------
>  arch/x86/platform/efi/efi_stub_64.S | 43 
> -------------------------------------
>  3 files changed, 36 insertions(+), 56 deletions(-)
> 
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index cfee9d4b02af..f9d99d4e7b1a 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -3,6 +3,7 @@
>  
>  #include <asm/fpu/api.h>
>  #include <asm/pgtable.h>
> +#include <asm/tlb.h>
>  
>  /*
>   * We map the EFI regions needed for runtime services non-contiguously,
> @@ -64,6 +65,17 @@ extern u64 asmlinkage efi_call(void *fp, ...);
>  
>  #define efi_call_phys(f, args...)            efi_call((f), args)
>  
> +/*
> + * Scratch space used for switching the pagetable in the EFI stub
> + */
> +struct efi_scratch {
> +     u64 r15;
> +     u64 prev_cr3;
> +     pgd_t *efi_pgt;
> +     bool use_pgd;
> +     u64 phys_stack;
> +} __packed;
> +
>  #define efi_call_virt(f, ...)                                                
> \
>  ({                                                                   \
>       efi_status_t __s;                                               \
> @@ -71,7 +83,20 @@ extern u64 asmlinkage efi_call(void *fp, ...);
>       efi_sync_low_kernel_mappings();                                 \
>       preempt_disable();                                              \
>       __kernel_fpu_begin();                                           \
> +                                                                     \
> +     if (efi_scratch.use_pgd) {                                      \
> +             efi_scratch.prev_cr3 = read_cr3();                      \
> +             write_cr3((unsigned long)efi_scratch.efi_pgt);          \
> +             __flush_tlb_all();                                      \
> +     }                                                               \
> +                                                                     \
>       __s = efi_call((void *)efi.systab->runtime->f, __VA_ARGS__);    \
> +                                                                     \
> +     if (efi_scratch.use_pgd) {                                      \
> +             write_cr3(efi_scratch.prev_cr3);                        \
> +             __flush_tlb_all();                                      \
> +     }                                                               \
> +                                                                     \
>       __kernel_fpu_end();                                             \
>       preempt_enable();                                               \
>       __s;                                                            \

checkpatch is bitching here - not that I agree with it:

WARNING: please, no space before tabs
#87: FILE: arch/x86/include/asm/efi.h:88:
+^I^Iefi_scratch.prev_cr3 = read_cr3(); ^I^I^I\$

WARNING: please, no space before tabs
#89: FILE: arch/x86/include/asm/efi.h:90:
+^I^I__flush_tlb_all(); ^I^I^I^I^I\$

WARNING: please, no space before tabs
#94: FILE: arch/x86/include/asm/efi.h:95:
+^Iif (efi_scratch.use_pgd) { ^I^I^I^I^I\$

WARNING: please, no space before tabs
#96: FILE: arch/x86/include/asm/efi.h:97:
+^I^I__flush_tlb_all(); ^I^I^I^I^I\$

WARNING: please, no space before tabs
#97: FILE: arch/x86/include/asm/efi.h:98:
+^I} ^I^I^I^I^I^I^I^I\$


> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 634536034e32..ab5f14a886cc 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -47,16 +47,7 @@
>   */
>  static u64 efi_va = EFI_VA_START;
>  
> -/*
> - * Scratch space used for switching the pagetable in the EFI stub
> - */
> -struct efi_scratch {
> -     u64 r15;
> -     u64 prev_cr3;
> -     pgd_t *efi_pgt;
> -     bool use_pgd;
> -     u64 phys_stack;
> -} __packed;
> +struct efi_scratch efi_scratch;
>  
>  static void __init early_code_mapping_set_exec(int executable)
>  {
> @@ -83,8 +74,11 @@ pgd_t * __init efi_call_phys_prolog(void)
>       int pgd;
>       int n_pgds;
>  
> -     if (!efi_enabled(EFI_OLD_MEMMAP))
> -             return NULL;
> +     if (!efi_enabled(EFI_OLD_MEMMAP)) {
> +             save_pgd = (pgd_t *)read_cr3();
> +             write_cr3((unsigned long)efi_scratch.efi_pgt);
> +             goto out;
> +     }
>  
>       early_code_mapping_set_exec(1);
>  
> @@ -96,6 +90,7 @@ pgd_t * __init efi_call_phys_prolog(void)
>               vaddress = (unsigned long)__va(pgd * PGDIR_SIZE);
>               set_pgd(pgd_offset_k(pgd * PGDIR_SIZE), 
> *pgd_offset_k(vaddress));
>       }
> +out:
>       __flush_tlb_all();
>  
>       return save_pgd;
> @@ -109,8 +104,11 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)

There's a comment here:

        /*
         * After the lock is released, the original page table is restored.
         */

Which lock are we talking about?

>       int pgd_idx;
>       int nr_pgds;
>  
> -     if (!save_pgd)
> +     if (!efi_enabled(EFI_OLD_MEMMAP)) {
> +             write_cr3((unsigned long)save_pgd);
> +             __flush_tlb_all();
>               return;
> +     }
>  
>       nr_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT) , PGDIR_SIZE);
>  

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to