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.
--- 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 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
© 2016 - 2026 Red Hat, Inc.