All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] x86/fpu: Remove CR0.TS support
@ 2016-10-31 22:18 Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 1/8] fpu/init: Get rid of two redundant clts() calls Andy Lutomirski
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, Andy Lutomirski

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(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 23+ messages in thread
* [PATCH 1/8] fpu/init: Get rid of two redundant clts() calls
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:13   ` [tip:x86/fpu] x86/fpu: " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 2/8] fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs() Andy Lutomirski
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, 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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/fpu/init.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 1a09d133c801..60dece392b3a 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -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();
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 2/8] fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs()
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 1/8] fpu/init: Get rid of two redundant clts() calls Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:14   ` [tip:x86/fpu] x86/fpu: " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 3/8] x86/fpu: Remove irq_ts_save() and irq_ts_restore() Andy Lutomirski
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, Andy Lutomirski

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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/fpu/bugs.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/x86/kernel/fpu/bugs.c b/arch/x86/kernel/fpu/bugs.c
index aad34aafc0e0..d913047f832c 100644
--- a/arch/x86/kernel/fpu/bugs.c
+++ b/arch/x86/kernel/fpu/bugs.c
@@ -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");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 3/8] x86/fpu: Remove irq_ts_save() and irq_ts_restore()
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 1/8] fpu/init: Get rid of two redundant clts() calls Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 2/8] fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs() Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:14   ` [tip:x86/fpu] " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 4/8] x86/kvm: Remove host CR0.TS manipulation Andy Lutomirski
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, 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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 1429a7c736db..0877ae018fc9 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -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
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 52f5684405c1..7d8e2628e82c 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -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.
diff --git a/drivers/char/hw_random/via-rng.c b/drivers/char/hw_random/via-rng.c
index 44ce80606944..d1f5bb534e0e 100644
--- a/drivers/char/hw_random/via-rng.c
+++ b/drivers/char/hw_random/via-rng.c
@@ -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;
 }
 
diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index 441e86b23571..b3869748cc6b 100644
--- a/drivers/crypto/padlock-aes.c
+++ b/drivers/crypto/padlock-aes.c
@@ -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;
diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index 8c5f90647b7a..bc72d20c32c3 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -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;
 		}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 4/8] x86/kvm: Remove host CR0.TS manipulation
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
                   ` (2 preceding siblings ...)
  2016-10-31 22:18 ` [PATCH 3/8] x86/fpu: Remove irq_ts_save() and irq_ts_restore() Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:15   ` [tip:x86/fpu] x86/fpu, kvm: " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 5/8] lguest: Remove CR0.TS support Andy Lutomirski
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, 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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kvm/vmx.c | 12 ++++--------
 arch/x86/kvm/x86.c |  5 -----
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index cf1b16dbc98a..531c44633190 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -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. */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d5700263dad2..123d428d319a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 5/8] lguest: Remove CR0.TS support
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
                   ` (3 preceding siblings ...)
  2016-10-31 22:18 ` [PATCH 4/8] x86/kvm: Remove host CR0.TS manipulation Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:15   ` [tip:x86/fpu] x86/fpu, " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 6/8] x86/fpu: #NM without FPU emulation is an error Andy Lutomirski
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, Andy Lutomirski

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

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/lguest_hcall.h b/arch/x86/include/asm/lguest_hcall.h
index ef01fef3eebc..6c119cfae218 100644
--- a/arch/x86/include/asm/lguest_hcall.h
+++ b/arch/x86/include/asm/lguest_hcall.h
@@ -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
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 25da5bc8d83d..d74afcdbc580 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -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;
 }
 
 /*
diff --git a/drivers/lguest/hypercalls.c b/drivers/lguest/hypercalls.c
index 19a32280731d..601f81c04873 100644
--- a/drivers/lguest/hypercalls.c
+++ b/drivers/lguest/hypercalls.c
@@ -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;
diff --git a/drivers/lguest/lg.h b/drivers/lguest/lg.h
index 69b3814afd2f..2356a2318034 100644
--- a/drivers/lguest/lg.h
+++ b/drivers/lguest/lg.h
@@ -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;
 
diff --git a/drivers/lguest/x86/core.c b/drivers/lguest/x86/core.c
index 6e9042e3d2a9..743253fc638f 100644
--- a/drivers/lguest/x86/core.c
+++ b/drivers/lguest/x86/core.c
@@ -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. */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 6/8] x86/fpu: #NM without FPU emulation is an error
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
                   ` (4 preceding siblings ...)
  2016-10-31 22:18 ` [PATCH 5/8] lguest: Remove CR0.TS support Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:16   ` [tip:x86/fpu] x86/fpu: Handle #NM without FPU emulation as " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 7/8] x86/fpu: Remove stts() Andy Lutomirski
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, 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.)

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/traps.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index bd4e3d4d3625..bf0c6d049080 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -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);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 7/8] x86/fpu: Remove stts()
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
                   ` (5 preceding siblings ...)
  2016-10-31 22:18 ` [PATCH 6/8] x86/fpu: #NM without FPU emulation is an error Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:16   ` [tip:x86/fpu] " tip-bot for Andy Lutomirski
  2016-10-31 22:18 ` [PATCH 8/8] x86/fpu: Remove clts() Andy Lutomirski
  2016-10-31 22:41 ` [PATCH 0/8] x86/fpu: Remove CR0.TS support Paul Bolle
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, Andy Lutomirski

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

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/special_insns.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 19a2224f9e16..29b417bd8a68 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -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));
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 8/8] x86/fpu: Remove clts()
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
                   ` (6 preceding siblings ...)
  2016-10-31 22:18 ` [PATCH 7/8] x86/fpu: Remove stts() Andy Lutomirski
@ 2016-10-31 22:18 ` Andy Lutomirski
  2016-11-01  7:17   ` [tip:x86/fpu] " tip-bot for Andy Lutomirski
  2016-10-31 22:41 ` [PATCH 0/8] x86/fpu: Remove CR0.TS support Paul Bolle
  8 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2016-10-31 22:18 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list, 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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index ce932812f142..f1fb4dbe9a3e 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -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);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 0f400c0e4979..545426aa61ef 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -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);
 
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 29b417bd8a68..12af3e35edfa 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -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)
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index bbf3d5933eaa..a1bfba0f7234 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -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,
diff --git a/arch/x86/kernel/paravirt_patch_32.c b/arch/x86/kernel/paravirt_patch_32.c
index 920c6ae08592..d3f7f14bb328 100644
--- a/arch/x86/kernel/paravirt_patch_32.c
+++ b/arch/x86/kernel/paravirt_patch_32.c
@@ -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()) {
diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
index bb3840cedb4f..915a4c0b217c 100644
--- a/arch/x86/kernel/paravirt_patch_64.c
+++ b/arch/x86/kernel/paravirt_patch_64.c
@@ -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)
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index d74afcdbc580..4ca0d78adcf0 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -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;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c0fdd57da7aa..69f22f7e80ed 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -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,
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
  2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
                   ` (7 preceding siblings ...)
  2016-10-31 22:18 ` [PATCH 8/8] x86/fpu: Remove clts() Andy Lutomirski
@ 2016-10-31 22:41 ` Paul Bolle
  2016-10-31 23:04   ` Borislav Petkov
  8 siblings, 1 reply; 23+ messages in thread
From: Paul Bolle @ 2016-10-31 22:41 UTC (permalink / raw)
  To: Andy Lutomirski, x86
  Cc: Borislav Petkov, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list

On Mon, 2016-10-31 at 15:18 -0700, Andy Lutomirski wrote:
> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
  2016-10-31 22:41 ` [PATCH 0/8] x86/fpu: Remove CR0.TS support Paul Bolle
@ 2016-10-31 23:04   ` Borislav Petkov
  2016-10-31 23:10     ` Paul Bolle
  0 siblings, 1 reply; 23+ messages in thread
From: Borislav Petkov @ 2016-10-31 23:04 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Andy Lutomirski, x86, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list

On Mon, Oct 31, 2016 at 11:41:19PM +0100, Paul Bolle wrote:
> 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...

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
  2016-10-31 23:04   ` Borislav Petkov
@ 2016-10-31 23:10     ` Paul Bolle
  2016-10-31 23:48       ` Borislav Petkov
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Bolle @ 2016-10-31 23:10 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Andy Lutomirski, x86, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list

On Tue, 2016-11-01 at 00:04 +0100, Borislav Petkov wrote:
> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
  2016-10-31 23:10     ` Paul Bolle
@ 2016-10-31 23:48       ` Borislav Petkov
  2016-11-01  7:51         ` Paul Bolle
  0 siblings, 1 reply; 23+ messages in thread
From: Borislav Petkov @ 2016-10-31 23:48 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Andy Lutomirski, x86, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list

On Tue, Nov 01, 2016 at 12:10:48AM +0100, Paul Bolle wrote:
> 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:

---
diff --git a/drivers/lguest/Kconfig b/drivers/lguest/Kconfig
index 169172d2ba05..9c08b3050bb7 100644
--- a/drivers/lguest/Kconfig
+++ b/drivers/lguest/Kconfig
@@ -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...

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu: Get rid of two redundant clts() calls
  2016-10-31 22:18 ` [PATCH 1/8] fpu/init: Get rid of two redundant clts() calls Andy Lutomirski
@ 2016-11-01  7:13   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:13 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: kvm, quentin.casasnovas, dave.hansen, riel, jpoimboe, luto,
	torvalds, peterz, rusty, oleg, bp, brgerst, linux-kernel, mingo,
	tglx, hpa, dvlasenk, fenghua.yu, pbonzini

Commit-ID:  36fd4f0249f8cb445835acb7c6937a0ffa2b5f14
Gitweb:     http://git.kernel.org/tip/36fd4f0249f8cb445835acb7c6937a0ffa2b5f14
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:42 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/476d2d5066eda24838853426ea74c94140b50c85.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/init.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 1a09d13..60dece3 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -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();

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu: Stop saving and restoring CR0.TS in fpu__init_check_bugs()
  2016-10-31 22:18 ` [PATCH 2/8] fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs() Andy Lutomirski
@ 2016-11-01  7:14   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:14 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, tglx, brgerst, torvalds, rusty, luto, dave.hansen,
	mingo, hpa, bp, peterz, dvlasenk, pbonzini, jpoimboe, kvm, riel,
	quentin.casasnovas, fenghua.yu, oleg

Commit-ID:  fc560a80bac91e512fc37cdfe03a982ef4543c6b
Gitweb:     http://git.kernel.org/tip/fc560a80bac91e512fc37cdfe03a982ef4543c6b
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:43 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/76d1f1eacb5caead98197d1eb50ac6110ab20c6a.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/bugs.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/x86/kernel/fpu/bugs.c b/arch/x86/kernel/fpu/bugs.c
index aad34aa..d913047 100644
--- a/arch/x86/kernel/fpu/bugs.c
+++ b/arch/x86/kernel/fpu/bugs.c
@@ -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");

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu: Remove irq_ts_save() and irq_ts_restore()
  2016-10-31 22:18 ` [PATCH 3/8] x86/fpu: Remove irq_ts_save() and irq_ts_restore() Andy Lutomirski
@ 2016-11-01  7:14   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:14 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, kvm, torvalds, linux-kernel, luto, brgerst, rusty,
	fenghua.yu, pbonzini, oleg, dvlasenk, peterz, riel, bp, tglx,
	dave.hansen, quentin.casasnovas, jpoimboe, hpa

Commit-ID:  5a83d60c074ddf4f6364be25654a643d0e941824
Gitweb:     http://git.kernel.org/tip/5a83d60c074ddf4f6364be25654a643d0e941824
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:44 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/70b9b9e7ba70659bedcb08aba63d0f9214f338f2.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 1429a7c..0877ae0 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -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
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 52f56844..7d8e262 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -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.
diff --git a/drivers/char/hw_random/via-rng.c b/drivers/char/hw_random/via-rng.c
index 44ce806..d1f5bb5 100644
--- a/drivers/char/hw_random/via-rng.c
+++ b/drivers/char/hw_random/via-rng.c
@@ -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;
 }
 
diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index 441e86b..b386974 100644
--- a/drivers/crypto/padlock-aes.c
+++ b/drivers/crypto/padlock-aes.c
@@ -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;
diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index 8c5f906..bc72d20 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -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;
 		}

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu, kvm: Remove host CR0.TS manipulation
  2016-10-31 22:18 ` [PATCH 4/8] x86/kvm: Remove host CR0.TS manipulation Andy Lutomirski
@ 2016-11-01  7:15   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: luto, bp, fenghua.yu, oleg, peterz, dave.hansen,
	quentin.casasnovas, dvlasenk, kvm, pbonzini, torvalds, brgerst,
	mingo, linux-kernel, rusty, jpoimboe, tglx, riel, hpa

Commit-ID:  04ac88abaf758bd76edcc3be5549003a017e7963
Gitweb:     http://git.kernel.org/tip/04ac88abaf758bd76edcc3be5549003a017e7963
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:45 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/b212064922537c05d0c81d931fc4dbe769127ce7.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kvm/vmx.c | 12 ++++--------
 arch/x86/kvm/x86.c |  5 -----
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index cf1b16d..531c446 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -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. */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7f9fa2d..cfe6a75b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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)

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu, lguest: Remove CR0.TS support
  2016-10-31 22:18 ` [PATCH 5/8] lguest: Remove CR0.TS support Andy Lutomirski
@ 2016-11-01  7:15   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: rusty, tglx, brgerst, mingo, luto, riel, pbonzini, oleg,
	fenghua.yu, linux-kernel, peterz, dvlasenk, jpoimboe, kvm, bp,
	torvalds, hpa, dave.hansen, quentin.casasnovas

Commit-ID:  cd95ea81f25608c403052d0508ee5c9b32e2bc7d
Gitweb:     http://git.kernel.org/tip/cd95ea81f25608c403052d0508ee5c9b32e2bc7d
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:46 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/8a7bf2c11231c082258fd67705d0f275639b8475.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/lguest_hcall.h b/arch/x86/include/asm/lguest_hcall.h
index ef01fef..6c119cf 100644
--- a/arch/x86/include/asm/lguest_hcall.h
+++ b/arch/x86/include/asm/lguest_hcall.h
@@ -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
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 25da5bc8..d74afcd 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -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;
 }
 
 /*
diff --git a/drivers/lguest/hypercalls.c b/drivers/lguest/hypercalls.c
index 19a3228..601f81c 100644
--- a/drivers/lguest/hypercalls.c
+++ b/drivers/lguest/hypercalls.c
@@ -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;
diff --git a/drivers/lguest/lg.h b/drivers/lguest/lg.h
index 69b3814..2356a23 100644
--- a/drivers/lguest/lg.h
+++ b/drivers/lguest/lg.h
@@ -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;
 
diff --git a/drivers/lguest/x86/core.c b/drivers/lguest/x86/core.c
index 6e9042e..743253f 100644
--- a/drivers/lguest/x86/core.c
+++ b/drivers/lguest/x86/core.c
@@ -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. */

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu: Handle #NM without FPU emulation as an error
  2016-10-31 22:18 ` [PATCH 6/8] x86/fpu: #NM without FPU emulation is an error Andy Lutomirski
@ 2016-11-01  7:16   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:16 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, jpoimboe, oleg, pbonzini, bp, quentin.casasnovas, rusty,
	riel, brgerst, luto, dave.hansen, kvm, torvalds, fenghua.yu, hpa,
	mingo, dvlasenk, peterz, linux-kernel

Commit-ID:  bef8b6da95229f780a0a7f63e23124058bfad6d3
Gitweb:     http://git.kernel.org/tip/bef8b6da95229f780a0a7f63e23124058bfad6d3
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:47 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.)

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/f1ee6bf73ed1025fccaab321ba43d0594245f927.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/traps.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index bd4e3d4..bf0c6d0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -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);
 

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu: Remove stts()
  2016-10-31 22:18 ` [PATCH 7/8] x86/fpu: Remove stts() Andy Lutomirski
@ 2016-11-01  7:16   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:16 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, jpoimboe, luto, mingo, dvlasenk, oleg, tglx, kvm, riel,
	linux-kernel, rusty, dave.hansen, fenghua.yu, torvalds, bp,
	peterz, pbonzini, quentin.casasnovas, brgerst

Commit-ID:  0d50612c041f213fb6b98e3ff06e306a859c36f2
Gitweb:     http://git.kernel.org/tip/0d50612c041f213fb6b98e3ff06e306a859c36f2
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:48 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/04247401710b230849e58bf2112ce4fd0b9840e1.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/special_insns.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 19a2224..29b417b 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -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));

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [tip:x86/fpu] x86/fpu: Remove clts()
  2016-10-31 22:18 ` [PATCH 8/8] x86/fpu: Remove clts() Andy Lutomirski
@ 2016-11-01  7:17   ` tip-bot for Andy Lutomirski
  0 siblings, 0 replies; 23+ messages in thread
From: tip-bot for Andy Lutomirski @ 2016-11-01  7:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: quentin.casasnovas, jpoimboe, fenghua.yu, bp, tglx, riel,
	torvalds, hpa, dvlasenk, brgerst, peterz, oleg, mingo, rusty,
	linux-kernel, pbonzini, kvm, luto, dave.hansen

Commit-ID:  af25ed59b5616b389d90877f7085dc5d457a3d49
Gitweb:     http://git.kernel.org/tip/af25ed59b5616b389d90877f7085dc5d457a3d49
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Mon, 31 Oct 2016 15:18:49 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm list <kvm@vger.kernel.org>
Link: http://lkml.kernel.org/r/3d3c8ca62f17579b9849a013d71e59a4d5d1b079.1477951965.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index ce93281..f1fb4db 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -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);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 0f400c0..545426a 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -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);
 
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 29b417b..12af3e3 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -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)
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index bbf3d59..a1bfba0 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -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,
diff --git a/arch/x86/kernel/paravirt_patch_32.c b/arch/x86/kernel/paravirt_patch_32.c
index 920c6ae..d3f7f14 100644
--- a/arch/x86/kernel/paravirt_patch_32.c
+++ b/arch/x86/kernel/paravirt_patch_32.c
@@ -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()) {
diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
index bb3840c..915a4c0 100644
--- a/arch/x86/kernel/paravirt_patch_64.c
+++ b/arch/x86/kernel/paravirt_patch_64.c
@@ -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)
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index d74afcd..4ca0d78 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -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;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index bdd8556..ced7027 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -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,
 

^ permalink raw reply related	[flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
  2016-10-31 23:48       ` Borislav Petkov
@ 2016-11-01  7:51         ` Paul Bolle
  2016-11-01  8:50           ` Borislav Petkov
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Bolle @ 2016-11-01  7:51 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Andy Lutomirski, x86, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list

On Tue, 2016-11-01 at 00:48 +0100, Borislav Petkov wrote:
> --- a/drivers/lguest/Kconfig
> +++ b/drivers/lguest/Kconfig

>  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

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.

> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] x86/fpu: Remove CR0.TS support
  2016-11-01  7:51         ` Paul Bolle
@ 2016-11-01  8:50           ` Borislav Petkov
  0 siblings, 0 replies; 23+ messages in thread
From: Borislav Petkov @ 2016-11-01  8:50 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Andy Lutomirski, x86, linux-kernel, Rusty Russell, Paolo Bonzini,
	Rik van Riel, kvm list

On Tue, Nov 01, 2016 at 08:51:57AM +0100, Paul Bolle wrote:
> 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.

:-)

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-11-01  8:50 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-31 22:18 [PATCH 0/8] x86/fpu: Remove CR0.TS support Andy Lutomirski
2016-10-31 22:18 ` [PATCH 1/8] fpu/init: Get rid of two redundant clts() calls Andy Lutomirski
2016-11-01  7:13   ` [tip:x86/fpu] x86/fpu: " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 2/8] fpu/bugs: Stop saving and restoring CR0.TS in fpu__init_check_bugs() Andy Lutomirski
2016-11-01  7:14   ` [tip:x86/fpu] x86/fpu: " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 3/8] x86/fpu: Remove irq_ts_save() and irq_ts_restore() Andy Lutomirski
2016-11-01  7:14   ` [tip:x86/fpu] " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 4/8] x86/kvm: Remove host CR0.TS manipulation Andy Lutomirski
2016-11-01  7:15   ` [tip:x86/fpu] x86/fpu, kvm: " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 5/8] lguest: Remove CR0.TS support Andy Lutomirski
2016-11-01  7:15   ` [tip:x86/fpu] x86/fpu, " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 6/8] x86/fpu: #NM without FPU emulation is an error Andy Lutomirski
2016-11-01  7:16   ` [tip:x86/fpu] x86/fpu: Handle #NM without FPU emulation as " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 7/8] x86/fpu: Remove stts() Andy Lutomirski
2016-11-01  7:16   ` [tip:x86/fpu] " tip-bot for Andy Lutomirski
2016-10-31 22:18 ` [PATCH 8/8] x86/fpu: Remove clts() Andy Lutomirski
2016-11-01  7:17   ` [tip:x86/fpu] " tip-bot for Andy Lutomirski
2016-10-31 22:41 ` [PATCH 0/8] x86/fpu: Remove CR0.TS support Paul Bolle
2016-10-31 23:04   ` Borislav Petkov
2016-10-31 23:10     ` Paul Bolle
2016-10-31 23:48       ` Borislav Petkov
2016-11-01  7:51         ` Paul Bolle
2016-11-01  8:50           ` Borislav Petkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.