diff mbox series

[kvm-unit-tests,v2] arm/arm64: Zero BSS and stack at startup

Message ID 20210322162721.108514-1-drjones@redhat.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests,v2] arm/arm64: Zero BSS and stack at startup | expand

Commit Message

Andrew Jones March 22, 2021, 4:27 p.m. UTC 
So far we've counted on QEMU or kvmtool implicitly zeroing all memory.
With our goal of eventually supporting bare-metal targets with
target-efi we should explicitly zero any memory we expect to be zeroed
ourselves. This obviously includes the BSS, but also the bootcpu's
stack, as the bootcpu's thread-info lives in the stack and may get
used in early setup to get the cpu index. Note, this means we still
assume the bootcpu's cpu index to be zero. That assumption can be
removed later.

Cc: Nikos Nikoleris <nikos.nikoleris@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/cstart.S   | 20 ++++++++++++++++++++
 arm/cstart64.S | 22 +++++++++++++++++++++-
 arm/flat.lds   |  4 ++++
 3 files changed, 45 insertions(+), 1 deletion(-)

Comments

Andrew Jones March 22, 2021, 4:34 p.m. UTC | #1 
On Mon, Mar 22, 2021 at 05:27:21PM +0100, Andrew Jones wrote:
> So far we've counted on QEMU or kvmtool implicitly zeroing all memory.
> With our goal of eventually supporting bare-metal targets with
> target-efi we should explicitly zero any memory we expect to be zeroed
> ourselves. This obviously includes the BSS, but also the bootcpu's
> stack, as the bootcpu's thread-info lives in the stack and may get
> used in early setup to get the cpu index. Note, this means we still
> assume the bootcpu's cpu index to be zero. That assumption can be
> removed later.
> 
> Cc: Nikos Nikoleris <nikos.nikoleris@arm.com>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---

Sorry, I forgot the v2 changelog.

- Optimize zero_range [Alexandru]
- Calculate the bottom of the thread-info rather than
  use a linker script symbol [Alexandru]

>  arm/cstart.S   | 20 ++++++++++++++++++++
>  arm/cstart64.S | 22 +++++++++++++++++++++-
>  arm/flat.lds   |  4 ++++
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/cstart.S b/arm/cstart.S
> index ef936ae2f874..9eb11ba4dcaf 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -15,12 +15,32 @@
>  
>  #define THREAD_START_SP ((THREAD_SIZE - S_FRAME_SIZE * 8) & ~7)
>  
> +.macro zero_range, tmp1, tmp2, tmp3, tmp4
> +	mov	\tmp3, #0
> +	mov	\tmp4, #0
> +9998:	cmp	\tmp1, \tmp2
> +	beq	9997f
> +	strd	\tmp3, \tmp4, [\tmp1], #8
> +	b	9998b
> +9997:
> +.endm
> +
>  .arm
>  
>  .section .init
>  
>  .globl start
>  start:
> +	/* zero BSS */
> +	ldr	r4, =bss
> +	ldr	r5, =ebss
> +	zero_range r4, r5, r6, r7
> +
> +	/* zero stack */
> +	ldr	r5, =stacktop
> +	sub	r4, r5, #THREAD_SIZE
> +	zero_range r4, r5, r6, r7
> +
>  	/*
>  	 * set stack, making room at top of stack for cpu0's
>  	 * exception stacks. Must start wtih stackptr, not
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index 0428014aa58a..2a691f8f5065 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -12,6 +12,15 @@
>  #include <asm/processor.h>
>  #include <asm/page.h>
>  #include <asm/pgtable-hwdef.h>
> +#include <asm/thread_info.h>
> +
> +.macro zero_range, tmp1, tmp2
> +9998:	cmp	\tmp1, \tmp2
> +	b.eq	9997f
> +	stp	xzr, xzr, [\tmp1], #16
> +	b	9998b
> +9997:
> +.endm
>  
>  .section .init
>  
> @@ -51,7 +60,18 @@ start:
>  	b	1b
>  
>  1:
> -	/* set up stack */
> +	/* zero BSS */
> +	adrp	x4, bss
> +	add	x4, x4, :lo12:bss
> +	adrp    x5, ebss
> +	add     x5, x5, :lo12:ebss
> +	zero_range x4, x5
> +
> +	/* zero and set up stack */
> +	adrp    x5, stacktop
> +	add     x5, x5, :lo12:stacktop
> +	sub	x4, x5, #THREAD_SIZE
> +	zero_range x4, x5
>  	mov	x4, #1
>  	msr	spsel, x4
>  	isb
> diff --git a/arm/flat.lds b/arm/flat.lds
> index 25f8d03cba87..4d43cdfeab41 100644
> --- a/arm/flat.lds
> +++ b/arm/flat.lds
> @@ -17,7 +17,11 @@ SECTIONS
>  
>      .rodata   : { *(.rodata*) }
>      .data     : { *(.data) }
> +    . = ALIGN(16);
> +    PROVIDE(bss = .);
>      .bss      : { *(.bss) }
> +    . = ALIGN(16);
> +    PROVIDE(ebss = .);
>      . = ALIGN(64K);
>      PROVIDE(edata = .);
>  
> -- 
> 2.26.3
>
Alexandru Elisei March 23, 2021, 11:09 a.m. UTC | #2 
Hi Drew,

On 3/22/21 4:27 PM, Andrew Jones wrote:
> So far we've counted on QEMU or kvmtool implicitly zeroing all memory.
> With our goal of eventually supporting bare-metal targets with
> target-efi we should explicitly zero any memory we expect to be zeroed
> ourselves. This obviously includes the BSS, but also the bootcpu's
> stack, as the bootcpu's thread-info lives in the stack and may get
> used in early setup to get the cpu index. Note, this means we still
> assume the bootcpu's cpu index to be zero. That assumption can be
> removed later.
>
> Cc: Nikos Nikoleris <nikos.nikoleris@arm.com>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/cstart.S   | 20 ++++++++++++++++++++
>  arm/cstart64.S | 22 +++++++++++++++++++++-
>  arm/flat.lds   |  4 ++++
>  3 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/arm/cstart.S b/arm/cstart.S
> index ef936ae2f874..9eb11ba4dcaf 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -15,12 +15,32 @@
>  
>  #define THREAD_START_SP ((THREAD_SIZE - S_FRAME_SIZE * 8) & ~7)
>  
> +.macro zero_range, tmp1, tmp2, tmp3, tmp4

tmp1 and tmp2 could be renamed to start and end (same for arm64), but the macro is
nice and simple, and it's also pretty clear what the arguments represent from the
call sites. Looks good either way:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

> +	mov	\tmp3, #0
> +	mov	\tmp4, #0
> +9998:	cmp	\tmp1, \tmp2
> +	beq	9997f
> +	strd	\tmp3, \tmp4, [\tmp1], #8
> +	b	9998b
> +9997:
> +.endm
> +
>  .arm
>  
>  .section .init
>  
>  .globl start
>  start:
> +	/* zero BSS */
> +	ldr	r4, =bss
> +	ldr	r5, =ebss
> +	zero_range r4, r5, r6, r7
> +
> +	/* zero stack */
> +	ldr	r5, =stacktop
> +	sub	r4, r5, #THREAD_SIZE
> +	zero_range r4, r5, r6, r7
> +
>  	/*
>  	 * set stack, making room at top of stack for cpu0's
>  	 * exception stacks. Must start wtih stackptr, not
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index 0428014aa58a..2a691f8f5065 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -12,6 +12,15 @@
>  #include <asm/processor.h>
>  #include <asm/page.h>
>  #include <asm/pgtable-hwdef.h>
> +#include <asm/thread_info.h>
> +
> +.macro zero_range, tmp1, tmp2
> +9998:	cmp	\tmp1, \tmp2
> +	b.eq	9997f
> +	stp	xzr, xzr, [\tmp1], #16
> +	b	9998b
> +9997:
> +.endm
>  
>  .section .init
>  
> @@ -51,7 +60,18 @@ start:
>  	b	1b
>  
>  1:
> -	/* set up stack */
> +	/* zero BSS */
> +	adrp	x4, bss
> +	add	x4, x4, :lo12:bss
> +	adrp    x5, ebss
> +	add     x5, x5, :lo12:ebss
> +	zero_range x4, x5
> +
> +	/* zero and set up stack */
> +	adrp    x5, stacktop
> +	add     x5, x5, :lo12:stacktop
> +	sub	x4, x5, #THREAD_SIZE
> +	zero_range x4, x5
>  	mov	x4, #1
>  	msr	spsel, x4
>  	isb
> diff --git a/arm/flat.lds b/arm/flat.lds
> index 25f8d03cba87..4d43cdfeab41 100644
> --- a/arm/flat.lds
> +++ b/arm/flat.lds
> @@ -17,7 +17,11 @@ SECTIONS
>  
>      .rodata   : { *(.rodata*) }
>      .data     : { *(.data) }
> +    . = ALIGN(16);
> +    PROVIDE(bss = .);
>      .bss      : { *(.bss) }
> +    . = ALIGN(16);
> +    PROVIDE(ebss = .);
>      . = ALIGN(64K);
>      PROVIDE(edata = .);
>
Andrew Jones March 23, 2021, 11:29 a.m. UTC | #3 
On Tue, Mar 23, 2021 at 11:09:27AM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On 3/22/21 4:27 PM, Andrew Jones wrote:
> > So far we've counted on QEMU or kvmtool implicitly zeroing all memory.
> > With our goal of eventually supporting bare-metal targets with
> > target-efi we should explicitly zero any memory we expect to be zeroed
> > ourselves. This obviously includes the BSS, but also the bootcpu's
> > stack, as the bootcpu's thread-info lives in the stack and may get
> > used in early setup to get the cpu index. Note, this means we still
> > assume the bootcpu's cpu index to be zero. That assumption can be
> > removed later.
> >
> > Cc: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> > Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  arm/cstart.S   | 20 ++++++++++++++++++++
> >  arm/cstart64.S | 22 +++++++++++++++++++++-
> >  arm/flat.lds   |  4 ++++
> >  3 files changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/arm/cstart.S b/arm/cstart.S
> > index ef936ae2f874..9eb11ba4dcaf 100644
> > --- a/arm/cstart.S
> > +++ b/arm/cstart.S
> > @@ -15,12 +15,32 @@
> >  
> >  #define THREAD_START_SP ((THREAD_SIZE - S_FRAME_SIZE * 8) & ~7)
> >  
> > +.macro zero_range, tmp1, tmp2, tmp3, tmp4
> 
> tmp1 and tmp2 could be renamed to start and end (same for arm64), but the macro is
> nice and simple, and it's also pretty clear what the arguments represent from the
> call sites. Looks good either way:

I considered tmp1 and tmp2 being start and end, but start gets used like
a temporary anyway.

> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks!

Applied to arm/queue

https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue

> 
> Thanks,
> 
> Alex
> 
> > +	mov	\tmp3, #0
> > +	mov	\tmp4, #0
> > +9998:	cmp	\tmp1, \tmp2
> > +	beq	9997f
> > +	strd	\tmp3, \tmp4, [\tmp1], #8
> > +	b	9998b
> > +9997:
> > +.endm
> > +
> >  .arm
> >  
> >  .section .init
> >  
> >  .globl start
> >  start:
> > +	/* zero BSS */
> > +	ldr	r4, =bss
> > +	ldr	r5, =ebss
> > +	zero_range r4, r5, r6, r7
> > +
> > +	/* zero stack */
> > +	ldr	r5, =stacktop
> > +	sub	r4, r5, #THREAD_SIZE
> > +	zero_range r4, r5, r6, r7
> > +
> >  	/*
> >  	 * set stack, making room at top of stack for cpu0's
> >  	 * exception stacks. Must start wtih stackptr, not
> > diff --git a/arm/cstart64.S b/arm/cstart64.S
> > index 0428014aa58a..2a691f8f5065 100644
> > --- a/arm/cstart64.S
> > +++ b/arm/cstart64.S
> > @@ -12,6 +12,15 @@
> >  #include <asm/processor.h>
> >  #include <asm/page.h>
> >  #include <asm/pgtable-hwdef.h>
> > +#include <asm/thread_info.h>
> > +
> > +.macro zero_range, tmp1, tmp2
> > +9998:	cmp	\tmp1, \tmp2
> > +	b.eq	9997f
> > +	stp	xzr, xzr, [\tmp1], #16
> > +	b	9998b
> > +9997:
> > +.endm
> >  
> >  .section .init
> >  
> > @@ -51,7 +60,18 @@ start:
> >  	b	1b
> >  
> >  1:
> > -	/* set up stack */
> > +	/* zero BSS */
> > +	adrp	x4, bss
> > +	add	x4, x4, :lo12:bss
> > +	adrp    x5, ebss
> > +	add     x5, x5, :lo12:ebss
> > +	zero_range x4, x5
> > +
> > +	/* zero and set up stack */
> > +	adrp    x5, stacktop
> > +	add     x5, x5, :lo12:stacktop
> > +	sub	x4, x5, #THREAD_SIZE
> > +	zero_range x4, x5
> >  	mov	x4, #1
> >  	msr	spsel, x4
> >  	isb
> > diff --git a/arm/flat.lds b/arm/flat.lds
> > index 25f8d03cba87..4d43cdfeab41 100644
> > --- a/arm/flat.lds
> > +++ b/arm/flat.lds
> > @@ -17,7 +17,11 @@ SECTIONS
> >  
> >      .rodata   : { *(.rodata*) }
> >      .data     : { *(.data) }
> > +    . = ALIGN(16);
> > +    PROVIDE(bss = .);
> >      .bss      : { *(.bss) }
> > +    . = ALIGN(16);
> > +    PROVIDE(ebss = .);
> >      . = ALIGN(64K);
> >      PROVIDE(edata = .);
> >  
>
diff mbox series

Patch

diff --git a/arm/cstart.S b/arm/cstart.S
index ef936ae2f874..9eb11ba4dcaf 100644
--- a/arm/cstart.S
+++ b/arm/cstart.S
@@ -15,12 +15,32 @@ 
 
 #define THREAD_START_SP ((THREAD_SIZE - S_FRAME_SIZE * 8) & ~7)
 
+.macro zero_range, tmp1, tmp2, tmp3, tmp4
+	mov	\tmp3, #0
+	mov	\tmp4, #0
+9998:	cmp	\tmp1, \tmp2
+	beq	9997f
+	strd	\tmp3, \tmp4, [\tmp1], #8
+	b	9998b
+9997:
+.endm
+
 .arm
 
 .section .init
 
 .globl start
 start:
+	/* zero BSS */
+	ldr	r4, =bss
+	ldr	r5, =ebss
+	zero_range r4, r5, r6, r7
+
+	/* zero stack */
+	ldr	r5, =stacktop
+	sub	r4, r5, #THREAD_SIZE
+	zero_range r4, r5, r6, r7
+
 	/*
 	 * set stack, making room at top of stack for cpu0's
 	 * exception stacks. Must start wtih stackptr, not
diff --git a/arm/cstart64.S b/arm/cstart64.S
index 0428014aa58a..2a691f8f5065 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -12,6 +12,15 @@ 
 #include <asm/processor.h>
 #include <asm/page.h>
 #include <asm/pgtable-hwdef.h>
+#include <asm/thread_info.h>
+
+.macro zero_range, tmp1, tmp2
+9998:	cmp	\tmp1, \tmp2
+	b.eq	9997f
+	stp	xzr, xzr, [\tmp1], #16
+	b	9998b
+9997:
+.endm
 
 .section .init
 
@@ -51,7 +60,18 @@  start:
 	b	1b
 
 1:
-	/* set up stack */
+	/* zero BSS */
+	adrp	x4, bss
+	add	x4, x4, :lo12:bss
+	adrp    x5, ebss
+	add     x5, x5, :lo12:ebss
+	zero_range x4, x5
+
+	/* zero and set up stack */
+	adrp    x5, stacktop
+	add     x5, x5, :lo12:stacktop
+	sub	x4, x5, #THREAD_SIZE
+	zero_range x4, x5
 	mov	x4, #1
 	msr	spsel, x4
 	isb
diff --git a/arm/flat.lds b/arm/flat.lds
index 25f8d03cba87..4d43cdfeab41 100644
--- a/arm/flat.lds
+++ b/arm/flat.lds
@@ -17,7 +17,11 @@  SECTIONS
 
     .rodata   : { *(.rodata*) }
     .data     : { *(.data) }
+    . = ALIGN(16);
+    PROVIDE(bss = .);
     .bss      : { *(.bss) }
+    . = ALIGN(16);
+    PROVIDE(ebss = .);
     . = ALIGN(64K);
     PROVIDE(edata = .);