* [PATCH v3 0/4] KVM: x86: emulate FXSAVE and FXRSTOR
@ 2016-11-08 19:54 Radim Krčmář
2016-11-08 19:54 ` [PATCH v3 1/4] KVM: x86: add Align16 instruction flag Radim Krčmář
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Radim Krčmář @ 2016-11-08 19:54 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Paolo Bonzini, Bandan Das, Nadav Amit
v2: http://www.spinics.net/lists/kvm/msg139681.html
v3 brings compatibility with old compilers and has been compile-tested
with GCC-4.4 on Debian Wheezy, GCC-4.4 on RHEL 6, and GCC-4.1 on RHEL 5.
[4/4] still has the hidden assumption that guest and host CPUID match.
Emulating a guest that does not deprecaste FCS and FDS on a host that
does (a modern host) will not necessarily cause a problem, which is why
patches don't handle the case.
Enforcing the equality in CPUID update would be best, but another series
could do that as Linux doesn't even have the CPUID bit defined yet.
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 | 184 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 157 insertions(+), 27 deletions(-)
--
2.10.2
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v3 1/4] KVM: x86: add Align16 instruction flag 2016-11-08 19:54 [PATCH v3 0/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář @ 2016-11-08 19:54 ` Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 2/4] KVM: x86: save one bit in ctxt->d Radim Krčmář ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Radim Krčmář @ 2016-11-08 19:54 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: Paolo Bonzini, Bandan Das, Nadav Amit Needed for FXSAVE and FXRSTOR. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- 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 cbd7b92585bb..eb74d3b56e1c 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.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 2/4] KVM: x86: save one bit in ctxt->d 2016-11-08 19:54 [PATCH v3 0/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 1/4] KVM: x86: add Align16 instruction flag Radim Krčmář @ 2016-11-08 19:54 ` Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 3/4] KVM: x86: add asm_safe wrapper Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 3 siblings, 0 replies; 12+ messages in thread From: Radim Krčmář @ 2016-11-08 19:54 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: Paolo Bonzini, Bandan Das, Nadav Amit Alignments are exclusive, so 5 modes can be expressed in 3 bits. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- 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 eb74d3b56e1c..14624d6bf112 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.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 3/4] KVM: x86: add asm_safe wrapper 2016-11-08 19:54 [PATCH v3 0/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 1/4] KVM: x86: add Align16 instruction flag Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 2/4] KVM: x86: save one bit in ctxt->d Radim Krčmář @ 2016-11-08 19:54 ` Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 3 siblings, 0 replies; 12+ messages in thread From: Radim Krčmář @ 2016-11-08 19:54 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: Paolo Bonzini, Bandan Das, Nadav Amit 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ář <rkrcmar@redhat.com> --- 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 14624d6bf112..6af3cac6ec89 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.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-08 19:54 [PATCH v3 0/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář ` (2 preceding siblings ...) 2016-11-08 19:54 ` [PATCH v3 3/4] KVM: x86: add asm_safe wrapper Radim Krčmář @ 2016-11-08 19:54 ` Radim Krčmář 2016-11-08 23:25 ` Paolo Bonzini 2016-11-09 18:07 ` [PATCH v4] " Radim Krčmář 3 siblings, 2 replies; 12+ messages in thread From: Radim Krčmář @ 2016-11-08 19:54 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: Paolo Bonzini, Bandan Das, Nadav Amit 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. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- v3: - remove fxsave64 and extra colons at the end of asm to make old GCCs happy (fxsave64 could have been implemented using other nmemonics, but there is no point when it won't be used + removing it makes the code nicer.) 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 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 6af3cac6ec89..1b3fab1fb8d3 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -3883,6 +3883,115 @@ 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); + + /* + * Don't emulate a case that should never be hit, instead of working + * around a lack of fxsave64/fxrstor64 on old compilers. + */ + if (ctxt->mode >= X86EMUL_MODE_PROT64) + return X86EMUL_UNHANDLEABLE; + + 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) and preserves XMM 8-15 to reach the + * desired result. (4) is not emulated. + * + * XXX: Guest and host CPUID.(EAX=07H,ECX=0H):EBX[bit 13] (deprecate FPU CS + * and FPU DS) should match. + */ +static int em_fxsave(struct x86_emulate_ctxt *ctxt) +{ + struct fxregs_state fx_state; + size_t size = 288; /* up to XMM7 */ + int rc; + + rc = check_fxsr(ctxt); + if (rc != X86EMUL_CONTINUE) + return rc; + + ctxt->ops->get_fpu(ctxt); + + rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); + + 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) + 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 +4344,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.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-08 19:54 ` [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář @ 2016-11-08 23:25 ` Paolo Bonzini 2016-11-09 12:12 ` Radim Krčmář 2016-11-09 18:07 ` [PATCH v4] " Radim Krčmář 1 sibling, 1 reply; 12+ messages in thread From: Paolo Bonzini @ 2016-11-08 23:25 UTC (permalink / raw) To: Radim Krčmář, linux-kernel, kvm; +Cc: Bandan Das, Nadav Amit On 08/11/2016 20:54, Radim Krčmář wrote: > 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. > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > --- > v3: > - remove fxsave64 and extra colons at the end of asm to make old GCCs > happy (fxsave64 could have been implemented using other nmemonics, > but there is no point when it won't be used + removing it makes the > code nicer.) > 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 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 112 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 6af3cac6ec89..1b3fab1fb8d3 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -3883,6 +3883,115 @@ 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); > + > + /* > + * Don't emulate a case that should never be hit, instead of working > + * around a lack of fxsave64/fxrstor64 on old compilers. > + */ > + if (ctxt->mode >= X86EMUL_MODE_PROT64) > + return X86EMUL_UNHANDLEABLE; > + > + 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) and preserves XMM 8-15 to reach the > + * desired result. (4) is not emulated. > + * > + * XXX: Guest and host CPUID.(EAX=07H,ECX=0H):EBX[bit 13] (deprecate FPU CS > + * and FPU DS) should match. > + */ > +static int em_fxsave(struct x86_emulate_ctxt *ctxt) > +{ > + struct fxregs_state fx_state; > + size_t size = 288; /* up to XMM7 */ Sorry for noticing this only now; if CR4.OSFXSR is 0, XMM and MXCSR should not be saved. I can apply the first three patches already if you prefer. Paolo > + int rc; > + > + rc = check_fxsr(ctxt); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + > + ctxt->ops->get_fpu(ctxt); > + > + rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); > + > + 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) > + 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 +4344,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, > } }; > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-08 23:25 ` Paolo Bonzini @ 2016-11-09 12:12 ` Radim Krčmář 2016-11-09 14:19 ` Radim Krčmář 0 siblings, 1 reply; 12+ messages in thread From: Radim Krčmář @ 2016-11-09 12:12 UTC (permalink / raw) To: Paolo Bonzini; +Cc: linux-kernel, kvm, Bandan Das, Nadav Amit 2016-11-09 00:25+0100, Paolo Bonzini: > On 08/11/2016 20:54, Radim Krčmář wrote: >> 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. >> >> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> >> --- >> v3: >> - remove fxsave64 and extra colons at the end of asm to make old GCCs >> happy (fxsave64 could have been implemented using other nmemonics, >> but there is no point when it won't be used + removing it makes the >> code nicer.) >> 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 | 113 ++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 112 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index 6af3cac6ec89..1b3fab1fb8d3 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -3883,6 +3883,115 @@ 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); >> + >> + /* >> + * Don't emulate a case that should never be hit, instead of working >> + * around a lack of fxsave64/fxrstor64 on old compilers. >> + */ >> + if (ctxt->mode >= X86EMUL_MODE_PROT64) >> + return X86EMUL_UNHANDLEABLE; >> + >> + 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) and preserves XMM 8-15 to reach the >> + * desired result. (4) is not emulated. >> + * >> + * XXX: Guest and host CPUID.(EAX=07H,ECX=0H):EBX[bit 13] (deprecate FPU CS >> + * and FPU DS) should match. >> + */ >> +static int em_fxsave(struct x86_emulate_ctxt *ctxt) >> +{ >> + struct fxregs_state fx_state; >> + size_t size = 288; /* up to XMM7 */ > > Sorry for noticing this only now; if CR4.OSFXSR is 0, XMM and MXCSR > should not be saved. Intel processors don't save it, but the spec allows saving even when CR4.OSFXSR is 0: If the OSFXSR bit in control register CR4 is not set, the FXSAVE instruction may not save this register (these registers). This behavior is implementation dependent. I let "implementation dependent" behavior be the one with less code, but haven't checked AMD spec, which doesn't seem to make it implementation dependent ... I'll add it. (On intel, OSFXSR gets written with 0 and XMM 0-7 isn't modified without OSFXSR, so I'll just assume that AMD won't break with that.) > I can apply the first three patches already if you prefer. Yes, they would not change, thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-09 12:12 ` Radim Krčmář @ 2016-11-09 14:19 ` Radim Krčmář 0 siblings, 0 replies; 12+ messages in thread From: Radim Krčmář @ 2016-11-09 14:19 UTC (permalink / raw) To: Paolo Bonzini; +Cc: linux-kernel, kvm, Bandan Das, Nadav Amit 2016-11-09 13:12+0100, Radim Krčmář: > 2016-11-09 00:25+0100, Paolo Bonzini: >> On 08/11/2016 20:54, Radim Krčmář wrote: >>> +static int em_fxsave(struct x86_emulate_ctxt *ctxt) >>> +{ >>> + struct fxregs_state fx_state; >>> + size_t size = 288; /* up to XMM7 */ >> >> Sorry for noticing this only now; if CR4.OSFXSR is 0, XMM and MXCSR >> should not be saved. > > Intel processors don't save it, but the spec allows saving even when > CR4.OSFXSR is 0: > > If the OSFXSR bit in control register CR4 is not set, the FXSAVE > instruction may not save this register (these registers). > This behavior is implementation dependent. > > I let "implementation dependent" behavior be the one with less code, but > haven't checked AMD spec, which doesn't seem to make it implementation > dependent ... I'll add it. (On intel, OSFXSR gets written with 0 and Nope, Intel always saves and restores MXCSR. I should have access to an AMD machine later today and will implement FXSR to match AMD. > XMM 0-7 isn't modified without OSFXSR, so I'll just assume that AMD > won't break with that.) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-08 19:54 ` [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 2016-11-08 23:25 ` Paolo Bonzini @ 2016-11-09 18:07 ` Radim Krčmář 2016-11-09 18:42 ` kbuild test robot 1 sibling, 1 reply; 12+ messages in thread From: Radim Krčmář @ 2016-11-09 18:07 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: Paolo Bonzini, Bandan Das, Nadav Amit 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. AMD and Intel differ in saving and restoring other fields in first 32 bytes. A test wrote 0xff to the fxsave area, 0 to upper bits of MCSXR in the fxsave area, executed fxrstor, rewrote the fxsave area to 0xee, and executed fxsave: Intel (Nehalem): 7f 1f 7f 7f ff 00 ff 07 ff ff ff ff ff ff 00 00 ff ff ff ff ff ff 00 00 ff ff 00 00 ff ff 00 00 Intel (Haswell -- deprecated FPU CS and FPU DS): 7f 1f 7f 7f ff 00 ff 07 ff ff ff ff 00 00 00 00 ff ff ff ff 00 00 00 00 ff ff 00 00 ff ff 00 00 AMD (Opteron 2300-series): 7f 1f 7f 7f ff 00 ee ee ee ee ee ee ee ee ee ee ee ee ee ee ee ee ee ee ff ff 00 00 ff ff 02 00 fxsave/fxrstor will only be emulated on early Intels, so KVM can't do much to improve the situation. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- v4: - add support for CR4.OSFXSR [Paolo] I tested hardware implementation on AMD and Intel: neither of them save XMM 0-7, but both save and restore MCSXR without CR4.OSFXSR, so KVM behaves like that as well. v3: - remove fxsave64 and extra colons at the end of asm to make old GCCs happy (fxsave64 could have been implemented using other nmemonics, but there is no point when it won't be used + removing it makes the code nicer.) 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 | 129 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 6af3cac6ec89..7d4f9b7f06ee 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -3883,6 +3883,131 @@ 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); + + /* + * Don't emulate a case that should never be hit, instead of working + * around a lack of fxsave64/fxrstor64 on old compilers. + */ + if (ctxt->mode >= X86EMUL_MODE_PROT64) + return X86EMUL_UNHANDLEABLE; + + 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) and preserves XMM 8-15 to reach the + * desired result. (4) is not emulated. + * + * Note: Guest and host CPUID.(EAX=07H,ECX=0H):EBX[bit 13] (deprecate FPU CS + * and FPU DS) should match. + */ +static int em_fxsave(struct x86_emulate_ctxt *ctxt) +{ + struct fxregs_state fx_state; + size_t size; + int rc; + + rc = check_fxsr(ctxt); + if (rc != X86EMUL_CONTINUE) + return rc; + + ctxt->ops->get_fpu(ctxt); + + rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); + + ctxt->ops->put_fpu(ctxt); + + if (rc != X86EMUL_CONTINUE) + return rc; + + if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR) + size = offsetof(struct fxregs_state, xmm_space[8 * 16/4]); + else + size = offsetof(struct fxregs_state, xmm_space[0]); + + return segmented_write(ctxt, ctxt->memop.addr.mem, &fx_state, size); +} + +static int fxrstor_fixup(struct x86_emulate_ctxt *ctxt, + struct fxregs_state *new) +{ + int rc = X86EMUL_CONTINUE; + struct fxregs_state old; + + rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); + if (rc != X86EMUL_CONTINUE) + return rc; + + /* + * 64 bit host will restore XMM 8-15, which is not correct on non-64 + * bit guests. Load the current values in order to preserve 64 bit + * XMMs after fxrstor. + */ +#ifdef CONFIG_X86_64 + /* XXX: accessing XMM 8-15 very awkwardly */ + memcpy(&new->xmm_space[8 * 16/4], &old.xmm_space[8 * 16/4], 8 * 16); +#endif + + /* + * Hardware doesn't save and restore XMM 0-7 without CR4.OSFXSR, but + * does save and restore MXCSR. + */ + if (!(ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR)) + memcpy(new->xmm_space, old.xmm_space, 8 * 16); + + 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); + + if (ctxt->mode < X86EMUL_MODE_PROT64) + rc = fxrstor_fixup(ctxt, &fx_state); + + if (rc == X86EMUL_CONTINUE) + 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 +4360,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.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-09 18:07 ` [PATCH v4] " Radim Krčmář @ 2016-11-09 18:42 ` kbuild test robot 2016-11-09 18:46 ` Radim Krčmář 0 siblings, 1 reply; 12+ messages in thread From: kbuild test robot @ 2016-11-09 18:42 UTC (permalink / raw) To: Radim Krčmář Cc: kbuild-all, linux-kernel, kvm, Paolo Bonzini, Bandan Das, Nadav Amit [-- Attachment #1: Type: text/plain, Size: 4941 bytes --] Hi Radim, [auto build test ERROR on kvm/linux-next] [also build test ERROR on v4.9-rc4 next-20161109] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Radim-Kr-m/KVM-x86-emulate-FXSAVE-and-FXRSTOR/20161110-021048 base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next config: x86_64-randconfig-x018-201645 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 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:3910:7: error: implicit declaration of function 'asm_safe' [-Werror=implicit-function-declaration] rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); ^~~~~~~~ >> arch/x86/kvm/emulate.c:3910:32: error: expected expression before ',' token rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); ^ >> arch/x86/kvm/emulate.c:3910:35: error: 'fx' undeclared (first use in this function) rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); ^~ arch/x86/kvm/emulate.c:3910:35: note: each undeclared identifier is reported only once for each function it appears in arch/x86/kvm/emulate.c: In function 'fxrstor_fixup': arch/x86/kvm/emulate.c:3931:32: error: expected expression before ',' token rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); ^ arch/x86/kvm/emulate.c:3931:35: error: 'fx' undeclared (first use in this function) rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); ^~ arch/x86/kvm/emulate.c: In function 'em_fxrstor': >> arch/x86/kvm/emulate.c:3977:34: error: expected expression before ':' token rc = asm_safe("fxrstor %[fx]", : [fx] "m"(fx_state)); ^ arch/x86/kvm/emulate.c: At top level: >> arch/x86/kvm/emulate.c:4336:12: error: 'Aligned16' undeclared here (not in a function) I(ModRM | Aligned16, em_fxsave), ^ arch/x86/kvm/emulate.c:4185:31: note: in definition of macro 'I' #define I(_f, _e) { .flags = (_f), .u.execute = (_e) } ^~ cc1: some warnings being treated as errors vim +/asm_safe +3910 arch/x86/kvm/emulate.c 3904 rc = check_fxsr(ctxt); 3905 if (rc != X86EMUL_CONTINUE) 3906 return rc; 3907 3908 ctxt->ops->get_fpu(ctxt); 3909 > 3910 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); 3911 3912 ctxt->ops->put_fpu(ctxt); 3913 3914 if (rc != X86EMUL_CONTINUE) 3915 return rc; 3916 3917 if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR) 3918 size = offsetof(struct fxregs_state, xmm_space[8 * 16/4]); 3919 else 3920 size = offsetof(struct fxregs_state, xmm_space[0]); 3921 3922 return segmented_write(ctxt, ctxt->memop.addr.mem, &fx_state, size); 3923 } 3924 3925 static int fxrstor_fixup(struct x86_emulate_ctxt *ctxt, 3926 struct fxregs_state *new) 3927 { 3928 int rc = X86EMUL_CONTINUE; 3929 struct fxregs_state old; 3930 > 3931 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); 3932 if (rc != X86EMUL_CONTINUE) 3933 return rc; 3934 3935 /* 3936 * 64 bit host will restore XMM 8-15, which is not correct on non-64 3937 * bit guests. Load the current values in order to preserve 64 bit 3938 * XMMs after fxrstor. 3939 */ 3940 #ifdef CONFIG_X86_64 3941 /* XXX: accessing XMM 8-15 very awkwardly */ 3942 memcpy(&new->xmm_space[8 * 16/4], &old.xmm_space[8 * 16/4], 8 * 16); 3943 #endif 3944 3945 /* 3946 * Hardware doesn't save and restore XMM 0-7 without CR4.OSFXSR, but 3947 * does save and restore MXCSR. 3948 */ 3949 if (!(ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR)) 3950 memcpy(new->xmm_space, old.xmm_space, 8 * 16); 3951 3952 return rc; 3953 } 3954 3955 static int em_fxrstor(struct x86_emulate_ctxt *ctxt) 3956 { 3957 struct fxregs_state fx_state; 3958 int rc; 3959 3960 rc = check_fxsr(ctxt); 3961 if (rc != X86EMUL_CONTINUE) 3962 return rc; 3963 3964 rc = segmented_read(ctxt, ctxt->memop.addr.mem, &fx_state, 512); 3965 if (rc != X86EMUL_CONTINUE) 3966 return rc; 3967 3968 if (fx_state.mxcsr >> 16) 3969 return emulate_gp(ctxt, 0); 3970 3971 ctxt->ops->get_fpu(ctxt); 3972 3973 if (ctxt->mode < X86EMUL_MODE_PROT64) 3974 rc = fxrstor_fixup(ctxt, &fx_state); 3975 3976 if (rc == X86EMUL_CONTINUE) > 3977 rc = asm_safe("fxrstor %[fx]", : [fx] "m"(fx_state)); 3978 3979 ctxt->ops->put_fpu(ctxt); 3980 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 26282 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-09 18:42 ` kbuild test robot @ 2016-11-09 18:46 ` Radim Krčmář 2016-11-10 2:47 ` Fengguang Wu 0 siblings, 1 reply; 12+ messages in thread From: Radim Krčmář @ 2016-11-09 18:46 UTC (permalink / raw) To: kbuild test robot Cc: kbuild-all, linux-kernel, kvm, Paolo Bonzini, Bandan Das, Nadav Amit 2016-11-10 02:42+0800, kbuild test robot: > Hi Radim, And I even replied to the series, so this would happen ... Btw. would the tool recognize it if the header was [PATCH v4 4/4]? > [auto build test ERROR on kvm/linux-next] > [also build test ERROR on v4.9-rc4 next-20161109] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Radim-Kr-m/KVM-x86-emulate-FXSAVE-and-FXRSTOR/20161110-021048 > base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next > config: x86_64-randconfig-x018-201645 (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > 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:3910:7: error: implicit declaration of function 'asm_safe' [-Werror=implicit-function-declaration] > rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); > ^~~~~~~~ > >> arch/x86/kvm/emulate.c:3910:32: error: expected expression before ',' token > rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); > ^ > >> arch/x86/kvm/emulate.c:3910:35: error: 'fx' undeclared (first use in this function) > rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); > ^~ > arch/x86/kvm/emulate.c:3910:35: note: each undeclared identifier is reported only once for each function it appears in > arch/x86/kvm/emulate.c: In function 'fxrstor_fixup': > arch/x86/kvm/emulate.c:3931:32: error: expected expression before ',' token > rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); > ^ > arch/x86/kvm/emulate.c:3931:35: error: 'fx' undeclared (first use in this function) > rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); > ^~ > arch/x86/kvm/emulate.c: In function 'em_fxrstor': > >> arch/x86/kvm/emulate.c:3977:34: error: expected expression before ':' token > rc = asm_safe("fxrstor %[fx]", : [fx] "m"(fx_state)); > ^ > arch/x86/kvm/emulate.c: At top level: > >> arch/x86/kvm/emulate.c:4336:12: error: 'Aligned16' undeclared here (not in a function) > I(ModRM | Aligned16, em_fxsave), > ^ > arch/x86/kvm/emulate.c:4185:31: note: in definition of macro 'I' > #define I(_f, _e) { .flags = (_f), .u.execute = (_e) } > ^~ > cc1: some warnings being treated as errors > > vim +/asm_safe +3910 arch/x86/kvm/emulate.c > > 3904 rc = check_fxsr(ctxt); > 3905 if (rc != X86EMUL_CONTINUE) > 3906 return rc; > 3907 > 3908 ctxt->ops->get_fpu(ctxt); > 3909 > > 3910 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); > 3911 > 3912 ctxt->ops->put_fpu(ctxt); > 3913 > 3914 if (rc != X86EMUL_CONTINUE) > 3915 return rc; > 3916 > 3917 if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR) > 3918 size = offsetof(struct fxregs_state, xmm_space[8 * 16/4]); > 3919 else > 3920 size = offsetof(struct fxregs_state, xmm_space[0]); > 3921 > 3922 return segmented_write(ctxt, ctxt->memop.addr.mem, &fx_state, size); > 3923 } > 3924 > 3925 static int fxrstor_fixup(struct x86_emulate_ctxt *ctxt, > 3926 struct fxregs_state *new) > 3927 { > 3928 int rc = X86EMUL_CONTINUE; > 3929 struct fxregs_state old; > 3930 > > 3931 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); > 3932 if (rc != X86EMUL_CONTINUE) > 3933 return rc; > 3934 > 3935 /* > 3936 * 64 bit host will restore XMM 8-15, which is not correct on non-64 > 3937 * bit guests. Load the current values in order to preserve 64 bit > 3938 * XMMs after fxrstor. > 3939 */ > 3940 #ifdef CONFIG_X86_64 > 3941 /* XXX: accessing XMM 8-15 very awkwardly */ > 3942 memcpy(&new->xmm_space[8 * 16/4], &old.xmm_space[8 * 16/4], 8 * 16); > 3943 #endif > 3944 > 3945 /* > 3946 * Hardware doesn't save and restore XMM 0-7 without CR4.OSFXSR, but > 3947 * does save and restore MXCSR. > 3948 */ > 3949 if (!(ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR)) > 3950 memcpy(new->xmm_space, old.xmm_space, 8 * 16); > 3951 > 3952 return rc; > 3953 } > 3954 > 3955 static int em_fxrstor(struct x86_emulate_ctxt *ctxt) > 3956 { > 3957 struct fxregs_state fx_state; > 3958 int rc; > 3959 > 3960 rc = check_fxsr(ctxt); > 3961 if (rc != X86EMUL_CONTINUE) > 3962 return rc; > 3963 > 3964 rc = segmented_read(ctxt, ctxt->memop.addr.mem, &fx_state, 512); > 3965 if (rc != X86EMUL_CONTINUE) > 3966 return rc; > 3967 > 3968 if (fx_state.mxcsr >> 16) > 3969 return emulate_gp(ctxt, 0); > 3970 > 3971 ctxt->ops->get_fpu(ctxt); > 3972 > 3973 if (ctxt->mode < X86EMUL_MODE_PROT64) > 3974 rc = fxrstor_fixup(ctxt, &fx_state); > 3975 > 3976 if (rc == X86EMUL_CONTINUE) > > 3977 rc = asm_safe("fxrstor %[fx]", : [fx] "m"(fx_state)); > 3978 > 3979 ctxt->ops->put_fpu(ctxt); > 3980 > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR 2016-11-09 18:46 ` Radim Krčmář @ 2016-11-10 2:47 ` Fengguang Wu 0 siblings, 0 replies; 12+ messages in thread From: Fengguang Wu @ 2016-11-10 2:47 UTC (permalink / raw) To: Radim Krčmář Cc: kbuild-all, linux-kernel, kvm, Paolo Bonzini, Bandan Das, Nadav Amit, Ye Xiaolong CC Xiaolong. On Wed, Nov 09, 2016 at 07:46:28PM +0100, Radim Krčmář wrote: >2016-11-10 02:42+0800, kbuild test robot: >> Hi Radim, > >And I even replied to the series, so this would happen ... >Btw. would the tool recognize it if the header was [PATCH v4 4/4]? > >> [auto build test ERROR on kvm/linux-next] >> [also build test ERROR on v4.9-rc4 next-20161109] >> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >> >> url: https://github.com/0day-ci/linux/commits/Radim-Kr-m/KVM-x86-emulate-FXSAVE-and-FXRSTOR/20161110-021048 >> base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next >> config: x86_64-randconfig-x018-201645 (attached as .config) >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 >> 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:3910:7: error: implicit declaration of function 'asm_safe' [-Werror=implicit-function-declaration] >> rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); >> ^~~~~~~~ >> >> arch/x86/kvm/emulate.c:3910:32: error: expected expression before ',' token >> rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); >> ^ >> >> arch/x86/kvm/emulate.c:3910:35: error: 'fx' undeclared (first use in this function) >> rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); >> ^~ >> arch/x86/kvm/emulate.c:3910:35: note: each undeclared identifier is reported only once for each function it appears in >> arch/x86/kvm/emulate.c: In function 'fxrstor_fixup': >> arch/x86/kvm/emulate.c:3931:32: error: expected expression before ',' token >> rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); >> ^ >> arch/x86/kvm/emulate.c:3931:35: error: 'fx' undeclared (first use in this function) >> rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); >> ^~ >> arch/x86/kvm/emulate.c: In function 'em_fxrstor': >> >> arch/x86/kvm/emulate.c:3977:34: error: expected expression before ':' token >> rc = asm_safe("fxrstor %[fx]", : [fx] "m"(fx_state)); >> ^ >> arch/x86/kvm/emulate.c: At top level: >> >> arch/x86/kvm/emulate.c:4336:12: error: 'Aligned16' undeclared here (not in a function) >> I(ModRM | Aligned16, em_fxsave), >> ^ >> arch/x86/kvm/emulate.c:4185:31: note: in definition of macro 'I' >> #define I(_f, _e) { .flags = (_f), .u.execute = (_e) } >> ^~ >> cc1: some warnings being treated as errors >> >> vim +/asm_safe +3910 arch/x86/kvm/emulate.c >> >> 3904 rc = check_fxsr(ctxt); >> 3905 if (rc != X86EMUL_CONTINUE) >> 3906 return rc; >> 3907 >> 3908 ctxt->ops->get_fpu(ctxt); >> 3909 >> > 3910 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); >> 3911 >> 3912 ctxt->ops->put_fpu(ctxt); >> 3913 >> 3914 if (rc != X86EMUL_CONTINUE) >> 3915 return rc; >> 3916 >> 3917 if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR) >> 3918 size = offsetof(struct fxregs_state, xmm_space[8 * 16/4]); >> 3919 else >> 3920 size = offsetof(struct fxregs_state, xmm_space[0]); >> 3921 >> 3922 return segmented_write(ctxt, ctxt->memop.addr.mem, &fx_state, size); >> 3923 } >> 3924 >> 3925 static int fxrstor_fixup(struct x86_emulate_ctxt *ctxt, >> 3926 struct fxregs_state *new) >> 3927 { >> 3928 int rc = X86EMUL_CONTINUE; >> 3929 struct fxregs_state old; >> 3930 >> > 3931 rc = asm_safe("fxsave %[fx]", , [fx] "+m"(old)); >> 3932 if (rc != X86EMUL_CONTINUE) >> 3933 return rc; >> 3934 >> 3935 /* >> 3936 * 64 bit host will restore XMM 8-15, which is not correct on non-64 >> 3937 * bit guests. Load the current values in order to preserve 64 bit >> 3938 * XMMs after fxrstor. >> 3939 */ >> 3940 #ifdef CONFIG_X86_64 >> 3941 /* XXX: accessing XMM 8-15 very awkwardly */ >> 3942 memcpy(&new->xmm_space[8 * 16/4], &old.xmm_space[8 * 16/4], 8 * 16); >> 3943 #endif >> 3944 >> 3945 /* >> 3946 * Hardware doesn't save and restore XMM 0-7 without CR4.OSFXSR, but >> 3947 * does save and restore MXCSR. >> 3948 */ >> 3949 if (!(ctxt->ops->get_cr(ctxt, 4) & X86_CR4_OSFXSR)) >> 3950 memcpy(new->xmm_space, old.xmm_space, 8 * 16); >> 3951 >> 3952 return rc; >> 3953 } >> 3954 >> 3955 static int em_fxrstor(struct x86_emulate_ctxt *ctxt) >> 3956 { >> 3957 struct fxregs_state fx_state; >> 3958 int rc; >> 3959 >> 3960 rc = check_fxsr(ctxt); >> 3961 if (rc != X86EMUL_CONTINUE) >> 3962 return rc; >> 3963 >> 3964 rc = segmented_read(ctxt, ctxt->memop.addr.mem, &fx_state, 512); >> 3965 if (rc != X86EMUL_CONTINUE) >> 3966 return rc; >> 3967 >> 3968 if (fx_state.mxcsr >> 16) >> 3969 return emulate_gp(ctxt, 0); >> 3970 >> 3971 ctxt->ops->get_fpu(ctxt); >> 3972 >> 3973 if (ctxt->mode < X86EMUL_MODE_PROT64) >> 3974 rc = fxrstor_fixup(ctxt, &fx_state); >> 3975 >> 3976 if (rc == X86EMUL_CONTINUE) >> > 3977 rc = asm_safe("fxrstor %[fx]", : [fx] "m"(fx_state)); >> 3978 >> 3979 ctxt->ops->put_fpu(ctxt); >> 3980 >> >> --- >> 0-DAY kernel test infrastructure Open Source Technology Center >> https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-11-10 2:47 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-08 19:54 [PATCH v3 0/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 1/4] KVM: x86: add Align16 instruction flag Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 2/4] KVM: x86: save one bit in ctxt->d Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 3/4] KVM: x86: add asm_safe wrapper Radim Krčmář 2016-11-08 19:54 ` [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář 2016-11-08 23:25 ` Paolo Bonzini 2016-11-09 12:12 ` Radim Krčmář 2016-11-09 14:19 ` Radim Krčmář 2016-11-09 18:07 ` [PATCH v4] " Radim Krčmář 2016-11-09 18:42 ` kbuild test robot 2016-11-09 18:46 ` Radim Krčmář 2016-11-10 2:47 ` Fengguang Wu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).