Re: [PATCH] x86/fpu/xstate: calculate the number by sizeof and offsetof
Thank you for your suggestion! On Wed, Jan 20, 2021 at 6:22 PM Borislav Petkov wrote: > > On Wed, Jan 20, 2021 at 02:44:15PM +0800, Yejune Deng wrote: > > In fpstate_sanitize_xstate(), use memset and offsetof instead of '= 0', > > and use sizeof instead of a constant. > > > > Signed-off-by: Yejune Deng > > --- > > arch/x86/kernel/fpu/xstate.c | 10 +++--- > > 1 file changed, 3 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > > index 5d8047441a0a..2e75630c86e9 100644 > > --- a/arch/x86/kernel/fpu/xstate.c > > +++ b/arch/x86/kernel/fpu/xstate.c > > @@ -161,20 +161,16 @@ void fpstate_sanitize_xstate(struct fpu *fpu) > >* FP is in init state > >*/ > > if (!(xfeatures & XFEATURE_MASK_FP)) { > > + memset(fx, 0, offsetof(struct fxregs_state, mxcsr)); > > I'd prefer the explicit zeroing instead of having to look at > fxregs_state and where the offset of mxcsr is. > > > + memset(fx->st_space, 0, sizeof(fx->st_space)); > > This is ok I guess. > > > fx->cwd = 0x37f; > > - fx->swd = 0; > > - fx->twd = 0; > > - fx->fop = 0; > > - fx->rip = 0; > > - fx->rdp = 0; > > - memset(>st_space[0], 0, 128); > > } > > > > /* > >* SSE is in init state > >*/ > > if (!(xfeatures & XFEATURE_MASK_SSE)) > > - memset(>xmm_space[0], 0, 256); > > + memset(fx->xmm_space, 0, sizeof(fx->xmm_space)); > > This too. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/fpu/xstate: calculate the number by sizeof and offsetof
On Wed, Jan 20, 2021 at 02:44:15PM +0800, Yejune Deng wrote: > In fpstate_sanitize_xstate(), use memset and offsetof instead of '= 0', > and use sizeof instead of a constant. > > Signed-off-by: Yejune Deng > --- > arch/x86/kernel/fpu/xstate.c | 10 +++--- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index 5d8047441a0a..2e75630c86e9 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -161,20 +161,16 @@ void fpstate_sanitize_xstate(struct fpu *fpu) >* FP is in init state >*/ > if (!(xfeatures & XFEATURE_MASK_FP)) { > + memset(fx, 0, offsetof(struct fxregs_state, mxcsr)); I'd prefer the explicit zeroing instead of having to look at fxregs_state and where the offset of mxcsr is. > + memset(fx->st_space, 0, sizeof(fx->st_space)); This is ok I guess. > fx->cwd = 0x37f; > - fx->swd = 0; > - fx->twd = 0; > - fx->fop = 0; > - fx->rip = 0; > - fx->rdp = 0; > - memset(>st_space[0], 0, 128); > } > > /* >* SSE is in init state >*/ > if (!(xfeatures & XFEATURE_MASK_SSE)) > - memset(>xmm_space[0], 0, 256); > + memset(fx->xmm_space, 0, sizeof(fx->xmm_space)); This too. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/fpu/xstate: calculate the number by sizeof and offsetof
On 1/19/21 10:44 PM, Yejune Deng wrote: > In fpstate_sanitize_xstate(), use memset and offsetof instead of '= 0', > and use sizeof instead of a constant. What's the benefit to doing this? Saving 4 lines of code? Your suggestions are not obviously wrong at a glance, but they're also not obviously right.
[PATCH] x86/fpu/xstate: calculate the number by sizeof and offsetof
In fpstate_sanitize_xstate(), use memset and offsetof instead of '= 0', and use sizeof instead of a constant. Signed-off-by: Yejune Deng --- arch/x86/kernel/fpu/xstate.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 5d8047441a0a..2e75630c86e9 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -161,20 +161,16 @@ void fpstate_sanitize_xstate(struct fpu *fpu) * FP is in init state */ if (!(xfeatures & XFEATURE_MASK_FP)) { + memset(fx, 0, offsetof(struct fxregs_state, mxcsr)); + memset(fx->st_space, 0, sizeof(fx->st_space)); fx->cwd = 0x37f; - fx->swd = 0; - fx->twd = 0; - fx->fop = 0; - fx->rip = 0; - fx->rdp = 0; - memset(>st_space[0], 0, 128); } /* * SSE is in init state */ if (!(xfeatures & XFEATURE_MASK_SSE)) - memset(>xmm_space[0], 0, 256); + memset(fx->xmm_space, 0, sizeof(fx->xmm_space)); /* * First two features are FPU and SSE, which above we handled -- 2.29.0