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

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

[Jisheng Zhang]

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/20221123161805.1579-1-jszhang@kernel.org/ [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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Jisheng Zhang]

These two symbols are not used, remove them.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Jisheng Zhang]

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

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Andrew Jones]

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 <jszhang@kernel.org>
> ---
>  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
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Jisheng Zhang]

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 <jszhang@kernel.org>
> > ---
> >  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
> > 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Jisheng Zhang]

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 <jszhang@kernel.org>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Andrew Jones]

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 <jszhang@kernel.org>
> ---
>  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 <ajones@ventanamicro.com>

Thanks,
drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 0/3] riscv: vdso.lds.S: some improvement

[Emil Renner Berthing]

On Wed, 26 Jul 2023 at 19:44, Jisheng Zhang <jszhang@kernel.org> wrote:
> 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/20221123161805.1579-1-jszhang@kernel.org/ [1]

Works fine on the VisionFive 2 and Unmatched.
Tested-by: Emil Renner Berthing <emil .renner.berthing@canonical.com>


> 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
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv