On Thu, Feb 28, 2019 at 06:04:19PM +0800, Baoquan He wrote:
> On 02/28/19 at 12:55pm, Kirill A. Shutemov wrote:
> > I cannot apply the first patch too. Hm?
>
> It's weird. I use 'git cherry-pick' and it can be cleanly applied.
>
> And git formatted patch can be applied too. Could you try below one?
> git am works for me with it, based on this commit on linus's tree.
> 7d762d69145a afs: Fix manually set volume location server list.
>
>
> From dcbe8e4846102fdd051deb3c2946125c17b270fb Mon Sep 17 00:00:00 2001
> From: Baoquan He <b...@redhat.com>
> Date: Thu, 21 Feb 2019 11:46:59 +0800
> Subject: [PATCH v3] x86/mm/KASLR: Only build one PUD entry of area for real
> mode
> trampoline
>
> The current code builds identity mapping for real mode treampoline by
> borrowing page tables from the direct mapping section if KASLR is
> enabled. It will copy present entries of the first PUD table in 4-level
> paging mode, or the first P4D table in 5-level paging mode.
>
> However, there's only a very small area under low 1 MB reserved
> for real mode trampoline in reserve_real_mode(). Makes no sense
> to build up so large area of mapping for it. Since the randomization
> granularity in 4-level is 1 GB, and 512 GB in 5-level, only copying
> one PUD entry is enough.
>
> Hence, only copy one PUD entry of area where physical address 0
> resides. And this is preparation for later changing the randomization
> granularity of 5-level paging mode from 512 GB to 1 GB.
>
> Signed-off-by: Baoquan He <b...@redhat.com>
> ---
> arch/x86/mm/kaslr.c | 82 +++++++++++++++++++++------------------------
> 1 file changed, 38 insertions(+), 44 deletions(-)
>
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 754b5da91d43..131e08a10a68 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -226,74 +226,68 @@ void __init kernel_randomize_memory(void)
>
> static void __meminit init_trampoline_pud(void)
> {
> - unsigned long paddr, paddr_next;
> + unsigned long paddr, vaddr;
> pgd_t *pgd;
> - pud_t *pud_page, *pud_page_tramp;
> - int i;
> +
Unneeded empty line.
> + pud_t *pud_page, *pud_page_tramp, *pud, *pud_tramp;
>
> pud_page_tramp = alloc_low_page();
>
> paddr = 0;
> + vaddr = (unsigned long)__va(paddr);
Maybe just
vaddr = PAGE_OFFSET;
?
> pgd = pgd_offset_k((unsigned long)__va(paddr));
We do have 'vaddr' already.
pgd = pgd_offset_k(vaddr);
> - pud_page = (pud_t *) pgd_page_vaddr(*pgd);
> -
> - for (i = pud_index(paddr); i < PTRS_PER_PUD; i++, paddr = paddr_next) {
> - pud_t *pud, *pud_tramp;
> - unsigned long vaddr = (unsigned long)__va(paddr);
>
> - pud_tramp = pud_page_tramp + pud_index(paddr);
> - pud = pud_page + pud_index(vaddr);
> - paddr_next = (paddr & PUD_MASK) + PUD_SIZE;
> -
> - *pud_tramp = *pud;
> - }
> + if (pgtable_l5_enabled()) {
> + p4d_t *p4d_page, *p4d_page_tramp, *p4d, *p4d_tramp;
> + p4d_page_tramp = alloc_low_page();
>
> - set_pgd(&trampoline_pgd_entry,
> - __pgd(_KERNPG_TABLE | __pa(pud_page_tramp)));
> -}
> + p4d_page = (p4d_t *) pgd_page_vaddr(*pgd);
> + p4d = p4d_page + p4d_index(vaddr);
>
> -static void __meminit init_trampoline_p4d(void)
> -{
> - unsigned long paddr, paddr_next;
> - pgd_t *pgd;
> - p4d_t *p4d_page, *p4d_page_tramp;
> - int i;
> + pud_page = (pud_t *) p4d_page_vaddr(*p4d);
> + pud = pud_page + pud_index(vaddr);
>
> - p4d_page_tramp = alloc_low_page();
> + p4d_tramp = p4d_page_tramp + p4d_index(paddr);
> + pud_tramp = pud_page_tramp + pud_index(paddr);
p?d_index() has to be called on the virtual address. It shouldn't break
anything, but it's wrong from conceptual point of view.
I don't think need paddr at all in the function.
BTW, any reason we cannot use p?d_offset() instead of playing with
p?d_index() directly?
--
Kirill A. Shutemov
