On 05/26/2010 12:19 PM, Sheng Yang wrote:
From: Dexuan Cui<dexuan.cui@xxxxxxxxx>
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
--
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
