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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-07-10 at 10:58 +0200, Jan Beulich wrote:
> On 10.07.2023 10:56, 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>
> 
> I'm sorry, the Cc list was incomplete here. Adding back the remaining
> REST
> maintainers.
> 
> Jan
> 
> > ---
> > 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.
What about introduction of conditional macros ?
Something similar to:
#include <stdio.h>

#define CONDITIONAL_RETURN(arg1, arg2) CONDITIONAL_RETURN_IMPL(arg1,
arg2, EMPTY)

#define EMPTY(...) ""

#define CONDITIONAL_RETURN_IMPL(arg1, arg2, empty_check) \
    CONDITIONAL_RETURN_##empty_check(arg1, arg2)

#define CONDITIONAL_RETURN_EMPTY(arg1, arg2) \
    CONDITIONAL_RETURN_ARG1(arg1, arg2)

#define CONDITIONAL_RETURN_ARG1(arg1, arg2) arg1, arg2

#define CONDITIONAL_RETURN_ARG2(arg1, arg2) arg1

int main() {
    int a = 42;
    const char* b = "hello";

    // Second argument is not empty, both arguments are returned
    printf("Case 1: %d, %s\n", CONDITIONAL_RETURN(a, b));  // Prints:
Case 1: 42, hello

    // Second argument is empty, only the first argument is returned
    printf("Case 2: %d, %s\n", CONDITIONAL_RETURN(a, "")); // Prints:
Case 2: 42,

    return 0;
}

and then define DO_CODE_ALIGN using CONDITIONAL_RETURN?


~ Oleksii

> > ---
> > 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
> > @@ -67,12 +67,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
> >  
> > @@ -30,13 +30,14 @@ ENTRY(start)
> >          jal     reset_stack
> >  
> >          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)
> > --- a/xen/include/xen/linkage.h
> > +++ b/xen/include/xen/linkage.h
> > @@ -37,17 +37,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(algn...) LASTARG(CODE_ALIGN, ## algn),
> > CODE_FILL
> > +#else
> > +# define DO_CODE_ALIGN(algn...) LASTARG(CODE_ALIGN, ## algn)
> > +#endif
> > +
> >  #define FUNC(name, algn...) \
> > -        SYM(name, FUNC, GLOBAL, LASTARG(CODE_ALIGN, ## algn),
> > CODE_FILL)
> > +        SYM(name, FUNC, GLOBAL, DO_CODE_ALIGN(algn))
> >  #define LABEL(name, algn...) \
> > -        SYM(name, NONE, GLOBAL, LASTARG(CODE_ALIGN, ## algn),
> > CODE_FILL)
> > +        SYM(name, NONE, GLOBAL, DO_CODE_ALIGN(algn))
> >  #define DATA(name, algn...) \
> >          SYM(name, DATA, GLOBAL, LASTARG(DATA_ALIGN, ## algn),
> > DATA_FILL)
> >  
> >  #define FUNC_LOCAL(name, algn...) \
> > -        SYM(name, FUNC, LOCAL, LASTARG(CODE_ALIGN, ## algn),
> > CODE_FILL)
> > +        SYM(name, FUNC, LOCAL, DO_CODE_ALIGN(algn))
> >  #define LABEL_LOCAL(name, algn...) \
> > -        SYM(name, NONE, LOCAL, LASTARG(CODE_ALIGN, ## algn),
> > CODE_FILL)
> > +        SYM(name, NONE, LOCAL, DO_CODE_ALIGN(algn))
> >  #define DATA_LOCAL(name, algn...) \
> >          SYM(name, DATA, LOCAL, LASTARG(DATA_ALIGN, ## algn),
> > DATA_FILL)
> >