[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hello Florian,



On 4/2/19 4:36 PM, Florian Schmidt wrote:
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))


I agree. I will fix it in v2.

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).
Ok I will fix it in v2.


Cheers,
Florian

I will send out the v2 as a standalone patch.

Thanks & Regards
Sharan

[*] 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,\



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.