Re: [PATCH][v3] KVM: VMX: Enable XSAVE/XRSTORE for guest

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

 



On 05/21/2010 10:26 AM, Sheng Yang wrote:

+
+static void update_cpuid(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 1, 0);
+	if (!best)
+		return;
+
+	/* Update OSXSAVE bit */
+	if (cpu_has_xsave&&   best->function == 0x1) {
+		best->ecx&= ~(bit(X86_FEATURE_OSXSAVE));
+		if (kvm_read_cr4(vcpu)&   X86_CR4_OSXSAVE)
+			best->ecx |= bit(X86_FEATURE_OSXSAVE);
+	}
+}
Note: need to update after userspace writes cpuid as well.
Not quite understand. Userspace set OSXSAVE should be trimmed IMO...

Two cases: userspace does KVM_SET_CPUID2 with osxsave set but cr4.xsave clear, or the other way round.

So we should set cpuid.osxsave depending to cr4.xsave whenever cr4 OR cpuid is modified, and completely ignore userspace setting for that bit.

@@ -497,6 +532,9 @@ int __kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned
long cr4)

   	if ((cr4 ^ old_cr4)&   pdptr_bits)
   	
   		kvm_mmu_reset_context(vcpu);

+	if ((cr4 ^ old_cr4)&   X86_CR4_OSXSAVE)
+		update_cpuid(vcpu);
+
I think we need to reload the guest's xcr0 at this point.
Alternatively, call vmx_load_host_state() to ensure the the next entry
will reload it.
Current xcr0 would be loaded when next vmentry.

True.

And if we use prepare_guest_switch(), how about SVM?

kvm_arch_vcpu_load() looks like a good place, as long as interrupts don't use the fpu.


@@ -5134,6 +5197,10 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)

   	vcpu->guest_fpu_loaded = 1;
   	unlazy_fpu(current);

+	/* Restore all possible states in the guest */
+	if (cpu_has_xsave&&   guest_cpuid_has_xsave(vcpu))
+		xsetbv(XCR_XFEATURE_ENABLED_MASK,
+			cpuid_get_possible_xcr0(vcpu));
Best to calculate it out of the fast path, when guest cpuid is set.
Need to check it at this time as well.
You mean guest_cpuid_has_xsave()? Not quite understand the point here...

Also cpuid_get_possible_cr0().  So we have something like

   if (vcpu->save_xcr0)
       xsetbv(vcpu->save_xcr0);

Those cpuid functions have loops, we don't want them running every context switch.

Also can avoid it if guest xcr0 == host xcr0.
I don't know the assumption that "host use all possible xcr0 bits" can apply. If
so, only use host_xcr0 should be fine.

I think we can rely on it. Those bits are a service to userspace and the guest is just a different kind of userspace, so it makes sense to expose the same set.

Would update other points. Thanks.

Thanks.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

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