[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   14 +-
 b/arch/x86/mm/pti.c|   23 ++-
 2 files changed, 35 insertions(+), 2 deletions(-)

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-04-06 
10:47:58.246796118 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-04-06 10:47:58.252796118 -0700
@@ -27,8 +27,20 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* The cpu_entry_area is shared between the user and kernel
+* page tables.  All of its ptes can safely be global.
+* _PAGE_GLOBAL gets reused to help indicate PROT_NONE for
+* non-present PTEs, so be careful not to set it in that
+* case to avoid confusion.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE) &&
+   (pgprot_val(flags) & _PAGE_PRESENT))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-04-06 10:47:58.248796118 
-0700
+++ b/arch/x86/mm/pti.c 2018-04-06 10:47:58.252796118 -0700
@@ -300,6 +300,27 @@ pti_clone_pmds(unsigned long start, unsi
return;
 
/*
+* Only clone present PMDs.  This ensures only setting
+* _PAGE_GLOBAL on present PMDs.  This should only be
+* called on well-known addresses anyway, so a non-
+* present PMD would be a surprise.
+*/
+   if (WARN_ON(!(pmd_flags(*pmd) & _PAGE_PRESENT)))
+   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.  Note:
+* the X86_FEATURE_PGE check is not _required_ because
+* the CPU ignores _PAGE_GLOBAL when PGE is not
+* supported.  The check keeps consistentency with
+* code that only set this bit when supported.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE))
+   *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 +369,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);
 }
 
 /*
_

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   14 +-
 b/arch/x86/mm/pti.c|   23 ++-
 2 files changed, 35 insertions(+), 2 deletions(-)

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-04-06 
10:47:58.246796118 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-04-06 10:47:58.252796118 -0700
@@ -27,8 +27,20 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* The cpu_entry_area is shared between the user and kernel
+* page tables.  All of its ptes can safely be global.
+* _PAGE_GLOBAL gets reused to help indicate PROT_NONE for
+* non-present PTEs, so be careful not to set it in that
+* case to avoid confusion.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE) &&
+   (pgprot_val(flags) & _PAGE_PRESENT))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-04-06 10:47:58.248796118 
-0700
+++ b/arch/x86/mm/pti.c 2018-04-06 10:47:58.252796118 -0700
@@ -300,6 +300,27 @@ pti_clone_pmds(unsigned long start, unsi
return;
 
/*
+* Only clone present PMDs.  This ensures only setting
+* _PAGE_GLOBAL on present PMDs.  This should only be
+* called on well-known addresses anyway, so a non-
+* present PMD would be a surprise.
+*/
+   if (WARN_ON(!(pmd_flags(*pmd) & _PAGE_PRESENT)))
+   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.  Note:
+* the X86_FEATURE_PGE check is not _required_ because
+* the CPU ignores _PAGE_GLOBAL when PGE is not
+* supported.  The check keeps consistentency with
+* code that only set this bit when supported.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE))
+   *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 +369,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);
 }
 
 /*
_

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Thomas Gleixner]

On Wed, 4 Apr 2018, Nadav Amit wrote:
> Dave Hansen  wrote:
> > On 04/03/2018 09:45 PM, Nadav Amit wrote:
> >> Dave Hansen  wrote:
> >>> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
> >>> {
> >>>   unsigned long va = (unsigned long) cea_vaddr;
> >>> + pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
> >>> 
> >>> - set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
> >>> + /*
> >>> +  * 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))
> >>> + pte = pte_set_flags(pte, _PAGE_GLOBAL);
> >> 
> >> I think it would be safer to check that the PTE is indeed present before
> >> setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets 
> >> PAGE_NONE
> >> for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
> >> the same bit, everything would be fine, but it might cause bugs one day.
> > 
> > That's a reasonable safety thing to add, I think.
> > 
> > But, looking at it, I am wondering why we did this in
> > percpu_setup_debug_store():
> > 
> >for (; npages; npages--, cea += PAGE_SIZE)
> >cea_set_pte(cea, 0, PAGE_NONE);
> > 
> > Did we really want that to be PAGE_NONE, or was it supposed to create a
> > PTE that returns true for pte_none()?
> 
> I yield it to others to answer...

My bad. I should have used pgprot(0).

Thanks,

tglx

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Thomas Gleixner]

On Wed, 4 Apr 2018, Nadav Amit wrote:
> Dave Hansen  wrote:
> > On 04/03/2018 09:45 PM, Nadav Amit wrote:
> >> Dave Hansen  wrote:
> >>> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
> >>> {
> >>>   unsigned long va = (unsigned long) cea_vaddr;
> >>> + pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
> >>> 
> >>> - set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
> >>> + /*
> >>> +  * 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))
> >>> + pte = pte_set_flags(pte, _PAGE_GLOBAL);
> >> 
> >> I think it would be safer to check that the PTE is indeed present before
> >> setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets 
> >> PAGE_NONE
> >> for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
> >> the same bit, everything would be fine, but it might cause bugs one day.
> > 
> > That's a reasonable safety thing to add, I think.
> > 
> > But, looking at it, I am wondering why we did this in
> > percpu_setup_debug_store():
> > 
> >for (; npages; npages--, cea += PAGE_SIZE)
> >cea_set_pte(cea, 0, PAGE_NONE);
> > 
> > Did we really want that to be PAGE_NONE, or was it supposed to create a
> > PTE that returns true for pte_none()?
> 
> I yield it to others to answer...

My bad. I should have used pgprot(0).

Thanks,

tglx

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Nadav Amit]

Dave Hansen  wrote:

> On 04/03/2018 09:45 PM, Nadav Amit wrote:
>> Dave Hansen  wrote:
>> 
>>> From: Dave Hansen 
>>> 
>>> 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.
>>> 
>>> Signed-off-by: Dave Hansen 
>>> Cc: Andrea Arcangeli 
>>> Cc: Andy Lutomirski 
>>> Cc: Linus Torvalds 
>>> Cc: Kees Cook 
>>> Cc: Hugh Dickins 
>>> Cc: Juergen Gross 
>>> Cc: x...@kernel.org
>>> Cc: Nadav Amit 
>>> ---
>>> 
>>> b/arch/x86/mm/cpu_entry_area.c |   10 +-
>>> b/arch/x86/mm/pti.c|   14 +-
>>> 2 files changed, 22 insertions(+), 2 deletions(-)
>>> 
>>> 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-04-02 
>>> 16:41:17.157605167 -0700
>>> +++ b/arch/x86/mm/cpu_entry_area.c  2018-04-02 16:41:17.162605167 -0700
>>> @@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
>>> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
>>> {
>>> unsigned long va = (unsigned long) cea_vaddr;
>>> +   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
>>> 
>>> -   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
>>> +   /*
>>> +* 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))
>>> +   pte = pte_set_flags(pte, _PAGE_GLOBAL);
>> 
>> I think it would be safer to check that the PTE is indeed present before
>> setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets PAGE_NONE
>> for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
>> the same bit, everything would be fine, but it might cause bugs one day.
> 
> That's a reasonable safety thing to add, I think.
> 
> But, looking at it, I am wondering why we did this in
> percpu_setup_debug_store():
> 
>for (; npages; npages--, cea += PAGE_SIZE)
>cea_set_pte(cea, 0, PAGE_NONE);
> 
> Did we really want that to be PAGE_NONE, or was it supposed to create a
> PTE that returns true for pte_none()?

I yield it to others to answer...

> 
>>> /*
>>> +* 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.  Note:
>>> +* the X86_FEATURE_PGE check is not _required_ because
>>> +* the CPU ignores _PAGE_GLOBAL when PGE is not
>>> +* supported.  The check keeps consistentency with
>>> +* code that only set this bit when supported.
>>> +*/
>>> +   if (boot_cpu_has(X86_FEATURE_PGE))
>>> +   *pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
>> 
>> Same here.
> 
> Is there  a reason that the pmd_none() check above this does not work?

For any practical reasons, right now, it should be fine. But pmd_none() will
not save us if _PAGE_PROTNONE ever changes, for example. Note that the check
is with pmd_none() and not pmd_protnone().

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Nadav Amit]

Dave Hansen  wrote:

> On 04/03/2018 09:45 PM, Nadav Amit wrote:
>> Dave Hansen  wrote:
>> 
>>> From: Dave Hansen 
>>> 
>>> 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.
>>> 
>>> Signed-off-by: Dave Hansen 
>>> Cc: Andrea Arcangeli 
>>> Cc: Andy Lutomirski 
>>> Cc: Linus Torvalds 
>>> Cc: Kees Cook 
>>> Cc: Hugh Dickins 
>>> Cc: Juergen Gross 
>>> Cc: x...@kernel.org
>>> Cc: Nadav Amit 
>>> ---
>>> 
>>> b/arch/x86/mm/cpu_entry_area.c |   10 +-
>>> b/arch/x86/mm/pti.c|   14 +-
>>> 2 files changed, 22 insertions(+), 2 deletions(-)
>>> 
>>> 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-04-02 
>>> 16:41:17.157605167 -0700
>>> +++ b/arch/x86/mm/cpu_entry_area.c  2018-04-02 16:41:17.162605167 -0700
>>> @@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
>>> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
>>> {
>>> unsigned long va = (unsigned long) cea_vaddr;
>>> +   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
>>> 
>>> -   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
>>> +   /*
>>> +* 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))
>>> +   pte = pte_set_flags(pte, _PAGE_GLOBAL);
>> 
>> I think it would be safer to check that the PTE is indeed present before
>> setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets PAGE_NONE
>> for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
>> the same bit, everything would be fine, but it might cause bugs one day.
> 
> That's a reasonable safety thing to add, I think.
> 
> But, looking at it, I am wondering why we did this in
> percpu_setup_debug_store():
> 
>for (; npages; npages--, cea += PAGE_SIZE)
>cea_set_pte(cea, 0, PAGE_NONE);
> 
> Did we really want that to be PAGE_NONE, or was it supposed to create a
> PTE that returns true for pte_none()?

I yield it to others to answer...

> 
>>> /*
>>> +* 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.  Note:
>>> +* the X86_FEATURE_PGE check is not _required_ because
>>> +* the CPU ignores _PAGE_GLOBAL when PGE is not
>>> +* supported.  The check keeps consistentency with
>>> +* code that only set this bit when supported.
>>> +*/
>>> +   if (boot_cpu_has(X86_FEATURE_PGE))
>>> +   *pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
>> 
>> Same here.
> 
> Is there  a reason that the pmd_none() check above this does not work?

For any practical reasons, right now, it should be fine. But pmd_none() will
not save us if _PAGE_PROTNONE ever changes, for example. Note that the check
is with pmd_none() and not pmd_protnone().

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

On 04/03/2018 09:45 PM, Nadav Amit wrote:
> Dave Hansen  wrote:
> 
>>
>> From: Dave Hansen 
>>
>> 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.
>>
>> Signed-off-by: Dave Hansen 
>> Cc: Andrea Arcangeli 
>> Cc: Andy Lutomirski 
>> Cc: Linus Torvalds 
>> Cc: Kees Cook 
>> Cc: Hugh Dickins 
>> Cc: Juergen Gross 
>> Cc: x...@kernel.org
>> Cc: Nadav Amit 
>> ---
>>
>> b/arch/x86/mm/cpu_entry_area.c |   10 +-
>> b/arch/x86/mm/pti.c|   14 +-
>> 2 files changed, 22 insertions(+), 2 deletions(-)
>>
>> 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-global2018-04-02 
>> 16:41:17.157605167 -0700
>> +++ b/arch/x86/mm/cpu_entry_area.c   2018-04-02 16:41:17.162605167 -0700
>> @@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
>> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
>> {
>>  unsigned long va = (unsigned long) cea_vaddr;
>> +pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
>>
>> -set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
>> +/*
>> + * 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))
>> +pte = pte_set_flags(pte, _PAGE_GLOBAL);
> 
> I think it would be safer to check that the PTE is indeed present before
> setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets PAGE_NONE
> for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
> the same bit, everything would be fine, but it might cause bugs one day.

That's a reasonable safety thing to add, I think.

But, looking at it, I am wondering why we did this in
percpu_setup_debug_store():

for (; npages; npages--, cea += PAGE_SIZE)
cea_set_pte(cea, 0, PAGE_NONE);

Did we really want that to be PAGE_NONE, or was it supposed to create a
PTE that returns true for pte_none()?

>>  /*
>> + * 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.  Note:
>> + * the X86_FEATURE_PGE check is not _required_ because
>> + * the CPU ignores _PAGE_GLOBAL when PGE is not
>> + * supported.  The check keeps consistentency with
>> + * code that only set this bit when supported.
>> + */
>> +if (boot_cpu_has(X86_FEATURE_PGE))
>> +*pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
> 
> Same here.

Is there  a reason that the pmd_none() check above this does not work?

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

On 04/03/2018 09:45 PM, Nadav Amit wrote:
> Dave Hansen  wrote:
> 
>>
>> From: Dave Hansen 
>>
>> 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.
>>
>> Signed-off-by: Dave Hansen 
>> Cc: Andrea Arcangeli 
>> Cc: Andy Lutomirski 
>> Cc: Linus Torvalds 
>> Cc: Kees Cook 
>> Cc: Hugh Dickins 
>> Cc: Juergen Gross 
>> Cc: x...@kernel.org
>> Cc: Nadav Amit 
>> ---
>>
>> b/arch/x86/mm/cpu_entry_area.c |   10 +-
>> b/arch/x86/mm/pti.c|   14 +-
>> 2 files changed, 22 insertions(+), 2 deletions(-)
>>
>> 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-global2018-04-02 
>> 16:41:17.157605167 -0700
>> +++ b/arch/x86/mm/cpu_entry_area.c   2018-04-02 16:41:17.162605167 -0700
>> @@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
>> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
>> {
>>  unsigned long va = (unsigned long) cea_vaddr;
>> +pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
>>
>> -set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
>> +/*
>> + * 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))
>> +pte = pte_set_flags(pte, _PAGE_GLOBAL);
> 
> I think it would be safer to check that the PTE is indeed present before
> setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets PAGE_NONE
> for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
> the same bit, everything would be fine, but it might cause bugs one day.

That's a reasonable safety thing to add, I think.

But, looking at it, I am wondering why we did this in
percpu_setup_debug_store():

for (; npages; npages--, cea += PAGE_SIZE)
cea_set_pte(cea, 0, PAGE_NONE);

Did we really want that to be PAGE_NONE, or was it supposed to create a
PTE that returns true for pte_none()?

>>  /*
>> + * 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.  Note:
>> + * the X86_FEATURE_PGE check is not _required_ because
>> + * the CPU ignores _PAGE_GLOBAL when PGE is not
>> + * supported.  The check keeps consistentency with
>> + * code that only set this bit when supported.
>> + */
>> +if (boot_cpu_has(X86_FEATURE_PGE))
>> +*pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);
> 
> Same here.

Is there  a reason that the pmd_none() check above this does not work?

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Nadav Amit]

Dave Hansen  wrote:

> 
> From: Dave Hansen 
> 
> 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.
> 
> Signed-off-by: Dave Hansen 
> Cc: Andrea Arcangeli 
> Cc: Andy Lutomirski 
> Cc: Linus Torvalds 
> Cc: Kees Cook 
> Cc: Hugh Dickins 
> Cc: Juergen Gross 
> Cc: x...@kernel.org
> Cc: Nadav Amit 
> ---
> 
> b/arch/x86/mm/cpu_entry_area.c |   10 +-
> b/arch/x86/mm/pti.c|   14 +-
> 2 files changed, 22 insertions(+), 2 deletions(-)
> 
> 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-04-02 
> 16:41:17.157605167 -0700
> +++ b/arch/x86/mm/cpu_entry_area.c2018-04-02 16:41:17.162605167 -0700
> @@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
> {
>   unsigned long va = (unsigned long) cea_vaddr;
> + pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
> 
> - set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
> + /*
> +  * 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))
> + pte = pte_set_flags(pte, _PAGE_GLOBAL);

I think it would be safer to check that the PTE is indeed present before
setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets PAGE_NONE
for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
the same bit, everything would be fine, but it might cause bugs one day.

> +
> + set_pte_vaddr(va, pte);
> }
> 
> static void __init
> 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-global2018-04-02 16:41:17.159605167 
> -0700
> +++ b/arch/x86/mm/pti.c   2018-04-02 16:41:17.163605167 -0700
> @@ -300,6 +300,18 @@ 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.  Note:
> +  * the X86_FEATURE_PGE check is not _required_ because
> +  * the CPU ignores _PAGE_GLOBAL when PGE is not
> +  * supported.  The check keeps consistentency with
> +  * code that only set this bit when supported.
> +  */
> + if (boot_cpu_has(X86_FEATURE_PGE))
> + *pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);

Same here.

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Nadav Amit]

Dave Hansen  wrote:

> 
> From: Dave Hansen 
> 
> 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.
> 
> Signed-off-by: Dave Hansen 
> Cc: Andrea Arcangeli 
> Cc: Andy Lutomirski 
> Cc: Linus Torvalds 
> Cc: Kees Cook 
> Cc: Hugh Dickins 
> Cc: Juergen Gross 
> Cc: x...@kernel.org
> Cc: Nadav Amit 
> ---
> 
> b/arch/x86/mm/cpu_entry_area.c |   10 +-
> b/arch/x86/mm/pti.c|   14 +-
> 2 files changed, 22 insertions(+), 2 deletions(-)
> 
> 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-04-02 
> 16:41:17.157605167 -0700
> +++ b/arch/x86/mm/cpu_entry_area.c2018-04-02 16:41:17.162605167 -0700
> @@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
> void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
> {
>   unsigned long va = (unsigned long) cea_vaddr;
> + pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
> 
> - set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
> + /*
> +  * 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))
> + pte = pte_set_flags(pte, _PAGE_GLOBAL);

I think it would be safer to check that the PTE is indeed present before
setting _PAGE_GLOBAL. For example, percpu_setup_debug_store() sets PAGE_NONE
for non-present entries. In this case, since PAGE_NONE and PAGE_GLOBAL use
the same bit, everything would be fine, but it might cause bugs one day.

> +
> + set_pte_vaddr(va, pte);
> }
> 
> static void __init
> 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-global2018-04-02 16:41:17.159605167 
> -0700
> +++ b/arch/x86/mm/pti.c   2018-04-02 16:41:17.163605167 -0700
> @@ -300,6 +300,18 @@ 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.  Note:
> +  * the X86_FEATURE_PGE check is not _required_ because
> +  * the CPU ignores _PAGE_GLOBAL when PGE is not
> +  * supported.  The check keeps consistentency with
> +  * code that only set this bit when supported.
> +  */
> + if (boot_cpu_has(X86_FEATURE_PGE))
> + *pmd = pmd_set_flags(*pmd, _PAGE_GLOBAL);

Same here.

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   10 +-
 b/arch/x86/mm/pti.c|   14 +-
 2 files changed, 22 insertions(+), 2 deletions(-)

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-04-02 
16:41:17.157605167 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-04-02 16:41:17.162605167 -0700
@@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* 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))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-04-02 16:41:17.159605167 
-0700
+++ b/arch/x86/mm/pti.c 2018-04-02 16:41:17.163605167 -0700
@@ -300,6 +300,18 @@ 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.  Note:
+* the X86_FEATURE_PGE check is not _required_ because
+* the CPU ignores _PAGE_GLOBAL when PGE is not
+* supported.  The check keeps consistentency with
+* code that only set this bit when supported.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE))
+   *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 +360,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);
 }
 
 /*
_

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   10 +-
 b/arch/x86/mm/pti.c|   14 +-
 2 files changed, 22 insertions(+), 2 deletions(-)

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-04-02 
16:41:17.157605167 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-04-02 16:41:17.162605167 -0700
@@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* 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))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-04-02 16:41:17.159605167 
-0700
+++ b/arch/x86/mm/pti.c 2018-04-02 16:41:17.163605167 -0700
@@ -300,6 +300,18 @@ 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.  Note:
+* the X86_FEATURE_PGE check is not _required_ because
+* the CPU ignores _PAGE_GLOBAL when PGE is not
+* supported.  The check keeps consistentency with
+* code that only set this bit when supported.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE))
+   *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 +360,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);
 }
 
 /*
_

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

On 04/02/2018 10:56 AM, Linus Torvalds wrote:
> On Mon, Apr 2, 2018 at 10:27 AM, Dave Hansen
>  wrote:
>> +   /*
>> +* 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))
>> +   pte = pte_set_flags(pte, _PAGE_GLOBAL);
> So this is where the quesion of "why is this conditional" is valid.
> 
> We could set _PAGE_GLOBAL unconditionally, not bothering with testing
> X86_FEATURE_PGE.

I think we should just keep the check for now.  Before this patch set,
on !X86_FEATURE_PGE systems, we cleared _PAGE_GLOBAL in virtually all
places due to masking via __supported_pte_mask.

It is rather harmless either way, but being _consistent_ (by keeping the
check) with all of our PTEs is nice.

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

On 04/02/2018 10:56 AM, Linus Torvalds wrote:
> On Mon, Apr 2, 2018 at 10:27 AM, Dave Hansen
>  wrote:
>> +   /*
>> +* 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))
>> +   pte = pte_set_flags(pte, _PAGE_GLOBAL);
> So this is where the quesion of "why is this conditional" is valid.
> 
> We could set _PAGE_GLOBAL unconditionally, not bothering with testing
> X86_FEATURE_PGE.

I think we should just keep the check for now.  Before this patch set,
on !X86_FEATURE_PGE systems, we cleared _PAGE_GLOBAL in virtually all
places due to masking via __supported_pte_mask.

It is rather harmless either way, but being _consistent_ (by keeping the
check) with all of our PTEs is nice.

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Linus Torvalds]

On Mon, Apr 2, 2018 at 10:27 AM, Dave Hansen
 wrote:
>
> +   /*
> +* 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))
> +   pte = pte_set_flags(pte, _PAGE_GLOBAL);

So this is where the quesion of "why is this conditional" is valid.

We could set _PAGE_GLOBAL unconditionally, not bothering with testing
X86_FEATURE_PGE.

Linus

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Linus Torvalds]

On Mon, Apr 2, 2018 at 10:27 AM, Dave Hansen
 wrote:
>
> +   /*
> +* 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))
> +   pte = pte_set_flags(pte, _PAGE_GLOBAL);

So this is where the quesion of "why is this conditional" is valid.

We could set _PAGE_GLOBAL unconditionally, not bothering with testing
X86_FEATURE_PGE.

Linus

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   10 +-
 b/arch/x86/mm/pti.c|   10 +-
 2 files changed, 18 insertions(+), 2 deletions(-)

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-04-02 
10:26:47.185661207 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-04-02 10:26:47.190661207 -0700
@@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* 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))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-04-02 10:26:47.187661207 
-0700
+++ b/arch/x86/mm/pti.c 2018-04-02 10:26:47.191661207 -0700
@@ -300,6 +300,14 @@ 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.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE))
+   *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 +356,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);
 }
 
 /*
_

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   10 +-
 b/arch/x86/mm/pti.c|   10 +-
 2 files changed, 18 insertions(+), 2 deletions(-)

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-04-02 
10:26:47.185661207 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-04-02 10:26:47.190661207 -0700
@@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* 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))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-04-02 10:26:47.187661207 
-0700
+++ b/arch/x86/mm/pti.c 2018-04-02 10:26:47.191661207 -0700
@@ -300,6 +300,14 @@ 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.
+*/
+   if (boot_cpu_has(X86_FEATURE_PGE))
+   *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 +356,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);
 }
 
 /*
_

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

On 03/23/2018 12:12 PM, Nadav Amit wrote:
>>  /*
>> + * 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);
> if (boot_cpu_has(X86_FEATURE_PGE)) ?

Good catch.  I'll update that.

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

On 03/23/2018 12:12 PM, Nadav Amit wrote:
>>  /*
>> + * 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);
> if (boot_cpu_has(X86_FEATURE_PGE)) ?

Good catch.  I'll update that.

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Nadav Amit]

Dave Hansen  wrote:

> 
> From: Dave Hansen 
> 
> 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.
> 
> static void __init
> 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-global2018-03-21 16:32:00.799192311 
> -0700
> +++ b/arch/x86/mm/pti.c   2018-03-21 16:32:00.803192311 -0700
> @@ -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);
if (boot_cpu_has(X86_FEATURE_PGE)) ?

Re: [PATCH 09/11] x86/pti: enable global pages for shared areas

[Nadav Amit]

Dave Hansen  wrote:

> 
> From: Dave Hansen 
> 
> 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.
> 
> static void __init
> 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-global2018-03-21 16:32:00.799192311 
> -0700
> +++ b/arch/x86/mm/pti.c   2018-03-21 16:32:00.803192311 -0700
> @@ -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);
if (boot_cpu_has(X86_FEATURE_PGE)) ?

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   10 +-
 b/arch/x86/mm/pti.c|9 -
 2 files changed, 17 insertions(+), 2 deletions(-)

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-03-21 
16:32:00.797192311 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-03-21 16:32:00.802192311 -0700
@@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* 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))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-03-21 16:32:00.799192311 
-0700
+++ b/arch/x86/mm/pti.c 2018-03-21 16:32:00.803192311 -0700
@@ -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);
 }
 
 /*
_

[PATCH 09/11] x86/pti: enable global pages for shared areas

[Dave Hansen]

From: Dave Hansen 

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.

Signed-off-by: Dave Hansen 
Cc: Andrea Arcangeli 
Cc: Andy Lutomirski 
Cc: Linus Torvalds 
Cc: Kees Cook 
Cc: Hugh Dickins 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Nadav Amit 
---

 b/arch/x86/mm/cpu_entry_area.c |   10 +-
 b/arch/x86/mm/pti.c|9 -
 2 files changed, 17 insertions(+), 2 deletions(-)

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-03-21 
16:32:00.797192311 -0700
+++ b/arch/x86/mm/cpu_entry_area.c  2018-03-21 16:32:00.802192311 -0700
@@ -27,8 +27,16 @@ EXPORT_SYMBOL(get_cpu_entry_area);
 void cea_set_pte(void *cea_vaddr, phys_addr_t pa, pgprot_t flags)
 {
unsigned long va = (unsigned long) cea_vaddr;
+   pte_t pte = pfn_pte(pa >> PAGE_SHIFT, flags);
 
-   set_pte_vaddr(va, pfn_pte(pa >> PAGE_SHIFT, flags));
+   /*
+* 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))
+   pte = pte_set_flags(pte, _PAGE_GLOBAL);
+
+   set_pte_vaddr(va, pte);
 }
 
 static void __init
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-03-21 16:32:00.799192311 
-0700
+++ b/arch/x86/mm/pti.c 2018-03-21 16:32:00.803192311 -0700
@@ -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);
 }
 
 /*
_