* [PATCH 2/4] Fix FPU interface changes
@ 2010-05-26 3:34 Zachary Amsden
2010-05-26 7:33 ` Jan Kiszka
0 siblings, 1 reply; 4+ messages in thread
From: Zachary Amsden @ 2010-05-26 3:34 UTC (permalink / raw)
To: kvm, Jan Kiszka, Avi Kivity, Gleb Natapov
[-- Attachment #1: Type: text/plain, Size: 136 bytes --]
The FPU interface changed to dynamic allocation; redefine the missing
struct fpu to be a static structure and add back the old macros.
[-- Attachment #2: 0002-Extend-module-compatability-to-FPU-code.patch --]
[-- Type: text/plain, Size: 2747 bytes --]
>From 95be2b1ac41a40f2f3d581f0674b25ca4b70a025 Mon Sep 17 00:00:00 2001
From: Zachary Amsden <zamsden@redhat.com>
Date: Tue, 25 May 2010 16:32:14 -1000
Subject: [PATCH 2/5] Extend module compatability to FPU code
Signed-off-by: Zachary Amsden <zamsden@redhat.com>
---
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-compat.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)
+#include <asm/processor.h>
+#include <asm/i387.h>
+struct _fpu {
+ struct i387_fxsave_struct fxsave;
+ struct i387_fxsave_struct host_fx_image;
+};
+
+struct fpu {
+ struct _fpu *state;
+ struct _fpu state_static;
+};
+
+static inline bool fpu_allocated(struct fpu *fpu)
+{
+ fpu->state = &fpu->state_static;
+ return true;
+}
+
+static inline int fpu_alloc(struct fpu *fpu)
+{
+ fpu->state = &fpu->state_static;
+ return 0;
+}
+
+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();
+
+ after_mxcsr_mask = offsetof(struct i387_fxsave_struct, st_space);
+ fpu->state->fxsave.mxcsr = 0x1f80;
+ memset((void *)&fpu->state->fxsave + after_mxcsr_mask,
+ 0, sizeof(struct i387_fxsave_struct) - after_mxcsr_mask);
+}
+
+static inline void fpu_restore_checking(struct fpu *fpu)
+{
+ kvm_fx_save(&fpu->state->host_fx_image);
+ kvm_fx_restore(&fpu->state->fxsave);
+}
+
+static inline void fpu_save_init(struct fpu *fpu)
+{
+ kvm_fx_save(&fpu->state->fxsave);
+ kvm_fx_restore(&fpu->state->host_fx_image);
+}
+#endif
--
1.7.0.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 2/4] Fix FPU interface changes
2010-05-26 3:34 [PATCH 2/4] Fix FPU interface changes Zachary Amsden
@ 2010-05-26 7:33 ` Jan Kiszka
2010-05-26 18:16 ` Zachary Amsden
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2010-05-26 7:33 UTC (permalink / raw)
To: Zachary Amsden; +Cc: kvm, Avi Kivity, Gleb Natapov
[-- Attachment #1: Type: text/plain, Size: 3740 bytes --]
Zachary Amsden wrote:
> 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 2001
> From: Zachary Amsden <zamsden@redhat.com>
> Date: Tue, 25 May 2010 16:32:14 -1000
> Subject: [PATCH 2/5] Extend module compatability to FPU code
>
> Signed-off-by: Zachary Amsden <zamsden@redhat.com>
> ---
> 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-compat.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)
The FPU changes look like 2.6.36 material.
> +#include <asm/processor.h>
> +#include <asm/i387.h>
> +struct _fpu {
> + struct i387_fxsave_struct fxsave;
> + struct i387_fxsave_struct host_fx_image;
The original code before 9144b51caf only used a single state - for the
guest.
> +};
> +
> +struct fpu {
> + struct _fpu *state;
> + struct _fpu state_static;
> +};
> +
> +static inline bool fpu_allocated(struct fpu *fpu)
> +{
> + fpu->state = &fpu->state_static;
> + return true;
> +}
Unused.
> +
> +static inline int fpu_alloc(struct fpu *fpu)
> +{
> + fpu->state = &fpu->state_static;
> + return 0;
> +}
That's smarter than the dumb copy of upstream fpu_alloc that I hacked up.
> +
> +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();
The original KVM code before 9144b51caf just did finit+fxsave. Why this
saving/restoring of the host state?
> +
> + after_mxcsr_mask = offsetof(struct i387_fxsave_struct, st_space);
> + fpu->state->fxsave.mxcsr = 0x1f80;
> + memset((void *)&fpu->state->fxsave + after_mxcsr_mask,
> + 0, sizeof(struct i387_fxsave_struct) - after_mxcsr_mask);
> +}
> +
> +static inline void fpu_restore_checking(struct fpu *fpu)
> +{
> + kvm_fx_save(&fpu->state->host_fx_image);
> + kvm_fx_restore(&fpu->state->fxsave);
> +}
> +
> +static inline void fpu_save_init(struct fpu *fpu)
> +{
> + kvm_fx_save(&fpu->state->fxsave);
> + kvm_fx_restore(&fpu->state->host_fx_image);
> +}
Also here: Why bother about the host state?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/4] Fix FPU interface changes
2010-05-26 7:33 ` Jan Kiszka
@ 2010-05-26 18:16 ` Zachary Amsden
2010-05-26 19:26 ` Jan Kiszka
0 siblings, 1 reply; 4+ messages in thread
From: Zachary Amsden @ 2010-05-26 18:16 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm, Avi Kivity, Gleb Natapov
On 05/25/2010 09:33 PM, Jan Kiszka wrote:
> Zachary Amsden wrote:
>
>> 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 2001
>> From: Zachary Amsden<zamsden@redhat.com>
>> Date: Tue, 25 May 2010 16:32:14 -1000
>> Subject: [PATCH 2/5] Extend module compatability to FPU code
>>
>> Signed-off-by: Zachary Amsden<zamsden@redhat.com>
>> ---
>> 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-compat.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)
>>
> The FPU changes look like 2.6.36 material.
>
It's quite likely I got the version wrong.
>> +#include<asm/processor.h>
>> +#include<asm/i387.h>
>> +struct _fpu {
>> + struct i387_fxsave_struct fxsave;
>> + struct i387_fxsave_struct host_fx_image;
>>
> The original code before 9144b51caf only used a single state - for the
> guest.
>
>
>> +};
>> +
>> +struct fpu {
>> + struct _fpu *state;
>> + struct _fpu state_static;
>> +};
>> +
>> +static inline bool fpu_allocated(struct fpu *fpu)
>> +{
>> + fpu->state =&fpu->state_static;
>> + return true;
>> +}
>>
> Unused.
>
>
>> +
>> +static inline int fpu_alloc(struct fpu *fpu)
>> +{
>> + fpu->state =&fpu->state_static;
>> + return 0;
>> +}
>>
> That's smarter than the dumb copy of upstream fpu_alloc that I hacked up.
>
>
>> +
>> +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();
>>
> The original KVM code before 9144b51caf just did finit+fxsave. Why this
> saving/restoring of the host state?
>
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
>> +
>> + after_mxcsr_mask = offsetof(struct i387_fxsave_struct, st_space);
>> + fpu->state->fxsave.mxcsr = 0x1f80;
>> + memset((void *)&fpu->state->fxsave + after_mxcsr_mask,
>> + 0, sizeof(struct i387_fxsave_struct) - after_mxcsr_mask);
>> +}
>> +
>> +static inline void fpu_restore_checking(struct fpu *fpu)
>> +{
>> + kvm_fx_save(&fpu->state->host_fx_image);
>> + kvm_fx_restore(&fpu->state->fxsave);
>> +}
>> +
>> +static inline void fpu_save_init(struct fpu *fpu)
>> +{
>> + kvm_fx_save(&fpu->state->fxsave);
>> + kvm_fx_restore(&fpu->state->host_fx_image);
>> +}
>>
> Also here: Why bother about the host state?
>
Ditto.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/4] Fix FPU interface changes
2010-05-26 18:16 ` Zachary Amsden
@ 2010-05-26 19:26 ` Jan Kiszka
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kiszka @ 2010-05-26 19:26 UTC (permalink / raw)
To: Zachary Amsden; +Cc: kvm, Avi Kivity, Gleb Natapov
[-- Attachment #1: Type: text/plain, Size: 4320 bytes --]
Zachary Amsden wrote:
> On 05/25/2010 09:33 PM, Jan Kiszka wrote:
>> Zachary Amsden wrote:
>>
>>> 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 2001
>>> From: Zachary Amsden<zamsden@redhat.com>
>>> Date: Tue, 25 May 2010 16:32:14 -1000
>>> Subject: [PATCH 2/5] Extend module compatability to FPU code
>>>
>>> Signed-off-by: Zachary Amsden<zamsden@redhat.com>
>>> ---
>>> 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-compat.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)
>>>
>> The FPU changes look like 2.6.36 material.
>>
>
> It's quite likely I got the version wrong.
>
>
>>> +#include<asm/processor.h>
>>> +#include<asm/i387.h>
>>> +struct _fpu {
>>> + struct i387_fxsave_struct fxsave;
>>> + struct i387_fxsave_struct host_fx_image;
>>>
>> The original code before 9144b51caf only used a single state - for the
>> guest.
>>
>>
>>> +};
>>> +
>>> +struct fpu {
>>> + struct _fpu *state;
>>> + struct _fpu state_static;
>>> +};
>>> +
>>> +static inline bool fpu_allocated(struct fpu *fpu)
>>> +{
>>> + fpu->state =&fpu->state_static;
>>> + return true;
>>> +}
>>>
>> Unused.
>>
>>
>>> +
>>> +static inline int fpu_alloc(struct fpu *fpu)
>>> +{
>>> + fpu->state =&fpu->state_static;
>>> + return 0;
>>> +}
>>>
>> That's smarter than the dumb copy of upstream fpu_alloc that I hacked up.
>>
>>
>>> +
>>> +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();
>>>
>> The original KVM code before 9144b51caf just did finit+fxsave. Why this
>> saving/restoring of the host state?
>>
>
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-26 19:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-26 3:34 [PATCH 2/4] Fix FPU interface changes Zachary Amsden
2010-05-26 7:33 ` Jan Kiszka
2010-05-26 18:16 ` Zachary Amsden
2010-05-26 19:26 ` Jan Kiszka
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).