Re: [Minios-devel] [UNIKRAFT PATCH 6/8] plat/*: Separate plat lib linker scripts from others

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Sharan,

since you asked me to fast-track this patch, because it's pretty much 
stand-alone, and it will help out with Vlad's patch series, I had a look 
at this first.
In general, I agree with the idea. However, I'm not sure about the 
ordering of the linker scripts.
In the full use case, there will be three linker scripts in play now: 
first, the default one provided with -dT. Then, potentially two more, 
one library specific (-T EXTRA_LD_SCRIPT-$(LIBNAME)) and one both 
library *and* platform specific (-T <PLAT>_LD_SCRIPT-$(LIBNAME)). The 
order in which the linker look at the scripts depends who can override 
who: it first looks at the linker scripts provided with -T in 
left-to-right order, and then finally at the -dT script. [*]
So... shouldn't the order be to first look at the PLAT_LD_SCRIPT, so for 
example, it could goggle up symbols into a section, then let 
EXTRA_LD_SCRIPT gobble up what it wants from the remaining symbols, then 
let the default linker script deal with the rest?
In short: Instead of
KVM_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,\
                        $(EXTRA_LD_SCRIPT-y) $(KVM_LD_SCRIPT-y))
do:
KVM_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,\
                        $(KVM_LD_SCRIPT-y) $(EXTRA_LD_SCRIPT-y))

Oh, and while you're at it, I think this is cleaner if you put an 
explicit comma between the -T and the files to create gcc command lines 
of the form -Wl,-T,<file> instead of -Wl,-T<file> (though both happen to 
work).
Cheers,
Florian

[*] The -T scripts then have to provide "INSERT [BEFORE|AFTER]" rules in 
their scripts, otherwise, the -dT script won't be parsed (and most 
likely, that will mean the linking will horribly fail.)
On 3/15/19 6:06 PM, Sharan Santhanam wrote:
> The Unikraft build system provides a option to include additional
> linker scripts. In the current implementation EXTRA_LD_SCRIPT-y
> is shared among all the platform which result in all platform using
> this additional linker script. We introduce [PLAT]_LD_SCRIPT-y for
> platform specific library.
>
> Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> ---
>   doc/guides/developers-app.rst | 4 ++++
>   plat/kvm/Linker.uk            | 5 +++--
>   plat/linuxu/Linker.uk         | 5 +++--
>   plat/xen/Linker.uk            | 7 ++++---
>   4 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/doc/guides/developers-app.rst b/doc/guides/developers-app.rst
> index 8974824..2a67b1f 100644
> --- a/doc/guides/developers-app.rst
> +++ b/doc/guides/developers-app.rst
> @@ -304,6 +304,10 @@ your Makefile.uk to add ::
>   
>       EXTRA_LD_SCRIPT-$(CONFIG_LIBYOURAPPNAME) += $(LIBYOURAPPNAME_BASE)/extra.ld
>   
> +If your library is specific to a platform, edit your Makefile.uk to add ::
> +
> +       [PLATFORM]_LD_SCRIPT-$(CONFIG_LIBYOURAPPNAME) += 
> $(LIBYOURAPPNAME_BASE)/extra.ld
> +
>   
>   An example context of extra.ld: ::
>   
> diff --git a/plat/kvm/Linker.uk b/plat/kvm/Linker.uk
> index 4b8b167..1b57dcb 100644
> --- a/plat/kvm/Linker.uk
> +++ b/plat/kvm/Linker.uk
> @@ -11,7 +11,8 @@ endif
>   ##
>   KVM_IMAGE := $(BUILD_DIR)/$(CONFIG_UK_NAME)_kvm-$(CONFIG_UK_ARCH)
>   
> -EXTRA_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,$(EXTRA_LD_SCRIPT-y))
> +KVM_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,\
> +                       $(EXTRA_LD_SCRIPT-y) $(KVM_LD_SCRIPT-y))
>   
>   $(KVM_IMAGE): $(KVM_ALIBS) $(KVM_ALIBS-y) $(KVM_OLIBS) $(KVM_OLIBS-y) \
>                 $(UK_ALIBS) $(UK_ALIBS-y) $(UK_OLIBS) $(UK_OLIBS-y) \
> @@ -33,7 +34,7 @@ ifneq ($(filter x86_64 arm64,$(CONFIG_UK_ARCH)),)
>                $(LD) $(LDFLAGS) $(LDFLAGS-y) \
>                      $(KVM_LDFLAGS) $(KVM_LDFLAGS-y) \
>                      -Wl$(comma)-dT$(comma)$(call strip,$(KVM_LDSCRIPT)) \
> -                    $(EXTRA_LD_SCRIPT_FLAGS) \
> +                    $(KVM_LD_SCRIPT_FLAGS) \
>                      $@.o -o $@)
>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>         $(call build_cmd,OBJCOPY,,$@.dbg,\
> diff --git a/plat/linuxu/Linker.uk b/plat/linuxu/Linker.uk
> index f4387fa..4d23cc2 100644
> --- a/plat/linuxu/Linker.uk
> +++ b/plat/linuxu/Linker.uk
> @@ -4,7 +4,8 @@ LINUXU_LDFLAGS-y += -Wl,-e,_liblinuxuplat_start
>   ## Link image
>   ##
>   LINUXU_IMAGE := $(BUILD_DIR)/$(CONFIG_UK_NAME)_linuxu-$(CONFIG_UK_ARCH)
> -EXTRA_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,$(EXTRA_LD_SCRIPT-y))
> +LINUXU_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,\
> +                           $(EXTRA_LD_SCRIPT-y) $(LINUXU_LD_SCRIPT-y))
>   
>   $(LINUXU_IMAGE): $(LINUXU_ALIBS) $(LINUXU_ALIBS-y) \
>                  $(LINUXU_OLIBS) $(LINUXU_OLIBS-y) \
> @@ -19,7 +20,7 @@ $(LINUXU_IMAGE): $(LINUXU_ALIBS) $(LINUXU_ALIBS-y) \
>                      $(LINUXU_ALIBS) $(LINUXU_ALIBS-y) \
>                      $(UK_ALIBS) $(UK_ALIBS-y) \
>                      -Wl$(comma)--end-group \
> -                    $(EXTRA_LD_SCRIPT_FLAGS) \
> +                    $(LINUXU_LD_SCRIPT_FLAGS) \
>                      -o $@)
>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>         $(call build_cmd,OBJCOPY,,$@.dbg,\
> diff --git a/plat/xen/Linker.uk b/plat/xen/Linker.uk
> index aea1089..ee61cdf 100644
> --- a/plat/xen/Linker.uk
> +++ b/plat/xen/Linker.uk
> @@ -18,7 +18,8 @@ endif
>   ##
>   XEN_IMAGE := $(BUILD_DIR)/$(CONFIG_UK_NAME)_xen-$(CONFIG_UK_ARCH)
>   
> -EXTRA_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,$(EXTRA_LD_SCRIPT-y))
> +XEN_LD_SCRIPT_FLAGS := $(addprefix -Wl$(comma)-T,\
> +                       $(EXTRA_LD_SCRIPT-y) $(XEN_LD_SCRIPT-y))
>   
>   $(XEN_IMAGE): $(XEN_ALIBS) $(XEN_ALIBS-y) $(XEN_OLIBS) $(XEN_OLIBS-y) \
>                 $(UK_ALIBS) $(UK_ALIBS-y) $(UK_OLIBS) $(UK_OLIBS-y) \
> @@ -39,7 +40,7 @@ ifeq (arm,$(CONFIG_UK_ARCH))
>         $(call build_cmd,LD,,$@.elf,\
>                $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) \
>                      -Wl$(comma)-dT$(comma)$(call strip,$(XEN_LDSCRIPT)) \
> -                    $(EXTRA_LD_SCRIPT_FLAGS) \
> +                    $(XEN_LD_SCRIPT_FLAGS) \
>                      $@.o -o $@.elf)
>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>         $(call build_cmd,OBJCOPY,,$@.dbg,\
> @@ -61,7 +62,7 @@ else
>         $(call build_cmd,LD,,$@,\
>                $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) \
>                      -Wl$(comma)-dT$(comma)$(call strip,$(XEN_LDSCRIPT)) \
> -                    $(EXTRA_LD_SCRIPT_FLAGS) \
> +                    $(XEN_LD_SCRIPT_FLAGS) \
>                      $@.o -o $@)
>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>         $(call build_cmd,OBJCOPY,,$@.dbg,\
--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

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