Re: [PATCH v3] riscv: asm: use .insn for making custom instructions

[Nathan Chancellor <nathan@kernel.org>]

Hey Ben,

On Fri, Oct 24, 2025 at 06:16:40PM +0100, Ben Dooks wrote:
> The assembler has .insn for building custom instructions
> now, so change the .4byte to .insn. This ensures the output
> is marked as an instruction and not as data which may
> confuse both debuggers and anything else that relies on
> this sort of marking.
> 
> Add an ASM_INSN_I() wrapper in asm.h to allow the selecting
> of how this is output so older assemblers are still good.
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> v2:
>   - fixed #ifndef v #ifdef
>   - added custom MIPS instructions too
>   - new patch description
> v3:
>   - fixed typo missed in compile test of NOP4
>   - fixed incorrect MIPS changes
>   - re-wored slightly to shorten sentences and remove typos
> ---
>  arch/riscv/include/asm/asm.h                    | 6 ++++++
>  arch/riscv/include/asm/insn-def.h               | 8 ++++----
>  arch/riscv/include/asm/vendor_extensions/mips.h | 6 +++---
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
> index 8bd2a11382a3..2d97436999ca 100644
> --- a/arch/riscv/include/asm/asm.h
> +++ b/arch/riscv/include/asm/asm.h
> @@ -12,6 +12,12 @@
>  #define __ASM_STR(x)	#x
>  #endif
>  
> +#ifdef CONFIG_AS_HAS_INSN

The current AS_HAS_INSN check in Kconfig is not sufficient for this
usage. With LLVM prior to [1] (i.e., older than 19) or GNU binutils
prior to [2] (i.e., older than 2.38), the build breaks:

  In file included from <built-in>:4:
  In file included from lib/vdso/gettimeofday.c:6:
  In file included from include/vdso/datapage.h:21:
  In file included from include/vdso/processor.h:10:
  arch/riscv/include/asm/vdso/processor.h:23:2: error: expected instruction format
     23 |         ALT_RISCV_PAUSE();
        |         ^
  arch/riscv/include/asm/errata_list.h:47:3: note: expanded from macro 'ALT_RISCV_PAUSE'
     47 |                 RISCV_PAUSE, /* Original RISC‑V pause insn */ \
        |                 ^
  arch/riscv/include/asm/insn-def.h:259:21: note: expanded from macro 'RISCV_PAUSE'
    259 | #define RISCV_PAUSE     ASM_INSN_I("0x100000f")
        |                         ^
  arch/riscv/include/asm/asm.h:16:26: note: expanded from macro 'ASM_INSN_I'
     16 | #define ASM_INSN_I(__x) ".insn " __x
        |                          ^
  <inline asm>:5:7: note: instantiated into assembly here
      5 | .insn 0x100000f
        |       ^

  arch/riscv/include/asm/vdso/processor.h: Assembler messages:
  arch/riscv/include/asm/vdso/processor.h:27: Error: unrecognized opcode `0x100000f'
  arch/riscv/include/asm/vdso/processor.h:27: Error: unrecognized opcode `0x100000f'
  arch/riscv/include/asm/vdso/processor.h:27: Error: unrecognized opcode `0x100000f'
  arch/riscv/include/asm/vdso/processor.h:27: Error: unrecognized opcode `0x100000f'
  make[4]: *** [scripts/Makefile.build:287: arch/riscv/kernel/vdso/vgettimeofday.o] Error 1

Maybe this wants a new symbol like CONFIG_AS_HAS_INSN_EXT that checks
for this support?

[1]: https://github.com/llvm/llvm-project/commit/2a086dce691e3cc34a2fc27f4fb255bb2cbbfac9
[2]: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a262b82fdbf4cda3b0648b1adc32245ca3f78b7a

Cheers,
Nathan

> +#define ASM_INSN_I(__x) ".insn " __x
> +#else
> +#define ASM_INSN_I(__x) ".4byte " __x
> +#endif
> +
>  #if __riscv_xlen == 64
>  #define __REG_SEL(a, b)	__ASM_STR(a)
>  #elif __riscv_xlen == 32
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index c9cfcea52cbb..d29da6ccd3dd 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -256,10 +256,10 @@
>  	INSN_S(OPCODE_OP_IMM, FUNC3(6), __RS2(3),		\
>  	       SIMM12((offset) & 0xfe0), RS1(base))
>  
> -#define RISCV_PAUSE	".4byte 0x100000f"
> -#define ZAWRS_WRS_NTO	".4byte 0x00d00073"
> -#define ZAWRS_WRS_STO	".4byte 0x01d00073"
> -#define RISCV_NOP4	".4byte 0x00000013"
> +#define RISCV_PAUSE	ASM_INSN_I("0x100000f")
> +#define ZAWRS_WRS_NTO	ASM_INSN_I("0x00d00073")
> +#define ZAWRS_WRS_STO	ASM_INSN_I("0x01d00073")
> +#define RISCV_NOP4	ASM_INSN_I("0x00000013")
>  
>  #define RISCV_INSN_NOP4	_AC(0x00000013, U)
>  
> diff --git a/arch/riscv/include/asm/vendor_extensions/mips.h b/arch/riscv/include/asm/vendor_extensions/mips.h
> index ea8ca747d691..ffeb12dc17a3 100644
> --- a/arch/riscv/include/asm/vendor_extensions/mips.h
> +++ b/arch/riscv/include/asm/vendor_extensions/mips.h
> @@ -30,8 +30,8 @@ extern struct riscv_isa_vendor_ext_data_list riscv_isa_vendor_ext_list_mips;
>   * allowing any subsequent instructions to fetch.
>   */
>  
> -#define MIPS_PAUSE	".4byte 0x00501013\n\t"
> -#define MIPS_EHB	".4byte 0x00301013\n\t"
> -#define MIPS_IHB	".4byte 0x00101013\n\t"
> +#define MIPS_PAUSE	ASM_INSN_I("0x00501013\n\t")
> +#define MIPS_EHB	ASM_INSN_I("0x00301013\n\t")
> +#define MIPS_IHB	ASM_INSN_I("0x00101013\n\t")
>  
>  #endif // _ASM_RISCV_VENDOR_EXTENSIONS_MIPS_H
> -- 
> 2.37.2.352.g3c44437643
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv