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

Groups > linux.kernel > #1517533 > unrolled thread

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

Started byRadim Krčmář <rkrcmar@redhat.com>
First post2016-11-08 21:00 +0100
Last post2016-11-10 03:50 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [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

#1517533 — [PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR

FromRadim Krčmář <rkrcmar@redhat.com>
Date2016-11-08 21:00 +0100
Subject[PATCH v3 4/4] KVM: x86: emulate FXSAVE and FXRSTOR
Message-ID<sBkIO-5kY-11@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1517646

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-11-09 00:30 +0100
Message-ID<sBo01-7H0-5@gated-at.bofh.it>
In reply to#1517533

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,
>  } };
>

[toc] | [prev] | [next] | [standalone]

#1518111

FromRadim Krčmář <rkrcmar@redhat.com>
Date2016-11-09 13:20 +0100
Message-ID<sBA1c-7dw-15@gated-at.bofh.it>
In reply to#1517646
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.

[toc] | [prev] | [next] | [standalone]

#1518196

FromRadim Krčmář <rkrcmar@redhat.com>
Date2016-11-09 15:20 +0100
Message-ID<sBBTj-8ph-17@gated-at.bofh.it>
In reply to#1518111
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.)

[toc] | [prev] | [next] | [standalone]

#1518346 — [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR

FromRadim Krčmář <rkrcmar@redhat.com>
Date2016-11-09 19:10 +0100
Subject[PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR
Message-ID<sBFtU-2lK-29@gated-at.bofh.it>
In reply to#1517533
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

[toc] | [prev] | [next] | [standalone]

#1518408 — Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR

FromRadim Krčmář <rkrcmar@redhat.com>
Date2016-11-09 19:50 +0100
SubjectRe: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR
Message-ID<sBG6B-2AD-15@gated-at.bofh.it>
In reply to#1518346
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

[toc] | [prev] | [next] | [standalone]

#1518653 — Re: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR

FromFengguang Wu <lkp@intel.com>
Date2016-11-10 03:50 +0100
SubjectRe: [PATCH v4] KVM: x86: emulate FXSAVE and FXRSTOR
Message-ID<sBNB7-7Q6-3@gated-at.bofh.it>
In reply to#1518408
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web