Re: [tip:x86/urgent] x86/fpu: Set the xcomp_bv when we fake up a XSAVES area

[Kevin Hao]

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