From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/4] Fix FPU interface changes Date: Wed, 26 May 2010 21:26:32 +0200 Message-ID: <4BFD75E8.7030306@web.de> References: <4BFC96DE.7040702@redhat.com> <4BFCCEDD.7090802@web.de> <4BFD6587.5070702@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig987368585BA98CFE4E318CB5" Cc: kvm , Avi Kivity , Gleb Natapov To: Zachary Amsden Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:41824 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755380Ab0EZT0e (ORCPT ); Wed, 26 May 2010 15:26:34 -0400 In-Reply-To: <4BFD6587.5070702@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig987368585BA98CFE4E318CB5 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Zachary Amsden wrote: > On 05/25/2010 09:33 PM, Jan Kiszka wrote: >> Zachary Amsden wrote: >> =20 >>> The FPU interface changed to dynamic allocation; redefine the missing= >>> struct fpu to be a static structure and add back the old macros. >>> >>> The FPU interface changed to dynamic allocation; redefine the missing= >>> struct fpu to be a static structure and add back the old macros. >>> >>> >>> >>> From 95be2b1ac41a40f2f3d581f0674b25ca4b70a025 Mon Sep 17 00:00:00 20= 01 >>> From: Zachary Amsden >>> Date: Tue, 25 May 2010 16:32:14 -1000 >>> Subject: [PATCH 2/5] Extend module compatability to FPU code >>> >>> Signed-off-by: Zachary Amsden >>> --- >>> x86/external-module-compat.h | 84 >>> ++++++++++++++++++++++++++++++++++++++++++ >>> 1 files changed, 84 insertions(+), 0 deletions(-) >>> >>> diff --git a/x86/external-module-compat.h b/x86/external-module-compa= t.h >>> index 09bf232..316f7ff 100644 >>> --- a/x86/external-module-compat.h >>> +++ b/x86/external-module-compat.h >>> @@ -788,3 +788,87 @@ struct kvm_pvclock_vcpu_time_info { >>> #else >>> #define kvm_pvclock_vcpu_time_info pvclock_vcpu_time_info >>> #endif >>> + >>> +#if LINUX_VERSION_CODE< KERNEL_VERSION(2,6,34) >>> =20 >> The FPU changes look like 2.6.36 material. >> =20 >=20 > It's quite likely I got the version wrong. >=20 >=20 >>> +#include >>> +#include >>> +struct _fpu { >>> + struct i387_fxsave_struct fxsave; >>> + struct i387_fxsave_struct host_fx_image; >>> =20 >> The original code before 9144b51caf only used a single state - for the= >> guest. >> >> =20 >>> +}; >>> + >>> +struct fpu { >>> + struct _fpu *state; >>> + struct _fpu state_static; >>> +}; >>> + >>> +static inline bool fpu_allocated(struct fpu *fpu) >>> +{ >>> + fpu->state =3D&fpu->state_static; >>> + return true; >>> +} >>> =20 >> Unused. >> >> =20 >>> + >>> +static inline int fpu_alloc(struct fpu *fpu) >>> +{ >>> + fpu->state =3D&fpu->state_static; >>> + return 0; >>> +} >>> =20 >> That's smarter than the dumb copy of upstream fpu_alloc that I hacked = up. >> >> =20 >>> + >>> +static inline void fpu_free(struct fpu *fpu) >>> +{ >>> +} >>> + >>> +static inline void kvm_fx_finit(void) >>> +{ >>> + asm("finit"); >>> +} >>> + >>> +static inline void kvm_fx_save(struct i387_fxsave_struct *image) >>> +{ >>> + asm("fxsave (%0)":: "r" (image)); >>> +} >>> + >>> +static inline void kvm_fx_restore(struct i387_fxsave_struct *image) >>> +{ >>> + asm("fxrstor (%0)":: "r" (image)); >>> +} >>> + >>> +static inline void fpu_finit(struct fpu *fpu) >>> +{ >>> + unsigned after_mxcsr_mask; >>> + >>> + /* >>> + * Touch the fpu the first time in non atomic context as if >>> + * this is the first fpu instruction the exception handler >>> + * will fire before the instruction returns and it'll have to= >>> + * allocate ram with GFP_KERNEL. >>> + */ >>> + if (!used_math()) >>> + kvm_fx_save(&fpu->state->host_fx_image); >>> + >>> + /* Initialize guest FPU by resetting ours and saving into >>> guest's */ >>> + preempt_disable(); >>> + kvm_fx_save(&fpu->state->host_fx_image); >>> + kvm_fx_finit(); >>> + kvm_fx_save(&fpu->state->fxsave); >>> + kvm_fx_restore(&fpu->state->host_fx_image); >>> + preempt_enable(); >>> =20 >> The original KVM code before 9144b51caf just did finit+fxsave. Why thi= s >> saving/restoring of the host state? >> =20 >=20 > It looks like the new FPU contract allows one to "allocate" an FPU > context for the thread. Older kernels might have some other thread's > FPU state lazily lying around? It wasn't clear, I didn't care to check= > all kernel, and this code is verbatim from 2.6.33 asm/i387.h I think it's safe to assume that previous KVM code, including that which is delivered with 2.6.33 or .34, is sufficient for our purpose. So I basically implanted it into the wrappers you can find in my next branch. Builds are fine for all versions (I'm now just waiting for the final warnings fix being merged). And it works fine for me, but I only tested with a 2.6.27 host kernel. Please give it a try. Jan --------------enig987368585BA98CFE4E318CB5 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkv9degACgkQitSsb3rl5xRP8ACeLk29AUeVYGrMeBflq7wYsSm+ ZvUAoMibfpMOTTFOkDY7DlpwDpA9Qswg =Rse0 -----END PGP SIGNATURE----- --------------enig987368585BA98CFE4E318CB5--