All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Use global pages with PTI
@ 2018-02-15 13:20 ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86

The later verions of the KAISER pathces (pre-PTI) allowed the user/kernel
shared areas to be GLOBAL.  The thought was that this would reduce the
TLB overhead of keeping two copies of these mappings.

During the switch over to PTI, we seem to have lost our ability to have
GLOBAL mappings.  This adds them back.

Did we have some reason for leaving out global pages entirely?  We
_should_ have all the TLB flushing mechanics in place to handle these
already since all of our kernel mapping changes have to know how to
flush global pages regardless.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] Use global pages with PTI
@ 2018-02-15 13:20 ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86

The later verions of the KAISER pathces (pre-PTI) allowed the user/kernel
shared areas to be GLOBAL.  The thought was that this would reduce the
TLB overhead of keeping two copies of these mappings.

During the switch over to PTI, we seem to have lost our ability to have
GLOBAL mappings.  This adds them back.

Did we have some reason for leaving out global pages entirely?  We
_should_ have all the TLB flushing mechanics in place to handle these
already since all of our kernel mapping changes have to know how to
flush global pages regardless.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] x86/mm: factor out conditional pageattr PTE bit setting code
  2018-02-15 13:20 ` Dave Hansen
@ 2018-02-15 13:20   ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86


From: Dave Hansen <dave.hansen@linux.intel.com>

The pageattr code has a pattern repeated where it sets a PTE bit
for present PTEs but clears it for non-present PTEs.  This helps
to keep pte_none() from getting messed up.  _PAGE_GLOBAL is the
most frequent target of this pattern.

This pattern also includes a nice, copy-and-pasted comment.

I want to do some special stuff with _PAGE_GLOBAL in a moment,
so refactor this a _bit_ to centralize the comment and the
bit operations.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/mm/pageattr.c |   65 ++++++++++++++---------------------------------
 1 file changed, 20 insertions(+), 45 deletions(-)

diff -puN arch/x86/mm/pageattr.c~kpti-centralize-global-setting arch/x86/mm/pageattr.c
--- a/arch/x86/mm/pageattr.c~kpti-centralize-global-setting	2018-02-13 15:17:55.602210062 -0800
+++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:55.606210062 -0800
@@ -512,6 +512,22 @@ static void __set_pmd_pte(pte_t *kpte, u
 #endif
 }
 
+static pgprot_t pgprot_set_on_present(pgprot_t prot, pteval_t flags)
+{
+	/*
+	 * Set 'flags' only if PRESENT.  Ensures that we do not
+	 * set flags in an otherwise empty PTE breaking pte_none().
+	 * A later function (such as canon_pgprot()) must clear
+	 * possibly unsupported flags (like _PAGE_GLOBAL).
+	 */
+	if (pgprot_val(prot) & _PAGE_PRESENT)
+		pgprot_val(prot) |= flags;
+	else
+		pgprot_val(prot) &= ~flags;
+
+	return prot;
+}
+
 static int
 try_preserve_large_page(pte_t *kpte, unsigned long address,
 			struct cpa_data *cpa)
@@ -577,18 +593,7 @@ try_preserve_large_page(pte_t *kpte, uns
 	 * different bit positions in the two formats.
 	 */
 	req_prot = pgprot_4k_2_large(req_prot);
-
-	/*
-	 * Set the PSE and GLOBAL flags only if the PRESENT flag is
-	 * set otherwise pmd_present/pmd_huge will return true even on
-	 * a non present pmd. The canon_pgprot will clear _PAGE_GLOBAL
-	 * for the ancient hardware that doesn't support it.
-	 */
-	if (pgprot_val(req_prot) & _PAGE_PRESENT)
-		pgprot_val(req_prot) |= _PAGE_PSE | _PAGE_GLOBAL;
-	else
-		pgprot_val(req_prot) &= ~(_PAGE_PSE | _PAGE_GLOBAL);
-
+	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
 	req_prot = canon_pgprot(req_prot);
 
 	/*
@@ -698,16 +703,7 @@ __split_large_page(struct cpa_data *cpa,
 		return 1;
 	}
 
-	/*
-	 * Set the GLOBAL flags only if the PRESENT flag is set
-	 * otherwise pmd/pte_present will return true even on a non
-	 * present pmd/pte. The canon_pgprot will clear _PAGE_GLOBAL
-	 * for the ancient hardware that doesn't support it.
-	 */
-	if (pgprot_val(ref_prot) & _PAGE_PRESENT)
-		pgprot_val(ref_prot) |= _PAGE_GLOBAL;
-	else
-		pgprot_val(ref_prot) &= ~_PAGE_GLOBAL;
+	ref_prot = pgprot_set_on_present(ref_prot, _PAGE_GLOBAL);
 
 	/*
 	 * Get the target pfn from the original entry:
@@ -930,18 +926,7 @@ static void populate_pte(struct cpa_data
 
 	pte = pte_offset_kernel(pmd, start);
 
-	/*
-	 * Set the GLOBAL flags only if the PRESENT flag is
-	 * set otherwise pte_present will return true even on
-	 * a non present pte. The canon_pgprot will clear
-	 * _PAGE_GLOBAL for the ancient hardware that doesn't
-	 * support it.
-	 */
-	if (pgprot_val(pgprot) & _PAGE_PRESENT)
-		pgprot_val(pgprot) |= _PAGE_GLOBAL;
-	else
-		pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
-
+	pgprot = pgprot_set_on_present(pgprot, _PAGE_GLOBAL);
 	pgprot = canon_pgprot(pgprot);
 
 	while (num_pages-- && start < end) {
@@ -1234,17 +1219,7 @@ repeat:
 
 		new_prot = static_protections(new_prot, address, pfn);
 
-		/*
-		 * Set the GLOBAL flags only if the PRESENT flag is
-		 * set otherwise pte_present will return true even on
-		 * a non present pte. The canon_pgprot will clear
-		 * _PAGE_GLOBAL for the ancient hardware that doesn't
-		 * support it.
-		 */
-		if (pgprot_val(new_prot) & _PAGE_PRESENT)
-			pgprot_val(new_prot) |= _PAGE_GLOBAL;
-		else
-			pgprot_val(new_prot) &= ~_PAGE_GLOBAL;
+		new_prot = pgprot_set_on_present(new_prot, _PAGE_GLOBAL);
 
 		/*
 		 * We need to keep the pfn from the existing PTE,
_

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] x86/mm: factor out conditional pageattr PTE bit setting code
@ 2018-02-15 13:20   ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86


From: Dave Hansen <dave.hansen@linux.intel.com>

The pageattr code has a pattern repeated where it sets a PTE bit
for present PTEs but clears it for non-present PTEs.  This helps
to keep pte_none() from getting messed up.  _PAGE_GLOBAL is the
most frequent target of this pattern.

This pattern also includes a nice, copy-and-pasted comment.

I want to do some special stuff with _PAGE_GLOBAL in a moment,
so refactor this a _bit_ to centralize the comment and the
bit operations.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/mm/pageattr.c |   65 ++++++++++++++---------------------------------
 1 file changed, 20 insertions(+), 45 deletions(-)

diff -puN arch/x86/mm/pageattr.c~kpti-centralize-global-setting arch/x86/mm/pageattr.c
--- a/arch/x86/mm/pageattr.c~kpti-centralize-global-setting	2018-02-13 15:17:55.602210062 -0800
+++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:55.606210062 -0800
@@ -512,6 +512,22 @@ static void __set_pmd_pte(pte_t *kpte, u
 #endif
 }
 
+static pgprot_t pgprot_set_on_present(pgprot_t prot, pteval_t flags)
+{
+	/*
+	 * Set 'flags' only if PRESENT.  Ensures that we do not
+	 * set flags in an otherwise empty PTE breaking pte_none().
+	 * A later function (such as canon_pgprot()) must clear
+	 * possibly unsupported flags (like _PAGE_GLOBAL).
+	 */
+	if (pgprot_val(prot) & _PAGE_PRESENT)
+		pgprot_val(prot) |= flags;
+	else
+		pgprot_val(prot) &= ~flags;
+
+	return prot;
+}
+
 static int
 try_preserve_large_page(pte_t *kpte, unsigned long address,
 			struct cpa_data *cpa)
@@ -577,18 +593,7 @@ try_preserve_large_page(pte_t *kpte, uns
 	 * different bit positions in the two formats.
 	 */
 	req_prot = pgprot_4k_2_large(req_prot);
-
-	/*
-	 * Set the PSE and GLOBAL flags only if the PRESENT flag is
-	 * set otherwise pmd_present/pmd_huge will return true even on
-	 * a non present pmd. The canon_pgprot will clear _PAGE_GLOBAL
-	 * for the ancient hardware that doesn't support it.
-	 */
-	if (pgprot_val(req_prot) & _PAGE_PRESENT)
-		pgprot_val(req_prot) |= _PAGE_PSE | _PAGE_GLOBAL;
-	else
-		pgprot_val(req_prot) &= ~(_PAGE_PSE | _PAGE_GLOBAL);
-
+	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
 	req_prot = canon_pgprot(req_prot);
 
 	/*
@@ -698,16 +703,7 @@ __split_large_page(struct cpa_data *cpa,
 		return 1;
 	}
 
-	/*
-	 * Set the GLOBAL flags only if the PRESENT flag is set
-	 * otherwise pmd/pte_present will return true even on a non
-	 * present pmd/pte. The canon_pgprot will clear _PAGE_GLOBAL
-	 * for the ancient hardware that doesn't support it.
-	 */
-	if (pgprot_val(ref_prot) & _PAGE_PRESENT)
-		pgprot_val(ref_prot) |= _PAGE_GLOBAL;
-	else
-		pgprot_val(ref_prot) &= ~_PAGE_GLOBAL;
+	ref_prot = pgprot_set_on_present(ref_prot, _PAGE_GLOBAL);
 
 	/*
 	 * Get the target pfn from the original entry:
@@ -930,18 +926,7 @@ static void populate_pte(struct cpa_data
 
 	pte = pte_offset_kernel(pmd, start);
 
-	/*
-	 * Set the GLOBAL flags only if the PRESENT flag is
-	 * set otherwise pte_present will return true even on
-	 * a non present pte. The canon_pgprot will clear
-	 * _PAGE_GLOBAL for the ancient hardware that doesn't
-	 * support it.
-	 */
-	if (pgprot_val(pgprot) & _PAGE_PRESENT)
-		pgprot_val(pgprot) |= _PAGE_GLOBAL;
-	else
-		pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
-
+	pgprot = pgprot_set_on_present(pgprot, _PAGE_GLOBAL);
 	pgprot = canon_pgprot(pgprot);
 
 	while (num_pages-- && start < end) {
@@ -1234,17 +1219,7 @@ repeat:
 
 		new_prot = static_protections(new_prot, address, pfn);
 
-		/*
-		 * Set the GLOBAL flags only if the PRESENT flag is
-		 * set otherwise pte_present will return true even on
-		 * a non present pte. The canon_pgprot will clear
-		 * _PAGE_GLOBAL for the ancient hardware that doesn't
-		 * support it.
-		 */
-		if (pgprot_val(new_prot) & _PAGE_PRESENT)
-			pgprot_val(new_prot) |= _PAGE_GLOBAL;
-		else
-			pgprot_val(new_prot) &= ~_PAGE_GLOBAL;
+		new_prot = pgprot_set_on_present(new_prot, _PAGE_GLOBAL);
 
 		/*
 		 * We need to keep the pfn from the existing PTE,
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-15 13:20 ` Dave Hansen
@ 2018-02-15 13:20   ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86


From: Dave Hansen <dave.hansen@linux.intel.com>

Kernel mappings are historically _PAGE_GLOBAL.  But, with PTI, we do not
want them to be _PAGE_GLOBAL.  We currently accomplish this by simply
clearing _PAGE_GLOBAL from the suppotred mask which ensures it is
cleansed from many of our PTE construction sites:

        if (!static_cpu_has(X86_FEATURE_PTI))
	                __supported_pte_mask |= _PAGE_GLOBAL;

But, this also means that we now get *no* opportunity to use global
pages with PTI, even for data which is shared such as the cpu_entry_area
and entry/exit text.

This patch introduces a new mask: __PAGE_KERNEL_GLOBAL.  This mask
can be thought of as the default global bit value when creating kernel
mappings.  We make it _PAGE_GLOBAL when PTI=n, but 0 when PTI=y.  This
ensures that on PTI kernels, all of the __PAGE_KERNEL_* users will not
get _PAGE_GLOBAL.

This also restores _PAGE_GLOBAL to __supported_pte_mask, allowing it
to be set in the first place.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/include/asm/pgtable_types.h |    9 ++++++++-
 b/arch/x86/mm/init.c                   |    8 +-------
 b/arch/x86/mm/pageattr.c               |    9 +++++----
 3 files changed, 14 insertions(+), 12 deletions(-)

diff -puN arch/x86/include/asm/pgtable_types.h~kpti-no-global-for-kernel-mappings arch/x86/include/asm/pgtable_types.h
--- a/arch/x86/include/asm/pgtable_types.h~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.144210060 -0800
+++ b/arch/x86/include/asm/pgtable_types.h	2018-02-13 15:17:56.152210060 -0800
@@ -180,8 +180,15 @@ enum page_cache_mode {
 #define PAGE_READONLY_EXEC	__pgprot(_PAGE_PRESENT | _PAGE_USER |	\
 					 _PAGE_ACCESSED)
 
+#ifdef CONFIG_PAGE_TABLE_ISOLATION
+#define __PAGE_KERNEL_GLOBAL		0
+#else
+#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
+#endif
+
 #define __PAGE_KERNEL_EXEC						\
-	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_GLOBAL)
+	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | 	\
+	 __PAGE_KERNEL_GLOBAL)
 #define __PAGE_KERNEL		(__PAGE_KERNEL_EXEC | _PAGE_NX)
 
 #define __PAGE_KERNEL_RO		(__PAGE_KERNEL & ~_PAGE_RW)
diff -puN arch/x86/mm/init.c~kpti-no-global-for-kernel-mappings arch/x86/mm/init.c
--- a/arch/x86/mm/init.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.146210060 -0800
+++ b/arch/x86/mm/init.c	2018-02-13 15:17:56.152210060 -0800
@@ -162,12 +162,6 @@ struct map_range {
 
 static int page_size_mask;
 
-static void enable_global_pages(void)
-{
-	if (!static_cpu_has(X86_FEATURE_PTI))
-		__supported_pte_mask |= _PAGE_GLOBAL;
-}
-
 static void __init probe_page_size_mask(void)
 {
 	/*
@@ -189,7 +183,7 @@ static void __init probe_page_size_mask(
 	__supported_pte_mask &= ~_PAGE_GLOBAL;
 	if (boot_cpu_has(X86_FEATURE_PGE)) {
 		cr4_set_bits_and_update_boot(X86_CR4_PGE);
-		enable_global_pages();
+		__supported_pte_mask |= _PAGE_GLOBAL;
 	}
 
 	/* Enable 1 GB linear kernel mappings if available: */
diff -puN arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings arch/x86/mm/pageattr.c
--- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
+++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
@@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
 	 * different bit positions in the two formats.
 	 */
 	req_prot = pgprot_4k_2_large(req_prot);
-	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
+	req_prot = pgprot_set_on_present(req_prot,
+			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
 	req_prot = canon_pgprot(req_prot);
 
 	/*
@@ -703,7 +704,7 @@ __split_large_page(struct cpa_data *cpa,
 		return 1;
 	}
 
-	ref_prot = pgprot_set_on_present(ref_prot, _PAGE_GLOBAL);
+	ref_prot = pgprot_set_on_present(ref_prot, __PAGE_KERNEL_GLOBAL);
 
 	/*
 	 * Get the target pfn from the original entry:
@@ -926,7 +927,7 @@ static void populate_pte(struct cpa_data
 
 	pte = pte_offset_kernel(pmd, start);
 
-	pgprot = pgprot_set_on_present(pgprot, _PAGE_GLOBAL);
+	pgprot = pgprot_set_on_present(pgprot, __PAGE_KERNEL_GLOBAL);
 	pgprot = canon_pgprot(pgprot);
 
 	while (num_pages-- && start < end) {
@@ -1219,7 +1220,7 @@ repeat:
 
 		new_prot = static_protections(new_prot, address, pfn);
 
-		new_prot = pgprot_set_on_present(new_prot, _PAGE_GLOBAL);
+		new_prot = pgprot_set_on_present(new_prot, __PAGE_KERNEL_GLOBAL);
 
 		/*
 		 * We need to keep the pfn from the existing PTE,
_

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-15 13:20   ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86


From: Dave Hansen <dave.hansen@linux.intel.com>

Kernel mappings are historically _PAGE_GLOBAL.  But, with PTI, we do not
want them to be _PAGE_GLOBAL.  We currently accomplish this by simply
clearing _PAGE_GLOBAL from the suppotred mask which ensures it is
cleansed from many of our PTE construction sites:

        if (!static_cpu_has(X86_FEATURE_PTI))
	                __supported_pte_mask |= _PAGE_GLOBAL;

But, this also means that we now get *no* opportunity to use global
pages with PTI, even for data which is shared such as the cpu_entry_area
and entry/exit text.

This patch introduces a new mask: __PAGE_KERNEL_GLOBAL.  This mask
can be thought of as the default global bit value when creating kernel
mappings.  We make it _PAGE_GLOBAL when PTI=n, but 0 when PTI=y.  This
ensures that on PTI kernels, all of the __PAGE_KERNEL_* users will not
get _PAGE_GLOBAL.

This also restores _PAGE_GLOBAL to __supported_pte_mask, allowing it
to be set in the first place.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/include/asm/pgtable_types.h |    9 ++++++++-
 b/arch/x86/mm/init.c                   |    8 +-------
 b/arch/x86/mm/pageattr.c               |    9 +++++----
 3 files changed, 14 insertions(+), 12 deletions(-)

diff -puN arch/x86/include/asm/pgtable_types.h~kpti-no-global-for-kernel-mappings arch/x86/include/asm/pgtable_types.h
--- a/arch/x86/include/asm/pgtable_types.h~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.144210060 -0800
+++ b/arch/x86/include/asm/pgtable_types.h	2018-02-13 15:17:56.152210060 -0800
@@ -180,8 +180,15 @@ enum page_cache_mode {
 #define PAGE_READONLY_EXEC	__pgprot(_PAGE_PRESENT | _PAGE_USER |	\
 					 _PAGE_ACCESSED)
 
+#ifdef CONFIG_PAGE_TABLE_ISOLATION
+#define __PAGE_KERNEL_GLOBAL		0
+#else
+#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
+#endif
+
 #define __PAGE_KERNEL_EXEC						\
-	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_GLOBAL)
+	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | 	\
+	 __PAGE_KERNEL_GLOBAL)
 #define __PAGE_KERNEL		(__PAGE_KERNEL_EXEC | _PAGE_NX)
 
 #define __PAGE_KERNEL_RO		(__PAGE_KERNEL & ~_PAGE_RW)
diff -puN arch/x86/mm/init.c~kpti-no-global-for-kernel-mappings arch/x86/mm/init.c
--- a/arch/x86/mm/init.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.146210060 -0800
+++ b/arch/x86/mm/init.c	2018-02-13 15:17:56.152210060 -0800
@@ -162,12 +162,6 @@ struct map_range {
 
 static int page_size_mask;
 
-static void enable_global_pages(void)
-{
-	if (!static_cpu_has(X86_FEATURE_PTI))
-		__supported_pte_mask |= _PAGE_GLOBAL;
-}
-
 static void __init probe_page_size_mask(void)
 {
 	/*
@@ -189,7 +183,7 @@ static void __init probe_page_size_mask(
 	__supported_pte_mask &= ~_PAGE_GLOBAL;
 	if (boot_cpu_has(X86_FEATURE_PGE)) {
 		cr4_set_bits_and_update_boot(X86_CR4_PGE);
-		enable_global_pages();
+		__supported_pte_mask |= _PAGE_GLOBAL;
 	}
 
 	/* Enable 1 GB linear kernel mappings if available: */
diff -puN arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings arch/x86/mm/pageattr.c
--- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
+++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
@@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
 	 * different bit positions in the two formats.
 	 */
 	req_prot = pgprot_4k_2_large(req_prot);
-	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
+	req_prot = pgprot_set_on_present(req_prot,
+			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
 	req_prot = canon_pgprot(req_prot);
 
 	/*
@@ -703,7 +704,7 @@ __split_large_page(struct cpa_data *cpa,
 		return 1;
 	}
 
-	ref_prot = pgprot_set_on_present(ref_prot, _PAGE_GLOBAL);
+	ref_prot = pgprot_set_on_present(ref_prot, __PAGE_KERNEL_GLOBAL);
 
 	/*
 	 * Get the target pfn from the original entry:
@@ -926,7 +927,7 @@ static void populate_pte(struct cpa_data
 
 	pte = pte_offset_kernel(pmd, start);
 
-	pgprot = pgprot_set_on_present(pgprot, _PAGE_GLOBAL);
+	pgprot = pgprot_set_on_present(pgprot, __PAGE_KERNEL_GLOBAL);
 	pgprot = canon_pgprot(pgprot);
 
 	while (num_pages-- && start < end) {
@@ -1219,7 +1220,7 @@ repeat:
 
 		new_prot = static_protections(new_prot, address, pfn);
 
-		new_prot = pgprot_set_on_present(new_prot, _PAGE_GLOBAL);
+		new_prot = pgprot_set_on_present(new_prot, __PAGE_KERNEL_GLOBAL);
 
 		/*
 		 * We need to keep the pfn from the existing PTE,
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] x86/pti: enable global pages for shared areas
  2018-02-15 13:20 ` Dave Hansen
@ 2018-02-15 13:20   ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86


From: Dave Hansen <dave.hansen@linux.intel.com>

The entry/exit text and cpu_entry_area are mapped into userspace and
the kernel.  But, they are not _PAGE_GLOBAL.  This creates unnecessary
TLB misses.

Add the _PAGE_GLOBAL flag for these areas.

After this patch, we can see that "GLB" shows up in each copy of the
page tables, that we have the same number of global entries in each
and that they are the *same* entries.

# grep -c GLB /sys/kernel/debug/page_tables/*
/sys/kernel/debug/page_tables/current_kernel:11
/sys/kernel/debug/page_tables/current_user:11
/sys/kernel/debug/page_tables/kernel:11

# for f in `ls /sys/kernel/debug/page_tables/`; do grep GLB /sys/kernel/debug/page_tables/$f > $f.GLB; done
# md5sum *.GLB
9caae8ad6a1fb53aca2407ec037f612d  current_kernel.GLB
9caae8ad6a1fb53aca2407ec037f612d  current_user.GLB
9caae8ad6a1fb53aca2407ec037f612d  kernel.GLB

A quick visual audit also shows that all the entries make sense.
0xfffffe0000000000 is the cpu_entry_area and 0xffffffff81c00000
is the entry/exit text:

# grep -c GLB /sys/kernel/debug/page_tables/current_user
0xfffffe0000000000-0xfffffe0000002000           8K     ro                 GLB NX pte
0xfffffe0000002000-0xfffffe0000003000           4K     RW                 GLB NX pte
0xfffffe0000003000-0xfffffe0000006000          12K     ro                 GLB NX pte
0xfffffe0000006000-0xfffffe0000007000           4K     ro                 GLB x  pte
0xfffffe0000007000-0xfffffe000000d000          24K     RW                 GLB NX pte
0xfffffe000002d000-0xfffffe000002e000           4K     ro                 GLB NX pte
0xfffffe000002e000-0xfffffe000002f000           4K     RW                 GLB NX pte
0xfffffe000002f000-0xfffffe0000032000          12K     ro                 GLB NX pte
0xfffffe0000032000-0xfffffe0000033000           4K     ro                 GLB x  pte
0xfffffe0000033000-0xfffffe0000039000          24K     RW                 GLB NX pte
0xffffffff81c00000-0xffffffff81e00000           2M     ro         PSE     GLB x  pmd

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/mm/cpu_entry_area.c |    7 +++++++
 b/arch/x86/mm/pti.c            |    9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff -puN arch/x86/mm/cpu_entry_area.c~kpti-why-no-global arch/x86/mm/cpu_entry_area.c
--- a/arch/x86/mm/cpu_entry_area.c~kpti-why-no-global	2018-02-13 15:17:56.735210059 -0800
+++ b/arch/x86/mm/cpu_entry_area.c	2018-02-13 15:17:56.740210059 -0800
@@ -28,6 +28,13 @@ void cea_set_pte(void *cea_vaddr, phys_a
 {
 	unsigned long va = (unsigned long) cea_vaddr;
 
+	/*
+	 * The cpu_entry_area is shared between the user and kernel
+	 * page tables.  All of its ptes can safely be global.
+	 */
+	if (boot_cpu_has(X86_FEATURE_PGE))
+		pgprot_val(flags) |= _PAGE_GLOBAL;
+
 	set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
 }
 
diff -puN arch/x86/mm/pti.c~kpti-why-no-global arch/x86/mm/pti.c
--- a/arch/x86/mm/pti.c~kpti-why-no-global	2018-02-13 15:17:56.737210059 -0800
+++ b/arch/x86/mm/pti.c	2018-02-13 15:17:56.740210059 -0800
@@ -300,6 +300,13 @@ pti_clone_pmds(unsigned long start, unsi
 			return;
 
 		/*
+		 * Setting 'target_pmd' below creates a mapping in both
+		 * the user and kernel page tables.  It is effectively
+		 * global, so set it as global in both copies.
+		 */
+		*pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
+
+		/*
 		 * Copy the PMD.  That is, the kernelmode and usermode
 		 * tables will share the last-level page tables of this
 		 * address range
@@ -348,7 +355,7 @@ static void __init pti_clone_entry_text(
 {
 	pti_clone_pmds((unsigned long) __entry_text_start,
 			(unsigned long) __irqentry_text_end,
-		       _PAGE_RW | _PAGE_GLOBAL);
+		       _PAGE_RW);
 }
 
 /*
_

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] x86/pti: enable global pages for shared areas
@ 2018-02-15 13:20   ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Dave Hansen, luto, torvalds, keescook, hughd, jgross, x86


From: Dave Hansen <dave.hansen@linux.intel.com>

The entry/exit text and cpu_entry_area are mapped into userspace and
the kernel.  But, they are not _PAGE_GLOBAL.  This creates unnecessary
TLB misses.

Add the _PAGE_GLOBAL flag for these areas.

After this patch, we can see that "GLB" shows up in each copy of the
page tables, that we have the same number of global entries in each
and that they are the *same* entries.

# grep -c GLB /sys/kernel/debug/page_tables/*
/sys/kernel/debug/page_tables/current_kernel:11
/sys/kernel/debug/page_tables/current_user:11
/sys/kernel/debug/page_tables/kernel:11

# for f in `ls /sys/kernel/debug/page_tables/`; do grep GLB /sys/kernel/debug/page_tables/$f > $f.GLB; done
# md5sum *.GLB
9caae8ad6a1fb53aca2407ec037f612d  current_kernel.GLB
9caae8ad6a1fb53aca2407ec037f612d  current_user.GLB
9caae8ad6a1fb53aca2407ec037f612d  kernel.GLB

A quick visual audit also shows that all the entries make sense.
0xfffffe0000000000 is the cpu_entry_area and 0xffffffff81c00000
is the entry/exit text:

# grep -c GLB /sys/kernel/debug/page_tables/current_user
0xfffffe0000000000-0xfffffe0000002000           8K     ro                 GLB NX pte
0xfffffe0000002000-0xfffffe0000003000           4K     RW                 GLB NX pte
0xfffffe0000003000-0xfffffe0000006000          12K     ro                 GLB NX pte
0xfffffe0000006000-0xfffffe0000007000           4K     ro                 GLB x  pte
0xfffffe0000007000-0xfffffe000000d000          24K     RW                 GLB NX pte
0xfffffe000002d000-0xfffffe000002e000           4K     ro                 GLB NX pte
0xfffffe000002e000-0xfffffe000002f000           4K     RW                 GLB NX pte
0xfffffe000002f000-0xfffffe0000032000          12K     ro                 GLB NX pte
0xfffffe0000032000-0xfffffe0000033000           4K     ro                 GLB x  pte
0xfffffe0000033000-0xfffffe0000039000          24K     RW                 GLB NX pte
0xffffffff81c00000-0xffffffff81e00000           2M     ro         PSE     GLB x  pmd

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/mm/cpu_entry_area.c |    7 +++++++
 b/arch/x86/mm/pti.c            |    9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff -puN arch/x86/mm/cpu_entry_area.c~kpti-why-no-global arch/x86/mm/cpu_entry_area.c
--- a/arch/x86/mm/cpu_entry_area.c~kpti-why-no-global	2018-02-13 15:17:56.735210059 -0800
+++ b/arch/x86/mm/cpu_entry_area.c	2018-02-13 15:17:56.740210059 -0800
@@ -28,6 +28,13 @@ void cea_set_pte(void *cea_vaddr, phys_a
 {
 	unsigned long va = (unsigned long) cea_vaddr;
 
+	/*
+	 * The cpu_entry_area is shared between the user and kernel
+	 * page tables.  All of its ptes can safely be global.
+	 */
+	if (boot_cpu_has(X86_FEATURE_PGE))
+		pgprot_val(flags) |= _PAGE_GLOBAL;
+
 	set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
 }
 
diff -puN arch/x86/mm/pti.c~kpti-why-no-global arch/x86/mm/pti.c
--- a/arch/x86/mm/pti.c~kpti-why-no-global	2018-02-13 15:17:56.737210059 -0800
+++ b/arch/x86/mm/pti.c	2018-02-13 15:17:56.740210059 -0800
@@ -300,6 +300,13 @@ pti_clone_pmds(unsigned long start, unsi
 			return;
 
 		/*
+		 * Setting 'target_pmd' below creates a mapping in both
+		 * the user and kernel page tables.  It is effectively
+		 * global, so set it as global in both copies.
+		 */
+		*pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
+
+		/*
 		 * Copy the PMD.  That is, the kernelmode and usermode
 		 * tables will share the last-level page tables of this
 		 * address range
@@ -348,7 +355,7 @@ static void __init pti_clone_entry_text(
 {
 	pti_clone_pmds((unsigned long) __entry_text_start,
 			(unsigned long) __irqentry_text_end,
-		       _PAGE_RW | _PAGE_GLOBAL);
+		       _PAGE_RW);
 }
 
 /*
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/3] Use global pages with PTI
  2018-02-15 13:20 ` Dave Hansen
@ 2018-02-15 17:47   ` Linus Torvalds
  -1 siblings, 0 replies; 29+ messages in thread
From: Linus Torvalds @ 2018-02-15 17:47 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Linux Kernel Mailing List, linux-mm, Andrew Lutomirski,
	Kees Cook, Hugh Dickins, Jürgen Groß,
	the arch/x86 maintainers

On Thu, Feb 15, 2018 at 5:20 AM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
>
> During the switch over to PTI, we seem to have lost our ability to have
> GLOBAL mappings.

Oops. Odd, I have this distinct memory of somebody even _testing_ the
global bit performance when I pointed out that we shouldn't just make
the bit go away entirely.

[ goes back and looks at archives ]

Oh, that was in fact you who did that performance test.

Heh. Anyway, back then you claimed a noticeable improvement on that
will-it-scale test (although a bigger one when pcid wasn't available),
so yes, if we lost the "global pages for the shared user/kernel
mapping" bit we should definitely get this fixed.

Did you perhaps re-run any benchmark numbers just to verify? Because
it's always good to back up patches that should improve performance
with actual numbers..

           Linus

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/3] Use global pages with PTI
@ 2018-02-15 17:47   ` Linus Torvalds
  0 siblings, 0 replies; 29+ messages in thread
From: Linus Torvalds @ 2018-02-15 17:47 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Linux Kernel Mailing List, linux-mm, Andrew Lutomirski,
	Kees Cook, Hugh Dickins, Jürgen Groß,
	the arch/x86 maintainers

On Thu, Feb 15, 2018 at 5:20 AM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
>
> During the switch over to PTI, we seem to have lost our ability to have
> GLOBAL mappings.

Oops. Odd, I have this distinct memory of somebody even _testing_ the
global bit performance when I pointed out that we shouldn't just make
the bit go away entirely.

[ goes back and looks at archives ]

Oh, that was in fact you who did that performance test.

Heh. Anyway, back then you claimed a noticeable improvement on that
will-it-scale test (although a bigger one when pcid wasn't available),
so yes, if we lost the "global pages for the shared user/kernel
mapping" bit we should definitely get this fixed.

Did you perhaps re-run any benchmark numbers just to verify? Because
it's always good to back up patches that should improve performance
with actual numbers..

           Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/3] Use global pages with PTI
  2018-02-15 17:47   ` Linus Torvalds
@ 2018-02-15 18:13     ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 18:13 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, linux-mm, Andrew Lutomirski,
	Kees Cook, Hugh Dickins, Jürgen Groß,
	the arch/x86 maintainers

On 02/15/2018 09:47 AM, Linus Torvalds wrote:
> On Thu, Feb 15, 2018 at 5:20 AM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>>
>> During the switch over to PTI, we seem to have lost our ability to have
>> GLOBAL mappings.
> 
> Oops. Odd, I have this distinct memory of somebody even _testing_ the
> global bit performance when I pointed out that we shouldn't just make
> the bit go away entirely.
> 
> [ goes back and looks at archives ]
> 
> Oh, that was in fact you who did that performance test.
...
> Did you perhaps re-run any benchmark numbers just to verify? Because
> it's always good to back up patches that should improve performance
> with actual numbers..

Nope, haven't done it yet, but I will.

I wanted to double-check that there was not a reason for doing the
global disabling other than the K8 TLB mismatch issues that Thomas fixed
a few weeks ago:

> commit 52994c256df36fda9a715697431cba9daecb6b11
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jan 3 15:57:59 2018 +0100
> 
>     x86/pti: Make sure the user/kernel PTEs match
>     
>     Meelis reported that his K8 Athlon64 emits MCE warnings when PTI is
>     enabled:
>     
>     [Hardware Error]: Error Addr: 0x0000ffff81e000e0
>     [Hardware Error]: MC1 Error: L1 TLB multimatch.
>     [Hardware Error]: cache level: L1, tx: INSN

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/3] Use global pages with PTI
@ 2018-02-15 18:13     ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 18:13 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, linux-mm, Andrew Lutomirski,
	Kees Cook, Hugh Dickins, Jürgen Groß,
	the arch/x86 maintainers

On 02/15/2018 09:47 AM, Linus Torvalds wrote:
> On Thu, Feb 15, 2018 at 5:20 AM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>>
>> During the switch over to PTI, we seem to have lost our ability to have
>> GLOBAL mappings.
> 
> Oops. Odd, I have this distinct memory of somebody even _testing_ the
> global bit performance when I pointed out that we shouldn't just make
> the bit go away entirely.
> 
> [ goes back and looks at archives ]
> 
> Oh, that was in fact you who did that performance test.
...
> Did you perhaps re-run any benchmark numbers just to verify? Because
> it's always good to back up patches that should improve performance
> with actual numbers..

Nope, haven't done it yet, but I will.

I wanted to double-check that there was not a reason for doing the
global disabling other than the K8 TLB mismatch issues that Thomas fixed
a few weeks ago:

> commit 52994c256df36fda9a715697431cba9daecb6b11
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jan 3 15:57:59 2018 +0100
> 
>     x86/pti: Make sure the user/kernel PTEs match
>     
>     Meelis reported that his K8 Athlon64 emits MCE warnings when PTI is
>     enabled:
>     
>     [Hardware Error]: Error Addr: 0x0000ffff81e000e0
>     [Hardware Error]: MC1 Error: L1 TLB multimatch.
>     [Hardware Error]: cache level: L1, tx: INSN

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/3] Use global pages with PTI
  2018-02-15 17:47   ` Linus Torvalds
@ 2018-02-15 23:55     ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 23:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, linux-mm, Andrew Lutomirski,
	Kees Cook, Hugh Dickins, Jürgen Groß,
	the arch/x86 maintainers

On 02/15/2018 09:47 AM, Linus Torvalds wrote:
> On Thu, Feb 15, 2018 at 5:20 AM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>> During the switch over to PTI, we seem to have lost our ability to have
>> GLOBAL mappings.
...
> Did you perhaps re-run any benchmark numbers just to verify? Because
> it's always good to back up patches that should improve performance
> with actual numbers..

Same test as last time except I'm using all 4 cores on a Skylake desktop
instead of just 1.  The test is this:

> https://github.com/antonblanchard/will-it-scale/blob/master/tests/lseek1.c

With PCIDs, lseek()s/second go up around 2% to 3% with the these patches
enabling the global bit (it's noisy).  I measured it at 3% before, so
definitely the same ballpark.  That was also before all of Andy's
trampoline stuff and the syscall fast path removal.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/3] Use global pages with PTI
@ 2018-02-15 23:55     ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-15 23:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, linux-mm, Andrew Lutomirski,
	Kees Cook, Hugh Dickins, Jürgen Groß,
	the arch/x86 maintainers

On 02/15/2018 09:47 AM, Linus Torvalds wrote:
> On Thu, Feb 15, 2018 at 5:20 AM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>> During the switch over to PTI, we seem to have lost our ability to have
>> GLOBAL mappings.
...
> Did you perhaps re-run any benchmark numbers just to verify? Because
> it's always good to back up patches that should improve performance
> with actual numbers..

Same test as last time except I'm using all 4 cores on a Skylake desktop
instead of just 1.  The test is this:

> https://github.com/antonblanchard/will-it-scale/blob/master/tests/lseek1.c

With PCIDs, lseek()s/second go up around 2% to 3% with the these patches
enabling the global bit (it's noisy).  I measured it at 3% before, so
definitely the same ballpark.  That was also before all of Andy's
trampoline stuff and the syscall fast path removal.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-15 13:20   ` Dave Hansen
@ 2018-02-16 17:47     ` Nadav Amit
  -1 siblings, 0 replies; 29+ messages in thread
From: Nadav Amit @ 2018-02-16 17:47 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> Kernel mappings are historically _PAGE_GLOBAL.  But, with PTI, we do not
> want them to be _PAGE_GLOBAL.  We currently accomplish this by simply
> clearing _PAGE_GLOBAL from the suppotred mask which ensures it is
> cleansed from many of our PTE construction sites:
> 
>        if (!static_cpu_has(X86_FEATURE_PTI))
> 	                __supported_pte_mask |= _PAGE_GLOBAL;
> 
> But, this also means that we now get *no* opportunity to use global
> pages with PTI, even for data which is shared such as the cpu_entry_area
> and entry/exit text.


Doesn’t this patch change the kernel behavior when the “nopti” parameter is used?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 17:47     ` Nadav Amit
  0 siblings, 0 replies; 29+ messages in thread
From: Nadav Amit @ 2018-02-16 17:47 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> Kernel mappings are historically _PAGE_GLOBAL.  But, with PTI, we do not
> want them to be _PAGE_GLOBAL.  We currently accomplish this by simply
> clearing _PAGE_GLOBAL from the suppotred mask which ensures it is
> cleansed from many of our PTE construction sites:
> 
>        if (!static_cpu_has(X86_FEATURE_PTI))
> 	                __supported_pte_mask |= _PAGE_GLOBAL;
> 
> But, this also means that we now get *no* opportunity to use global
> pages with PTI, even for data which is shared such as the cpu_entry_area
> and entry/exit text.


Doesn’t this patch change the kernel behavior when the “nopti” parameter is used?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-16 17:47     ` Nadav Amit
@ 2018-02-16 18:03       ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 18:03 UTC (permalink / raw)
  To: Nadav Amit
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

On 02/16/2018 09:47 AM, Nadav Amit wrote:
>> But, this also means that we now get *no* opportunity to use
>> global pages with PTI, even for data which is shared such as the
>> cpu_entry_area and entry/exit text.
> 
> Doesn’t this patch change the kernel behavior when the “nopti”
> parameter is used?

I don't think so.  It takes the "nopti" behavior and effectively makes
it apply everywhere.  So it changes the PTI behavior, not the "nopti"
behavior.

Maybe it would help to quote the code that you think does this instead
of the description. :)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 18:03       ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 18:03 UTC (permalink / raw)
  To: Nadav Amit
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

On 02/16/2018 09:47 AM, Nadav Amit wrote:
>> But, this also means that we now get *no* opportunity to use
>> global pages with PTI, even for data which is shared such as the
>> cpu_entry_area and entry/exit text.
> 
> Doesna??t this patch change the kernel behavior when the a??noptia??
> parameter is used?

I don't think so.  It takes the "nopti" behavior and effectively makes
it apply everywhere.  So it changes the PTI behavior, not the "nopti"
behavior.

Maybe it would help to quote the code that you think does this instead
of the description. :)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-16 18:03       ` Dave Hansen
@ 2018-02-16 18:25         ` Nadav Amit
  -1 siblings, 0 replies; 29+ messages in thread
From: Nadav Amit @ 2018-02-16 18:25 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 02/16/2018 09:47 AM, Nadav Amit wrote:
>>> But, this also means that we now get *no* opportunity to use
>>> global pages with PTI, even for data which is shared such as the
>>> cpu_entry_area and entry/exit text.
>> 
>> Doesn’t this patch change the kernel behavior when the “nopti”
>> parameter is used?
> 
> I don't think so.  It takes the "nopti" behavior and effectively makes
> it apply everywhere.  So it changes the PTI behavior, not the "nopti"
> behavior.
> 
> Maybe it would help to quote the code that you think does this instead
> of the description. :)

Sorry, I thought it is obvious - so I must be missing something.

> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
> +#define __PAGE_KERNEL_GLOBAL		0
> +#else
> +#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
> +#endif
...
> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
> 	 * different bit positions in the two formats.
> 	 */
> 	req_prot = pgprot_4k_2_large(req_prot);
> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
> +	req_prot = pgprot_set_on_present(req_prot,
> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
> 	req_prot = canon_pgprot(req_prot);

>From these chunks, it seems to me as req_prot will not have the global bit
on when “nopti” parameter is provided. What am I missing?
 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 18:25         ` Nadav Amit
  0 siblings, 0 replies; 29+ messages in thread
From: Nadav Amit @ 2018-02-16 18:25 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 02/16/2018 09:47 AM, Nadav Amit wrote:
>>> But, this also means that we now get *no* opportunity to use
>>> global pages with PTI, even for data which is shared such as the
>>> cpu_entry_area and entry/exit text.
>> 
>> Doesn’t this patch change the kernel behavior when the “nopti”
>> parameter is used?
> 
> I don't think so.  It takes the "nopti" behavior and effectively makes
> it apply everywhere.  So it changes the PTI behavior, not the "nopti"
> behavior.
> 
> Maybe it would help to quote the code that you think does this instead
> of the description. :)

Sorry, I thought it is obvious - so I must be missing something.

> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
> +#define __PAGE_KERNEL_GLOBAL		0
> +#else
> +#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
> +#endif
...
> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
> 	 * different bit positions in the two formats.
> 	 */
> 	req_prot = pgprot_4k_2_large(req_prot);
> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
> +	req_prot = pgprot_set_on_present(req_prot,
> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
> 	req_prot = canon_pgprot(req_prot);

From these chunks, it seems to me as req_prot will not have the global bit
on when “nopti” parameter is provided. What am I missing?
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-16 18:25         ` Nadav Amit
@ 2018-02-16 19:06           ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 19:06 UTC (permalink / raw)
  To: Nadav Amit
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

On 02/16/2018 10:25 AM, Nadav Amit wrote:
>> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
>> +#define __PAGE_KERNEL_GLOBAL		0
>> +#else
>> +#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
>> +#endif
> ...
>> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
>> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
>> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
>> 	 * different bit positions in the two formats.
>> 	 */
>> 	req_prot = pgprot_4k_2_large(req_prot);
>> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
>> +	req_prot = pgprot_set_on_present(req_prot,
>> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
>> 	req_prot = canon_pgprot(req_prot);
> From these chunks, it seems to me as req_prot will not have the global bit
> on when “nopti” parameter is provided. What am I missing?

That's a good point.  The current patch does not allow the use of
_PAGE_GLOBAL via _PAGE_KERNEL_GLOBAL when CONFIG_PAGE_TABLE_ISOLATION=y,
but booted with nopti.  It's a simple enough fix.  Logically:

#ifdef CONFIG_PAGE_TABLE_ISOLATION
#define __PAGE_KERNEL_GLOBAL	static_cpu_has(X86_FEATURE_PTI) ?
					0 : _PAGE_GLOBAL
#else
#define __PAGE_KERNEL_GLOBAL	_PAGE_GLOBAL
#endif

But I don't really want to hide that gunk in a macro like that.  It
might make more sense as a static inline.  I'll give that a shot and resent.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 19:06           ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 19:06 UTC (permalink / raw)
  To: Nadav Amit
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

On 02/16/2018 10:25 AM, Nadav Amit wrote:
>> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
>> +#define __PAGE_KERNEL_GLOBAL		0
>> +#else
>> +#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
>> +#endif
> ...
>> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
>> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
>> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
>> 	 * different bit positions in the two formats.
>> 	 */
>> 	req_prot = pgprot_4k_2_large(req_prot);
>> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
>> +	req_prot = pgprot_set_on_present(req_prot,
>> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
>> 	req_prot = canon_pgprot(req_prot);
> From these chunks, it seems to me as req_prot will not have the global bit
> on when a??noptia?? parameter is provided. What am I missing?

That's a good point.  The current patch does not allow the use of
_PAGE_GLOBAL via _PAGE_KERNEL_GLOBAL when CONFIG_PAGE_TABLE_ISOLATION=y,
but booted with nopti.  It's a simple enough fix.  Logically:

#ifdef CONFIG_PAGE_TABLE_ISOLATION
#define __PAGE_KERNEL_GLOBAL	static_cpu_has(X86_FEATURE_PTI) ?
					0 : _PAGE_GLOBAL
#else
#define __PAGE_KERNEL_GLOBAL	_PAGE_GLOBAL
#endif

But I don't really want to hide that gunk in a macro like that.  It
might make more sense as a static inline.  I'll give that a shot and resent.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-16 19:06           ` Dave Hansen
@ 2018-02-16 19:54             ` Nadav Amit
  -1 siblings, 0 replies; 29+ messages in thread
From: Nadav Amit @ 2018-02-16 19:54 UTC (permalink / raw)
  To: Dave Hansen
  Cc: LKML, open list:MEMORY MANAGEMENT, Andy Lutomirski,
	Linus Torvalds, keescook, Hugh Dickins, Juergen Gross, x86

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 02/16/2018 10:25 AM, Nadav Amit wrote:
>>> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
>>> +#define __PAGE_KERNEL_GLOBAL		0
>>> +#else
>>> +#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
>>> +#endif
>> ...
>>> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
>>> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
>>> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
>>> 	 * different bit positions in the two formats.
>>> 	 */
>>> 	req_prot = pgprot_4k_2_large(req_prot);
>>> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
>>> +	req_prot = pgprot_set_on_present(req_prot,
>>> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
>>> 	req_prot = canon_pgprot(req_prot);
>> From these chunks, it seems to me as req_prot will not have the global bit
>> on when “nopti” parameter is provided. What am I missing?
> 
> That's a good point.  The current patch does not allow the use of
> _PAGE_GLOBAL via _PAGE_KERNEL_GLOBAL when CONFIG_PAGE_TABLE_ISOLATION=y,
> but booted with nopti.  It's a simple enough fix.  Logically:
> 
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
> #define __PAGE_KERNEL_GLOBAL	static_cpu_has(X86_FEATURE_PTI) ?
> 					0 : _PAGE_GLOBAL
> #else
> #define __PAGE_KERNEL_GLOBAL	_PAGE_GLOBAL
> #endif
> 
> But I don't really want to hide that gunk in a macro like that.  It
> might make more sense as a static inline.  I'll give that a shot and resent.

Since determining whether PTI is on is done in several places in the kernel,
maybe there should a single function to determine whether PTI is on,
something like:

static inline bool is_pti_on(void)
{
	return IS_ENABLED(CONFIG_PAGE_TABLE_ISOLATION) && 
		static_cpu_has(X86_FEATURE_PTI);
}

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 19:54             ` Nadav Amit
  0 siblings, 0 replies; 29+ messages in thread
From: Nadav Amit @ 2018-02-16 19:54 UTC (permalink / raw)
  To: Dave Hansen
  Cc: LKML, open list:MEMORY MANAGEMENT, Andy Lutomirski,
	Linus Torvalds, keescook, Hugh Dickins, Juergen Gross, x86

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 02/16/2018 10:25 AM, Nadav Amit wrote:
>>> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
>>> +#define __PAGE_KERNEL_GLOBAL		0
>>> +#else
>>> +#define __PAGE_KERNEL_GLOBAL		_PAGE_GLOBAL
>>> +#endif
>> ...
>>> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
>>> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
>>> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
>>> 	 * different bit positions in the two formats.
>>> 	 */
>>> 	req_prot = pgprot_4k_2_large(req_prot);
>>> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
>>> +	req_prot = pgprot_set_on_present(req_prot,
>>> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
>>> 	req_prot = canon_pgprot(req_prot);
>> From these chunks, it seems to me as req_prot will not have the global bit
>> on when “nopti” parameter is provided. What am I missing?
> 
> That's a good point.  The current patch does not allow the use of
> _PAGE_GLOBAL via _PAGE_KERNEL_GLOBAL when CONFIG_PAGE_TABLE_ISOLATION=y,
> but booted with nopti.  It's a simple enough fix.  Logically:
> 
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
> #define __PAGE_KERNEL_GLOBAL	static_cpu_has(X86_FEATURE_PTI) ?
> 					0 : _PAGE_GLOBAL
> #else
> #define __PAGE_KERNEL_GLOBAL	_PAGE_GLOBAL
> #endif
> 
> But I don't really want to hide that gunk in a macro like that.  It
> might make more sense as a static inline.  I'll give that a shot and resent.

Since determining whether PTI is on is done in several places in the kernel,
maybe there should a single function to determine whether PTI is on,
something like:

static inline bool is_pti_on(void)
{
	return IS_ENABLED(CONFIG_PAGE_TABLE_ISOLATION) && 
		static_cpu_has(X86_FEATURE_PTI);
}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-16 19:54             ` Nadav Amit
@ 2018-02-16 20:00               ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 20:00 UTC (permalink / raw)
  To: Nadav Amit
  Cc: LKML, open list:MEMORY MANAGEMENT, Andy Lutomirski,
	Linus Torvalds, keescook, Hugh Dickins, Juergen Gross, x86

On 02/16/2018 11:54 AM, Nadav Amit wrote:
>> But I don't really want to hide that gunk in a macro like that.  It
>> might make more sense as a static inline.  I'll give that a shot and resent.
> Since determining whether PTI is on is done in several places in the kernel,
> maybe there should a single function to determine whether PTI is on,
> something like:
> 
> static inline bool is_pti_on(void)
> {
> 	return IS_ENABLED(CONFIG_PAGE_TABLE_ISOLATION) && 
> 		static_cpu_has(X86_FEATURE_PTI);
> }

We should be able to do it with disabled-features.h and the X86_FEATURE
bit.  I'll look into it.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 20:00               ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 20:00 UTC (permalink / raw)
  To: Nadav Amit
  Cc: LKML, open list:MEMORY MANAGEMENT, Andy Lutomirski,
	Linus Torvalds, keescook, Hugh Dickins, Juergen Gross, x86

On 02/16/2018 11:54 AM, Nadav Amit wrote:
>> But I don't really want to hide that gunk in a macro like that.  It
>> might make more sense as a static inline.  I'll give that a shot and resent.
> Since determining whether PTI is on is done in several places in the kernel,
> maybe there should a single function to determine whether PTI is on,
> something like:
> 
> static inline bool is_pti_on(void)
> {
> 	return IS_ENABLED(CONFIG_PAGE_TABLE_ISOLATION) && 
> 		static_cpu_has(X86_FEATURE_PTI);
> }

We should be able to do it with disabled-features.h and the X86_FEATURE
bit.  I'll look into it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
  2018-02-16 18:25         ` Nadav Amit
@ 2018-02-16 23:19           ` Dave Hansen
  -1 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 23:19 UTC (permalink / raw)
  To: Nadav Amit
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

On 02/16/2018 10:25 AM, Nadav Amit wrote:
>> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
>> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
>> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
>> 	 * different bit positions in the two formats.
>> 	 */
>> 	req_prot = pgprot_4k_2_large(req_prot);
>> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
>> +	req_prot = pgprot_set_on_present(req_prot,
>> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
>> 	req_prot = canon_pgprot(req_prot);
> From these chunks, it seems to me as req_prot will not have the global bit
> on when “nopti” parameter is provided. What am I missing?

BTW, this code is broken.  It's trying to unconditionally set
_PAGE_GLOBAL whenever set do change_page_attr() and friends.  It gets
fixed up by canon_pgprot(), but it's wrong to do in the first place.
I've got a better fix for this coming.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL
@ 2018-02-16 23:19           ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-16 23:19 UTC (permalink / raw)
  To: Nadav Amit
  Cc: linux-kernel, linux-mm, luto, torvalds, keescook, hughd, jgross, x86

On 02/16/2018 10:25 AM, Nadav Amit wrote:
>> --- a/arch/x86/mm/pageattr.c~kpti-no-global-for-kernel-mappings	2018-02-13 15:17:56.148210060 -0800
>> +++ b/arch/x86/mm/pageattr.c	2018-02-13 15:17:56.153210060 -0800
>> @@ -593,7 +593,8 @@ try_preserve_large_page(pte_t *kpte, uns
>> 	 * different bit positions in the two formats.
>> 	 */
>> 	req_prot = pgprot_4k_2_large(req_prot);
>> -	req_prot = pgprot_set_on_present(req_prot, _PAGE_GLOBAL | _PAGE_PSE);
>> +	req_prot = pgprot_set_on_present(req_prot,
>> +			__PAGE_KERNEL_GLOBAL | _PAGE_PSE);
>> 	req_prot = canon_pgprot(req_prot);
> From these chunks, it seems to me as req_prot will not have the global bit
> on when a??noptia?? parameter is provided. What am I missing?

BTW, this code is broken.  It's trying to unconditionally set
_PAGE_GLOBAL whenever set do change_page_attr() and friends.  It gets
fixed up by canon_pgprot(), but it's wrong to do in the first place.
I've got a better fix for this coming.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] x86/pti: enable global pages for shared areas
  2018-02-13 23:19 Dave Hansen
@ 2018-02-13 23:19 ` Dave Hansen
  0 siblings, 0 replies; 29+ messages in thread
From: Dave Hansen @ 2018-02-13 23:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: Dave Hansen


From: Dave Hansen <dave.hansen@linux.intel.com>

The entry/exit text and cpu_entry_area are mapped into userspace and
the kernel.  But, they are not _PAGE_GLOBAL.  This creates unnecessary
TLB misses.

Add the _PAGE_GLOBAL flag for these areas.

After this patch, we can see that "GLB" shows up in each copy of the
page tables, that we have the same number of global entries in each
and that they are the *same* entries.

# grep -c GLB /sys/kernel/debug/page_tables/*
/sys/kernel/debug/page_tables/current_kernel:11
/sys/kernel/debug/page_tables/current_user:11
/sys/kernel/debug/page_tables/kernel:11

# for f in `ls /sys/kernel/debug/page_tables/`; do grep GLB /sys/kernel/debug/page_tables/$f > $f.GLB; done
# md5sum *.GLB
9caae8ad6a1fb53aca2407ec037f612d  current_kernel.GLB
9caae8ad6a1fb53aca2407ec037f612d  current_user.GLB
9caae8ad6a1fb53aca2407ec037f612d  kernel.GLB

A quick visual audit also shows that all the entries make sense.
0xfffffe0000000000 is the cpu_entry_area and 0xffffffff81c00000
is the entry/exit text:

# grep -c GLB /sys/kernel/debug/page_tables/current_user
0xfffffe0000000000-0xfffffe0000002000           8K     ro                 GLB NX pte
0xfffffe0000002000-0xfffffe0000003000           4K     RW                 GLB NX pte
0xfffffe0000003000-0xfffffe0000006000          12K     ro                 GLB NX pte
0xfffffe0000006000-0xfffffe0000007000           4K     ro                 GLB x  pte
0xfffffe0000007000-0xfffffe000000d000          24K     RW                 GLB NX pte
0xfffffe000002d000-0xfffffe000002e000           4K     ro                 GLB NX pte
0xfffffe000002e000-0xfffffe000002f000           4K     RW                 GLB NX pte
0xfffffe000002f000-0xfffffe0000032000          12K     ro                 GLB NX pte
0xfffffe0000032000-0xfffffe0000033000           4K     ro                 GLB x  pte
0xfffffe0000033000-0xfffffe0000039000          24K     RW                 GLB NX pte
0xffffffff81c00000-0xffffffff81e00000           2M     ro         PSE     GLB x  pmd

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@google.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: x86@kernel.org
---

 b/arch/x86/mm/cpu_entry_area.c |    7 +++++++
 b/arch/x86/mm/pti.c            |    9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff -puN arch/x86/mm/cpu_entry_area.c~kpti-why-no-global arch/x86/mm/cpu_entry_area.c
--- a/arch/x86/mm/cpu_entry_area.c~kpti-why-no-global	2018-02-13 15:17:56.735210059 -0800
+++ b/arch/x86/mm/cpu_entry_area.c	2018-02-13 15:17:56.740210059 -0800
@@ -28,6 +28,13 @@ void cea_set_pte(void *cea_vaddr, phys_a
 {
 	unsigned long va = (unsigned long) cea_vaddr;
 
+	/*
+	 * The cpu_entry_area is shared between the user and kernel
+	 * page tables.  All of its ptes can safely be global.
+	 */
+	if (boot_cpu_has(X86_FEATURE_PGE))
+		pgprot_val(flags) |= _PAGE_GLOBAL;
+
 	set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
 }
 
diff -puN arch/x86/mm/pti.c~kpti-why-no-global arch/x86/mm/pti.c
--- a/arch/x86/mm/pti.c~kpti-why-no-global	2018-02-13 15:17:56.737210059 -0800
+++ b/arch/x86/mm/pti.c	2018-02-13 15:17:56.740210059 -0800
@@ -300,6 +300,13 @@ pti_clone_pmds(unsigned long start, unsi
 			return;
 
 		/*
+		 * Setting 'target_pmd' below creates a mapping in both
+		 * the user and kernel page tables.  It is effectively
+		 * global, so set it as global in both copies.
+		 */
+		*pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
+
+		/*
 		 * Copy the PMD.  That is, the kernelmode and usermode
 		 * tables will share the last-level page tables of this
 		 * address range
@@ -348,7 +355,7 @@ static void __init pti_clone_entry_text(
 {
 	pti_clone_pmds((unsigned long) __entry_text_start,
 			(unsigned long) __irqentry_text_end,
-		       _PAGE_RW | _PAGE_GLOBAL);
+		       _PAGE_RW);
 }
 
 /*
_

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2018-02-16 23:19 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-15 13:20 [PATCH 0/3] Use global pages with PTI Dave Hansen
2018-02-15 13:20 ` Dave Hansen
2018-02-15 13:20 ` [PATCH 1/3] x86/mm: factor out conditional pageattr PTE bit setting code Dave Hansen
2018-02-15 13:20   ` Dave Hansen
2018-02-15 13:20 ` [PATCH 2/3] x86/mm: introduce __PAGE_KERNEL_GLOBAL Dave Hansen
2018-02-15 13:20   ` Dave Hansen
2018-02-16 17:47   ` Nadav Amit
2018-02-16 17:47     ` Nadav Amit
2018-02-16 18:03     ` Dave Hansen
2018-02-16 18:03       ` Dave Hansen
2018-02-16 18:25       ` Nadav Amit
2018-02-16 18:25         ` Nadav Amit
2018-02-16 19:06         ` Dave Hansen
2018-02-16 19:06           ` Dave Hansen
2018-02-16 19:54           ` Nadav Amit
2018-02-16 19:54             ` Nadav Amit
2018-02-16 20:00             ` Dave Hansen
2018-02-16 20:00               ` Dave Hansen
2018-02-16 23:19         ` Dave Hansen
2018-02-16 23:19           ` Dave Hansen
2018-02-15 13:20 ` [PATCH 3/3] x86/pti: enable global pages for shared areas Dave Hansen
2018-02-15 13:20   ` Dave Hansen
2018-02-15 17:47 ` [PATCH 0/3] Use global pages with PTI Linus Torvalds
2018-02-15 17:47   ` Linus Torvalds
2018-02-15 18:13   ` Dave Hansen
2018-02-15 18:13     ` Dave Hansen
2018-02-15 23:55   ` Dave Hansen
2018-02-15 23:55     ` Dave Hansen
  -- strict thread matches above, loose matches on Subject: below --
2018-02-13 23:19 Dave Hansen
2018-02-13 23:19 ` [PATCH 3/3] x86/pti: enable global pages for shared areas Dave Hansen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.