Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1671353 > unrolled thread

[PATCH v3 00/11] PCID and improved laziness

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/11] PCID and improved laziness Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
    [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 15:40 +0200
        Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 15:50 +0200
        Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-06-22 05:00 +0200
          Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Thomas Gleixner <tglx@linutronix.de> - 2017-06-22 14:30 +0200
            Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-06-22 20:20 +0200
              Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Thomas Gleixner <tglx@linutronix.de> - 2017-06-22 23:30 +0200
                Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-06-23 05:10 +0200
                  Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Thomas Gleixner <tglx@linutronix.de> - 2017-06-23 09:30 +0200
      Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Nadav Amit <nadav.amit@gmail.com> - 2017-06-22 18:20 +0200
        Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-06-22 20:20 +0200
      Re: [PATCH v3 11/11] x86/mm: Try to preserve old TLB entries using  PCID Borislav Petkov <bp@alien8.de> - 2017-06-26 18:10 +0200
    [PATCH v3 08/11] x86/mm: Disable PCID on 32-bit kernels Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 08/11] x86/mm: Disable PCID on 32-bit kernels Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 11:30 +0200
      Re: [PATCH v3 08/11] x86/mm: Disable PCID on 32-bit kernels Borislav Petkov <bp@alien8.de> - 2017-06-23 11:30 +0200
    [PATCH v3 02/11] x86/ldt: Simplify LDT switching logic Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 02/11] x86/ldt: Simplify LDT switching logic Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 10:10 +0200
      Re: [PATCH v3 02/11] x86/ldt: Simplify LDT switching logic Borislav Petkov <bp@alien8.de> - 2017-06-21 11:50 +0200
      [tip:x86/mm] x86/ldt: Simplify the LDT switching logic tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2017-06-22 13:20 +0200
    [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 10:10 +0200
      Re: [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Borislav Petkov <bp@alien8.de> - 2017-06-21 10:50 +0200
        Re: [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Andy Lutomirski <luto@kernel.org> - 2017-06-21 17:20 +0200
      Re: [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Nadav Amit <nadav.amit@gmail.com> - 2017-06-22 01:30 +0200
        Re: [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Andy Lutomirski <luto@kernel.org> - 2017-06-22 04:30 +0200
          Re: [PATCH v3 01/11] x86/mm: Don't reenter flush_tlb_func_common() Ingo Molnar <mingo@kernel.org> - 2017-06-22 09:40 +0200
    [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness tracking Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness  tracking Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 11:10 +0200
        Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness tracking Andy Lutomirski <luto@kernel.org> - 2017-06-21 18:30 +0200
          Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness  tracking Borislav Petkov <bp@alien8.de> - 2017-06-21 19:30 +0200
      Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness  tracking Borislav Petkov <bp@alien8.de> - 2017-06-22 17:00 +0200
        Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness tracking Andy Lutomirski <luto@kernel.org> - 2017-06-22 19:50 +0200
          Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness  tracking Borislav Petkov <bp@alien8.de> - 2017-06-22 21:10 +0200
      Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness  tracking Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-06-23 15:40 +0200
        Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness tracking Andy Lutomirski <luto@kernel.org> - 2017-06-23 17:30 +0200
    [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a unique ID Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a  unique ID Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 10:10 +0200
      Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a  unique ID Borislav Petkov <bp@alien8.de> - 2017-06-21 12:40 +0200
        Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a  unique ID Andy Lutomirski <luto@kernel.org> - 2017-06-21 18:40 +0200
          Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a  unique ID Borislav Petkov <bp@alien8.de> - 2017-06-21 19:10 +0200
      Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a  unique ID Borislav Petkov <bp@alien8.de> - 2017-06-21 19:50 +0200
        Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a  unique ID Andy Lutomirski <luto@kernel.org> - 2017-06-22 04:40 +0200
    [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 11:40 +0200
        Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 15:50 +0200
        Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Andy Lutomirski <luto@kernel.org> - 2017-06-21 22:40 +0200
      Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Borislav Petkov <bp@alien8.de> - 2017-06-23 14:00 +0200
        Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Andy Lutomirski <luto@kernel.org> - 2017-06-23 17:30 +0200
      Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-06-23 15:40 +0200
    [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code Andy Lutomirski <luto@kernel.org> - 2017-06-21 07:30 +0200
      Re: [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code Thomas Gleixner <tglx@linutronix.de> - 2017-06-21 11:30 +0200
        Re: [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code Andy Lutomirski <luto@kernel.org> - 2017-06-21 18:30 +0200
      Re: [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code Borislav Petkov <bp@alien8.de> - 2017-06-23 11:10 +0200
    Re: [PATCH v3 00/11] PCID and improved laziness Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-21 20:30 +0200
      Re: [PATCH v3 00/11] PCID and improved laziness Andy Lutomirski <luto@kernel.org> - 2017-06-22 07:30 +0200

Page 3 of 3 — ← Prev page 1 2 [3]

#1671816 — Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a unique ID

On Wed, Jun 21, 2017 at 08:23:07AM -0700, Andy Lutomirski wrote:
> It's stated explicitly in the comment where it's declared in the same file.

Doh, it says "zero" there. I should learn how to read.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1671853 — Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a unique ID

On Tue, Jun 20, 2017 at 10:22:10PM -0700, Andy Lutomirski wrote:
> - * The x86 doesn't have a mmu context, but
> - * we put the segment information here.
> + * x86 has arch-specific MMU state beyond what lives in mm_struct.
>   */
>  typedef struct {
> +	/*
> +	 * ctx_id uniquely identifies this mm_struct.  A ctx_id will never
> +	 * be reused, and zero is not a valid ctx_id.
> +	 */
> +	u64 ctx_id;
> +
> +	/*
> +	 * Any code that needs to do any sort of TLB flushing for this
> +	 * mm will first make its changes to the page tables, then
> +	 * increment tlb_gen, then flush.  This lets the low-level
> +	 * flushing code keep track of what needs flushing.
> +	 *
> +	 * This is not used on Xen PV.
> +	 */
> +	atomic64_t tlb_gen;

Btw, can this just be a 4-byte int instead? I.e., simply atomic_t. I
mean, it should be enough for all the TLB generations in flight, no?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1672263 — Re: [PATCH v3 04/11] x86/mm: Give each mm TLB flush generation a unique ID

On Wed, Jun 21, 2017 at 10:43 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Jun 20, 2017 at 10:22:10PM -0700, Andy Lutomirski wrote:
>> - * The x86 doesn't have a mmu context, but
>> - * we put the segment information here.
>> + * x86 has arch-specific MMU state beyond what lives in mm_struct.
>>   */
>>  typedef struct {
>> +     /*
>> +      * ctx_id uniquely identifies this mm_struct.  A ctx_id will never
>> +      * be reused, and zero is not a valid ctx_id.
>> +      */
>> +     u64 ctx_id;
>> +
>> +     /*
>> +      * Any code that needs to do any sort of TLB flushing for this
>> +      * mm will first make its changes to the page tables, then
>> +      * increment tlb_gen, then flush.  This lets the low-level
>> +      * flushing code keep track of what needs flushing.
>> +      *
>> +      * This is not used on Xen PV.
>> +      */
>> +     atomic64_t tlb_gen;
>
> Btw, can this just be a 4-byte int instead? I.e., simply atomic_t. I
> mean, it should be enough for all the TLB generations in flight, no?

There can only be NR_CPUS generations that actually mean anything at
any given time, but I think they can be arbitrarily discontinuous.
Imagine a malicious program that does:

set affiinitiy to CPU 1
mmap()
set affinity to CPU 0
for (i = 0; i < (1ULL<<32); i++) {
  munmap();
  mmap();
}
set affinity to CPU 1

With just atomic_t, this could blow up.

[toc] | [prev] | [next] | [standalone]

#1671365 — [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

We can use PCID if the CPU has PCID and PGE and we're not on Xen.

By itself, this has no effect.  The next patch will start using
PCID.

Cc: Juergen Gross <jgross@suse.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/tlbflush.h |  8 ++++++++
 arch/x86/kernel/cpu/common.c    | 15 +++++++++++++++
 arch/x86/xen/enlighten_pv.c     |  6 ++++++
 3 files changed, 29 insertions(+)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 87b13e51e867..57b305e13c4c 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -243,6 +243,14 @@ static inline void __flush_tlb_all(void)
 		__flush_tlb_global();
 	else
 		__flush_tlb();
+
+	/*
+	 * Note: if we somehow had PCID but not PGE, then this wouldn't work --
+	 * we'd end up flushing kernel translations for the current ASID but
+	 * we might fail to flush kernel translations for other cached ASIDs.
+	 *
+	 * To avoid this issue, we force PCID off if PGE is off.
+	 */
 }
 
 static inline void __flush_tlb_one(unsigned long addr)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 904485e7b230..01caf66b270f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1143,6 +1143,21 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	setup_smep(c);
 	setup_smap(c);
 
+	/* Set up PCID */
+	if (cpu_has(c, X86_FEATURE_PCID)) {
+		if (cpu_has(c, X86_FEATURE_PGE)) {
+			cr4_set_bits(X86_CR4_PCIDE);
+		} else {
+			/*
+			 * flush_tlb_all(), as currently implemented, won't
+			 * work if PCID is on but PGE is not.  Since that
+			 * combination doesn't exist on real hardware, there's
+			 * no reason to try to fully support it.
+			 */
+			clear_cpu_cap(c, X86_FEATURE_PCID);
+		}
+	}
+
 	/*
 	 * The vendor-specific functions might have changed features.
 	 * Now we do "generic changes."
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index f33eef4ebd12..a136aac543c3 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -295,6 +295,12 @@ static void __init xen_init_capabilities(void)
 	setup_clear_cpu_cap(X86_FEATURE_ACC);
 	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 
+	/*
+	 * Xen PV would need some work to support PCID: CR3 handling as well
+	 * as xen_flush_tlb_others() would need updating.
+	 */
+	setup_clear_cpu_cap(X86_FEATURE_PCID);
+
 	if (!xen_initial_domain())
 		setup_clear_cpu_cap(X86_FEATURE_ACPI);
 
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1671562 — Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

On Tue, 20 Jun 2017, Andy Lutomirski wrote:
> +	/* Set up PCID */
> +	if (cpu_has(c, X86_FEATURE_PCID)) {
> +		if (cpu_has(c, X86_FEATURE_PGE)) {
> +			cr4_set_bits(X86_CR4_PCIDE);

So I assume that you made sure that the PCID bits in CR3 are zero under all
circumstances as setting PCIDE would cause a #GP if not.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[toc] | [prev] | [next] | [standalone]

#1671681 — Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

On Wed, 21 Jun 2017, Thomas Gleixner wrote:

> On Tue, 20 Jun 2017, Andy Lutomirski wrote:
> > +	/* Set up PCID */
> > +	if (cpu_has(c, X86_FEATURE_PCID)) {
> > +		if (cpu_has(c, X86_FEATURE_PGE)) {
> > +			cr4_set_bits(X86_CR4_PCIDE);
> 
> So I assume that you made sure that the PCID bits in CR3 are zero under all
> circumstances as setting PCIDE would cause a #GP if not.

And what happens on kexec etc? We need to reset the asid and PCIDE I assume.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1672014 — Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

On Wed, Jun 21, 2017 at 2:39 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 20 Jun 2017, Andy Lutomirski wrote:
>> +     /* Set up PCID */
>> +     if (cpu_has(c, X86_FEATURE_PCID)) {
>> +             if (cpu_has(c, X86_FEATURE_PGE)) {
>> +                     cr4_set_bits(X86_CR4_PCIDE);
>
> So I assume that you made sure that the PCID bits in CR3 are zero under all
> circumstances as setting PCIDE would cause a #GP if not.

Yes.  All existing code just shoves a PA of a page table in there.  As
far as I know, neither Linux nor anyone else uses the silly PCD and
PWT bits.  It's not even clear to me that they are functional if PAT
is enabled.

>
> And what happens on kexec etc? We need to reset the asid and PCIDE I assume.
>

I assume it works roughly the same way as suspend, etc --
mmu_cr4_features has the desired CR4 and the init code deals with it.
And PGE, PKE, etc all work correctly.  I'm not sure why PCIDE needs to
be cleared -- the init code will work just fine even if PCIDE is
unexpectedly set.

That being said, I haven't managed to understand what exactly the
kexec code is doing.  But I think the relevant bit is here in
relocate_kernel_64.S:

        /*
         * Set cr4 to a known state:
         *  - physical address extension enabled
         */
        movl    $X86_CR4_PAE, %eax
        movq    %rax, %cr4

Kexec folks, is it safe to assume that kexec can already deal with the
new and old kernels disagreeing on what CR4 should be?

[toc] | [prev] | [next] | [standalone]

#1673495 — Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

On Tue, Jun 20, 2017 at 10:22:16PM -0700, Andy Lutomirski wrote:
> We can use PCID if the CPU has PCID and PGE and we're not on Xen.
> 
> By itself, this has no effect.  The next patch will start using
> PCID.
> 
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/tlbflush.h |  8 ++++++++
>  arch/x86/kernel/cpu/common.c    | 15 +++++++++++++++
>  arch/x86/xen/enlighten_pv.c     |  6 ++++++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 87b13e51e867..57b305e13c4c 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -243,6 +243,14 @@ static inline void __flush_tlb_all(void)
>  		__flush_tlb_global();
>  	else
>  		__flush_tlb();
> +
> +	/*
> +	 * Note: if we somehow had PCID but not PGE, then this wouldn't work --
> +	 * we'd end up flushing kernel translations for the current ASID but
> +	 * we might fail to flush kernel translations for other cached ASIDs.
> +	 *
> +	 * To avoid this issue, we force PCID off if PGE is off.
> +	 */
>  }
>  
>  static inline void __flush_tlb_one(unsigned long addr)
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 904485e7b230..01caf66b270f 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1143,6 +1143,21 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  	setup_smep(c);
>  	setup_smap(c);
>  
> +	/* Set up PCID */
> +	if (cpu_has(c, X86_FEATURE_PCID)) {
> +		if (cpu_has(c, X86_FEATURE_PGE)) {

What are we protecting ourselves here against? Funny virtualization guests?

Because PGE should be ubiquitous by now. Or have you heard something?

> +			cr4_set_bits(X86_CR4_PCIDE);
> +		} else {
> +			/*
> +			 * flush_tlb_all(), as currently implemented, won't
> +			 * work if PCID is on but PGE is not.  Since that
> +			 * combination doesn't exist on real hardware, there's
> +			 * no reason to try to fully support it.
> +			 */
> +			clear_cpu_cap(c, X86_FEATURE_PCID);
> +		}
> +	}

This whole in setup_pcid() I guess, like the rest of the features.

> +
>  	/*
>  	 * The vendor-specific functions might have changed features.
>  	 * Now we do "generic changes."
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index f33eef4ebd12..a136aac543c3 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -295,6 +295,12 @@ static void __init xen_init_capabilities(void)
>  	setup_clear_cpu_cap(X86_FEATURE_ACC);
>  	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
>  
> +	/*
> +	 * Xen PV would need some work to support PCID: CR3 handling as well
> +	 * as xen_flush_tlb_others() would need updating.
> +	 */
> +	setup_clear_cpu_cap(X86_FEATURE_PCID);
> +
>  	if (!xen_initial_domain())
>  		setup_clear_cpu_cap(X86_FEATURE_ACPI);
>  
> -- 
> 2.9.4
> 

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1673637 — Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

On Fri, Jun 23, 2017 at 4:50 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Jun 20, 2017 at 10:22:16PM -0700, Andy Lutomirski wrote:
>> We can use PCID if the CPU has PCID and PGE and we're not on Xen.
>>
>> By itself, this has no effect.  The next patch will start using
>> PCID.
>>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/include/asm/tlbflush.h |  8 ++++++++
>>  arch/x86/kernel/cpu/common.c    | 15 +++++++++++++++
>>  arch/x86/xen/enlighten_pv.c     |  6 ++++++
>>  3 files changed, 29 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
>> index 87b13e51e867..57b305e13c4c 100644
>> --- a/arch/x86/include/asm/tlbflush.h
>> +++ b/arch/x86/include/asm/tlbflush.h
>> @@ -243,6 +243,14 @@ static inline void __flush_tlb_all(void)
>>               __flush_tlb_global();
>>       else
>>               __flush_tlb();
>> +
>> +     /*
>> +      * Note: if we somehow had PCID but not PGE, then this wouldn't work --
>> +      * we'd end up flushing kernel translations for the current ASID but
>> +      * we might fail to flush kernel translations for other cached ASIDs.
>> +      *
>> +      * To avoid this issue, we force PCID off if PGE is off.
>> +      */
>>  }
>>
>>  static inline void __flush_tlb_one(unsigned long addr)
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 904485e7b230..01caf66b270f 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1143,6 +1143,21 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>>       setup_smep(c);
>>       setup_smap(c);
>>
>> +     /* Set up PCID */
>> +     if (cpu_has(c, X86_FEATURE_PCID)) {
>> +             if (cpu_has(c, X86_FEATURE_PGE)) {
>
> What are we protecting ourselves here against? Funny virtualization guests?
>
> Because PGE should be ubiquitous by now. Or have you heard something?

Yes, funny VM guests.  I've been known to throw weird options at qemu
myself, and I prefer when the system works.  In this particular case,
I think the failure mode would be stale kernel TLB entries, and that
would be really annoying.

>
>> +                     cr4_set_bits(X86_CR4_PCIDE);
>> +             } else {
>> +                     /*
>> +                      * flush_tlb_all(), as currently implemented, won't
>> +                      * work if PCID is on but PGE is not.  Since that
>> +                      * combination doesn't exist on real hardware, there's
>> +                      * no reason to try to fully support it.
>> +                      */
>> +                     clear_cpu_cap(c, X86_FEATURE_PCID);
>> +             }
>> +     }
>
> This whole in setup_pcid() I guess, like the rest of the features.

Done.

[toc] | [prev] | [next] | [standalone]

#1673560 — Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index f33eef4ebd12..a136aac543c3 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -295,6 +295,12 @@ static void __init xen_init_capabilities(void)
>   	setup_clear_cpu_cap(X86_FEATURE_ACC);
>   	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
>   
> +	/*
> +	 * Xen PV would need some work to support PCID: CR3 handling as well
> +	 * as xen_flush_tlb_others() would need updating.
> +	 */
> +	setup_clear_cpu_cap(X86_FEATURE_PCID);
> +
>   	if (!xen_initial_domain())
>   		setup_clear_cpu_cap(X86_FEATURE_ACPI);
>   


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

[toc] | [prev] | [next] | [standalone]

#1671366 — [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code

Now that lazy TLB suppresses all flush IPIs (as opposed to all but
the first), there's no need to leave_mm() when going idle.

This means we can get rid of the rcuidle hack in
switch_mm_irqs_off() and we can unexport leave_mm().

This also removes acpi_unlazy_tlb() from the x86 and ia64 headers,
since it has no callers any more.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/ia64/include/asm/acpi.h  |  2 --
 arch/x86/include/asm/acpi.h   |  2 --
 arch/x86/mm/tlb.c             | 19 +++----------------
 drivers/acpi/processor_idle.c |  2 --
 drivers/idle/intel_idle.c     |  9 ++++-----
 5 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h
index a3d0211970e9..c86a947f5368 100644
--- a/arch/ia64/include/asm/acpi.h
+++ b/arch/ia64/include/asm/acpi.h
@@ -112,8 +112,6 @@ static inline void arch_acpi_set_pdc_bits(u32 *buf)
 	buf[2] |= ACPI_PDC_EST_CAPABILITY_SMP;
 }
 
-#define acpi_unlazy_tlb(x)
-
 #ifdef CONFIG_ACPI_NUMA
 extern cpumask_t early_cpu_possible_map;
 #define for_each_possible_early_cpu(cpu)  \
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 2efc768e4362..562286fa151f 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -150,8 +150,6 @@ static inline void disable_acpi(void) { }
 extern int x86_acpi_numa_init(void);
 #endif /* CONFIG_ACPI_NUMA */
 
-#define acpi_unlazy_tlb(x)	leave_mm(x)
-
 #ifdef CONFIG_ACPI_APEI
 static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 {
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index fea2b07ac7d8..5f932fd80881 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -50,7 +50,6 @@ void leave_mm(int cpu)
 
 	switch_mm(NULL, &init_mm, NULL);
 }
-EXPORT_SYMBOL_GPL(leave_mm);
 
 void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	       struct task_struct *tsk)
@@ -113,14 +112,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 			this_cpu_write(cpu_tlbstate.ctxs[0].tlb_gen,
 				       next_tlb_gen);
 			write_cr3(__pa(next->pgd));
-			/*
-			 * This gets called via leave_mm() in the idle path
-			 * where RCU functions differently.  Tracing normally
-			 * uses RCU, so we have to call the tracepoint
-			 * specially here.
-			 */
-			trace_tlb_flush_rcuidle(TLB_FLUSH_ON_TASK_SWITCH,
-						TLB_FLUSH_ALL);
+			trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH,
+					TLB_FLUSH_ALL);
 		}
 
 		/*
@@ -166,13 +159,7 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 		this_cpu_write(cpu_tlbstate.loaded_mm, next);
 		write_cr3(__pa(next->pgd));
 
-		/*
-		 * This gets called via leave_mm() in the idle path where RCU
-		 * functions differently.  Tracing normally uses RCU, so we
-		 * have to call the tracepoint specially here.
-		 */
-		trace_tlb_flush_rcuidle(TLB_FLUSH_ON_TASK_SWITCH,
-					TLB_FLUSH_ALL);
+		trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL);
 	}
 
 	load_mm_cr4(next);
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 5c8aa9cf62d7..fe3d2a40f311 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -708,8 +708,6 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
 static void acpi_idle_enter_bm(struct acpi_processor *pr,
 			       struct acpi_processor_cx *cx, bool timer_bc)
 {
-	acpi_unlazy_tlb(smp_processor_id());
-
 	/*
 	 * Must be done before busmaster disable as we might need to
 	 * access HPET !
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 216d7ec88c0c..2ae43f59091d 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -912,16 +912,15 @@ static __cpuidle int intel_idle(struct cpuidle_device *dev,
 	struct cpuidle_state *state = &drv->states[index];
 	unsigned long eax = flg2MWAIT(state->flags);
 	unsigned int cstate;
-	int cpu = smp_processor_id();
 
 	cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
 
 	/*
-	 * leave_mm() to avoid costly and often unnecessary wakeups
-	 * for flushing the user TLB's associated with the active mm.
+	 * NB: if CPUIDLE_FLAG_TLB_FLUSHED is set, this idle transition
+	 * will probably flush the TLB.  It's not guaranteed to flush
+	 * the TLB, though, so it's not clear that we can do anything
+	 * useful with this knowledge.
 	 */
-	if (state->flags & CPUIDLE_FLAG_TLB_FLUSHED)
-		leave_mm(cpu);
 
 	if (!(lapic_timer_reliable_states & (1 << (cstate))))
 		tick_broadcast_enter();
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1671540 — Re: [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code

On Tue, 20 Jun 2017, Andy Lutomirski wrote:
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 216d7ec88c0c..2ae43f59091d 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -912,16 +912,15 @@ static __cpuidle int intel_idle(struct cpuidle_device *dev,
>  	struct cpuidle_state *state = &drv->states[index];
>  	unsigned long eax = flg2MWAIT(state->flags);
>  	unsigned int cstate;
> -	int cpu = smp_processor_id();
>  
>  	cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
>  
>  	/*
> -	 * leave_mm() to avoid costly and often unnecessary wakeups
> -	 * for flushing the user TLB's associated with the active mm.
> +	 * NB: if CPUIDLE_FLAG_TLB_FLUSHED is set, this idle transition
> +	 * will probably flush the TLB.  It's not guaranteed to flush
> +	 * the TLB, though, so it's not clear that we can do anything
> +	 * useful with this knowledge.

So my understanding here is:

      The C-state transition might flush the TLB, when cstate->flags has
      CPUIDLE_FLAG_TLB_FLUSHED set. The idle transition already took the
      CPU out of the set of CPUs which are remotely flushed, so the
      knowledge about this potential flush is not useful for the kernels
      view of the TLB state.

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[toc] | [prev] | [next] | [standalone]

#1671767 — Re: [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code

On Wed, Jun 21, 2017 at 2:22 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 20 Jun 2017, Andy Lutomirski wrote:
>> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
>> index 216d7ec88c0c..2ae43f59091d 100644
>> --- a/drivers/idle/intel_idle.c
>> +++ b/drivers/idle/intel_idle.c
>> @@ -912,16 +912,15 @@ static __cpuidle int intel_idle(struct cpuidle_device *dev,
>>       struct cpuidle_state *state = &drv->states[index];
>>       unsigned long eax = flg2MWAIT(state->flags);
>>       unsigned int cstate;
>> -     int cpu = smp_processor_id();
>>
>>       cstate = (((eax) >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
>>
>>       /*
>> -      * leave_mm() to avoid costly and often unnecessary wakeups
>> -      * for flushing the user TLB's associated with the active mm.
>> +      * NB: if CPUIDLE_FLAG_TLB_FLUSHED is set, this idle transition
>> +      * will probably flush the TLB.  It's not guaranteed to flush
>> +      * the TLB, though, so it's not clear that we can do anything
>> +      * useful with this knowledge.
>
> So my understanding here is:
>
>       The C-state transition might flush the TLB, when cstate->flags has
>       CPUIDLE_FLAG_TLB_FLUSHED set. The idle transition already took the
>       CPU out of the set of CPUs which are remotely flushed, so the
>       knowledge about this potential flush is not useful for the kernels
>       view of the TLB state.

Indeed.  I assume the theory behind the old code was that leave_mm()
was expensive and that CPUIDLE_FLAG_TLB_FLUSHED would be a decent
heuristic for when to do it.

[toc] | [prev] | [next] | [standalone]

#1673413 — Re: [PATCH v3 07/11] x86/mm: Stop calling leave_mm() in idle code

On Tue, Jun 20, 2017 at 10:22:13PM -0700, Andy Lutomirski wrote:
> Now that lazy TLB suppresses all flush IPIs (as opposed to all but
> the first), there's no need to leave_mm() when going idle.
> 
> This means we can get rid of the rcuidle hack in
> switch_mm_irqs_off() and we can unexport leave_mm().
> 
> This also removes acpi_unlazy_tlb() from the x86 and ia64 headers,
> since it has no callers any more.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/ia64/include/asm/acpi.h  |  2 --
>  arch/x86/include/asm/acpi.h   |  2 --
>  arch/x86/mm/tlb.c             | 19 +++----------------
>  drivers/acpi/processor_idle.c |  2 --
>  drivers/idle/intel_idle.c     |  9 ++++-----
>  5 files changed, 7 insertions(+), 27 deletions(-)

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1671920

On Tue, Jun 20, 2017 at 10:22 PM, Andy Lutomirski <luto@kernel.org> wrote:
> There are three performance benefits here:

Side note: can you post the actual performance numbers, even if only
from some silly test program on just one platform? Things like lmbench
pipe benchmark or something?

Or maybe you did, and I just missed it. But when talking about
performance, I'd really like to always see some actual numbers.

                 Linus

[toc] | [prev] | [next] | [standalone]

#1672309

On Wed, Jun 21, 2017 at 11:23 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jun 20, 2017 at 10:22 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> There are three performance benefits here:
>
> Side note: can you post the actual performance numbers, even if only
> from some silly test program on just one platform? Things like lmbench
> pipe benchmark or something?
>
> Or maybe you did, and I just missed it. But when talking about
> performance, I'd really like to always see some actual numbers.

Here are some timings using KVM:

pingpong between two processes using eventfd:
patched: 883ns
unpatched: 1046ns (with considerably higher variance)

madvise(MADV_DONTNEED); write to the page; switch CPUs:
patched: ~12.5us
unpatched: 19us

The latter test is a somewhat contrived example to show off the
improved laziness.  Current kernels send an IPI on each iteration if
the system is otherwise idle.

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web