Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1518111

Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR

From Radim Krčmář <rkrcmar@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR
Date 2016-11-09 13:20 +0100
Message-ID <sBA1c-7dw-15@gated-at.bofh.it> (permalink)
References <sBkIO-5kY-3@gated-at.bofh.it> <sBkIO-5kY-11@gated-at.bofh.it> <sBo01-7H0-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář <rkrcmar@redhat.com> - 2016-11-08 21:00 +0100
  Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Paolo Bonzini <pbonzini@redhat.com> - 2016-11-09 00:30 +0100
    Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář <rkrcmar@redhat.com> - 2016-11-09 13:20 +0100
      Re: [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář <rkrcmar@redhat.com> - 2016-11-09 15:20 +0100
  [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář <rkrcmar@redhat.com> - 2016-11-09 19:10 +0100
    Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR Radim Krčmář <rkrcmar@redhat.com> - 2016-11-09 19:50 +0100
      Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR Fengguang Wu <lkp@intel.com> - 2016-11-10 03:50 +0100

csiph-web