On Mon, Jan 23, 2017 at 04:53:25PM -0800, Dave Hansen wrote: > >> The fix I am proposing is... > >> > >> state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | > >> xfeatures_mask; > > > > Actually I thought about this change before I made this patch, but I don't this > > is the right fix. It is always error prone to init the xcomp_bv to all the > > supported feature. In case like copyin_to_xsaves(), it is possible that the > > features which should be set in xcomp_bv do not equal to all the supported > > features. Please see the following codes in copyin_to_xsaves(): > > /* > > * The state that came in from userspace was user-state only. > > * Mask all the user states out of 'xfeatures': > > */ > > xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR; > > > > /* > > * Add back in the features that came in from userspace: > > */ > > xsave->header.xfeatures |= xfeatures; > > Hi Kevin, > > I think you may be confusing 'xfeatures' with 'xcomp_bv'. xfeatures > tells you what features are present in the buffer while xcomp_bv tells > you what *format* the buffer is in. According to the ISA manual, XSAVES also set the XCOMP_BV[62:0]. My code only try to be compatible with what the cpu does when excuting XSAVES. The following is quoted from 325462-sdm-vol-1-2abcd-3abcd.pdf. The XSAVES instructions sets bit 63 of the XCOMP_BV field of the XSAVE header while writing RFBM[62:0] to XCOMP_BV[62:0]. The XSAVES instruction does not write any part of the XSAVE header other than the XSTATE_BV and XCOMP_BV fields. Thanks, Kevin > > Userspace never dictates the *format* of the kernel buffer, only the > contents of each state. So, it makes sense that the copyin code would > not (and should not) modify xcomp_bv. > > We ensure that xcomp_bv has all supported states set all the time, or > we're *supposed* to.
Attachment:
signature.asc
Description: PGP signature
|