diff mbox series

[v3,20/20] linux-user: Add RISC-V zicfilp support in VDSO

Message ID 20240807000652.1417776-21-debug@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series riscv support for control flow integrity extensions | expand

Commit Message

Deepak Gupta Aug. 7, 2024, 12:06 a.m. UTC 
Add zicfilp support in VDSO. VDSO functions need lpad instruction
so that userspace could call this function when landing pad extension is
enabled. This solution only works when toolchain always use landing pad
label 1.

Otherwise, If extension is not enabled, lpad instructions will be lui
instructions with rd=x0 (which is nop). Prebuilt VDSO is still
compatible with RISC-V core w/o zicfilp extension.

Signed-off-by: Jim Shu <jim.shu@sifive.com>
Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 linux-user/riscv/vdso-64.so | Bin 3944 -> 4128 bytes
 linux-user/riscv/vdso.S     |  50 ++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

Comments

Richard Henderson Aug. 7, 2024, 3:41 a.m. UTC | #1 
On 8/7/24 10:06, Deepak Gupta wrote:
> Add zicfilp support in VDSO. VDSO functions need lpad instruction
> so that userspace could call this function when landing pad extension is
> enabled. This solution only works when toolchain always use landing pad
> label 1.

Well, no, the lpad insns *could* use imm=0.
Why would the toolchain always use label 1?

Much more explanation is required here.

> +/* GNU_PROPERTY_RISCV64_* macros from elf.h for use in asm code.  */
> +#define FEATURE_1_AND 0xc0000000
> +
> +#define GNU_PROPERTY(type, value)	\
> +  .section .note.gnu.property, "a";	\
> +  .p2align 3;                       \
> +  .word 4;                          \
> +  .word 16;                         \
> +  .word 5;                          \
> +  .asciz "GNU";                     \
> +  .word type;                       \
> +  .word 4;                          \
> +  .word value;                      \
> +  .word 0;                          \
> +  .text
> +
> +/* Add GNU property note with the supported features to all asm code
> +   where sysdep.h is included.  */
> +#undef __VALUE_FOR_FEATURE_1_AND
> +#if defined (__riscv_zicfilp) || defined (__riscv_zicfiss)
> +#  if defined (__riscv_zicfilp)
> +#    if defined (__riscv_zicfiss)

Why are you checking __riscv_* symbols, for when the toolchain
has these features enabled on the command-line?

This is the kind of feature you want enabled always.

> +#ifdef __riscv_zicfilp
> +# define LPAD       lpad 1
> +#else
> +# define LPAD
> +#endif

Here, especially, you should be using ".insn", not omitting the lpad insn.


r~
Deepak Gupta Aug. 7, 2024, 9 p.m. UTC | #2 
On Wed, Aug 07, 2024 at 01:41:37PM +1000, Richard Henderson wrote:
>On 8/7/24 10:06, Deepak Gupta wrote:
>>Add zicfilp support in VDSO. VDSO functions need lpad instruction
>>so that userspace could call this function when landing pad extension is
>>enabled. This solution only works when toolchain always use landing pad
>>label 1.
>
>Well, no, the lpad insns *could* use imm=0.
>Why would the toolchain always use label 1?
>
>Much more explanation is required here.

Sorry this is amiss on my end. label=1 is kind of experimental, dev enabling
to ensure that label checkings are working. Eventually label scheme would be
hash (20bit truncates) based label scheme. Hash calculated over function
prototype.

This is again chicken and an egg problem. Things have to make it to upstream
in toolchain and libc for this scheme to be usable.

For now, I am thinking of doing `lpad 0` (as you also hinted) for vDSO.
I hope that's fine.

>
>>+/* GNU_PROPERTY_RISCV64_* macros from elf.h for use in asm code.  */
>>+#define FEATURE_1_AND 0xc0000000
>>+
>>+#define GNU_PROPERTY(type, value)	\
>>+  .section .note.gnu.property, "a";	\
>>+  .p2align 3;                       \
>>+  .word 4;                          \
>>+  .word 16;                         \
>>+  .word 5;                          \
>>+  .asciz "GNU";                     \
>>+  .word type;                       \
>>+  .word 4;                          \
>>+  .word value;                      \
>>+  .word 0;                          \
>>+  .text
>>+
>>+/* Add GNU property note with the supported features to all asm code
>>+   where sysdep.h is included.  */
>>+#undef __VALUE_FOR_FEATURE_1_AND
>>+#if defined (__riscv_zicfilp) || defined (__riscv_zicfiss)
>>+#  if defined (__riscv_zicfilp)
>>+#    if defined (__riscv_zicfiss)
>
>Why are you checking __riscv_* symbols, for when the toolchain
>has these features enabled on the command-line?
>

Yes, you're right.

>This is the kind of feature you want enabled always.

Yes, you're right here as well.
It should be compiled into vDSO unconditionally.

Will do that.

>
>>+#ifdef __riscv_zicfilp
>>+# define LPAD       lpad 1
>>+#else
>>+# define LPAD
>>+#endif
>
>Here, especially, you should be using ".insn", not omitting the lpad insn.
>
>
>r~
>
diff mbox series

Patch

diff --git a/linux-user/riscv/vdso-64.so b/linux-user/riscv/vdso-64.so
index ae49f5b043b5941b9d304a056c2b50c185f413b0..cd7f2fa7bdb811af6be2dcc3fb9601b66e3d1c81 100755
GIT binary patch
delta 1345
zcmah}O=uHA6rRa8*`3W#v-vSWgUvy(QL#}%mV&j76iPuQBHAj2(ng|zmM)E!P>`g>
ziy+eQP!K%yD2kv2Jb7q5_;(V#<f8T>2zrPIp$E0T*_|nLFFu%g^L_K)%k2BfxBcts
zwSKzU%uIMvDl|QN=xp<WSxBkG7O6?t!5&mSxH{CqZapIS5in@ND0&^M9SwtYn2kCl
z8HHvbTIYee+1S|&oZsNl6@EhD=NK-I`TfVcovANRA6MVpdHd;BIWn<tz;C}Zg!f#o
zJIeOs$1M@)*Wc|0j<Y-<ig*^WdPuKL`0SmKqyiq##X<Z^OE9+jz3uoXh2tNAc{aFo
z1twr<QCRm}!4()%@Eu+GDUKoG>4}g4*$}@d(n<~qepB*LP!3)Siz)<!cU)L~kXC{}
zEqLOxKPmXG%f8cUD^<!G_?jY`yo4d|;kbMX*GF*m<L!yoO|MGplL+N?0uP|A=~d*e
zHVAQWSlx}&F5K8<AH_U!-zdg#{GVyuzc7Q_Vx?MIB6I?e-o>SSu5ui{`}Ri4l{qVG
z<))V_rE;ZO#UoHP&Zd{==Wom%v$EK`eUMXAv;*hV1Wm$<8dtCYs1tO{Mm~~-=ZGxa
z<BEpgVQ6uMk)*A4Qbe7gH5*}xprDP>jpj-e9%`|fX?zbQKeuJaBedlj?wn7H+zXnl
z+6N3On@wEY6ZY=Tcmf7X)L-B&?+<q+-wWQ|H=hOXuJ8}RyE}+CAdkP(XK2SI=J0*Q
z-CCqns+~DMS-yO9fgGs8S6}+Sm1w<YuP7ab3>M^(KWxa1Nj(DZ`~!MYOa>phK%U8T
zbfFM1nH*fK8zMQjS!g4|p|!;V8Z`BtS@y!IU|yFKn)JeIFwbQ2i_i|5tR_lP0~#_7
pnM$drU_4|p`G+?Pw?igvKsz+7r*-ESGZggRJRA2r@IJ6$-#>;G!0Z43

delta 1236
zcmZux&1(};5TBP#+z*pYV+<*_)IAj1YN_?B+bY<^rnX2aRgfZHEGbA2QZyn~#MXa6
z69zo>Q1McbLL?%ocu2)V4jv*F!CQ|xnnMLet@AeXqAU))otfYJy|4M$HK*Q{?-jj;
zzFD!24I_@VKv0a~RwP*{I_d3w;EB@E*7O6Uf;7sa>HGB{<AWFz$(R#rvRWEP#3-gj
z=kh_C&}d9dUxAJB?DWLN7i-UF5AA;Y{Ap&b@ba}>XUh-Cou=~6m1b2gB-#DFx9A!2
zLL__`q}b;tKwVy%#A+&d0)Qt2<9$E(n(OP#|HVGj;Vb(!Jnn^O9`m|=HV73ypSJ_~
z<O2|fJRcb5i6e}!D;#fWJy$=lXD}<ltX0Kge2VdkkAIHwS3Z8Z)X;Lmd_cFEA<8=5
z3>{6VtH|v2)99wz{?bNB7jqeM)ifG;D@Xo~6$*{frvJ5_f9#bOCr+W3+&Ha4qi9He
z`aFGZFXXa!K@5`_!U4;{c`Jrnfi7ItJ4G3v>4^@ll@B7dM5F9h@S~p4LQq9vBn42^
z6PgYw(n%q6kkCx1d)fjA=g8j=lUShHyjQ?)jZ>c0vk;|y1vK_lb*f|98Q<a9Dg1<I
z(5|Y4cw(IS{)}HEJVyHiUb5rMGWY&0-6zKXYT_~D{_L$X?yrs_<E<JZU&?zLa(}9t
z4i8YNglu-<^o7ju<$*=$zE^r^y%V@%J9s_Z7E|F+dJrFlk6Efc>H&Nc9x~NiEBHO^
znyS~TI1+KqRtw@1d8*G+xEXP+8h24Gh(97jmTIbc5YN~{ri!eCOSWrHa-1h|({^L3
qZ<NlUh`Ofw^Ne9S>WX$;ijJCP(|ap?q2JVD+=;fE1#ar668QsJ{<Wt7

diff --git a/linux-user/riscv/vdso.S b/linux-user/riscv/vdso.S
index c37275233a..d0817c58ce 100644
--- a/linux-user/riscv/vdso.S
+++ b/linux-user/riscv/vdso.S
@@ -14,6 +14,52 @@ 
 #endif
 #include "vdso-asmoffset.h"
 
+/* GNU_PROPERTY_RISCV64_* macros from elf.h for use in asm code.  */
+#define FEATURE_1_AND 0xc0000000
+
+#define GNU_PROPERTY(type, value)	\
+  .section .note.gnu.property, "a";	\
+  .p2align 3;                       \
+  .word 4;                          \
+  .word 16;                         \
+  .word 5;                          \
+  .asciz "GNU";                     \
+  .word type;                       \
+  .word 4;                          \
+  .word value;                      \
+  .word 0;                          \
+  .text
+
+/* Add GNU property note with the supported features to all asm code
+   where sysdep.h is included.  */
+#undef __VALUE_FOR_FEATURE_1_AND
+#if defined (__riscv_zicfilp) || defined (__riscv_zicfiss)
+#  if defined (__riscv_zicfilp)
+#    if defined (__riscv_zicfiss)
+#      define __VALUE_FOR_FEATURE_1_AND 0x3
+#    else
+#      define __VALUE_FOR_FEATURE_1_AND 0x1
+#    endif
+#  else
+#    if defined (__riscv_zicfiss)
+#      define __VALUE_FOR_FEATURE_1_AND 0x2
+#    else
+#      error "What?"
+#    endif
+#  endif
+#endif
+
+#if defined (__VALUE_FOR_FEATURE_1_AND)
+GNU_PROPERTY (FEATURE_1_AND, __VALUE_FOR_FEATURE_1_AND)
+#endif
+#undef __VALUE_FOR_FEATURE_1_AND
+
+#ifdef __riscv_zicfilp
+# define LPAD       lpad 1
+#else
+# define LPAD
+#endif
+
 	.text
 
 .macro endf name
@@ -29,6 +75,7 @@ 
 
 .macro vdso_syscall name, nr
 \name:
+	LPAD
 	raw_syscall \nr
 	ret
 endf	\name
@@ -36,6 +83,7 @@  endf	\name
 
 __vdso_gettimeofday:
 	.cfi_startproc
+	LPAD
 #ifdef __NR_gettimeofday
 	raw_syscall __NR_gettimeofday
 	ret
@@ -86,6 +134,7 @@  vdso_syscall __vdso_getcpu, __NR_getcpu
 
 __vdso_flush_icache:
 	/* qemu does not need to flush the icache */
+	LPAD
 	li	a0, 0
 	ret
 endf __vdso_flush_icache
@@ -181,6 +230,7 @@  endf __vdso_flush_icache
 	nop
 
 __vdso_rt_sigreturn:
+	LPAD
 	raw_syscall __NR_rt_sigreturn
 endf __vdso_rt_sigreturn