| Message ID | 1573031953-12894-6-git-send-email-andrii.anisov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Build XEN with ARM Compiler 6.6.3 | expand |
On Wed, 6 Nov 2019, Andrii Anisov wrote: > From: Andrii Anisov <andrii_anisov@epam.com> > > Here several ARM Compiler 6.6 issues are solved or provided a work-around: > > - Scatter file is pretty primitive, it has no feature to define symbols > - ARM linker defined symbols are not counted as referred if only mentioned > in a steering file for rename or resolve, so a header file is used to > redefine GNU linker script symbols into armlink defined symbols. > > - _srodata type clashes by type with __start_bug_frames so can not be both > redefined as Load$$_rodata_bug_frames_0$$Base. Use resolve feature of armlink > steering file. Why _srodata and __start_bug_frames cannot both be defined as Load$$_rodata_bug_frames_0$$Base when actually in the case of: > +#define __per_cpu_data_end Load$$_bss_percpu$$Limit > +#define __bss_end Load$$_bss_percpu$$Limit > +#define _end Load$$_bss_percpu$$Limit They are all defined as "Load$$_bss_percpu$$Limit"? And: > +#define __init_end Load$$_bss$$Base > +#define __bss_start Load$$_bss$$Base They are both defined as "Load$$_bss$$Base"? What's special about "Load$$_rodata_bug_frames_0$$Base"? > - C style shift operators are missed among supported scatter file expressions, > so some needed values are hardcoded in scatter file. > > - Rename correspondent ARM Linker defined symbols to those needed by `symbols` tool > using steering file feature. > > - ARM Compiler 6.6 tools are not able to rename sections, so we still need > GNU toolchain's objcopy for this. > > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com> > --- > xen/Rules.mk | 6 + > xen/arch/arm/Makefile | 24 ++++ > xen/arch/arm/xen.scat.S | 266 ++++++++++++++++++++++++++++++++++++++++++++ I would strongly suggest to rename this file with something not potentially related to scat > xen/arch/arm/xen.steer | 5 + > xen/include/asm-arm/armds.h | 91 +++++++++++++++ > 5 files changed, 392 insertions(+) > create mode 100644 xen/arch/arm/xen.scat.S > create mode 100644 xen/arch/arm/xen.steer > create mode 100644 xen/include/asm-arm/armds.h > > diff --git a/xen/Rules.mk b/xen/Rules.mk > index 41a1c26..67bedce 100644 > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -60,6 +60,12 @@ CFLAGS += -nostdinc -fno-builtin -fno-common > CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith > $(call cc-option-add,CFLAGS,CC,-Wvla) > CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h > + > +ifeq ($(armds),y) > +CFLAGS += -nostdlibinc -nostdlib -Wno-unused-command-line-argument > +CFLAGS += -include $(BASEDIR)/include/asm/armds.h > +endif > + > CFLAGS-$(CONFIG_DEBUG_INFO) += -g > CFLAGS += '-D__OBJECT_FILE__="$@"' > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 70f532e..a5a3479 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -83,11 +83,16 @@ else > all_symbols = > endif > > +ifeq ($(armds),y) > +$(TARGET): $(TARGET)-syms > + fromelf --bin $< --output $@ > +else > $(TARGET): $(TARGET)-syms > $(OBJCOPY) -O binary -S $< $@ > ifeq ($(CONFIG_ARM_64),y) > ln -sf $(notdir $@) ../../$(notdir $@).efi > endif > +endif > > ifeq ($(CONFIG_LTO),y) > # Gather all LTO objects together > @@ -102,6 +107,19 @@ prelink.o: $(ALL_OBJS) > $(LD) $(LDFLAGS) -r -o $@ $^ > endif > > +ifeq ($(armds),y) > +$(TARGET)-syms: prelink.o xen.scat > + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 > + $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ > + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S > + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o > + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 > + $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ > + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S > + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o > + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib --symdefs="$(@D)/$(@F).map" $(LDFLAGS) prelink.o $(build_id_linker) $(@D)/.$(@F).1.o -o $@ > + rm -f $(@D)/.$(@F).[0-9]* > +else > $(TARGET)-syms: prelink.o xen.lds > $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ > $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 > @@ -119,14 +137,20 @@ $(TARGET)-syms: prelink.o xen.lds > | $(BASEDIR)/tools/symbols --xensyms --sysv --sort \ > >$(@D)/$(@F).map > rm -f $(@D)/.$(@F).[0-9]* > +endif > > asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c > $(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $< > > +ifeq ($(armds),y) > +xen.scat: xen.scat.S > + $(CC) -P -E --target=aarch64-arm-none-eabi -o $@ $< > +else > xen.lds: xen.lds.S > $(CC) -P -E -Ui386 $(AFLAGS) -o $@ $< > sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new > mv -f .xen.lds.d.new .xen.lds.d > +endif > > dtb.o: $(CONFIG_DTB_FILE) > > diff --git a/xen/arch/arm/xen.scat.S b/xen/arch/arm/xen.scat.S > new file mode 100644 > index 0000000..3bb405f > --- /dev/null > +++ b/xen/arch/arm/xen.scat.S > @@ -0,0 +1,266 @@ > +#if 0 #if 0 must be a mistake? Also, is this basically the ARMCC version of a linker script? Is xen.lds.S still used with ARMCC, or only xen.scat.S is used? > +/* > + * armlink does not understand shifts in scat file expressions > + * so hardcode needed values > + */ > +#include <xen/cache.h> > +#include <asm/page.h> > +#include <asm/percpu.h> > +#undef ENTRY > +#undef ALIGN > +#else > + #define PAGE_SIZE 4096 > + #define POINTER_ALIGN 8 > + #define SMP_CACHE_BYTES 128 > + #define STACK_SIZE 32768 > + #define XEN_VIRT_START 0x00200000 > +#endif > + > +LOAD XEN_VIRT_START > +{ > +;_start > +;_stext > + _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090 > + { > + *(.text*) > + *(.text.cold) > + *(.text.unlikely) > + *(.fixup) > + *(.gnu.warning) > + } > +;_etext > + > +;_srodata > +;__start_bug_frames > + _rodata_bug_frames_0 AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD > + { > + *(.bug_frames.0) > + } > +;__stop_bug_frames_0 > + > + _rodata_bug_frames_1 +0 FIXED ZEROPAD > + { > + *(.bug_frames.1) > + } > +;__stop_bug_frames_1 > + > + _rodata_bug_frames_2 +0 FIXED ZEROPAD > + { > + *(.bug_frames.2) > + } > +;__stop_bug_frames_2 > + > + _rodata_data +0 > + { > + *(.rodata) > + *(.rodata.*) > + *(.data.rel.ro) > + *(.data.rel.ro.*) > + } > + > +#ifdef CONFIG_LOCK_PROFILE > +;__lock_profile_start > + _rodata_lockprofile_data AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD > + { > + *(.lockprofile.data) > + } > +;__lock_profile_end > +#endif Should be below > +;__param_start > + _rodata_data_param AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD > + { > + *(.data.param) > + } > +;__param_end > + > +;__proc_info_start > + _rodata_proc_info +0 FIXED ZEROPAD > + { > + *(.proc.info) > + } > +;__proc_info_end > + > +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) > +;__start_vpci_array > + _rodata_data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD > + { > + *(SORT(.data.vpci.*)) > + } > +;__end_vpci_array > +#endif > + > +#if defined(BUILD_ID) > +;__note_gnu_build_id_start > + _note_gnu_build_id +0 FIXED ZEROPAD > + { > + *(.note.gnu.build-id) > + } > +;__note_gnu_build_id_end > +#endif > + > +;_erodata > + > + _data AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD > + { > + *(.data.page_aligned.*) > + *(.data.*) > + } > + > +;__start_schedulers_array > + _data_schedulers AlignExpr(+0, 8) FIXED ZEROPAD > + { > + *(.data.schedulers) > + } > +;__end_schedulers_array > + > + _data_rel +0 FIXED ZEROPAD > + { > + *(.data.rel) > + *(.data.rel.*) > +;#CONSTRUCTORS ???? Honestly I am not sure what this is either > + } > + > +;__start___ex_table > + _data_ex_table AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD > + { > + *(.ex_table) > + } > +;__stop___ex_table > + > +;__start___pre_ex_table > + _data_ex_table_pre +0 FIXED ZEROPAD > + { > + *(.ex_table.pre) > + } > +;__stop___pre_ex_table > + > + _data_read_mostly +0 FIXED ZEROPAD > + { > + *(.data.read_mostly) > + } > + > +;_splatform > + _arch_info AlignExpr(+0, 8) FIXED ZEROPAD > + { > + *(.arch.info) > + } > +;_eplatform > + > +;_sdevice > + _dev_info AlignExpr(+0, 8) FIXED ZEROPAD > + { > + *(.dev.info) > + } > +;_edevice > + > +;_asdevice > + _adev_info AlignExpr(+0, 8) FIXED ZEROPAD > + { > + *(.adev.info) > + } > +;_aedevice _steemediator/_eteemediator > +;__init_begin > +;_sinittext > + _init_text AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD > + { > + *(.init.text) > + } > +;_einittext > + > + _init_rodata AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD > + { > + *(.init.rodata) > + *(.init.rodata.rel) > + *(.init.rodata.str*) > + } > + > +;__setup_start > + _init_setup AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD > + { > + *(.init.setup) > + } > +;__setup_end > + > +;__initcall_start > + _initcallpresmp_init +0 FIXED ZEROPAD > + { > + *(.initcallpresmp.init) > + } > +;__presmp_initcall_end > + > + _initcall1_init +0 FIXED ZEROPAD > + { > + *(.initcall1.init) > + } > +;__initcall_end > + > +;__alt_instructions > + _altinstructions AlignExpr(+0, 4) FIXED ZEROPAD > + { > + *(.altinstructions) > + } > +;__alt_instructions_end > + > + _altinstr_replacement AlignExpr(+0, 4) FIXED ZEROPAD > + { > + *(.altinstr_replacement) > + } __lock_profile_start should be here > + > + _init_data +0 FIXED ZEROPAD > + { > + *(.init.data) > + *(.init.data.rel) > + *(.init.data.rel.*) > + } > + > +;__ctors_start > + _ctors AlignExpr(+0, 8) FIXED ZEROPAD > + { > + *(.ctors) > + *(.init_array) > + } > + > + _init_array_sorted AlignExpr(+0, 8) SORTTYPE Lexical FIXED ZEROPAD > + { > + *(.init_array.*) > + } > +;__ctors_end > + > +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) > + _data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD > + { > + *(.data.vpci.*) > + } > +#endif __start_vpci_array/__end_vpci_array? > +;__init_end_efi > + > +;__init_end > +;__bss_start > + _bss AlignExpr(+0, STACK_SIZE) FIXED ZEROPAD > + { > + *(.bss.stack_aligned*) > + *(.bss.page_aligned*, OVERALIGN PAGE_SIZE) > + *(.bss*) > + } > + > +;__per_cpu_start this should be page aligned too? > + _bss_percpu AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD > + { > + *(.bss.percpu) > + *(.bss.percpu.read_mostly, OVERALIGN SMP_CACHE_BYTES) > + } > +;__per_cpu_data_end > +;__bss_end __bss_end should be page aligned? > +;_end > + > +#ifdef CONFIG_DTB_FILE > +;_sdtb > + _dtb FIXED ZEROPAD > + { > + *(.dtb) > + } > +#endif > + > +} > diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer > new file mode 100644 > index 0000000..646e912 > --- /dev/null > +++ b/xen/arch/arm/xen.steer > @@ -0,0 +1,5 @@ > +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base > +RENAME Load$$_text$$Base AS _stext > +RENAME Load$$_text$$Limit AS _etext > +RENAME Load$$_init_text$$Base AS _sinittext > +RENAME Load$$_init_text$$Limit AS _einittext I don't get why some if the "symbols" get renamed using RENAME here, and some other we are using a #define in xen/include/asm-arm/armds.h. Can't we rename them all here? > diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h > new file mode 100644 > index 0000000..5ee2e5d > --- /dev/null > +++ b/xen/include/asm-arm/armds.h Missing guards. Also, probably you want to make sure this is only #ifdef ARMCC. Is this meant to be used when building C files, asm files, or both? I would avoid this header file if we can get away with just xen.steer. > @@ -0,0 +1,91 @@ > +#define _start Load$$_text$$Base > +#define _stext Load$$_text$$Base > + > +#define _etext Load$$_text$$Limit > + > +//#define _srodata Load$$_rodata_bug_frames_0$$Base > +#define __start_bug_frames Load$$_rodata_bug_frames_0$$Base > + > +#define __stop_bug_frames_0 Load$$_rodata_bug_frames_0$$Limit > +#define __stop_bug_frames_1 Load$$_rodata_bug_frames_1$$Limit > +#define __stop_bug_frames_2 Load$$_rodata_bug_frames_2$$Limit > + > +#ifdef CONFIG_LOCK_PROFILE > +#define __lock_profile_start Load$$_rodata_lockprofile_data$$Base > +#define __lock_profile_end Load$$_rodata_lockprofile_data$$Limit > +#endif > + > +#define __param_start Load$$_rodata_data_param$$Base > +#define __param_end Load$$_rodata_data_param$$Limit > + > +#define __proc_info_start Load$$_rodata_proc_info$$Base > +#define __proc_info_end Load$$_rodata_proc_info$$Limit > + > +#define _erodata Load$$_rodata_proc_info$$Limit > + > +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) > +#define __start_vpci_array Load$$_rodata_data_vpci$$Base > +#define __end_vpci_array Load$$_rodata_data_vpci$$Limit > + > +#undef _erodata > +#define _erodata Load$$_rodata_data_vpci$$Limit > +#endif > + > +#if defined(BUILD_ID) > +#define __note_gnu_build_id_start Load$$_note_gnu_build_id$$Base > +#define __note_gnu_build_id_end Load$$_note_gnu_build_id$$Limit > + > +#undef _erodata > +#define _erodata Load$$_note_gnu_build_id$$Limit > +#endif > + > +#define __start_schedulers_array Load$$_data_schedulers$$Base > +#define __end_schedulers_array Load$$_data_schedulers$$Limit > + > +/* Does not exist for ARM > +#define __start___ex_table Load$$_data_ex_table$$Base > +#define __stop___ex_table Load$$_data_ex_table$$Limit > +*/ > + > +#define __start___pre_ex_table Load$$_data_ex_table_pre$$Base > +#define __stop___pre_ex_table Load$$_data_ex_table_pre$$Limit > + > +#define _splatform Load$$_arch_info$$Base > +#define _eplatform Load$$_arch_info$$Limit > + > +#define _sdevice Load$$_dev_info$$Base > +#define _edevice Load$$_dev_info$$Limit > + > +#define _asdevice Load$$_adev_info$$Base > +#define _aedevice Load$$_adev_info$$Limit > + > +#define __init_begin Load$$_init_text$$Base > +#define _sinittext Load$$_init_text$$Base > +#define _einittext Load$$_init_text$$Limit > + > +#define __setup_start Load$$_init_setup$$Base > +#define __setup_end Load$$_init_setup$$Limit > + > +#define __initcall_start Load$$_initcallpresmp_init$$Base > +#define __presmp_initcall_end Load$$_initcallpresmp_init$$Limit > +#define __initcall_end Load$$_initcall1_init$$Limit > + > +#define __alt_instructions Load$$_altinstructions$$Base > +#define __alt_instructions_end Load$$_altinstructions$$Limit > + > +#define __ctors_start Load$$_ctors$$Base > +#define __ctors_end Load$$_init_array_sorted$$Limit > +#define __init_end_efi Load$$_init_array_sorted$$Limit > + > +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) > +#undef __init_end_efi > +#define __init_end_efi Load$$_data_vpci$$Limit > +#endif > + > +#define __init_end Load$$_bss$$Base > +#define __bss_start Load$$_bss$$Base > + > +#define __per_cpu_start Load$$_bss_percpu$$Base > +#define __per_cpu_data_end Load$$_bss_percpu$$Limit > +#define __bss_end Load$$_bss_percpu$$Limit > +#define _end Load$$_bss_percpu$$Limit
On Tue, 12 Nov 2019, 06:27 Stefano Stabellini, <sstabellini@kernel.org> wrote: > On Wed, 6 Nov 2019, Andrii Anisov wrote: > > From: Andrii Anisov <andrii_anisov@epam.com> > > > > Here several ARM Compiler 6.6 issues are solved or provided a > work-around: > > > > - Scatter file is pretty primitive, it has no feature to define symbols > > - ARM linker defined symbols are not counted as referred if only > mentioned > > in a steering file for rename or resolve, so a header file is used to > > redefine GNU linker script symbols into armlink defined symbols. > > > > - _srodata type clashes by type with __start_bug_frames so can not be > both > > redefined as Load$$_rodata_bug_frames_0$$Base. Use resolve feature of > armlink > > steering file. > > Why _srodata and __start_bug_frames cannot both be defined as > Load$$_rodata_bug_frames_0$$Base when actually in the case of: > > > +#define __per_cpu_data_end Load$$_bss_percpu$$Limit > > +#define __bss_end Load$$_bss_percpu$$Limit > > +#define _end Load$$_bss_percpu$$Limit > > They are all defined as "Load$$_bss_percpu$$Limit"? And: > > > +#define __init_end Load$$_bss$$Base > > +#define __bss_start Load$$_bss$$Base > > They are both defined as "Load$$_bss$$Base"? What's special about > "Load$$_rodata_bug_frames_0$$Base"? > > > > - C style shift operators are missed among supported scatter file > expressions, > > so some needed values are hardcoded in scatter file. > > > > - Rename correspondent ARM Linker defined symbols to those needed by > `symbols` tool > > using steering file feature. > > > > - ARM Compiler 6.6 tools are not able to rename sections, so we still > need > > GNU toolchain's objcopy for this. > > > > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com> > > --- > > xen/Rules.mk | 6 + > > xen/arch/arm/Makefile | 24 ++++ > > xen/arch/arm/xen.scat.S | 266 > ++++++++++++++++++++++++++++++++++++++++++++ > > I would strongly suggest to rename this file with something not > potentially related to scat > To be honest, I don't think this file should even exist. This looks like a copy of xen.lds.S with a different syntax. Furthermore, the comments from Stefano shows that is going to be hard to maintain/check everything has been written correctly. So how about trying to abstract xen.lds.S? > > > +/* > > + * armlink does not understand shifts in scat file expressions > > + * so hardcode needed values > > + */ > Please give a pointer to the doc of the armlink in the commit message. So we can easily cross-check what's happening. In this case, I don't particularly like the re-definition of the defines outside of their header. This is going to make more difficult if we have to update them in the future. I can see a few ways to do it: - Avoid using shifts in the definitions - Find a way to evaluate the value (maybe similar to asn-offset) before using them. My preference would be the latter but I could be convinced for the former. Cheers,
On 11.11.19 23:26, Stefano Stabellini wrote: > Why _srodata and __start_bug_frames cannot both be defined as > Load$$_rodata_bug_frames_0$$Base when actually in the case of: > >> +#define __per_cpu_data_end Load$$_bss_percpu$$Limit >> +#define __bss_end Load$$_bss_percpu$$Limit >> +#define _end Load$$_bss_percpu$$Limit > > They are all defined as "Load$$_bss_percpu$$Limit"? And: > >> +#define __init_end Load$$_bss$$Base >> +#define __bss_start Load$$_bss$$Base > > They are both defined as "Load$$_bss$$Base"? What's special about > "Load$$_rodata_bug_frames_0$$Base"? Because in xen/include/asm-arm/bug.h: extern const struct bug_frame __start_bug_frames[] and in xen/include/xen/kernel.h extern const char _srodata[]; After preprocessing we, effectively, have symbol declared with conflicting types: extern const struct bug_frame Load$$_rodata_bug_frames_0$$Base[]; extern const char Load$$_rodata_bug_frames_0$$Base[]; Eventually other symbols do not have such conflicts. > > >> - C style shift operators are missed among supported scatter file expressions, >> so some needed values are hardcoded in scatter file. >> >> - Rename correspondent ARM Linker defined symbols to those needed by `symbols` tool >> using steering file feature. >> >> - ARM Compiler 6.6 tools are not able to rename sections, so we still need >> GNU toolchain's objcopy for this. >> >> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com> >> --- >> xen/Rules.mk | 6 + >> xen/arch/arm/Makefile | 24 ++++ >> xen/arch/arm/xen.scat.S | 266 ++++++++++++++++++++++++++++++++++++++++++++ > > I would strongly suggest to rename this file with something not > potentially related to scat I just followed examples from ARM documentation, e.g. [1]. I suppose they know something about their product. Yet, the suggestion is reasonable. > > >> xen/arch/arm/xen.steer | 5 + >> xen/include/asm-arm/armds.h | 91 +++++++++++++++ >> 5 files changed, 392 insertions(+) >> create mode 100644 xen/arch/arm/xen.scat.S >> create mode 100644 xen/arch/arm/xen.steer >> create mode 100644 xen/include/asm-arm/armds.h >> >> diff --git a/xen/Rules.mk b/xen/Rules.mk >> index 41a1c26..67bedce 100644 >> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -60,6 +60,12 @@ CFLAGS += -nostdinc -fno-builtin -fno-common >> CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith >> $(call cc-option-add,CFLAGS,CC,-Wvla) >> CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h >> + >> +ifeq ($(armds),y) >> +CFLAGS += -nostdlibinc -nostdlib -Wno-unused-command-line-argument >> +CFLAGS += -include $(BASEDIR)/include/asm/armds.h >> +endif >> + >> CFLAGS-$(CONFIG_DEBUG_INFO) += -g >> CFLAGS += '-D__OBJECT_FILE__="$@"' >> >> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile >> index 70f532e..a5a3479 100644 >> --- a/xen/arch/arm/Makefile >> +++ b/xen/arch/arm/Makefile >> @@ -83,11 +83,16 @@ else >> all_symbols = >> endif >> >> +ifeq ($(armds),y) >> +$(TARGET): $(TARGET)-syms >> + fromelf --bin $< --output $@ >> +else >> $(TARGET): $(TARGET)-syms >> $(OBJCOPY) -O binary -S $< $@ >> ifeq ($(CONFIG_ARM_64),y) >> ln -sf $(notdir $@) ../../$(notdir $@).efi >> endif >> +endif >> >> ifeq ($(CONFIG_LTO),y) >> # Gather all LTO objects together >> @@ -102,6 +107,19 @@ prelink.o: $(ALL_OBJS) >> $(LD) $(LDFLAGS) -r -o $@ $^ >> endif >> >> +ifeq ($(armds),y) >> +$(TARGET)-syms: prelink.o xen.scat >> + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 >> + $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ >> + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S >> + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o >> + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 >> + $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ >> + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S >> + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o >> + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib --symdefs="$(@D)/$(@F).map" $(LDFLAGS) prelink.o $(build_id_linker) $(@D)/.$(@F).1.o -o $@ >> + rm -f $(@D)/.$(@F).[0-9]* >> +else >> $(TARGET)-syms: prelink.o xen.lds >> $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ >> $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 >> @@ -119,14 +137,20 @@ $(TARGET)-syms: prelink.o xen.lds >> | $(BASEDIR)/tools/symbols --xensyms --sysv --sort \ >> >$(@D)/$(@F).map >> rm -f $(@D)/.$(@F).[0-9]* >> +endif >> >> asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c >> $(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $< >> >> +ifeq ($(armds),y) >> +xen.scat: xen.scat.S >> + $(CC) -P -E --target=aarch64-arm-none-eabi -o $@ $< >> +else >> xen.lds: xen.lds.S >> $(CC) -P -E -Ui386 $(AFLAGS) -o $@ $< >> sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new >> mv -f .xen.lds.d.new .xen.lds.d >> +endif >> >> dtb.o: $(CONFIG_DTB_FILE) >> >> diff --git a/xen/arch/arm/xen.scat.S b/xen/arch/arm/xen.scat.S >> new file mode 100644 >> index 0000000..3bb405f >> --- /dev/null >> +++ b/xen/arch/arm/xen.scat.S >> @@ -0,0 +1,266 @@ >> +#if 0 > > #if 0 must be a mistake? It is here because of RFC. Because I do not like the hardcodes below, can't find the better way, but eager for suggestions. > > Also, is this basically the ARMCC version of a linker script? Is > xen.lds.S still used with ARMCC, or only xen.scat.S is used? It is the ARMCC version of a linker script. xen.lds.S is not used. > > >> +/* >> + * armlink does not understand shifts in scat file expressions >> + * so hardcode needed values >> + */ >> +#include <xen/cache.h> >> +#include <asm/page.h> >> +#include <asm/percpu.h> >> +#undef ENTRY >> +#undef ALIGN >> +#else >> + #define PAGE_SIZE 4096 >> + #define POINTER_ALIGN 8 >> + #define SMP_CACHE_BYTES 128 >> + #define STACK_SIZE 32768 >> + #define XEN_VIRT_START 0x00200000 >> +#endif >> + >> +LOAD XEN_VIRT_START >> +{ >> +;_start >> +;_stext >> + _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090 >> + { >> + *(.text*) >> + *(.text.cold) >> + *(.text.unlikely) >> + *(.fixup) >> + *(.gnu.warning) >> + } >> +;_etext >> + >> +;_srodata >> +;__start_bug_frames >> + _rodata_bug_frames_0 AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD >> + { >> + *(.bug_frames.0) >> + } >> +;__stop_bug_frames_0 >> + >> + _rodata_bug_frames_1 +0 FIXED ZEROPAD >> + { >> + *(.bug_frames.1) >> + } >> +;__stop_bug_frames_1 >> + >> + _rodata_bug_frames_2 +0 FIXED ZEROPAD >> + { >> + *(.bug_frames.2) >> + } >> +;__stop_bug_frames_2 >> + >> + _rodata_data +0 >> + { >> + *(.rodata) >> + *(.rodata.*) >> + *(.data.rel.ro) >> + *(.data.rel.ro.*) >> + } >> + >> +#ifdef CONFIG_LOCK_PROFILE >> +;__lock_profile_start >> + _rodata_lockprofile_data AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD >> + { >> + *(.lockprofile.data) >> + } >> +;__lock_profile_end >> +#endif > > Should be below OK. > > >> +;__param_start >> + _rodata_data_param AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD >> + { >> + *(.data.param) >> + } >> +;__param_end >> + >> +;__proc_info_start >> + _rodata_proc_info +0 FIXED ZEROPAD >> + { >> + *(.proc.info) >> + } >> +;__proc_info_end >> + >> +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) >> +;__start_vpci_array >> + _rodata_data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD >> + { >> + *(SORT(.data.vpci.*)) >> + } >> +;__end_vpci_array >> +#endif >> + >> +#if defined(BUILD_ID) >> +;__note_gnu_build_id_start >> + _note_gnu_build_id +0 FIXED ZEROPAD >> + { >> + *(.note.gnu.build-id) >> + } >> +;__note_gnu_build_id_end >> +#endif >> + >> +;_erodata >> + >> + _data AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD >> + { >> + *(.data.page_aligned.*) >> + *(.data.*) >> + } >> + >> +;__start_schedulers_array >> + _data_schedulers AlignExpr(+0, 8) FIXED ZEROPAD >> + { >> + *(.data.schedulers) >> + } >> +;__end_schedulers_array >> + >> + _data_rel +0 FIXED ZEROPAD >> + { >> + *(.data.rel) >> + *(.data.rel.*) >> +;#CONSTRUCTORS ???? > > Honestly I am not sure what this is either Ah, it was a hint for myself to find place for constructors. > > >> + } >> + >> +;__start___ex_table >> + _data_ex_table AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD >> + { >> + *(.ex_table) >> + } >> +;__stop___ex_table >> + >> +;__start___pre_ex_table >> + _data_ex_table_pre +0 FIXED ZEROPAD >> + { >> + *(.ex_table.pre) >> + } >> +;__stop___pre_ex_table >> + >> + _data_read_mostly +0 FIXED ZEROPAD >> + { >> + *(.data.read_mostly) >> + } >> + >> +;_splatform >> + _arch_info AlignExpr(+0, 8) FIXED ZEROPAD >> + { >> + *(.arch.info) >> + } >> +;_eplatform >> + >> +;_sdevice >> + _dev_info AlignExpr(+0, 8) FIXED ZEROPAD >> + { >> + *(.dev.info) >> + } >> +;_edevice >> + >> +;_asdevice >> + _adev_info AlignExpr(+0, 8) FIXED ZEROPAD >> + { >> + *(.adev.info) >> + } >> +;_aedevice > > _steemediator/_eteemediator I suppose I did this .scat before _steemediator/_eteemediator were introduced to .lds. OK. > >> +;__init_begin >> +;_sinittext >> + _init_text AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD >> + { >> + *(.init.text) >> + } >> +;_einittext >> + >> + _init_rodata AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD >> + { >> + *(.init.rodata) >> + *(.init.rodata.rel) >> + *(.init.rodata.str*) >> + } >> + >> +;__setup_start >> + _init_setup AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD >> + { >> + *(.init.setup) >> + } >> +;__setup_end >> + >> +;__initcall_start >> + _initcallpresmp_init +0 FIXED ZEROPAD >> + { >> + *(.initcallpresmp.init) >> + } >> +;__presmp_initcall_end >> + >> + _initcall1_init +0 FIXED ZEROPAD >> + { >> + *(.initcall1.init) >> + } >> +;__initcall_end >> + >> +;__alt_instructions >> + _altinstructions AlignExpr(+0, 4) FIXED ZEROPAD >> + { >> + *(.altinstructions) >> + } >> +;__alt_instructions_end >> + >> + _altinstr_replacement AlignExpr(+0, 4) FIXED ZEROPAD >> + { >> + *(.altinstr_replacement) >> + } > > __lock_profile_start should be here OK. > > >> + >> + _init_data +0 FIXED ZEROPAD >> + { >> + *(.init.data) >> + *(.init.data.rel) >> + *(.init.data.rel.*) >> + } >> + >> +;__ctors_start >> + _ctors AlignExpr(+0, 8) FIXED ZEROPAD >> + { >> + *(.ctors) >> + *(.init_array) >> + } >> + >> + _init_array_sorted AlignExpr(+0, 8) SORTTYPE Lexical FIXED ZEROPAD >> + { >> + *(.init_array.*) >> + } >> +;__ctors_end >> + >> +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) >> + _data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD >> + { >> + *(.data.vpci.*) >> + } >> +#endif > > __start_vpci_array/__end_vpci_array? OK. > >> +;__init_end_efi >> + >> +;__init_end >> +;__bss_start >> + _bss AlignExpr(+0, STACK_SIZE) FIXED ZEROPAD >> + { >> + *(.bss.stack_aligned*) >> + *(.bss.page_aligned*, OVERALIGN PAGE_SIZE) >> + *(.bss*) >> + } >> + >> +;__per_cpu_start > > this should be page aligned too? > >> + _bss_percpu AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD >> + { >> + *(.bss.percpu) >> + *(.bss.percpu.read_mostly, OVERALIGN SMP_CACHE_BYTES) >> + } >> +;__per_cpu_data_end >> +;__bss_end > > __bss_end should be page aligned? Well... Maybe, but I'm not sure how to do that. > >> +;_end >> + >> +#ifdef CONFIG_DTB_FILE >> +;_sdtb >> + _dtb FIXED ZEROPAD >> + { >> + *(.dtb) >> + } >> +#endif >> + >> +} >> diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer >> new file mode 100644 >> index 0000000..646e912 >> --- /dev/null >> +++ b/xen/arch/arm/xen.steer >> @@ -0,0 +1,5 @@ >> +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base >> +RENAME Load$$_text$$Base AS _stext >> +RENAME Load$$_text$$Limit AS _etext >> +RENAME Load$$_init_text$$Base AS _sinittext >> +RENAME Load$$_init_text$$Limit AS _einittext > > I don't get why some if the "symbols" get renamed using RENAME here, and > some other we are using a #define in xen/include/asm-arm/armds.h. Can't > we rename them all here? Well, the situation here is really complicated. I described above a reason why _srodata is resolved here. Other symbols are renamed here because the tool "xen/tools/symbols", used at the latest linking stages, needs `_stext`, `_etext`, and the rest to be present in the elf. > >> diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h >> new file mode 100644 >> index 0000000..5ee2e5d >> --- /dev/null >> +++ b/xen/include/asm-arm/armds.h > > Missing guards. Also, probably you want to make sure this is only #ifdef > ARMCC. OK. > > Is this meant to be used when building C files, asm files, or both? Well, I have to check. > > I would avoid this header file if we can get away with just xen.steer. We can't go with xen.steer only. One of the armlink issues is "ARM linker defined symbols are not counted as referred if only mentioned in a steering file for rename or resolve". Also, linker-defined symbols are only generated when the code references them [2]. I tried resolving existing symbols (e.g. _start) to armlink defined symbols with .steer only, and got errors that armlink can't find those linker-defined symbols. I tried a specific C file with references to all needed linker-defined symbols, then resolving all .lds-style symbols to armlink defined symbols with the steering file. But it did not work, I don't remember exactly the issue. Maybe C file with externs only (without using them in the effective code) did not result in an object file referring those linker-defined symbols. > > >> @@ -0,0 +1,91 @@ >> +#define _start Load$$_text$$Base >> +#define _stext Load$$_text$$Base >> + >> +#define _etext Load$$_text$$Limit >> + >> +//#define _srodata Load$$_rodata_bug_frames_0$$Base >> +#define __start_bug_frames Load$$_rodata_bug_frames_0$$Base >> + >> +#define __stop_bug_frames_0 Load$$_rodata_bug_frames_0$$Limit >> +#define __stop_bug_frames_1 Load$$_rodata_bug_frames_1$$Limit >> +#define __stop_bug_frames_2 Load$$_rodata_bug_frames_2$$Limit >> + >> +#ifdef CONFIG_LOCK_PROFILE >> +#define __lock_profile_start Load$$_rodata_lockprofile_data$$Base >> +#define __lock_profile_end Load$$_rodata_lockprofile_data$$Limit >> +#endif >> + >> +#define __param_start Load$$_rodata_data_param$$Base >> +#define __param_end Load$$_rodata_data_param$$Limit >> + >> +#define __proc_info_start Load$$_rodata_proc_info$$Base >> +#define __proc_info_end Load$$_rodata_proc_info$$Limit >> + >> +#define _erodata Load$$_rodata_proc_info$$Limit >> + >> +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) >> +#define __start_vpci_array Load$$_rodata_data_vpci$$Base >> +#define __end_vpci_array Load$$_rodata_data_vpci$$Limit >> + >> +#undef _erodata >> +#define _erodata Load$$_rodata_data_vpci$$Limit >> +#endif >> + >> +#if defined(BUILD_ID) >> +#define __note_gnu_build_id_start Load$$_note_gnu_build_id$$Base >> +#define __note_gnu_build_id_end Load$$_note_gnu_build_id$$Limit >> + >> +#undef _erodata >> +#define _erodata Load$$_note_gnu_build_id$$Limit >> +#endif >> + >> +#define __start_schedulers_array Load$$_data_schedulers$$Base >> +#define __end_schedulers_array Load$$_data_schedulers$$Limit >> + >> +/* Does not exist for ARM >> +#define __start___ex_table Load$$_data_ex_table$$Base >> +#define __stop___ex_table Load$$_data_ex_table$$Limit >> +*/ >> + >> +#define __start___pre_ex_table Load$$_data_ex_table_pre$$Base >> +#define __stop___pre_ex_table Load$$_data_ex_table_pre$$Limit >> + >> +#define _splatform Load$$_arch_info$$Base >> +#define _eplatform Load$$_arch_info$$Limit >> + >> +#define _sdevice Load$$_dev_info$$Base >> +#define _edevice Load$$_dev_info$$Limit >> + >> +#define _asdevice Load$$_adev_info$$Base >> +#define _aedevice Load$$_adev_info$$Limit >> + >> +#define __init_begin Load$$_init_text$$Base >> +#define _sinittext Load$$_init_text$$Base >> +#define _einittext Load$$_init_text$$Limit >> + >> +#define __setup_start Load$$_init_setup$$Base >> +#define __setup_end Load$$_init_setup$$Limit >> + >> +#define __initcall_start Load$$_initcallpresmp_init$$Base >> +#define __presmp_initcall_end Load$$_initcallpresmp_init$$Limit >> +#define __initcall_end Load$$_initcall1_init$$Limit >> + >> +#define __alt_instructions Load$$_altinstructions$$Base >> +#define __alt_instructions_end Load$$_altinstructions$$Limit >> + >> +#define __ctors_start Load$$_ctors$$Base >> +#define __ctors_end Load$$_init_array_sorted$$Limit >> +#define __init_end_efi Load$$_init_array_sorted$$Limit >> + >> +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) >> +#undef __init_end_efi >> +#define __init_end_efi Load$$_data_vpci$$Limit >> +#endif >> + >> +#define __init_end Load$$_bss$$Base >> +#define __bss_start Load$$_bss$$Base >> + >> +#define __per_cpu_start Load$$_bss_percpu$$Base >> +#define __per_cpu_data_end Load$$_bss_percpu$$Limit >> +#define __bss_end Load$$_bss_percpu$$Limit >> +#define _end Load$$_bss_percpu$$Limit [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0803j/rbm1505486312921.html [2] http://infocenter.arm.com/help/topic/com.arm.doc.dui0803j/nbd1509536435303.html
On 13.11.19 07:50, Julien Grall wrote: > To be honest, I don't think this file should even exist. This looks like a copy of xen.lds.S with a different syntax. And lacking features like symbols definition, current address setup, etc. > Furthermore, the comments from Stefano shows that is going to be hard to maintain/check everything has been written correctly. It will be terribly hard. > So how about trying to abstract xen.lds.S? I failed to find the common ground for them. You are very welcomed to suggest that piece of code. > > +/* > > + * armlink does not understand shifts in scat file expressions > > + * so hardcode needed values > > + */ > > > Please give a pointer to the doc of the armlink in the commit message. So we can easily cross-check what's happening. The best cross-check would be running the compiler. Yet, this particular thing is somehow documented [1]. > In this case, I don't particularly like the re-definition of the defines outside of their header. This is going to make more difficult if we have to update them in the future. > > I can see a few ways to do it: > > - Avoid using shifts in the definitions > - Find a way to evaluate the value (maybe similar to asn-offset) before using them. > > My preference would be the latter but I could be convinced for the former. The first option is not realistic. I suggested ARM to consider shifts support as an improvement for their compiler. I'd be very happy to adopt the second option. Do you have any code examples or hints how to evaluate expressions on the pre-processing stage? [1] https://developer.arm.com/docs/100070/0612/scatter-file-syntax/expression-evaluation-in-scatter-files/expression-rules-in-scatter-files
Hi Andrii, On 14/11/2019 11:31, Andrii Anisov wrote: > > > On 13.11.19 07:50, Julien Grall wrote: >> To be honest, I don't think this file should even exist. This looks >> like a copy of xen.lds.S with a different syntax. > > And lacking features like symbols definition, current address setup, etc. That's fine, you can introduce macro that will just be a NOP for armlinker and implemented for ld. > >> Furthermore, the comments from Stefano shows that is going to be hard >> to maintain/check everything has been written correctly. > > It will be terribly hard. > >> So how about trying to abstract xen.lds.S? > > I failed to find the common ground for them. > You are very welcomed to suggest that piece of code. I don't have time to work on the full feature, but I can provide a few ideas so you can implement it. If we look at the .text section. For the GNU linker script, it is: _start = .; .text : { _stext = .; /* Text section */ *(.text) *(.text.cold) *(.text.unlikely) *(.fixup) *(.gnu.warning) _etext = .; /* End of text section */ } :text = 0x9090 For armds, you implement as ;_start ;_stext _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090 { *(.text*) *(.text.cold) *(.text.unlikely) *(.fixup) *(.gnu.warning) } ;_etext You could imagine the following abstraction: SYMBOL(_start) SECTION(_text) SYMBOL(_stext) *(.text) *(.text.cold) *(.text.unlikely) *(.fixup) *(.gnu.warning) SYMBOL(_etext) ESECTION(text) For GNU linker scripts, the macros would be implemented as #define SYMBOL(sym) sym := .; #define SECTION(sect) sect : { #define ESECTION(phdr) } :phdr = 0x9090 For the Arm scatter file, the macros would be implemented as /* Symbols are not declared in the scatter file */ #define SYMBOL(sym) ;sym #define SECTION(sect) sect AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090 { #define ESECTION(phdr) } A few caveats: - I am not entirely sure why we specific the pading value only for .text and not the other. I also don't understand the 0x9090 value. Stefano? It may be possible to remove this completely. - The alignment could be passed as a parameter for the macro - The linker script may need some reshuffle in order to make it generic. On a side note, I noticed that you are using *(.text*), rather than *(.text). Could you explain why? > > >> > +/* >> > + * armlink does not understand shifts in scat file expressions >> > + * so hardcode needed values >> > + */ >> >> >> Please give a pointer to the doc of the armlink in the commit message. >> So we can easily cross-check what's happening. > > The best cross-check would be running the compiler. Yet, this particular > thing is somehow documented [1]. > >> In this case, I don't particularly like the re-definition of the >> defines outside of their header. This is going to make more difficult >> if we have to update them in the future. >> >> I can see a few ways to do it: >> >> - Avoid using shifts in the definitions >> - Find a way to evaluate the value (maybe similar to asn-offset) >> before using them. >> >> My preference would be the latter but I could be convinced for the >> former. > > The first option is not realistic. I suggested ARM to consider shifts > support as an improvement for their compiler. > I'd be very happy to adopt the second option. Do you have any code > examples or hints how to evaluate expressions on the pre-processing stage? I wasn't thinking to do it a pre-processing state but rather generating an header that will have the value already computed. Have a look at how we generate defininition for assembly (see arch/arm32/asm-offsets.c). Looking at the link you provided, I noticed that * is supported by the scatter file. So it might be possible to introduce a macro LSHIFT(a, b) that will be implemented as a << b or (a * a * a ...) depending on the users. The major difficulty here would be to find a way to generate a * a * a... nicely during pre-processing. Cheers,
On Thu, 14 Nov 2019, Andrii Anisov wrote: > On 11.11.19 23:26, Stefano Stabellini wrote: > > > Why _srodata and __start_bug_frames cannot both be defined as > > Load$$_rodata_bug_frames_0$$Base when actually in the case of: > > > > > +#define __per_cpu_data_end Load$$_bss_percpu$$Limit > > > +#define __bss_end Load$$_bss_percpu$$Limit > > > +#define _end Load$$_bss_percpu$$Limit > > > > They are all defined as "Load$$_bss_percpu$$Limit"? And: > > > > > +#define __init_end Load$$_bss$$Base > > > +#define __bss_start Load$$_bss$$Base > > > > They are both defined as "Load$$_bss$$Base"? What's special about > > "Load$$_rodata_bug_frames_0$$Base"? > > > Because in xen/include/asm-arm/bug.h: > extern const struct bug_frame __start_bug_frames[] > > and in xen/include/xen/kernel.h > extern const char _srodata[]; > > After preprocessing we, effectively, have symbol declared with conflicting > types: > extern const struct bug_frame Load$$_rodata_bug_frames_0$$Base[]; > extern const char Load$$_rodata_bug_frames_0$$Base[]; > > Eventually other symbols do not have such conflicts. That is something to add to the commit message > > > - C style shift operators are missed among supported scatter file > > > expressions, > > > so some needed values are hardcoded in scatter file. > > > > > > - Rename correspondent ARM Linker defined symbols to those needed by > > > `symbols` tool > > > using steering file feature. > > > > > > - ARM Compiler 6.6 tools are not able to rename sections, so we still > > > need > > > GNU toolchain's objcopy for this. > > > > > > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com> > > > --- > > > xen/Rules.mk | 6 + > > > xen/arch/arm/Makefile | 24 ++++ > > > xen/arch/arm/xen.scat.S | 266 > > > ++++++++++++++++++++++++++++++++++++++++++++ > > > > I would strongly suggest to rename this file with something not > > potentially related to scat > > I just followed examples from ARM documentation, e.g. [1]. I suppose they know > something about their product. > Yet, the suggestion is reasonable. LOL!! > > > diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer > > > new file mode 100644 > > > index 0000000..646e912 > > > --- /dev/null > > > +++ b/xen/arch/arm/xen.steer > > > @@ -0,0 +1,5 @@ > > > +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base > > > +RENAME Load$$_text$$Base AS _stext > > > +RENAME Load$$_text$$Limit AS _etext > > > +RENAME Load$$_init_text$$Base AS _sinittext > > > +RENAME Load$$_init_text$$Limit AS _einittext > > > > I don't get why some if the "symbols" get renamed using RENAME here, and > > some other we are using a #define in xen/include/asm-arm/armds.h. Can't > > we rename them all here? > > Well, the situation here is really complicated. I described above a reason why > _srodata is resolved here. > Other symbols are renamed here because the tool "xen/tools/symbols", used at > the latest linking stages, needs `_stext`, `_etext`, and the rest to be > present in the elf. > > > > > > diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h > > > new file mode 100644 > > > index 0000000..5ee2e5d > > > --- /dev/null > > > +++ b/xen/include/asm-arm/armds.h > > > > Missing guards. Also, probably you want to make sure this is only #ifdef > > ARMCC. > > OK. > > > > > Is this meant to be used when building C files, asm files, or both? > > Well, I have to check. > > > > > I would avoid this header file if we can get away with just xen.steer. > > We can't go with xen.steer only. One of the armlink issues is "ARM linker > defined symbols are not counted as referred if only mentioned in a steering > file for rename or resolve". Also, linker-defined symbols are only generated > when the code references them [2]. > I tried resolving existing symbols (e.g. _start) to armlink defined symbols > with .steer only, and got errors that armlink can't find those linker-defined > symbols. > I tried a specific C file with references to all needed linker-defined > symbols, then resolving all .lds-style symbols to armlink defined symbols with > the steering file. But it did not work, I don't remember exactly the issue. > Maybe C file with externs only (without using them in the effective code) did > not result in an object file referring those linker-defined symbols. I don't know what the right solution is, but it would be nice to have some consistency. Otherwise the next time a new symbol is added we won't know whether it needs to be added to xen.steer or armds.h. We need to have a clear rule to follow so that we can easily figure out why each symbol is in xen.steer and/or armds.h.
diff --git a/xen/Rules.mk b/xen/Rules.mk index 41a1c26..67bedce 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -60,6 +60,12 @@ CFLAGS += -nostdinc -fno-builtin -fno-common CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith $(call cc-option-add,CFLAGS,CC,-Wvla) CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h + +ifeq ($(armds),y) +CFLAGS += -nostdlibinc -nostdlib -Wno-unused-command-line-argument +CFLAGS += -include $(BASEDIR)/include/asm/armds.h +endif + CFLAGS-$(CONFIG_DEBUG_INFO) += -g CFLAGS += '-D__OBJECT_FILE__="$@"' diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 70f532e..a5a3479 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -83,11 +83,16 @@ else all_symbols = endif +ifeq ($(armds),y) +$(TARGET): $(TARGET)-syms + fromelf --bin $< --output $@ +else $(TARGET): $(TARGET)-syms $(OBJCOPY) -O binary -S $< $@ ifeq ($(CONFIG_ARM_64),y) ln -sf $(notdir $@) ../../$(notdir $@).efi endif +endif ifeq ($(CONFIG_LTO),y) # Gather all LTO objects together @@ -102,6 +107,19 @@ prelink.o: $(ALL_OBJS) $(LD) $(LDFLAGS) -r -o $@ $^ endif +ifeq ($(armds),y) +$(TARGET)-syms: prelink.o xen.scat + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 + $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib $(LDFLAGS) prelink.o $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 + $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o + armlink --scatter="xen.scat" --edit="xen.steer" --no_scanlib --symdefs="$(@D)/$(@F).map" $(LDFLAGS) prelink.o $(build_id_linker) $(@D)/.$(@F).1.o -o $@ + rm -f $(@D)/.$(@F).[0-9]* +else $(TARGET)-syms: prelink.o xen.lds $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 @@ -119,14 +137,20 @@ $(TARGET)-syms: prelink.o xen.lds | $(BASEDIR)/tools/symbols --xensyms --sysv --sort \ >$(@D)/$(@F).map rm -f $(@D)/.$(@F).[0-9]* +endif asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $< +ifeq ($(armds),y) +xen.scat: xen.scat.S + $(CC) -P -E --target=aarch64-arm-none-eabi -o $@ $< +else xen.lds: xen.lds.S $(CC) -P -E -Ui386 $(AFLAGS) -o $@ $< sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new mv -f .xen.lds.d.new .xen.lds.d +endif dtb.o: $(CONFIG_DTB_FILE) diff --git a/xen/arch/arm/xen.scat.S b/xen/arch/arm/xen.scat.S new file mode 100644 index 0000000..3bb405f --- /dev/null +++ b/xen/arch/arm/xen.scat.S @@ -0,0 +1,266 @@ +#if 0 +/* + * armlink does not understand shifts in scat file expressions + * so hardcode needed values + */ +#include <xen/cache.h> +#include <asm/page.h> +#include <asm/percpu.h> +#undef ENTRY +#undef ALIGN +#else + #define PAGE_SIZE 4096 + #define POINTER_ALIGN 8 + #define SMP_CACHE_BYTES 128 + #define STACK_SIZE 32768 + #define XEN_VIRT_START 0x00200000 +#endif + +LOAD XEN_VIRT_START +{ +;_start +;_stext + _text AlignExpr(+0, PAGE_SIZE) PADVALUE 0x9090 + { + *(.text*) + *(.text.cold) + *(.text.unlikely) + *(.fixup) + *(.gnu.warning) + } +;_etext + +;_srodata +;__start_bug_frames + _rodata_bug_frames_0 AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD + { + *(.bug_frames.0) + } +;__stop_bug_frames_0 + + _rodata_bug_frames_1 +0 FIXED ZEROPAD + { + *(.bug_frames.1) + } +;__stop_bug_frames_1 + + _rodata_bug_frames_2 +0 FIXED ZEROPAD + { + *(.bug_frames.2) + } +;__stop_bug_frames_2 + + _rodata_data +0 + { + *(.rodata) + *(.rodata.*) + *(.data.rel.ro) + *(.data.rel.ro.*) + } + +#ifdef CONFIG_LOCK_PROFILE +;__lock_profile_start + _rodata_lockprofile_data AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD + { + *(.lockprofile.data) + } +;__lock_profile_end +#endif + +;__param_start + _rodata_data_param AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD + { + *(.data.param) + } +;__param_end + +;__proc_info_start + _rodata_proc_info +0 FIXED ZEROPAD + { + *(.proc.info) + } +;__proc_info_end + +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) +;__start_vpci_array + _rodata_data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD + { + *(SORT(.data.vpci.*)) + } +;__end_vpci_array +#endif + +#if defined(BUILD_ID) +;__note_gnu_build_id_start + _note_gnu_build_id +0 FIXED ZEROPAD + { + *(.note.gnu.build-id) + } +;__note_gnu_build_id_end +#endif + +;_erodata + + _data AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD + { + *(.data.page_aligned.*) + *(.data.*) + } + +;__start_schedulers_array + _data_schedulers AlignExpr(+0, 8) FIXED ZEROPAD + { + *(.data.schedulers) + } +;__end_schedulers_array + + _data_rel +0 FIXED ZEROPAD + { + *(.data.rel) + *(.data.rel.*) +;#CONSTRUCTORS ???? + } + +;__start___ex_table + _data_ex_table AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD + { + *(.ex_table) + } +;__stop___ex_table + +;__start___pre_ex_table + _data_ex_table_pre +0 FIXED ZEROPAD + { + *(.ex_table.pre) + } +;__stop___pre_ex_table + + _data_read_mostly +0 FIXED ZEROPAD + { + *(.data.read_mostly) + } + +;_splatform + _arch_info AlignExpr(+0, 8) FIXED ZEROPAD + { + *(.arch.info) + } +;_eplatform + +;_sdevice + _dev_info AlignExpr(+0, 8) FIXED ZEROPAD + { + *(.dev.info) + } +;_edevice + +;_asdevice + _adev_info AlignExpr(+0, 8) FIXED ZEROPAD + { + *(.adev.info) + } +;_aedevice + +;__init_begin +;_sinittext + _init_text AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD + { + *(.init.text) + } +;_einittext + + _init_rodata AlignExpr(+0, PAGE_SIZE) FIXED ZEROPAD + { + *(.init.rodata) + *(.init.rodata.rel) + *(.init.rodata.str*) + } + +;__setup_start + _init_setup AlignExpr(+0, POINTER_ALIGN) FIXED ZEROPAD + { + *(.init.setup) + } +;__setup_end + +;__initcall_start + _initcallpresmp_init +0 FIXED ZEROPAD + { + *(.initcallpresmp.init) + } +;__presmp_initcall_end + + _initcall1_init +0 FIXED ZEROPAD + { + *(.initcall1.init) + } +;__initcall_end + +;__alt_instructions + _altinstructions AlignExpr(+0, 4) FIXED ZEROPAD + { + *(.altinstructions) + } +;__alt_instructions_end + + _altinstr_replacement AlignExpr(+0, 4) FIXED ZEROPAD + { + *(.altinstr_replacement) + } + + _init_data +0 FIXED ZEROPAD + { + *(.init.data) + *(.init.data.rel) + *(.init.data.rel.*) + } + +;__ctors_start + _ctors AlignExpr(+0, 8) FIXED ZEROPAD + { + *(.ctors) + *(.init_array) + } + + _init_array_sorted AlignExpr(+0, 8) SORTTYPE Lexical FIXED ZEROPAD + { + *(.init_array.*) + } +;__ctors_end + +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) + _data_vpci AlignExpr(+0, POINTER_ALIGN) SORTTYPE Lexical FIXED ZEROPAD + { + *(.data.vpci.*) + } +#endif +;__init_end_efi + +;__init_end +;__bss_start + _bss AlignExpr(+0, STACK_SIZE) FIXED ZEROPAD + { + *(.bss.stack_aligned*) + *(.bss.page_aligned*, OVERALIGN PAGE_SIZE) + *(.bss*) + } + +;__per_cpu_start + _bss_percpu AlignExpr(+0, SMP_CACHE_BYTES) FIXED ZEROPAD + { + *(.bss.percpu) + *(.bss.percpu.read_mostly, OVERALIGN SMP_CACHE_BYTES) + } +;__per_cpu_data_end +;__bss_end +;_end + +#ifdef CONFIG_DTB_FILE +;_sdtb + _dtb FIXED ZEROPAD + { + *(.dtb) + } +#endif + +} diff --git a/xen/arch/arm/xen.steer b/xen/arch/arm/xen.steer new file mode 100644 index 0000000..646e912 --- /dev/null +++ b/xen/arch/arm/xen.steer @@ -0,0 +1,5 @@ +RESOLVE _srodata AS Load$$_rodata_bug_frames_0$$Base +RENAME Load$$_text$$Base AS _stext +RENAME Load$$_text$$Limit AS _etext +RENAME Load$$_init_text$$Base AS _sinittext +RENAME Load$$_init_text$$Limit AS _einittext diff --git a/xen/include/asm-arm/armds.h b/xen/include/asm-arm/armds.h new file mode 100644 index 0000000..5ee2e5d --- /dev/null +++ b/xen/include/asm-arm/armds.h @@ -0,0 +1,91 @@ +#define _start Load$$_text$$Base +#define _stext Load$$_text$$Base + +#define _etext Load$$_text$$Limit + +//#define _srodata Load$$_rodata_bug_frames_0$$Base +#define __start_bug_frames Load$$_rodata_bug_frames_0$$Base + +#define __stop_bug_frames_0 Load$$_rodata_bug_frames_0$$Limit +#define __stop_bug_frames_1 Load$$_rodata_bug_frames_1$$Limit +#define __stop_bug_frames_2 Load$$_rodata_bug_frames_2$$Limit + +#ifdef CONFIG_LOCK_PROFILE +#define __lock_profile_start Load$$_rodata_lockprofile_data$$Base +#define __lock_profile_end Load$$_rodata_lockprofile_data$$Limit +#endif + +#define __param_start Load$$_rodata_data_param$$Base +#define __param_end Load$$_rodata_data_param$$Limit + +#define __proc_info_start Load$$_rodata_proc_info$$Base +#define __proc_info_end Load$$_rodata_proc_info$$Limit + +#define _erodata Load$$_rodata_proc_info$$Limit + +#if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) +#define __start_vpci_array Load$$_rodata_data_vpci$$Base +#define __end_vpci_array Load$$_rodata_data_vpci$$Limit + +#undef _erodata +#define _erodata Load$$_rodata_data_vpci$$Limit +#endif + +#if defined(BUILD_ID) +#define __note_gnu_build_id_start Load$$_note_gnu_build_id$$Base +#define __note_gnu_build_id_end Load$$_note_gnu_build_id$$Limit + +#undef _erodata +#define _erodata Load$$_note_gnu_build_id$$Limit +#endif + +#define __start_schedulers_array Load$$_data_schedulers$$Base +#define __end_schedulers_array Load$$_data_schedulers$$Limit + +/* Does not exist for ARM +#define __start___ex_table Load$$_data_ex_table$$Base +#define __stop___ex_table Load$$_data_ex_table$$Limit +*/ + +#define __start___pre_ex_table Load$$_data_ex_table_pre$$Base +#define __stop___pre_ex_table Load$$_data_ex_table_pre$$Limit + +#define _splatform Load$$_arch_info$$Base +#define _eplatform Load$$_arch_info$$Limit + +#define _sdevice Load$$_dev_info$$Base +#define _edevice Load$$_dev_info$$Limit + +#define _asdevice Load$$_adev_info$$Base +#define _aedevice Load$$_adev_info$$Limit + +#define __init_begin Load$$_init_text$$Base +#define _sinittext Load$$_init_text$$Base +#define _einittext Load$$_init_text$$Limit + +#define __setup_start Load$$_init_setup$$Base +#define __setup_end Load$$_init_setup$$Limit + +#define __initcall_start Load$$_initcallpresmp_init$$Base +#define __presmp_initcall_end Load$$_initcallpresmp_init$$Limit +#define __initcall_end Load$$_initcall1_init$$Limit + +#define __alt_instructions Load$$_altinstructions$$Base +#define __alt_instructions_end Load$$_altinstructions$$Limit + +#define __ctors_start Load$$_ctors$$Base +#define __ctors_end Load$$_init_array_sorted$$Limit +#define __init_end_efi Load$$_init_array_sorted$$Limit + +#if defined(CONFIG_HAS_VPCI) && !defined(CONFIG_LATE_HWDOM) +#undef __init_end_efi +#define __init_end_efi Load$$_data_vpci$$Limit +#endif + +#define __init_end Load$$_bss$$Base +#define __bss_start Load$$_bss$$Base + +#define __per_cpu_start Load$$_bss_percpu$$Base +#define __per_cpu_data_end Load$$_bss_percpu$$Limit +#define __bss_end Load$$_bss_percpu$$Limit +#define _end Load$$_bss_percpu$$Limit