On 03/04/2024 1:51 pm, Jan Beulich wrote: > On 03.04.2024 14:03, Juergen Gross wrote: >> ASM_INT() is defined in arch/[arm|x86]/include/asm/asm_defns.h in >> exactly the same way. Instead of replicating this definition for riscv >> and ppc, move it to include/xen/linkage.h, where other arch agnostic >> definitions for assembler code are living already. > And this is why I didn't make a change right away, back when noticing the > duplication: Arch-agnostic really means ... > >> --- a/xen/include/xen/linkage.h >> +++ b/xen/include/xen/linkage.h >> @@ -60,6 +60,8 @@ >> #define DATA_LOCAL(name, align...) \ >> SYM(name, DATA, LOCAL, LASTARG(DATA_ALIGN, ## align), DATA_FILL) >> >> +#define ASM_INT(label, val) DATA(label, 4) .long (val); END(label) > ... to avoid .long [1]. There's no arch-independent aspect guaranteeing > that what .long emits matches "unsigned int" as used e.g. in the > declaration of xen_config_data_size.
I'd forgotten that point, but I don't think it's a good reason force every architecture to implement the same thing. Borrowing a trick from the alternatives, what about this as a sanity check? diff --git a/xen/tools/binfile b/xen/tools/binfile index 0299326ccc3f..21593debc872 100755 --- a/xen/tools/binfile +++ b/xen/tools/binfile @@ -35,4 +35,10 @@ DATA($varname, 1 << $align) END($varname) ASM_INT(${varname}_size, .Lend - $varname) +.Lsize_end: + + .section .discard + # Build assert sizeof(ASM_INT) == 4 + .byte 0xff - ((.Lsize_end - ${varname}_size) == 4) + EOF Ideally we'd want BYTES_PER_INT here but it turns out that doesn't exist in Xen. If we find an architecture where .long isn't the right thing, we can make ASM_INT optionally arch-specific. ~Andrew