Re: [PATCH 2/4] Fix FPU interface changes

[Jan Kiszka <jan.kiszka@web.de>]

[-- 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 --]