On Sat, 2019-06-22 at 08:26 +0200, Maxime Villard wrote: > Le 22/06/2019 à 07:42, Michał Górny a écrit : > > On Sat, 2019-06-22 at 07:35 +0200, Maxime Villard wrote: > > > Le 19/06/2019 à 14:20, Michał Górny a écrit :> On Wed, 2019-06-19 at 06:32 +0200, Maxime Villard wrote: > > > > > Can you provide a unified patch so I can review quickly? Sorry for the delay > > > > > > > > Here you are. Note that the manpage is not updated yet, as I'm waiting > > > > for your comments on the xs_rfbm approach. > > > > + memcpy(&(xstate -> field), \ > > > > + (char*)fpu_save + x86_xsave_offsets[xsave_val], \ > > > > + sizeof(xstate -> field)); \ > > > > > > Please use proper KNF here and in other places > > > > Are you talking of indentation? It would be really helpful to be more > > specific here. > > Yes > > memcpy(&xstate->field, \ > (char*)fpu_save + x86_xsave_offsets[xsave_val], \ > sizeof(xstate->field)); \ > > But that's just cosmetic > > > > > + fpu_save->sv_xsave_hdr.xsh_xstate_bv = > > > > + (~xstate->xs_rfbm | xstate->xs_xstate_bv) & > > > > + (xstate->xs_rfbm | fpu_save->sv_xsave_hdr.xsh_xstate_bv); > > > > > > Can't we just do this? Seems simpler > > > > > > fpu_save->sv_xsave_hdr.xsh_xstate_bv = > > > (fpu_save->sv_xsave_hdr.xsh_xstate_bv & ~xstate->xs_rfbm) | > > > xstate->xs_xstate_bv; > > > > Unless I'm mistaken, this would prevent the user from being able to > > force xs_xstate_bv to 0 if it was set previously, and therefore clear > > registers to 'unmodified' state. > > If the user gives RFBM=all and XSTATE_BV=0, it does force the result to zero I'm sorry, I read it the other way around. Yes, given that we force xs_xstate_bv to be a subset of xs_rfbm, this is going to work correctly. I'm going to update the patch, the manpage and send you a new version. -- Best regards, Michał Górny
Attachment:
signature.asc
Description: This is a digitally signed message part