| Message ID | 1456119321-10384-2-git-send-email-shuai.ruan@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 22.02.16 at 06:35, <shuai.ruan@linux.intel.com> wrote: > Previous patch using all available features caculate xstate_comp_offsets. > This is wrong.This patch fix this bug by caculating the xstate_comp_offset In the title and above: calculate (and alike). > based on xcomp_bv of current guest. > Also, the xstate_comp_offset should take alignment into consideration. Reported-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com> [...] > @@ -106,34 +107,44 @@ static int setup_xstate_features(bool_t bsp) > xstate_sizes = xzalloc_array(unsigned int, xstate_features); > if ( !xstate_sizes ) > return -ENOMEM; > + > + xstate_align = xzalloc_array(unsigned int, xstate_features); > + if ( !xstate_align ) > + return -ENOMEM; > } > > for ( leaf = 2; leaf < xstate_features; leaf++ ) > { > if ( bsp ) > + { > cpuid_count(XSTATE_CPUID, leaf, &xstate_sizes[leaf], > - &xstate_offsets[leaf], &tmp, &tmp); > + &xstate_offsets[leaf], &ecx, &edx); > + xstate_align[leaf] = ecx & XSTATE_ALIGN64; > + } Bogus hard tabs. > -static void __init setup_xstate_comp(void) > +static void setup_xstate_comp(const struct xsave_struct *xsave) > { > unsigned int i; > + u64 xcomp_bv = xsave->xsave_hdr.xcomp_bv; It looks like it would suffice if the caller passed xcomp_bv into this function. > /* > * 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. > */ > + memset(xstate_comp_offsets, 0, sizeof(xstate_comp_offsets)); > xstate_comp_offsets[0] = 0; The addition makes this line redundant. However, you act on static data here, and I don't think there's any serialization between the various callers. > @@ -141,8 +152,10 @@ static void __init setup_xstate_comp(void) > > for ( i = 3; i < xstate_features; i++ ) > { > - xstate_comp_offsets[i] = xstate_comp_offsets[i - 1] + > - (((1ul << i) & xfeature_mask) > + xstate_comp_offsets[i] = (xstate_align[i] ? > + ROUNDUP(xstate_comp_offsets[i-1], 64) : Coding style. > @@ -172,6 +185,7 @@ void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size) > } > > ASSERT(xsave_area_compressed(xsave)); > + setup_xstate_comp(xsave); > /* Please add a blank line before the comment. > @@ -223,6 +237,7 @@ void compress_xsave_states(struct vcpu *v, const void > *src, unsigned int size) > xsave->xsave_hdr.xstate_bv = xstate_bv; > xsave->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED; > > + setup_xstate_comp(xsave); > /* Please retain a blank line before the comment. > --- a/xen/include/asm-x86/xstate.h > +++ b/xen/include/asm-x86/xstate.h > @@ -44,6 +44,7 @@ > #define XSTATE_LAZY (XSTATE_ALL & ~XSTATE_NONLAZY) > #define XSTATE_COMPACTION_ENABLED (1ULL << 63) > > +#define XSTATE_ALIGN64 (1ULL << 1) > extern u64 xfeature_mask; Missing blank line again. Jan
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c index 4f2fb8e..0e7643b 100644 --- a/xen/arch/x86/xstate.c +++ b/xen/arch/x86/xstate.c @@ -26,6 +26,7 @@ u64 __read_mostly xfeature_mask; static unsigned int *__read_mostly xstate_offsets; unsigned int *__read_mostly xstate_sizes; +static unsigned int *__read_mostly xstate_align; static unsigned int __read_mostly xstate_features; static unsigned int __read_mostly xstate_comp_offsets[sizeof(xfeature_mask)*8]; @@ -94,7 +95,7 @@ static bool_t xsave_area_compressed(const struct xsave_struct *xsave_area) static int setup_xstate_features(bool_t bsp) { - unsigned int leaf, tmp, eax, ebx; + unsigned int leaf, eax, ebx, ecx, edx; if ( bsp ) { @@ -106,34 +107,44 @@ static int setup_xstate_features(bool_t bsp) xstate_sizes = xzalloc_array(unsigned int, xstate_features); if ( !xstate_sizes ) return -ENOMEM; + + xstate_align = xzalloc_array(unsigned int, xstate_features); + if ( !xstate_align ) + return -ENOMEM; } for ( leaf = 2; leaf < xstate_features; leaf++ ) { if ( bsp ) + { cpuid_count(XSTATE_CPUID, leaf, &xstate_sizes[leaf], - &xstate_offsets[leaf], &tmp, &tmp); + &xstate_offsets[leaf], &ecx, &edx); + xstate_align[leaf] = ecx & XSTATE_ALIGN64; + } else { cpuid_count(XSTATE_CPUID, leaf, &eax, - &ebx, &tmp, &tmp); + &ebx, &ecx, &edx); BUG_ON(eax != xstate_sizes[leaf]); BUG_ON(ebx != xstate_offsets[leaf]); + BUG_ON((ecx & XSTATE_ALIGN64) != xstate_align[leaf]); } } return 0; } -static void __init setup_xstate_comp(void) +static void setup_xstate_comp(const struct xsave_struct *xsave) { unsigned int i; + u64 xcomp_bv = xsave->xsave_hdr.xcomp_bv; /* * 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. */ + memset(xstate_comp_offsets, 0, sizeof(xstate_comp_offsets)); xstate_comp_offsets[0] = 0; xstate_comp_offsets[1] = XSAVE_SSE_OFFSET; @@ -141,8 +152,10 @@ static void __init setup_xstate_comp(void) for ( i = 3; i < xstate_features; i++ ) { - xstate_comp_offsets[i] = xstate_comp_offsets[i - 1] + - (((1ul << i) & xfeature_mask) + xstate_comp_offsets[i] = (xstate_align[i] ? + ROUNDUP(xstate_comp_offsets[i-1], 64) : + xstate_comp_offsets[i - 1]) + + (((1ul << i) & xcomp_bv) ? xstate_sizes[i - 1] : 0); ASSERT(xstate_comp_offsets[i] + xstate_sizes[i] <= xsave_cntxt_size); } @@ -172,6 +185,7 @@ void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size) } ASSERT(xsave_area_compressed(xsave)); + setup_xstate_comp(xsave); /* * Copy legacy XSAVE area and XSAVE hdr area. */ @@ -223,6 +237,7 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size) xsave->xsave_hdr.xstate_bv = xstate_bv; xsave->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED; + setup_xstate_comp(xsave); /* * Copy each region from the non-compacted offset to the * possibly compacted offset. @@ -568,8 +583,6 @@ void xstate_init(struct cpuinfo_x86 *c) if ( setup_xstate_features(bsp) && 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 84f0af9..0215070 100644 --- a/xen/include/asm-x86/xstate.h +++ b/xen/include/asm-x86/xstate.h @@ -44,6 +44,7 @@ #define XSTATE_LAZY (XSTATE_ALL & ~XSTATE_NONLAZY) #define XSTATE_COMPACTION_ENABLED (1ULL << 63) +#define XSTATE_ALIGN64 (1ULL << 1) extern u64 xfeature_mask; extern unsigned int *xstate_sizes;
Previous patch using all available features caculate xstate_comp_offsets. This is wrong.This patch fix this bug by caculating the xstate_comp_offset based on xcomp_bv of current guest. Also, the xstate_comp_offset should take alignment into consideration. Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com> --- xen/arch/x86/xstate.c | 29 +++++++++++++++++++++-------- xen/include/asm-x86/xstate.h | 1 + 2 files changed, 22 insertions(+), 8 deletions(-)