[PATCH RFC v2] RISCV: insert zimop instruction at the start

[Deepak Gupta]

On Fri, Jun 13, 2025 at 11:23:57AM -0400, enh wrote:
>On Fri, Jun 13, 2025 at 12:42 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> RVA23 mandates that CPU must implement zimop and zcmop extensions. These
>> extensions are termed as "may be operations" to signify that some instructions
>> might be leveraged by some future extensions to turn into different operations.
>> Unless those extensions are used for codegen, there is no way to ensure that
>> compiled software has zimop/zcmop instruction in them. Without any software
>> using these instructions, there is no way to ensure that target hardware is
>> RVA23 compatible.
>>
>> zicfiss extension converts some of the zimop instructions into shadow stack
>> instructions. Support for shadow stack is still in the flux. This patch ensures
>> that if compiler supports shadow stack codegen then sspush/sspopchk
>> instructions are at the start when userspace starts life. Kernel doesn't enable
>> shadow stack for userspace by default. Userspace will issue shadow stack enable
>> prtctls to kernel. Thus always these (sspush/sspopchk) instructions will
>> default to zimop behavior.
>>
>> Note that this is not a backward hardware compatible change.
>>
>> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> ---
>> RVA23 mandates that CPU must implement zimop and zcmop extensions. These
>> extensions are termed as "may be operations" to signify that some instructions
>> might be leveraged by some future extensions to turn into different operations.
>> Unless those extensions are used for codegen, there is no way to ensure that
>> compiled software has zimop/zcmop instruction in them. Without any software
>> using these instructions, there is no way to ensure that target hardware is
>> RVA23 compatible.
>>
>> zicfiss extension converts some of the zimop instructions into shadow stack
>> instructions. Support for shadow stack is still in the flux. This patch ensures
>> that if compiler supports shadow stack codegen then sspush/sspopchk
>> instructions are at the start when userspace starts life. Kernel doesn't enable
>> shadow stack for userspace by default. Userspace will issue shadow stack enable
>> prctls to kernel. Thus always these (sspush/sspopchk) instructions will
>> default to zimop behavior.
>>
>> Note that this is not a backward hardware compatible change.
>> ---
>> Changes in v2:
>> - changing sspop to c.sspopchk. sspopchk is the instruction. making it
>>   c.sspopchk to check for zcmop.
>>
>> - Link to v1: https://inbox.sourceware.org/20250612-glibc_zimop-v1-1-88f15a37d233@rivosinc.com
>> ---
>>  sysdeps/riscv/dl-machine.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
>> index dcc3e0883b..9870320cf0 100644
>> --- a/sysdeps/riscv/dl-machine.h
>> +++ b/sysdeps/riscv/dl-machine.h
>> @@ -53,6 +53,15 @@
>>       || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64)))  \
>>     | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
>>
>> +
>> +#ifdef __riscv_zicfiss
>
>doesn't the #ifdef make this useless? you'll only emit these
>instructions if you've enabled ss in your build (and thus presumably
>emitting them for real), so you're not really testing the "maybe" side
>of things at all?

Yes good point.
Facepalm on me :(. Yes if user doesn't compile with -fcf-protection=return
then it won't be emitted. I wanted to initiate a discussion. That's why
kept it as RFC.

>
>don't you actually want to have a couple of unconditional `.inst`s for
>any build claiming zimop/zicmop instead?

We can do that but it'll start breaking current users and hardware. So it has
to hide behind some compile condition at configure time. It can't be
`__riscv_zicfiss` because I am treating this change as insurance. If
`__riscv_zicfiss` shows up in 2.43 glibc release we won't need this change.

>
>and even if you do that, is this a good place to do it? crashes here
>tend to be hard to interpret/debug, especially without a SIGILL signal
>handler to say "hey, you're running a build that requires zimop on a
>non-zimop device".

I imagine whoever is doing their software release would test it and catch it
on their hardware. Intention is to break as early as possible.

>
>does __riscv_hwprobe() for zimop make more sense?

I think this is another path to take. However kernel also doesn't test for
the instruction, all it looks for is ISA string.

>
>(full disclosure: i have a todo to do something like this for bionic,
>but haven't really worked out _what_ exactly. and i think the bionic
>case is easier because we have no intention of supporting pre-zimop
>devices, whereas i assume glibc wants to support everything? i guess
>that's the real question before answering any of the rest: "what exact
>set of combinations does glibc intend to support [and how is that
>going to work in terms of being able to roll out ss if zimop can't be
>assumed]?")

glibc consumers are many. I imagine distros wouldn't want to support
two different builds (one RVA23 compatible and one prior to RVA23).

I can imagine that glibc would want these instructions in codegen on
configure conditional basis. So that condition could be either

fcf-protection=full or fcf-protection=return

OR

If `zicfiss` doesn't make in time into glibc then we would need some
configure option like --with-arch=rv64gc_zimop_zcmop which would define
`__riscv_zimop` and `__riscv_zcmop` and then based on that it could be
included codegen at loader start.

>
>> +#define INSERT_ZIMOP \
>> +     sspush x1       \
>> +     c.sspopchk  x1
>> +#else
>> +#define INSERT_ZIMOP
>> +#endif
>> +
>>  /* Return nonzero iff ELF header is compatible with the running host.  */
>>  static inline int __attribute_used__
>>  elf_machine_matches_host (const ElfW(Ehdr) *ehdr)
>> @@ -102,6 +111,7 @@ elf_machine_dynamic (void)
>>  #define RTLD_START asm (\
>>         ".text\n\
>>         " _RTLD_PROLOGUE (ENTRY_POINT) "\
>> +        INSERT_ZIMOP \
>>         mv a0, sp\n\
>>         jal _dl_start\n\
>>         " _RTLD_PROLOGUE (_dl_start_user) "\
>>
>> ---
>> base-commit: 77930e0447e0b37a129db0e13c6c6f5e60a3019e
>> change-id: 20250612-glibc_zimop-480880581b2b
>> --
>> - debug
>>