This patchset do some refactorings related to riscv's uaccess and extable,
mainly for the usage of __get/put_user_nocheck() which not distinguish user
access and kernel access.
v1 -> v2:
According to Conor's suggestion, split into two logically independent
patches.
Tong Tiangen (2):
riscv: uaccess: rename __get/put_user_nocheck to __get/put_mem_nocheck
riscv: extable: add new extable type EX_TYPE_KACCESS_ERR_ZERO support
arch/riscv/include/asm/asm-extable.h | 12 ++
arch/riscv/include/asm/uaccess.h | 162 +++++++++++++--------------
arch/riscv/mm/extable.c | 1 +
3 files changed, 94 insertions(+), 81 deletions(-)
--
2.25.1
Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
__get/put_kernel_nofault(), but those helpers are not uaccess type, so we
add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
__get/put_kernel_no_fault().
Only refactor code without any functional changes.
Signed-off-by: Tong Tiangen <[email protected]>
---
arch/riscv/include/asm/asm-extable.h | 12 ++
arch/riscv/include/asm/uaccess.h | 160 +++++++++++++--------------
arch/riscv/mm/extable.c | 1 +
3 files changed, 93 insertions(+), 80 deletions(-)
diff --git a/arch/riscv/include/asm/asm-extable.h b/arch/riscv/include/asm/asm-extable.h
index 14be0673f5b5..73c70098a9c8 100644
--- a/arch/riscv/include/asm/asm-extable.h
+++ b/arch/riscv/include/asm/asm-extable.h
@@ -6,6 +6,7 @@
#define EX_TYPE_FIXUP 1
#define EX_TYPE_BPF 2
#define EX_TYPE_UACCESS_ERR_ZERO 3
+#define EX_TYPE_KACCESS_ERR_ZERO 4
#ifdef __ASSEMBLY__
@@ -57,9 +58,20 @@
EX_DATA_REG(ZERO, zero) \
")")
+#define _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, zero) \
+ __DEFINE_ASM_GPR_NUMS \
+ __ASM_EXTABLE_RAW(#insn, #fixup, \
+ __stringify(EX_TYPE_KACCESS_ERR_ZERO), \
+ "(" \
+ EX_DATA_REG(ERR, err) " | " \
+ EX_DATA_REG(ZERO, zero) \
+ ")")
+
#define _ASM_EXTABLE_UACCESS_ERR(insn, fixup, err) \
_ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, zero)
+#define _ASM_EXTABLE_KACCESS_ERR(insn, fixup, err) \
+ _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, zero)
#endif /* __ASSEMBLY__ */
#endif /* __ASM_ASM_EXTABLE_H */
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 1370da055b44..a79af147636e 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -50,62 +50,62 @@
* call.
*/
-#define __get_mem_asm(insn, x, ptr, err) \
-do { \
- __typeof__(x) __x; \
- __asm__ __volatile__ ( \
- "1:\n" \
- " " insn " %1, %2\n" \
- "2:\n" \
- _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %0, %1) \
- : "+r" (err), "=&r" (__x) \
- : "m" (*(ptr))); \
- (x) = __x; \
+#define __get_mem_asm(insn, x, ptr, err, type) \
+do { \
+ __typeof__(x) __x; \
+ __asm__ __volatile__ ( \
+ "1:\n" \
+ " " insn " %1, %2\n" \
+ "2:\n" \
+ _ASM_EXTABLE_##type##ACCESS_ERR_ZERO(1b, 2b, %0, %1) \
+ : "+r" (err), "=&r" (__x) \
+ : "m" (*(ptr))); \
+ (x) = __x; \
} while (0)
#ifdef CONFIG_64BIT
-#define __get_mem_8(x, ptr, err) \
- __get_mem_asm("ld", x, ptr, err)
+#define __get_mem_8(x, ptr, err, type) \
+ __get_mem_asm("ld", x, ptr, err, type)
#else /* !CONFIG_64BIT */
-#define __get_mem_8(x, ptr, err) \
-do { \
- u32 *__ptr = (u32 *)(ptr); \
- u32 __lo, __hi; \
- __asm__ __volatile__ ( \
- "1:\n" \
- " lw %1, %3\n" \
- "2:\n" \
- " lw %2, %4\n" \
- "3:\n" \
- _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 3b, %0, %1) \
- _ASM_EXTABLE_UACCESS_ERR_ZERO(2b, 3b, %0, %1) \
- : "+r" (err), "=&r" (__lo), "=r" (__hi) \
- : "m" (__ptr[__LSW]), "m" (__ptr[__MSW])); \
- if (err) \
- __hi = 0; \
- (x) = (__typeof__(x))((__typeof__((x)-(x)))( \
- (((u64)__hi << 32) | __lo))); \
+#define __get_mem_8(x, ptr, err, type) \
+do { \
+ u32 *__ptr = (u32 *)(ptr); \
+ u32 __lo, __hi; \
+ __asm__ __volatile__ ( \
+ "1:\n" \
+ " lw %1, %3\n" \
+ "2:\n" \
+ " lw %2, %4\n" \
+ "3:\n" \
+ _ASM_EXTABLE_##type##ACCESS_ERR_ZERO(1b, 3b, %0, %1) \
+ _ASM_EXTABLE_##type##ACCESS_ERR_ZERO(2b, 3b, %0, %1) \
+ : "+r" (err), "=&r" (__lo), "=r" (__hi) \
+ : "m" (__ptr[__LSW]), "m" (__ptr[__MSW])); \
+ if (err) \
+ __hi = 0; \
+ (x) = (__typeof__(x))((__typeof__((x)-(x)))( \
+ (((u64)__hi << 32) | __lo))); \
} while (0)
#endif /* CONFIG_64BIT */
-#define __get_mem_nocheck(x, __gu_ptr, __gu_err) \
-do { \
- switch (sizeof(*__gu_ptr)) { \
- case 1: \
- __get_mem_asm("lb", (x), __gu_ptr, __gu_err); \
- break; \
- case 2: \
- __get_mem_asm("lh", (x), __gu_ptr, __gu_err); \
- break; \
- case 4: \
- __get_mem_asm("lw", (x), __gu_ptr, __gu_err); \
- break; \
- case 8: \
- __get_mem_8((x), __gu_ptr, __gu_err); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
+#define __get_mem_nocheck(x, __gu_ptr, __gu_err, type) \
+do { \
+ switch (sizeof(*__gu_ptr)) { \
+ case 1: \
+ __get_mem_asm("lb", (x), __gu_ptr, __gu_err, type); \
+ break; \
+ case 2: \
+ __get_mem_asm("lh", (x), __gu_ptr, __gu_err, type); \
+ break; \
+ case 4: \
+ __get_mem_asm("lw", (x), __gu_ptr, __gu_err, type); \
+ break; \
+ case 8: \
+ __get_mem_8((x), __gu_ptr, __gu_err, type); \
+ break; \
+ default: \
+ BUILD_BUG(); \
+ } \
} while (0)
/**
@@ -136,7 +136,7 @@ do { \
__chk_user_ptr(__gu_ptr); \
\
__enable_user_access(); \
- __get_mem_nocheck(x, __gu_ptr, __gu_err); \
+ __get_mem_nocheck(x, __gu_ptr, __gu_err, U); \
__disable_user_access(); \
\
__gu_err; \
@@ -163,28 +163,28 @@ do { \
({ \
const __typeof__(*(ptr)) __user *__p = (ptr); \
might_fault(); \
- access_ok(__p, sizeof(*__p)) ? \
+ access_ok(__p, sizeof(*__p)) ? \
__get_user((x), __p) : \
((x) = 0, -EFAULT); \
})
-#define __put_mem_asm(insn, x, ptr, err) \
+#define __put_mem_asm(insn, x, ptr, err, type) \
do { \
__typeof__(*(ptr)) __x = x; \
__asm__ __volatile__ ( \
"1:\n" \
" " insn " %z2, %1\n" \
"2:\n" \
- _ASM_EXTABLE_UACCESS_ERR(1b, 2b, %0) \
+ _ASM_EXTABLE_##type##ACCESS_ERR(1b, 2b, %0) \
: "+r" (err), "=m" (*(ptr)) \
: "rJ" (__x)); \
} while (0)
#ifdef CONFIG_64BIT
-#define __put_mem_8(x, ptr, err) \
- __put_mem_asm("sd", x, ptr, err)
+#define __put_mem_8(x, ptr, err, type) \
+ __put_mem_asm("sd", x, ptr, err, type)
#else /* !CONFIG_64BIT */
-#define __put_mem_8(x, ptr, err) \
+#define __put_mem_8(x, ptr, err, type) \
do { \
u32 *__ptr = (u32 *)(ptr); \
u64 __x = (__typeof__((x)-(x)))(x); \
@@ -194,8 +194,8 @@ do { \
"2:\n" \
" sw %z4, %2\n" \
"3:\n" \
- _ASM_EXTABLE_UACCESS_ERR(1b, 3b, %0) \
- _ASM_EXTABLE_UACCESS_ERR(2b, 3b, %0) \
+ _ASM_EXTABLE_##type##ACCESS_ERR(1b, 3b, %0) \
+ _ASM_EXTABLE_##type##ACCESS_ERR(2b, 3b, %0) \
: "+r" (err), \
"=m" (__ptr[__LSW]), \
"=m" (__ptr[__MSW]) \
@@ -203,24 +203,24 @@ do { \
} while (0)
#endif /* CONFIG_64BIT */
-#define __put_mem_nocheck(x, __gu_ptr, __pu_err) \
-do { \
- switch (sizeof(*__gu_ptr)) { \
- case 1: \
- __put_mem_asm("sb", (x), __gu_ptr, __pu_err); \
- break; \
- case 2: \
- __put_mem_asm("sh", (x), __gu_ptr, __pu_err); \
- break; \
- case 4: \
- __put_mem_asm("sw", (x), __gu_ptr, __pu_err); \
- break; \
- case 8: \
- __put_mem_8((x), __gu_ptr, __pu_err); \
- break; \
- default: \
- BUILD_BUG(); \
- } \
+#define __put_mem_nocheck(x, __gu_ptr, __pu_err, type) \
+do { \
+ switch (sizeof(*__gu_ptr)) { \
+ case 1: \
+ __put_mem_asm("sb", (x), __gu_ptr, __pu_err, type); \
+ break; \
+ case 2: \
+ __put_mem_asm("sh", (x), __gu_ptr, __pu_err, type); \
+ break; \
+ case 4: \
+ __put_mem_asm("sw", (x), __gu_ptr, __pu_err, type); \
+ break; \
+ case 8: \
+ __put_mem_8((x), __gu_ptr, __pu_err, type); \
+ break; \
+ default: \
+ BUILD_BUG(); \
+ } \
} while (0)
/**
@@ -253,7 +253,7 @@ do { \
__chk_user_ptr(__gu_ptr); \
\
__enable_user_access(); \
- __put_mem_nocheck(__val, __gu_ptr, __pu_err); \
+ __put_mem_nocheck(__val, __gu_ptr, __pu_err, U); \
__disable_user_access(); \
\
__pu_err; \
@@ -279,7 +279,7 @@ do { \
({ \
__typeof__(*(ptr)) __user *__p = (ptr); \
might_fault(); \
- access_ok(__p, sizeof(*__p)) ? \
+ access_ok(__p, sizeof(*__p)) ? \
__put_user((x), __p) : \
-EFAULT; \
})
@@ -321,7 +321,7 @@ unsigned long __must_check clear_user(void __user *to, unsigned long n)
do { \
long __kr_err; \
\
- __get_mem_nocheck(*((type *)(dst)), (type *)(src), __kr_err); \
+ __get_mem_nocheck(*((type *)(dst)), (type *)(src), __kr_err, K);\
if (unlikely(__kr_err)) \
goto err_label; \
} while (0)
@@ -330,7 +330,7 @@ do { \
do { \
long __kr_err; \
\
- __put_mem_nocheck(*((type *)(src)), (type *)(dst), __kr_err); \
+ __put_mem_nocheck(*((type *)(src)), (type *)(dst), __kr_err, K);\
if (unlikely(__kr_err)) \
goto err_label; \
} while (0)
diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
index 35484d830fd6..a21ad8237189 100644
--- a/arch/riscv/mm/extable.c
+++ b/arch/riscv/mm/extable.c
@@ -64,6 +64,7 @@ bool fixup_exception(struct pt_regs *regs)
case EX_TYPE_BPF:
return ex_handler_bpf(ex, regs);
case EX_TYPE_UACCESS_ERR_ZERO:
+ case EX_TYPE_KACCESS_ERR_ZERO:
return ex_handler_uaccess_err_zero(ex, regs);
}
--
2.25.1
Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
__get/put_kernel_nofault(), which is not always uaccess, so the name with
*user* is not appropriate.
Also rename xxx_user_xxx to xxx_mem_xx on the call path of
__get/put_user_nocheck()
Only refactor code without any functional changes.
Signed-off-by: Tong Tiangen <[email protected]>
---
arch/riscv/include/asm/uaccess.h | 48 ++++++++++++++++----------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 855450bed9f5..1370da055b44 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -50,7 +50,7 @@
* call.
*/
-#define __get_user_asm(insn, x, ptr, err) \
+#define __get_mem_asm(insn, x, ptr, err) \
do { \
__typeof__(x) __x; \
__asm__ __volatile__ ( \
@@ -64,12 +64,12 @@ do { \
} while (0)
#ifdef CONFIG_64BIT
-#define __get_user_8(x, ptr, err) \
- __get_user_asm("ld", x, ptr, err)
+#define __get_mem_8(x, ptr, err) \
+ __get_mem_asm("ld", x, ptr, err)
#else /* !CONFIG_64BIT */
-#define __get_user_8(x, ptr, err) \
+#define __get_mem_8(x, ptr, err) \
do { \
- u32 __user *__ptr = (u32 __user *)(ptr); \
+ u32 *__ptr = (u32 *)(ptr); \
u32 __lo, __hi; \
__asm__ __volatile__ ( \
"1:\n" \
@@ -88,20 +88,20 @@ do { \
} while (0)
#endif /* CONFIG_64BIT */
-#define __get_user_nocheck(x, __gu_ptr, __gu_err) \
+#define __get_mem_nocheck(x, __gu_ptr, __gu_err) \
do { \
switch (sizeof(*__gu_ptr)) { \
case 1: \
- __get_user_asm("lb", (x), __gu_ptr, __gu_err); \
+ __get_mem_asm("lb", (x), __gu_ptr, __gu_err); \
break; \
case 2: \
- __get_user_asm("lh", (x), __gu_ptr, __gu_err); \
+ __get_mem_asm("lh", (x), __gu_ptr, __gu_err); \
break; \
case 4: \
- __get_user_asm("lw", (x), __gu_ptr, __gu_err); \
+ __get_mem_asm("lw", (x), __gu_ptr, __gu_err); \
break; \
case 8: \
- __get_user_8((x), __gu_ptr, __gu_err); \
+ __get_mem_8((x), __gu_ptr, __gu_err); \
break; \
default: \
BUILD_BUG(); \
@@ -136,7 +136,7 @@ do { \
__chk_user_ptr(__gu_ptr); \
\
__enable_user_access(); \
- __get_user_nocheck(x, __gu_ptr, __gu_err); \
+ __get_mem_nocheck(x, __gu_ptr, __gu_err); \
__disable_user_access(); \
\
__gu_err; \
@@ -168,7 +168,7 @@ do { \
((x) = 0, -EFAULT); \
})
-#define __put_user_asm(insn, x, ptr, err) \
+#define __put_mem_asm(insn, x, ptr, err) \
do { \
__typeof__(*(ptr)) __x = x; \
__asm__ __volatile__ ( \
@@ -181,12 +181,12 @@ do { \
} while (0)
#ifdef CONFIG_64BIT
-#define __put_user_8(x, ptr, err) \
- __put_user_asm("sd", x, ptr, err)
+#define __put_mem_8(x, ptr, err) \
+ __put_mem_asm("sd", x, ptr, err)
#else /* !CONFIG_64BIT */
-#define __put_user_8(x, ptr, err) \
+#define __put_mem_8(x, ptr, err) \
do { \
- u32 __user *__ptr = (u32 __user *)(ptr); \
+ u32 *__ptr = (u32 *)(ptr); \
u64 __x = (__typeof__((x)-(x)))(x); \
__asm__ __volatile__ ( \
"1:\n" \
@@ -203,20 +203,20 @@ do { \
} while (0)
#endif /* CONFIG_64BIT */
-#define __put_user_nocheck(x, __gu_ptr, __pu_err) \
+#define __put_mem_nocheck(x, __gu_ptr, __pu_err) \
do { \
switch (sizeof(*__gu_ptr)) { \
case 1: \
- __put_user_asm("sb", (x), __gu_ptr, __pu_err); \
+ __put_mem_asm("sb", (x), __gu_ptr, __pu_err); \
break; \
case 2: \
- __put_user_asm("sh", (x), __gu_ptr, __pu_err); \
+ __put_mem_asm("sh", (x), __gu_ptr, __pu_err); \
break; \
case 4: \
- __put_user_asm("sw", (x), __gu_ptr, __pu_err); \
+ __put_mem_asm("sw", (x), __gu_ptr, __pu_err); \
break; \
case 8: \
- __put_user_8((x), __gu_ptr, __pu_err); \
+ __put_mem_8((x), __gu_ptr, __pu_err); \
break; \
default: \
BUILD_BUG(); \
@@ -253,7 +253,7 @@ do { \
__chk_user_ptr(__gu_ptr); \
\
__enable_user_access(); \
- __put_user_nocheck(__val, __gu_ptr, __pu_err); \
+ __put_mem_nocheck(__val, __gu_ptr, __pu_err); \
__disable_user_access(); \
\
__pu_err; \
@@ -321,7 +321,7 @@ unsigned long __must_check clear_user(void __user *to, unsigned long n)
do { \
long __kr_err; \
\
- __get_user_nocheck(*((type *)(dst)), (type *)(src), __kr_err); \
+ __get_mem_nocheck(*((type *)(dst)), (type *)(src), __kr_err); \
if (unlikely(__kr_err)) \
goto err_label; \
} while (0)
@@ -330,7 +330,7 @@ do { \
do { \
long __kr_err; \
\
- __put_user_nocheck(*((type *)(src)), (type *)(dst), __kr_err); \
+ __put_mem_nocheck(*((type *)(src)), (type *)(dst), __kr_err); \
if (unlikely(__kr_err)) \
goto err_label; \
} while (0)
--
2.25.1
Hi riscv maintainers, kindly ping...
Thanks,
Tong.
在 2022/8/15 11:20, Tong Tiangen 写道:
> This patchset do some refactorings related to riscv's uaccess and extable,
> mainly for the usage of __get/put_user_nocheck() which not distinguish user
> access and kernel access.
>
> v1 -> v2:
> According to Conor's suggestion, split into two logically independent
> patches.
>
> Tong Tiangen (2):
> riscv: uaccess: rename __get/put_user_nocheck to __get/put_mem_nocheck
> riscv: extable: add new extable type EX_TYPE_KACCESS_ERR_ZERO support
>
> arch/riscv/include/asm/asm-extable.h | 12 ++
> arch/riscv/include/asm/uaccess.h | 162 +++++++++++++--------------
> arch/riscv/mm/extable.c | 1 +
> 3 files changed, 94 insertions(+), 81 deletions(-)
>
On 24/08/2022 07:31, Tong Tiangen wrote:
> Hi riscv maintainers, kindly ping...
>
> Thanks,
> Tong.
>
> 在 2022/8/15 11:20, Tong Tiangen 写道:
It's barely been more than a week, relax :)
checkpatch really does not like one of the macros you added. Please
consider whether this is valid:
ERROR: Macros with complex values should be enclosed in parentheses
#38: FILE: arch/riscv/include/asm/asm-extable.h:61:
+#define _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, zero) \
+ __DEFINE_ASM_GPR_NUMS \
+ __ASM_EXTABLE_RAW(#insn, #fixup, \
+ __stringify(EX_TYPE_KACCESS_ERR_ZERO), \
+ "(" \
+ EX_DATA_REG(ERR, err) " | " \
+ EX_DATA_REG(ZERO, zero) \
+ ")")
Thanks,
Conor.
>> This patchset do some refactorings related to riscv's uaccess and extable,
>> mainly for the usage of __get/put_user_nocheck() which not distinguish user
>> access and kernel access.
>>
>> v1 -> v2:
>> According to Conor's suggestion, split into two logically independent
>> patches.
>>
>> Tong Tiangen (2):
>> riscv: uaccess: rename __get/put_user_nocheck to __get/put_mem_nocheck
>> riscv: extable: add new extable type EX_TYPE_KACCESS_ERR_ZERO support
>>
>> arch/riscv/include/asm/asm-extable.h | 12 ++
>> arch/riscv/include/asm/uaccess.h | 162 +++++++++++++--------------
>> arch/riscv/mm/extable.c | 1 +
>> 3 files changed, 94 insertions(+), 81 deletions(-)
>>
在 2022/8/25 0:49, [email protected] 写道:
> On 24/08/2022 07:31, Tong Tiangen wrote:
>> Hi riscv maintainers, kindly ping...
>>
>> Thanks,
>> Tong.
>>
>> 在 2022/8/15 11:20, Tong Tiangen 写道:
>
> It's barely been more than a week, relax :)
>
> checkpatch really does not like one of the macros you added. Please
> consider whether this is valid:
>
> ERROR: Macros with complex values should be enclosed in parentheses
> #38: FILE: arch/riscv/include/asm/asm-extable.h:61:
> +#define _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, zero) \
> + __DEFINE_ASM_GPR_NUMS \
> + __ASM_EXTABLE_RAW(#insn, #fixup, \
> + __stringify(EX_TYPE_KACCESS_ERR_ZERO), \
> + "(" \
> + EX_DATA_REG(ERR, err) " | " \
> + EX_DATA_REG(ZERO, zero) \
> + ")")
>
> Thanks,
> Conor.
Judging from the use context of this macro, there is no problem with the
definition of this macro.
In addition, I refer to the definition of macro
_ASM_EXTABLE_UACCESS_ERR_ZERO for the style of this macro. The
difference is that the types used in the macro are different.
:)
Thanks,
Tong.
>
>>> This patchset do some refactorings related to riscv's uaccess and extable,
>>> mainly for the usage of __get/put_user_nocheck() which not distinguish user
>>> access and kernel access.
>>>
>>> v1 -> v2:
>>> According to Conor's suggestion, split into two logically independent
>>> patches.
>>>
>>> Tong Tiangen (2):
>>> riscv: uaccess: rename __get/put_user_nocheck to __get/put_mem_nocheck
>>> riscv: extable: add new extable type EX_TYPE_KACCESS_ERR_ZERO support
>>>
>>> arch/riscv/include/asm/asm-extable.h | 12 ++
>>> arch/riscv/include/asm/uaccess.h | 162 +++++++++++++--------------
>>> arch/riscv/mm/extable.c | 1 +
>>> 3 files changed, 94 insertions(+), 81 deletions(-)
>>>
On Mon, Aug 15, 2022 at 03:20:25AM +0000, Tong Tiangen wrote:
> Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
> __get/put_kernel_nofault(), but those helpers are not uaccess type, so we
> add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
> __get/put_kernel_no_fault().
>
> Only refactor code without any functional changes.
This isn't quite true. __get/put_kernel_nofault now sets a different
extable type (as the commit message says). But, nothing special seems
to be done with that, so there's effectively no functional change. Can
you please elaborate on the motivation for this change? Where will the
KACCESS type need to be distinguished from the UACCESS type?
Thanks,
drew
On Mon, Aug 15, 2022 at 03:20:24AM +0000, Tong Tiangen wrote:
> Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
> __get/put_kernel_nofault(), which is not always uaccess, so the name with
> *user* is not appropriate.
>
> Also rename xxx_user_xxx to xxx_mem_xx on the call path of
> __get/put_user_nocheck()
>
> Only refactor code without any functional changes.
>
> Signed-off-by: Tong Tiangen <[email protected]>
> ---
> arch/riscv/include/asm/uaccess.h | 48 ++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> index 855450bed9f5..1370da055b44 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -50,7 +50,7 @@
> * call.
> */
>
> -#define __get_user_asm(insn, x, ptr, err) \
> +#define __get_mem_asm(insn, x, ptr, err) \
> do { \
> __typeof__(x) __x; \
> __asm__ __volatile__ ( \
> @@ -64,12 +64,12 @@ do { \
> } while (0)
>
> #ifdef CONFIG_64BIT
> -#define __get_user_8(x, ptr, err) \
> - __get_user_asm("ld", x, ptr, err)
> +#define __get_mem_8(x, ptr, err) \
> + __get_mem_asm("ld", x, ptr, err)
> #else /* !CONFIG_64BIT */
> -#define __get_user_8(x, ptr, err) \
> +#define __get_mem_8(x, ptr, err) \
> do { \
> - u32 __user *__ptr = (u32 __user *)(ptr); \
> + u32 *__ptr = (u32 *)(ptr); \
Doesn't casting away __user reduce sparse's utility?
> u32 __lo, __hi; \
> __asm__ __volatile__ ( \
> "1:\n" \
> @@ -88,20 +88,20 @@ do { \
> } while (0)
> #endif /* CONFIG_64BIT */
>
> -#define __get_user_nocheck(x, __gu_ptr, __gu_err) \
> +#define __get_mem_nocheck(x, __gu_ptr, __gu_err) \
The patch replaces all get/put_user instances with get/put_mem,
but what about 'gu' and 'pu' instances (which are presumably short
for get/put_user)?
Thanks,
drew
在 2022/8/25 18:56, Andrew Jones 写道:
> On Mon, Aug 15, 2022 at 03:20:24AM +0000, Tong Tiangen wrote:
>> Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
>> __get/put_kernel_nofault(), which is not always uaccess, so the name with
>> *user* is not appropriate.
>>
>> Also rename xxx_user_xxx to xxx_mem_xx on the call path of
>> __get/put_user_nocheck()
>>
>> Only refactor code without any functional changes.
>>
>> Signed-off-by: Tong Tiangen <[email protected]>
>> ---
>> arch/riscv/include/asm/uaccess.h | 48 ++++++++++++++++----------------
>> 1 file changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>> index 855450bed9f5..1370da055b44 100644
>> --- a/arch/riscv/include/asm/uaccess.h
>> +++ b/arch/riscv/include/asm/uaccess.h
>> @@ -50,7 +50,7 @@
>> * call.
>> */
>>
>> -#define __get_user_asm(insn, x, ptr, err) \
>> +#define __get_mem_asm(insn, x, ptr, err) \
>> do { \
>> __typeof__(x) __x; \
>> __asm__ __volatile__ ( \
>> @@ -64,12 +64,12 @@ do { \
>> } while (0)
>>
>> #ifdef CONFIG_64BIT
>> -#define __get_user_8(x, ptr, err) \
>> - __get_user_asm("ld", x, ptr, err)
>> +#define __get_mem_8(x, ptr, err) \
>> + __get_mem_asm("ld", x, ptr, err)
>> #else /* !CONFIG_64BIT */
>> -#define __get_user_8(x, ptr, err) \
>> +#define __get_mem_8(x, ptr, err) \
>> do { \
>> - u32 __user *__ptr = (u32 __user *)(ptr); \
>> + u32 *__ptr = (u32 *)(ptr); \
>
> Doesn't casting away __user reduce sparse's utility?
From the call logic[1], the address passed into this macro is not
necessarily __user. I understand that no problem will be introduced for
sparse's utility.
In addition, there is no need to do a pointer conversion here, will be
fixed next version.
[1] __get_kernel_nofault -> __get_mem_nocheck -> __get_mem_8
>
>> u32 __lo, __hi; \
>> __asm__ __volatile__ ( \
>> "1:\n" \
>> @@ -88,20 +88,20 @@ do { \
>> } while (0)
>> #endif /* CONFIG_64BIT */
>>
>> -#define __get_user_nocheck(x, __gu_ptr, __gu_err) \
>> +#define __get_mem_nocheck(x, __gu_ptr, __gu_err) \
>
> The patch replaces all get/put_user instances with get/put_mem,
> but what about 'gu' and 'pu' instances (which are presumably short
> for get/put_user)?
ok, missing that, It is not appropriate to use __gu_xxx，will be fixed
next version.
Thanks,
Tong.
>
> Thanks,
> drew
> .
在 2022/8/25 19:06, Andrew Jones 写道:
> On Mon, Aug 15, 2022 at 03:20:25AM +0000, Tong Tiangen wrote:
>> Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
>> __get/put_kernel_nofault(), but those helpers are not uaccess type, so we
>> add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
>> __get/put_kernel_no_fault().
>>
>> Only refactor code without any functional changes.
>
> This isn't quite true. __get/put_kernel_nofault now sets a different
> extable type (as the commit message says). But, nothing special seems
> to be done with that, so there's effectively no functional change. Can
> you please elaborate on the motivation for this change? Where will the
> KACCESS type need to be distinguished from the UACCESS type?
The introduction of EX_TYPE_KACCESS_ERR_ZERO does not change any
function, but makes a correct distinction in the actual type, indicating
that there are indeed some kaccess entries in extable. I think this
optimization is more clear and reasonable.
A few weeks ago, I did something similar on arm64[1]. I think this
optimization can also be used on riscv.
We can do some features that are used on uaccss but not applicable on
kaccess in the future[2].
[1]
https://lore.kernel.org/lkml/[email protected]/
[2]https://lore.kernel.org/lkml/[email protected]/
Thanks,
Tong.
>
> Thanks,
> drew
> .
On Fri, Aug 26, 2022 at 02:33:47PM +0800, Tong Tiangen wrote:
>
>
> 在 2022/8/25 18:56, Andrew Jones 写道:
> > On Mon, Aug 15, 2022 at 03:20:24AM +0000, Tong Tiangen wrote:
> > > Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
> > > __get/put_kernel_nofault(), which is not always uaccess, so the name with
> > > *user* is not appropriate.
> > >
> > > Also rename xxx_user_xxx to xxx_mem_xx on the call path of
> > > __get/put_user_nocheck()
> > >
> > > Only refactor code without any functional changes.
> > >
> > > Signed-off-by: Tong Tiangen <[email protected]>
> > > ---
> > > arch/riscv/include/asm/uaccess.h | 48 ++++++++++++++++----------------
> > > 1 file changed, 24 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> > > index 855450bed9f5..1370da055b44 100644
> > > --- a/arch/riscv/include/asm/uaccess.h
> > > +++ b/arch/riscv/include/asm/uaccess.h
> > > @@ -50,7 +50,7 @@
> > > * call.
> > > */
> > > -#define __get_user_asm(insn, x, ptr, err) \
> > > +#define __get_mem_asm(insn, x, ptr, err) \
> > > do { \
> > > __typeof__(x) __x; \
> > > __asm__ __volatile__ ( \
> > > @@ -64,12 +64,12 @@ do { \
> > > } while (0)
> > > #ifdef CONFIG_64BIT
> > > -#define __get_user_8(x, ptr, err) \
> > > - __get_user_asm("ld", x, ptr, err)
> > > +#define __get_mem_8(x, ptr, err) \
> > > + __get_mem_asm("ld", x, ptr, err)
> > > #else /* !CONFIG_64BIT */
> > > -#define __get_user_8(x, ptr, err) \
> > > +#define __get_mem_8(x, ptr, err) \
> > > do { \
> > > - u32 __user *__ptr = (u32 __user *)(ptr); \
> > > + u32 *__ptr = (u32 *)(ptr); \
> >
> > Doesn't casting away __user reduce sparse's utility?
>
> From the call logic[1], the address passed into this macro is not
> necessarily __user. I understand that no problem will be introduced for
> sparse's utility.
>
> In addition, there is no need to do a pointer conversion here, will be fixed
> next version.
>
> [1] __get_kernel_nofault -> __get_mem_nocheck -> __get_mem_8
Yes, I understood that. My concern was for the times that the address was
__user as we'd no longer get that check for them.
Thanks,
drew
On Fri, Aug 26, 2022 at 02:44:48PM +0800, Tong Tiangen wrote:
>
>
> 在 2022/8/25 19:06, Andrew Jones 写道:
> > On Mon, Aug 15, 2022 at 03:20:25AM +0000, Tong Tiangen wrote:
> > > Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
> > > __get/put_kernel_nofault(), but those helpers are not uaccess type, so we
> > > add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
> > > __get/put_kernel_no_fault().
> > >
> > > Only refactor code without any functional changes.
> >
> > This isn't quite true. __get/put_kernel_nofault now sets a different
> > extable type (as the commit message says). But, nothing special seems
> > to be done with that, so there's effectively no functional change. Can
> > you please elaborate on the motivation for this change? Where will the
> > KACCESS type need to be distinguished from the UACCESS type?
>
> The introduction of EX_TYPE_KACCESS_ERR_ZERO does not change any function,
> but makes a correct distinction in the actual type, indicating that there
> are indeed some kaccess entries in extable. I think this optimization is
> more clear and reasonable.
Well, creating new types, just for new type sake, just bloats code.
>
> A few weeks ago, I did something similar on arm64[1]. I think this
> optimization can also be used on riscv.
>
> We can do some features that are used on uaccss but not applicable on
> kaccess in the future[2].
>
> [1]
> https://lore.kernel.org/lkml/[email protected]/
> [2]https://lore.kernel.org/lkml/[email protected]/
>
This is part of the information, but I had already found this. What's
still missing to me are the riscv patches, or at least a riscv plan, for
actually implementing something which requires kaccess and uaccess to have
distinct types.
Thanks,
drew
On Mon, Aug 15, 2022 at 5:20 AM Tong Tiangen <[email protected]> wrote:
>
> Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
> __get/put_kernel_nofault(), which is not always uaccess, so the name with
> *user* is not appropriate.
>
> Also rename xxx_user_xxx to xxx_mem_xx on the call path of
> __get/put_user_nocheck()
>
> Only refactor code without any functional changes.
>
> Signed-off-by: Tong Tiangen <[email protected]>
I would prefer this not being done, it just makes riscv diverge from the
code on other architectures. While the new name does make more sense,
it ends up making it harder to refactor this across architectures in the end.
There are two important cleanups that I would like to see done in
asm/uaccess.h across architectures:
- generalize the __get_user()/__put_user()/__get_kernel_nofault()/
__put_kernel_nofault() wrappers to the point that architectures do not
need to worry about the variable type stuff but instead just provide
trivial fixed-length helpers of some sort
- change the calling conventions in a way that allows the use of the
asm-goto-with-output method for better object code on modern
compilers.
The x86 version already has most of this, with their
__get_user_size() macro supporting both the asm-goto label
and the error code assignment, so the generalized code should
probably be based on that approach.
Arnd
在 2022/8/26 16:16, Andrew Jones 写道:
> On Fri, Aug 26, 2022 at 02:44:48PM +0800, Tong Tiangen wrote:
>>
>>
>> 在 2022/8/25 19:06, Andrew Jones 写道:
>>> On Mon, Aug 15, 2022 at 03:20:25AM +0000, Tong Tiangen wrote:
>>>> Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
>>>> __get/put_kernel_nofault(), but those helpers are not uaccess type, so we
>>>> add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
>>>> __get/put_kernel_no_fault().
>>>>
>>>> Only refactor code without any functional changes.
>>>
>>> This isn't quite true. __get/put_kernel_nofault now sets a different
>>> extable type (as the commit message says). But, nothing special seems
>>> to be done with that, so there's effectively no functional change. Can
>>> you please elaborate on the motivation for this change? Where will the
>>> KACCESS type need to be distinguished from the UACCESS type?
>>
>> The introduction of EX_TYPE_KACCESS_ERR_ZERO does not change any function,
>> but makes a correct distinction in the actual type, indicating that there
>> are indeed some kaccess entries in extable. I think this optimization is
>> more clear and reasonable.
>
> Well, creating new types, just for new type sake, just bloats code.
>
>>
>> A few weeks ago, I did something similar on arm64[1]. I think this
>> optimization can also be used on riscv.
>>
>> We can do some features that are used on uaccss but not applicable on
>> kaccess in the future[2].
>>
>> [1]
>> https://lore.kernel.org/lkml/[email protected]/
>> [2]https://lore.kernel.org/lkml/[email protected]/
>>
>
> This is part of the information, but I had already found this. What's
> still missing to me are the riscv patches, or at least a riscv plan, for
> actually implementing something which requires kaccess and uaccess to have
> distinct types.
>
> Thanks,
> drew
At present, there is no such plan on riscv, because it is rely on
hardware support.
I think this patch can be merged as a small code optimization and
without any function change.
Thanks,
Tong.
>
> .
在 2022/8/26 17:30, Arnd Bergmann 写道:
> On Mon, Aug 15, 2022 at 5:20 AM Tong Tiangen <[email protected]> wrote:
>>
>> Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
>> __get/put_kernel_nofault(), which is not always uaccess, so the name with
>> *user* is not appropriate.
>>
>> Also rename xxx_user_xxx to xxx_mem_xx on the call path of
>> __get/put_user_nocheck()
>>
>> Only refactor code without any functional changes.
>>
>> Signed-off-by: Tong Tiangen <[email protected]>
>
> I would prefer this not being done, it just makes riscv diverge from the
> code on other architectures. While the new name does make more sense,
> it ends up making it harder to refactor this across architectures in the end.
>
> There are two important cleanups that I would like to see done in
> asm/uaccess.h across architectures:
>
> - generalize the __get_user()/__put_user()/__get_kernel_nofault()/
> __put_kernel_nofault() wrappers to the point that architectures do not
> need to worry about the variable type stuff but instead just provide
> trivial fixed-length helpers of some sort
>
> - change the calling conventions in a way that allows the use of the
> asm-goto-with-output method for better object code on modern
> compilers.
>
> The x86 version already has most of this, with their
> __get_user_size() macro supporting both the asm-goto label
> and the error code assignment, so the generalized code should
> probably be based on that approach.
I am very interested in the implementation of X86. I need to investigate
and consider a cross architecture implementation.
However, I understand that the modification of the current patch has
little to do with the two points mentioned above. We can optimize the
code step by step.
Thanks,
Tong.
>
> Arnd
>
> .
在 2022/8/26 15:43, Andrew Jones 写道:
> On Fri, Aug 26, 2022 at 02:33:47PM +0800, Tong Tiangen wrote:
>>
>>
>> 在 2022/8/25 18:56, Andrew Jones 写道:
>>> On Mon, Aug 15, 2022 at 03:20:24AM +0000, Tong Tiangen wrote:
>>>> Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
>>>> __get/put_kernel_nofault(), which is not always uaccess, so the name with
>>>> *user* is not appropriate.
>>>>
>>>> Also rename xxx_user_xxx to xxx_mem_xx on the call path of
>>>> __get/put_user_nocheck()
>>>>
>>>> Only refactor code without any functional changes.
>>>>
>>>> Signed-off-by: Tong Tiangen <[email protected]>
>>>> ---
>>>> arch/riscv/include/asm/uaccess.h | 48 ++++++++++++++++----------------
>>>> 1 file changed, 24 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>>>> index 855450bed9f5..1370da055b44 100644
>>>> --- a/arch/riscv/include/asm/uaccess.h
>>>> +++ b/arch/riscv/include/asm/uaccess.h
>>>> @@ -50,7 +50,7 @@
>>>> * call.
>>>> */
>>>> -#define __get_user_asm(insn, x, ptr, err) \
>>>> +#define __get_mem_asm(insn, x, ptr, err) \
>>>> do { \
>>>> __typeof__(x) __x; \
>>>> __asm__ __volatile__ ( \
>>>> @@ -64,12 +64,12 @@ do { \
>>>> } while (0)
>>>> #ifdef CONFIG_64BIT
>>>> -#define __get_user_8(x, ptr, err) \
>>>> - __get_user_asm("ld", x, ptr, err)
>>>> +#define __get_mem_8(x, ptr, err) \
>>>> + __get_mem_asm("ld", x, ptr, err)
>>>> #else /* !CONFIG_64BIT */
>>>> -#define __get_user_8(x, ptr, err) \
>>>> +#define __get_mem_8(x, ptr, err) \
>>>> do { \
>>>> - u32 __user *__ptr = (u32 __user *)(ptr); \
>>>> + u32 *__ptr = (u32 *)(ptr); \
>>>
>>> Doesn't casting away __user reduce sparse's utility?
>>
>> From the call logic[1], the address passed into this macro is not
>> necessarily __user. I understand that no problem will be introduced for
>> sparse's utility.
>>
>> In addition, there is no need to do a pointer conversion here, will be fixed
>> next version.
>>
>> [1] __get_kernel_nofault -> __get_mem_nocheck -> __get_mem_8
>
> Yes, I understood that. My concern was for the times that the address was
> __user as we'd no longer get that check for them.
Check __user ptr at __get_user() has the same effect? Is this
understanding correct？
Thanks,
Tong.
>
> Thanks,
> drew
>
> .
On Sat, Aug 27, 2022 at 12:43 PM Tong Tiangen <[email protected]> wrote:
> 在 2022/8/26 17:30, Arnd Bergmann 写道:
>
> I am very interested in the implementation of X86. I need to investigate
> and consider a cross architecture implementation.
One more point about the cross-architecture work: it generally makes
sense to do the most commonly used architectures first, usually
that would be x86, arm64 and powerpc64, followed by riscv, arm,
s390 and mips. If we can find something that the first architecture
maintainers like, everyone else can follow and you don't have to
rework all of them multiple times before getting to a consensus.
> However, I understand that the modification of the current patch has
> little to do with the two points mentioned above. We can optimize the
> code step by step.
You are correct that this has little to do with your patch, my point
was mainly that your patch is moving the code further away from
the other architectures, so it would make it harder to then do the
changes we actually want.
Arnd
在 2022/8/27 20:49, Arnd Bergmann 写道:
> On Sat, Aug 27, 2022 at 12:43 PM Tong Tiangen <[email protected]> wrote:
>> 在 2022/8/26 17:30, Arnd Bergmann 写道:
>>
>> I am very interested in the implementation of X86. I need to investigate
>> and consider a cross architecture implementation.
>
> One more point about the cross-architecture work: it generally makes
> sense to do the most commonly used architectures first, usually
> that would be x86, arm64 and powerpc64, followed by riscv, arm,
> s390 and mips. If we can find something that the first architecture
> maintainers like, everyone else can follow and you don't have to
> rework all of them multiple times before getting to a consensus.
>
>> However, I understand that the modification of the current patch has
>> little to do with the two points mentioned above. We can optimize the
>> code step by step.
>
> You are correct that this has little to do with your patch, my point
> was mainly that your patch is moving the code further away from
> the other architectures, so it would make it harder to then do the
> changes we actually want.
>
> Arnd
I understand what you mean，it's reasonable.
Thanks,
Tong.
>
> .
On Sat, 27 Aug 2022 03:39:38 PDT (-0700), [email protected] wrote:
>
>
> 在 2022/8/26 16:16, Andrew Jones 写道:
>> On Fri, Aug 26, 2022 at 02:44:48PM +0800, Tong Tiangen wrote:
>>>
>>>
>>> 在 2022/8/25 19:06, Andrew Jones 写道:
>>>> On Mon, Aug 15, 2022 at 03:20:25AM +0000, Tong Tiangen wrote:
>>>>> Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
>>>>> __get/put_kernel_nofault(), but those helpers are not uaccess type, so we
>>>>> add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
>>>>> __get/put_kernel_no_fault().
>>>>>
>>>>> Only refactor code without any functional changes.
>>>>
>>>> This isn't quite true. __get/put_kernel_nofault now sets a different
>>>> extable type (as the commit message says). But, nothing special seems
>>>> to be done with that, so there's effectively no functional change. Can
>>>> you please elaborate on the motivation for this change? Where will the
>>>> KACCESS type need to be distinguished from the UACCESS type?
>>>
>>> The introduction of EX_TYPE_KACCESS_ERR_ZERO does not change any function,
>>> but makes a correct distinction in the actual type, indicating that there
>>> are indeed some kaccess entries in extable. I think this optimization is
>>> more clear and reasonable.
>>
>> Well, creating new types, just for new type sake, just bloats code.
>>
>>>
>>> A few weeks ago, I did something similar on arm64[1]. I think this
>>> optimization can also be used on riscv.
>>>
>>> We can do some features that are used on uaccss but not applicable on
>>> kaccess in the future[2].
>>>
>>> [1]
>>> https://lore.kernel.org/lkml/[email protected]/
>>> [2]https://lore.kernel.org/lkml/[email protected]/
>>>
>>
>> This is part of the information, but I had already found this. What's
>> still missing to me are the riscv patches, or at least a riscv plan, for
>> actually implementing something which requires kaccess and uaccess to have
>> distinct types.
>>
>> Thanks,
>> drew
>
> At present, there is no such plan on riscv, because it is rely on
> hardware support.
> I think this patch can be merged as a small code optimization and
> without any function change.
Generally we need some use of the code in the upstream kernel to justify
its existence. In this case I don't really see that: it's just another
type that's exactly the same as the existing one, having some out of
tree code that depends on making these types do something different
isn't a sufficient justification.
在 2022/9/22 4:25, Palmer Dabbelt 写道:
> On Sat, 27 Aug 2022 03:39:38 PDT (-0700), [email protected] wrote:
>>
>>
>> 在 2022/8/26 16:16, Andrew Jones 写道:
>>> On Fri, Aug 26, 2022 at 02:44:48PM +0800, Tong Tiangen wrote:
>>>>
>>>>
>>>> 在 2022/8/25 19:06, Andrew Jones 写道:
>>>>> On Mon, Aug 15, 2022 at 03:20:25AM +0000, Tong Tiangen wrote:
>>>>>> Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
>>>>>> __get/put_kernel_nofault(), but those helpers are not uaccess
>>>>>> type, so we
>>>>>> add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
>>>>>> __get/put_kernel_no_fault().
>>>>>>
>>>>>> Only refactor code without any functional changes.
>>>>>
>>>>> This isn't quite true. __get/put_kernel_nofault now sets a different
>>>>> extable type (as the commit message says). But, nothing special seems
>>>>> to be done with that, so there's effectively no functional change. Can
>>>>> you please elaborate on the motivation for this change? Where will the
>>>>> KACCESS type need to be distinguished from the UACCESS type?
>>>>
>>>> The introduction of EX_TYPE_KACCESS_ERR_ZERO does not change any
>>>> function,
>>>> but makes a correct distinction in the actual type, indicating that
>>>> there
>>>> are indeed some kaccess entries in extable. I think this
>>>> optimization is
>>>> more clear and reasonable.
>>>
>>> Well, creating new types, just for new type sake, just bloats code.
>>>
>>>>
>>>> A few weeks ago, I did something similar on arm64[1]. I think this
>>>> optimization can also be used on riscv.
>>>>
>>>> We can do some features that are used on uaccss but not applicable on
>>>> kaccess in the future[2].
>>>>
>>>> [1]
>>>> https://lore.kernel.org/lkml/[email protected]/
>>>>
>>>> [2]https://lore.kernel.org/lkml/[email protected]/
>>>>
>>>>
>>>
>>> This is part of the information, but I had already found this. What's
>>> still missing to me are the riscv patches, or at least a riscv plan, for
>>> actually implementing something which requires kaccess and uaccess to
>>> have
>>> distinct types.
>>>
>>> Thanks,
>>> drew
>>
>> At present, there is no such plan on riscv, because it is rely on
>> hardware support.
>> I think this patch can be merged as a small code optimization and
>> without any function change.
>
> Generally we need some use of the code in the upstream kernel to justify
> its existence. In this case I don't really see that: it's just another
> type that's exactly the same as the existing one, having some out of
> tree code that depends on making these types do something different
> isn't a sufficient justification.
> .
Hi palmer:
I agree with this point very much，many thanks.
Tong.