2023-07-26 18:07:21

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 0/3] riscv: vdso.lds.S: some improvement

This series renews one of my last year RFC patch[1], tries to improve
the vdso layout a bit.

patch1 removes useless symbols
patch2 merges .data section of vdso into .rodata because they are
readonly
patch3 is the real renew patch, it removes hardcoded 0x800 .text start
addr. But I rewrite the commit msg per Andrew's suggestions and move
move .note, .eh_frame_hdr, and .eh_frame between .rodata and .text to
keep the actual code well away from the non-instruction data.

Link: https://lore.kernel.org/linux-riscv/[email protected]/ [1]

Jisheng Zhang (3):
riscv: vdso.lds.S: drop __alt_start and __alt_end symbols
riscv: vdso.lds.S: merge .data section into .rodata section
riscv: vdso.lds.S: remove hardcoded 0x800 .text start addr

arch/riscv/kernel/vdso/vdso.lds.S | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)

--
2.40.1



2023-07-26 18:30:14

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 3/3] riscv: vdso.lds.S: remove hardcoded 0x800 .text start addr

I believe the hardcoded 0x800 and related comments come from the long
history VDSO_TEXT_OFFSET in x86 vdso code, but commit 5b9304933730
("x86 vDSO: generate vdso-syms.lds") and commit f6b46ebf904f ("x86
vDSO: new layout") removes the comment and hard coding for x86.

Similar as x86 and other arch, riscv doesn't need the rigid layout
using VDSO_TEXT_OFFSET since it "no longer matters to the kernel".
so we could remove the hard coding now, and removing it brings a
small vdso.so and aligns with other architectures.

Also, having enough separation between data and text is important for
I-cache, so similar as x86, move .note, .eh_frame_hdr, and .eh_frame
between .rodata and .text.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/kernel/vdso/vdso.lds.S | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
index 671aa21769bc..cbe2a179331d 100644
--- a/arch/riscv/kernel/vdso/vdso.lds.S
+++ b/arch/riscv/kernel/vdso/vdso.lds.S
@@ -23,12 +23,8 @@ SECTIONS
.gnu.version_d : { *(.gnu.version_d) }
.gnu.version_r : { *(.gnu.version_r) }

- .note : { *(.note.*) } :text :note
.dynamic : { *(.dynamic) } :text :dynamic

- .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
- .eh_frame : { KEEP (*(.eh_frame)) } :text
-
.rodata : {
*(.rodata .rodata.* .gnu.linkonce.r.*)
*(.got.plt) *(.got)
@@ -37,13 +33,16 @@ SECTIONS
*(.bss .bss.* .gnu.linkonce.b.*)
}

+ .note : { *(.note.*) } :text :note
+
+ .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
+ .eh_frame : { KEEP (*(.eh_frame)) } :text
+
/*
- * This linker script is used both with -r and with -shared.
- * For the layouts to match, we need to skip more than enough
- * space for the dynamic symbol table, etc. If this amount is
- * insufficient, ld -shared will error; simply increase it here.
+ * Text is well-separated from actual data: there's plenty of
+ * stuff that isn't used at runtime in between.
*/
- . = 0x800;
+ . = ALIGN(16);
.text : { *(.text .text.*) } :text

. = ALIGN(4);
--
2.40.1


2023-07-26 18:35:31

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 1/3] riscv: vdso.lds.S: drop __alt_start and __alt_end symbols

These two symbols are not used, remove them.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/kernel/vdso/vdso.lds.S | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
index 82ce64900f3d..d43fd7c7dd11 100644
--- a/arch/riscv/kernel/vdso/vdso.lds.S
+++ b/arch/riscv/kernel/vdso/vdso.lds.S
@@ -42,9 +42,7 @@ SECTIONS

. = ALIGN(4);
.alternative : {
- __alt_start = .;
*(.alternative)
- __alt_end = .;
}

.data : {
--
2.40.1


2023-07-26 18:43:02

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 2/3] riscv: vdso.lds.S: merge .data section into .rodata section

The .data section doesn't need to be separate from .rodata section,
they are both readonly.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/kernel/vdso/vdso.lds.S | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
index d43fd7c7dd11..671aa21769bc 100644
--- a/arch/riscv/kernel/vdso/vdso.lds.S
+++ b/arch/riscv/kernel/vdso/vdso.lds.S
@@ -29,7 +29,13 @@ SECTIONS
.eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
.eh_frame : { KEEP (*(.eh_frame)) } :text

- .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) }
+ .rodata : {
+ *(.rodata .rodata.* .gnu.linkonce.r.*)
+ *(.got.plt) *(.got)
+ *(.data .data.* .gnu.linkonce.d.*)
+ *(.dynbss)
+ *(.bss .bss.* .gnu.linkonce.b.*)
+ }

/*
* This linker script is used both with -r and with -shared.
@@ -44,13 +50,6 @@ SECTIONS
.alternative : {
*(.alternative)
}
-
- .data : {
- *(.got.plt) *(.got)
- *(.data .data.* .gnu.linkonce.d.*)
- *(.dynbss)
- *(.bss .bss.* .gnu.linkonce.b.*)
- }
}

/*
--
2.40.1


2023-07-28 13:34:42

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH 2/3] riscv: vdso.lds.S: merge .data section into .rodata section

On Thu, Jul 27, 2023 at 01:30:23AM +0800, Jisheng Zhang wrote:
> The .data section doesn't need to be separate from .rodata section,
> they are both readonly.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> arch/riscv/kernel/vdso/vdso.lds.S | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> index d43fd7c7dd11..671aa21769bc 100644
> --- a/arch/riscv/kernel/vdso/vdso.lds.S
> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> @@ -29,7 +29,13 @@ SECTIONS
> .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
> .eh_frame : { KEEP (*(.eh_frame)) } :text
>
> - .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) }
> + .rodata : {
> + *(.rodata .rodata.* .gnu.linkonce.r.*)
> + *(.got.plt) *(.got)
> + *(.data .data.* .gnu.linkonce.d.*)
> + *(.dynbss)
> + *(.bss .bss.* .gnu.linkonce.b.*)

Looking at other architectures, it appears the last three lines of
sections could be discarded, but I don't know enough about this to
state they should be.

Thanks,
drew


> + }
>
> /*
> * This linker script is used both with -r and with -shared.
> @@ -44,13 +50,6 @@ SECTIONS
> .alternative : {
> *(.alternative)
> }
> -
> - .data : {
> - *(.got.plt) *(.got)
> - *(.data .data.* .gnu.linkonce.d.*)
> - *(.dynbss)
> - *(.bss .bss.* .gnu.linkonce.b.*)
> - }
> }
>
> /*
> --
> 2.40.1
>

2023-07-28 13:39:29

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH 3/3] riscv: vdso.lds.S: remove hardcoded 0x800 .text start addr

On Thu, Jul 27, 2023 at 01:30:24AM +0800, Jisheng Zhang wrote:
> I believe the hardcoded 0x800 and related comments come from the long
> history VDSO_TEXT_OFFSET in x86 vdso code, but commit 5b9304933730
> ("x86 vDSO: generate vdso-syms.lds") and commit f6b46ebf904f ("x86
> vDSO: new layout") removes the comment and hard coding for x86.
>
> Similar as x86 and other arch, riscv doesn't need the rigid layout
> using VDSO_TEXT_OFFSET since it "no longer matters to the kernel".
> so we could remove the hard coding now, and removing it brings a
> small vdso.so and aligns with other architectures.
>
> Also, having enough separation between data and text is important for
> I-cache, so similar as x86, move .note, .eh_frame_hdr, and .eh_frame
> between .rodata and .text.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> arch/riscv/kernel/vdso/vdso.lds.S | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> index 671aa21769bc..cbe2a179331d 100644
> --- a/arch/riscv/kernel/vdso/vdso.lds.S
> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> @@ -23,12 +23,8 @@ SECTIONS
> .gnu.version_d : { *(.gnu.version_d) }
> .gnu.version_r : { *(.gnu.version_r) }
>
> - .note : { *(.note.*) } :text :note
> .dynamic : { *(.dynamic) } :text :dynamic
>
> - .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
> - .eh_frame : { KEEP (*(.eh_frame)) } :text
> -
> .rodata : {
> *(.rodata .rodata.* .gnu.linkonce.r.*)
> *(.got.plt) *(.got)
> @@ -37,13 +33,16 @@ SECTIONS
> *(.bss .bss.* .gnu.linkonce.b.*)
> }
>
> + .note : { *(.note.*) } :text :note
> +
> + .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
> + .eh_frame : { KEEP (*(.eh_frame)) } :text
> +
> /*
> - * This linker script is used both with -r and with -shared.
> - * For the layouts to match, we need to skip more than enough
> - * space for the dynamic symbol table, etc. If this amount is
> - * insufficient, ld -shared will error; simply increase it here.
> + * Text is well-separated from actual data: there's plenty of
> + * stuff that isn't used at runtime in between.
> */
> - . = 0x800;
> + . = ALIGN(16);
> .text : { *(.text .text.*) } :text
>
> . = ALIGN(4);
> --
> 2.40.1
>

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2023-07-28 14:55:46

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/3] riscv: vdso.lds.S: merge .data section into .rodata section

On Fri, Jul 28, 2023 at 02:57:03PM +0200, Andrew Jones wrote:
> On Thu, Jul 27, 2023 at 01:30:23AM +0800, Jisheng Zhang wrote:
> > The .data section doesn't need to be separate from .rodata section,
> > they are both readonly.
> >
> > Signed-off-by: Jisheng Zhang <[email protected]>
> > ---
> > arch/riscv/kernel/vdso/vdso.lds.S | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> > index d43fd7c7dd11..671aa21769bc 100644
> > --- a/arch/riscv/kernel/vdso/vdso.lds.S
> > +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> > @@ -29,7 +29,13 @@ SECTIONS
> > .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
> > .eh_frame : { KEEP (*(.eh_frame)) } :text
> >
> > - .rodata : { *(.rodata .rodata.* .gnu.linkonce.r.*) }
> > + .rodata : {
> > + *(.rodata .rodata.* .gnu.linkonce.r.*)
> > + *(.got.plt) *(.got)
> > + *(.data .data.* .gnu.linkonce.d.*)
> > + *(.dynbss)
> > + *(.bss .bss.* .gnu.linkonce.b.*)
>
> Looking at other architectures, it appears the last three lines of
> sections could be discarded, but I don't know enough about this to

Hi Andrew,

I checked x86, they still keep those sections. From another side,
even if those sections are not needed, removing unused sections could
be an independent patch, for safe reason or bisect reason.

Thanks


> state they should be.
>
> Thanks,
> drew
>
>
> > + }
> >
> > /*
> > * This linker script is used both with -r and with -shared.
> > @@ -44,13 +50,6 @@ SECTIONS
> > .alternative : {
> > *(.alternative)
> > }
> > -
> > - .data : {
> > - *(.got.plt) *(.got)
> > - *(.data .data.* .gnu.linkonce.d.*)
> > - *(.dynbss)
> > - *(.bss .bss.* .gnu.linkonce.b.*)
> > - }
> > }
> >
> > /*
> > --
> > 2.40.1
> >