2018-03-14 21:02:52

by Kani, Toshimitsu

[permalink] [raw]
Subject: [PATCH] x86/mm: remove pointless checks in vmalloc_fault

vmalloc_fault() sets user's pgd or p4d from the kernel page table.
Once it's set, all tables underneath are identical. There is no point
of following the same page table with two separate pointers and makes
sure they see the same with BUG().

Remove the pointless checks in vmalloc_fault(). Also rename the kernel
pgd/p4d pointers to pgd_k/p4d_k so that their names are consistent in
the file.

Suggested-by: Andy Lutomirski <[email protected]>
Signed-off-by: Toshi Kani <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Gratian Crisan <[email protected]>
---
Rebased 2/2 patch on tip.
---
arch/x86/mm/fault.c | 56 +++++++++++++++------------------------------------
1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 8e012c3f6ad6..73bd8c95ac71 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -417,11 +417,11 @@ void vmalloc_sync_all(void)
*/
static noinline int vmalloc_fault(unsigned long address)
{
- pgd_t *pgd, *pgd_ref;
- p4d_t *p4d, *p4d_ref;
- pud_t *pud, *pud_ref;
- pmd_t *pmd, *pmd_ref;
- pte_t *pte, *pte_ref;
+ pgd_t *pgd, *pgd_k;
+ p4d_t *p4d, *p4d_k;
+ pud_t *pud;
+ pmd_t *pmd;
+ pte_t *pte;

/* Make sure we are in vmalloc area: */
if (!(address >= VMALLOC_START && address < VMALLOC_END))
@@ -435,73 +435,51 @@ static noinline int vmalloc_fault(unsigned long address)
* case just flush:
*/
pgd = (pgd_t *)__va(read_cr3_pa()) + pgd_index(address);
- pgd_ref = pgd_offset_k(address);
- if (pgd_none(*pgd_ref))
+ pgd_k = pgd_offset_k(address);
+ if (pgd_none(*pgd_k))
return -1;

if (pgtable_l5_enabled) {
if (pgd_none(*pgd)) {
- set_pgd(pgd, *pgd_ref);
+ set_pgd(pgd, *pgd_k);
arch_flush_lazy_mmu_mode();
} else {
- BUG_ON(pgd_page_vaddr(*pgd) != pgd_page_vaddr(*pgd_ref));
+ BUG_ON(pgd_page_vaddr(*pgd) != pgd_page_vaddr(*pgd_k));
}
}

/* With 4-level paging, copying happens on the p4d level. */
p4d = p4d_offset(pgd, address);
- p4d_ref = p4d_offset(pgd_ref, address);
- if (p4d_none(*p4d_ref))
+ p4d_k = p4d_offset(pgd_k, address);
+ if (p4d_none(*p4d_k))
return -1;

if (p4d_none(*p4d) && !pgtable_l5_enabled) {
- set_p4d(p4d, *p4d_ref);
+ set_p4d(p4d, *p4d_k);
arch_flush_lazy_mmu_mode();
} else {
- BUG_ON(p4d_pfn(*p4d) != p4d_pfn(*p4d_ref));
+ BUG_ON(p4d_pfn(*p4d) != p4d_pfn(*p4d_k));
}

- /*
- * Below here mismatches are bugs because these lower tables
- * are shared:
- */
BUILD_BUG_ON(CONFIG_PGTABLE_LEVELS < 4);

pud = pud_offset(p4d, address);
- pud_ref = pud_offset(p4d_ref, address);
- if (pud_none(*pud_ref))
+ if (pud_none(*pud))
return -1;

- if (pud_none(*pud) || pud_pfn(*pud) != pud_pfn(*pud_ref))
- BUG();
-
if (pud_large(*pud))
return 0;

pmd = pmd_offset(pud, address);
- pmd_ref = pmd_offset(pud_ref, address);
- if (pmd_none(*pmd_ref))
+ if (pmd_none(*pmd))
return -1;

- if (pmd_none(*pmd) || pmd_pfn(*pmd) != pmd_pfn(*pmd_ref))
- BUG();
-
if (pmd_large(*pmd))
return 0;

- pte_ref = pte_offset_kernel(pmd_ref, address);
- if (!pte_present(*pte_ref))
- return -1;
-
pte = pte_offset_kernel(pmd, address);
-
- /*
- * Don't use pte_page here, because the mappings can point
- * outside mem_map, and the NUMA hash lookup cannot handle
- * that:
- */
- if (!pte_present(*pte) || pte_pfn(*pte) != pte_pfn(*pte_ref))
- BUG();
+ if (!pte_present(*pte))
+ return -1;

return 0;
}


Subject: [tip:x86/mm] x86/mm: Remove pointless checks in vmalloc_fault

Commit-ID: 565977a3d929fc4427769117a8ac976ec16776d5
Gitweb: https://git.kernel.org/tip/565977a3d929fc4427769117a8ac976ec16776d5
Author: Toshi Kani <[email protected]>
AuthorDate: Wed, 14 Mar 2018 14:59:32 -0600
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 15 Mar 2018 15:27:47 +0100

x86/mm: Remove pointless checks in vmalloc_fault

vmalloc_fault() sets user's pgd or p4d from the kernel page table. Once
it's set, all tables underneath are identical. There is no point of
following the same page table with two separate pointers and make sure they
see the same with BUG().

Remove the pointless checks in vmalloc_fault(). Also rename the kernel
pgd/p4d pointers to pgd_k/p4d_k so that their names are consistent in the
file.

Suggested-by: Andy Lutomirski <[email protected]>
Signed-off-by: Toshi Kani <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: Borislav Petkov <[email protected]>
Cc: Gratian Crisan <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]

---
arch/x86/mm/fault.c | 56 ++++++++++++++++-------------------------------------
1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 84d702a71afe..70c3b1c43676 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -417,11 +417,11 @@ void vmalloc_sync_all(void)
*/
static noinline int vmalloc_fault(unsigned long address)
{
- pgd_t *pgd, *pgd_ref;
- p4d_t *p4d, *p4d_ref;
- pud_t *pud, *pud_ref;
- pmd_t *pmd, *pmd_ref;
- pte_t *pte, *pte_ref;
+ pgd_t *pgd, *pgd_k;
+ p4d_t *p4d, *p4d_k;
+ pud_t *pud;
+ pmd_t *pmd;
+ pte_t *pte;

/* Make sure we are in vmalloc area: */
if (!(address >= VMALLOC_START && address < VMALLOC_END))
@@ -435,73 +435,51 @@ static noinline int vmalloc_fault(unsigned long address)
* case just flush:
*/
pgd = (pgd_t *)__va(read_cr3_pa()) + pgd_index(address);
- pgd_ref = pgd_offset_k(address);
- if (pgd_none(*pgd_ref))
+ pgd_k = pgd_offset_k(address);
+ if (pgd_none(*pgd_k))
return -1;

if (pgtable_l5_enabled) {
if (pgd_none(*pgd)) {
- set_pgd(pgd, *pgd_ref);
+ set_pgd(pgd, *pgd_k);
arch_flush_lazy_mmu_mode();
} else {
- BUG_ON(pgd_page_vaddr(*pgd) != pgd_page_vaddr(*pgd_ref));
+ BUG_ON(pgd_page_vaddr(*pgd) != pgd_page_vaddr(*pgd_k));
}
}

/* With 4-level paging, copying happens on the p4d level. */
p4d = p4d_offset(pgd, address);
- p4d_ref = p4d_offset(pgd_ref, address);
- if (p4d_none(*p4d_ref))
+ p4d_k = p4d_offset(pgd_k, address);
+ if (p4d_none(*p4d_k))
return -1;

if (p4d_none(*p4d) && !pgtable_l5_enabled) {
- set_p4d(p4d, *p4d_ref);
+ set_p4d(p4d, *p4d_k);
arch_flush_lazy_mmu_mode();
} else {
- BUG_ON(p4d_pfn(*p4d) != p4d_pfn(*p4d_ref));
+ BUG_ON(p4d_pfn(*p4d) != p4d_pfn(*p4d_k));
}

- /*
- * Below here mismatches are bugs because these lower tables
- * are shared:
- */
BUILD_BUG_ON(CONFIG_PGTABLE_LEVELS < 4);

pud = pud_offset(p4d, address);
- pud_ref = pud_offset(p4d_ref, address);
- if (pud_none(*pud_ref))
+ if (pud_none(*pud))
return -1;

- if (pud_none(*pud) || pud_pfn(*pud) != pud_pfn(*pud_ref))
- BUG();
-
if (pud_large(*pud))
return 0;

pmd = pmd_offset(pud, address);
- pmd_ref = pmd_offset(pud_ref, address);
- if (pmd_none(*pmd_ref))
+ if (pmd_none(*pmd))
return -1;

- if (pmd_none(*pmd) || pmd_pfn(*pmd) != pmd_pfn(*pmd_ref))
- BUG();
-
if (pmd_large(*pmd))
return 0;

- pte_ref = pte_offset_kernel(pmd_ref, address);
- if (!pte_present(*pte_ref))
- return -1;
-
pte = pte_offset_kernel(pmd, address);
-
- /*
- * Don't use pte_page here, because the mappings can point
- * outside mem_map, and the NUMA hash lookup cannot handle
- * that:
- */
- if (!pte_present(*pte) || pte_pfn(*pte) != pte_pfn(*pte_ref))
- BUG();
+ if (!pte_present(*pte))
+ return -1;

return 0;
}