On Tue, 12 Nov 2019, 06:27 Stefano Stabellini, <sstabell...@kernel.org>
wrote:

> On Wed, 6 Nov 2019, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_ani...@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_ani...@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,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to