Re: [PATCH v9] common: honor CONFIG_CC_SPLIT_SECTIONS also for assembly functions

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-04-01 06:58, Jan Beulich wrote:
> Leverage the new infrastructure in xen/linkage.h to also switch to per-
> function sections (when configured), deriving the specific name from the
> "base" section in use at the time FUNC() is invoked.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Tested-by: Luca Fancellu <luca.fancellu@xxxxxxx> # arm
Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>

Though I have two suggestions below.

> ---
> TBD: Since we use .subsection in UNLIKELY_START(), a perhaps not really
>       wanted side effect of this change is that respective out-of-line
>       code now moves much closer to its original (invoking) code.
>
> TBD: Of course something with the same overall effect, but less
>       impactful might do in Config.mk. E.g. $(filter-out -D%,$(3))
>       instead of $(firstword (3)). In fact Roger wants the detection to
>       be in Kconfig, for LIVEPATCH to depend on it. Yet the whole
>       underlying discussion there imo would first need settling (and
>       therefore reviving).
>
> Note that we'd need to split DATA() in order to separate r/w, r/o, and
> BSS contributions. Further splitting might be needed to also support
> more advanced attributes (e.g. merge), hence why this isn't done right
> here. Sadly while a new section's name can be derived from the presently
> in use, its attributes cannot be. Perhaps the only thing we can do is
> give DATA() a 2nd mandatory parameter. Then again I guess most data
> definitions could be moved to C anyway.
> ---
> v9: Move Arm32 SYM_PUSH_SECTION() overrides here.
> v7: Override SYM_PUSH_SECTION() in arch/x86/indirect-thunk.S. Re-base,
>      notably to deal with fallout from fba250ae604e ("xen/arm64: head:
>      Add missing code symbol annotations").
> v6: Deal with x86'es entry_PF() and entry_int82() falling through to the
>      next "function". Re-base.
> v5: Re-base over changes earlier in the series.
> v4: Re-base.
> v2: Make detection properly fail on old gas (by adjusting
>      cc-option-add-closure).
>
> --- a/Config.mk
> +++ b/Config.mk
> @@ -102,7 +102,7 @@ cc-option = $(shell if $(1) $(2:-Wno-%=-
>   # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
Maybe expand to illustrate extra flags will also be passed 
(-DHAVE_AS_SECTNAME_SUBST)?
>   cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
>   define cc-option-add-closure
> -    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
> +    ifneq ($$(call cc-option,$$($(2)),$(firstword $(3)),n),n)
>           $(1) += $(3)
>       endif
>   endef
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -48,6 +48,13 @@
>   
>           .section .text.header, "ax", %progbits
>           .arm
> +/*
> + * Code below wants to all live in the section established above.  Annotations
> + * from xen/linkage.h therefore may not switch sections (honoring
> + * CONFIG_CC_SPLIT_SECTIONS).  Override the respective macro.
> + */
> +#undef SYM_PUSH_SECTION
> +#define SYM_PUSH_SECTION(name, attr)
I put this through CI and it passed as-is, so it doesn't need to change. 
 However, included in a different branch with some --gc-sections 
experiments, I needed to add SYM_PUSH_SECTION re-definitions like above to:
xen/arch/ppc/ppc64/head.S

or ppc failed the linker script
ASSERT(_stext_exceptions == EXCEPTION_VECTORS_START);

And these:

xen/arch/riscv/riscv64/head.S
xen/arch/arm/arm64/mmu/head.S

riscv and arm64 built, but hung when booting in CI tests.

There are also these:
xen/arch/arm/arm32/mpu/head.S
xen/arch/arm/arm64/mpu/head.S

They aren't built or tested in CI, FWICT.  And they are not in alternate 
sections.  Maybe an ARM person can chime in on those.
Thanks,
Jason