Re: [Minios-devel] [UNIKRAFT PATCH 1/8] plat/kvm, plat/xen: Add linker script as build dependency

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:

> On 11.09.2018 15:32, Yuri Volchkov wrote:
>> Hi Simon.
>> 
>> just a couple of cosmetic notes inline.
>> 
>> - Yuri
>> 
>> Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:
>> 
>>> Modify tha platforms' Linker.uk so that the build rules that create
>>> the final image also depend on changes in the corresponding linker scripts.
>>>
>>> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
>>> ---
>>>   plat/kvm/Linker.uk |  8 ++++----
>>>   plat/xen/Linker.uk | 16 ++++++++--------
>>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/plat/kvm/Linker.uk b/plat/kvm/Linker.uk
>>> index 2292b03..b43072f 100644
>>> --- a/plat/kvm/Linker.uk
>>> +++ b/plat/kvm/Linker.uk
>>> @@ -1,5 +1,5 @@
>>>   ifeq (x86_64,$(CONFIG_UK_ARCH))
>>> -KVM_LDSCRIPT  := -Wl,-T,$(CONFIG_UK_BASE)/plat/kvm/x86/link64.ld
>>> +KVM_LDSCRIPT  := $(CONFIG_UK_BASE)/plat/kvm/x86/link64.ld
>>>   KVM_LDFLAGS-y += -Wl,-m,elf_x86_64
>> Would it make sense to add -Wl,-T$(KVM_LDSCRIPT) to the KV_LDFLAGS-y?
>> With necessary comma handling. Same regarding the XEN part.
>> 
>
> This is possible to do but I prefer treating the KVM_LDSCRIPT 
> separately. The script is an important input file that is required for 
> the linking process.
> You insist on this? ;-)
No, not at all. IMHO it just feels it belongs there. And to be clear, I
did not mean to remove KVM_LDSCRIPT variable. It would still appear in
the dependencies.

So, you decide which option to take for v2. I'm fine with either of them

>
>>>   endif
>>>   
>>> @@ -8,14 +8,14 @@ endif
>>>   ##
>>>   KVM_IMAGE := $(BUILD_DIR)/$(CONFIG_UK_NAME)_kvm-$(CONFIG_UK_ARCH)
>>>   
>>> -$(KVM_IMAGE): $(KVM_LINK) $(KVM_LINK-y) $(UK_LINK) $(UK_LINK-y)
>>> +$(KVM_IMAGE): $(KVM_LINK) $(KVM_LINK-y) $(UK_LINK) $(UK_LINK-y) 
>>> $(KVM_LDSCRIPT)
>>>     $(call build_cmd,LD,,$@.ld.o,\
>>> -          $(LD) -r $(LIBLDFLAGS) $(LIBLDFLAGS-y) $(KVM_LDFLAGS) 
>>> $(KVM_LDFLAGS-y) $^ -o $@.ld.o)
>>> +          $(LD) -r $(LIBLDFLAGS) $(LIBLDFLAGS-y) $(KVM_LDFLAGS) 
>>> $(KVM_LDFLAGS-y) $(KVM_LINK) $(KVM_LINK-y) $(UK_LINK) $(UK_LINK-y) -o 
>>> $@.ld.o)
>> It looks reasonable to divide this line to avoid crossing the 80 chars
>> limit.  Same regarding the XEN part.
>
> Fine, I ll update this.
>
>> 
>>>     $(call build_cmd,OBJCOPY,,$@.o,\
>>>            $(OBJCOPY) -w -G kvmos_* -G _libkvmplat_entry $@.ld.o $@.o)
>>>   ifeq (x86_64,$(CONFIG_UK_ARCH))
>>>     $(call build_cmd,LD,,$@,\
>>> -          $(LD) $(LDFLAGS) $(LDFLAGS-y) $(KVM_LDFLAGS) $(KVM_LDFLAGS-y) 
>>> $(KVM_LDSCRIPT) $@.o -o $@)
>>> +          $(LD) $(LDFLAGS) $(LDFLAGS-y) $(KVM_LDFLAGS) $(KVM_LDFLAGS-y) 
>>> -Wl$(comma)-T$(comma)$(call strip,$(KVM_LDSCRIPT)) $@.o -o $@)
>>>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>>>     $(call build_cmd,OBJCOPY,,$@.dbg,\
>>>            $(OBJCOPY) --only-keep-debug $@ $@.dbg)
>>> diff --git a/plat/xen/Linker.uk b/plat/xen/Linker.uk
>>> index e3f5a9a..0c0c4ee 100644
>>> --- a/plat/xen/Linker.uk
>>> +++ b/plat/xen/Linker.uk
>>> @@ -1,16 +1,16 @@
>>>   ifeq (x86_64,$(CONFIG_UK_ARCH))
>>> -XEN_LDSCRIPT  := -Wl,-T,$(CONFIG_UK_BASE)/plat/xen/x86/link64.ld
>>> +XEN_LDSCRIPT  := $(CONFIG_UK_BASE)/plat/xen/x86/link64.ld
>>>   XEN_LDFLAGS-y += -Wl,-m,elf_x86_64
>>>   endif
>>>   ifeq (x86_32,$(CONFIG_UK_ARCH))
>>> -XEN_LDSCRIPT  := -Wl,-T,$(CONFIG_UK_BASE)/plat/xen/x86/link32.ld
>>> +XEN_LDSCRIPT  := $(CONFIG_UK_BASE)/plat/xen/x86/link32.ld
>>>   XEN_LDFLAGS-y += -Wl,-m,elf_x86_32
>>>   endif
>>>   ifeq (arm_64,$(CONFIG_UK_ARCH))
>>> -XEN_LDSCRIPT  := -Wl,-T,$(CONFIG_UK_BASE)/plat/xen/arm/link64.ld
>>> +XEN_LDSCRIPT  := $(CONFIG_UK_BASE)/plat/xen/arm/link64.ld
>>>   endif
>>>   ifeq (arm,$(CONFIG_UK_ARCH))
>>> -XEN_LDSCRIPT  := -Wl,-T,$(CONFIG_UK_BASE)/plat/xen/arm/link32.ld
>>> +XEN_LDSCRIPT  := $(CONFIG_UK_BASE)/plat/xen/arm/link32.ld
>>>   endif
>>>   
>>>   ##
>>> @@ -18,14 +18,14 @@ endif
>>>   ##
>>>   XEN_IMAGE := $(BUILD_DIR)/$(CONFIG_UK_NAME)_xen-$(CONFIG_UK_ARCH)
>>>   
>>> -$(XEN_IMAGE): $(XEN_LINK) $(XEN_LINK-y) $(UK_LINK) $(UK_LINK-y)
>>> +$(XEN_IMAGE): $(XEN_LINK) $(XEN_LINK-y) $(UK_LINK) $(UK_LINK-y) 
>>> $(XEN_LDSCRIPT)
>>>     $(call build_cmd,LD,,$@.ld.o,\
>>> -          $(LD) -r $(LIBLDFLAGS) $(LIBLDFLAGS-y) $(XEN_LDFLAGS) 
>>> $(XEN_LDFLAGS-y) $^ -o $@.ld.o)
>>> +          $(LD) -r $(LIBLDFLAGS) $(LIBLDFLAGS-y) $(XEN_LDFLAGS) 
>>> $(XEN_LDFLAGS-y) $(XEN_LINK) $(XEN_LINK-y) $(UK_LINK) $(UK_LINK-y) -o 
>>> $@.ld.o)
>>>     $(call build_cmd,OBJCOPY,,$@.o,\
>>>            $(OBJCOPY) -w -G xenos_* -G _libxenplat_start $@.ld.o $@.o)
>>>   ifeq (arm,$(CONFIG_UK_ARCH))
>>>     $(call build_cmd,LD,,$@.elf,\
>>> -          $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) 
>>> $(XEN_LDSCRIPT) $@.o -o $@.elf)
>>> +          $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) 
>>> -Wl$(comma)-T$(comma)$(call strip,$(XEN_LDSCRIPT)) $@.o -o $@.elf)
>>>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>>>     $(call build_cmd,OBJCOPY,,$@.dbg,\
>>>            $(OBJCOPY) --only-keep-debug $@.elf $@.dbg)
>>> @@ -44,7 +44,7 @@ endif
>>>            $(GZIP) -f -9 -c $@ >$@.gz)
>>>   else
>>>     $(call build_cmd,LD,,$@,\
>>> -          $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) 
>>> $(XEN_LDSCRIPT) $@.o -o $@)
>>> +          $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) 
>>> -Wl$(comma)-T$(comma)$(call strip,$(XEN_LDSCRIPT)) $@.o -o $@)
>>>   ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
>>>     $(call build_cmd,OBJCOPY,,$@.dbg,\
>>>            $(OBJCOPY) --only-keep-debug $@ $@.dbg)
>>> -- 
>>> 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