Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO
On Mon, Jun 20, 2022 at 10:13:41PM +0800, Tong Tiangen wrote: > > > 在 2022/6/20 17:10, Mark Rutland 写道: > > On Mon, Jun 20, 2022 at 10:59:12AM +0800, Tong Tiangen wrote: > > > 在 2022/6/18 20:40, Mark Rutland 写道: > > > The following errors are reported during compilation: > > > [...] > > > arch/arm64/lib/clear_user.S:45: Error: invalid operands (*ABS* and *UND* > > > sections) for `<<' > > > [...] > > > > As above, I'm not seeing this. > > > > This suggests that the EX_DATA_REG() macro is going wrong somehow. Assuming > > the > > operand types correspond to the LHS and RHS of the expression, this would > > mean > > the GPR number is defined, but the REG value is not, and I can't currently > > see > > how that can happen. > Now I can compile success, both versions 9.4.0 and 11.2.0. > > I should have made a mistake. There is no problem using your implementation. > I will send a new version these days. No problem; thanks for confirming! Mark.
Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO
On Mon, Jun 20, 2022 at 10:59:12AM +0800, Tong Tiangen wrote: > 在 2022/6/18 20:40, Mark Rutland 写道: > > On Sat, Jun 18, 2022 at 04:42:06PM +0800, Tong Tiangen wrote: > > > > > > diff --git a/arch/arm64/include/asm/asm-extable.h > > > > > > b/arch/arm64/include/asm/asm-extable.h > > > > > > index 56ebe183e78b..9c94ac1f082c 100644 > > > > > > --- a/arch/arm64/include/asm/asm-extable.h > > > > > > +++ b/arch/arm64/include/asm/asm-extable.h > > > > > > @@ -28,6 +28,14 @@ > > > > > > __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > > > > > > .endm > > > > > > +/* > > > > > > + * Create an exception table entry for uaccess `insn`, which > > > > > > will branch to `fixup` > > > > > > + * when an unhandled fault is taken. > > > > > > + * ex->data = ~0 means both reg_err and reg_zero is set to > > > > > > wzr(x31). > > > > > > + */ > > > > > > + .macro _asm_extable_uaccess, insn, fixup > > > > > > + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_ERR_ZERO, ~0) > > > > > > + .endm > > > > > > > > > > I'm not too keen on using `~0` here, since that also sets other bits > > > > > in the > > > > > data field, and its somewhat opaque. > > > > > > > > > > How painful is it to generate the data fields as with the C version > > > > > of this > > > > > macro, so that we can pass in wzr explciitly for the two sub-fields? > > > > > > > > > > Other than that, this looks good to me. > > > > > > > > > > Thanks, > > > > > Mark. > > > > > > > > ok, will fix next version. > > > > > > > > Thanks, > > > > Tong. > > > > > > I tried to using data filelds as with C version, but here assembly code we > > > can not using operator such as << and |, if we use lsl and orr > > > instructions, > > > the gpr will be occupied. > > > > > > So how about using 0x3ff directly here? it means err register and zero > > > register both set to x31. > > > > I had a go at implementing this, and it seems simple enough. Please see: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/extable/asm-uaccess > > > > I made the following modifications, and the other parts are based on your > implementation: > > arch/arm64/include/asm/asm-extable.h > [...] > .macro _asm_extable_uaccess, insn, fixup > _ASM_EXTABLE_UACCESS(\insn, \fixup) > .endm > [...] I also made this same change locally when testing, and building with GCC 11.1.0 or LLVM 14.0.0 I am not seeing any problem when building, and the result is as expected: | [mark@lakrids:~/src/linux]% usekorg 11.1.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- defconfig | *** Default configuration is based on 'defconfig' | # | # No change to .config | # | [mark@lakrids:~/src/linux]% usekorg 11.1.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- -j50 arch/arm64/lib/ | CALLscripts/atomic/check-atomics.sh | CC arch/arm64/kernel/asm-offsets.s | CALLscripts/checksyscalls.sh | AS arch/arm64/kernel/vdso/note.o | AS arch/arm64/kernel/vdso/sigreturn.o | LD arch/arm64/kernel/vdso/vdso.so.dbg | VDSOSYM include/generated/vdso-offsets.h | OBJCOPY arch/arm64/kernel/vdso/vdso.so | make[2]: Nothing to be done for 'arch/arm64/lib/'. | AS arch/arm64/lib/clear_page.o | AS arch/arm64/lib/clear_user.o | AS arch/arm64/lib/copy_from_user.o | AS arch/arm64/lib/copy_page.o | AS arch/arm64/lib/copy_to_user.o | CC arch/arm64/lib/csum.o | CC arch/arm64/lib/delay.o | AS arch/arm64/lib/memchr.o | AS arch/arm64/lib/memcmp.o | AS arch/arm64/lib/memcpy.o | AS arch/arm64/lib/memset.o | AS arch/arm64/lib/strchr.o | AS arch/arm64/lib/strcmp.o | AS arch/arm64/lib/strlen.o | AS arch/arm64/lib/strncmp.o | AS arch/arm64/lib/strnlen.o | AS arch/arm64/lib/strrchr.o | AS arch/arm64/lib/tishift.o | AS arch/arm64/lib/crc32.o | AS arch/arm64/lib/mte.o | CC [M] arch/arm64/lib/xor-neon.o | AR arch/arm64/lib/built-in.a | AR arch/arm64/lib/lib.a | [mark@lakrids:~/src/linux]% usekorg 12.1.0 aarch64-linux-objdump -j __ex_table -D arch/arm64/lib/clear_user.o | | arch/arm64/lib/clear_user.o: file format elf64-littleaarch64 | | | Disassembly of section __ex_table: | | <__ex_table>: | ... |8: 03ff0003.inst 0x03ff0003 ; undefined | ... | 14: 03ff0003.inst 0x03ff0003 ; undefined | ... | 20: 03ff0003.inst 0x03ff0003 ; undefined | ... | 2c: 03ff0003.inst 0x03ff0003 ; undefined | ... | 38: 03ff0003.inst 0x03ff0003 ; undefined | ... | 44: 03ff0003.inst 0x03ff0003 ; undefined > The following errors are reported during compilation: > [...] > arch/arm64/lib/clear_user.S:45: Error: invalid operands (*ABS* and *UND* > sections) for `<<' > [...] As above, I'm not seeing this. This suggests that the EX_DATA_REG() macro is going
Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO
On Sat, Jun 18, 2022 at 04:42:06PM +0800, Tong Tiangen wrote: > > > > diff --git a/arch/arm64/include/asm/asm-extable.h > > > > b/arch/arm64/include/asm/asm-extable.h > > > > index 56ebe183e78b..9c94ac1f082c 100644 > > > > --- a/arch/arm64/include/asm/asm-extable.h > > > > +++ b/arch/arm64/include/asm/asm-extable.h > > > > @@ -28,6 +28,14 @@ > > > > __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > > > > .endm > > > > +/* > > > > + * Create an exception table entry for uaccess `insn`, which > > > > will branch to `fixup` > > > > + * when an unhandled fault is taken. > > > > + * ex->data = ~0 means both reg_err and reg_zero is set to wzr(x31). > > > > + */ > > > > + .macro _asm_extable_uaccess, insn, fixup > > > > + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_ERR_ZERO, ~0) > > > > + .endm > > > > > > I'm not too keen on using `~0` here, since that also sets other bits > > > in the > > > data field, and its somewhat opaque. > > > > > > How painful is it to generate the data fields as with the C version > > > of this > > > macro, so that we can pass in wzr explciitly for the two sub-fields? > > > > > > Other than that, this looks good to me. > > > > > > Thanks, > > > Mark. > > > > ok, will fix next version. > > > > Thanks, > > Tong. > > I tried to using data filelds as with C version, but here assembly code we > can not using operator such as << and |, if we use lsl and orr instructions, > the gpr will be occupied. > > So how about using 0x3ff directly here? it means err register and zero > register both set to x31. I had a go at implementing this, and it seems simple enough. Please see: https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/extable/asm-uaccess Mark.
Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO
On Sat, May 28, 2022 at 06:50:50AM +, Tong Tiangen wrote: > Currnetly, the extable type used by __arch_copy_from/to_user() is > EX_TYPE_FIXUP. In fact, It is more clearly to use meaningful > EX_TYPE_UACCESS_*. > > Suggested-by: Mark Rutland > Signed-off-by: Tong Tiangen > --- > arch/arm64/include/asm/asm-extable.h | 8 > arch/arm64/include/asm/asm-uaccess.h | 12 ++-- > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h > b/arch/arm64/include/asm/asm-extable.h > index 56ebe183e78b..9c94ac1f082c 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -28,6 +28,14 @@ > __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > .endm > > +/* > + * Create an exception table entry for uaccess `insn`, which will branch to > `fixup` > + * when an unhandled fault is taken. > + * ex->data = ~0 means both reg_err and reg_zero is set to wzr(x31). > + */ > + .macro _asm_extable_uaccess, insn, fixup > + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_ERR_ZERO, ~0) > + .endm I'm not too keen on using `~0` here, since that also sets other bits in the data field, and its somewhat opaque. How painful is it to generate the data fields as with the C version of this macro, so that we can pass in wzr explciitly for the two sub-fields? Other than that, this looks good to me. Thanks, Mark. > /* > * Create an exception table entry for `insn` if `fixup` is provided. > Otherwise > * do nothing. > diff --git a/arch/arm64/include/asm/asm-uaccess.h > b/arch/arm64/include/asm/asm-uaccess.h > index 0557af834e03..75b211c98dea 100644 > --- a/arch/arm64/include/asm/asm-uaccess.h > +++ b/arch/arm64/include/asm/asm-uaccess.h > @@ -61,7 +61,7 @@ alternative_else_nop_endif > > #define USER(l, x...)\ > :x; \ > - _asm_extableb, l > + _asm_extable_uaccessb, l > > /* > * Generate the assembly for LDTR/STTR with exception table entries. > @@ -73,8 +73,8 @@ alternative_else_nop_endif > 8889:ldtr\reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extableb,\l; > - _asm_extable8889b,\l; > + _asm_extable_uaccessb, \l; > + _asm_extable_uaccess8889b, \l; > .endm > > .macro user_stp l, reg1, reg2, addr, post_inc > @@ -82,14 +82,14 @@ alternative_else_nop_endif > 8889:sttr\reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extableb,\l; > - _asm_extable8889b,\l; > + _asm_extable_uaccessb,\l; > + _asm_extable_uaccess8889b,\l; > .endm > > .macro user_ldst l, inst, reg, addr, post_inc > :\inst \reg, [\addr]; > add \addr, \addr, \post_inc; > > - _asm_extableb,\l; > + _asm_extable_uaccessb, \l; > .endm > #endif > -- > 2.25.1 >
