v1: http://www.spinics.net/lists/kvm/msg139287.html
v2 is far more defensive and hopefully correct ...
Radim Krčmář (4):
KVM: x86: add Align16 instruction flag
KVM: x86: save one bit in ctxt->d
KVM: x86: add asm_safe wrapper
KVM: x86: emulate FXSAVE and FXRSTOR
arch/x86/kvm/emulate.c | 191 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 164 insertions(+), 27 deletions(-)
--
2.10.1
Needed for FXSAVE and FXRSTOR.
Signed-off-by: Radim Krčmář <[email protected]>
---
v2: split into a separate patch
---
arch/x86/kvm/emulate.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 4e95d3eb2955..557dbb9e5bec 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -171,6 +171,7 @@
#define NearBranch ((u64)1 << 52) /* Near branches */
#define No16 ((u64)1 << 53) /* No 16 bit operand */
#define IncSP ((u64)1 << 54) /* SP is incremented before ModRM calc */
+#define Aligned16 ((u64)1 << 55) /* Aligned to 16 byte boundary (e.g. FXSAVE) */
#define DstXacc (DstAccLo | SrcAccHi | SrcWrite)
@@ -632,21 +633,24 @@ static void set_segment_selector(struct x86_emulate_ctxt *ctxt, u16 selector,
* depending on whether they're AVX encoded or not.
*
* Also included is CMPXCHG16B which is not a vector instruction, yet it is
- * subject to the same check.
+ * subject to the same check. FXSAVE and FXRSTOR are checked here too as their
+ * 512 bytes of data must be aligned to a 16 byte boundary.
*/
-static bool insn_aligned(struct x86_emulate_ctxt *ctxt, unsigned size)
+static unsigned insn_alignment(struct x86_emulate_ctxt *ctxt, unsigned size)
{
if (likely(size < 16))
- return false;
+ return 1;
if (ctxt->d & Aligned)
- return true;
+ return size;
else if (ctxt->d & Unaligned)
- return false;
+ return 1;
else if (ctxt->d & Avx)
- return false;
+ return 1;
+ else if (ctxt->d & Aligned16)
+ return 16;
else
- return true;
+ return size;
}
static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
@@ -704,7 +708,7 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
}
break;
}
- if (insn_aligned(ctxt, size) && ((la & (size - 1)) != 0))
+ if (la & (insn_alignment(ctxt, size) - 1))
return emulate_gp(ctxt, 0);
return X86EMUL_CONTINUE;
bad:
--
2.10.1
Internal errors were reported on 16 bit fxsave and fxrstor with ipxe.
Old Intels don't have unrestricted_guest, so we have to emulate them.
The patch takes advantage of the hardware implementation.
A small omission is done on FP CS and DS, because they are hard to deal
with if the CPU deprecates them, but the guest we try to emulate does
not. Catching this in CPUID sanity checks would be best.
Signed-off-by: Radim Krčmář <[email protected]>
---
v2:
- throws #GP to the guest when reserved MXCSR are set [Nadav]
- returns internal emulation error if an exception is hit during
execution
- preserves XMM 8-15
---
arch/x86/kvm/emulate.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 119 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 247e7204deae..891b8f86f0bc 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3883,6 +3883,122 @@ static int em_movsxd(struct x86_emulate_ctxt *ctxt)
return X86EMUL_CONTINUE;
}
+static int check_fxsr(struct x86_emulate_ctxt *ctxt)
+{
+ u32 eax = 1, ebx, ecx = 0, edx;
+
+ ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx);
+ if (!(edx & FFL(FXSR)))
+ return emulate_ud(ctxt);
+
+ if (ctxt->ops->get_cr(ctxt, 0) & (X86_CR0_TS | X86_CR0_EM))
+ return emulate_nm(ctxt);
+
+ return X86EMUL_CONTINUE;
+}
+
+/*
+ * FXSAVE and FXRSTOR have 4 different formats depending on execution mode,
+ * 1) 16 bit mode
+ * 2) 32 bit mode
+ * - like (1), but FIP and FDP (foo) are only 16 bit. At least Intel CPUs
+ * preserve whole 32 bit values, though, so (1) and (2) are the same wrt.
+ * save and restore
+ * 3) 64-bit mode with REX.W prefix
+ * - like (2), but XMM 8-15 are being saved and restored
+ * 4) 64-bit mode without REX.W prefix
+ * - like (3), but FIP and FDP are 64 bit
+ *
+ * Emulation uses (3) for (1) and (2), but preserves XMM 8-15 to reach the same
+ * result.
+ * XXX: Guest and host CPUID.(EAX=07H,ECX=0H):EBX[bit 13] (deprecate FPU CS and
+ * FPU DS) is expected to match.
+ */
+static int em_fxsave(struct x86_emulate_ctxt *ctxt)
+{
+ struct fxregs_state fx_state;
+ size_t size = 416; /* up to XMM15 */
+ int rc;
+
+ rc = check_fxsr(ctxt);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
+
+ ctxt->ops->get_fpu(ctxt);
+
+#ifdef CONFIG_X86_64
+ if (ctxt->rex_prefix & (1 << 3))
+ rc = asm_safe("fxsave64 %[fx]", , [fx] "+m"(fx_state) ::);
+ else
+#endif
+ rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state) ::);
+
+ if (ctxt->mode < X86EMUL_MODE_PROT64)
+ size = 288; /* up to XMM7 */
+
+ ctxt->ops->put_fpu(ctxt);
+
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
+
+ return segmented_write(ctxt, ctxt->memop.addr.mem, &fx_state, size);
+}
+
+static int fx_load_64bit_xmm(struct fxregs_state *new)
+{
+ int rc = X86EMUL_CONTINUE;
+#ifdef CONFIG_X86_64
+ struct fxregs_state old;
+
+ rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old) ::);
+
+ /* XXX: accessing XMM 8-15 is very awkward */
+ memcpy(&new->xmm_space[8*16 / 4], &old.xmm_space[8*16 / 4], 8*16);
+#endif
+ return rc;
+}
+
+static int em_fxrstor(struct x86_emulate_ctxt *ctxt)
+{
+ struct fxregs_state fx_state;
+ int rc;
+
+ rc = check_fxsr(ctxt);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
+
+ rc = segmented_read(ctxt, ctxt->memop.addr.mem, &fx_state, 512);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
+
+ if (fx_state.mxcsr >> 16)
+ return emulate_gp(ctxt, 0);
+
+ ctxt->ops->get_fpu(ctxt);
+
+ /*
+ * 64 bit host will always restore XMM8-15, which is not correct on
+ * non-64 bit guests. Load the current values in order to preserve 64
+ * bit XMMs after fxrstor.
+ */
+ if (ctxt->mode < X86EMUL_MODE_PROT64)
+ rc = fx_load_64bit_xmm(&fx_state);
+
+ if (rc == X86EMUL_CONTINUE) {
+#ifdef CONFIG_X86_64
+ if (ctxt->rex_prefix & (1 << 3))
+ rc = asm_safe("fxrstor64 %[fx]",
+ : [fx] "m"(fx_state) :);
+ else
+#endif
+ rc = asm_safe("fxrstor %[fx]",
+ : [fx] "m"(fx_state) :);
+ }
+ ctxt->ops->put_fpu(ctxt);
+
+ return rc;
+}
+
static bool valid_cr(int nr)
{
switch (nr) {
@@ -4235,7 +4351,9 @@ static const struct gprefix pfx_0f_ae_7 = {
};
static const struct group_dual group15 = { {
- N, N, N, N, N, N, N, GP(0, &pfx_0f_ae_7),
+ I(ModRM | Aligned16, em_fxsave),
+ I(ModRM | Aligned16, em_fxrstor),
+ N, N, N, N, N, GP(0, &pfx_0f_ae_7),
}, {
N, N, N, N, N, N, N, N,
} };
--
2.10.1
Move the existing exception handling for inline assembly into a macro
and switch its return values to X86EMUL type.
Signed-off-by: Radim Krčmář <[email protected]>
---
v2: new
---
arch/x86/kvm/emulate.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 18616b6bdebb..247e7204deae 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -448,6 +448,26 @@ FOP_END;
FOP_START(salc) "pushf; sbb %al, %al; popf \n\t" FOP_RET
FOP_END;
+/*
+ * XXX: inoutclob user must know where the argument is being expanded.
+ * Relying on CC_HAVE_ASM_GOTO would allow us to remove _fault.
+ */
+#define asm_safe(insn, inoutclob...) \
+({ \
+ int _fault = 0; \
+ \
+ asm volatile("1:" insn "\n" \
+ "2:\n" \
+ ".pushsection .fixup, \"ax\"\n" \
+ "3: movl $1, %[_fault]\n" \
+ " jmp 2b\n" \
+ ".popsection\n" \
+ _ASM_EXTABLE(1b, 3b) \
+ : [_fault] "+qm"(_fault) inoutclob ); \
+ \
+ _fault ? X86EMUL_UNHANDLEABLE : X86EMUL_CONTINUE; \
+})
+
static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt,
enum x86_intercept intercept,
enum x86_intercept_stage stage)
@@ -5087,21 +5107,13 @@ static bool string_insn_completed(struct x86_emulate_ctxt *ctxt)
static int flush_pending_x87_faults(struct x86_emulate_ctxt *ctxt)
{
- bool fault = false;
+ int rc;
ctxt->ops->get_fpu(ctxt);
- asm volatile("1: fwait \n\t"
- "2: \n\t"
- ".pushsection .fixup,\"ax\" \n\t"
- "3: \n\t"
- "movb $1, %[fault] \n\t"
- "jmp 2b \n\t"
- ".popsection \n\t"
- _ASM_EXTABLE(1b, 3b)
- : [fault]"+qm"(fault));
+ rc = asm_safe("fwait");
ctxt->ops->put_fpu(ctxt);
- if (unlikely(fault))
+ if (unlikely(rc != X86EMUL_CONTINUE))
return emulate_exception(ctxt, MF_VECTOR, 0, false);
return X86EMUL_CONTINUE;
--
2.10.1
Alignments are exclusive, so 5 modes can be expressed in 3 bits.
Signed-off-by: Radim Krčmář <[email protected]>
---
arch/x86/kvm/emulate.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 557dbb9e5bec..18616b6bdebb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -158,9 +158,11 @@
#define Src2GS (OpGS << Src2Shift)
#define Src2Mask (OpMask << Src2Shift)
#define Mmx ((u64)1 << 40) /* MMX Vector instruction */
+#define AlignMask ((u64)7 << 41)
#define Aligned ((u64)1 << 41) /* Explicitly aligned (e.g. MOVDQA) */
-#define Unaligned ((u64)1 << 42) /* Explicitly unaligned (e.g. MOVDQU) */
-#define Avx ((u64)1 << 43) /* Advanced Vector Extensions */
+#define Unaligned ((u64)2 << 41) /* Explicitly unaligned (e.g. MOVDQU) */
+#define Avx ((u64)3 << 41) /* Advanced Vector Extensions */
+#define Aligned16 ((u64)4 << 41) /* Aligned to 16 byte boundary (e.g. FXSAVE) */
#define Fastop ((u64)1 << 44) /* Use opcode::u.fastop */
#define NoWrite ((u64)1 << 45) /* No writeback */
#define SrcWrite ((u64)1 << 46) /* Write back src operand */
@@ -171,7 +173,6 @@
#define NearBranch ((u64)1 << 52) /* Near branches */
#define No16 ((u64)1 << 53) /* No 16 bit operand */
#define IncSP ((u64)1 << 54) /* SP is incremented before ModRM calc */
-#define Aligned16 ((u64)1 << 55) /* Aligned to 16 byte boundary (e.g. FXSAVE) */
#define DstXacc (DstAccLo | SrcAccHi | SrcWrite)
@@ -638,19 +639,21 @@ static void set_segment_selector(struct x86_emulate_ctxt *ctxt, u16 selector,
*/
static unsigned insn_alignment(struct x86_emulate_ctxt *ctxt, unsigned size)
{
+ u64 alignment = ctxt->d & AlignMask;
+
if (likely(size < 16))
return 1;
- if (ctxt->d & Aligned)
- return size;
- else if (ctxt->d & Unaligned)
+ switch (alignment) {
+ case Unaligned:
+ case Avx:
return 1;
- else if (ctxt->d & Avx)
- return 1;
- else if (ctxt->d & Aligned16)
+ case Aligned16:
return 16;
- else
+ case Aligned:
+ default:
return size;
+ }
}
static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
--
2.10.1
Hi Radim,
[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.9-rc3 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Radim-Kr-m/KVM-x86-emulate-FXSAVE-and-FXRSTOR/20161103-025333
base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-s2-11030650 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
arch/x86/kvm/emulate.c: In function 'em_fxsave':
>> arch/x86/kvm/emulate.c:3931: error: expected string literal before ')' token
arch/x86/kvm/emulate.c:3934: error: expected string literal before ')' token
arch/x86/kvm/emulate.c: In function 'fx_load_64bit_xmm':
arch/x86/kvm/emulate.c:3953: error: expected string literal before ')' token
arch/x86/kvm/emulate.c: In function 'em_fxrstor':
arch/x86/kvm/emulate.c:3990: error: expected string literal before ')' token
arch/x86/kvm/emulate.c:3994: error: expected string literal before ')' token
vim +3931 arch/x86/kvm/emulate.c
3925 return rc;
3926
3927 ctxt->ops->get_fpu(ctxt);
3928
3929 #ifdef CONFIG_X86_64
3930 if (ctxt->rex_prefix & (1 << 3))
> 3931 rc = asm_safe("fxsave64 %[fx]", , [fx] "+m"(fx_state) ::);
3932 else
3933 #endif
3934 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state) ::);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
2016-11-03 07:58+0800, kbuild test robot:
> Hi Radim,
>
> [auto build test ERROR on kvm/linux-next]
> [also build test ERROR on v4.9-rc3 next-20161028]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> [Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
> [Check https://git-scm.com/docs/git-format-patch for more information]
>
> url: https://github.com/0day-ci/linux/commits/Radim-Kr-m/KVM-x86-emulate-FXSAVE-and-FXRSTOR/20161103-025333
> base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
> config: x86_64-randconfig-s2-11030650 (attached as .config)
> compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
Oh well ...
I dug out a RHEL6.9 machine with "gcc (GCC) 4.4.7 20120313 (Red Hat
4.4.7-18)" and it doesn't compile either, but for a different reason:
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
CC [M] arch/x86/kvm/emulate.o
arch/x86/kvm/emulate.c: Assembler messages:
arch/x86/kvm/emulate.c:3931: Error: no such instruction: `fxsave64 -544(%rbp)'
arch/x86/kvm/emulate.c:3990: Error: no such instruction: `fxrstor64 -528(%rbp)'
make[1]: *** [arch/x86/kvm/emulate.o] Error 1
make: *** [_module_arch/x86/kvm] Error 2
so I wrote the REX.W manually, ".byte 48; fxsave %[fx]", and then the
compilation succeeded:
CC [M] arch/x86/kvm/emulate.o
arch/x86/kvm/emulate.o: warning: objtool: read_sse_reg()+0xb6: function has unreachable instruction
(The output was riddled with warnings about unreachable instructions.)
> All errors (new ones prefixed by >>):
>
> arch/x86/kvm/emulate.c: In function 'em_fxsave':
> >> arch/x86/kvm/emulate.c:3931: error: expected string literal before ')' token
> arch/x86/kvm/emulate.c:3934: error: expected string literal before ')' token
> arch/x86/kvm/emulate.c: In function 'fx_load_64bit_xmm':
> arch/x86/kvm/emulate.c:3953: error: expected string literal before ')' token
> arch/x86/kvm/emulate.c: In function 'em_fxrstor':
> arch/x86/kvm/emulate.c:3990: error: expected string literal before ')' token
> arch/x86/kvm/emulate.c:3994: error: expected string literal before ')' token
>
> vim +3931 arch/x86/kvm/emulate.c
> > 3931 rc = asm_safe("fxsave64 %[fx]", , [fx] "+m"(fx_state) ::);
Seems like it expects something in the clobber list ... maybe it would
work with
rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state));
I'll install old Debian and send a fix for both issues.
The fix will drop emulation of protected modes, so we won't need the
fxsave64 instruction.