Skip to contentSkip to conversation

x86/fpu: Remove CR0.TS support

PATCH In review Andy Lutomirski 4 participants 23 messages Oct 31, 2016 – Nov 1, 2016

Subsystems (7)
x86 KVM Crypto API KVM x86 +3 more subsystems
lore
Share & tools
lore
#0 lore raw

cover x86/fpu: Remove CR0.TS support

We don't do anything useful with CR0.TS anymore, so remove most of our support for it.

I havne't meaningfully tested lguest because I can't get it to work even without these patches.

Andy Lutomirski (8):
fpu/init: Get rid of two redundant clts() calls
fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs() x86/fpu: Remove irq_ts_save() and irq_ts_restore() x86/kvm: Remove host CR0.TS manipulation
lguest: Remove CR0.TS support
x86/fpu: #NM without FPU emulation is an error
x86/fpu: Remove stts()
x86/fpu: Remove clts()

 arch/x86/include/asm/fpu/api.h        | 10 ----------
 arch/x86/include/asm/lguest_hcall.h   |  1 -
 arch/x86/include/asm/paravirt.h       |  5 -----
 arch/x86/include/asm/paravirt_types.h |  2 --
 arch/x86/include/asm/special_insns.h  | 13 -------------
 arch/x86/kernel/fpu/bugs.c            |  7 -------
 arch/x86/kernel/fpu/core.c            | 29 -----------------------------
 arch/x86/kernel/fpu/init.c            | 18 ------------------
 arch/x86/kernel/paravirt.c            |  1 -
 arch/x86/kernel/paravirt_patch_32.c   |  2 --
 arch/x86/kernel/paravirt_patch_64.c   |  2 --
 arch/x86/kernel/traps.c               | 20 ++++++++++++++++----
 arch/x86/kvm/vmx.c                    | 12 ++++--------
 arch/x86/kvm/x86.c                    |  5 -----
 arch/x86/lguest/boot.c                | 29 +++++++----------------------
 arch/x86/xen/enlighten.c              | 13 -------------
 drivers/char/hw_random/via-rng.c      |  8 ++------
 drivers/crypto/padlock-aes.c          | 23 ++---------------------
 drivers/crypto/padlock-sha.c          | 18 ------------------
 drivers/lguest/hypercalls.c           |  4 ----
 drivers/lguest/lg.h                   |  1 -
 drivers/lguest/x86/core.c             | 19 +------------------
 22 files changed, 32 insertions(+), 210 deletions(-)
Signature · 1 line
2.7.4
#3 lore raw

PATCH 2/8 fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs()

fpu__init_check_bugs() runs long after the early FPU init, so CR0.TS will be clear by the time it runs. The save-and-restore dance would have been unnecessary anyway, though, as kernel_fpu_begin() would have been good enough.

---
 arch/x86/kernel/fpu/bugs.c | 7 -------
 1 file changed, 7 deletions(-)
arch/x86/kernel/fpu/bugs.c +0 -7
index aad34aafc0e0..d913047f832c 100644
@@ -23,17 +23,12 @@ static double __initdata y = 3145727.0;
  */
 void __init fpu__init_check_bugs(void)
 {
-	u32 cr0_saved;
 	s32 fdiv_bug;
 
 	/* kernel_fpu_begin/end() relies on patched alternative instructions. */
 	if (!boot_cpu_has(X86_FEATURE_FPU))
 		return;
 
-	/* We might have CR0::TS set already, clear it: */
-	cr0_saved = read_cr0();
-	write_cr0(cr0_saved & ~X86_CR0_TS);
-
 	kernel_fpu_begin();
 
 	/*
@@ -56,8 +51,6 @@ void __init fpu__init_check_bugs(void)
 
 	kernel_fpu_end();
 
-	write_cr0(cr0_saved);
-
 	if (fdiv_bug) {
 		set_cpu_bug(&boot_cpu_data, X86_BUG_FDIV);
 		pr_warn("Hmm, FPU with FDIV bug\n");
Signature · 1 line
2.7.4
#5 lore raw

PATCH 3/8 x86/fpu: Remove irq_ts_save() and irq_ts_restore()

In reply to Andy Lutomirski

Now that lazy FPU is gone, we don't use CR0.TS (except possibly in KVM guest mode). Remove irq_ts_save(), irq_ts_restore(), and all of their callers.

---
 arch/x86/include/asm/fpu/api.h   | 10 ----------
 arch/x86/kernel/fpu/core.c       | 29 -----------------------------
 drivers/char/hw_random/via-rng.c |  8 ++------
 drivers/crypto/padlock-aes.c     | 23 ++---------------------
 drivers/crypto/padlock-sha.c     | 18 ------------------
 5 files changed, 4 insertions(+), 84 deletions(-)
arch/x86/include/asm/fpu/api.h +0 -10
index 1429a7c736db..0877ae018fc9 100644
@@ -27,16 +27,6 @@ extern void kernel_fpu_end(void);
 extern bool irq_fpu_usable(void);
 
 /*
- * Some instructions like VIA's padlock instructions generate a spurious
- * DNA fault but don't modify SSE registers. And these instructions
- * get used from interrupt context as well. To prevent these kernel instructions
- * in interrupt context interacting wrongly with other user/kernel fpu usage, we
- * should use them only in the context of irq_ts_save/restore()
- */
-extern int  irq_ts_save(void);
-extern void irq_ts_restore(int TS_state);
-
-/*
  * Query the presence of one or more xfeatures. Works on any legacy CPU as well.
  *
  * If 'feature_name' is set then put a human-readable description of
arch/x86/kernel/fpu/core.c +0 -29
index 52f5684405c1..7d8e2628e82c 100644
@@ -138,35 +138,6 @@ void kernel_fpu_end(void)
 EXPORT_SYMBOL_GPL(kernel_fpu_end);
 
 /*
- * CR0::TS save/restore functions:
- */
-int irq_ts_save(void)
-{
-	/*
-	 * If in process context and not atomic, we can take a spurious DNA fault.
-	 * Otherwise, doing clts() in process context requires disabling preemption
-	 * or some heavy lifting like kernel_fpu_begin()
-	 */
-	if (!in_atomic())
-		return 0;
-
-	if (read_cr0() & X86_CR0_TS) {
-		clts();
-		return 1;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(irq_ts_save);
-
-void irq_ts_restore(int TS_state)
-{
-	if (TS_state)
-		stts();
-}
-EXPORT_SYMBOL_GPL(irq_ts_restore);
-
-/*
  * Save the FPU state (mark it for reload if necessary):
  *
  * This only ever gets called for the current task.
drivers/char/hw_random/via-rng.c +2 -6
index 44ce80606944..d1f5bb534e0e 100644
@@ -70,21 +70,17 @@ enum {
  * until we have 4 bytes, thus returning a u32 at a time,
  * instead of the current u8-at-a-time.
  *
- * Padlock instructions can generate a spurious DNA fault, so
- * we have to call them in the context of irq_ts_save/restore()
+ * Padlock instructions can generate a spurious DNA fault, but the
+ * kernel doesn't use CR0.TS, so this doesn't matter.
  */
 
 static inline u32 xstore(u32 *addr, u32 edx_in)
 {
 	u32 eax_out;
-	int ts_state;
-
-	ts_state = irq_ts_save();
 
 	asm(".byte 0x0F,0xA7,0xC0 /* xstore %%edi (addr=%0) */"
 		: "=m" (*addr), "=a" (eax_out), "+d" (edx_in), "+D" (addr));
 
-	irq_ts_restore(ts_state);
 	return eax_out;
 }
 
drivers/crypto/padlock-aes.c +2 -21
index 441e86b23571..b3869748cc6b 100644
@@ -183,8 +183,8 @@ static inline void padlock_store_cword(struct cword *cword)
 
 /*
  * While the padlock instructions don't use FP/SSE registers, they
- * generate a spurious DNA fault when cr0.ts is '1'. These instructions
- * should be used only inside the irq_ts_save/restore() context
+ * generate a spurious DNA fault when CR0.TS is '1'.  Fortunately,
+ * the kernel doesn't use CR0.TS.
  */
 
 static inline void rep_xcrypt_ecb(const u8 *input, u8 *output, void *key,
@@ -298,24 +298,18 @@ static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key,
 static void aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
 {
 	struct aes_ctx *ctx = aes_ctx(tfm);
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
-	ts_state = irq_ts_save();
 	ecb_crypt(in, out, ctx->E, &ctx->cword.encrypt, 1);
-	irq_ts_restore(ts_state);
 	padlock_store_cword(&ctx->cword.encrypt);
 }
 
 static void aes_decrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
 {
 	struct aes_ctx *ctx = aes_ctx(tfm);
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
-	ts_state = irq_ts_save();
 	ecb_crypt(in, out, ctx->D, &ctx->cword.decrypt, 1);
-	irq_ts_restore(ts_state);
 	padlock_store_cword(&ctx->cword.encrypt);
 }
 
@@ -346,14 +340,12 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->E, &ctx->cword.encrypt,
@@ -361,7 +353,6 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
 		nbytes &= AES_BLOCK_SIZE - 1;
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
-	irq_ts_restore(ts_state);
 
 	padlock_store_cword(&ctx->cword.encrypt);
 
@@ -375,14 +366,12 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.decrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->D, &ctx->cword.decrypt,
@@ -390,7 +379,6 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
 		nbytes &= AES_BLOCK_SIZE - 1;
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
-	irq_ts_restore(ts_state);
 
 	padlock_store_cword(&ctx->cword.encrypt);
 
@@ -425,14 +413,12 @@ static int cbc_aes_encrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		u8 *iv = padlock_xcrypt_cbc(walk.src.virt.addr,
 					    walk.dst.virt.addr, ctx->E,
@@ -442,7 +428,6 @@ static int cbc_aes_encrypt(struct blkcipher_desc *desc,
 		nbytes &= AES_BLOCK_SIZE - 1;
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
-	irq_ts_restore(ts_state);
 
 	padlock_store_cword(&ctx->cword.decrypt);
 
@@ -456,14 +441,12 @@ static int cbc_aes_decrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		padlock_xcrypt_cbc(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->D, walk.iv, &ctx->cword.decrypt,
@@ -472,8 +455,6 @@ static int cbc_aes_decrypt(struct blkcipher_desc *desc,
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
 
-	irq_ts_restore(ts_state);
-
 	padlock_store_cword(&ctx->cword.encrypt);
 
 	return err;
drivers/crypto/padlock-sha.c +0 -18
index 8c5f90647b7a..bc72d20c32c3 100644
@@ -89,7 +89,6 @@ static int padlock_sha1_finup(struct shash_desc *desc, const u8 *in,
 	struct sha1_state state;
 	unsigned int space;
 	unsigned int leftover;
-	int ts_state;
 	int err;
 
 	dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
@@ -120,14 +119,11 @@ static int padlock_sha1_finup(struct shash_desc *desc, const u8 *in,
 
 	memcpy(result, &state.state, SHA1_DIGEST_SIZE);
 
-	/* prevent taking the spurious DNA fault with padlock. */
-	ts_state = irq_ts_save();
 	asm volatile (".byte 0xf3,0x0f,0xa6,0xc8" /* rep xsha1 */
 		      : \
 		      : "c"((unsigned long)state.count + count), \
 			"a"((unsigned long)state.count), \
 			"S"(in), "D"(result));
-	irq_ts_restore(ts_state);
 
 	padlock_output_block((uint32_t *)result, (uint32_t *)out, 5);
 
@@ -155,7 +151,6 @@ static int padlock_sha256_finup(struct shash_desc *desc, const u8 *in,
 	struct sha256_state state;
 	unsigned int space;
 	unsigned int leftover;
-	int ts_state;
 	int err;
 
 	dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
@@ -186,14 +181,11 @@ static int padlock_sha256_finup(struct shash_desc *desc, const u8 *in,
 
 	memcpy(result, &state.state, SHA256_DIGEST_SIZE);
 
-	/* prevent taking the spurious DNA fault with padlock. */
-	ts_state = irq_ts_save();
 	asm volatile (".byte 0xf3,0x0f,0xa6,0xd0" /* rep xsha256 */
 		      : \
 		      : "c"((unsigned long)state.count + count), \
 			"a"((unsigned long)state.count), \
 			"S"(in), "D"(result));
-	irq_ts_restore(ts_state);
 
 	padlock_output_block((uint32_t *)result, (uint32_t *)out, 8);
 
@@ -312,7 +304,6 @@ static int padlock_sha1_update_nano(struct shash_desc *desc,
 	u8 buf[128 + PADLOCK_ALIGNMENT - STACK_ALIGN] __attribute__
 		((aligned(STACK_ALIGN)));
 	u8 *dst = PTR_ALIGN(&buf[0], PADLOCK_ALIGNMENT);
-	int ts_state;
 
 	partial = sctx->count & 0x3f;
 	sctx->count += len;
@@ -328,23 +319,19 @@ static int padlock_sha1_update_nano(struct shash_desc *desc,
 			memcpy(sctx->buffer + partial, data,
 				done + SHA1_BLOCK_SIZE);
 			src = sctx->buffer;
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xc8"
 			: "+S"(src), "+D"(dst) \
 			: "a"((long)-1), "c"((unsigned long)1));
-			irq_ts_restore(ts_state);
 			done += SHA1_BLOCK_SIZE;
 			src = data + done;
 		}
 
 		/* Process the left bytes from the input data */
 		if (len - done >= SHA1_BLOCK_SIZE) {
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xc8"
 			: "+S"(src), "+D"(dst)
 			: "a"((long)-1),
 			"c"((unsigned long)((len - done) / SHA1_BLOCK_SIZE)));
-			irq_ts_restore(ts_state);
 			done += ((len - done) - (len - done) % SHA1_BLOCK_SIZE);
 			src = data + done;
 		}
@@ -401,7 +388,6 @@ static int padlock_sha256_update_nano(struct shash_desc *desc, const u8 *data,
 	u8 buf[128 + PADLOCK_ALIGNMENT - STACK_ALIGN] __attribute__
 		((aligned(STACK_ALIGN)));
 	u8 *dst = PTR_ALIGN(&buf[0], PADLOCK_ALIGNMENT);
-	int ts_state;
 
 	partial = sctx->count & 0x3f;
 	sctx->count += len;
@@ -417,23 +403,19 @@ static int padlock_sha256_update_nano(struct shash_desc *desc, const u8 *data,
 			memcpy(sctx->buf + partial, data,
 				done + SHA256_BLOCK_SIZE);
 			src = sctx->buf;
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xd0"
 			: "+S"(src), "+D"(dst)
 			: "a"((long)-1), "c"((unsigned long)1));
-			irq_ts_restore(ts_state);
 			done += SHA256_BLOCK_SIZE;
 			src = data + done;
 		}
 
 		/* Process the left bytes from input data*/
 		if (len - done >= SHA256_BLOCK_SIZE) {
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xd0"
 			: "+S"(src), "+D"(dst)
 			: "a"((long)-1),
 			"c"((unsigned long)((len - done) / 64)));
-			irq_ts_restore(ts_state);
 			done += ((len - done) - (len - done) % 64);
 			src = data + done;
 		}
Signature · 1 line
2.7.4
#7 lore raw

PATCH 4/8 x86/kvm: Remove host CR0.TS manipulation

In reply to Andy Lutomirski

Now that x86 always uses eager fpu switching on the host, there's no need for KVM to manipulate the host's CR0.TS.

This should be both simpler and faster.

---
 arch/x86/kvm/vmx.c | 12 ++++--------
 arch/x86/kvm/x86.c |  5 -----
 2 files changed, 4 insertions(+), 13 deletions(-)
arch/x86/kvm/vmx.c +4 -8
index cf1b16dbc98a..531c44633190 100644
@@ -2144,12 +2144,6 @@ static void __vmx_load_host_state(struct vcpu_vmx *vmx)
 #endif
 	if (vmx->host_state.msr_host_bndcfgs)
 		wrmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
-	/*
-	 * If the FPU is not active (through the host task or
-	 * the guest vcpu), then restore the cr0.TS bit.
-	 */
-	if (!fpregs_active() && !vmx->vcpu.guest_fpu_loaded)
-		stts();
 	load_gdt(this_cpu_ptr(&host_gdt));
 }
 
@@ -4871,9 +4865,11 @@ static void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
 	u32 low32, high32;
 	unsigned long tmpl;
 	struct desc_ptr dt;
-	unsigned long cr4;
+	unsigned long cr0, cr4;
 
-	vmcs_writel(HOST_CR0, read_cr0() & ~X86_CR0_TS);  /* 22.2.3 */
+	cr0 = read_cr0();
+	WARN_ON(cr0 & X86_CR0_TS);
+	vmcs_writel(HOST_CR0, cr0);  /* 22.2.3 */
 	vmcs_writel(HOST_CR3, read_cr3());  /* 22.2.3  FIXME: shadow tables */
 
 	/* Save the most likely value for this task's CR4 in the VMCS. */
arch/x86/kvm/x86.c +0 -5
index d5700263dad2..123d428d319a 100644
@@ -5060,11 +5060,6 @@ static void emulator_get_fpu(struct x86_emulate_ctxt *ctxt)
 {
 	preempt_disable();
 	kvm_load_guest_fpu(emul_to_vcpu(ctxt));
-	/*
-	 * CR0.TS may reference the host fpu state, not the guest fpu state,
-	 * so it may be clear at this point.
-	 */
-	clts();
 }
 
 static void emulator_put_fpu(struct x86_emulate_ctxt *ctxt)
Signature · 1 line
2.7.4
#9 lore raw

PATCH 5/8 lguest: Remove CR0.TS support

In reply to Andy Lutomirski

Now that Linux never sets CR0.TS, lguest doesn't need to support it.

---
 arch/x86/include/asm/lguest_hcall.h |  1 -
 arch/x86/lguest/boot.c              | 17 +++++++----------
 drivers/lguest/hypercalls.c         |  4 ----
 drivers/lguest/lg.h                 |  1 -
 drivers/lguest/x86/core.c           | 19 +------------------
 5 files changed, 8 insertions(+), 34 deletions(-)
arch/x86/include/asm/lguest_hcall.h +0 -1
index ef01fef3eebc..6c119cfae218 100644
@@ -9,7 +9,6 @@
 #define LHCALL_FLUSH_TLB	5
 #define LHCALL_LOAD_IDT_ENTRY	6
 #define LHCALL_SET_STACK	7
-#define LHCALL_TS		8
 #define LHCALL_SET_CLOCKEVENT	9
 #define LHCALL_HALT		10
 #define LHCALL_SET_PMD		13
arch/x86/lguest/boot.c +7 -10
index 25da5bc8d83d..d74afcdbc580 100644
@@ -497,27 +497,24 @@ static void lguest_cpuid(unsigned int *ax, unsigned int *bx,
  * a whole series of functions like read_cr0() and write_cr0().
  *
  * We start with cr0.  cr0 allows you to turn on and off all kinds of basic
- * features, but Linux only really cares about one: the horrifically-named Task
- * Switched (TS) bit at bit 3 (ie. 8)
+ * features, but the only cr0 bit that Linux ever used at runtime was the
+ * horrifically-named Task Switched (TS) bit at bit 3 (ie. 8)
  *
  * What does the TS bit do?  Well, it causes the CPU to trap (interrupt 7) if
  * the floating point unit is used.  Which allows us to restore FPU state
- * lazily after a task switch, and Linux uses that gratefully, but wouldn't a
- * name like "FPUTRAP bit" be a little less cryptic?
+ * lazily after a task switch if we wanted to, but wouldn't a name like
+ * "FPUTRAP bit" be a little less cryptic?
  *
- * We store cr0 locally because the Host never changes it.  The Guest sometimes
- * wants to read it and we'd prefer not to bother the Host unnecessarily.
+ * Fortunately, Linux keeps it simple and doesn't use TS, so we can ignore
+ * cr0.
  */
-static unsigned long current_cr0;
 static void lguest_write_cr0(unsigned long val)
 {
-	lazy_hcall1(LHCALL_TS, val & X86_CR0_TS);
-	current_cr0 = val;
 }
 
 static unsigned long lguest_read_cr0(void)
 {
-	return current_cr0;
+	return 0;
 }
 
 /*
drivers/lguest/hypercalls.c +0 -4
index 19a32280731d..601f81c04873 100644
@@ -109,10 +109,6 @@ static void do_hcall(struct lg_cpu *cpu, struct hcall_args *args)
 	case LHCALL_SET_CLOCKEVENT:
 		guest_set_clockevent(cpu, args->arg1);
 		break;
-	case LHCALL_TS:
-		/* This sets the TS flag, as we saw used in run_guest(). */
-		cpu->ts = args->arg1;
-		break;
 	case LHCALL_HALT:
 		/* Similarly, this sets the halted flag for run_guest(). */
 		cpu->halted = 1;
drivers/lguest/lg.h +0 -1
index 69b3814afd2f..2356a2318034 100644
@@ -43,7 +43,6 @@ struct lg_cpu {
 	struct mm_struct *mm; 	/* == tsk->mm, but that becomes NULL on exit */
 
 	u32 cr2;
-	int ts;
 	u32 esp1;
 	u16 ss1;
 
drivers/lguest/x86/core.c +1 -18
index 6e9042e3d2a9..743253fc638f 100644
@@ -247,14 +247,6 @@ unsigned long *lguest_arch_regptr(struct lg_cpu *cpu, size_t reg_off, bool any)
 void lguest_arch_run_guest(struct lg_cpu *cpu)
 {
 	/*
-	 * Remember the awfully-named TS bit?  If the Guest has asked to set it
-	 * we set it now, so we can trap and pass that trap to the Guest if it
-	 * uses the FPU.
-	 */
-	if (cpu->ts && fpregs_active())
-		stts();
-
-	/*
 	 * SYSENTER is an optimized way of doing system calls.  We can't allow
 	 * it because it always jumps to privilege level 0.  A normal Guest
 	 * won't try it because we don't advertise it in CPUID, but a malicious
@@ -282,10 +274,6 @@ void lguest_arch_run_guest(struct lg_cpu *cpu)
 	 if (boot_cpu_has(X86_FEATURE_SEP))
 		wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
 
-	/* Clear the host TS bit if it was set above. */
-	if (cpu->ts && fpregs_active())
-		clts();
-
 	/*
 	 * If the Guest page faulted, then the cr2 register will tell us the
 	 * bad virtual address.  We have to grab this now, because once we
@@ -421,12 +409,7 @@ void lguest_arch_handle_trap(struct lg_cpu *cpu)
 			kill_guest(cpu, "Writing cr2");
 		break;
 	case 7: /* We've intercepted a Device Not Available fault. */
-		/*
-		 * If the Guest doesn't want to know, we already restored the
-		 * Floating Point Unit, so we just continue without telling it.
-		 */
-		if (!cpu->ts)
-			return;
+		/* No special handling is needed here. */
 		break;
 	case 32 ... 255:
 		/* This might be a syscall. */
Signature · 1 line
2.7.4
#13 lore raw

PATCH 7/8 x86/fpu: Remove stts()

In reply to Andy Lutomirski

It has no callers any more, and it was always a bit confusing, as there is no STTS instruction.

---
 arch/x86/include/asm/special_insns.h | 2 --
 1 file changed, 2 deletions(-)
arch/x86/include/asm/special_insns.h +0 -2
index 19a2224f9e16..29b417bd8a68 100644
@@ -216,8 +216,6 @@ static inline void clts(void)
 
 #endif/* CONFIG_PARAVIRT */
 
-#define stts() write_cr0(read_cr0() | X86_CR0_TS)
-
 static inline void clflush(volatile void *__p)
 {
 	asm volatile("clflush %0" : "+m" (*(volatile char __force *)__p));
Signature · 1 line
2.7.4
#15 lore raw

PATCH 8/8 x86/fpu: Remove clts()

In reply to Andy Lutomirski

The kernel doesn't use clts() any more. Remove it and all of its paravirt infrastructure.

A careful reader may notice that xen_clts() appears to have been buggy -- it didn't update xen_cr0_value.

---
 arch/x86/include/asm/paravirt.h       |  5 -----
 arch/x86/include/asm/paravirt_types.h |  2 --
 arch/x86/include/asm/special_insns.h  | 11 -----------
 arch/x86/kernel/paravirt.c            |  1 -
 arch/x86/kernel/paravirt_patch_32.c   |  2 --
 arch/x86/kernel/paravirt_patch_64.c   |  2 --
 arch/x86/lguest/boot.c                | 12 ------------
 arch/x86/xen/enlighten.c              | 13 -------------
 8 files changed, 48 deletions(-)
arch/x86/include/asm/paravirt.h +0 -5
index ce932812f142..f1fb4dbe9a3e 100644
@@ -41,11 +41,6 @@ static inline void set_debugreg(unsigned long val, int reg)
 	PVOP_VCALL2(pv_cpu_ops.set_debugreg, reg, val);
 }
 
-static inline void clts(void)
-{
-	PVOP_VCALL0(pv_cpu_ops.clts);
-}
-
 static inline unsigned long read_cr0(void)
 {
 	return PVOP_CALL0(unsigned long, pv_cpu_ops.read_cr0);
arch/x86/include/asm/paravirt_types.h +0 -2
index 0f400c0e4979..545426aa61ef 100644
@@ -103,8 +103,6 @@ struct pv_cpu_ops {
 	unsigned long (*get_debugreg)(int regno);
 	void (*set_debugreg)(int regno, unsigned long value);
 
-	void (*clts)(void);
-
 	unsigned long (*read_cr0)(void);
 	void (*write_cr0)(unsigned long);
 
arch/x86/include/asm/special_insns.h +0 -11
index 29b417bd8a68..12af3e35edfa 100644
@@ -6,11 +6,6 @@
 
 #include <asm/nops.h>
 
-static inline void native_clts(void)
-{
-	asm volatile("clts");
-}
-
 /*
  * Volatile isn't enough to prevent the compiler from reordering the
  * read/write functions for the control registers and messing everything up.
@@ -208,12 +203,6 @@ static inline void load_gs_index(unsigned selector)
 
 #endif
 
-/* Clear the 'TS' bit */
-static inline void clts(void)
-{
-	native_clts();
-}
-
 #endif/* CONFIG_PARAVIRT */
 
 static inline void clflush(volatile void *__p)
arch/x86/kernel/paravirt.c +0 -1
index bbf3d5933eaa..a1bfba0f7234 100644
@@ -328,7 +328,6 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.cpuid = native_cpuid,
 	.get_debugreg = native_get_debugreg,
 	.set_debugreg = native_set_debugreg,
-	.clts = native_clts,
 	.read_cr0 = native_read_cr0,
 	.write_cr0 = native_write_cr0,
 	.read_cr4 = native_read_cr4,
arch/x86/kernel/paravirt_patch_32.c +0 -2
index 920c6ae08592..d3f7f14bb328 100644
@@ -8,7 +8,6 @@ DEF_NATIVE(pv_cpu_ops, iret, "iret");
 DEF_NATIVE(pv_mmu_ops, read_cr2, "mov %cr2, %eax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "mov %eax, %cr3");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "mov %cr3, %eax");
-DEF_NATIVE(pv_cpu_ops, clts, "clts");
 
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
 DEF_NATIVE(pv_lock_ops, queued_spin_unlock, "movb $0, (%eax)");
@@ -48,7 +47,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 		PATCH_SITE(pv_mmu_ops, read_cr2);
 		PATCH_SITE(pv_mmu_ops, read_cr3);
 		PATCH_SITE(pv_mmu_ops, write_cr3);
-		PATCH_SITE(pv_cpu_ops, clts);
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
 		case PARAVIRT_PATCH(pv_lock_ops.queued_spin_unlock):
 			if (pv_is_native_spin_unlock()) {
arch/x86/kernel/paravirt_patch_64.c +0 -2
index bb3840cedb4f..915a4c0b217c 100644
@@ -10,7 +10,6 @@ DEF_NATIVE(pv_mmu_ops, read_cr2, "movq %cr2, %rax");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "movq %cr3, %rax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "movq %rdi, %cr3");
 DEF_NATIVE(pv_mmu_ops, flush_tlb_single, "invlpg (%rdi)");
-DEF_NATIVE(pv_cpu_ops, clts, "clts");
 DEF_NATIVE(pv_cpu_ops, wbinvd, "wbinvd");
 
 DEF_NATIVE(pv_cpu_ops, usergs_sysret64, "swapgs; sysretq");
@@ -58,7 +57,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 		PATCH_SITE(pv_mmu_ops, read_cr2);
 		PATCH_SITE(pv_mmu_ops, read_cr3);
 		PATCH_SITE(pv_mmu_ops, write_cr3);
-		PATCH_SITE(pv_cpu_ops, clts);
 		PATCH_SITE(pv_mmu_ops, flush_tlb_single);
 		PATCH_SITE(pv_cpu_ops, wbinvd);
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
arch/x86/lguest/boot.c +0 -12
index d74afcdbc580..4ca0d78adcf0 100644
@@ -518,17 +518,6 @@ static unsigned long lguest_read_cr0(void)
 }
 
 /*
- * Intel provided a special instruction to clear the TS bit for people too cool
- * to use write_cr0() to do it.  This "clts" instruction is faster, because all
- * the vowels have been optimized out.
- */
-static void lguest_clts(void)
-{
-	lazy_hcall1(LHCALL_TS, 0);
-	current_cr0 &= ~X86_CR0_TS;
-}
-
-/*
  * cr2 is the virtual address of the last page fault, which the Guest only ever
  * reads.  The Host kindly writes this into our "struct lguest_data", so we
  * just read it out of there.
@@ -1429,7 +1418,6 @@ __init void lguest_init(void)
 	pv_cpu_ops.load_tls = lguest_load_tls;
 	pv_cpu_ops.get_debugreg = lguest_get_debugreg;
 	pv_cpu_ops.set_debugreg = lguest_set_debugreg;
-	pv_cpu_ops.clts = lguest_clts;
 	pv_cpu_ops.read_cr0 = lguest_read_cr0;
 	pv_cpu_ops.write_cr0 = lguest_write_cr0;
 	pv_cpu_ops.read_cr4 = lguest_read_cr4;
arch/x86/xen/enlighten.c +0 -13
index c0fdd57da7aa..69f22f7e80ed 100644
@@ -980,17 +980,6 @@ static void xen_io_delay(void)
 {
 }
 
-static void xen_clts(void)
-{
-	struct multicall_space mcs;
-
-	mcs = xen_mc_entry(0);
-
-	MULTI_fpu_taskswitch(mcs.mc, 0);
-
-	xen_mc_issue(PARAVIRT_LAZY_CPU);
-}
-
 static DEFINE_PER_CPU(unsigned long, xen_cr0_value);
 
 static unsigned long xen_read_cr0(void)
@@ -1233,8 +1222,6 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 	.set_debugreg = xen_set_debugreg,
 	.get_debugreg = xen_get_debugreg,
 
-	.clts = xen_clts,
-
 	.read_cr0 = xen_read_cr0,
 	.write_cr0 = xen_write_cr0,
 
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/8 fpu/init: Get rid of two redundant clts() calls

In reply to Andy Lutomirski

CR0.TS is cleared by a direct CR0 write in fpu__init_cpu_generic(). We don't need to call clts() two more times right after that.

---
 arch/x86/kernel/fpu/init.c | 18 ------------------
 1 file changed, 18 deletions(-)
arch/x86/kernel/fpu/init.c +0 -18
index 1a09d133c801..60dece392b3a 100644
@@ -10,15 +10,6 @@
 #include <linux/init.h>
 
 /*
- * Initialize the TS bit in CR0 according to the style of context-switches
- * we are using:
- */
-static void fpu__init_cpu_ctx_switch(void)
-{
-	clts();
-}
-
-/*
  * Initialize the registers found in all CPUs, CR0 and CR4:
  */
 static void fpu__init_cpu_generic(void)
@@ -55,7 +46,6 @@ void fpu__init_cpu(void)
 {
 	fpu__init_cpu_generic();
 	fpu__init_cpu_xstate();
-	fpu__init_cpu_ctx_switch();
 }
 
 /*
@@ -290,14 +280,6 @@ void __init fpu__init_system(struct cpuinfo_x86 *c)
 	 */
 	fpu__init_cpu();
 
-	/*
-	 * But don't leave CR0::TS set yet, as some of the FPU setup
-	 * methods depend on being able to execute FPU instructions
-	 * that will fault on a set TS, such as the FXSAVE in
-	 * fpu__init_system_mxcsr().
-	 */
-	clts();
-
 	fpu__init_system_generic();
 	fpu__init_system_xstate_size_legacy();
 	fpu__init_system_xstate();
Signature · 1 line
2.7.4
#11 lore raw

PATCH 6/8 x86/fpu: #NM without FPU emulation is an error

In reply to Andy Lutomirski

don't use CR0.TS. Make it an error rather than making nonsensical changes to the FPU state.

(The cond_local_irq_enable() appears to have been pointless, too.)

---
 arch/x86/kernel/traps.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
arch/x86/kernel/traps.c +16 -4
index bd4e3d4d3625..bf0c6d049080 100644
@@ -853,6 +853,8 @@ do_spurious_interrupt_bug(struct pt_regs *regs, long error_code)
 dotraplinkage void
 do_device_not_available(struct pt_regs *regs, long error_code)
 {
+	unsigned long cr0;
+
 	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
 
 #ifdef CONFIG_MATH_EMULATION
@@ -866,10 +868,20 @@ do_device_not_available(struct pt_regs *regs, long error_code)
 		return;
 	}
 #endif
-	fpu__restore(&current->thread.fpu); /* interrupts still off */
-#ifdef CONFIG_X86_32
-	cond_local_irq_enable(regs);
-#endif
+
+	/* This should not happen. */
+	cr0 = read_cr0();
+	if (WARN(cr0 & X86_CR0_TS, "CR0.TS was set")) {
+		/* Try to fix it up and carry on. */
+		write_cr0(cr0 & ~X86_CR0_TS);
+	} else {
+		/*
+		 * Something terrible happened, and we're better off trying
+		 * to kill the task than getting stuck in a never-ending
+		 * loop of #NM faults.
+		 */
+		die("unexpected #NM exception", regs, error_code);
+	}
 }
 NOKPROBE_SYMBOL(do_device_not_available);
 
Signature · 1 line
2.7.4
#17 lore raw
In reply to Andy Lutomirski
Quoted 2 lines · Andy Lutomirski

I havne't meaningfully tested lguest because I can't get it to work even without these patches.

Have you disabled CONFIG_OLPC in the .config for the guest kernel? Otherwise you will run into
    lguest: Reinjecting trap 13 for fault at 0x1000062: Invalid argument

A similar obscure gotcha (I think it is "unhandled trap 13") can be avoided by launching the guest with the dis_ucode_ldr kernel option.

Hope this helps,

Paul Bolle

maintainer
#18 lore raw
Quoted 2 lines · Paul Bolle

A similar obscure gotcha (I think it is "unhandled trap 13") can be avoided by launching the guest with the dis_ucode_ldr kernel option.

					  ^^^^^^^^^^^^^

Huh, what does that have to do with disabling the microcode loader?

Funny...

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#19 lore raw
Quoted 3 lines · Borislav Petkov

Huh, what does that have to do with disabling the microcode loader?

Funny...

See https://lists.ozlabs.org/pipermail/lguest/2013-May/002001.html . Nobody cared enough to fix it. I cared enough to figure it all out. But I didn't understand much of the possible solutions that where suggested three years ago.

I'm certain I still ran into this issue with the microcode loader in the beginning of this year, so that was probably with a v4.4 based guest.

Paul Bolle

maintainer
#20 lore raw
Quoted 4 lines · Paul Bolle

See https://lists.ozlabs.org/pipermail/lguest/2013-May/002001.html . Nobody cared enough to fix it. I cared enough to figure it all out. But I didn't understand much of the possible solutions that where suggested three years ago.

I'm guessing the simple fix would be this:

---

drivers/lguest/Kconfig +1 -1
index 169172d2ba05..9c08b3050bb7 100644
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EVENTFD && TTY && PCI_DIRECT
+	depends on X86_32 && EVENTFD && TTY && PCI_DIRECT && !MICROCODE
 	select HVC_DRIVER
 	---help---
 	  This is a very simple module which allows you to run

---

but maybe the better fix is to hack in MSR emulation in lguest and intercept the *MSR accesses and do the writes/reads in the exception fixup and ...

I haven't looked at the lguest code, of course and whether that's easily doable and whether it even makes sense and whether one should simply use qemu/kvm instead and, and, and...

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#2 lore raw

[tip:x86/fpu] x86/fpu: Get rid of two redundant clts() calls

In reply to Andy Lutomirski
x86/fpu: Get rid of two redundant clts() calls
Commit-ID:  36fd4f0249f8cb445835acb7c6937a0ffa2b5f14
Gitweb:     http://git.kernel.org/tip/36fd4f0249f8cb445835acb7c6937a0ffa2b5f14
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:42 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:53 +0100

x86/fpu: Get rid of two redundant clts() calls

CR0.TS is cleared by a direct CR0 write in fpu__init_cpu_generic(). We don't need to call clts() two more times right after that.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/kernel/fpu/init.c | 18 ------------------
 1 file changed, 18 deletions(-)
arch/x86/kernel/fpu/init.c +0 -18
index 1a09d13..60dece3 100644
@@ -10,15 +10,6 @@
 #include <linux/init.h>
 
 /*
- * Initialize the TS bit in CR0 according to the style of context-switches
- * we are using:
- */
-static void fpu__init_cpu_ctx_switch(void)
-{
-	clts();
-}
-
-/*
  * Initialize the registers found in all CPUs, CR0 and CR4:
  */
 static void fpu__init_cpu_generic(void)
@@ -55,7 +46,6 @@ void fpu__init_cpu(void)
 {
 	fpu__init_cpu_generic();
 	fpu__init_cpu_xstate();
-	fpu__init_cpu_ctx_switch();
 }
 
 /*
@@ -290,14 +280,6 @@ void __init fpu__init_system(struct cpuinfo_x86 *c)
 	 */
 	fpu__init_cpu();
 
-	/*
-	 * But don't leave CR0::TS set yet, as some of the FPU setup
-	 * methods depend on being able to execute FPU instructions
-	 * that will fault on a set TS, such as the FXSAVE in
-	 * fpu__init_system_mxcsr().
-	 */
-	clts();
-
 	fpu__init_system_generic();
 	fpu__init_system_xstate_size_legacy();
 	fpu__init_system_xstate();
#4 lore raw

[tip:x86/fpu] x86/fpu: Stop saving and restoring CR0.TS in fpu__init_check_bugs()

In reply to Andy Lutomirski
x86/fpu: Stop saving and restoring CR0.TS in fpu__init_check_bugs()
Commit-ID:  fc560a80bac91e512fc37cdfe03a982ef4543c6b
Gitweb:     http://git.kernel.org/tip/fc560a80bac91e512fc37cdfe03a982ef4543c6b
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:43 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:53 +0100

x86/fpu: Stop saving and restoring CR0.TS in fpu__init_check_bugs()

fpu__init_check_bugs() runs long after the early FPU init, so CR0.TS will be clear by the time it runs. The save-and-restore dance would have been unnecessary anyway, though, as kernel_fpu_begin() would have been good enough.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/kernel/fpu/bugs.c | 7 -------
 1 file changed, 7 deletions(-)
arch/x86/kernel/fpu/bugs.c +0 -7
index aad34aa..d913047 100644
@@ -23,17 +23,12 @@ static double __initdata y = 3145727.0;
  */
 void __init fpu__init_check_bugs(void)
 {
-	u32 cr0_saved;
 	s32 fdiv_bug;
 
 	/* kernel_fpu_begin/end() relies on patched alternative instructions. */
 	if (!boot_cpu_has(X86_FEATURE_FPU))
 		return;
 
-	/* We might have CR0::TS set already, clear it: */
-	cr0_saved = read_cr0();
-	write_cr0(cr0_saved & ~X86_CR0_TS);
-
 	kernel_fpu_begin();
 
 	/*
@@ -56,8 +51,6 @@ void __init fpu__init_check_bugs(void)
 
 	kernel_fpu_end();
 
-	write_cr0(cr0_saved);
-
 	if (fdiv_bug) {
 		set_cpu_bug(&boot_cpu_data, X86_BUG_FDIV);
 		pr_warn("Hmm, FPU with FDIV bug\n");
#6 lore raw

[tip:x86/fpu] x86/fpu: Remove irq_ts_save() and irq_ts_restore()

In reply to Andy Lutomirski
x86/fpu: Remove irq_ts_save() and irq_ts_restore()
Commit-ID:  5a83d60c074ddf4f6364be25654a643d0e941824
Gitweb:     http://git.kernel.org/tip/5a83d60c074ddf4f6364be25654a643d0e941824
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:44 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:54 +0100

x86/fpu: Remove irq_ts_save() and irq_ts_restore()

Now that lazy FPU is gone, we don't use CR0.TS (except possibly in KVM guest mode). Remove irq_ts_save(), irq_ts_restore(), and all of their callers.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/include/asm/fpu/api.h   | 10 ----------
 arch/x86/kernel/fpu/core.c       | 29 -----------------------------
 drivers/char/hw_random/via-rng.c |  8 ++------
 drivers/crypto/padlock-aes.c     | 23 ++---------------------
 drivers/crypto/padlock-sha.c     | 18 ------------------
 5 files changed, 4 insertions(+), 84 deletions(-)
arch/x86/include/asm/fpu/api.h +0 -10
index 1429a7c..0877ae0 100644
@@ -27,16 +27,6 @@ extern void kernel_fpu_end(void);
 extern bool irq_fpu_usable(void);
 
 /*
- * Some instructions like VIA's padlock instructions generate a spurious
- * DNA fault but don't modify SSE registers. And these instructions
- * get used from interrupt context as well. To prevent these kernel instructions
- * in interrupt context interacting wrongly with other user/kernel fpu usage, we
- * should use them only in the context of irq_ts_save/restore()
- */
-extern int  irq_ts_save(void);
-extern void irq_ts_restore(int TS_state);
-
-/*
  * Query the presence of one or more xfeatures. Works on any legacy CPU as well.
  *
  * If 'feature_name' is set then put a human-readable description of
arch/x86/kernel/fpu/core.c +0 -29
index 52f56844..7d8e262 100644
@@ -138,35 +138,6 @@ void kernel_fpu_end(void)
 EXPORT_SYMBOL_GPL(kernel_fpu_end);
 
 /*
- * CR0::TS save/restore functions:
- */
-int irq_ts_save(void)
-{
-	/*
-	 * If in process context and not atomic, we can take a spurious DNA fault.
-	 * Otherwise, doing clts() in process context requires disabling preemption
-	 * or some heavy lifting like kernel_fpu_begin()
-	 */
-	if (!in_atomic())
-		return 0;
-
-	if (read_cr0() & X86_CR0_TS) {
-		clts();
-		return 1;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(irq_ts_save);
-
-void irq_ts_restore(int TS_state)
-{
-	if (TS_state)
-		stts();
-}
-EXPORT_SYMBOL_GPL(irq_ts_restore);
-
-/*
  * Save the FPU state (mark it for reload if necessary):
  *
  * This only ever gets called for the current task.
drivers/char/hw_random/via-rng.c +2 -6
index 44ce806..d1f5bb5 100644
@@ -70,21 +70,17 @@ enum {
  * until we have 4 bytes, thus returning a u32 at a time,
  * instead of the current u8-at-a-time.
  *
- * Padlock instructions can generate a spurious DNA fault, so
- * we have to call them in the context of irq_ts_save/restore()
+ * Padlock instructions can generate a spurious DNA fault, but the
+ * kernel doesn't use CR0.TS, so this doesn't matter.
  */
 
 static inline u32 xstore(u32 *addr, u32 edx_in)
 {
 	u32 eax_out;
-	int ts_state;
-
-	ts_state = irq_ts_save();
 
 	asm(".byte 0x0F,0xA7,0xC0 /* xstore %%edi (addr=%0) */"
 		: "=m" (*addr), "=a" (eax_out), "+d" (edx_in), "+D" (addr));
 
-	irq_ts_restore(ts_state);
 	return eax_out;
 }
 
drivers/crypto/padlock-aes.c +2 -21
index 441e86b..b386974 100644
@@ -183,8 +183,8 @@ static inline void padlock_store_cword(struct cword *cword)
 
 /*
  * While the padlock instructions don't use FP/SSE registers, they
- * generate a spurious DNA fault when cr0.ts is '1'. These instructions
- * should be used only inside the irq_ts_save/restore() context
+ * generate a spurious DNA fault when CR0.TS is '1'.  Fortunately,
+ * the kernel doesn't use CR0.TS.
  */
 
 static inline void rep_xcrypt_ecb(const u8 *input, u8 *output, void *key,
@@ -298,24 +298,18 @@ static inline u8 *padlock_xcrypt_cbc(const u8 *input, u8 *output, void *key,
 static void aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
 {
 	struct aes_ctx *ctx = aes_ctx(tfm);
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
-	ts_state = irq_ts_save();
 	ecb_crypt(in, out, ctx->E, &ctx->cword.encrypt, 1);
-	irq_ts_restore(ts_state);
 	padlock_store_cword(&ctx->cword.encrypt);
 }
 
 static void aes_decrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
 {
 	struct aes_ctx *ctx = aes_ctx(tfm);
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
-	ts_state = irq_ts_save();
 	ecb_crypt(in, out, ctx->D, &ctx->cword.decrypt, 1);
-	irq_ts_restore(ts_state);
 	padlock_store_cword(&ctx->cword.encrypt);
 }
 
@@ -346,14 +340,12 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->E, &ctx->cword.encrypt,
@@ -361,7 +353,6 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
 		nbytes &= AES_BLOCK_SIZE - 1;
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
-	irq_ts_restore(ts_state);
 
 	padlock_store_cword(&ctx->cword.encrypt);
 
@@ -375,14 +366,12 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.decrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->D, &ctx->cword.decrypt,
@@ -390,7 +379,6 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
 		nbytes &= AES_BLOCK_SIZE - 1;
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
-	irq_ts_restore(ts_state);
 
 	padlock_store_cword(&ctx->cword.encrypt);
 
@@ -425,14 +413,12 @@ static int cbc_aes_encrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		u8 *iv = padlock_xcrypt_cbc(walk.src.virt.addr,
 					    walk.dst.virt.addr, ctx->E,
@@ -442,7 +428,6 @@ static int cbc_aes_encrypt(struct blkcipher_desc *desc,
 		nbytes &= AES_BLOCK_SIZE - 1;
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
-	irq_ts_restore(ts_state);
 
 	padlock_store_cword(&ctx->cword.decrypt);
 
@@ -456,14 +441,12 @@ static int cbc_aes_decrypt(struct blkcipher_desc *desc,
 	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
 	struct blkcipher_walk walk;
 	int err;
-	int ts_state;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
 	blkcipher_walk_init(&walk, dst, src, nbytes);
 	err = blkcipher_walk_virt(desc, &walk);
 
-	ts_state = irq_ts_save();
 	while ((nbytes = walk.nbytes)) {
 		padlock_xcrypt_cbc(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->D, walk.iv, &ctx->cword.decrypt,
@@ -472,8 +455,6 @@ static int cbc_aes_decrypt(struct blkcipher_desc *desc,
 		err = blkcipher_walk_done(desc, &walk, nbytes);
 	}
 
-	irq_ts_restore(ts_state);
-
 	padlock_store_cword(&ctx->cword.encrypt);
 
 	return err;
drivers/crypto/padlock-sha.c +0 -18
index 8c5f906..bc72d20 100644
@@ -89,7 +89,6 @@ static int padlock_sha1_finup(struct shash_desc *desc, const u8 *in,
 	struct sha1_state state;
 	unsigned int space;
 	unsigned int leftover;
-	int ts_state;
 	int err;
 
 	dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
@@ -120,14 +119,11 @@ static int padlock_sha1_finup(struct shash_desc *desc, const u8 *in,
 
 	memcpy(result, &state.state, SHA1_DIGEST_SIZE);
 
-	/* prevent taking the spurious DNA fault with padlock. */
-	ts_state = irq_ts_save();
 	asm volatile (".byte 0xf3,0x0f,0xa6,0xc8" /* rep xsha1 */
 		      : \
 		      : "c"((unsigned long)state.count + count), \
 			"a"((unsigned long)state.count), \
 			"S"(in), "D"(result));
-	irq_ts_restore(ts_state);
 
 	padlock_output_block((uint32_t *)result, (uint32_t *)out, 5);
 
@@ -155,7 +151,6 @@ static int padlock_sha256_finup(struct shash_desc *desc, const u8 *in,
 	struct sha256_state state;
 	unsigned int space;
 	unsigned int leftover;
-	int ts_state;
 	int err;
 
 	dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
@@ -186,14 +181,11 @@ static int padlock_sha256_finup(struct shash_desc *desc, const u8 *in,
 
 	memcpy(result, &state.state, SHA256_DIGEST_SIZE);
 
-	/* prevent taking the spurious DNA fault with padlock. */
-	ts_state = irq_ts_save();
 	asm volatile (".byte 0xf3,0x0f,0xa6,0xd0" /* rep xsha256 */
 		      : \
 		      : "c"((unsigned long)state.count + count), \
 			"a"((unsigned long)state.count), \
 			"S"(in), "D"(result));
-	irq_ts_restore(ts_state);
 
 	padlock_output_block((uint32_t *)result, (uint32_t *)out, 8);
 
@@ -312,7 +304,6 @@ static int padlock_sha1_update_nano(struct shash_desc *desc,
 	u8 buf[128 + PADLOCK_ALIGNMENT - STACK_ALIGN] __attribute__
 		((aligned(STACK_ALIGN)));
 	u8 *dst = PTR_ALIGN(&buf[0], PADLOCK_ALIGNMENT);
-	int ts_state;
 
 	partial = sctx->count & 0x3f;
 	sctx->count += len;
@@ -328,23 +319,19 @@ static int padlock_sha1_update_nano(struct shash_desc *desc,
 			memcpy(sctx->buffer + partial, data,
 				done + SHA1_BLOCK_SIZE);
 			src = sctx->buffer;
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xc8"
 			: "+S"(src), "+D"(dst) \
 			: "a"((long)-1), "c"((unsigned long)1));
-			irq_ts_restore(ts_state);
 			done += SHA1_BLOCK_SIZE;
 			src = data + done;
 		}
 
 		/* Process the left bytes from the input data */
 		if (len - done >= SHA1_BLOCK_SIZE) {
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xc8"
 			: "+S"(src), "+D"(dst)
 			: "a"((long)-1),
 			"c"((unsigned long)((len - done) / SHA1_BLOCK_SIZE)));
-			irq_ts_restore(ts_state);
 			done += ((len - done) - (len - done) % SHA1_BLOCK_SIZE);
 			src = data + done;
 		}
@@ -401,7 +388,6 @@ static int padlock_sha256_update_nano(struct shash_desc *desc, const u8 *data,
 	u8 buf[128 + PADLOCK_ALIGNMENT - STACK_ALIGN] __attribute__
 		((aligned(STACK_ALIGN)));
 	u8 *dst = PTR_ALIGN(&buf[0], PADLOCK_ALIGNMENT);
-	int ts_state;
 
 	partial = sctx->count & 0x3f;
 	sctx->count += len;
@@ -417,23 +403,19 @@ static int padlock_sha256_update_nano(struct shash_desc *desc, const u8 *data,
 			memcpy(sctx->buf + partial, data,
 				done + SHA256_BLOCK_SIZE);
 			src = sctx->buf;
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xd0"
 			: "+S"(src), "+D"(dst)
 			: "a"((long)-1), "c"((unsigned long)1));
-			irq_ts_restore(ts_state);
 			done += SHA256_BLOCK_SIZE;
 			src = data + done;
 		}
 
 		/* Process the left bytes from input data*/
 		if (len - done >= SHA256_BLOCK_SIZE) {
-			ts_state = irq_ts_save();
 			asm volatile (".byte 0xf3,0x0f,0xa6,0xd0"
 			: "+S"(src), "+D"(dst)
 			: "a"((long)-1),
 			"c"((unsigned long)((len - done) / 64)));
-			irq_ts_restore(ts_state);
 			done += ((len - done) - (len - done) % 64);
 			src = data + done;
 		}
#10 lore raw

[tip:x86/fpu] x86/fpu, lguest: Remove CR0.TS support

In reply to Andy Lutomirski
x86/fpu, lguest: Remove CR0.TS support
Commit-ID:  cd95ea81f25608c403052d0508ee5c9b32e2bc7d
Gitweb:     http://git.kernel.org/tip/cd95ea81f25608c403052d0508ee5c9b32e2bc7d
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:46 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:54 +0100

x86/fpu, lguest: Remove CR0.TS support

Now that Linux never sets CR0.TS, lguest doesn't need to support it.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/include/asm/lguest_hcall.h |  1 -
 arch/x86/lguest/boot.c              | 17 +++++++----------
 drivers/lguest/hypercalls.c         |  4 ----
 drivers/lguest/lg.h                 |  1 -
 drivers/lguest/x86/core.c           | 19 +------------------
 5 files changed, 8 insertions(+), 34 deletions(-)
arch/x86/include/asm/lguest_hcall.h +0 -1
index ef01fef..6c119cf 100644
@@ -9,7 +9,6 @@
 #define LHCALL_FLUSH_TLB	5
 #define LHCALL_LOAD_IDT_ENTRY	6
 #define LHCALL_SET_STACK	7
-#define LHCALL_TS		8
 #define LHCALL_SET_CLOCKEVENT	9
 #define LHCALL_HALT		10
 #define LHCALL_SET_PMD		13
arch/x86/lguest/boot.c +7 -10
index 25da5bc8..d74afcd 100644
@@ -497,27 +497,24 @@ static void lguest_cpuid(unsigned int *ax, unsigned int *bx,
  * a whole series of functions like read_cr0() and write_cr0().
  *
  * We start with cr0.  cr0 allows you to turn on and off all kinds of basic
- * features, but Linux only really cares about one: the horrifically-named Task
- * Switched (TS) bit at bit 3 (ie. 8)
+ * features, but the only cr0 bit that Linux ever used at runtime was the
+ * horrifically-named Task Switched (TS) bit at bit 3 (ie. 8)
  *
  * What does the TS bit do?  Well, it causes the CPU to trap (interrupt 7) if
  * the floating point unit is used.  Which allows us to restore FPU state
- * lazily after a task switch, and Linux uses that gratefully, but wouldn't a
- * name like "FPUTRAP bit" be a little less cryptic?
+ * lazily after a task switch if we wanted to, but wouldn't a name like
+ * "FPUTRAP bit" be a little less cryptic?
  *
- * We store cr0 locally because the Host never changes it.  The Guest sometimes
- * wants to read it and we'd prefer not to bother the Host unnecessarily.
+ * Fortunately, Linux keeps it simple and doesn't use TS, so we can ignore
+ * cr0.
  */
-static unsigned long current_cr0;
 static void lguest_write_cr0(unsigned long val)
 {
-	lazy_hcall1(LHCALL_TS, val & X86_CR0_TS);
-	current_cr0 = val;
 }
 
 static unsigned long lguest_read_cr0(void)
 {
-	return current_cr0;
+	return 0;
 }
 
 /*
drivers/lguest/hypercalls.c +0 -4
index 19a3228..601f81c 100644
@@ -109,10 +109,6 @@ static void do_hcall(struct lg_cpu *cpu, struct hcall_args *args)
 	case LHCALL_SET_CLOCKEVENT:
 		guest_set_clockevent(cpu, args->arg1);
 		break;
-	case LHCALL_TS:
-		/* This sets the TS flag, as we saw used in run_guest(). */
-		cpu->ts = args->arg1;
-		break;
 	case LHCALL_HALT:
 		/* Similarly, this sets the halted flag for run_guest(). */
 		cpu->halted = 1;
drivers/lguest/lg.h +0 -1
index 69b3814..2356a23 100644
@@ -43,7 +43,6 @@ struct lg_cpu {
 	struct mm_struct *mm; 	/* == tsk->mm, but that becomes NULL on exit */
 
 	u32 cr2;
-	int ts;
 	u32 esp1;
 	u16 ss1;
 
drivers/lguest/x86/core.c +1 -18
index 6e9042e..743253f 100644
@@ -247,14 +247,6 @@ unsigned long *lguest_arch_regptr(struct lg_cpu *cpu, size_t reg_off, bool any)
 void lguest_arch_run_guest(struct lg_cpu *cpu)
 {
 	/*
-	 * Remember the awfully-named TS bit?  If the Guest has asked to set it
-	 * we set it now, so we can trap and pass that trap to the Guest if it
-	 * uses the FPU.
-	 */
-	if (cpu->ts && fpregs_active())
-		stts();
-
-	/*
 	 * SYSENTER is an optimized way of doing system calls.  We can't allow
 	 * it because it always jumps to privilege level 0.  A normal Guest
 	 * won't try it because we don't advertise it in CPUID, but a malicious
@@ -282,10 +274,6 @@ void lguest_arch_run_guest(struct lg_cpu *cpu)
 	 if (boot_cpu_has(X86_FEATURE_SEP))
 		wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
 
-	/* Clear the host TS bit if it was set above. */
-	if (cpu->ts && fpregs_active())
-		clts();
-
 	/*
 	 * If the Guest page faulted, then the cr2 register will tell us the
 	 * bad virtual address.  We have to grab this now, because once we
@@ -421,12 +409,7 @@ void lguest_arch_handle_trap(struct lg_cpu *cpu)
 			kill_guest(cpu, "Writing cr2");
 		break;
 	case 7: /* We've intercepted a Device Not Available fault. */
-		/*
-		 * If the Guest doesn't want to know, we already restored the
-		 * Floating Point Unit, so we just continue without telling it.
-		 */
-		if (!cpu->ts)
-			return;
+		/* No special handling is needed here. */
 		break;
 	case 32 ... 255:
 		/* This might be a syscall. */
#14 lore raw

[tip:x86/fpu] x86/fpu: Remove stts()

In reply to Andy Lutomirski
x86/fpu: Remove stts()
Commit-ID:  0d50612c041f213fb6b98e3ff06e306a859c36f2
Gitweb:     http://git.kernel.org/tip/0d50612c041f213fb6b98e3ff06e306a859c36f2
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:48 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:55 +0100

x86/fpu: Remove stts()

It has no callers any more, and it was always a bit confusing, as there is no STTS instruction.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/include/asm/special_insns.h | 2 --
 1 file changed, 2 deletions(-)
arch/x86/include/asm/special_insns.h +0 -2
index 19a2224..29b417b 100644
@@ -216,8 +216,6 @@ static inline void clts(void)
 
 #endif/* CONFIG_PARAVIRT */
 
-#define stts() write_cr0(read_cr0() | X86_CR0_TS)
-
 static inline void clflush(volatile void *__p)
 {
 	asm volatile("clflush %0" : "+m" (*(volatile char __force *)__p));
#16 lore raw

[tip:x86/fpu] x86/fpu: Remove clts()

In reply to Andy Lutomirski
x86/fpu: Remove clts()
Commit-ID:  af25ed59b5616b389d90877f7085dc5d457a3d49
Gitweb:     http://git.kernel.org/tip/af25ed59b5616b389d90877f7085dc5d457a3d49
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:49 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:55 +0100

x86/fpu: Remove clts()

The kernel doesn't use clts() any more. Remove it and all of its paravirt infrastructure.

A careful reader may notice that xen_clts() appears to have been buggy -- it didn't update xen_cr0_value.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/include/asm/paravirt.h       |  5 -----
 arch/x86/include/asm/paravirt_types.h |  2 --
 arch/x86/include/asm/special_insns.h  | 11 -----------
 arch/x86/kernel/paravirt.c            |  1 -
 arch/x86/kernel/paravirt_patch_32.c   |  2 --
 arch/x86/kernel/paravirt_patch_64.c   |  2 --
 arch/x86/lguest/boot.c                | 12 ------------
 arch/x86/xen/enlighten.c              | 13 -------------
 8 files changed, 48 deletions(-)
arch/x86/include/asm/paravirt.h +0 -5
index ce93281..f1fb4db 100644
@@ -41,11 +41,6 @@ static inline void set_debugreg(unsigned long val, int reg)
 	PVOP_VCALL2(pv_cpu_ops.set_debugreg, reg, val);
 }
 
-static inline void clts(void)
-{
-	PVOP_VCALL0(pv_cpu_ops.clts);
-}
-
 static inline unsigned long read_cr0(void)
 {
 	return PVOP_CALL0(unsigned long, pv_cpu_ops.read_cr0);
arch/x86/include/asm/paravirt_types.h +0 -2
index 0f400c0..545426a 100644
@@ -103,8 +103,6 @@ struct pv_cpu_ops {
 	unsigned long (*get_debugreg)(int regno);
 	void (*set_debugreg)(int regno, unsigned long value);
 
-	void (*clts)(void);
-
 	unsigned long (*read_cr0)(void);
 	void (*write_cr0)(unsigned long);
 
arch/x86/include/asm/special_insns.h +0 -11
index 29b417b..12af3e3 100644
@@ -6,11 +6,6 @@
 
 #include <asm/nops.h>
 
-static inline void native_clts(void)
-{
-	asm volatile("clts");
-}
-
 /*
  * Volatile isn't enough to prevent the compiler from reordering the
  * read/write functions for the control registers and messing everything up.
@@ -208,12 +203,6 @@ static inline void load_gs_index(unsigned selector)
 
 #endif
 
-/* Clear the 'TS' bit */
-static inline void clts(void)
-{
-	native_clts();
-}
-
 #endif/* CONFIG_PARAVIRT */
 
 static inline void clflush(volatile void *__p)
arch/x86/kernel/paravirt.c +0 -1
index bbf3d59..a1bfba0 100644
@@ -328,7 +328,6 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.cpuid = native_cpuid,
 	.get_debugreg = native_get_debugreg,
 	.set_debugreg = native_set_debugreg,
-	.clts = native_clts,
 	.read_cr0 = native_read_cr0,
 	.write_cr0 = native_write_cr0,
 	.read_cr4 = native_read_cr4,
arch/x86/kernel/paravirt_patch_32.c +0 -2
index 920c6ae..d3f7f14 100644
@@ -8,7 +8,6 @@ DEF_NATIVE(pv_cpu_ops, iret, "iret");
 DEF_NATIVE(pv_mmu_ops, read_cr2, "mov %cr2, %eax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "mov %eax, %cr3");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "mov %cr3, %eax");
-DEF_NATIVE(pv_cpu_ops, clts, "clts");
 
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
 DEF_NATIVE(pv_lock_ops, queued_spin_unlock, "movb $0, (%eax)");
@@ -48,7 +47,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 		PATCH_SITE(pv_mmu_ops, read_cr2);
 		PATCH_SITE(pv_mmu_ops, read_cr3);
 		PATCH_SITE(pv_mmu_ops, write_cr3);
-		PATCH_SITE(pv_cpu_ops, clts);
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
 		case PARAVIRT_PATCH(pv_lock_ops.queued_spin_unlock):
 			if (pv_is_native_spin_unlock()) {
arch/x86/kernel/paravirt_patch_64.c +0 -2
index bb3840c..915a4c0 100644
@@ -10,7 +10,6 @@ DEF_NATIVE(pv_mmu_ops, read_cr2, "movq %cr2, %rax");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "movq %cr3, %rax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "movq %rdi, %cr3");
 DEF_NATIVE(pv_mmu_ops, flush_tlb_single, "invlpg (%rdi)");
-DEF_NATIVE(pv_cpu_ops, clts, "clts");
 DEF_NATIVE(pv_cpu_ops, wbinvd, "wbinvd");
 
 DEF_NATIVE(pv_cpu_ops, usergs_sysret64, "swapgs; sysretq");
@@ -58,7 +57,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 		PATCH_SITE(pv_mmu_ops, read_cr2);
 		PATCH_SITE(pv_mmu_ops, read_cr3);
 		PATCH_SITE(pv_mmu_ops, write_cr3);
-		PATCH_SITE(pv_cpu_ops, clts);
 		PATCH_SITE(pv_mmu_ops, flush_tlb_single);
 		PATCH_SITE(pv_cpu_ops, wbinvd);
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
arch/x86/lguest/boot.c +0 -12
index d74afcd..4ca0d78 100644
@@ -518,17 +518,6 @@ static unsigned long lguest_read_cr0(void)
 }
 
 /*
- * Intel provided a special instruction to clear the TS bit for people too cool
- * to use write_cr0() to do it.  This "clts" instruction is faster, because all
- * the vowels have been optimized out.
- */
-static void lguest_clts(void)
-{
-	lazy_hcall1(LHCALL_TS, 0);
-	current_cr0 &= ~X86_CR0_TS;
-}
-
-/*
  * cr2 is the virtual address of the last page fault, which the Guest only ever
  * reads.  The Host kindly writes this into our "struct lguest_data", so we
  * just read it out of there.
@@ -1429,7 +1418,6 @@ __init void lguest_init(void)
 	pv_cpu_ops.load_tls = lguest_load_tls;
 	pv_cpu_ops.get_debugreg = lguest_get_debugreg;
 	pv_cpu_ops.set_debugreg = lguest_set_debugreg;
-	pv_cpu_ops.clts = lguest_clts;
 	pv_cpu_ops.read_cr0 = lguest_read_cr0;
 	pv_cpu_ops.write_cr0 = lguest_write_cr0;
 	pv_cpu_ops.read_cr4 = lguest_read_cr4;
arch/x86/xen/enlighten.c +0 -13
index bdd8556..ced7027 100644
@@ -980,17 +980,6 @@ static void xen_io_delay(void)
 {
 }
 
-static void xen_clts(void)
-{
-	struct multicall_space mcs;
-
-	mcs = xen_mc_entry(0);
-
-	MULTI_fpu_taskswitch(mcs.mc, 0);
-
-	xen_mc_issue(PARAVIRT_LAZY_CPU);
-}
-
 static DEFINE_PER_CPU(unsigned long, xen_cr0_value);
 
 static unsigned long xen_read_cr0(void)
@@ -1233,8 +1222,6 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 	.set_debugreg = xen_set_debugreg,
 	.get_debugreg = xen_get_debugreg,
 
-	.clts = xen_clts,
-
 	.read_cr0 = xen_read_cr0,
 	.write_cr0 = xen_write_cr0,
 
#12 lore raw

[tip:x86/fpu] x86/fpu: Handle #NM without FPU emulation as an error

In reply to Andy Lutomirski
x86/fpu: Handle #NM without FPU emulation as an error
Commit-ID:  bef8b6da95229f780a0a7f63e23124058bfad6d3
Gitweb:     http://git.kernel.org/tip/bef8b6da95229f780a0a7f63e23124058bfad6d3
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:47 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:54 +0100

x86/fpu: Handle #NM without FPU emulation as an error

Don't use CR0.TS. Make it an error rather than making nonsensical changes to the FPU state.

(The cond_local_irq_enable() appears to have been pointless, too.)

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/kernel/traps.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
arch/x86/kernel/traps.c +16 -4
index bd4e3d4..bf0c6d0 100644
@@ -853,6 +853,8 @@ do_spurious_interrupt_bug(struct pt_regs *regs, long error_code)
 dotraplinkage void
 do_device_not_available(struct pt_regs *regs, long error_code)
 {
+	unsigned long cr0;
+
 	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
 
 #ifdef CONFIG_MATH_EMULATION
@@ -866,10 +868,20 @@ do_device_not_available(struct pt_regs *regs, long error_code)
 		return;
 	}
 #endif
-	fpu__restore(&current->thread.fpu); /* interrupts still off */
-#ifdef CONFIG_X86_32
-	cond_local_irq_enable(regs);
-#endif
+
+	/* This should not happen. */
+	cr0 = read_cr0();
+	if (WARN(cr0 & X86_CR0_TS, "CR0.TS was set")) {
+		/* Try to fix it up and carry on. */
+		write_cr0(cr0 & ~X86_CR0_TS);
+	} else {
+		/*
+		 * Something terrible happened, and we're better off trying
+		 * to kill the task than getting stuck in a never-ending
+		 * loop of #NM faults.
+		 */
+		die("unexpected #NM exception", regs, error_code);
+	}
 }
 NOKPROBE_SYMBOL(do_device_not_available);
 
#8 lore raw

[tip:x86/fpu] x86/fpu, kvm: Remove host CR0.TS manipulation

In reply to Andy Lutomirski
x86/fpu, kvm: Remove host CR0.TS manipulation
Commit-ID:  04ac88abaf758bd76edcc3be5549003a017e7963
Gitweb:     http://git.kernel.org/tip/04ac88abaf758bd76edcc3be5549003a017e7963
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 31 Oct 2016 15:18:45 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Nov 2016 07:47:54 +0100

x86/fpu, kvm: Remove host CR0.TS manipulation

Now that x86 always uses eager FPU switching on the host, there's no need for KVM to manipulate the host's CR0.TS.

This should be both simpler and faster.

Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Quentin Casasnovas <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Rusty Russell <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc kvm list <[email protected]>
---
 arch/x86/kvm/vmx.c | 12 ++++--------
 arch/x86/kvm/x86.c |  5 -----
 2 files changed, 4 insertions(+), 13 deletions(-)
arch/x86/kvm/vmx.c +4 -8
index cf1b16d..531c446 100644
@@ -2144,12 +2144,6 @@ static void __vmx_load_host_state(struct vcpu_vmx *vmx)
 #endif
 	if (vmx->host_state.msr_host_bndcfgs)
 		wrmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
-	/*
-	 * If the FPU is not active (through the host task or
-	 * the guest vcpu), then restore the cr0.TS bit.
-	 */
-	if (!fpregs_active() && !vmx->vcpu.guest_fpu_loaded)
-		stts();
 	load_gdt(this_cpu_ptr(&host_gdt));
 }
 
@@ -4871,9 +4865,11 @@ static void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
 	u32 low32, high32;
 	unsigned long tmpl;
 	struct desc_ptr dt;
-	unsigned long cr4;
+	unsigned long cr0, cr4;
 
-	vmcs_writel(HOST_CR0, read_cr0() & ~X86_CR0_TS);  /* 22.2.3 */
+	cr0 = read_cr0();
+	WARN_ON(cr0 & X86_CR0_TS);
+	vmcs_writel(HOST_CR0, cr0);  /* 22.2.3 */
 	vmcs_writel(HOST_CR3, read_cr3());  /* 22.2.3  FIXME: shadow tables */
 
 	/* Save the most likely value for this task's CR4 in the VMCS. */
arch/x86/kvm/x86.c +0 -5
index 7f9fa2d..cfe6a75b 100644
@@ -5060,11 +5060,6 @@ static void emulator_get_fpu(struct x86_emulate_ctxt *ctxt)
 {
 	preempt_disable();
 	kvm_load_guest_fpu(emul_to_vcpu(ctxt));
-	/*
-	 * CR0.TS may reference the host fpu state, not the guest fpu state,
-	 * so it may be clear at this point.
-	 */
-	clts();
 }
 
 static void emulator_put_fpu(struct x86_emulate_ctxt *ctxt)
#21 lore raw
In reply to Borislav Petkov
Quoted 10 lines · Borislav Petkov
drivers/lguest/Kconfig +0 -0

 config LGUEST
  tristate "Linux hypervisor example code"

drivers/lguest/Kconfig +0 -1
-	depends on X86_32 && EVENTFD && TTY && PCI_DIRECT

drivers/lguest/Kconfig line 3

Quoted 1 line
drivers/lguest/Kconfig +1 -0
+	depends on X86_32 && EVENTFD && TTY && PCI_DIRECT && !MICROCODE

  select HVC_DRIVER
  ---help---
    This is a very simple module which allows you to run

LGUEST is the symbol for host support. The symbol for guest support is LGUEST_GUEST and it lives in arch/x86/. Yes, it's a bit of a gotcha.

Quoted 7 lines

but maybe the better fix is to hack in MSR emulation in lguest and intercept the *MSR accesses and do the writes/reads in the exception fixup and ...

I haven't looked at the lguest code, of course and whether that's easily doable and whether it even makes sense and whether one should simply use qemu/kvm instead and, and, and...

Yeah, I thought about adding negative dependencies (eg, "!OLPC && !MICROCODE") too. But that would be contrary to the neat lguest goal to be able to use the same kernel image as a host and a guest. At least, I think that is one of its goals.

And as probably everybody capable of hacking on lguest (ie, other people than me) came up with doubts similar to yours, these two issues never got fixed.

Thanks,

Paul Bolle

maintainer
#22 lore raw
Quoted 3 lines · Paul Bolle

And as probably everybody capable of hacking on lguest (ie, other people than me) came up with doubts similar to yours, these two issues never got fixed.

You can always try to fix them and see where it gets ya. I think that's the *ultimate* goal of lguest.

:-)

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.