On 05/26/2010 12:19 PM, Sheng Yang wrote:
From: Dexuan Cuidexuan@intel.com
This patch enable guest to use XSAVE/XRSTORE instructions.
We assume that host_xcr0 would use all possible bits that OS supported.
And we loaded xcr0 in the same way we handled fpu - do it as late as we can.
Looks really good now, only a couple of minor comments and I think we're
done.
I've done a prototype of LM support, would send out tomorrow. But the test
case in QEmu side seems got something wrong. I always got an segfault at:
qemu-kvm/hw/fw_cfg.c:223
223 s-entries[arch][key].data = data;
Haven't looked into it yet. But maybe someone knows the reason...
I saw this too, then it disappeared, can't remember why. Perhaps a
clean build is needed?
+static int handle_xsetbv(struct kvm_vcpu *vcpu)
+{
+ u64 new_bv = kvm_read_edx_eax(vcpu);
+
+ if (kvm_register_read(vcpu, VCPU_REGS_RCX) != 0)
+ goto err;
+ if (vmx_get_cpl(vcpu) != 0)
+ goto err;
+ if (!(new_bv XSTATE_FP))
+ goto err;
+ if ((new_bv XSTATE_YMM) !(new_bv XSTATE_SSE))
+ goto err;
+ if (new_bv ~host_xcr0)
+ goto err;
+ vcpu-arch.xcr0 = new_bv;
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu-arch.xcr0);
Please move all the code above to kvm_set_xcr0() in x86.c, since it's
not vendor specific. This would also allow you to make host_xcr0 local
to x86.c.
+ skip_emulated_instruction(vcpu);
+ return 1;
+err:
+ kvm_inject_gp(vcpu, 0);
+ return 1;
+}
/*
* List of msr numbers which we expose to userspace through KVM_GET_MSRS
* and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
@@ -1813,6 +1847,14 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu
*vcpu,
r = 0;
kvm_apic_set_version(vcpu);
kvm_x86_ops-cpuid_update(vcpu);
+ update_cpuid(vcpu);
+
+ /*
+* Ensure guest xcr0 is valid for loading, also using as
+* the indicator of if guest cpuid has XSAVE
+*/
+ if (guest_cpuid_has_xsave(vcpu))
+ vcpu-arch.xcr0 = XSTATE_FP;
This is problematic because it enforces an ordering between KVM_SET_XCR
and KVM_SET_CPUID. So I think you can use kvm_read_cr4_bits(OSXSAVE)
instead of checking vcpu-arch.xcr0. Sorry for the bad advice earlier.
@@ -5134,12 +5207,26 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
vcpu-guest_fpu_loaded = 1;
unlazy_fpu(current);
+ /*
+* Restore all possible states in the guest,
+* and assume host would use all available bits.
+* Guest xcr0 would be loaded later.
+*/
+ if (cpu_has_xsave vcpu-arch.xcr0) {
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
+ vcpu-guest_xcr0_loaded = 0;
+ }
Has to be before unlazy_fpu(), so host fpu uses host xcr0.
It's sufficient to check for guest cr4.osxsave, no need to check for
cpu_has_xsave. But you need to check for guest_xcr0_loaded!
fpu_restore_checking(vcpu-arch.guest_fpu);
trace_kvm_fpu(1);
}
void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
{
+ if (vcpu-guest_xcr0_loaded) {
+ vcpu-guest_xcr0_loaded = 0;
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
+ }
+
This duplicates the above. So better to have
kvm_load_guest_xcr0()/kvm_put_guest_xcr0().
--
error compiling committee.c: too many arguments to function
