* [parisc-linux] Need help to improve uaccess.h patch
@ 2002-10-04 15:24 Joel.soete
2002-10-04 16:08 ` Randolph Chung
0 siblings, 1 reply; 6+ messages in thread
From: Joel.soete @ 2002-10-04 15:24 UTC (permalink / raw)
To: parisc-linux
[-- Attachment #1: Type: text/plain, Size: 8837 bytes --]
Hi all,
With evms-1.2.0, I test following draft of uaccess.h.
Can somebody have a close look on this code to make it relevant for all
(I not sure of exception condition (ie segv) because I do not find any way
to stress it and do not find usage of get_user_asm_64)?
Thanks in advance for attention and futher help,
Joel
########
--- uaccess.h.orig 2002-10-04 10:23:59.000000000 +0200
+++ uaccess.h 2002-10-04 17:07:54.000000000 +0200
@@ -35,10 +35,15 @@
#define get_user __get_user
#if BITS_PER_LONG == 32
-#define LDD_KERNEL(ptr) BUG()
-#define LDD_USER(ptr) BUG()
-#define STD_KERNEL(x, ptr) BUG()
-#define STD_USER(x, ptr) BUG()
+
+#define LDD_KERNEL(x, ptr) __get_kernel_asm_64(x, ptr)
+
+#define LDD_USER(x, ptr) __get_user_asm_64(x, ptr)
+
+#define STD_KERNEL(x, ptr) __put_kernel_asm_64(x, ptr)
+
+#define STD_USER(x, ptr) __put_user_asm_64(x, ptr)
+
#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; \
})
@@ -113,7 +119,7 @@
: "=r"(__gu_val), "=r"(__gu_err) \
: "r"(ptr), "1"(__gu_err));
#else
-#define __get_kernel_asm(ldx,ptr) \
+#define __get_kernel_asm(ldx,x, ptr) \
__asm__("\n1:\t" ldx "\t0(%2),%0\n" \
"2:\n" \
"\t.section __ex_table,\"a\"\n" \
@@ -121,9 +127,25 @@
"\t.word\t(2b-1b)+3\n" \
"\t.previous" \
: "=r"(__gu_val), "=r"(__gu_err) \
- : "r"(ptr), "1"(__gu_err));
+ : "r"(ptr), "1"(__gu_err)); \
+ (x) = (__typeof__(*(ptr))) __gu_val;
-#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" \
+ "\t.previous" \
+ : "=r"(__gu_err) \
+ : "r"(ptr), "r"(&(x)), "r"(__tmp), "0"(__gu_err));
+
+#define __get_user_asm(ldx,x, ptr) \
__asm__("\n1:\t" ldx "\t0(%%sr3,%2),%0\n" \
"2:\n" \
"\t.section __ex_table,\"a\"\n" \
@@ -131,19 +153,38 @@
"\t.word\t(2b-1b)+3\n" \
"\t.previous" \
: "=r"(__gu_val), "=r"(__gu_err) \
- : "r"(ptr), "1"(__gu_err));
+ : "r"(ptr), "1"(__gu_err)); \
+ (x) = (__typeof__(*(ptr))) __gu_val;
+
+#define __get_user_asm_64(x, ptr) \
+ __asm__ ("\n\tldw\t4(%%sr3,%1),%3\n" \
+ "1:\tstw\t%3,4(%2)\n" \
+ "\tldw\t0(%%sr3,%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" \
+ "\t.previous" \
+ : "=r"(__gu_err) \
+ : "r"(ptr), "r"(&(x)), "r"(__tmp), "0"(__gu_err));
+
#endif
#define __put_user(x,ptr) \
({ \
register long __pu_err __asm__ ("r8") = 0; \
+ unsigned long long X = (unsigned long long) x; \
+ unsigned long long * __tmp = 0; \
\
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" \
+ "\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" \
+ "\t.previous" \
+ : "=r"(__pu_err) \
+ : "r"(ptr), "r"(&(X)), "r"(__tmp), "0"(__pu_err))
+
#endif
#########
[-- Attachment #2: pa-uaccess_h.patch --]
[-- Type: application/octet-stream, Size: 8493 bytes --]
--- uaccess.h.orig 2002-10-04 10:23:59.000000000 +0200
+++ uaccess.h 2002-10-04 17:07:54.000000000 +0200
@@ -35,10 +35,15 @@
#define get_user __get_user
#if BITS_PER_LONG == 32
-#define LDD_KERNEL(ptr) BUG()
-#define LDD_USER(ptr) BUG()
-#define STD_KERNEL(x, ptr) BUG()
-#define STD_USER(x, ptr) BUG()
+
+#define LDD_KERNEL(x, ptr) __get_kernel_asm_64(x, ptr)
+
+#define LDD_USER(x, ptr) __get_user_asm_64(x, ptr)
+
+#define STD_KERNEL(x, ptr) __put_kernel_asm_64(x, ptr)
+
+#define STD_USER(x, ptr) __put_user_asm_64(x, ptr)
+
#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; \
})
@@ -113,7 +119,7 @@
: "=r"(__gu_val), "=r"(__gu_err) \
: "r"(ptr), "1"(__gu_err));
#else
-#define __get_kernel_asm(ldx,ptr) \
+#define __get_kernel_asm(ldx,x, ptr) \
__asm__("\n1:\t" ldx "\t0(%2),%0\n" \
"2:\n" \
"\t.section __ex_table,\"a\"\n" \
@@ -121,9 +127,25 @@
"\t.word\t(2b-1b)+3\n" \
"\t.previous" \
: "=r"(__gu_val), "=r"(__gu_err) \
- : "r"(ptr), "1"(__gu_err));
+ : "r"(ptr), "1"(__gu_err)); \
+ (x) = (__typeof__(*(ptr))) __gu_val;
-#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" \
+ "\t.previous" \
+ : "=r"(__gu_err) \
+ : "r"(ptr), "r"(&(x)), "r"(__tmp), "0"(__gu_err));
+
+#define __get_user_asm(ldx,x, ptr) \
__asm__("\n1:\t" ldx "\t0(%%sr3,%2),%0\n" \
"2:\n" \
"\t.section __ex_table,\"a\"\n" \
@@ -131,19 +153,38 @@
"\t.word\t(2b-1b)+3\n" \
"\t.previous" \
: "=r"(__gu_val), "=r"(__gu_err) \
- : "r"(ptr), "1"(__gu_err));
+ : "r"(ptr), "1"(__gu_err)); \
+ (x) = (__typeof__(*(ptr))) __gu_val;
+
+#define __get_user_asm_64(x, ptr) \
+ __asm__ ("\n\tldw\t4(%%sr3,%1),%3\n" \
+ "1:\tstw\t%3,4(%2)\n" \
+ "\tldw\t0(%%sr3,%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" \
+ "\t.previous" \
+ : "=r"(__gu_err) \
+ : "r"(ptr), "r"(&(x)), "r"(__tmp), "0"(__gu_err));
+
#endif
#define __put_user(x,ptr) \
({ \
register long __pu_err __asm__ ("r8") = 0; \
+ unsigned long long X = (unsigned long long) x; \
+ unsigned long long * __tmp = 0; \
\
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" \
+ "\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" \
+ "\t.previous" \
+ : "=r"(__pu_err) \
+ : "r"(ptr), "r"(&(X)), "r"(__tmp), "0"(__pu_err))
+
#endif
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [parisc-linux] Need help to improve uaccess.h patch
2002-10-04 15:24 [parisc-linux] Need help to improve uaccess.h patch Joel.soete
@ 2002-10-04 16:08 ` Randolph Chung
2002-10-05 20:22 ` Joel Soete
0 siblings, 1 reply; 6+ messages in thread
From: Randolph Chung @ 2002-10-04 16:08 UTC (permalink / raw)
To: Joel.soete; +Cc: parisc-linux
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/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [parisc-linux] Need help to improve uaccess.h patch
2002-10-04 16:08 ` Randolph Chung
@ 2002-10-05 20:22 ` Joel Soete
2002-10-05 22:10 ` John David Anglin
0 siblings, 1 reply; 6+ messages in thread
From: Joel Soete @ 2002-10-05 20:22 UTC (permalink / raw)
To: Randolph Chung; +Cc: parisc-linux
Randolph Chung wrote:
> 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?
>
Not actualy in the asm but well in the std #define __get_user_asm just
after asm, because with gcc-3.0.4 I do not find any way to access (in
__asm extension) the two integer part of a long long (which C allow). So
I try (as better as I can) to use ptr and this cast does not seems
anymore usefull in 64bit parts.
[Well, in mips 32 bits uaccess.h, I see that some prefix as D, M, L can
be used to do this access. But if I test those one (example %D1) with
hppa, gcc asks me send a bug report. I think that I would have to do it
to improve gcc-3.3?]
>
>>-#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?
That is my main doubt :(
In this uaccess.h, I read (but i am not quit sure to have understand all
fine aspect) that we have to 'jump' after the erronious code (for me
3b-[12]b + 1 ? am i wrong? ). And understand +3 in get_user_asm because
we would have to jump after the cast "(x) = (__typeof__(*(ptr)))
__gu_val;". Is it wrong?
>
>
>> #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?
I need in fact to assigned x to X because I encounter put_user(0,PTR)
(which became later .. (&0)... which is wrong). And the cast is just to
avoid anoying warning if x is not a long long.
>
>
>>+ 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?
As early mentionned I do not well understand the original +1 (or +3) in
the original put_user (get_user) and use what I think to have understand
(which seems to be wrong :( )
>
>
>>+ "\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.
Ok (I do not have the oppotunity to test this patch in wrong conditions
ie segv :(, I would so much), I have to analyse in more details
exception table handler to try to better understand this.
>
> 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.
IMHO the absolute solution would be, to have in 32bits a pseudo ldd asm
extension which do the actual 64bits processor ldd job. Don't you think so?
[as %D1, I also test ldd with a gcc 32bits but it also ask me to send a
bug report. What do you thing? would I have to report this one (I doubt
: it is not an actual pa1.1 code but more a dream of mine)?]
Thanks a lot for your kind attention and relevant remarks,
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [parisc-linux] Need help to improve uaccess.h patch
2002-10-05 20:22 ` Joel Soete
@ 2002-10-05 22:10 ` John David Anglin
2002-10-06 0:07 ` Randolph Chung
0 siblings, 1 reply; 6+ messages in thread
From: John David Anglin @ 2002-10-05 22:10 UTC (permalink / raw)
To: Joel Soete; +Cc: randolph, parisc-linux
> >>+ "\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?
>
> That is my main doubt :(
> In this uaccess.h, I read (but i am not quit sure to have understand all
> fine aspect) that we have to 'jump' after the erronious code (for me
> 3b-[12]b + 1 ? am i wrong? ). And understand +3 in get_user_asm because
> we would have to jump after the cast "(x) = (__typeof__(*(ptr)))
> __gu_val;". Is it wrong?
I may be wrong but I think the code is trying to build a PLABEL. In
which case, the value should be 2 or 3. The least significant bit is
not used. See the runtime architecture manual for more info on PLABELs.
Dave
--
J. David Anglin dave.anglin@nrc.ca
National Research Council of Canada (613) 990-0752 (FAX: 952-6605)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [parisc-linux] Need help to improve uaccess.h patch
2002-10-05 22:10 ` John David Anglin
@ 2002-10-06 0:07 ` Randolph Chung
0 siblings, 0 replies; 6+ messages in thread
From: Randolph Chung @ 2002-10-06 0:07 UTC (permalink / raw)
To: John David Anglin; +Cc: Joel Soete, parisc-linux
> > That is my main doubt :(
> > In this uaccess.h, I read (but i am not quit sure to have understand all
> > fine aspect) that we have to 'jump' after the erronious code (for me
> > 3b-[12]b + 1 ? am i wrong? ). And understand +3 in get_user_asm because
> > we would have to jump after the cast "(x) = (__typeof__(*(ptr)))
> > __gu_val;". Is it wrong?
>
> I may be wrong but I think the code is trying to build a PLABEL. In
> which case, the value should be 2 or 3. The least significant bit is
> not used. See the runtime architecture manual for more info on PLABELs.
nah, the comment says:
/*
* The exception table contains two values: the first is an address
* for an instruction that is allowed to fault, and the second is
* the number of bytes to skip if a fault occurs. We also support in
* two bit flags: 0x2 tells the exception handler to clear register
* r9 and 0x1 tells the exception handler to put -EFAULT in r8.
* This allows us to handle the simple cases for put_user and
* get_user without having to have .fixup sections.
*/
struct exception_table_entry {
unsigned long addr; /* address of insn that is allowed to fault. */
long skip; /* pcoq skip | r9 clear flag | r8 -EFAULT flag */
};
so let's take __get_user() ...
#define __get_user(x,ptr) \
({ \
register long __gu_err __asm__ ("r8") = 0; \
register long __gu_val __asm__ ("r9") = 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; \
default: BUG(); break; \
} \
} \
(x) = (__typeof__(*(ptr))) __gu_val; \
__gu_err; \
})
iow, at the end of __get_user, x == r9, and the return value is r8
so, if the extable says:
"\t.section __ex_table,\"a\"\n" \
"\t.word\t1b\n" \
"\t.word\t(2b-1b)+3\n" \
"\t.previous" \
this means that:
if the insn at label 1 faults, handle the fault (see
arch/parisc/mm/fault.c) and then continue at
label1+((label2-label1)&~3) == label2; also, since the lowest 2 bits are
set (+3), set r9 = 0 and r8 = -EFAULT --> get_user will set x = 0 and
return -EFAULT
randolph
--
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [parisc-linux] Need help to improve uaccess.h patch
@ 2002-10-07 16:12 jsoe0708
0 siblings, 0 replies; 6+ messages in thread
From: jsoe0708 @ 2002-10-07 16:12 UTC (permalink / raw)
To: Randolph Chung; +Cc: parisc-linux, John David Anglin
Randolph Chung wrote:
>>>That is my main doubt :(
>>>In this uaccess.h, I read (but i am not quit sure to have understand all
>>> fine aspect) that we have to 'jump' after the erronious code (for me
>>>3b-[12]b + 1 ? am i wrong? ). And understand +3 in get_user_asm because
>>>we would have to jump after the cast "(x) = (__typeof__(*(ptr)))
>>>__gu_val;". Is it wrong?
>>
>>I may be wrong but I think the code is trying to build a PLABEL. In
>>which case, the value should be 2 or 3. The least significant bit is
>>not used. See the runtime architecture manual for more info on PLABELs.
>
>
> nah, the comment says:
>
> /*
> * The exception table contains two values: the first is an address
> * for an instruction that is allowed to fault, and the second is
> * the number of bytes to skip if a fault occurs. We also support in
> * two bit flags: 0x2 tells the exception handler to clear register
> * r9 and 0x1 tells the exception handler to put -EFAULT in r8.
> * This allows us to handle the simple cases for put_user and
> * get_user without having to have .fixup sections.
> */
But may I any way use a .fixup section (I do not find any usage of a .fixup
section in parisc code). Even lusercopy.S do not use it (just a kind of)?
>
> struct exception_table_entry {
> unsigned long addr; /* address of insn that is allowed to fault.
*/
> long skip; /* pcoq skip | r9 clear flag | r8 -EFAULT
flag */
> };
>
> so let's take __get_user() ...
>
> #define __get_user(x,ptr) \
> ({ \
> register long __gu_err __asm__ ("r8") = 0; \
> register long __gu_val __asm__ ("r9") = 0; \
> [...]
Well in the case of a 64bits int, I could not use only one "r9" register.
May I used a third register: "r10" for instance?
> } \
> (x) = (__typeof__(*(ptr))) __gu_val; \
> __gu_err; \
> })
>
> iow, at the end of __get_user, x == r9, and the return value is r8
>
> so, if the extable says:
> "\t.section __ex_table,\"a\"\n" \
> "\t.word\t1b\n" \
> "\t.word\t(2b-1b)+3\n" \
> "\t.previous" \
>
> this means that:
> if the insn at label 1 faults, handle the fault (see
> arch/parisc/mm/fault.c) and then continue at
> label1+((label2-label1)&~3) == label2; also, since the lowest 2 bits are
> set (+3), set r9 = 0 and r8 = -EFAULT --> get_user will set x = 0 and
> return -EFAULT
>
Thanks a lot for all of your understand and help,
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-10-07 16:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-10-04 15:24 [parisc-linux] Need help to improve uaccess.h patch Joel.soete
2002-10-04 16:08 ` Randolph Chung
2002-10-05 20:22 ` Joel Soete
2002-10-05 22:10 ` John David Anglin
2002-10-06 0:07 ` Randolph Chung
-- strict thread matches above, loose matches on Subject: below --
2002-10-07 16:12 jsoe0708
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.