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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

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 <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.
Agree, then no need for the comment:

Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

~ Oleksii



> Jan