x86/tlb: Disable interrupts when changing CR4
authorNadav Amit <namit@vmware.com>
Sat, 25 Nov 2017 03:29:07 +0000 (19:29 -0800)
committerThomas Gleixner <tglx@linutronix.de>
Sat, 25 Nov 2017 12:28:43 +0000 (13:28 +0100)
CR4 modifications are implemented as RMW operations which update a shadow
variable and write the result to CR4. The RMW operation is protected by
preemption disable, but there is no enforcement or debugging mechanism.

CR4 modifications happen also in interrupt context via
__native_flush_tlb_global(). This implementation does not affect a
interrupted thread context CR4 operation, because the CR4 toggle restores
the original content and does not modify the shadow variable.

So the current situation seems to be safe, but a recent patch tried to add
an actual RMW operation in interrupt context, which will cause subtle
corruptions.

To prevent that and make the CR4 handling future proof:

 - Add a lockdep assertion to __cr4_set() which will catch interrupt
   enabled invocations

 - Disable interrupts in the cr4 manipulator inlines

 - Rename cr4_toggle_bits() to cr4_toggle_bits_irqsoff(). This is called
   from __switch_to_xtra() where interrupts are already disabled and
   performance matters.

All other call sites are not performance critical, so the extra overhead of
an additional local_irq_save/restore() pair is not a problem. If new call
sites care about performance then the necessary _irqsoff() variants can be
added.

[ tglx: Condensed the patch by moving the irq protection inside the
   manipulator functions. Updated changelog ]

Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Luck <tony.luck@intel.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: nadav.amit@gmail.com
Cc: linux-edac@vger.kernel.org
Link: https://lkml.kernel.org/r/20171125032907.2241-3-namit@vmware.com
arch/x86/include/asm/tlbflush.h
arch/x86/kernel/process.c

index e736f7f0ba924397badf49bb92298ce51b1efeb0..877b5c1a1b1247116e20e7272dbade77e1874fc4 100644 (file)
@@ -175,6 +175,7 @@ static inline void cr4_init_shadow(void)
 
 static inline void __cr4_set(unsigned long cr4)
 {
+       lockdep_assert_irqs_disabled();
        this_cpu_write(cpu_tlbstate.cr4, cr4);
        __write_cr4(cr4);
 }
@@ -182,24 +183,28 @@ static inline void __cr4_set(unsigned long cr4)
 /* Set in this cpu's CR4. */
 static inline void cr4_set_bits(unsigned long mask)
 {
-       unsigned long cr4;
+       unsigned long cr4, flags;
 
+       local_irq_save(flags);
        cr4 = this_cpu_read(cpu_tlbstate.cr4);
        if ((cr4 | mask) != cr4)
                __cr4_set(cr4 | mask);
+       local_irq_restore(flags);
 }
 
 /* Clear in this cpu's CR4. */
 static inline void cr4_clear_bits(unsigned long mask)
 {
-       unsigned long cr4;
+       unsigned long cr4, flags;
 
+       local_irq_save(flags);
        cr4 = this_cpu_read(cpu_tlbstate.cr4);
        if ((cr4 & ~mask) != cr4)
                __cr4_set(cr4 & ~mask);
+       local_irq_restore(flags);
 }
 
-static inline void cr4_toggle_bits(unsigned long mask)
+static inline void cr4_toggle_bits_irqsoff(unsigned long mask)
 {
        unsigned long cr4;
 
index 97fb3e5737f5d0b5d50f8d9232726923c2692e65..bb988a24db927d758f9120d45f90d1c160628790 100644 (file)
@@ -299,7 +299,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
        }
 
        if ((tifp ^ tifn) & _TIF_NOTSC)
-               cr4_toggle_bits(X86_CR4_TSD);
+               cr4_toggle_bits_irqsoff(X86_CR4_TSD);
 
        if ((tifp ^ tifn) & _TIF_NOCPUID)
                set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));