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

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

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.

>
> 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.

>
>> 
>> 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?

>>> +           $(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
>>>
>> 

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel