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

[Richard Henderson]

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~