| Message ID | 959bdb6d-9b6c-cde0-9459-c83cd3f58b18@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | annotate entry points with type and size | expand |
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@suse.com> 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. > --- > 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) > > >
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@suse.com> > > 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) > >
On 26.07.2023 17:28, Oleksii wrote: > 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@suse.com> >> >> 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 I don't see how this would be used in your scheme. It ... > 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, ... certainly isn't here, or this likely would cause at least a warning from the compiler (for there being too few arguments to printf()) and then a runtime UB for interpreting something as a pointer to a string which likely isn't. > return 0; > } > > and then define DO_CODE_ALIGN using CONDITIONAL_RETURN? Afaict instead of getting rid of the comma, you'd actually add "" after it. What am I missing? Jan
On Wed, 2023-07-26 at 17:43 +0200, Jan Beulich wrote: > On 26.07.2023 17:28, Oleksii wrote: > > 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@suse.com> > > > > > > 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 > > I don't see how this would be used in your scheme. It ... > > > 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, > > ... certainly isn't here, or this likely would cause at least a > warning > from the compiler (for there being too few arguments to printf()) and > then a runtime UB for interpreting something as a pointer to a string > which likely isn't. > > > return 0; > > } > > > > and then define DO_CODE_ALIGN using CONDITIONAL_RETURN? > > Afaict instead of getting rid of the comma, you'd actually add "" > after it. What am I missing? You are right. I missed that actually it returns "". ~ Oleksii
--- 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)
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@suse.com> --- 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. --- v3: New.