* [V6 0/4] add xsaves/xrstors support
@ 2015-10-12 6:07 Shuai Ruan
2015-10-12 6:07 ` [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves Shuai Ruan
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Shuai Ruan @ 2015-10-12 6:07 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
jun.nakajima, andrew.cooper3, ian.jackson, eddie.dong, jbeulich,
keir
From: Shuai Ruan <shuai.ruan@intel.com>
Changes in v6:
* Address comments from Jan.
* Rebase the code based on Andrew'patch "xen/x86: Record xsave features in c->x86_capabilities".
* Re-split the patch to avoid building errors. Move some func definitions from patch1 to patch2.
* Change func name load/save_xsave_states to compress/expand_xsave_states in patch2.
* Add macro to handle redundancy in xrstor.
* Fix some code errors and coding style errors.
* Add some descriptions in commit message.
Changes in v5:
*Address comments from Andrew/Jan,mainly:
*Add lazy writes of the xss_msr.
*Add xsave_area check when save/restore guest.
*Add xsavec support.
*Use word 2 in xen/include/asm-x86/cpufeature.h.
*Fix some code errors.
Changes in v4:
* Address comments from Andrew, mainly:
* No xsaves suporting for pv guest.
* Using xstate_sizes instead of domain_cpuid in hvm_cupid in patch 3.
* Add support for format translation when perform pv guest migration in patch 2.
* Fix some code errors.
Changes in v3:
* Address comments from Jan/Konrad
* Change the bits of eax/ebx/ecx/edx passed to guest in patch 4.
* Add code to verify whether host supports xsaves or not in patch 1.
Changes in v2:
* Address comments from Andrew/chao/Jan, mainly:
* Add details information of xsaves/xrstors feature.
* Test migration between xsaves-support machine and none-xsaves-support machine
* Remove XGETBV1/XSAVEC/XSAVEOPT out of 'else' in patch 3.
* Change macro name XGETBV to XGETBV1 in patch 4.
This patchset enable xsaves/xrstors feature which will be available on
Intel Skylake and later platform. Like xsave/xrstor, xsaves/xrstors
feature will save and load processor state from a region of memory called
XSAVE area. While unlike xsave/xrstor, xsaves/xrstors:
a) use the compacted format only for the extended region
of the XSAVE area which saves memory for you;
b) can operate on supervisor state components so the feature
is prerequisite to support new supervisor state components;
c) execute only if CPL=0.
Detail hardware spec can be found in chapter 13 (section 13.11 13.12) of the
Intel SDM [1].
patch1: add basic definition/function to support xsaves
patch2: add xsaves/xrstors support for xen
patch3-4: add xsaves/xrstors support for hvm guest
[1] Intel SDM (http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf)
Shuai Ruan (4):
x86/xsaves: add basic definitions/helpers to support xsaves
x86/xsaves: enable xsaves/xrstors/xsavec in xen
x86/xsaves: enable xsaves/xrstors for hvm guest
libxc: expose xsaves/xgetbv1/xsavec to hvm guest
tools/libxc/xc_cpuid_x86.c | 16 ++-
xen/arch/x86/domain.c | 2 +
xen/arch/x86/domctl.c | 35 +++++-
xen/arch/x86/hvm/hvm.c | 46 ++++++-
xen/arch/x86/hvm/vmx/vmcs.c | 5 +-
xen/arch/x86/hvm/vmx/vmx.c | 20 ++++
xen/arch/x86/i387.c | 4 +
xen/arch/x86/traps.c | 6 +-
xen/arch/x86/xstate.c | 237 ++++++++++++++++++++++++++++++++-----
xen/include/asm-x86/hvm/vcpu.h | 1 +
xen/include/asm-x86/hvm/vmx/vmcs.h | 4 +
xen/include/asm-x86/hvm/vmx/vmx.h | 2 +
xen/include/asm-x86/msr-index.h | 2 +
xen/include/asm-x86/xstate.h | 27 ++++-
14 files changed, 356 insertions(+), 51 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves
2015-10-12 6:07 [V6 0/4] add xsaves/xrstors support Shuai Ruan
@ 2015-10-12 6:07 ` Shuai Ruan
2015-10-12 9:25 ` Andrew Cooper
2015-10-13 13:33 ` Jan Beulich
2015-10-12 6:07 ` [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Shuai Ruan
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Shuai Ruan @ 2015-10-12 6:07 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
jun.nakajima, andrew.cooper3, ian.jackson, eddie.dong, jbeulich,
keir
This patch add basic definitions/helpers which will be used in
later patches.
Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
---
xen/arch/x86/xstate.c | 16 ++++++++++++++++
xen/include/asm-x86/hvm/vcpu.h | 1 +
xen/include/asm-x86/msr-index.h | 2 ++
xen/include/asm-x86/xstate.h | 25 ++++++++++++++++++++++++-
4 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 9ddff90..730368a 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -23,6 +23,11 @@ static u32 __read_mostly xsave_cntxt_size;
/* A 64-bit bitmask of the XSAVE/XRSTOR features supported by processor. */
u64 __read_mostly xfeature_mask;
+unsigned int * __read_mostly xstate_offsets;
+unsigned int * __read_mostly xstate_sizes;
+
+/* Cached xss for fast read */
+static DEFINE_PER_CPU(uint64_t, xss);
/* Cached xcr0 for fast read */
static DEFINE_PER_CPU(uint64_t, xcr0);
@@ -60,6 +65,17 @@ uint64_t get_xcr0(void)
return this_cpu(xcr0);
}
+void set_msr_xss(u64 xss)
+{
+ wrmsrl(MSR_IA32_XSS, xss);
+ this_cpu(xss) = xss;
+}
+
+uint64_t get_msr_xss(void)
+{
+ return this_cpu(xss);
+}
+
void xsave(struct vcpu *v, uint64_t mask)
{
struct xsave_struct *ptr = v->arch.xsave_area;
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index f553814..de81f8a 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -173,6 +173,7 @@ struct hvm_vcpu {
u32 msr_tsc_aux;
u64 msr_tsc_adjust;
+ u64 msr_xss;
union {
struct arch_vmx_struct vmx;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index e9c4723..4e5b31f 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -58,6 +58,8 @@
#define MSR_IA32_BNDCFGS 0x00000D90
+#define MSR_IA32_XSS 0x00000da0
+
#define MSR_MTRRfix64K_00000 0x00000250
#define MSR_MTRRfix16K_80000 0x00000258
#define MSR_MTRRfix16K_A0000 0x00000259
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index f0d8f0b..f23435a 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -19,7 +19,11 @@
#define XCR_XFEATURE_ENABLED_MASK 0x00000000 /* index of XCR0 */
+#define XSAVE_HDR_SIZE 64
+#define XSAVE_SSE_OFFSET 160
#define XSTATE_YMM_SIZE 256
+#define FXSAVE_SIZE 512
+#define XSAVE_HDR_OFFSET FXSAVE_SIZE
#define XSTATE_AREA_MIN_SIZE (512 + 64) /* FP/SSE + XSAVE.HEADER */
#define XSTATE_FP (1ULL << 0)
@@ -38,8 +42,24 @@
#define XSTATE_ALL (~(1ULL << 63))
#define XSTATE_NONLAZY (XSTATE_LWP | XSTATE_BNDREGS | XSTATE_BNDCSR)
#define XSTATE_LAZY (XSTATE_ALL & ~XSTATE_NONLAZY)
+#define XSTATE_COMPACTION_ENABLED (1ULL << 63)
+
+#define XSTATE_FIXUP ".section .fixup,\"ax\" \n" \
+ "2: mov %5,%%ecx \n" \
+ " xor %1,%1 \n" \
+ " rep stosb \n" \
+ " lea %2,%0 \n" \
+ " mov %3,%1 \n" \
+ " jmp 1b \n" \
+ ".previous \n" \
+ _ASM_EXTABLE(1b, 2b) \
+ : "+&D" (ptr), "+&a" (lmask) \
+ : "m" (*ptr), "g" (lmask), "d" (hmask), \
+ "m" (xsave_cntxt_size) \
+ : "ecx"
extern u64 xfeature_mask;
+extern unsigned int *xstate_offsets, *xstate_sizes;
/* extended state save area */
struct __packed __attribute__((aligned (64))) xsave_struct
@@ -68,7 +88,8 @@ struct __packed __attribute__((aligned (64))) xsave_struct
struct {
u64 xstate_bv;
- u64 reserved[7];
+ u64 xcomp_bv;
+ u64 reserved[6];
} xsave_hdr; /* The 64-byte header */
struct { char x[XSTATE_YMM_SIZE]; } ymm; /* YMM */
@@ -78,6 +99,8 @@ struct __packed __attribute__((aligned (64))) xsave_struct
/* extended state operations */
bool_t __must_check set_xcr0(u64 xfeatures);
uint64_t get_xcr0(void);
+void set_msr_xss(u64 xss);
+uint64_t get_msr_xss(void);
void xsave(struct vcpu *v, uint64_t mask);
void xrstor(struct vcpu *v, uint64_t mask);
bool_t xsave_enabled(const struct vcpu *v);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen
2015-10-12 6:07 [V6 0/4] add xsaves/xrstors support Shuai Ruan
2015-10-12 6:07 ` [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves Shuai Ruan
@ 2015-10-12 6:07 ` Shuai Ruan
2015-10-12 10:05 ` Andrew Cooper
2015-10-13 13:49 ` Jan Beulich
2015-10-12 6:07 ` [V6 3/4] x86/xsaves: enable xsaves/xrstors for hvm guest Shuai Ruan
2015-10-12 6:07 ` [V6 4/4] libxc: expose xsaves/xgetbv1/xsavec to " Shuai Ruan
3 siblings, 2 replies; 13+ messages in thread
From: Shuai Ruan @ 2015-10-12 6:07 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
jun.nakajima, andrew.cooper3, ian.jackson, eddie.dong, jbeulich,
keir
This patch uses xsaves/xrstors/xsavec instead of xsaveopt/xrstor
to perform the xsave_area switching so that xen itself
can benefit from them when available.
For xsaves/xrstors/xsavec only use compact format. Add format conversion
support when perform guest os migration.
Also, pv guest will not support xsaves/xrstors.
Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
---
xen/arch/x86/domain.c | 2 +
xen/arch/x86/domctl.c | 35 ++++++-
xen/arch/x86/hvm/hvm.c | 19 +++-
xen/arch/x86/i387.c | 4 +
xen/arch/x86/traps.c | 6 +-
xen/arch/x86/xstate.c | 221 ++++++++++++++++++++++++++++++++++++-------
xen/include/asm-x86/xstate.h | 2 +
7 files changed, 244 insertions(+), 45 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 7ca9b93..7b3d9f4 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1569,6 +1569,8 @@ static void __context_switch(void)
if ( xcr0 != get_xcr0() && !set_xcr0(xcr0) )
BUG();
}
+ if ( cpu_has_xsaves && is_hvm_vcpu(n) )
+ set_msr_xss(n->arch.hvm_vcpu.msr_xss);
vcpu_restore_fpu_eager(n);
n->arch.ctxt_switch_to(n);
}
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index f8a559c..9517741 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -897,9 +897,30 @@ long arch_do_domctl(
ret = -EFAULT;
offset += sizeof(v->arch.xcr0_accum);
- if ( !ret && copy_to_guest_offset(evc->buffer, offset,
- (void *)v->arch.xsave_area,
- size - 2 * sizeof(uint64_t)) )
+
+ if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) )
+ {
+ void *xsave_area;
+
+ xsave_area = xmalloc_bytes(size);
+ if ( !xsave_area )
+ {
+ ret = -ENOMEM;
+ vcpu_unpause(v);
+ goto vcpuextstate_out;
+ }
+
+ expand_xsave_states(v, xsave_area,
+ evc->size - 2 * sizeof(uint64_t));
+
+ if ( copy_to_guest_offset(evc->buffer, offset, xsave_area,
+ size - 2 * sizeof(uint64_t)) )
+ ret = -EFAULT;
+ xfree(xsave_area);
+ }
+ else if ( !ret && copy_to_guest_offset(evc->buffer, offset,
+ (void *)v->arch.xsave_area,
+ size - 2 * sizeof(uint64_t)) )
ret = -EFAULT;
vcpu_unpause(v);
@@ -955,8 +976,12 @@ long arch_do_domctl(
v->arch.xcr0_accum = _xcr0_accum;
if ( _xcr0_accum & XSTATE_NONLAZY )
v->arch.nonlazy_xstate_used = 1;
- memcpy(v->arch.xsave_area, _xsave_area,
- evc->size - 2 * sizeof(uint64_t));
+ if ( (cpu_has_xsaves || cpu_has_xsavec) )
+ compress_xsave_states(v, _xsave_area,
+ evc->size - 2 * sizeof(uint64_t));
+ else
+ memcpy(v->arch.xsave_area, (void *)_xsave_area,
+ evc->size - 2 * sizeof(uint64_t));
vcpu_unpause(v);
}
else
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3fa2280..177cf5f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2157,8 +2157,12 @@ static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
ctxt->xfeature_mask = xfeature_mask;
ctxt->xcr0 = v->arch.xcr0;
ctxt->xcr0_accum = v->arch.xcr0_accum;
- memcpy(&ctxt->save_area, v->arch.xsave_area,
- size - offsetof(struct hvm_hw_cpu_xsave, save_area));
+ if ( (cpu_has_xsaves || cpu_has_xsavec) )
+ expand_xsave_states(v, &ctxt->save_area,
+ size - offsetof(typeof(*ctxt), save_area));
+ else
+ memcpy(&ctxt->save_area, v->arch.xsave_area,
+ size - offsetof(struct hvm_hw_cpu_xsave, save_area));
}
return 0;
@@ -2257,9 +2261,14 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
v->arch.xcr0_accum = ctxt->xcr0_accum;
if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
v->arch.nonlazy_xstate_used = 1;
- memcpy(v->arch.xsave_area, &ctxt->save_area,
- min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
- save_area));
+ if ( (cpu_has_xsaves || cpu_has_xsavec) )
+ compress_xsave_states(v, &ctxt->save_area,
+ min(desc->length, size) -
+ offsetof(struct hvm_hw_cpu_xsave,save_area));
+ else
+ memcpy(v->arch.xsave_area, &ctxt->save_area,
+ min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
+ save_area));
return 0;
}
diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 14f2a79..736197f 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -309,7 +309,11 @@ int vcpu_init_fpu(struct vcpu *v)
return rc;
if ( v->arch.xsave_area )
+ {
v->arch.fpu_ctxt = &v->arch.xsave_area->fpu_sse;
+ if ( cpu_has_xsaves || cpu_has_xsavec )
+ v->arch.xsave_area->xsave_hdr.xcomp_bv |= XSTATE_COMPACTION_ENABLED;
+ }
else
{
v->arch.fpu_ctxt = _xzalloc(sizeof(v->arch.xsave_area->fpu_sse), 16);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 07feb6d..316fc7f 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -935,9 +935,9 @@ void pv_cpuid(struct cpu_user_regs *regs)
goto unsupported;
if ( regs->_ecx == 1 )
{
- a &= boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32];
- if ( !cpu_has_xsaves )
- b = c = d = 0;
+ a &= (boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32] &
+ ~cpufeat_mask(X86_FEATURE_XSAVES));
+ b = c = d = 0;
}
break;
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 730368a..323c6f2 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -25,6 +25,8 @@ static u32 __read_mostly xsave_cntxt_size;
u64 __read_mostly xfeature_mask;
unsigned int * __read_mostly xstate_offsets;
unsigned int * __read_mostly xstate_sizes;
+static unsigned int __read_mostly xstate_features;
+static unsigned int __read_mostly xstate_comp_offsets[sizeof(xfeature_mask)*8];
/* Cached xss for fast read */
static DEFINE_PER_CPU(uint64_t, xss);
@@ -76,6 +78,154 @@ uint64_t get_msr_xss(void)
return this_cpu(xss);
}
+static bool_t xsave_area_compressed(const struct xsave_struct *xsave_area)
+{
+ return xsave_area && (xsave_area->xsave_hdr.xcomp_bv
+ & XSTATE_COMPACTION_ENABLED);
+}
+
+static int setup_xstate_features(bool_t bsp)
+{
+ unsigned int leaf, tmp, eax, ebx;
+
+ if ( bsp )
+ {
+ xstate_features = fls(xfeature_mask);
+ xstate_offsets = xzalloc_array(unsigned int, xstate_features);
+ if ( !xstate_offsets )
+ return -ENOMEM;
+
+ xstate_sizes = xzalloc_array(unsigned int, xstate_features);
+ if ( !xstate_sizes )
+ return -ENOMEM;
+ }
+
+ for ( leaf = 2; leaf < xstate_features; leaf++ )
+ {
+ if ( bsp )
+ cpuid_count(XSTATE_CPUID, leaf, &xstate_sizes[leaf],
+ &xstate_offsets[leaf], &tmp, &tmp);
+ else
+ {
+ cpuid_count(XSTATE_CPUID, leaf, &eax,
+ &ebx, &tmp, &tmp);
+ BUG_ON(eax != xstate_sizes[leaf]);
+ BUG_ON(ebx != xstate_offsets[leaf]);
+ }
+ }
+
+ return 0;
+}
+
+static void __init setup_xstate_comp(void)
+{
+ unsigned int i;
+
+ /*
+ * The FP xstates and SSE xstates are legacy states. They are always
+ * in the fixed offsets in the xsave area in either compacted form
+ * or standard form.
+ */
+ xstate_comp_offsets[0] = 0;
+ xstate_comp_offsets[1] = XSAVE_SSE_OFFSET;
+
+ xstate_comp_offsets[2] = FXSAVE_SIZE + XSAVE_HDR_SIZE;
+
+ for ( i = 3; i < xstate_features; i++ )
+ {
+ xstate_comp_offsets[i] = xstate_comp_offsets[i-1] +
+ (((1ul << i) & xfeature_mask)
+ ? xstate_sizes[i-1] : 0);
+ ASSERT(xstate_comp_offsets[i] + xstate_sizes[i] <= xsave_cntxt_size);
+ }
+}
+
+static void *get_xsave_addr(const struct xsave_struct *xsave, unsigned int xfeature_idx)
+{
+ if ( !((1ul << xfeature_idx) & xfeature_mask) )
+ return NULL;
+
+ return (void *)xsave + xstate_comp_offsets[xfeature_idx];
+}
+
+void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size)
+{
+ const struct xsave_struct *xsave = v->arch.xsave_area;
+ struct xsave_struct *dest_xsave = (struct xsave_struct *)dest;
+ u64 xstate_bv = xsave->xsave_hdr.xstate_bv;
+ u64 valid;
+
+ ASSERT(xsave_area_compressed(xsave));
+ /*
+ * Copy legacy XSAVE area, to avoid complications with CPUID
+ * leaves 0 and 1 in the loop below.
+ */
+ memcpy(dest, xsave, FXSAVE_SIZE);
+
+ dest_xsave->xsave_hdr.xstate_bv = xstate_bv;
+ dest_xsave->xsave_hdr.xcomp_bv = 0;
+
+ /*
+ * Copy each region from the possibly compacted offset to the
+ * non-compacted offset.
+ */
+ valid = xstate_bv & ~XSTATE_FP_SSE;
+ while ( valid )
+ {
+ u64 feature = valid & -valid;
+ unsigned int index = fls(feature) - 1;
+ void *src = get_xsave_addr(xsave, index);
+
+ if ( src )
+ {
+ ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= size);
+ memcpy(dest + xstate_offsets[index], src, xstate_sizes[index]);
+ }
+
+ valid &= ~feature;
+ }
+
+}
+
+void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
+{
+ struct xsave_struct *xsave = v->arch.xsave_area;
+ struct xsave_struct *src_xsave = (struct xsave_struct *)src;
+ u64 xstate_bv = src_xsave->xsave_hdr.xstate_bv;
+ u64 valid;
+
+ ASSERT(!xsave_area_compressed((struct xsave_struct *)src));
+ /*
+ * Copy legacy XSAVE area, to avoid complications with CPUID
+ * leaves 0 and 1 in the loop below.
+ */
+ memcpy(xsave, src, FXSAVE_SIZE);
+
+ /* Set XSTATE_BV and XCOMP_BV. */
+ xsave->xsave_hdr.xstate_bv = xstate_bv;
+ xsave->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED;
+
+ /*
+ * Copy each region from the non-compacted offset to the
+ * possibly compacted offset.
+ */
+ valid = xstate_bv & ~XSTATE_FP_SSE;
+ while ( valid )
+ {
+ u64 feature = valid & -valid;
+ unsigned int index = fls(feature) - 1;
+ void *dest = get_xsave_addr(xsave, index);
+
+ if ( dest )
+ {
+ ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= size);
+ memcpy(dest, src + xstate_offsets[index], xstate_sizes[index]);
+ }
+
+ valid &= ~feature;
+ }
+}
+
void xsave(struct vcpu *v, uint64_t mask)
{
struct xsave_struct *ptr = v->arch.xsave_area;
@@ -88,7 +238,15 @@ void xsave(struct vcpu *v, uint64_t mask)
typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel;
typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel;
- if ( cpu_has_xsaveopt )
+ if ( cpu_has_xsaves )
+ asm volatile ( ".byte 0x48,0x0f,0xc7,0x2f"
+ : "=m" (*ptr)
+ : "a" (lmask), "d" (hmask), "D" (ptr) );
+ else if ( cpu_has_xsavec )
+ asm volatile ( ".byte 0x48,0x0f,0xc7,0x27"
+ : "=m" (*ptr)
+ : "a" (lmask), "d" (hmask), "D" (ptr) );
+ else if ( cpu_has_xsaveopt )
{
/*
* xsaveopt may not write the FPU portion even when the respective
@@ -141,7 +299,15 @@ void xsave(struct vcpu *v, uint64_t mask)
}
else
{
- if ( cpu_has_xsaveopt )
+ if ( cpu_has_xsaves )
+ asm volatile ( ".byte 0x48,0x0f,0xc7,0x2f"
+ : "=m" (*ptr)
+ : "a" (lmask), "d" (hmask), "D" (ptr) );
+ else if ( cpu_has_xsavec )
+ asm volatile ( ".byte 0x48,0x0f,0xc7,0x27"
+ : "=m" (*ptr)
+ : "a" (lmask), "d" (hmask), "D" (ptr) );
+ else if ( cpu_has_xsaveopt )
asm volatile ( ".byte 0x0f,0xae,0x37"
: "=m" (*ptr)
: "a" (lmask), "d" (hmask), "D" (ptr) );
@@ -184,36 +350,20 @@ void xrstor(struct vcpu *v, uint64_t mask)
switch ( __builtin_expect(ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET], 8) )
{
default:
- asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f\n"
- ".section .fixup,\"ax\" \n"
- "2: mov %5,%%ecx \n"
- " xor %1,%1 \n"
- " rep stosb \n"
- " lea %2,%0 \n"
- " mov %3,%1 \n"
- " jmp 1b \n"
- ".previous \n"
- _ASM_EXTABLE(1b, 2b)
- : "+&D" (ptr), "+&a" (lmask)
- : "m" (*ptr), "g" (lmask), "d" (hmask),
- "m" (xsave_cntxt_size)
- : "ecx" );
- break;
+ if ( cpu_has_xsaves )
+ asm volatile ( "1: .byte 0x48,0x0f,0xc7,0x1f\n"
+ XSTATE_FIXUP );
+ else
+ asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f\n"
+ XSTATE_FIXUP );
+ break;
case 4: case 2:
- asm volatile ( "1: .byte 0x0f,0xae,0x2f\n"
- ".section .fixup,\"ax\" \n"
- "2: mov %5,%%ecx \n"
- " xor %1,%1 \n"
- " rep stosb \n"
- " lea %2,%0 \n"
- " mov %3,%1 \n"
- " jmp 1b \n"
- ".previous \n"
- _ASM_EXTABLE(1b, 2b)
- : "+&D" (ptr), "+&a" (lmask)
- : "m" (*ptr), "g" (lmask), "d" (hmask),
- "m" (xsave_cntxt_size)
- : "ecx" );
+ if ( cpu_has_xsaves )
+ asm volatile ( "1: .byte 0x48,0x0f,0xc7,0x1f\n"
+ XSTATE_FIXUP );
+ else
+ asm volatile ( "1: .byte 0x0f,0xae,0x2f\n"
+ XSTATE_FIXUP );
break;
}
}
@@ -340,11 +490,18 @@ void xstate_init(struct cpuinfo_x86 *c)
/* Mask out features not currently understood by Xen. */
eax &= (cpufeat_mask(X86_FEATURE_XSAVEOPT) |
- cpufeat_mask(X86_FEATURE_XSAVEC));
+ cpufeat_mask(X86_FEATURE_XSAVEC) |
+ cpufeat_mask(X86_FEATURE_XGETBV1) |
+ cpufeat_mask(X86_FEATURE_XSAVES));
c->x86_capability[X86_FEATURE_XSAVEOPT / 32] = eax;
BUG_ON(eax != boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32]);
+
+ if ( setup_xstate_features(bsp) )
+ BUG();
+ if ( bsp && (cpu_has_xsaves || cpu_has_xsavec) )
+ setup_xstate_comp();
}
static bool_t valid_xcr0(u64 xcr0)
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index f23435a..1c4c5fd 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -106,6 +106,8 @@ void xrstor(struct vcpu *v, uint64_t mask);
bool_t xsave_enabled(const struct vcpu *v);
int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum, u64 xstate_bv);
int __must_check handle_xsetbv(u32 index, u64 new_bv);
+void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size);
+void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size);
/* extended state init and cleanup functions */
void xstate_free_save_area(struct vcpu *v);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [V6 3/4] x86/xsaves: enable xsaves/xrstors for hvm guest
2015-10-12 6:07 [V6 0/4] add xsaves/xrstors support Shuai Ruan
2015-10-12 6:07 ` [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves Shuai Ruan
2015-10-12 6:07 ` [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Shuai Ruan
@ 2015-10-12 6:07 ` Shuai Ruan
2015-10-12 6:07 ` [V6 4/4] libxc: expose xsaves/xgetbv1/xsavec to " Shuai Ruan
3 siblings, 0 replies; 13+ messages in thread
From: Shuai Ruan @ 2015-10-12 6:07 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
jun.nakajima, andrew.cooper3, ian.jackson, eddie.dong, jbeulich,
keir
This patch enables xsaves for hvm guest, includes:
1.handle xsaves vmcs init and vmexit.
2.add logic to write/read the XSS msr.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
---
xen/arch/x86/hvm/hvm.c | 27 +++++++++++++++++++++++++++
xen/arch/x86/hvm/vmx/vmcs.c | 5 ++++-
xen/arch/x86/hvm/vmx/vmx.c | 20 ++++++++++++++++++++
xen/include/asm-x86/hvm/vmx/vmcs.h | 4 ++++
xen/include/asm-x86/hvm/vmx/vmx.h | 2 ++
5 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 177cf5f..a7c45be 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4561,6 +4561,20 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
*ebx = _eax + _ebx;
}
}
+ if ( count == 1 )
+ {
+ if ( cpu_has_xsaves && cpu_has_vmx_xsaves )
+ {
+ *ebx = XSTATE_AREA_MIN_SIZE;
+ if ( v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss )
+ for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
+ if ( (v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss)
+ & (1ULL << sub_leaf) )
+ *ebx += xstate_sizes[sub_leaf];
+ }
+ else
+ *ebx = *ecx = *edx = 0;
+ }
break;
case 0x80000001:
@@ -4660,6 +4674,12 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
*msr_content = v->arch.hvm_vcpu.guest_efer;
break;
+ case MSR_IA32_XSS:
+ if ( !cpu_has_xsaves )
+ goto gp_fault;
+ *msr_content = v->arch.hvm_vcpu.msr_xss;
+ break;
+
case MSR_IA32_TSC:
*msr_content = _hvm_rdtsc_intercept();
break;
@@ -4792,6 +4812,13 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
return X86EMUL_EXCEPTION;
break;
+ case MSR_IA32_XSS:
+ /* No XSS features currently supported for guests. */
+ if ( !cpu_has_xsaves || msr_content != 0 )
+ goto gp_fault;
+ v->arch.hvm_vcpu.msr_xss = msr_content;
+ break;
+
case MSR_IA32_TSC:
hvm_set_guest_tsc(v, msr_content);
break;
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 3592a88..7185d55 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -240,7 +240,8 @@ static int vmx_init_vmcs_config(void)
SECONDARY_EXEC_PAUSE_LOOP_EXITING |
SECONDARY_EXEC_ENABLE_INVPCID |
SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
- SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
+ SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS |
+ SECONDARY_EXEC_XSAVES);
rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL )
opt |= SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
@@ -1249,6 +1250,8 @@ static int construct_vmcs(struct vcpu *v)
__vmwrite(HOST_PAT, host_pat);
__vmwrite(GUEST_PAT, guest_pat);
}
+ if ( cpu_has_vmx_xsaves )
+ __vmwrite(XSS_EXIT_BITMAP, 0);
vmx_vmcs_exit(v);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index bbec0e8..5d723e8 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2852,6 +2852,18 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
}
}
+static void vmx_handle_xsaves(void)
+{
+ gdprintk(XENLOG_ERR, "xsaves should not cause vmexit\n");
+ domain_crash(current->domain);
+}
+
+static void vmx_handle_xrstors(void)
+{
+ gdprintk(XENLOG_ERR, "xrstors should not cause vmexit\n");
+ domain_crash(current->domain);
+}
+
static int vmx_handle_apic_write(void)
{
unsigned long exit_qualification;
@@ -3423,6 +3435,14 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
vmx_vcpu_flush_pml_buffer(v);
break;
+ case EXIT_REASON_XSAVES:
+ vmx_handle_xsaves();
+ break;
+
+ case EXIT_REASON_XRSTORS:
+ vmx_handle_xrstors();
+ break;
+
case EXIT_REASON_ACCESS_GDTR_OR_IDTR:
case EXIT_REASON_ACCESS_LDTR_OR_TR:
case EXIT_REASON_VMX_PREEMPTION_TIMER_EXPIRED:
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index f1126d4..79c2c58 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -225,6 +225,7 @@ extern u32 vmx_vmentry_control;
#define SECONDARY_EXEC_ENABLE_VMCS_SHADOWING 0x00004000
#define SECONDARY_EXEC_ENABLE_PML 0x00020000
#define SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS 0x00040000
+#define SECONDARY_EXEC_XSAVES 0x00100000
extern u32 vmx_secondary_exec_control;
#define VMX_EPT_EXEC_ONLY_SUPPORTED 0x00000001
@@ -291,6 +292,8 @@ extern u32 vmx_secondary_exec_control;
(vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
#define cpu_has_vmx_pml \
(vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
+#define cpu_has_vmx_xsaves \
+ (vmx_secondary_exec_control & SECONDARY_EXEC_XSAVES)
#define VMCS_RID_TYPE_MASK 0x80000000
@@ -365,6 +368,7 @@ enum vmcs_field {
VMREAD_BITMAP = 0x00002026,
VMWRITE_BITMAP = 0x00002028,
VIRT_EXCEPTION_INFO = 0x0000202a,
+ XSS_EXIT_BITMAP = 0x0000202c,
GUEST_PHYSICAL_ADDRESS = 0x00002400,
VMCS_LINK_POINTER = 0x00002800,
GUEST_IA32_DEBUGCTL = 0x00002802,
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index 2ed62f9..cb66925 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -188,6 +188,8 @@ static inline unsigned long pi_get_pir(struct pi_desc *pi_desc, int group)
#define EXIT_REASON_INVPCID 58
#define EXIT_REASON_VMFUNC 59
#define EXIT_REASON_PML_FULL 62
+#define EXIT_REASON_XSAVES 63
+#define EXIT_REASON_XRSTORS 64
/*
* Interruption-information format
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [V6 4/4] libxc: expose xsaves/xgetbv1/xsavec to hvm guest
2015-10-12 6:07 [V6 0/4] add xsaves/xrstors support Shuai Ruan
` (2 preceding siblings ...)
2015-10-12 6:07 ` [V6 3/4] x86/xsaves: enable xsaves/xrstors for hvm guest Shuai Ruan
@ 2015-10-12 6:07 ` Shuai Ruan
2015-10-12 10:11 ` Andrew Cooper
3 siblings, 1 reply; 13+ messages in thread
From: Shuai Ruan @ 2015-10-12 6:07 UTC (permalink / raw)
To: xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
jun.nakajima, andrew.cooper3, ian.jackson, eddie.dong, jbeulich,
keir
This patch exposes xsaves/xgetbv1/xsavec to hvm guest.
The reserved bits of eax/ebx/ecx/edx must be cleaned up
when call cpuid(0dh) with leaf 1 or 2..63.
According to the spec the following bits must be reserved:
For leaf 1, bits 03-04/08-31 of ecx is reserved. Edx is reserved.
For leaf 2...63, bits 01-31 of ecx is reserved. Edx is reserved.
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
---
tools/libxc/xc_cpuid_x86.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index e146a3e..b0d6ecc 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -210,6 +210,10 @@ static void intel_xc_cpuid_policy(
}
#define XSAVEOPT (1 << 0)
+#define XSAVEC (1 << 1)
+#define XGETBV1 (1 << 2)
+#define XSAVES (1 << 3)
+#define XSS_SUPPORT (1 << 0)
/* Configure extended state enumeration leaves (0x0000000D for xsave) */
static void xc_cpuid_config_xsave(
xc_interface *xch, domid_t domid, uint64_t xfeature_mask,
@@ -246,8 +250,9 @@ static void xc_cpuid_config_xsave(
regs[1] = 512 + 64; /* FP/SSE + XSAVE.HEADER */
break;
case 1: /* leaf 1 */
- regs[0] &= XSAVEOPT;
- regs[1] = regs[2] = regs[3] = 0;
+ regs[0] &= (XSAVEOPT | XSAVEC | XGETBV1 | XSAVES);
+ regs[2] &= xfeature_mask;
+ regs[3] = 0;
break;
case 2 ... 63: /* sub-leaves */
if ( !(xfeature_mask & (1ULL << input[1])) )
@@ -255,8 +260,11 @@ static void xc_cpuid_config_xsave(
regs[0] = regs[1] = regs[2] = regs[3] = 0;
break;
}
- /* Don't touch EAX, EBX. Also cleanup ECX and EDX */
- regs[2] = regs[3] = 0;
+ /* Don't touch EAX, EBX and cleanup EDX.
+ * Bit 0 of ECX represent whether sub-leave is supported in IA32_XSS
+ * msr or supported in XCR0.*/
+ regs[2] &= XSS_SUPPORT;
+ regs[3] = 0;
break;
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves
2015-10-12 6:07 ` [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves Shuai Ruan
@ 2015-10-12 9:25 ` Andrew Cooper
2015-10-13 13:33 ` Jan Beulich
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2015-10-12 9:25 UTC (permalink / raw)
To: Shuai Ruan, xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
eddie.dong, ian.jackson, jbeulich, jun.nakajima, keir
On 12/10/15 07:07, Shuai Ruan wrote:
> This patch add basic definitions/helpers which will be used in
> later patches.
>
> Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
> ---
> xen/arch/x86/xstate.c | 16 ++++++++++++++++
> xen/include/asm-x86/hvm/vcpu.h | 1 +
> xen/include/asm-x86/msr-index.h | 2 ++
> xen/include/asm-x86/xstate.h | 25 ++++++++++++++++++++++++-
> 4 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> index 9ddff90..730368a 100644
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -23,6 +23,11 @@ static u32 __read_mostly xsave_cntxt_size;
>
> /* A 64-bit bitmask of the XSAVE/XRSTOR features supported by processor. */
> u64 __read_mostly xfeature_mask;
> +unsigned int * __read_mostly xstate_offsets;
> +unsigned int * __read_mostly xstate_sizes;
> +
> +/* Cached xss for fast read */
> +static DEFINE_PER_CPU(uint64_t, xss);
>
> /* Cached xcr0 for fast read */
> static DEFINE_PER_CPU(uint64_t, xcr0);
> @@ -60,6 +65,17 @@ uint64_t get_xcr0(void)
> return this_cpu(xcr0);
> }
>
> +void set_msr_xss(u64 xss)
> +{
> + wrmsrl(MSR_IA32_XSS, xss);
> + this_cpu(xss) = xss;
This can become faster by doing a lazy write.
u64 * this_xss = &this_cpu(xss);
if ( *this_xss != xss )
{
wrmsrl(MSR_IA32_XSS, xss);
*this_xss = xss;
}
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen
2015-10-12 6:07 ` [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Shuai Ruan
@ 2015-10-12 10:05 ` Andrew Cooper
2015-10-13 7:28 ` Shuai Ruan
2015-10-13 13:49 ` Jan Beulich
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2015-10-12 10:05 UTC (permalink / raw)
To: Shuai Ruan, xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
eddie.dong, ian.jackson, jbeulich, jun.nakajima, keir
On 12/10/15 07:07, Shuai Ruan wrote:
> This patch uses xsaves/xrstors/xsavec instead of xsaveopt/xrstor
> to perform the xsave_area switching so that xen itself
> can benefit from them when available.
>
> For xsaves/xrstors/xsavec only use compact format. Add format conversion
> support when perform guest os migration.
>
> Also, pv guest will not support xsaves/xrstors.
>
> Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
> ---
> xen/arch/x86/domain.c | 2 +
> xen/arch/x86/domctl.c | 35 ++++++-
> xen/arch/x86/hvm/hvm.c | 19 +++-
> xen/arch/x86/i387.c | 4 +
> xen/arch/x86/traps.c | 6 +-
> xen/arch/x86/xstate.c | 221 ++++++++++++++++++++++++++++++++++++-------
> xen/include/asm-x86/xstate.h | 2 +
> 7 files changed, 244 insertions(+), 45 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 7ca9b93..7b3d9f4 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1569,6 +1569,8 @@ static void __context_switch(void)
> if ( xcr0 != get_xcr0() && !set_xcr0(xcr0) )
> BUG();
> }
> + if ( cpu_has_xsaves && is_hvm_vcpu(n) )
Because of the current implementation of PVH mode, the correct predicate
here is has_hvm_container(n)
> + set_msr_xss(n->arch.hvm_vcpu.msr_xss);
> vcpu_restore_fpu_eager(n);
> n->arch.ctxt_switch_to(n);
> }
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index f8a559c..9517741 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -897,9 +897,30 @@ long arch_do_domctl(
> ret = -EFAULT;
>
> offset += sizeof(v->arch.xcr0_accum);
> - if ( !ret && copy_to_guest_offset(evc->buffer, offset,
> - (void *)v->arch.xsave_area,
> - size - 2 * sizeof(uint64_t)) )
> +
> + if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) )
> + {
> + void *xsave_area;
> +
> + xsave_area = xmalloc_bytes(size);
> + if ( !xsave_area )
> + {
> + ret = -ENOMEM;
> + vcpu_unpause(v);
> + goto vcpuextstate_out;
> + }
> +
> + expand_xsave_states(v, xsave_area,
> + evc->size - 2 * sizeof(uint64_t));
> +
> + if ( copy_to_guest_offset(evc->buffer, offset, xsave_area,
> + size - 2 * sizeof(uint64_t)) )
> + ret = -EFAULT;
> + xfree(xsave_area);
> + }
> + else if ( !ret && copy_to_guest_offset(evc->buffer, offset,
> + (void *)v->arch.xsave_area,
> + size - 2 * sizeof(uint64_t)) )
> ret = -EFAULT;
>
> vcpu_unpause(v);
> @@ -955,8 +976,12 @@ long arch_do_domctl(
> v->arch.xcr0_accum = _xcr0_accum;
> if ( _xcr0_accum & XSTATE_NONLAZY )
> v->arch.nonlazy_xstate_used = 1;
> - memcpy(v->arch.xsave_area, _xsave_area,
> - evc->size - 2 * sizeof(uint64_t));
> + if ( (cpu_has_xsaves || cpu_has_xsavec) )
You can drop the inner set of brackets.
> + compress_xsave_states(v, _xsave_area,
> + evc->size - 2 * sizeof(uint64_t));
> + else
> + memcpy(v->arch.xsave_area, (void *)_xsave_area,
> + evc->size - 2 * sizeof(uint64_t));
> vcpu_unpause(v);
> }
> else
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 3fa2280..177cf5f 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2157,8 +2157,12 @@ static int hvm_save_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> ctxt->xfeature_mask = xfeature_mask;
> ctxt->xcr0 = v->arch.xcr0;
> ctxt->xcr0_accum = v->arch.xcr0_accum;
> - memcpy(&ctxt->save_area, v->arch.xsave_area,
> - size - offsetof(struct hvm_hw_cpu_xsave, save_area));
> + if ( (cpu_has_xsaves || cpu_has_xsavec) )
And here
> + expand_xsave_states(v, &ctxt->save_area,
> + size - offsetof(typeof(*ctxt), save_area));
> + else
> + memcpy(&ctxt->save_area, v->arch.xsave_area,
> + size - offsetof(struct hvm_hw_cpu_xsave, save_area));
> }
>
> return 0;
> @@ -2257,9 +2261,14 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> v->arch.xcr0_accum = ctxt->xcr0_accum;
> if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
> v->arch.nonlazy_xstate_used = 1;
> - memcpy(v->arch.xsave_area, &ctxt->save_area,
> - min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
> - save_area));
> + if ( (cpu_has_xsaves || cpu_has_xsavec) )
And here.
With these 4 things fixed, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 4/4] libxc: expose xsaves/xgetbv1/xsavec to hvm guest
2015-10-12 6:07 ` [V6 4/4] libxc: expose xsaves/xgetbv1/xsavec to " Shuai Ruan
@ 2015-10-12 10:11 ` Andrew Cooper
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2015-10-12 10:11 UTC (permalink / raw)
To: Shuai Ruan, xen-devel
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
eddie.dong, ian.jackson, jbeulich, jun.nakajima, keir
On 12/10/15 07:07, Shuai Ruan wrote:
> This patch exposes xsaves/xgetbv1/xsavec to hvm guest.
> The reserved bits of eax/ebx/ecx/edx must be cleaned up
> when call cpuid(0dh) with leaf 1 or 2..63.
>
> According to the spec the following bits must be reserved:
> For leaf 1, bits 03-04/08-31 of ecx is reserved. Edx is reserved.
> For leaf 2...63, bits 01-31 of ecx is reserved. Edx is reserved.
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
This patch needs rebasing over c/s bf87f3a "tools/libxc: Improve
efficiency of xc_cpuid_apply_policy()"
> ---
> tools/libxc/xc_cpuid_x86.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index e146a3e..b0d6ecc 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -210,6 +210,10 @@ static void intel_xc_cpuid_policy(
> }
>
> #define XSAVEOPT (1 << 0)
> +#define XSAVEC (1 << 1)
> +#define XGETBV1 (1 << 2)
> +#define XSAVES (1 << 3)
> +#define XSS_SUPPORT (1 << 0)
> /* Configure extended state enumeration leaves (0x0000000D for xsave) */
> static void xc_cpuid_config_xsave(
> xc_interface *xch, domid_t domid, uint64_t xfeature_mask,
> @@ -246,8 +250,9 @@ static void xc_cpuid_config_xsave(
> regs[1] = 512 + 64; /* FP/SSE + XSAVE.HEADER */
> break;
> case 1: /* leaf 1 */
> - regs[0] &= XSAVEOPT;
> - regs[1] = regs[2] = regs[3] = 0;
> + regs[0] &= (XSAVEOPT | XSAVEC | XGETBV1 | XSAVES);
The correct mask here depends on !info->hvm, to hide XSAVES from PV guests.
> + regs[2] &= xfeature_mask;
> + regs[3] = 0;
> break;
> case 2 ... 63: /* sub-leaves */
> if ( !(xfeature_mask & (1ULL << input[1])) )
> @@ -255,8 +260,11 @@ static void xc_cpuid_config_xsave(
> regs[0] = regs[1] = regs[2] = regs[3] = 0;
> break;
> }
> - /* Don't touch EAX, EBX. Also cleanup ECX and EDX */
> - regs[2] = regs[3] = 0;
> + /* Don't touch EAX, EBX and cleanup EDX.
> + * Bit 0 of ECX represent whether sub-leave is supported in IA32_XSS
> + * msr or supported in XCR0.*/
> + regs[2] &= XSS_SUPPORT;
No XSS features are currently supported, even in HVM guests. This
should be unilaterally zero at the moment.
~Andrew
> + regs[3] = 0;
> break;
> }
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen
2015-10-12 10:05 ` Andrew Cooper
@ 2015-10-13 7:28 ` Shuai Ruan
0 siblings, 0 replies; 13+ messages in thread
From: Shuai Ruan @ 2015-10-13 7:28 UTC (permalink / raw)
To: Andrew Cooper
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
jun.nakajima, ian.jackson, eddie.dong, xen-devel, jbeulich, keir
On Mon, Oct 12, 2015 at 11:05:33AM +0100, Andrew Cooper wrote:
> On 12/10/15 07:07, Shuai Ruan wrote:
> > This patch uses xsaves/xrstors/xsavec instead of xsaveopt/xrstor
> > to perform the xsave_area switching so that xen itself
> > can benefit from them when available.
> >
> > For xsaves/xrstors/xsavec only use compact format. Add format conversion
> > support when perform guest os migration.
> >
> > Also, pv guest will not support xsaves/xrstors.
> >
> > Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com>
> > ---
>
> With these 4 things fixed, Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>
>
Ok.
Thanks.
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves
2015-10-12 6:07 ` [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves Shuai Ruan
2015-10-12 9:25 ` Andrew Cooper
@ 2015-10-13 13:33 ` Jan Beulich
1 sibling, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2015-10-13 13:33 UTC (permalink / raw)
To: Shuai Ruan
Cc: kevin.tian, wei.liu2, eddie.dong, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, jun.nakajima, keir,
Ian.Campbell
>>> On 12.10.15 at 08:07, <shuai.ruan@linux.intel.com> wrote:
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -23,6 +23,11 @@ static u32 __read_mostly xsave_cntxt_size;
>
> /* A 64-bit bitmask of the XSAVE/XRSTOR features supported by processor. */
> u64 __read_mostly xfeature_mask;
> +unsigned int * __read_mostly xstate_offsets;
> +unsigned int * __read_mostly xstate_sizes;
You appear to need the latter outside of this file in patch 3, but the
former could (and hence should) be static afaict.
> --- a/xen/include/asm-x86/xstate.h
> +++ b/xen/include/asm-x86/xstate.h
> @@ -19,7 +19,11 @@
>
> #define XCR_XFEATURE_ENABLED_MASK 0x00000000 /* index of XCR0 */
>
> +#define XSAVE_HDR_SIZE 64
> +#define XSAVE_SSE_OFFSET 160
> #define XSTATE_YMM_SIZE 256
> +#define FXSAVE_SIZE 512
> +#define XSAVE_HDR_OFFSET FXSAVE_SIZE
> #define XSTATE_AREA_MIN_SIZE (512 + 64) /* FP/SSE + XSAVE.HEADER */
This 512 should now get replaced too.
> @@ -38,8 +42,24 @@
> #define XSTATE_ALL (~(1ULL << 63))
> #define XSTATE_NONLAZY (XSTATE_LWP | XSTATE_BNDREGS | XSTATE_BNDCSR)
> #define XSTATE_LAZY (XSTATE_ALL & ~XSTATE_NONLAZY)
> +#define XSTATE_COMPACTION_ENABLED (1ULL << 63)
> +
> +#define XSTATE_FIXUP ".section .fixup,\"ax\" \n" \
> + "2: mov %5,%%ecx \n" \
> + " xor %1,%1 \n" \
> + " rep stosb \n" \
> + " lea %2,%0 \n" \
> + " mov %3,%1 \n" \
> + " jmp 1b \n" \
> + ".previous \n" \
> + _ASM_EXTABLE(1b, 2b) \
> + : "+&D" (ptr), "+&a" (lmask) \
> + : "m" (*ptr), "g" (lmask), "d" (hmask), \
> + "m" (xsave_cntxt_size) \
> + : "ecx"
First of all I don't think this belongs here - neither in this patch, nor
in this header. It's not being used by this patch, and the connection
between variable names used here and in patch 2 is completely lost.
If you put this into xrstor() as a local #define, then this may be
okay. In any other case the user of the macro has to have a way
to control the variable names used.
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen
2015-10-12 6:07 ` [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Shuai Ruan
2015-10-12 10:05 ` Andrew Cooper
@ 2015-10-13 13:49 ` Jan Beulich
2015-10-15 1:58 ` Shuai Ruan
[not found] ` <20151015015832.GA11071@shuai.ruan@linux.intel.com>
1 sibling, 2 replies; 13+ messages in thread
From: Jan Beulich @ 2015-10-13 13:49 UTC (permalink / raw)
To: Shuai Ruan
Cc: kevin.tian, wei.liu2, eddie.dong, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, jun.nakajima, keir,
Ian.Campbell
>>> On 12.10.15 at 08:07, <shuai.ruan@linux.intel.com> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -897,9 +897,30 @@ long arch_do_domctl(
> ret = -EFAULT;
>
> offset += sizeof(v->arch.xcr0_accum);
> - if ( !ret && copy_to_guest_offset(evc->buffer, offset,
> - (void *)v->arch.xsave_area,
> - size - 2 * sizeof(uint64_t)) )
> +
> + if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) )
> + {
> + void *xsave_area;
> +
> + xsave_area = xmalloc_bytes(size);
> + if ( !xsave_area )
> + {
> + ret = -ENOMEM;
> + vcpu_unpause(v);
> + goto vcpuextstate_out;
> + }
> +
> + expand_xsave_states(v, xsave_area,
> + evc->size - 2 * sizeof(uint64_t));
> +
> + if ( copy_to_guest_offset(evc->buffer, offset, xsave_area,
> + size - 2 * sizeof(uint64_t)) )
Here you use size, which is fine, but why do you use evc->size
three lines up from here?
> + ret = -EFAULT;
> + xfree(xsave_area);
> + }
> + else if ( !ret && copy_to_guest_offset(evc->buffer, offset,
> + (void *)v->arch.xsave_area,
> + size - 2 * sizeof(uint64_t)) )
I also think code readability would benefit from folding the two
copy_to_guest_offset()s, which differ in only the pointer used.
> @@ -955,8 +976,12 @@ long arch_do_domctl(
> v->arch.xcr0_accum = _xcr0_accum;
> if ( _xcr0_accum & XSTATE_NONLAZY )
> v->arch.nonlazy_xstate_used = 1;
> - memcpy(v->arch.xsave_area, _xsave_area,
> - evc->size - 2 * sizeof(uint64_t));
> + if ( (cpu_has_xsaves || cpu_has_xsavec) )
> + compress_xsave_states(v, _xsave_area,
> + evc->size - 2 * sizeof(uint64_t));
> + else
> + memcpy(v->arch.xsave_area, (void *)_xsave_area,
Stray cast got added here.
> @@ -2257,9 +2261,14 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> v->arch.xcr0_accum = ctxt->xcr0_accum;
> if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
> v->arch.nonlazy_xstate_used = 1;
> - memcpy(v->arch.xsave_area, &ctxt->save_area,
> - min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
> - save_area));
> + if ( (cpu_has_xsaves || cpu_has_xsavec) )
> + compress_xsave_states(v, &ctxt->save_area,
> + min(desc->length, size) -
> + offsetof(struct hvm_hw_cpu_xsave,save_area));
> + else
> + memcpy(v->arch.xsave_area, &ctxt->save_area,
> + min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
> + save_area));
Each time I look at these two hunks I wonder whether the condition
and memcpy() wouldn't better move into the new called functions.
> +void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size)
> +{
> + const struct xsave_struct *xsave = v->arch.xsave_area;
> + struct xsave_struct *dest_xsave = (struct xsave_struct *)dest;
Pointless cast.
> +void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
> +{
> + struct xsave_struct *xsave = v->arch.xsave_area;
> + struct xsave_struct *src_xsave = (struct xsave_struct *)src;
Here the cast is even bogus, as it removes const-ness.
> + u64 xstate_bv = src_xsave->xsave_hdr.xstate_bv;
> + u64 valid;
> +
> + ASSERT(!xsave_area_compressed((struct xsave_struct *)src));
Pointless cast (to same type).
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen
2015-10-13 13:49 ` Jan Beulich
@ 2015-10-15 1:58 ` Shuai Ruan
[not found] ` <20151015015832.GA11071@shuai.ruan@linux.intel.com>
1 sibling, 0 replies; 13+ messages in thread
From: Shuai Ruan @ 2015-10-15 1:58 UTC (permalink / raw)
To: Jan Beulich
Cc: kevin.tian, wei.liu2, Ian.Campbell, stefano.stabellini,
andrew.cooper3, eddie.dong, xen-devel, jun.nakajima, keir,
ian.jackson
On Tue, Oct 13, 2015 at 07:49:12AM -0600, Jan Beulich wrote:
> >>> On 12.10.15 at 08:07, <shuai.ruan@linux.intel.com> wrote:
>
> > @@ -2257,9 +2261,14 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> > v->arch.xcr0_accum = ctxt->xcr0_accum;
> > if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
> > v->arch.nonlazy_xstate_used = 1;
> > - memcpy(v->arch.xsave_area, &ctxt->save_area,
> > - min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
> > - save_area));
> > + if ( (cpu_has_xsaves || cpu_has_xsavec) )
> > + compress_xsave_states(v, &ctxt->save_area,
> > + min(desc->length, size) -
> > + offsetof(struct hvm_hw_cpu_xsave,save_area));
> > + else
> > + memcpy(v->arch.xsave_area, &ctxt->save_area,
> > + min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
> > + save_area));
>
> Each time I look at these two hunks I wonder whether the condition
> and memcpy() wouldn't better move into the new called functions.
>
Will func name
void load_xsave_area(struct vcpu *v, struct hvm_hw_cpu_xsave *ctxt)
void save_xsave_area(struct vcpu *v, struct hvm_hw_cpu_xsave *ctxt)
Ok ?
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen
[not found] ` <20151015015832.GA11071@shuai.ruan@linux.intel.com>
@ 2015-10-15 6:48 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2015-10-15 6:48 UTC (permalink / raw)
To: Shuai Ruan
Cc: kevin.tian, wei.liu2, eddie.dong, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, jun.nakajima, keir,
Ian.Campbell
>>> On 15.10.15 at 03:58, <shuai.ruan@linux.intel.com> wrote:
> On Tue, Oct 13, 2015 at 07:49:12AM -0600, Jan Beulich wrote:
>> >>> On 12.10.15 at 08:07, <shuai.ruan@linux.intel.com> wrote:
>>
>> > @@ -2257,9 +2261,14 @@ static int hvm_load_cpu_xsave_states(struct domain
> *d, hvm_domain_context_t *h)
>> > v->arch.xcr0_accum = ctxt->xcr0_accum;
>> > if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
>> > v->arch.nonlazy_xstate_used = 1;
>> > - memcpy(v->arch.xsave_area, &ctxt->save_area,
>> > - min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
>> > - save_area));
>> > + if ( (cpu_has_xsaves || cpu_has_xsavec) )
>> > + compress_xsave_states(v, &ctxt->save_area,
>> > + min(desc->length, size) -
>> > + offsetof(struct
> hvm_hw_cpu_xsave,save_area));
>> > + else
>> > + memcpy(v->arch.xsave_area, &ctxt->save_area,
>> > + min(desc->length, size) - offsetof(struct hvm_hw_cpu_xsave,
>> > + save_area));
>>
>> Each time I look at these two hunks I wonder whether the condition
>> and memcpy() wouldn't better move into the new called functions.
>>
> Will func name
> void load_xsave_area(struct vcpu *v, struct hvm_hw_cpu_xsave *ctxt)
> void save_xsave_area(struct vcpu *v, struct hvm_hw_cpu_xsave *ctxt)
> Ok ?
Yes, or just keep the current names. (As a side note - one of the
two clearly wants its second argument constified.)
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-10-15 6:48 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12 6:07 [V6 0/4] add xsaves/xrstors support Shuai Ruan
2015-10-12 6:07 ` [V6 1/4] x86/xsaves: add basic definitions/helpers to support xsaves Shuai Ruan
2015-10-12 9:25 ` Andrew Cooper
2015-10-13 13:33 ` Jan Beulich
2015-10-12 6:07 ` [V6 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen Shuai Ruan
2015-10-12 10:05 ` Andrew Cooper
2015-10-13 7:28 ` Shuai Ruan
2015-10-13 13:49 ` Jan Beulich
2015-10-15 1:58 ` Shuai Ruan
[not found] ` <20151015015832.GA11071@shuai.ruan@linux.intel.com>
2015-10-15 6:48 ` Jan Beulich
2015-10-12 6:07 ` [V6 3/4] x86/xsaves: enable xsaves/xrstors for hvm guest Shuai Ruan
2015-10-12 6:07 ` [V6 4/4] libxc: expose xsaves/xgetbv1/xsavec to " Shuai Ruan
2015-10-12 10:11 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.