On 03.04.2024 15:59, Andrew Cooper wrote:
> 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.
Of course.
> 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
Hmm, tools/binfile may not be involved in a build, yet ASM_INT() may
still be used. Since there may not be any good place, I think we're
okay-ish for now without such a check.
> 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.
We don't even need to go this far - merely introducing an abstraction
for .long would suffice, and then also allow using that in bug.h.
Jan
