Re: [PATCH v3] x86/efi: Correct ident mapping of efi old_map when kalsr enabled

[Baoquan He]

On 05/17/17 at 10:55am, Dave Young wrote:
> Hi, Baoquan
> >  arch/x86/platform/efi/efi_64.c | 69 +++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 61 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> > index c488625..c9dffec 100644
> > --- a/arch/x86/platform/efi/efi_64.c
> > +++ b/arch/x86/platform/efi/efi_64.c
> > @@ -71,11 +71,13 @@ static void __init early_code_mapping_set_exec(int executable)
> >  
> >  pgd_t * __init efi_call_phys_prolog(void)
> >  {
> > -	unsigned long vaddress;
> > -	pgd_t *save_pgd;
> > +	unsigned long vaddr, addr_pgd, addr_p4d, addr_pud;
> > +	pgd_t *save_pgd, *pgd_k, *pgd_efi;
> > +	p4d_t *p4d, *p4d_k, *p4d_efi;
> > +	pud_t *pud;
> >  
> >  	int pgd;
> > -	int n_pgds;
> > +	int n_pgds, i, j;
> >  
> >  	if (!efi_enabled(EFI_OLD_MEMMAP)) {
> >  		save_pgd = (pgd_t *)read_cr3();
> > @@ -88,10 +90,44 @@ pgd_t * __init efi_call_phys_prolog(void)
> >  	n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PGDIR_SIZE);
> >  	save_pgd = kmalloc_array(n_pgds, sizeof(*save_pgd), GFP_KERNEL);
> >  
> > +	/*
> > +	 * Build 1:1 ident mapping for old_map usage. It needs to be noticed
> > +	 * that PAGE_OFFSET is PGDIR_SIZE aligned with KASLR disabled, while
> > +	 * PUD_SIZE ALIGNED with KASLR enabled. So for a given physical
> > +	 * address X, the pud_index(X) != pud_index(__va(X)), we can only copy
> > +	 * pud entry of __va(X) to fill in pud entry of X to build 1:1 mapping
> > +	 * . Means here we can only reuse pmd table of direct mapping.
> > +	 */
> >  	for (pgd = 0; pgd < n_pgds; pgd++) {
> > -		save_pgd[pgd] = *pgd_offset_k(pgd * PGDIR_SIZE);
> > -		vaddress = (unsigned long)__va(pgd * PGDIR_SIZE);
> > -		set_pgd(pgd_offset_k(pgd * PGDIR_SIZE), *pgd_offset_k(vaddress));
> > +		addr_pgd = (unsigned long)(pgd * PGDIR_SIZE);
> > +		vaddr = (unsigned long)__va(pgd * PGDIR_SIZE);
> > +		pgd_efi = pgd_offset_k(addr_pgd);
> > +		save_pgd[pgd] = *pgd_efi;
> > +		p4d =  p4d_alloc(&init_mm, pgd_efi, addr_pgd);
> > +
> > +		if (!p4d) {
> > +			pr_err("Failed to allocate p4d table \n");
> > +			goto out;
> > +		}
> > +		for(i=0; i<PTRS_PER_P4D; i++) {
> 
> There is code style issue here and other for loop later..

Oops, I forget running scripts/checkpatch.pl to check patch. Will change
and post v4. May post with sgi v4 fix about the sgi uv mmioh region
issue, have discussed with HPE SGI developer about that.

Thanks for pointing it out!

Thanks
Baoquan

> 
> > +			addr_p4d = addr_pgd + i * P4D_SIZE;
> > +			p4d_efi = p4d + p4d_index(addr_p4d);
> > +			pud = pud_alloc(&init_mm, p4d_efi, addr_p4d);
> > +			if (!pud) {
> > +				pr_err("Failed to allocate pud table \n");
> > +				goto out;
> > +			}
> > +			for(j=0; j<PTRS_PER_PUD; j++) {
> > +				addr_pud = addr_p4d + j * PUD_SIZE;
> > +				if (addr_pud > (max_pfn << PAGE_SHIFT))
> > +					break;
> > +				vaddr = (unsigned long)__va(addr_pud);
> > +
> > +				pgd_k = pgd_offset_k(vaddr);
> > +				p4d_k = p4d_offset(pgd_k, vaddr);
> > +				pud[j] = *pud_offset(p4d_k, vaddr);
> > +			}
> > +		}
> >  	}
> >  out:
> >  	__flush_tlb_all();
> > @@ -104,8 +140,11 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
> >  	/*
> >  	 * After the lock is released, the original page table is restored.
> >  	 */
> > -	int pgd_idx;
> > +	int pgd_idx, i;
> >  	int nr_pgds;
> > +	pgd_t *pgd;
> > +        p4d_t *p4d;
> > +        pud_t *pud;
> >  
> >  	if (!efi_enabled(EFI_OLD_MEMMAP)) {
> >  		write_cr3((unsigned long)save_pgd);
> > @@ -115,9 +154,23 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
> >  
> >  	nr_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT) , PGDIR_SIZE);
> >  
> > -	for (pgd_idx = 0; pgd_idx < nr_pgds; pgd_idx++)
> > +	for (pgd_idx = 0; pgd_idx < nr_pgds; pgd_idx++) {
> > +		pgd = pgd_offset_k(pgd_idx * PGDIR_SIZE);
> >  		set_pgd(pgd_offset_k(pgd_idx * PGDIR_SIZE), save_pgd[pgd_idx]);
> >  
> > +		if (!(pgd_val(*pgd) & _PAGE_PRESENT))
> > +			continue;
> > +		for(i=0; i<PTRS_PER_P4D; i++) {
> > +			p4d = p4d_offset(pgd, pgd_idx * PGDIR_SIZE + i * P4D_SIZE);
> > +			if (!(p4d_val(*p4d) & _PAGE_PRESENT))
> > +				continue;
> > +			pud = (pud_t*)p4d_page_vaddr(*p4d);
> > +			pud_free(&init_mm, pud);
> > +                }
> > +		p4d = (p4d_t*)pgd_page_vaddr(*pgd);
> > +		p4d_free(&init_mm, p4d);
> > +	}
> > +
> >  	kfree(save_pgd);
> >  
> >  	__flush_tlb_all();
> > -- 
> > 2.5.5
> > 
> 
> Thanks
> Dave