Messages in this thread | | | Date | Mon, 20 Jun 2022 22:13:41 +0800 | Subject | Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO | From | Tong Tiangen <> |
| |
在 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 写道: >>> 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/ > | CALL scripts/atomic/check-atomics.sh > | CC arch/arm64/kernel/asm-offsets.s > | CALL scripts/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: > | > | 0000000000000000 <__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 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. > >> "<<" is invalid operands in assembly, is there something wrong with me? > > At the moment I can only assume there is a local problem. I'd suspect a typo > somewhere, but maybe you have a toolchain which behaves differently? > > Thanks, > Mark. > .
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.
Thans, Tong.
| |