On 06/10/2010 06:27 AM, Sheng Yang wrote:
This patch enable save/restore of xsave state.

Signed-off-by: Sheng Yang<[email protected]>
---
  Documentation/kvm/api.txt  |   76 ++++++++++++++++++++++++++++++
  arch/x86/include/asm/kvm.h |   24 ++++++++++
  arch/x86/kvm/x86.c         |  110 ++++++++++++++++++++++++++++++++++++++++++++
  include/linux/kvm.h        |   12 +++++
  4 files changed, 222 insertions(+), 0 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 159b4ef..cb0afa2 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -922,6 +922,82 @@ Define which vcpu is the Bootstrap Processor (BSP).  
Values are the same
  as the vcpu id in KVM_CREATE_VCPU.  If this ioctl is not called, the default
  is vcpu 0.

+4.41 KVM_GET_XSAVE
+
+Capability: KVM_CAP_XSAVE
+Architectures: x86
+Type: vcpu ioctl
+Parameters: struct kvm_xsave (out)
+Returns: 0 on success, -1 on error
+
+struct kvm_xsave {
+       __u32 size;
+       __u32 region[1000];
+};
+
+This ioctl would copy current vcpu's xsave struct to the userspace.

How is size interpreted?

I think we can leave it out since it is implied by the xsave header.


+/* for KVM_CAP_XSAVE */
+struct kvm_xsave {
+       __u32 size;
+       __u32 reserved[3];
+       __u32 region[1020];
+};

That's different from the documentation.

+
+#define KVM_MAX_XCRS   1

Needs to be much bigger for future compatibility, since it's part of the ABI.

@@ -2373,6 +2375,60 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct 
kvm_vcpu *vcpu,
        return 0;
  }

+static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
+                                       struct kvm_xsave *guest_xsave)
+{
+       u32 size;
+
+       if (cpu_has_xsave)
+               size = sizeof(struct xsave_struct);
+       else
+               size = sizeof(struct i387_fxsave_struct);
+
+       guest_xsave->size = size;
+       memcpy(guest_xsave->region,&vcpu->arch.guest_fpu.state->xsave, size);

If !cpu_has_xsave, we can create a fake xsave header that says we have the fpu and sse, so userspace doesn't have to look at size.

You're memcpy()ing state->xsave unconditionally, even if !cpu_has_xsave. That works, but it's cleaner to access the ->fxsave member.

+
+static void kvm_vcpu_ioctl_x86_get_xcrs(struct kvm_vcpu *vcpu,
+                                       struct kvm_xcrs *guest_xcrs)
+{
+       guest_xcrs->nr_xcrs = 1;
+       guest_xcrs->flags = 0;
+       guest_xcrs->xcrs[0].value = vcpu->arch.xcr0;
+}


guest_xcrs[0].xcr = ...

What if !cpu_has_xsave?  Probably should set nr_xcrs = 0.

+
+static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
+                                      struct kvm_xcrs *guest_xcrs)
+{
+       if (guest_xcrs->nr_xcrs<  1)
+               return 1;

Not really needed.

+
+       /* Userspace may override the initial value of xcr0... */
+       if (guest_xcrs->xcrs[0].value != 0) {

Why check the value?  What about checking the xcrs[].xcr?

+               vcpu->arch.xcr0 = guest_xcrs->xcrs[0].value;
+               vcpu->guest_xcr0_loaded = 0;
+       }

Need to loop over the array. Also, call kvm_set_xcr() to get checks to be performed.

--
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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to