From: "H. Peter Anvin" <hpa@zytor.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Jamie Lokier <jamie@shareable.org>
Subject: Re: [PATCH] x86: Add support for 64bit get_user() on x86-32
Date: Wed, 12 Dec 2012 08:45:23 -0800 [thread overview]
Message-ID: <8badf086-abc1-4057-8886-c5d3a3fd696c@email.android.com> (raw)
In-Reply-To: <20121212163228.GC29018@intel.com>
It's a good change -- and one which opens up even more unification -- but right now we're worried about 3.8.
"Ville Syrjälä" <ville.syrjala@linux.intel.com> wrote:
>On Wed, Dec 12, 2012 at 08:15:12AM -0800, H. Peter Anvin wrote:
>> Can we worry about this after the merge window?
>
>Sure. The only use I have for this is in my drm/kms atomic
>modeset/pageflip patch set, and that's not 3.8 material.
>
>>
>> ville.syrjala@linux.intel.com wrote:
>>
>> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> >Implement __get_user_8() for x86-32. It will return the
>> >64bit result in edx:eax register pair, and ecx is used
>> >to pass in the address and return the error value.
>> >
>> >For consistency, change the register assignment for all
>> >other __get_user_x() variants, so that address is passed in
>> >ecx/rcx, the error value is returned in ecx/rcx, and eax/rax
>> >contains the actual value.
>> >
>> >This is a partial refresh of a patch [1] by Jamie Lokier from
>> >2004. Only the minimal changes to implement 64bit get_user()
>> >were picked from the original patch.
>> >
>> >[1] http://article.gmane.org/gmane.linux.kernel/198823
>> >
>> >Cc: Jamie Lokier <jamie@shareable.org>
>> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >---
>> > arch/x86/include/asm/uaccess.h | 17 ++++++--
>> > arch/x86/kernel/i386_ksyms_32.c | 1 +
>> >arch/x86/lib/getuser.S | 82
>> >++++++++++++++++++++++++++------------
>> > 3 files changed, 69 insertions(+), 31 deletions(-)
>> >
>> >diff --git a/arch/x86/include/asm/uaccess.h
>> >b/arch/x86/include/asm/uaccess.h
>> >index 7ccf8d1..3f4387e 100644
>> >--- a/arch/x86/include/asm/uaccess.h
>> >+++ b/arch/x86/include/asm/uaccess.h
>> >@@ -127,7 +127,7 @@ extern int __get_user_bad(void);
>> >
>> > #define __get_user_x(size, ret, x, ptr) \
>> > asm volatile("call __get_user_" #size \
>> >- : "=a" (ret), "=d" (x) \
>> >+ : "=c" (ret), "=a" (x) \
>> > : "0" (ptr)) \
>> >
>> > /* Careful: we have to cast the result to the type of the pointer
>> >@@ -151,8 +151,11 @@ extern int __get_user_bad(void);
>> > * On error, the variable @x is set to zero.
>> > */
>> > #ifdef CONFIG_X86_32
>> >-#define __get_user_8(__ret_gu, __val_gu, ptr) \
>> >- __get_user_x(X, __ret_gu, __val_gu, ptr)
>> >+#define __get_user_8(ret, x, ptr) \
>> >+ asm volatile("call __get_user_8" \
>> >+ : "=c" (ret), "=A" (x) \
>> >+ : "0" (ptr)) \
>> >+
>> > #else
>> > #define __get_user_8(__ret_gu, __val_gu, ptr) \
>> > __get_user_x(8, __ret_gu, __val_gu, ptr)
>> >@@ -162,6 +165,7 @@ extern int __get_user_bad(void);
>> > ({ \
>> > int __ret_gu; \
>> > unsigned long __val_gu; \
>> >+ unsigned long long __val_gu8; \
>> > __chk_user_ptr(ptr); \
>> > might_fault(); \
>> > switch (sizeof(*(ptr))) { \
>> >@@ -175,13 +179,16 @@ extern int __get_user_bad(void);
>> > __get_user_x(4, __ret_gu, __val_gu, ptr); \
>> > break; \
>> > case 8: \
>> >- __get_user_8(__ret_gu, __val_gu, ptr); \
>> >+ __get_user_8(__ret_gu, __val_gu8, ptr); \
>> > break; \
>> > default: \
>> > __get_user_x(X, __ret_gu, __val_gu, ptr); \
>> > break; \
>> > } \
>> >- (x) = (__typeof__(*(ptr)))__val_gu; \
>> >+ if (sizeof(*(ptr)) == 8) \
>> >+ (x) = (__typeof__(*(ptr)))__val_gu8; \
>> >+ else \
>> >+ (x) = (__typeof__(*(ptr)))__val_gu; \
>> > __ret_gu; \
>> > })
>> >
>> >diff --git a/arch/x86/kernel/i386_ksyms_32.c
>> >b/arch/x86/kernel/i386_ksyms_32.c
>> >index 9c3bd4a..0fa6912 100644
>> >--- a/arch/x86/kernel/i386_ksyms_32.c
>> >+++ b/arch/x86/kernel/i386_ksyms_32.c
>> >@@ -26,6 +26,7 @@ EXPORT_SYMBOL(csum_partial_copy_generic);
>> > EXPORT_SYMBOL(__get_user_1);
>> > EXPORT_SYMBOL(__get_user_2);
>> > EXPORT_SYMBOL(__get_user_4);
>> >+EXPORT_SYMBOL(__get_user_8);
>> >
>> > EXPORT_SYMBOL(__put_user_1);
>> > EXPORT_SYMBOL(__put_user_2);
>> >diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S
>> >index 156b9c8..38afef0 100644
>> >--- a/arch/x86/lib/getuser.S
>> >+++ b/arch/x86/lib/getuser.S
>> >@@ -14,12 +14,11 @@
>> > /*
>> > * __get_user_X
>> > *
>> >- * Inputs: %[r|e]ax contains the address.
>> >- * The register is modified, but all changes are undone
>> >- * before returning because the C code doesn't know about it.
>> >+ * Inputs: %[r|e]cx contains the address.
>> > *
>> >- * Outputs: %[r|e]ax is error code (0 or -EFAULT)
>> >- * %[r|e]dx contains zero-extended value
>> >+ * Outputs: %[r|e]cx is error code (0 or -EFAULT)
>> >+ * %[r|e]ax contains zero-extended value
>> >+ * %edx contains the high bits of the value for __get_user_8 on
>> >x86-32
>> > *
>> > *
>> > * These functions should not modify any other registers,
>> >@@ -38,12 +37,12 @@
>> > .text
>> > ENTRY(__get_user_1)
>> > CFI_STARTPROC
>> >- GET_THREAD_INFO(%_ASM_DX)
>> >- cmp TI_addr_limit(%_ASM_DX),%_ASM_AX
>> >+ GET_THREAD_INFO(%_ASM_AX)
>> >+ cmp TI_addr_limit(%_ASM_AX),%_ASM_CX
>> > jae bad_get_user
>> > ASM_STAC
>> >-1: movzb (%_ASM_AX),%edx
>> >- xor %eax,%eax
>> >+1: movzb (%_ASM_CX),%eax
>> >+ xor %ecx,%ecx
>> > ASM_CLAC
>> > ret
>> > CFI_ENDPROC
>> >@@ -51,14 +50,14 @@ ENDPROC(__get_user_1)
>> >
>> > ENTRY(__get_user_2)
>> > CFI_STARTPROC
>> >- add $1,%_ASM_AX
>> >+ add $1,%_ASM_CX
>> > jc bad_get_user
>> >- GET_THREAD_INFO(%_ASM_DX)
>> >- cmp TI_addr_limit(%_ASM_DX),%_ASM_AX
>> >+ GET_THREAD_INFO(%_ASM_AX)
>> >+ cmp TI_addr_limit(%_ASM_AX),%_ASM_CX
>> > jae bad_get_user
>> > ASM_STAC
>> >-2: movzwl -1(%_ASM_AX),%edx
>> >- xor %eax,%eax
>> >+2: movzwl -1(%_ASM_CX),%eax
>> >+ xor %ecx,%ecx
>> > ASM_CLAC
>> > ret
>> > CFI_ENDPROC
>> >@@ -66,14 +65,14 @@ ENDPROC(__get_user_2)
>> >
>> > ENTRY(__get_user_4)
>> > CFI_STARTPROC
>> >- add $3,%_ASM_AX
>> >+ add $3,%_ASM_CX
>> > jc bad_get_user
>> >- GET_THREAD_INFO(%_ASM_DX)
>> >- cmp TI_addr_limit(%_ASM_DX),%_ASM_AX
>> >+ GET_THREAD_INFO(%_ASM_AX)
>> >+ cmp TI_addr_limit(%_ASM_AX),%_ASM_CX
>> > jae bad_get_user
>> > ASM_STAC
>> >-3: mov -3(%_ASM_AX),%edx
>> >- xor %eax,%eax
>> >+3: mov -3(%_ASM_CX),%eax
>> >+ xor %ecx,%ecx
>> > ASM_CLAC
>> > ret
>> > CFI_ENDPROC
>> >@@ -82,14 +81,30 @@ ENDPROC(__get_user_4)
>> > #ifdef CONFIG_X86_64
>> > ENTRY(__get_user_8)
>> > CFI_STARTPROC
>> >- add $7,%_ASM_AX
>> >+ add $7,%_ASM_CX
>> > jc bad_get_user
>> >- GET_THREAD_INFO(%_ASM_DX)
>> >- cmp TI_addr_limit(%_ASM_DX),%_ASM_AX
>> >+ GET_THREAD_INFO(%_ASM_AX)
>> >+ cmp TI_addr_limit(%_ASM_AX),%_ASM_CX
>> > jae bad_get_user
>> > ASM_STAC
>> >-4: movq -7(%_ASM_AX),%_ASM_DX
>> >- xor %eax,%eax
>> >+4: movq -7(%_ASM_CX),%_ASM_AX
>> >+ xor %ecx,%ecx
>> >+ ASM_CLAC
>> >+ ret
>> >+ CFI_ENDPROC
>> >+ENDPROC(__get_user_8)
>> >+#else
>> >+ENTRY(__get_user_8)
>> >+ CFI_STARTPROC
>> >+ add $7,%_ASM_CX
>> >+ jc bad_get_user_8
>> >+ GET_THREAD_INFO(%_ASM_AX)
>> >+ cmp TI_addr_limit(%_ASM_AX),%_ASM_CX
>> >+ jae bad_get_user_8
>> >+ ASM_STAC
>> >+4: mov -7(%_ASM_CX),%eax
>> >+5: mov -3(%_ASM_CX),%edx
>> >+ xor %ecx,%ecx
>> > ASM_CLAC
>> > ret
>> > CFI_ENDPROC
>> >@@ -98,16 +113,31 @@ ENDPROC(__get_user_8)
>> >
>> > bad_get_user:
>> > CFI_STARTPROC
>> >- xor %edx,%edx
>> >- mov $(-EFAULT),%_ASM_AX
>> >+ xor %eax,%eax
>> >+ mov $(-EFAULT),%_ASM_CX
>> > ASM_CLAC
>> > ret
>> > CFI_ENDPROC
>> > END(bad_get_user)
>> >
>> >+#ifdef CONFIG_X86_32
>> >+bad_get_user_8:
>> >+ CFI_STARTPROC
>> >+ xor %eax,%eax
>> >+ xor %edx,%edx
>> >+ mov $(-EFAULT),%_ASM_CX
>> >+ ASM_CLAC
>> >+ ret
>> >+ CFI_ENDPROC
>> >+END(bad_get_user_8)
>> >+#endif
>> >+
>> > _ASM_EXTABLE(1b,bad_get_user)
>> > _ASM_EXTABLE(2b,bad_get_user)
>> > _ASM_EXTABLE(3b,bad_get_user)
>> > #ifdef CONFIG_X86_64
>> > _ASM_EXTABLE(4b,bad_get_user)
>> >+#else
>> >+ _ASM_EXTABLE(4b,bad_get_user_8)
>> >+ _ASM_EXTABLE(5b,bad_get_user_8)
>> > #endif
>>
>> --
>> Sent from my mobile phone. Please excuse brevity and lack of
>formatting.
--
Sent from my mobile phone. Please excuse brevity and lack of formatting.
next prev parent reply other threads:[~2012-12-12 16:45 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-12 11:34 ville.syrjala
2012-12-12 16:15 ` H. Peter Anvin
2012-12-12 16:32 ` Ville Syrjälä
2012-12-12 16:45 ` H. Peter Anvin [this message]
2013-02-07 16:53 ` Ville Syrjälä
2013-02-07 17:59 ` H. Peter Anvin
2013-02-08 16:24 ` Ville Syrjälä
2013-02-08 17:30 ` H. Peter Anvin
2013-02-08 18:23 ` Ville Syrjälä
2013-02-08 19:08 ` H. Peter Anvin
2013-02-09 10:41 ` Borislav Petkov
2013-02-09 11:00 ` Russell King - ARM Linux
2013-02-12 1:37 ` [tip:x86/mm] x86, mm: Use a bitfield to mask nuisance get_user() warnings tip-bot for H. Peter Anvin
2013-02-12 3:33 ` Linus Torvalds
2013-02-12 4:21 ` H. Peter Anvin
2013-02-12 4:42 ` Linus Torvalds
2013-02-12 4:47 ` Linus Torvalds
2013-02-12 4:51 ` H. Peter Anvin
2013-02-12 7:12 ` H. Peter Anvin
2013-02-12 8:10 ` Ingo Molnar
2013-02-12 16:38 ` H.J. Lu
2013-02-12 17:00 ` Linus Torvalds
2013-02-12 17:14 ` H. Peter Anvin
2013-02-12 17:30 ` H.J. Lu
2013-02-12 18:25 ` H. Peter Anvin
2013-02-12 18:29 ` H.J. Lu
2013-02-12 18:46 ` Linus Torvalds
2013-02-12 18:58 ` H. Peter Anvin
2013-02-12 20:55 ` [tip:x86/mm] x86, mm: Redesign get_user with a __builtin_choose_expr hack tip-bot for H. Peter Anvin
2013-02-12 23:06 ` Linus Torvalds
2013-02-12 23:19 ` H. Peter Anvin
2013-02-12 23:49 ` Linus Torvalds
2013-02-12 23:52 ` H. Peter Anvin
2013-02-13 0:01 ` [tip:x86/mm] x86, doc: Clarify the use of asm("%edx") in uaccess.h tip-bot for H. Peter Anvin
2013-02-12 23:21 ` [tip:x86/mm] x86, mm: Redesign get_user with a __builtin_choose_expr hack Russell King - ARM Linux
2013-02-12 17:32 ` [tip:x86/mm] x86, mm: Use a bitfield to mask nuisance get_user() warnings Linus Torvalds
2013-02-12 17:35 ` H. Peter Anvin
2013-02-12 17:49 ` Linus Torvalds
2013-02-12 17:57 ` Russell King - ARM Linux
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=8badf086-abc1-4057-8886-c5d3a3fd696c@email.android.com \
--to=hpa@zytor.com \
--cc=jamie@shareable.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=ville.syrjala@linux.intel.com \
--cc=x86@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®