[Top][All Lists]

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

[Qemu-devel] Re: [PATCH v5] KVM: VMX: Enable XSAVE/XRSTORE for guest

From: Avi Kivity
Subject: [Qemu-devel] Re: [PATCH v5] KVM: VMX: Enable XSAVE/XRSTORE for guest
Date: Wed, 26 May 2010 12:54:12 +0300
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20100330 Fedora/3.0.4-1.fc12 Thunderbird/3.0.4

On 05/26/2010 12:19 PM, Sheng Yang wrote:
From: Dexuan Cui<address@hidden>

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:
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;
+       kvm_inject_gp(vcpu, 0);
+       return 1;

   * List of msr numbers which we expose to userspace through KVM_GET_MSRS
@@ -1813,6 +1847,14 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu 
        r = 0;
+       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;
+       /*
+        * 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!


  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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]