[parisc-linux] Need help to improve uaccess.h patch
Randolph Chung
Randolph Chung <randolph@tausq.org>
Fri, 4 Oct 2002 09:08:33 -0700
Note: i haven't tested the patch, just some first-pass comments:
> #else
> #define LDD_KERNEL(ptr) __get_kernel_asm("ldd",ptr)
> #define LDD_USER(ptr) __get_user_asm("ldd",ptr)
> @@ -68,27 +73,28 @@
> ({ \
> register long __gu_err __asm__ ("r8") = 0; \
> register long __gu_val __asm__ ("r9") = 0; \
> + \
> + unsigned int __tmp = 0; \
> \
> if (segment_eq(get_fs(),KERNEL_DS)) { \
> switch (sizeof(*(ptr))) { \
> - case 1: __get_kernel_asm("ldb",ptr); break; \
> - case 2: __get_kernel_asm("ldh",ptr); break; \
> - case 4: __get_kernel_asm("ldw",ptr); break; \
> - case 8: LDD_KERNEL(ptr); break; \
> + case 1: __get_kernel_asm("ldb",x, ptr); break; \
> + case 2: __get_kernel_asm("ldh",x, ptr); break; \
> + case 4: __get_kernel_asm("ldw",x, ptr); break; \
> + case 8: LDD_KERNEL(x, ptr); break; \
> default: BUG(); break; \
> } \
> } \
> else { \
> switch (sizeof(*(ptr))) { \
> - case 1: __get_user_asm("ldb",ptr); break; \
> - case 2: __get_user_asm("ldh",ptr); break; \
> - case 4: __get_user_asm("ldw",ptr); break; \
> - case 8: LDD_USER(ptr); break; \
> + case 1: __get_user_asm("ldb",x, ptr); break; \
> + case 2: __get_user_asm("ldh",x, ptr); break; \
> + case 4: __get_user_asm("ldw",x, ptr); break; \
> + case 8: LDD_USER(x, ptr); break; \
> default: BUG(); break; \
> } \
> } \
> \
> - (x) = (__typeof__(*(ptr))) __gu_val; \
> __gu_err; \
> })
why did you push the cast into the asm functions?
> -#define __get_user_asm(ldx,ptr) \
> +#define __get_kernel_asm_64(x, ptr) \
> + __asm__ ("\n\tldw\t4(%1),%3\n" \
> + "1:\tstw\t%3,4(%2)\n" \
> + "\tldw\t0(%1),%3\n" \
> + "2:\tstw\t%3,0(%2)\n" \
> + "3:\n" \
> + "\t.section __ex_table,\"a\"\n" \
> + "\t.word\t1b\n" \
> + "\t.word\t(3b-1b)+1\n" \
> + "\t.word\t2b\n" \
> + "\t.word\t(3b-2b)+1\n" \
what's with the +1?
> #define __put_user(x,ptr) \
> ({ \
> register long __pu_err __asm__ ("r8") = 0; \
> + unsigned long long X = (unsigned long long) x; \
why do you need this cast?
> + unsigned long long * __tmp = 0; \
if __tmp is only used in the STD macro, you should eiter pass it to the
macro or define it inside that macro.
> if (segment_eq(get_fs(),KERNEL_DS)) { \
> switch (sizeof(*(ptr))) { \
> case 1: __put_kernel_asm("stb",x,ptr); break; \
> case 2: __put_kernel_asm("sth",x,ptr); break; \
> case 4: __put_kernel_asm("stw",x,ptr); break; \
> - case 8: STD_KERNEL(x,ptr); break; \
> + case 8: STD_KERNEL(X,ptr); break; \
> default: BUG(); break; \
> } \
> } \
> @@ -152,7 +193,7 @@
> case 1: __put_user_asm("stb",x,ptr); break; \
> case 2: __put_user_asm("sth",x,ptr); break; \
> case 4: __put_user_asm("stw",x,ptr); break; \
> - case 8: STD_USER(x,ptr); break; \
> + case 8: STD_USER(X,ptr); break; \
> default: BUG(); break; \
> } \
> } \
> @@ -200,6 +241,22 @@
> : "=r"(__pu_err) \
> : "r"(ptr), "r"(x), "0"(__pu_err))
>
> +#define __put_kernel_asm_64(X, ptr) \
> + __asm__ __volatile__ ( \
> + "\n\tldw\t0(%2),%3\n" \
> + "1:\tstw\t%3,0(%1)\n" \
> + "\tldw\t4(%2),%3\n" \
> + "2:\tstw\t%3,4(%1)\n" \
> + "3:\n" \
> + "\t.section __ex_table,\"a\"\n" \
> + "\t.word\t1b\n" \
> + "\t.word\t(3b-1b)+1\n" \
> + "\t.word\t2b\n" \
> + "\t.word\t(3b-2b)+1\n" \
as above, why +1?
> + "\t.previous" \
> + : "=r"(__pu_err) \
> + : "r"(ptr), "r"(&(X)), "r"(__tmp), "0"(__pu_err))
> +
> #define __put_user_asm(stx,x,ptr) \
> __asm__ __volatile__ ( \
> "\n1:\t" stx "\t%2,0(%%sr3,%1)\n" \
> @@ -210,6 +267,23 @@
> "\t.previous" \
> : "=r"(__pu_err) \
> : "r"(ptr), "r"(x), "0"(__pu_err))
> +
> +#define __put_user_asm_64(X, ptr) \
> + __asm__ __volatile__ ( \
> + "\n\tldw\t0(%2),%3\n" \
> + "1:\tstw\t%3,0(%%sr3,%1)\n" \
> + "\tldw\t4(%2),%3\n" \
> + "2:\tstw\t%3,4(%%sr3,%1)\n" \
> + "3:\n" \
> + "\t.section __ex_table,\"a\"\n" \
> + "\t.word\t1b\n" \
> + "\t.word\t(3b-1b)+1\n" \
> + "\t.word\t2b\n" \
> + "\t.word\t(3b-2b)+1\n" \
ditto, this probably should be +3 to indicate a userspace address.
your code also creates an artifact that may or may not be ok -- if you
do a 64-bit put_user or get_user on a 32-bit aligned address, this will
work with a 32-bit kernel but will trap on a 64-bit kernel. i guess
that's ok, but just something to keep in mind.
randolph
--
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/