Re: [Minios-devel] [UNIKRAFT PATCH 6/8] build/Makefile.rules: Add support for linker scripts requiring the pre-processor '*.ldS'

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 12.09.2018 10:39, Yuri Volchkov wrote:
> Replies are inline. Plus I added a forgotten comment for the code.
>
> Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:
>
> > On 11.09.2018 18:12, Yuri Volchkov wrote:
> > > Hi,
> > >
> > > why to invent own naming. Why don't use names like are used in other
> > > famous projects. From linux/Documentation/kbuild/makefiles.txt:
> > >
> > >         When the vmlinux image is built, the linker script
> > >         arch/$(ARCH)/kernel/vmlinux.lds is used.
> > >         The script is a preprocessed variant of the file vmlinux.lds.S
> > >         located in the same directory.
> > >         kbuild knows .lds files and includes a rule *lds.S -> *lds.
> > >
> > > Also this patch looks incomplete. I propose to introduce the rule here
> > > already for the "lds.S -> lds".
> > What is incomplete?
> The function is not used anywhere. Even in the following patches in the
> series. I thought it would be logical to introduce a build rule in this
> patch too.
I see what you mean. Actually, the Linker.uk's of the platforms should 
use it by themselves to generate a final linker script. The reason is 
that these files currently taking care of the whole image linking 
process anyways. However, we do not have yet any linker script that 
needs to be processed. Wei's patch series will introduce the first one 
so this patch is a preparation.
However, I am going to add buildrule_ldS to the buildrule switch. THis 
does not hurt anyways. For installing the build rule, one could then 
also use the generic buildrule function instead.
> > If you prefer .lds.S, it is fine. I am not insisting on .ldS. There is
> > no official standard defined, so better use something people are
> > familiar with.
> >
> > > And renaming plat/kvm/x86/link64.lds to plat/kvm/x86/link64.lds.S as Wei
> > > Chen did in the upcoming patch series also belongs here. Obviously that
> > > would require adding a signed-off with his name.
> > He introduced a new one for Arm64.
> He also renames the x86 one in "plat/kvm: Generate linker scripts from
> source code" patch.
Understood. It is just a rename without using the pre-processor in the 
linker script. I told him that I prefer keeping the filename as .ld(s) 
as long as we are not needing the pre-processor on it.
> > > And one last thing, this mechanism requires a bit more explanation in
> > > the comments (probably next to the build rule). It's a bit hard to
> > > understand what is happening here if you never met such thing before.
> > Fine. Let me do a v2 ;-)
> >
> > > - Yuri.
> > >
> > > Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:
> > >
> > > > Introduce support for linker scripts that have to be pre-processed. Such a 
> > > > linker
> > > > script source file has to have the file extension *.ldS. In order to generate
> > > >
> > > > Within a platforms' Linker.uk file, two Makefile functions are
> > > >
> > > > Called *.ldS, normal linker script is *.lds
> > > >
> > > > Usage example in a Linker.uk
> > > >
> > > >    MY_LDSCRIPT_SRC := $(CONFIG_UK_BASE)/plat/my/linker.ldS
> > > >    MY_LDSCRIPT     := $(call ldS2lds,libmyplat,$(KVM_LDSCRIPT_SRC))
> > > >
> > > >    # Install build rule for linker script
> > > >    $(eval $(call buildrule_ldS,libkvmplat,$(MY_LDSCRIPT_SRC),$(MY_LDSCRIPT)))
> > > >
> > > >    # Afterwards refer with $(MY_LDSCRIPT) to the generated linker script.
> > > >
> > > > Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> > > > ---
> > > >    support/build/Makefile.rules | 37 +++++++++++++++++++++++++++++++++++++
> > > >    1 file changed, 37 insertions(+)
> > > >
> > > > diff --git a/support/build/Makefile.rules b/support/build/Makefile.rules
> > > > index ebbb8dd..aad6942 100644
> > > > --- a/support/build/Makefile.rules
> > > > +++ b/support/build/Makefile.rules
> > > > @@ -78,6 +78,26 @@ obj2dep = $(patsubst %.o,%.d,$(1))
> > > >    # src2dep $libname,$source(s),$variant(optional)
> > > >    src2dep = $(call obj2dep,$(call src2obj,$(1),$(2),$(3)))
> > > >    
> > > > +# converts paths of linker script sources (*.ldS) to paths of corresponding
> > > > +# processed linker scripts (*.lds) within the build directory
> > > > +# ldS2lds $libname,$ldS(s),$variant(optional)
> > > > +define ldS2lds =
> > > > +$(if $(3),\
> > > > +$(addprefix $(call sub_build_dir,$(1))/,$(addsuffix .$(3).lds,$(basename 
> > > > $(notdir $(2))))),\
> > > > +$(addprefix $(call sub_build_dir,$(1))/,$(addsuffix .lds,$(basename $(notdir 
> > > > $(2)))))\
> > > > +)
> > > > +endef
> > > > +
> > > > +# converts paths of processed linker scripts (*.lds) to corresponding
> > > > +# dependency files
> > > > +# lds2dep $lds(s)
> > > > +lds2dep = $(addsuffix .d,$(1))
> > > > +
> > > > +# converts paths of linker script sources (*.ldS) to paths of corresponding
> > > > +# dependency files
> > > > +# ldS2dep $libname,$ldS(s),$variant(optional)
> > > > +ldS2dep = $(call lds2dep,$(call ldS2lds,$(1),$(2),$(3)))
> > > > +
> > > >    
> > > >    ################################################################################
> > > >    #
> > > > @@ -392,6 +412,23 @@ $(eval $(call vprefix_lib,$(1),OBJS-y) += $(3))
> > > >    $(eval $(call vprefix_lib,$(1),CLEAN-y) += $(call build_clean,$(3)) $(call 
> > > > obj2dep,$(3)))
> > > >    endef
> > > >    
> > > > +define buildrule_ldS =
> > > > +$(3): $(2) | prepare
> > > > +       $(call build_cmd_fixdep,LDS,$(1),$(3),\
> > > > +               $(AS)  -E -P -x assembler-with-cpp $(ASINCLUDES) 
> > > > $(ASINCLUDES-y) \
> > > > +                      $($(call vprefix_lib,$(1),ASINCLUDES)) $($(call 
> > > > vprefix_lib,$(1),ASINCLUDES-y)) \
> > > > +                      $(ASFLAGS) $(ASFLAGS-y) \
> > > > +                      $($(call vprefix_lib,$(1),ASFLAGS)) $($(call 
> > > > vprefix_lib,$(1),ASFLAGS-y)) \
> > > > +                      $(4) \
> > > > +                      -c $(2) -o $(3) $(depflags),\
> Did you forget to remove '-c' here?
Yes, I did. However, it seemed not to hurt because stopping right after 
the preprocessing seems to exit GCC earlier.
> > > > +               $(call lds2dep,$(3))
> > > > +       )
> > > > +
> > > > +UK_SRCS-y += $(2)
> > > > +UK_DEPS-y += $(call lds2dep,$(3))
> > > > +$(eval $(call vprefix_lib,$(1),CLEAN-y) += $(call build_clean,$(3)) $(call 
> > > > lds2dep,$(3)))
> > > > +endef
> > > > +
> > > >    # wrapper for buildrule_*,
> > > >    # selects appropriate buildrule depending on file extension
> > > >    #
> > > > --
> > > > 2.7.4
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel