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

On 8/27/26 6:01 PM, Jan Beulich wrote:
> 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 <[email protected]>
>>>
>>> --- 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 \
>>> -		> [email protected]
>>> -	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.

Agree, then no need for the comment:

Reviewed-by: Oleksii Kurochko <[email protected]>

~ Oleksii



> 
> Jan