On Mon, Jan 23, 2017 at 06:38:42PM -0800, Dave Hansen wrote: > On 01/23/2017 06:09 PM, Kevin Hao wrote: > > On Mon, Jan 23, 2017 at 06:01:10PM -0800, Dave Hansen wrote: > >> On 01/23/2017 05:50 PM, Kevin Hao wrote: > >>> 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. > >> What purpose does it serve to make copyin_to_xsaves() set that bit, > > We try to fake up a memory area which is supposed to be composed by XSAVES > > instruction. My code is just trying to do what the XSAVES do. > > No. copyin_to_xsaves() copies data into an *existing* XSAVES-formatted > buffer. If you want to change what it does, fine, but that's not what > it does or tries to do today. No, I didn't change what the function copyin_to_xsaves() does. I just tried to fix the bug in it. > > >> other than helping to hide bugs? > > Why do you think it hide the bug? In contrast, I think my patch fixes what the > > bug really is. The memory area we fake up is bug, we should fix it there. > > Yu-cheng found the bug. That bug will probably manifest in other code > paths than copyin_to_xsaves(). If we did your patch, it would hide the > bug in those other code paths. The XCOMP_BV[62:0] is supposed to be updated by XSAVEC/XSAVES instructions. It should not be touched by the software in theory except some special cases like what we do in copyin_to_xsaves(). Trying to init the XCOMP_BV[62:0] to some assuming values in fpstate_init() should be error prone and just paper over the real bug. Thanks, Kevin
Attachment:
signature.asc
Description: PGP signature
|