Re: [PATCH] x86: Add support for 64bit get_user() on x86-32

From: Ville Syrjälä
Date: Wed Dec 12 2012 - 11:32:39 EST

Next message: Jacob Shin: "[PATCH 0/5] perf: Add support for hardware breakpoint address masks"
Previous message: Jacob Shin: "[PATCH 5/5] perf tools: Add breakpoint address mask test case to tests/parse-events"
In reply to: H. Peter Anvin: "Re: [PATCH] x86: Add support for 64bit get_user() on x86-32"
Next in thread: H. Peter Anvin: "Re: [PATCH] x86: Add support for 64bit get_user() on x86-32"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxxxxxxx wrote:
>
> >From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> >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@xxxxxxxxxxxxx>
> >Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >---
> > 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.

--
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Jacob Shin: "[PATCH 0/5] perf: Add support for hardware breakpoint address masks"
Previous message: Jacob Shin: "[PATCH 5/5] perf tools: Add breakpoint address mask test case to tests/parse-events"
In reply to: H. Peter Anvin: "Re: [PATCH] x86: Add support for 64bit get_user() on x86-32"
Next in thread: H. Peter Anvin: "Re: [PATCH] x86: Add support for 64bit get_user() on x86-32"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]