tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: [PATCH v3 2/2] Implement PT_GETXSTATE and PT_SETXSTATE

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.


		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) |

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

Unfortunately, in this code you will likely hit a race; between the moment you
set fpu_save->sv_xsave_hdr.xsh_xstate_bv and the moment you reach COPY_COMPONENT,
the thread could have been context-switched, and xstate_bv could have been

Unless I'm mistaken, this code should only be executed on stopped
processes.  Or at least that was the idea Kamil suggested to me.

Mmh yes indeed, I even said it earlier in the thread; forget it, I was trying
to remember where I had seen races in the FPU code, and it's not a problem

Home | Main Index | Thread Index | Old Index