diff mbox series

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

Message ID 20221216214310.13155-1-prabhakar.mahadev-lad.rj@bp.renesas.com
State Accepted
Headers
Series [RFC] ld/emulparams: elf32lriscv-defs: Add support tune the text segment start address |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Prabhakar Mahadev Lad Dec. 16, 2022, 9:43 p.m. UTC 
  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(-)

Comments

Andrew Waterman Dec. 17, 2022, 9:19 a.m. UTC | #1 
I would like to clarify for the record that the purpose of this patch
has little to do with tuning, as that suggests a performance matter
rather than a functional one.  This is a workaround for incompliant
behavior in a processor on the chip that Lad mentioned.

I have no technical objections to this patch, but I think it's
important that we be forthright about our intent.


On Fri, Dec 16, 2022 at 1:43 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> 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
> +    TEXT_START_ADDR=0x10000
> +else
> +    TEXT_START_ADDR=$DEFAULT_TEXT_START_ADDR
> +fi
>  MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
>  COMMONPAGESIZE="CONSTANT (COMMONPAGESIZE)"
>
> --
> 2.17.1
>
Fangrui Song Dec. 18, 2022, 6:24 a.m. UTC | #2 
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
>+    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) . 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.
Frager, Neal via Binutils Dec. 18, 2022, 10:44 p.m. UTC | #3 
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
> >+    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).

Cheers,
Prabhakar
Fangrui Song Dec. 18, 2022, 11:09 p.m. UTC | #4 
On 2022-12-18, Lad, Prabhakar 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
>> >+    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.

Sometimes the opposite. With tunable TEXT_START_ADDR you will need to
ensure every user targeting your platform use the value. It's
inconvenient for users who want to use a generic toolchain instead of a
customized one. The emerging popularity of Clang in some part is it
somewhat discourages such configure-time tuning.

>> 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).

I think GCC provides a way to customize its specs file.
If 0x10000 is not a good value, I suggest just changing it to 0x200000 (lld aarch64/x86) or 0x400000 (ld.bfd aarch64/x86).
Do not provide a customization
Jeff Law Dec. 19, 2022, 5 a.m. UTC | #5 
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
>>> +    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.

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.



Jeff
Frager, Neal via Binutils Dec. 19, 2022, 9:02 a.m. UTC | #6 
Hi Fangrui,

On Sun, Dec 18, 2022 at 11:09 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2022-12-18, Lad, Prabhakar 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
> >> >+    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.
>
> Sometimes the opposite. With tunable TEXT_START_ADDR you will need to
> ensure every user targeting your platform use the value. It's
> inconvenient for users who want to use a generic toolchain instead of a
> customized one. The emerging popularity of Clang in some part is it
> somewhat discourages such configure-time tuning.
>
Agreed the individual user will have to use the toolchain generated by
this change (yocto SDK). On the other hand users can still use the
generic toolchain but only for non statically compiled applications.

> >> 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).
>
> I think GCC provides a way to customize its specs file.
> If 0x10000 is not a good value, I suggest just changing it to 0x200000 (lld aarch64/x86) or 0x400000 (ld.bfd aarch64/x86).
I'll wait for RISC-V maintainers to comment on the above.

Cheers,
Prabhakar
Frager, Neal via Binutils Dec. 19, 2022, 9:11 a.m. UTC | #7 
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
> >>> +    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'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.

> 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.
>
:-)

Cheers,
Prabhakar
Palmer Dabbelt Dec. 19, 2022, 9:33 p.m. UTC | #8 
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
Nelson Chu Dec. 21, 2022, 1:24 p.m. UTC | #9 
On Tue, Dec 20, 2022 at 5:33 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> 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 think it should be similar to what nds32 did in the
ld/emulparams/nds32elf_linux.sh, but I'm not really sure how it works.
When should the DEFAULT_TEXT_START_ADDR be defined?  Maybe when
configure or make, not sure about that.  But at least I only see the
changes in nds32, all of the other targets won't do that, so this must
be a handy little trick, but there are always other ways to do the
same things - for example, what maskray said should be the general way
to do.  I don't have any opinion on this change, if it won't break the
general build, then probably acceptable.  Just that it seems need to
introduce the DEFAULT_TEXT_START_ADDR or whatever configure options,
so this isn't only be the riscv gnu ld's issue, this will also be the
issue of lld, so if the maintainers of lld already have some
suggestions, then it will definitely worth to referring to.

Thanks
Nelson

> 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
Frager, Neal via Binutils Dec. 21, 2022, 8:06 p.m. UTC | #10 
Hi Palmer,

Thank you for the feedback.

On Mon, Dec 19, 2022 at 9:33 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> 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?
>
where ${DEFAULT_TEXT_START_ADDR+x} is a parameter expansion which
evaluates to nothing if var is unset, and substitutes the string x
otherwise

> 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?
>
There is no autoconf for this, so when trying to build with a
different offset we just export DEFAULT_TEXT_START_ADDR in the shell
with a value and then build it. So for example when trying to build
from Yocto we just bbappend the binutils file and export the
DEFAULT_TEXT_START_ADDR variable.

> >> >>> +    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?
So that means we want GCC to accept  "renesas-rzfive" as an mcpu
option and then we update LINK_SPEC [0] based on the mcpu parameter.

[0] https://github.com/gcc-mirror/gcc/blob/releases/gcc-9/gcc/config/riscv/linux.h#L60

Is my understanding correct here?

> 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 not sure how maintainers will react to this if we propose a
solution. I wonder if we are just alone with such a problem.

> > 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.
>
Okay, so I won't play the huge page card ;)

Cheers,
Prabhakar
Frager, Neal via Binutils Dec. 21, 2022, 8:15 p.m. UTC | #11 
Hi Nelson,

Thank you for the feedback.

On Wed, Dec 21, 2022 at 1:24 PM Nelson Chu <nelson@rivosinc.com> wrote:
>
> On Tue, Dec 20, 2022 at 5:33 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > 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 think it should be similar to what nds32 did in the
> ld/emulparams/nds32elf_linux.sh, but I'm not really sure how it works.
> When should the DEFAULT_TEXT_START_ADDR be defined?  Maybe when
> configure or make, not sure about that.  But at least I only see the
> changes in nds32, all of the other targets won't do that, so this must
> be a handy little trick, but there are always other ways to do the
DEFAULT_TEXT_START_ADDR needs to be exported in the shell before doing
configure && make.. For example for testing I bbappended the binutils
yocto recipe and exported the variable in do_configure() so that it
picks up the new offset.

> same things - for example, what maskray said should be the general way
> to do.  I don't have any opinion on this change, if it won't break the
> general build, then probably acceptable.  Just that it seems need to
> introduce the DEFAULT_TEXT_START_ADDR or whatever configure options,
> so this isn't only be the riscv gnu ld's issue, this will also be the
> issue of lld, so if the maintainers of lld already have some
> suggestions, then it will definitely worth to referring to.
>
Is there a mailing list for lld so that we could include them in this
mail thread (I couldn't find one)?

Cheers,
Prabhakar
Nelson Chu Dec. 22, 2022, 2:12 a.m. UTC | #12 
On Thu, Dec 22, 2022 at 4:15 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
>> I think it should be similar to what nds32 did in the
>> ld/emulparams/nds32elf_linux.sh, but I'm not really sure how it works.
>> When should the DEFAULT_TEXT_START_ADDR be defined?  Maybe when
>> configure or make, not sure about that.  But at least I only see the
>> changes in nds32, all of the other targets won't do that, so this must
>> be a handy little trick, but there are always other ways to do the
>>
>DEFAULT_TEXT_START_ADDR needs to be exported in the shell before doing
>configure && make.. For example for testing I bbappended the binutils
>yocto recipe and exported the variable in do_configure() so that it
>picks up the new offset.

Thanks, that pretty clear to explain how this works.

> > same things - for example, what maskray said should be the general way
> > to do.  I don't have any opinion on this change, if it won't break the
> > general build, then probably acceptable.  Just that it seems need to
> > introduce the DEFAULT_TEXT_START_ADDR or whatever configure options,
> > so this isn't only be the riscv gnu ld's issue, this will also be the
> > issue of lld, so if the maintainers of lld already have some
> > suggestions, then it will definitely worth to referring to.
> >
> Is there a mailing list for lld so that we could include them in this
> mail thread (I couldn't find one)?

Not really sure which mailing list is the right place to discuss this
for llvm, I only know that they have a patch review system -
Phabricator.  But anyway, Maskray (Fangrui) is the lld maintainer, so
he is already here ;)


Thanks
Nelson
diff mbox series

Patch

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
+    TEXT_START_ADDR=0x10000
+else
+    TEXT_START_ADDR=$DEFAULT_TEXT_START_ADDR
+fi
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
 COMMONPAGESIZE="CONSTANT (COMMONPAGESIZE)"