On Thu, 12 Nov, at 07:38:13PM, Borislav Petkov wrote:
> > @@ -831,6 +831,12 @@ static void __init __efi_enter_virtual_mode(void)
> >     efi.systab = NULL;
> >  
> >     efi_merge_regions();
> > +   if (efi_alloc_page_tables()) {
> > +           pr_err("Failed to allocate EFI page tables\n");
> > +           clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> > +           return;
> > +   }
> 
> This should happen before efi_merge_regions() - no need to merge if we
> can't alloc PGT.
 
Fair point.

> > +
> >     new_memmap = efi_map_regions(&count, &pg_shift);
> >     if (!new_memmap) {
> >             pr_err("Error reallocating memory, EFI runtime 
> > non-functional!\n");
> > @@ -888,29 +894,12 @@ static void __init __efi_enter_virtual_mode(void)
> >  
> >     efi_runtime_mkexec();
> >  
> > -   /*
> > -    * We mapped the descriptor array into the EFI pagetable above but we're
> > -    * not unmapping it here. Here's why:
> > -    *
> > -    * We're copying select PGDs from the kernel page table to the EFI page
> > -    * table and when we do so and make changes to those PGDs like unmapping
> > -    * stuff from them, those changes appear in the kernel page table and we
> > -    * go boom.
> > -    *
> > -    * From setup_real_mode():
> > -    *
> > -    * ...
> > -    * trampoline_pgd[0] = init_level4_pgt[pgd_index(__PAGE_OFFSET)].pgd;
> > -    *
> > -    * In this particular case, our allocation is in PGD 0 of the EFI page
> > -    * table but we've copied that PGD from PGD[272] of the EFI page table:
> > -    *
> > -    *      pgd_index(__PAGE_OFFSET = 0xffff880000000000) = 272
> > -    *
> > -    * where the direct memory mapping in kernel space is.
> > -    *
> > -    * new_memmap's VA comes from that direct mapping and thus clearing it,
> > -    * it would get cleared in the kernel page table too.
> > +        /*
> 
> ERROR: code indent should use tabs where possible
> #149: FILE: arch/x86/platform/efi/efi.c:897:
> +        /*$
> 
> ...
 
Dammit vim. I'll fix this.

> >  void efi_sync_low_kernel_mappings(void)
> >  {
> > -   unsigned num_pgds;
> > -   pgd_t *pgd = (pgd_t *)__va(real_mode_header->trampoline_pgd);
> > +   unsigned num_entries;
> > +   pgd_t *pgd_k, *pgd_efi;
> > +   pud_t *pud_k, *pud_efi;
> >  
> >     if (efi_enabled(EFI_OLD_MEMMAP))
> >             return;
> >  
> > -   num_pgds = pgd_index(MODULES_END - 1) - pgd_index(PAGE_OFFSET);
> > +   /*
> > +    * We can share all PGD entries apart from the one entry that
> > +    * covers the EFI runtime mapping space.
> > +    *
> > +    * Make sure the EFI runtime region mappings are guaranteed to
> > +    * only span a single PGD entry and that the entry also maps
> > +    * other important kernel regions.
> > +    */
> > +   BUILD_BUG_ON(pgd_index(EFI_VA_END) != pgd_index(MODULES_END));
> > +   BUILD_BUG_ON((EFI_VA_START & PGDIR_MASK) !=
> > +                           (EFI_VA_END & PGDIR_MASK));
> 
> You can align them in a more readable way:
> 
>       BUILD_BUG_ON((EFI_VA_START & PGDIR_MASK) !=
>                      (EFI_VA_END & PGDIR_MASK));

Sure.
--
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