Re: [PATCH 08/21] KVM: x86: Reset FPU state during reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/11/2014 10:13, Nadav Amit wrote:
> 
>> On Nov 6, 2014, at 10:58, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>>
>> On 05/11/2014 21:31, Nadav Amit wrote:
>>> You are correct, it does not appear clearly in the SDM, but that is what real hardware does.
>>> If you look at bochs - http://code.metager.de/source/xref/bochs/bochs/cpu/init.cc - you’ll see they call
>>> "BX_CPU_THIS_PTR xcr0.set32(0x1);” regardless to whether it is hardware or software reset (the latter happens on INIT).
>>
>> Fair enough. :)
> Thanks. It is turning harder to find references for the crazy x86 behaviour. :)

Indeed, I'll ask Intel to clarify this one too.

The crazy thing is that AMD doesn't say anything, either!  Their own 
manual just says "Hardware initializes XCR0 to 0000_0000_0000_0001h", 
but it doesn't say when.

>> Does the patch in
>> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/129060 look good?
> 
> Yes. However, re-reviewing the patches both my patch and yours actually do something slightly different than the spec: they clear XMM8-15 and YMM[128:…] which should not happen on INIT according to the spec.

Yes, my patch just wanted to have the same effect as yours, but 
fpu_finit must remain in fx_alloc.  Setting cr0 is also unnecessary, 
since vmx_vcpu_reset and svm_vcpu_reset both do this.

> Fixing it might be a bit intrusive. What do you say?

I think it's easy if we start with my version of the change:

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index dc932d388c43..aba13df4e0ec 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -915,8 +915,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 e0260ccd78a4..0ef4c0b27248 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6868,7 +6868,7 @@ 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;
 
@@ -6878,16 +6878,8 @@ int fx_init(struct kvm_vcpu *vcpu)
 
 	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;
 }
-EXPORT_SYMBOL_GPL(fx_init);
 
 static void fx_free(struct kvm_vcpu *vcpu)
 {
@@ -7025,6 +7017,11 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu)
 	vcpu->arch.regs_avail = ~0;
 	vcpu->arch.regs_dirty = ~0;
 
+	/*
+	 * Ensure guest xcr0 is valid for loading
+	 */
+	vcpu->arch.xcr0 = XSTATE_FP;
+
 	kvm_x86_ops->vcpu_reset(vcpu);
 }
 
--
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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux