>>> On 12.09.16 at 11:51, <andrew.coop...@citrix.com> wrote: > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -1158,7 +1158,15 @@ long arch_do_domctl( > goto vcpuextstate_out; > } > > - if ( evc->size <= PV_XSAVE_SIZE(_xcr0_accum) ) > + if ( evc->size == PV_XSAVE_HDR_SIZE ) > + ; /* Nothing to restore. */ > + else if ( evc->size < PV_XSAVE_HDR_SIZE + XSTATE_AREA_MIN_SIZE ) > + ret = -EINVAL; /* Can't be legitimate data. */ > + else if ( xsave_area_compressed(_xsave_area) ) > + ret = -EOPNOTSUPP; /* Don't support compressed data. */ > + else if ( evc->size != PV_XSAVE_SIZE(_xcr0_accum) ) > + ret = -EINVAL; /* Not legitimate data. */
Can't this be moved ahead of the xsave_area_compressed() check, eliminating (as redundant) the check that's there right now? In any event the tightening to != you do here supports my desire to not do any relaxation of the size check in patch 2. Or alternatively you would want to consider restoring the behavior from prior to the xsaves change, where the <= which you now remove was still okay. > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -1345,6 +1345,23 @@ static int hvm_load_cpu_xsave_states(struct domain *d, > hvm_domain_context_t *h) > printk(XENLOG_G_WARNING > "HVM%d.%u restore mismatch: xsave length %#x > %#x\n", > d->domain_id, vcpuid, desc->length, size); > + /* Rewind desc->length to ignore the extraneous zeros. */ > + desc->length = size; This is slightly ugly, as it will prevent eventually constifying dest (which it really should have been from the beginning). > --- a/xen/include/asm-x86/xstate.h > +++ b/xen/include/asm-x86/xstate.h > @@ -131,4 +131,10 @@ static inline bool_t xstate_all(const struct vcpu *v) > (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE); > } > > +static inline bool __nonnull(1) > + xsave_area_compressed(const struct xsave_struct *xsave_area) This is certainly odd indentation. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
