Re: [PATCH 2/2] powerpc/vdso: Implement __arch_get_vdso_rng_data()

From: Christophe Leroy
Date: Sat Oct 12 2024 - 05:19:44 EST

Next message: Yanteng Si: "Re: [PATCH v4] Docs/zh_CN: Translate page_tables.rst to Simplified Chinese"
Previous message: Lizhi Xu: "Re: [usb?] WARNING: ODEBUG bug in corrupted (3)"
In reply to: Michael Ellerman: "Re: [PATCH 2/2] powerpc/vdso: Implement __arch_get_vdso_rng_data()"
Next in thread: Michael Ellerman: "Re: [PATCH 2/2] powerpc/vdso: Implement __arch_get_vdso_rng_data()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Le 11/10/2024 à 13:46, Michael Ellerman a écrit :

Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:

Le 10/10/2024 à 11:12, Thomas Weißschuh a écrit :

I'll try to see why this doesn't work for ppc32.

PC-rel instructions only exist on very very recent powerpc CPUs (power10 ?)

Yeah power10 or later.

On PPC64, ELF ABI v2 requires caller to put called function address in
r12 and it looks like GCC uses that:

0000000000000000 <__c_kernel_getrandom>:
0: 3c 4c 00 00 addis r2,r12,0
2: R_PPC64_REL16_HA .TOC.+0x2
4: 38 42 00 00 addi r2,r2,0
6: R_PPC64_REL16_LO .TOC.+0x6
...
64: 3d 22 00 00 addis r9,r2,0
66: R_PPC64_TOC16_HA _vdso_datapage+0x100
68: 89 29 00 00 lbz r9,0(r9)
6a: R_PPC64_TOC16_LO _vdso_datapage+0x100

Setting up r12 is only required for calls to the global entry point
(offset 0), local calls can be made to offset 8 and use/don't require
r12 to be set. That's because local calls should already have the
correct toc pointer in r2.

But that's not true in VDSO code. >

Which after final link results in:

0000000000001060 <__c_kernel_getrandom>:
1060: 3c 4c 00 01 addis r2,r12,1
1064: 38 42 8e a0 addi r2,r2,-29024
...
10c4: 3d 22 ff fc addis r9,r2,-4
10c8: 89 29 62 00 lbz r9,25088(r9)

The call to __c_kernel_getrandom skips over the r2 setup because it's a
local call, even though we haven't setup r2 correctly:

Yes indeed I forgot that. So even if the final check doesn't complain, it won't work at the end.

Don't know if we could find a way to detect that and fail the build.

0000000000000758 <__kernel_getrandom>:
758: 91 ff 21 f8 stdu r1,-112(r1)
75c: a6 02 08 7c mflr r0
760: 91 ff 21 f8 stdu r1,-112(r1)
764: 80 00 01 f8 std r0,128(r1)
768: 88 00 41 f8 std r2,136(r1)
76c: 05 00 9f 42 bcl 20,4*cr7+so,770 <__kernel_getrandom+0x18>
770: a6 02 08 7d mflr r8
774: fe ff 08 3d addis r8,r8,-2
778: 90 f8 08 39 addi r8,r8,-1904
77c: fc 00 68 81 lwz r11,252(r8)
780: ff 7f 6b 6d xoris r11,r11,32767
784: ff ff 6b 69 xori r11,r11,65535
788: 34 00 6b 7d cntlzw r11,r11
78c: de 5b 6b 55 rlwinm r11,r11,11,15,15
790: 14 5a 08 7d add r8,r8,r11
794: d8 02 08 39 addi r8,r8,728
798: 41 09 00 48 bl 10d8 <__c_kernel_getrandom+0x8>

We could setup r2, but that would only help 64-bit.

This also makes me notice that we have a mixture of ELF ABI v1 and v2
code in the VDSO, depending on whether the kernel is building itself ABI
v1 or v2:

arch/powerpc/kernel/vdso/cacheflush-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/datapage-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/getcpu-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/getrandom-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/gettimeofday-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/note-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/sigtramp64-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/vgetrandom-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, OpenPOWER ELF V2 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/vgetrandom-chacha-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/kernel/vdso/vgettimeofday-64.o: ELF 64-bit LSB relocatable, 64-bit PowerPC or cisco 7500, OpenPOWER ELF V2 ABI, version 1 (SYSV), not stripped

All the asm files are ABI v1 because they historically were, and don't
say otherwise. The C code comes out as ABI v1 or v2 depending on what
we're building the kernel as. Which is a bit fishy.

That's not related to VDSO it seems. There is the same thing in arch/powerpc/lib for instance:

$ file arch/powerpc/lib/*.o
arch/powerpc/lib/checksum_64.o: ELF 64-bit MSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/lib/checksum_wrappers.o: ELF 64-bit MSB relocatable, 64-bit PowerPC or cisco 7500, OpenPOWER ELF V2 ABI, version 1 (SYSV), not stripped
arch/powerpc/lib/code-patching.o: ELF 64-bit MSB relocatable, 64-bit PowerPC or cisco 7500, OpenPOWER ELF V2 ABI, version 1 (SYSV), not stripped
arch/powerpc/lib/copy_mc_64.o: ELF 64-bit MSB relocatable, 64-bit PowerPC or cisco 7500, Unspecified or Power ELF V1 ABI, version 1 (SYSV), not stripped
arch/powerpc/lib/copypage_64.o: ELF 64-bit MSB relocatable, 64-bit PowerPC or cisco 7500, OpenPOWER ELF V2 ABI, version 1 (SYSV), not stripped
...

Seems like all .c files result in a ELF V2 while some of .S files are V1 et some are V2. That's odd because the build arguments seems to be the same:

# AS arch/powerpc/lib/checksum_64.o
powerpc64-linux-gcc -Wp,-MMD,arch/powerpc/lib/.checksum_64.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -D__KERNEL__ -I ./arch/powerpc -DHAVE_AS_ATHIGH=1 -D__ASSEMBLY__ -fno-PIE -m64 -mcpu=power8 -mabi=elfv2 -mlittle-endian -Wa,--fatal-warnings -DKBUILD_MODFILE='"arch/powerpc/lib/checksum_64"' -DKBUILD_MODNAME='"checksum_64"' -D__KBUILD_MODNAME=kmod_checksum_64 -c -o arch/powerpc/lib/checksum_64.o arch/powerpc/lib/checksum_64.S ; ./tools/objtool/objtool --mcount arch/powerpc/lib/checksum_64.o

# AS arch/powerpc/lib/copypage_64.o
powerpc64-linux-gcc -Wp,-MMD,arch/powerpc/lib/.copypage_64.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -D__KERNEL__ -I ./arch/powerpc -DHAVE_AS_ATHIGH=1 -D__ASSEMBLY__ -fno-PIE -m64 -mcpu=power8 -mabi=elfv2 -mlittle-endian -Wa,--fatal-warnings -DKBUILD_MODFILE='"arch/powerpc/lib/copypage_64"' -DKBUILD_MODNAME='"copypage_64"' -D__KBUILD_MODNAME=kmod_copypage_64 -c -o arch/powerpc/lib/copypage_64.o arch/powerpc/lib/copypage_64.S ; ./tools/objtool/objtool --mcount arch/powerpc/lib/copypage_64.o

Next message: Yanteng Si: "Re: [PATCH v4] Docs/zh_CN: Translate page_tables.rst to Simplified Chinese"
Previous message: Lizhi Xu: "Re: [usb?] WARNING: ODEBUG bug in corrupted (3)"
In reply to: Michael Ellerman: "Re: [PATCH 2/2] powerpc/vdso: Implement __arch_get_vdso_rng_data()"
Next in thread: Michael Ellerman: "Re: [PATCH 2/2] powerpc/vdso: Implement __arch_get_vdso_rng_data()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]