Skip to contentSkip to conversation

KVM: VMX: Enable XSAVE/XRSTOR for guest

PATCH Applied Sheng Yang 3 participants 6 messages Jun 10, 2010 – Jun 11, 2010

Subsystems (4)
lore
Share & tools
lore

Sheng Yang

author
#0 lore raw

PATCH v8 1/2 KVM: VMX: Enable XSAVE/XRSTOR for guest

From: Dexuan Cui <[email protected]>

This patch enable guest to use XSAVE/XRSTOR 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.

---
 arch/x86/include/asm/kvm_host.h |    2 +
 arch/x86/include/asm/vmx.h      |    1 +
 arch/x86/kvm/kvm_cache_regs.h   |    6 ++
 arch/x86/kvm/vmx.c              |   13 ++++
 arch/x86/kvm/x86.c              |  130 ++++++++++++++++++++++++++++++++++++--
 include/linux/kvm_host.h        |    2 +-
 6 files changed, 146 insertions(+), 8 deletions(-)
arch/x86/include/asm/kvm_host.h +2 -0
index 0cd0f29..91631b8 100644
@@ -302,6 +302,7 @@ struct kvm_vcpu_arch {
 	} update_pte;
 
 	struct fpu guest_fpu;
+	u64 xcr0;
 
 	gva_t mmio_fault_cr2;
 	struct kvm_pio_request pio;
@@ -605,6 +606,7 @@ int kvm_get_dr(struct kvm_vcpu *vcpu, int dr, unsigned long *val);
 unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu);
 void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw);
 void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l);
+int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr);
 
 int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
 int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data);
arch/x86/include/asm/vmx.h +1 -0
index 96a5886..9f0cbd9 100644
@@ -267,6 +267,7 @@ enum vmcs_field {
 #define EXIT_REASON_EPT_VIOLATION       48
 #define EXIT_REASON_EPT_MISCONFIG       49
 #define EXIT_REASON_WBINVD		54
+#define EXIT_REASON_XSETBV		55
 
 /*
  * Interruption-information format
arch/x86/kvm/kvm_cache_regs.h +6 -0
index d2a98f8..6491ac8 100644
@@ -71,4 +71,10 @@ static inline ulong kvm_read_cr4(struct kvm_vcpu *vcpu)
 	return kvm_read_cr4_bits(vcpu, ~0UL);
 }
 
+static inline u64 kvm_read_edx_eax(struct kvm_vcpu *vcpu)
+{
+	return (kvm_register_read(vcpu, VCPU_REGS_RAX) & -1u)
+		| ((u64)(kvm_register_read(vcpu, VCPU_REGS_RDX) & -1u) << 32);
+}
+
 #endif
arch/x86/kvm/vmx.c +13 -0
index 7fa6ea7..d5d6a89 100644
@@ -37,6 +37,8 @@
 #include <asm/vmx.h>
 #include <asm/virtext.h>
 #include <asm/mce.h>
+#include <asm/i387.h>
+#include <asm/xcr.h>
 
 #include "trace.h"
 
@@ -3393,6 +3395,16 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+static int handle_xsetbv(struct kvm_vcpu *vcpu)
+{
+	u64 new_bv = kvm_read_edx_eax(vcpu);
+	u32 index = kvm_register_read(vcpu, VCPU_REGS_RCX);
+
+	if (kvm_set_xcr(vcpu, index, new_bv) == 0)
+		skip_emulated_instruction(vcpu);
+	return 1;
+}
+
 static int handle_apic_access(struct kvm_vcpu *vcpu)
 {
 	return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
@@ -3671,6 +3683,7 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_TPR_BELOW_THRESHOLD]     = handle_tpr_below_threshold,
 	[EXIT_REASON_APIC_ACCESS]             = handle_apic_access,
 	[EXIT_REASON_WBINVD]                  = handle_wbinvd,
+	[EXIT_REASON_XSETBV]                  = handle_xsetbv,
 	[EXIT_REASON_TASK_SWITCH]             = handle_task_switch,
 	[EXIT_REASON_MCE_DURING_VMENTRY]      = handle_machine_check,
 	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
arch/x86/kvm/x86.c +123 -7

232 lines not on this page · raw message

include/linux/kvm_host.h +1 -1
index 2c62319..2d96555 100644
@@ -88,7 +88,7 @@ struct kvm_vcpu {
 	int srcu_idx;
 
 	int fpu_active;
-	int guest_fpu_loaded;
+	int guest_fpu_loaded, guest_xcr0_loaded;
 	wait_queue_head_t wq;
 	int sigset_active;
 	sigset_t sigset;
Signature · 1 line
1.7.0.1
author
#1 lore raw

PATCH 2/2 KVM: x86: XSAVE/XRSTOR live migration support

This patch enable save/restore of xsave state.

---
 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(-)
Documentation/kvm/api.txt +76 -0
index 159b4ef..cb0afa2 100644
@@ -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.
+
+4.42 KVM_SET_XSAVE
+
+Capability: KVM_CAP_XSAVE
+Architectures: x86
+Type: vcpu ioctl
+Parameters: struct kvm_xsave (in)
+Returns: 0 on success, -1 on error
+
+struct kvm_xsave {
+	__u32 size;
+	__u32 region[1000];
+};
+
+This ioctl would copy userspace's xsave struct to the kernel.
+
+4.43 KVM_GET_XCRS
+
+Capability: KVM_CAP_XCRS
+Architectures: x86
+Type: vcpu ioctl
+Parameters: struct kvm_xcrs (out)
+Returns: 0 on success, -1 on error
+
+struct kvm_xcr {
+	__u32 xcr;
+	__u32 reserved;
+	__u64 value;
+};
+
+struct kvm_xcrs {
+	__u32 nr_xcrs;
+	__u32 flags;
+	struct kvm_xcr xcrs[KVM_MAX_XCRS];
+	__u64 padding[16];
+};
+
+This ioctl would copy current vcpu's xcrs to the userspace.
+
+4.44 KVM_SET_XCRS
+
+Capability: KVM_CAP_XCRS
+Architectures: x86
+Type: vcpu ioctl
+Parameters: struct kvm_xcrs (in)
+Returns: 0 on success, -1 on error
+
+struct kvm_xcr {
+	__u32 xcr;
+	__u32 reserved;
+	__u64 value;
+};
+
+struct kvm_xcrs {
+	__u32 nr_xcrs;
+	__u32 flags;
+	struct kvm_xcr xcrs[KVM_MAX_XCRS];
+	__u64 padding[16];
+};
+
+This ioctl would set vcpu's xcr to the value userspace specified.
+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by
arch/x86/include/asm/kvm.h +24 -0
index ff90055..a12a24a 100644
@@ -22,6 +22,8 @@
 #define __KVM_HAVE_XEN_HVM
 #define __KVM_HAVE_VCPU_EVENTS
 #define __KVM_HAVE_DEBUGREGS
+#define __KVM_HAVE_XSAVE
+#define __KVM_HAVE_XCRS
 
 /* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256
@@ -299,4 +301,26 @@ struct kvm_debugregs {
 	__u64 reserved[9];
 };
 
+/* for KVM_CAP_XSAVE */
+struct kvm_xsave {
+	__u32 size;
+	__u32 reserved[3];
+	__u32 region[1020];
+};
+
+#define KVM_MAX_XCRS	1
+
+struct kvm_xcr {
+	__u32 xcr;
+	__u32 reserved;
+	__u64 value;
+};
+
+struct kvm_xcrs {
+	__u32 nr_xcrs;
+	__u32 flags;
+	struct kvm_xcr xcrs[KVM_MAX_XCRS];
+	__u64 padding[16];
+};
+
 #endif /* _ASM_X86_KVM_H */
arch/x86/kvm/x86.c +110 -0

128 lines not on this page · raw message

include/linux/kvm.h +12 -0
index 23ea022..6fd40f5 100644
@@ -524,6 +524,12 @@ struct kvm_enable_cap {
 #define KVM_CAP_PPC_OSI 52
 #define KVM_CAP_PPC_UNSET_IRQ 53
 #define KVM_CAP_ENABLE_CAP 54
+#ifdef __KVM_HAVE_XSAVE
+#define KVM_CAP_XSAVE 55
+#endif
+#ifdef __KVM_HAVE_XCRS
+#define KVM_CAP_XCRS 56
+#endif
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -714,6 +720,12 @@ struct kvm_clock_data {
 #define KVM_GET_DEBUGREGS         _IOR(KVMIO,  0xa1, struct kvm_debugregs)
 #define KVM_SET_DEBUGREGS         _IOW(KVMIO,  0xa2, struct kvm_debugregs)
 #define KVM_ENABLE_CAP            _IOW(KVMIO,  0xa3, struct kvm_enable_cap)
+/* Available with KVM_CAP_XSAVE */
+#define KVM_GET_XSAVE		  _IOR(KVMIO,  0xa4, struct kvm_xsave)
+#define KVM_SET_XSAVE		  _IOW(KVMIO,  0xa5, struct kvm_xsave)
+/* Available with KVM_CAP_XCRS */
+#define KVM_GET_XCRS		  _IOR(KVMIO,  0xa6, struct kvm_xcrs)
+#define KVM_SET_XCRS		  _IOW(KVMIO,  0xa7, struct kvm_xcrs)
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 
Signature · 1 line
1.7.0.1
#2 lore raw

Re: [PATCH 2/2] KVM: x86: XSAVE/XRSTOR live migration support

Quoted 23 lines · Sheng Yang

This patch enable save/restore of xsave state.

---
  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(-)
Documentation/kvm/api.txt +5 -0
index 159b4ef..cb0afa2 100644
@@ -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

Documentation/kvm/api.txt Define which vcpu is the Bootstrap Processor (BSP). Values are the same · line 939

Quoted 10 lines
Documentation/kvm/api.txt +9 -0
+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.

Documentation/kvm/api.txt Define which vcpu is the Bootstrap Processor (BSP). Values are the same · line 946

Quoted 8 lines
Documentation/kvm/api.txt +6 -0
+/* for KVM_CAP_XSAVE */
+struct kvm_xsave {
+	__u32 size;
+	__u32 reserved[3];
+	__u32 region[1020];
+};
 

That's different from the documentation.

Documentation/kvm/api.txt Define which vcpu is the Bootstrap Processor (BSP). Values are the same · line 949

Quoted 3 lines
Documentation/kvm/api.txt +2 -0
+
+#define KVM_MAX_XCRS	1
 

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

Quoted 7 lines
Documentation/kvm/api.txt +3 -0
@@ -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)
+{

Documentation/kvm/api.txt static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2390

Quoted 10 lines
Documentation/kvm/api.txt +9 -0
+	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.

Documentation/kvm/api.txt static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2399

Quoted 9 lines
Documentation/kvm/api.txt +8 -0
+
+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.

Documentation/kvm/api.txt static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2406

Quoted 7 lines
Documentation/kvm/api.txt +6 -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.

Documentation/kvm/api.txt static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2409

Quoted 4 lines
Documentation/kvm/api.txt +3 -0
+
+	/* 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?

Documentation/kvm/api.txt static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu, · line 2412

Quoted 4 lines
Documentation/kvm/api.txt +3 -0
+		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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#3 lore raw
In reply to Sheng Yang
Quoted 9 lines · Sheng Yang

From: Dexuan Cui<[email protected]>

This patch enable guest to use XSAVE/XRSTOR 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 good. Will wait for Marcelo to review before applying.

Signature · 1 line
error compiling committee.c: too many arguments to function
Reviewed-by on v8 1/2
#4 lore raw
In reply to Sheng Yang
Quoted 10 lines · Sheng Yang

From: Dexuan Cui <[email protected]>

This patch enable guest to use XSAVE/XRSTOR 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.

Quoted 8 lines
---
 arch/x86/include/asm/kvm_host.h |    2 +
 arch/x86/include/asm/vmx.h      |    1 +
 arch/x86/kvm/kvm_cache_regs.h   |    6 ++
 arch/x86/kvm/vmx.c              |   13 ++++
 arch/x86/kvm/x86.c              |  130 ++++++++++++++++++++++++++++++++++++--
 include/linux/kvm_host.h        |    2 +-
 6 files changed, 146 insertions(+), 8 deletions(-)
Applied
#5 lore raw
In reply to Sheng Yang
Quoted 9 lines · Sheng Yang

From: Dexuan Cui<[email protected]>

This patch enable guest to use XSAVE/XRSTOR 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.

Applied, thanks.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.