All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] x86/fpu: minor cleanups
       [not found]     ` <20150302174818.GA16886@redhat.com>
@ 2015-03-02 18:32       ` Oleg Nesterov
  2015-03-02 18:32         ` [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave Oleg Nesterov
  2015-03-02 18:32         ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Oleg Nesterov
  0 siblings, 2 replies; 12+ messages in thread
From: Oleg Nesterov @ 2015-03-02 18:32 UTC (permalink / raw)
  To: Tavis Ormandy, Borislav Petkov, Ingo Molnar
  Cc: Rik van Riel, Andy Lutomirski, Linus Torvalds, linux-kernel

add lkml/cc's.

On 03/02, Oleg Nesterov wrote:
>
> Btw, why xstateregs_get/set() looks so confusing? The comment says
> "Copy the 48bytes defined by the software first into the xstate" but the
> code uses ->fxsave. Yes, this is the same memory, but still. I'll send
> the cosmetic cleanup.
>
> Or fx_finit()... Again, memset(fx, 0, xstate_size) is correct but only
> because of the current layout. I think this needs a cleanup too.

Of course this is purely cosmetic, but still...

Oleg.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave
  2015-03-02 18:32       ` [PATCH 0/2] x86/fpu: minor cleanups Oleg Nesterov
@ 2015-03-02 18:32         ` Oleg Nesterov
  2015-03-03 19:23           ` Rik van Riel
                             ` (2 more replies)
  2015-03-02 18:32         ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Oleg Nesterov
  1 sibling, 3 replies; 12+ messages in thread
From: Oleg Nesterov @ 2015-03-02 18:32 UTC (permalink / raw)
  To: Tavis Ormandy, Borislav Petkov, Ingo Molnar
  Cc: Rik van Riel, Andy Lutomirski, Linus Torvalds, linux-kernel

Cosmetic. xstateregs_get() and xstateregs_set() abuse ->fxsave to access
xsave->i387.sw_reserved. This is correct, ->fxsave and xsave->i387 share
the same memory, but imho this looks confusing.

And we can make this code more readable if we add "struct xsave_struct *"
local.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/kernel/i387.c |   25 +++++++++----------------
 1 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 8e070a6..4b12df8 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -339,6 +339,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 		unsigned int pos, unsigned int count,
 		void *kbuf, void __user *ubuf)
 {
+	struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
 	int ret;
 
 	if (!cpu_has_xsave)
@@ -353,14 +354,12 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 	 * memory layout in the thread struct, so that we can copy the entire
 	 * xstateregs to the user using one user_regset_copyout().
 	 */
-	memcpy(&target->thread.fpu.state->fxsave.sw_reserved,
-	       xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes));
-
+	memcpy(&xsave->i387.sw_reserved,
+		xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes));
 	/*
 	 * Copy the xstate memory layout.
 	 */
-	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
-				  &target->thread.fpu.state->xsave, 0, -1);
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 	return ret;
 }
 
@@ -368,8 +367,8 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 		  unsigned int pos, unsigned int count,
 		  const void *kbuf, const void __user *ubuf)
 {
+	struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
 	int ret;
-	struct xsave_hdr_struct *xsave_hdr;
 
 	if (!cpu_has_xsave)
 		return -ENODEV;
@@ -378,22 +377,16 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 	if (ret)
 		return ret;
 
-	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
-				 &target->thread.fpu.state->xsave, 0, -1);
-
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
-	target->thread.fpu.state->fxsave.mxcsr &= mxcsr_feature_mask;
-
-	xsave_hdr = &target->thread.fpu.state->xsave.xsave_hdr;
-
-	xsave_hdr->xstate_bv &= pcntxt_mask;
+	xsave->i387.mxcsr &= mxcsr_feature_mask;
+	xsave->xsave_hdr.xstate_bv &= pcntxt_mask;
 	/*
 	 * These bits must be zero.
 	 */
-	memset(xsave_hdr->reserved, 0, 48);
-
+	memset(&xsave->xsave_hdr.reserved, 0, 48);
 	return ret;
 }
 
-- 
1.5.5.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths
  2015-03-02 18:32       ` [PATCH 0/2] x86/fpu: minor cleanups Oleg Nesterov
  2015-03-02 18:32         ` [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave Oleg Nesterov
@ 2015-03-02 18:32         ` Oleg Nesterov
  2015-03-03 22:01           ` Rik van Riel
                             ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Oleg Nesterov @ 2015-03-02 18:32 UTC (permalink / raw)
  To: Tavis Ormandy, Borislav Petkov, Ingo Molnar
  Cc: Rik van Riel, Andy Lutomirski, Linus Torvalds, linux-kernel

fx_finit() has 2 users but only fpu_finit() needs to nullify xstate,
alloc_bootmem_align() in setup_init_fpu_buf() returns zero-filled
memory.

And note that both memset()'s look confusing. Yes, offsetof() is 0
for ->fxsave or ->fsave, but it would be more clean to turn them into
a single memset() which nullifies fpu->state.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/include/asm/fpu-internal.h |    1 -
 arch/x86/kernel/i387.c              |    3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 02f2e08..8809b5a 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -107,7 +107,6 @@ static __always_inline __pure bool use_fxsr(void)
 
 static inline void fx_finit(struct i387_fxsave_struct *fx)
 {
-	memset(fx, 0, xstate_size);
 	fx->cwd = 0x37f;
 	fx->mxcsr = MXCSR_DEFAULT;
 }
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 4b12df8..9b7759f 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -224,11 +224,12 @@ void fpu_finit(struct fpu *fpu)
 		return;
 	}
 
+	memset(fpu->state, 0, xstate_size);
+
 	if (cpu_has_fxsr) {
 		fx_finit(&fpu->state->fxsave);
 	} else {
 		struct i387_fsave_struct *fp = &fpu->state->fsave;
-		memset(fp, 0, xstate_size);
 		fp->cwd = 0xffff037fu;
 		fp->swd = 0xffff0000u;
 		fp->twd = 0xffffffffu;
-- 
1.5.5.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave
  2015-03-02 18:32         ` [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave Oleg Nesterov
@ 2015-03-03 19:23           ` Rik van Riel
  2015-03-04 10:30           ` Borislav Petkov
  2015-03-10 10:08           ` [tip:x86/fpu] x86/fpu: Change xstateregs_get()/set() to use -> xsave.i387 " tip-bot for Oleg Nesterov
  2 siblings, 0 replies; 12+ messages in thread
From: Rik van Riel @ 2015-03-03 19:23 UTC (permalink / raw)
  To: Oleg Nesterov, Tavis Ormandy, Borislav Petkov, Ingo Molnar
  Cc: Andy Lutomirski, Linus Torvalds, linux-kernel

On 03/02/2015 01:32 PM, Oleg Nesterov wrote:
> Cosmetic. xstateregs_get() and xstateregs_set() abuse ->fxsave to access
> xsave->i387.sw_reserved. This is correct, ->fxsave and xsave->i387 share
> the same memory, but imho this looks confusing.
> 
> And we can make this code more readable if we add "struct xsave_struct *"
> local.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths
  2015-03-02 18:32         ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Oleg Nesterov
@ 2015-03-03 22:01           ` Rik van Riel
  2015-03-04 11:38           ` Borislav Petkov
  2015-03-10 10:08           ` [tip:x86/fpu] x86/fpu: Factor out memset(xstate, 0) in fpu_finit( ) paths tip-bot for Oleg Nesterov
  2 siblings, 0 replies; 12+ messages in thread
From: Rik van Riel @ 2015-03-03 22:01 UTC (permalink / raw)
  To: Oleg Nesterov, Tavis Ormandy, Borislav Petkov, Ingo Molnar
  Cc: Andy Lutomirski, Linus Torvalds, linux-kernel

On 03/02/2015 01:32 PM, Oleg Nesterov wrote:
> fx_finit() has 2 users but only fpu_finit() needs to nullify xstate,
> alloc_bootmem_align() in setup_init_fpu_buf() returns zero-filled
> memory.
> 
> And note that both memset()'s look confusing. Yes, offsetof() is 0
> for ->fxsave or ->fsave, but it would be more clean to turn them into
> a single memset() which nullifies fpu->state.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave
  2015-03-02 18:32         ` [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave Oleg Nesterov
  2015-03-03 19:23           ` Rik van Riel
@ 2015-03-04 10:30           ` Borislav Petkov
  2015-03-10 10:08           ` [tip:x86/fpu] x86/fpu: Change xstateregs_get()/set() to use -> xsave.i387 " tip-bot for Oleg Nesterov
  2 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2015-03-04 10:30 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Tavis Ormandy, Ingo Molnar, Rik van Riel, Andy Lutomirski,
	Linus Torvalds, linux-kernel

On Mon, Mar 02, 2015 at 07:32:37PM +0100, Oleg Nesterov wrote:
> Cosmetic. xstateregs_get() and xstateregs_set() abuse ->fxsave to access
> xsave->i387.sw_reserved. This is correct, ->fxsave and xsave->i387 share
> the same memory, but imho this looks confusing.
> 
> And we can make this code more readable if we add "struct xsave_struct *"
> local.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths
  2015-03-02 18:32         ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Oleg Nesterov
  2015-03-03 22:01           ` Rik van Riel
@ 2015-03-04 11:38           ` Borislav Petkov
  2015-03-10 10:08           ` [tip:x86/fpu] x86/fpu: Factor out memset(xstate, 0) in fpu_finit( ) paths tip-bot for Oleg Nesterov
  2 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2015-03-04 11:38 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Tavis Ormandy, Ingo Molnar, Rik van Riel, Andy Lutomirski,
	Linus Torvalds, linux-kernel

On Mon, Mar 02, 2015 at 07:32:57PM +0100, Oleg Nesterov wrote:
> fx_finit() has 2 users but only fpu_finit() needs to nullify xstate,
> alloc_bootmem_align() in setup_init_fpu_buf() returns zero-filled
> memory.
> 
> And note that both memset()'s look confusing. Yes, offsetof() is 0
> for ->fxsave or ->fsave, but it would be more clean to turn them into
> a single memset() which nullifies fpu->state.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [GIT PULL] x86/fpu cleanups
@ 2015-03-09 18:08 Borislav Petkov
  2015-03-10  6:06 ` [PATCH 1/2] x86/fpu: Change xstateregs_get()/set() to use ->xsave.i387 rather than ->fxsave Borislav Petkov
  0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2015-03-09 18:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Oleg Nesterov, x86-ml, lkml

Hi Ingo,

please pull two more cleanups to the FPU insanity from Oleg.

So this is *not* the urgent stuff - the urgent stuff will come later.
This is the unload-some-more-onto-tip stuff which can be safely queued
for 4.1 and I can forget about it here. :)

Thanks.

---
The following changes since commit ae486033b980346eb6a77240101210cb66924a91:

  Merge tag 'tip_x86_fpu' of git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp into x86/fpu (2015-03-03 12:12:15 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git tags/tip_x86_fpu_2

for you to fetch changes up to 9ed010d31f173be786f0398a64f08bb4a0300c3b:

  x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths (2015-03-04 11:57:46 +0100)

----------------------------------------------------------------
Two more cleanups from Oleg.

----------------------------------------------------------------
Oleg Nesterov (2):
      x86/fpu: Change xstateregs_get()/set() to use ->xsave.i387 rather than ->fxsave
      x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths

 arch/x86/include/asm/fpu-internal.h |  1 -
 arch/x86/kernel/i387.c              | 28 +++++++++++-----------------
 2 files changed, 11 insertions(+), 18 deletions(-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] x86/fpu: Change xstateregs_get()/set() to use ->xsave.i387 rather than ->fxsave
  2015-03-09 18:08 [GIT PULL] x86/fpu cleanups Borislav Petkov
@ 2015-03-10  6:06 ` Borislav Petkov
  2015-03-10  6:06   ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Borislav Petkov
  0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2015-03-10  6:06 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Oleg Nesterov <oleg@redhat.com>

This is a cosmetic change: xstateregs_get() and xstateregs_set() abuse
->fxsave to access xsave->i387.sw_reserved. This is correct, ->fxsave
and xsave->i387 share the same memory, but IMHO this looks confusing.

And we can make this code more readable if we add "struct xsave_struct
*" local.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Rik van Riel <riel@redhat.com>
Cc: Tavis Ormandy <taviso@google.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: <x86@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20150302183237.GB23085@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/i387.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 8416b5f85806..03cc0add8694 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -339,6 +339,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 		unsigned int pos, unsigned int count,
 		void *kbuf, void __user *ubuf)
 {
+	struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
 	int ret;
 
 	if (!cpu_has_xsave)
@@ -353,14 +354,12 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 	 * memory layout in the thread struct, so that we can copy the entire
 	 * xstateregs to the user using one user_regset_copyout().
 	 */
-	memcpy(&target->thread.fpu.state->fxsave.sw_reserved,
-	       xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes));
-
+	memcpy(&xsave->i387.sw_reserved,
+		xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes));
 	/*
 	 * Copy the xstate memory layout.
 	 */
-	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
-				  &target->thread.fpu.state->xsave, 0, -1);
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 	return ret;
 }
 
@@ -368,8 +367,8 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 		  unsigned int pos, unsigned int count,
 		  const void *kbuf, const void __user *ubuf)
 {
+	struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
 	int ret;
-	struct xsave_hdr_struct *xsave_hdr;
 
 	if (!cpu_has_xsave)
 		return -ENODEV;
@@ -378,22 +377,16 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 	if (ret)
 		return ret;
 
-	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
-				 &target->thread.fpu.state->xsave, 0, -1);
-
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
-	target->thread.fpu.state->fxsave.mxcsr &= mxcsr_feature_mask;
-
-	xsave_hdr = &target->thread.fpu.state->xsave.xsave_hdr;
-
-	xsave_hdr->xstate_bv &= pcntxt_mask;
+	xsave->i387.mxcsr &= mxcsr_feature_mask;
+	xsave->xsave_hdr.xstate_bv &= pcntxt_mask;
 	/*
 	 * These bits must be zero.
 	 */
-	memset(xsave_hdr->reserved, 0, 48);
-
+	memset(&xsave->xsave_hdr.reserved, 0, 48);
 	return ret;
 }
 
-- 
2.2.0.33.gc18b867


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths
  2015-03-10  6:06 ` [PATCH 1/2] x86/fpu: Change xstateregs_get()/set() to use ->xsave.i387 rather than ->fxsave Borislav Petkov
@ 2015-03-10  6:06   ` Borislav Petkov
  0 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2015-03-10  6:06 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: X86 ML, LKML

From: Oleg Nesterov <oleg@redhat.com>

fx_finit() has 2 users but only fpu_finit() needs to nullify xstate,
alloc_bootmem_align() in setup_init_fpu_buf() returns zero-filled
memory.

And note that both memset()'s look confusing. Yes, offsetof() is 0
for ->fxsave or ->fsave, but it would be more clean to turn them into
a single memset() which nullifies fpu->state.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Rik van Riel <riel@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tavis Ormandy <taviso@google.com>
Cc: <x86@kernel.org>
Link: http://lkml.kernel.org/r/20150302183257.GC23085@redhat.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/fpu-internal.h | 1 -
 arch/x86/kernel/i387.c              | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 61609b963eab..5fa1be21ac2a 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -135,7 +135,6 @@ static __always_inline __pure bool use_fxsr(void)
 
 static inline void fx_finit(struct i387_fxsave_struct *fx)
 {
-	memset(fx, 0, xstate_size);
 	fx->cwd = 0x37f;
 	fx->mxcsr = MXCSR_DEFAULT;
 }
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 03cc0add8694..0f3de6674ae3 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -224,11 +224,12 @@ void fpu_finit(struct fpu *fpu)
 		return;
 	}
 
+	memset(fpu->state, 0, xstate_size);
+
 	if (cpu_has_fxsr) {
 		fx_finit(&fpu->state->fxsave);
 	} else {
 		struct i387_fsave_struct *fp = &fpu->state->fsave;
-		memset(fp, 0, xstate_size);
 		fp->cwd = 0xffff037fu;
 		fp->swd = 0xffff0000u;
 		fp->twd = 0xffffffffu;
-- 
2.2.0.33.gc18b867


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:x86/fpu] x86/fpu: Change xstateregs_get()/set() to use -> xsave.i387 rather than ->fxsave
  2015-03-02 18:32         ` [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave Oleg Nesterov
  2015-03-03 19:23           ` Rik van Riel
  2015-03-04 10:30           ` Borislav Petkov
@ 2015-03-10 10:08           ` tip-bot for Oleg Nesterov
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-03-10 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: luto, riel, oleg, bp, hpa, taviso, linux-kernel, mingo, bp,
	torvalds, tglx

Commit-ID:  e7f180dcd8ab48f18b20d7e8a7e9b39192bdf8e0
Gitweb:     http://git.kernel.org/tip/e7f180dcd8ab48f18b20d7e8a7e9b39192bdf8e0
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 10 Mar 2015 07:06:24 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 Mar 2015 07:14:31 +0100

x86/fpu: Change xstateregs_get()/set() to use ->xsave.i387 rather than ->fxsave

This is a cosmetic change: xstateregs_get() and xstateregs_set()
abuse ->fxsave to access xsave->i387.sw_reserved.

This practice is correct, ->fxsave and xsave->i387 share the same memory,
but IMHO this looks confusing.

And we can make this code more readable if we add a
"struct xsave_struct *" local variable as well.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Rik van Riel <riel@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tavis Ormandy <taviso@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1425967585-4725-1-git-send-email-bp@alien8.de
Link: http://lkml.kernel.org/r/20150302183237.GB23085@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/i387.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 8416b5f..03cc0ad 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -339,6 +339,7 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 		unsigned int pos, unsigned int count,
 		void *kbuf, void __user *ubuf)
 {
+	struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
 	int ret;
 
 	if (!cpu_has_xsave)
@@ -353,14 +354,12 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 	 * memory layout in the thread struct, so that we can copy the entire
 	 * xstateregs to the user using one user_regset_copyout().
 	 */
-	memcpy(&target->thread.fpu.state->fxsave.sw_reserved,
-	       xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes));
-
+	memcpy(&xsave->i387.sw_reserved,
+		xstate_fx_sw_bytes, sizeof(xstate_fx_sw_bytes));
 	/*
 	 * Copy the xstate memory layout.
 	 */
-	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
-				  &target->thread.fpu.state->xsave, 0, -1);
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 	return ret;
 }
 
@@ -368,8 +367,8 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 		  unsigned int pos, unsigned int count,
 		  const void *kbuf, const void __user *ubuf)
 {
+	struct xsave_struct *xsave = &target->thread.fpu.state->xsave;
 	int ret;
-	struct xsave_hdr_struct *xsave_hdr;
 
 	if (!cpu_has_xsave)
 		return -ENODEV;
@@ -378,22 +377,16 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 	if (ret)
 		return ret;
 
-	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
-				 &target->thread.fpu.state->xsave, 0, -1);
-
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
-	target->thread.fpu.state->fxsave.mxcsr &= mxcsr_feature_mask;
-
-	xsave_hdr = &target->thread.fpu.state->xsave.xsave_hdr;
-
-	xsave_hdr->xstate_bv &= pcntxt_mask;
+	xsave->i387.mxcsr &= mxcsr_feature_mask;
+	xsave->xsave_hdr.xstate_bv &= pcntxt_mask;
 	/*
 	 * These bits must be zero.
 	 */
-	memset(xsave_hdr->reserved, 0, 48);
-
+	memset(&xsave->xsave_hdr.reserved, 0, 48);
 	return ret;
 }
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:x86/fpu] x86/fpu: Factor out memset(xstate, 0) in fpu_finit( ) paths
  2015-03-02 18:32         ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Oleg Nesterov
  2015-03-03 22:01           ` Rik van Riel
  2015-03-04 11:38           ` Borislav Petkov
@ 2015-03-10 10:08           ` tip-bot for Oleg Nesterov
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Oleg Nesterov @ 2015-03-10 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, linux-kernel, tglx, luto, bp, bp, taviso, oleg, hpa,
	mingo, riel

Commit-ID:  1d23c4518b1f3a03c278f23333149245c178d2a6
Gitweb:     http://git.kernel.org/tip/1d23c4518b1f3a03c278f23333149245c178d2a6
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Tue, 10 Mar 2015 07:06:25 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 Mar 2015 07:14:31 +0100

x86/fpu: Factor out memset(xstate, 0) in fpu_finit() paths

fx_finit() has two users but only fpu_finit() needs to clear
xstate, alloc_bootmem_align() in setup_init_fpu_buf() returns
zero-filled memory.

And note that both memset()'s look confusing. Yes, offsetof() is
0 for ->fxsave or ->fsave, but it would be cleaner to turn
them into a single memset() which zeroes fpu->state.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Rik van Riel <riel@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tavis Ormandy <taviso@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1425967585-4725-2-git-send-email-bp@alien8.de
Link: http://lkml.kernel.org/r/20150302183257.GC23085@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/fpu-internal.h | 1 -
 arch/x86/kernel/i387.c              | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index 61609b9..5fa1be2 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -135,7 +135,6 @@ static __always_inline __pure bool use_fxsr(void)
 
 static inline void fx_finit(struct i387_fxsave_struct *fx)
 {
-	memset(fx, 0, xstate_size);
 	fx->cwd = 0x37f;
 	fx->mxcsr = MXCSR_DEFAULT;
 }
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 03cc0ad..0f3de66 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -224,11 +224,12 @@ void fpu_finit(struct fpu *fpu)
 		return;
 	}
 
+	memset(fpu->state, 0, xstate_size);
+
 	if (cpu_has_fxsr) {
 		fx_finit(&fpu->state->fxsave);
 	} else {
 		struct i387_fsave_struct *fp = &fpu->state->fsave;
-		memset(fp, 0, xstate_size);
 		fp->cwd = 0xffff037fu;
 		fp->swd = 0xffff0000u;
 		fp->twd = 0xffffffffu;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-03-10 10:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 18:08 [GIT PULL] x86/fpu cleanups Borislav Petkov
2015-03-10  6:06 ` [PATCH 1/2] x86/fpu: Change xstateregs_get()/set() to use ->xsave.i387 rather than ->fxsave Borislav Petkov
2015-03-10  6:06   ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Borislav Petkov
     [not found] <CAJ_zFkJkHpU4Y8wrrZjO0VnhzUVqJfyrLmNfRsBRT29bgbDSHg@mail.gmail.com>
     [not found] ` <20150301184650.GA12758@redhat.com>
     [not found]   ` <20150301185943.GA14318@redhat.com>
     [not found]     ` <20150302174818.GA16886@redhat.com>
2015-03-02 18:32       ` [PATCH 0/2] x86/fpu: minor cleanups Oleg Nesterov
2015-03-02 18:32         ` [PATCH 1/2] x86/regsets: change xstateregs_get/set to use ->xsave.i387 rather than ->fxsave Oleg Nesterov
2015-03-03 19:23           ` Rik van Riel
2015-03-04 10:30           ` Borislav Petkov
2015-03-10 10:08           ` [tip:x86/fpu] x86/fpu: Change xstateregs_get()/set() to use -> xsave.i387 " tip-bot for Oleg Nesterov
2015-03-02 18:32         ` [PATCH 2/2] x86/fpu: factor out memset(xstate, 0) in fpu_finit() paths Oleg Nesterov
2015-03-03 22:01           ` Rik van Riel
2015-03-04 11:38           ` Borislav Petkov
2015-03-10 10:08           ` [tip:x86/fpu] x86/fpu: Factor out memset(xstate, 0) in fpu_finit( ) paths tip-bot for Oleg Nesterov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.