All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Rik van Riel <riel@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>
Subject: [PATCH 05/14] x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs
Date: Thu, 26 Jan 2017 11:22:50 +0100	[thread overview]
Message-ID: <1485426179-13681-6-git-send-email-mingo@kernel.org> (raw)
In-Reply-To: <1485426179-13681-1-git-send-email-mingo@kernel.org>

Parameter ordering is weird:

  int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
  int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);

'pos' and 'count', which are attributes of the destination buffer, are listed before the destination
buffer itself ...

List them after the primary arguments instead.

This makes the code more similar to regular memcpy() variant APIs.

No change in functionality.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/fpu/xstate.h |  4 ++--
 arch/x86/kernel/fpu/regset.c      |  4 ++--
 arch/x86/kernel/fpu/xstate.c      | 25 ++++++++++++-------------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 65bd68c30cd0..e4430b84939d 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
 void *get_xsave_addr(struct xregs_state *xsave, int xstate);
 const void *get_xsave_field_ptr(int xstate_field);
 int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int pos, unsigned int count);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int pos, unsigned int count);
 int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
 		     struct xregs_state *xsave);
 #endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 62a2d1154e45..72cf367627e2 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -92,9 +92,9 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 
 	if (using_compacted_format()) {
 		if (kbuf)
-			ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
+			ret = copy_xstate_to_kernel(kbuf, xsave, pos, count);
 		else
-			ret = copy_xstate_to_user(pos, count, ubuf, xsave);
+			ret = copy_xstate_to_user(ubuf, xsave, pos, count);
 	} else {
 		fpstate_sanitize_xstate(fpu);
 		/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 1ac3d2ae73e6..d2f4912c15e7 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -924,10 +924,9 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
  * the source data pointer or increment pos, count, kbuf, and ubuf.
  */
 static inline int
-__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
-			void *kbuf,
-			const void *data, const int start_pos,
-			const int end_pos)
+__copy_xstate_to_kernel(void *kbuf,
+			const void *data,
+			unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
 {
 	if ((count == 0) || (pos < start_pos))
 		return 0;
@@ -947,7 +946,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int count,
  * It supports partial copy but pos always starts from zero. This is called
  * from xstateregs_get() and there we check the CPU has XSAVES.
  */
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, struct xregs_state *xsave)
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int pos, unsigned int count)
 {
 	unsigned int offset, size;
 	int ret, i;
@@ -972,7 +971,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
 	offset = offsetof(struct xregs_state, header);
 	size = sizeof(header);
 
-	ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
+	ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, 0, count);
 	if (ret)
 		return ret;
 
@@ -986,7 +985,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
 			offset = xstate_offsets[i];
 			size = xstate_sizes[i];
 
-			ret = __copy_xstate_to_kernel(offset, size, kbuf, src, 0, count);
+			ret = __copy_xstate_to_kernel(kbuf, src, offset, size, 0, count);
 			if (ret)
 				return ret;
 
@@ -1002,7 +1001,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
 	offset = offsetof(struct fxregs_state, sw_reserved);
 	size = sizeof(xstate_fx_sw_bytes);
 
-	ret = __copy_xstate_to_kernel(offset, size, kbuf, xstate_fx_sw_bytes, 0, count);
+	ret = __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, 0, count);
 	if (ret)
 		return ret;
 
@@ -1010,7 +1009,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
 }
 
 static inline int
-__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, const void *data, const int start_pos, const int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos, unsigned int count, const int start_pos, const int end_pos)
 {
 	if ((count == 0) || (pos < start_pos))
 		return 0;
@@ -1030,7 +1029,7 @@ __copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, c
  * zero. This is called from xstateregs_get() and there we check the CPU
  * has XSAVES.
  */
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int pos, unsigned int count)
 {
 	unsigned int offset, size;
 	int ret, i;
@@ -1055,7 +1054,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
 	offset = offsetof(struct xregs_state, header);
 	size = sizeof(header);
 
-	ret = __copy_xstate_to_user(offset, size, ubuf, &header, 0, count);
+	ret = __copy_xstate_to_user(ubuf, &header, offset, size, 0, count);
 	if (ret)
 		return ret;
 
@@ -1069,7 +1068,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
 			offset = xstate_offsets[i];
 			size = xstate_sizes[i];
 
-			ret = __copy_xstate_to_user(offset, size, ubuf, src, 0, count);
+			ret = __copy_xstate_to_user(ubuf, src, offset, size, 0, count);
 			if (ret)
 				return ret;
 
@@ -1085,7 +1084,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
 	offset = offsetof(struct fxregs_state, sw_reserved);
 	size = sizeof(xstate_fx_sw_bytes);
 
-	ret = __copy_xstate_to_user(offset, size, ubuf, xstate_fx_sw_bytes, 0, count);
+	ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size, 0, count);
 	if (ret)
 		return ret;
 
-- 
2.7.4

  parent reply	other threads:[~2017-01-26 10:24 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-26 10:22 [PATCH 00/14] x86/fpu: Clean up ptrace copying functions Ingo Molnar
2017-01-26 10:22 ` [PATCH 01/14] x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user() Ingo Molnar
2017-01-26 10:22 ` [PATCH 02/14] x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user() Ingo Molnar
2017-01-26 10:22 ` [PATCH 03/14] x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs Ingo Molnar
2017-01-26 10:22 ` [PATCH 04/14] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs Ingo Molnar
2017-01-27 10:16   ` Borislav Petkov
2017-01-30  9:57     ` Ingo Molnar
2017-01-30 15:45       ` Borislav Petkov
2017-01-30 17:23         ` Yu-cheng Yu
2017-01-26 10:22 ` Ingo Molnar [this message]
2017-01-26 10:22 ` [PATCH 06/14] x86/fpu: Clean up the parameter definitions of copy_xstate_to_*() Ingo Molnar
2017-01-26 10:22 ` [PATCH 07/14] x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions Ingo Molnar
2017-01-26 10:22 ` [PATCH 08/14] x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods Ingo Molnar
2017-01-26 10:22 ` [PATCH 09/14] x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*() Ingo Molnar
2017-01-30 17:11   ` Yu-cheng Yu
2017-01-26 10:22 ` [PATCH 10/14] x86/fpu: Simplify __copy_xstate_to_kernel() return values Ingo Molnar
2017-01-26 10:22 ` [PATCH 11/14] x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate() Ingo Molnar
2017-01-27 10:54   ` Borislav Petkov
2017-01-26 10:22 ` [PATCH 12/14] x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API Ingo Molnar
2017-01-26 10:22 ` [PATCH 13/14] x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API Ingo Molnar
2017-01-26 10:22 ` [PATCH 14/14] x86/fpu: Flip the parameter order in copy_*_to_xstate() Ingo Molnar
2017-01-26 10:28 ` [PATCH 00/14] x86/fpu: Clean up ptrace copying functions Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1485426179-13681-6-git-send-email-mingo@kernel.org \
    --to=mingo@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yu-cheng.yu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.