Re: [PATCH v5 5/8] RISC-V: annotate entry points with type and size

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2024-01-15 at 15:37 +0100, Jan Beulich wrote:
> Use the generic framework in xen/linkage.h. No change in generated
> code
> except of course the converted symbols change to be hidden ones and
> gain
> a valid size.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Probably count_args_exp() should move to macros.h, but I first wanted
> to
> see whether anyone can suggest any better approach for checking
> whether
> a defined macro expands to nothing.
> ---
The current one approach looks good to me.
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

~ Oleksii

> v5: Re-base.
> v4: Also drop #undef-s from linker script.
> v3: New.
> 
> --- a/xen/arch/riscv/entry.S
> +++ b/xen/arch/riscv/entry.S
> @@ -5,7 +5,7 @@
>  #include <asm/traps.h>
>  
>  /* WIP: only works while interrupting Xen context */
> -ENTRY(handle_trap)
> +FUNC(handle_trap)
>  
>      /* Exceptions from xen */
>  save_to_stack:
> @@ -92,3 +92,4 @@ restore_registers:
>          REG_L   sp, CPU_USER_REGS_SP(sp)
>  
>          sret
> +END(handle_trap)
> --- a/xen/arch/riscv/include/asm/asm.h
> +++ b/xen/arch/riscv/include/asm/asm.h
> @@ -7,6 +7,7 @@
>  #define _ASM_RISCV_ASM_H
>  
>  #ifdef __ASSEMBLY__
> +#include <xen/linkage.h>
>  #define __ASM_STR(x) x
>  #else
>  #define __ASM_STR(x) #x
> --- a/xen/arch/riscv/include/asm/config.h
> +++ b/xen/arch/riscv/include/asm/config.h
> @@ -69,12 +69,8 @@
>  
>  /* Linkage for RISCV */
>  #ifdef __ASSEMBLY__
> -#define ALIGN .align 4
> -
> -#define ENTRY(name)                                \
> -  .globl name;                                     \
> -  ALIGN;                                           \
> -  name:
> +#define CODE_ALIGN 16
> +#define CODE_FILL /* empty */
>  #endif
>  
>  #ifdef CONFIG_RISCV_64
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -8,7 +8,7 @@
>           *   a0 -> hart_id ( bootcpu_id )
>           *   a1 -> dtb_base 
>           */
> -ENTRY(start)
> +FUNC(start)
>          /* Mask all interrupts */
>          csrw    CSR_SIE, zero
>  
> @@ -60,19 +60,21 @@ ENTRY(start)
>          mv      a1, s1
>  
>          tail    start_xen
> +END(start)
>  
>          .section .text, "ax", %progbits
>  
> -ENTRY(reset_stack)
> +FUNC(reset_stack)
>          la      sp, cpu0_boot_stack
>          li      t0, STACK_SIZE
>          add     sp, sp, t0
>  
>          ret
> +END(reset_stack)
>  
>          .section .text.ident, "ax", %progbits
>  
> -ENTRY(turn_on_mmu)
> +FUNC(turn_on_mmu)
>          sfence.vma
>  
>          li      t0, RV_STAGE1_MODE
> @@ -84,3 +86,4 @@ ENTRY(turn_on_mmu)
>          csrw    CSR_SATP, t1
>  
>          jr      a0
> +END(turn_on_mmu)
> --- a/xen/arch/riscv/xen.lds.S
> +++ b/xen/arch/riscv/xen.lds.S
> @@ -1,9 +1,6 @@
>  #include <xen/lib.h>
>  #include <xen/xen.lds.h>
>  
> -#undef ENTRY
> -#undef ALIGN
> -
>  OUTPUT_ARCH(riscv)
>  ENTRY(start)
>  
> --- a/xen/include/xen/linkage.h
> +++ b/xen/include/xen/linkage.h
> @@ -35,17 +35,28 @@
>  
>  #define END(name) .size name, . - name
>  
> +/*
> + * CODE_FILL in particular may need to expand to nothing (e.g. for
> RISC-V), in
> + * which case we also need to get rid of the comma in the .balign
> directive.
> + */
> +#define count_args_exp(args...) count_args(args)
> +#if count_args_exp(CODE_FILL)
> +# define DO_CODE_ALIGN(align...) LASTARG(CODE_ALIGN, ## align),
> CODE_FILL
> +#else
> +# define DO_CODE_ALIGN(align...) LASTARG(CODE_ALIGN, ## align)
> +#endif
> +
>  #define FUNC(name, align...) \
> -        SYM(name, FUNC, GLOBAL, LASTARG(CODE_ALIGN, ## align),
> CODE_FILL)
> +        SYM(name, FUNC, GLOBAL, DO_CODE_ALIGN(align))
>  #define LABEL(name, align...) \
> -        SYM(name, NONE, GLOBAL, LASTARG(CODE_ALIGN, ## align),
> CODE_FILL)
> +        SYM(name, NONE, GLOBAL, DO_CODE_ALIGN(align))
>  #define DATA(name, align...) \
>          SYM(name, DATA, GLOBAL, LASTARG(DATA_ALIGN, ## align),
> DATA_FILL)
>  
>  #define FUNC_LOCAL(name, align...) \
> -        SYM(name, FUNC, LOCAL, LASTARG(CODE_ALIGN, ## align),
> CODE_FILL)
> +        SYM(name, FUNC, LOCAL, DO_CODE_ALIGN(align))
>  #define LABEL_LOCAL(name, align...) \
> -        SYM(name, NONE, LOCAL, LASTARG(CODE_ALIGN, ## align),
> CODE_FILL)
> +        SYM(name, NONE, LOCAL, DO_CODE_ALIGN(align))
>  #define DATA_LOCAL(name, align...) \
>          SYM(name, DATA, LOCAL, LASTARG(DATA_ALIGN, ## align),
> DATA_FILL)
>  
>