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

[Andy Lutomirski]

On Fri, Jun 23, 2017 at 4:50 AM, Borislav Petkov <bp@xxxxxxxxx> 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@xxxxxxxx>
>> Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
>> ---
>>  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.