On 02/11/2014 10:54, Nadav Amit wrote:
> When resetting the VCPU, the FPU should be reset as well (e.g., XCR0 state).
> Call fx_init during reset as well.
Actually it shouldn't be after INIT. XCR0 is not mentioned explicitly
in Table 9-1 of the SDM (IA-32 Processor States Following Power-up,
Reset, or INIT), but since MSR_IA32_XSS is not specified, I think XCR0
should fall under "All other MSRs".
> Signed-off-by: Nadav Amit <namit@xxxxxxxxxxxxxxxxx>
> ---
> arch/x86/kvm/x86.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 773c17e..9b90ea7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7020,6 +7020,9 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu)
> vcpu->arch.regs_avail = ~0;
> vcpu->arch.regs_dirty = ~0;
>
> + /* should never fail, since fpu_alloc already done */
> + fx_init(vcpu);
> +
> kvm_x86_ops->vcpu_reset(vcpu);
> }
>
>
Even then, I think this patch is not really nice... The call sequence
leading to kvm_vcpu_reset is:
kvm_vm_ioctl_create_vcpu
kvm_arch_vcpu_create
kvm_vcpu_init
kvm_arch_vcpu_init
fx_init (does fpu_alloc)
kvm_arch_vcpu_setup
kvm_vcpu_reset
fx_init (no fpu_alloc)
The FPU state is not needed between kvm_arch_vcpu_init and
kvm_arch_vcpu_setup. So we could simply move the reset from
kvm_vcpu_init to kvm_vcpu_reset, like this:
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 904535fe825e..eaa3be26dfdc 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -914,8 +914,6 @@ void kvm_pic_clear_all(struct kvm_pic *pic, int irq_source_id);
void kvm_inject_nmi(struct kvm_vcpu *vcpu);
-int fx_init(struct kvm_vcpu *vcpu);
-
void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
const u8 *new, int bytes);
int kvm_mmu_unprotect_page(struct kvm *kvm, gfn_t gfn);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 773c17ec42dd..a0566efbb77f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6863,26 +6863,10 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
return 0;
}
-int fx_init(struct kvm_vcpu *vcpu)
+static int fx_init(struct kvm_vcpu *vcpu)
{
- int err;
-
- err = fpu_alloc(&vcpu->arch.guest_fpu);
- if (err)
- return err;
-
- fpu_finit(&vcpu->arch.guest_fpu);
-
- /*
- * Ensure guest xcr0 is valid for loading
- */
- vcpu->arch.xcr0 = XSTATE_FP;
-
- vcpu->arch.cr0 |= X86_CR0_ET;
-
- return 0;
+ return fpu_alloc(&vcpu->arch.guest_fpu);
}
-EXPORT_SYMBOL_GPL(fx_init);
static void fx_free(struct kvm_vcpu *vcpu)
{
@@ -7020,6 +7004,15 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu)
vcpu->arch.regs_avail = ~0;
vcpu->arch.regs_dirty = ~0;
+ fpu_finit(&vcpu->arch.guest_fpu);
+
+ /*
+ * Ensure guest xcr0 is valid for loading
+ */
+ vcpu->arch.xcr0 = XSTATE_FP;
+
+ vcpu->arch.cr0 |= X86_CR0_ET;
+
kvm_x86_ops->vcpu_reset(vcpu);
}
However, as said above I'm not applying either patch, at least for now.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
