Re: [Xen-devel] [RFC 5/7] WIP:arm64:armds: Build XEN with ARM Compiler 6.6

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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

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