Re: [RFC PATCH] ld/emulparams: elf32lriscv-defs: Add support tune the text segment start address

[Palmer Dabbelt <palmer@dabbelt.com>]

On Mon, 19 Dec 2022 01:11:13 PST (-0800), prabhakar.csengg@gmail.com wrote:
> Hi Jeff,
>
> On Mon, Dec 19, 2022 at 5:00 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>>
>>
>> On 12/18/22 15:44, Lad, Prabhakar via Binutils wrote:
>> > Hi Fangrui,
>> >
>> > Thank you for the feedback.
>> >
>> > On Sun, Dec 18, 2022 at 6:24 AM Fangrui Song <i@maskray.me> wrote:
>> >>
>> >> On 2022-12-16, Lad Prabhakar via Binutils wrote:
>> >>> On the RISC-V architecture the TEXT_START_ADDR defaults to 0x10000. On
>> >>> some RISC-V platforms we want to set this offset to something else. So
>> >>> this patch provides a way to tune the text segment start address.
>> >>> elf32lriscv-defs.sh now checks for DEFAULT_TEXT_START_ADDR variable and
>> >>> if being set it overrides TEXT_START_ADDR to the value set by
>> >>> DEFAULT_TEXT_START_ADDR or else defaults to 0x10000.
>> >>>
>> >>> Renesas RZ/Five RISC-V SoC has Instruction local memory and Data local
>> >>> memory (ILM & DLM) which maps between region 0x30000 - 0x4FFFF. When the
>> >>> virtual address falls in this range the MMU doesn't trigger a page
>> >>> fault and assumes the virtual address as physical address and causes
>> >>> undesired behaviours of statically applications/libraries. Hence introduce
>> >>> an option to tune the TEXT_START_ADDR.
>> >>>
>> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> >>> ---
>> >>> Hi All,
>> >>>
>> >>> This patch is inspired from the current ld/emulparams/nds32elf_linux.sh file
>> >>> where similar approach is being used and DEFAULT_TEXT_START_ADDR variable is
>> >>> checked to adjust the TEXT_START_ADDR for the platform.
>> >>>
>> >>> I am not sure if this is the right approach the above issue has been discussed
>> >>> on the ML [0].
>> >>>
>> >>> [0] https://sourceware.org/pipermail/binutils/2022-November/124813.html
>> >>>
>> >>> Cheers,
>> >>> Prabhakar
>> >>> ---
>> >>> ld/emulparams/elf32lriscv-defs.sh | 6 +++++-
>> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/ld/emulparams/elf32lriscv-defs.sh b/ld/emulparams/elf32lriscv-defs.sh
>> >>> index b823cedacab..026aef4714b 100644
>> >>> --- a/ld/emulparams/elf32lriscv-defs.sh
>> >>> +++ b/ld/emulparams/elf32lriscv-defs.sh
>> >>> @@ -27,7 +27,11 @@ case "$target" in
>> >>> esac
>> >>>
>> >>> IREL_IN_PLT=
>> >>> -TEXT_START_ADDR=0x10000
>> >>> +if [ -z ${DEFAULT_TEXT_START_ADDR+x} ]; then

I'm never great with shell expansion, what does the "+x" do here?

I'm also not sure how this is meant to be used/set, is there some 
autoconf changes that this needs to come along with so this can be set?

>> >>> +    TEXT_START_ADDR=0x10000
>> >>> +else
>> >>> +    TEXT_START_ADDR=$DEFAULT_TEXT_START_ADDR
>> >>> +fi
>> >>> MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
>> >>> COMMONPAGESIZE="CONSTANT (COMMONPAGESIZE)"
>> >>>
>> >>> --
>> >>> 2.17.1
>> >>>
>> >>
>> >> Changing ld does not look like a good change to me.  This tuning can be
>> >> placed at the compiler driver side as that is the usual place to tune
>> >> these things. We can ask users to specify -Wl,-Ttext-segment= (with lld
>> >> use --image-base= instead) .
>> > With the above approach we will have to target each and individual
>> > application/library that is statically compiled.
>> >
>> >> If that is inconvenient, with Clang you can
>> >> add the option to a configuration file
>> >> (https://clang.llvm.org/docs/UsersManual.html#configuration-files).
>> >> With GCC there may be an option to add a fragment to the default specs
>> >> file.
>> > I'm more specifically concerned about yocto/debian builds which use
>> > GCC. With the proposed approach the changes are central and we can
>> > make sure everything will fall in place for yocto/debian rootfs builds
>> > with a single change.
>> >
>> > Also what I get from Palmer is that moving forward we want to adjust
>> > the TEXT_START_ADDR based on the page size (Huge page).
>> Another approach here is to consider this system independent of the
>> other risc-v platforms  and give it its own emulation template rather
>> than polluting the generic risc-v template with this issue.
>>
> Sounds good to me, For example it would include the RISC-V generic
> template and we just override the offset in the platform specific conf
> file. Having said that, I'm not sure of the maintainability of such
> files and RISC-V maintainers will be OK for such change.

I was assuming we'd have a configure option somewhere in the toolchain 
to control the default.  I hadn't really thought of where, either 
binutils or GCC could make sense (as it's driven by a `-Wl` option).  I 
haven't quite sorted out how this modification to ldparams does that (as 
above).

That said, maybe the right answer here is actually to just have 
"-mcpu=renesas-rzfve" just include -Ttext-segement= in LINK_SPEC?  If we 
implement the Linux side of this as "don't mmap() unless the hint says 
to, and then context switch the memory" then it's essentially just a CPU 
tuning parameter (with a giant security hole, but nothing we can do 
about that).

> I'm curious what Palmer has in his mind on tuning this parameter for
> huge pages. If that exposes some sort of config option we could use
> that option to set the offset.

The hugepage thing might not be all that exiciting: it's just a question 
of whether we're defaulting to 2nd-level or 3rd-level paging.  Having 
the default as-is for 2nd-level paging seems reasonable as it gives a 
lot more range for symbols, so maybe nobody even wants to default to the 
3rd-level offsets.

>> I'm leery of including these bits in the generic risc-v code given it's
>> really working around, umm, quirks, mis-features or whatever we might
>> want to call them.
>>
> :-)

I think we have way bigger pollution problems than this.

>
> Cheers,
> Prabhakar