Re: [PATCH v2 3/7] RISC-V: split xen-syms linking rule

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.08.2026 17:56, Oleksii Kurochko wrote:
> On 8/26/26 2:01 PM, Jan Beulich wrote:
>> Doing so, besides (hopefully) adding clarity (not the least by way of
>> [re-]using pattern rules where possible), also avoids explicit recursive
>> $(MAKE) invocations.
>>
>> By re-using the generic rules introduced when the respective x86 rule was
>> split,
>> - the .map file now isn't created after the final binary anymore,
>> - --strip-debug is passed to $(LD) during early linking passes (for
>>    consistency the option is also explicitly added to the optional linking
>>    pass rule),
>> - CONFIG_{SUPPRESS_DUPLICATE_SYMBOL_WARNINGS,ENFORCE_UNIQUE_SYMBOLS} are
>>    now properly respected.
>> Orphan section checking, otoh, is getting suppressed for now, until the
>> about a dozen warnings which would result have been taken care of.
>>
>> While the 4th linking step continues to be avoided when possible, a
>> redundant invocation of $(NM) and tools/symbols (plus the assembling of
>> the resulting .S file) is hopefully deemed acceptable.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/riscv/Makefile
>> +++ b/xen/arch/riscv/Makefile
>> @@ -31,40 +31,12 @@ obj-y += vtimer.o
>>   $(TARGET): $(TARGET)-syms
>>      $(OBJCOPY) -O binary -S $< $@
>>   
>> -$(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
>> -    $(objtree)/tools/symbols $(all_symbols) --empty > $(dot-target).0.S
>> -    $(MAKE) $(build)=$(@D) $(dot-target).0.o
>> -    $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> -          $(dot-target).0.o -o $(dot-target).0
>> -    $(NM) -pa --format=sysv $(dot-target).0 \
>> -            | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>> -            > $(dot-target).1.S
>> -    $(MAKE) $(build)=$(@D) $(dot-target).1.o
>> -    $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> -        $(dot-target).1.o -o $(dot-target).1
>> -    $(NM) -pa --format=sysv $(dot-target).1 \
>> -            | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>> -            > $(dot-target).2.S
>> -    $(MAKE) $(build)=$(@D) $(dot-target).2.o
>> -    if ! { $(call compare-symbol-tables, $(dot-target).1.o, 
>> $(dot-target).2.o) >/dev/null; }; \
>> -    then \
>> -            set -e; \
>> -            $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> -                $(dot-target).2.o -o $(dot-target).2; \
>> -            $(NM) -pa --format=sysv $(dot-target).2 \
>> -                    | $(objtree)/tools/symbols $(all_symbols) --sysv --sort 
>> \
>> -                    > $(dot-target).3.S; \
>> -            $(MAKE) $(build)=$(@D) $(dot-target).3.o; \
>> -            $(call compare-symbol-tables, $(dot-target).2.o, 
>> $(dot-target).3.o); \
>> -    else \
>> -            ln -sf $(dot-target).2.o $(dot-target).3.o; \
>> -    fi
>> -    $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds $< $(build_id_linker) \
>> -        $(dot-target).3.o -o $@
>> -    $(NM) -pa --format=sysv $@ \
>> -            | $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
>> --sort \
>> -            > $@.map
>> -    rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>> +LAST_LINKING_PASS := 3
>> +
>> +include scripts/Makefile.link
>> +
>> +# Suppress orphan section checking for the time being.
>> +orphan-handling-y :=
> 
> This works, but I think it's worth reconsidering the shape of it.
> 
> It works only by virtue of deferred expansion: $(orphan-handling-y) is
> referenced solely inside the recipe of the final-pass rule in 
> Makefile.link, so the value that matters is the one in effect when that
> recipe is expanded, not when the rule was defined. Nothing states that
> requirement, and nothing enforces it.
> 
> What makes me uneasy is that the ordering is not merely undocumented,
> it's inverted with respect to the obvious reading. Makefile.link has
> 
>    orphan-handling-$(call ld-option,--orphan-handling=warn) := 
> --orphan-handling=warn
> 
> i.e. an unconditional := to orphan-handling-y whenever the linker
> supports the option. So an arch that sets orphan-handling-y *before*
> the include has its setting silently discarded and ends up with orphan
> checking enabled after all: no warning, no error, just a dozen new
> linker diagnostics appearing at some later point. And "before the
> include" is exactly where one would naturally put it: right next to
> LAST_LINKING_PASS, which is the one knob the arch Makefile does set up
> front.
> 
> I am not insisting on reworking but probably a small comment (in the 
> commit mesage at least?) somewhere about that "+orphan-handling-y :=" 
> should go after include will be useful.

I can add a comment (albeit the ordering looks very obvious to me, and
not counterintuitive at all), but the better thing would be for all
arch-es to quickly deal with getting rid of this override again: No
need for an override, no need for a comment.

Jan