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

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





On Tue, 12 Nov 2019, 06:27 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:
On Wed, 6 Nov 2019, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@xxxxxxxx>
>
> 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@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

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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