Re: [PATCH v5 1/8] common: assembly entry point type/size annotations

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jan 15, 2024 at 03:34:05PM +0100, Jan Beulich wrote:
> Recent gas versions generate minimalistic Dwarf debug info for items
> annotated as functions and having their sizes specified [1]. Furthermore
> generating live patches wants items properly annotated. "Borrow" Arm's
> END() and (remotely) derive other annotation infrastructure from
> Linux'es, for all architectures to use.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> 
> [1] 
> https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=591cc9fbbfd6d51131c0f1d4a92e7893edcc7a28
> ---
> v5: Drop CODE_ALIGN fallback. s/algn/align/g.
> v3: New, generalized from earlier x86-only version. LAST() (now
>     LASTARG()) moved to macros.h.
> ---
> TBD: {CODE,DATA}_ALIGN are byte granular, such that a value of 0 can be
>      specified (in case this has some special meaning on an arch;
>      conceivably it could mean to use some kind of arch default). We may
>      not strictly need that, and hence we could also make these power-
>      of-2 values (using .p2align).
> 
> Note that we can't use ALIGN() (in place of SYM_ALIGN()) as long as we
> still have ALIGN.

SYM_ALIGN seems fine for the purpose here.

> 
> Note further that FUNC()'s etc "align" parameter is intended to allow
> for only no or a single argument. If we wanted to also make the fill
> value customizable per call site, the constructs would need re-doing to
> some degree.
> 
> --- /dev/null
> +++ b/xen/include/xen/linkage.h
> @@ -0,0 +1,54 @@
> +#ifndef __LINKAGE_H__
> +#define __LINKAGE_H__
> +
> +#ifdef __ASSEMBLY__
> +
> +#include <xen/macros.h>
> +
> +/* CODE_ALIGN needs to be specified by every architecture. */
> +#ifndef CODE_FILL
> +# define CODE_FILL ~0
> +#endif
> +
> +#ifndef DATA_ALIGN
> +# define DATA_ALIGN 0
> +#endif
> +#ifndef DATA_FILL
> +# define DATA_FILL ~0

I find the fills a bit odd, compared to what we use now (nops for x86
IIRC).

Thanks, Roger.