[PATCH] ARM+RISC-V: BSS handling improvements

Andrew Cooper posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230324222451.3295023-1-andrew.cooper3@citrix.com
Test gitlab-ci failed
xen/arch/arm/arm32/head.S | 2 +-
xen/arch/arm/arm64/head.S | 2 +-
xen/arch/arm/xen.lds.S    | 2 ++
xen/arch/riscv/xen.lds.S  | 4 ++++
4 files changed, 8 insertions(+), 2 deletions(-)
[PATCH] ARM+RISC-V: BSS handling improvements
Posted by Andrew Cooper 1 year ago
 * Correct comments in arm{32,64}/head.S
 * Provide Linker assertions to check the safety of the zeroing loops

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Bob Eshleman <bobbyeshleman@gmail.com>
CC: Alistair Francis <alistair.francis@wdc.com>
CC: Connor Davis <connojdavis@gmail.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Pulled out of the very start of my work to try and unify the handling of
xen_phys_addr across architectures.
---
 xen/arch/arm/arm32/head.S | 2 +-
 xen/arch/arm/arm64/head.S | 2 +-
 xen/arch/arm/xen.lds.S    | 2 ++
 xen/arch/riscv/xen.lds.S  | 4 ++++
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index df51550baa8a..f9f7be9588b1 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -301,7 +301,7 @@ ENDPROC(check_cpu_mode)
 zero_bss:
         PRINT("- Zero BSS -\r\n")
         mov_w r0, __bss_start        /* r0 := vaddr(__bss_start) */
-        mov_w r1, __bss_end          /* r1 := vaddr(__bss_start) */
+        mov_w r1, __bss_end          /* r1 := vaddr(__bss_end)   */
 
         mov   r2, #0
 1:      str   r2, [r0], #4
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 4a3f87117c83..8a4dd64c99ad 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -437,7 +437,7 @@ zero_bss:
 
         PRINT("- Zero BSS -\r\n")
         ldr   x0, =__bss_start       /* x0 := vaddr(__bss_start) */
-        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_start) */
+        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_end)   */
 
 1:      str   xzr, [x0], #8
         cmp   x0, x1
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1b392345bc3b..6ca3caefe607 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -240,3 +240,5 @@ ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is larger t
  */
 ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
 ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
+ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is misaligned")
+ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index ca57cce75cba..2ed70eccc62a 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -1,3 +1,4 @@
+#include <xen/lib.h>
 #include <xen/xen.lds.h>
 
 #undef ENTRY
@@ -156,3 +157,6 @@ SECTIONS
 
     ELF_DETAILS_SECTIONS
 }
+
+ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is misaligned")
+ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
-- 
2.30.2
Re: [PATCH] ARM+RISC-V: BSS handling improvements
Posted by Julien Grall 1 year ago
Hi,

On 24/03/2023 22:24, Andrew Cooper wrote:
>   * Correct comments in arm{32,64}/head.S
>   * Provide Linker assertions to check the safety of the zeroing loops
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

It is now fully acked. So I have committed the patch.

Cheers,

> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Bob Eshleman <bobbyeshleman@gmail.com>
> CC: Alistair Francis <alistair.francis@wdc.com>
> CC: Connor Davis <connojdavis@gmail.com>
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Pulled out of the very start of my work to try and unify the handling of
> xen_phys_addr across architectures.
> ---
>   xen/arch/arm/arm32/head.S | 2 +-
>   xen/arch/arm/arm64/head.S | 2 +-
>   xen/arch/arm/xen.lds.S    | 2 ++
>   xen/arch/riscv/xen.lds.S  | 4 ++++
>   4 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index df51550baa8a..f9f7be9588b1 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -301,7 +301,7 @@ ENDPROC(check_cpu_mode)
>   zero_bss:
>           PRINT("- Zero BSS -\r\n")
>           mov_w r0, __bss_start        /* r0 := vaddr(__bss_start) */
> -        mov_w r1, __bss_end          /* r1 := vaddr(__bss_start) */
> +        mov_w r1, __bss_end          /* r1 := vaddr(__bss_end)   */
>   
>           mov   r2, #0
>   1:      str   r2, [r0], #4
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 4a3f87117c83..8a4dd64c99ad 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -437,7 +437,7 @@ zero_bss:
>   
>           PRINT("- Zero BSS -\r\n")
>           ldr   x0, =__bss_start       /* x0 := vaddr(__bss_start) */
> -        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_start) */
> +        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_end)   */
>   
>   1:      str   xzr, [x0], #8
>           cmp   x0, x1
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1b392345bc3b..6ca3caefe607 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -240,3 +240,5 @@ ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is larger t
>    */
>   ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
>   ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
> +ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is misaligned")
> +ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
> diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
> index ca57cce75cba..2ed70eccc62a 100644
> --- a/xen/arch/riscv/xen.lds.S
> +++ b/xen/arch/riscv/xen.lds.S
> @@ -1,3 +1,4 @@
> +#include <xen/lib.h>
>   #include <xen/xen.lds.h>
>   
>   #undef ENTRY
> @@ -156,3 +157,6 @@ SECTIONS
>   
>       ELF_DETAILS_SECTIONS
>   }
> +
> +ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is misaligned")
> +ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")

-- 
Julien Grall
Re: [PATCH] ARM+RISC-V: BSS handling improvements
Posted by Alistair Francis 1 year ago
On Sat, Mar 25, 2023 at 8:25 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
>  * Correct comments in arm{32,64}/head.S
>  * Provide Linker assertions to check the safety of the zeroing loops
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Bob Eshleman <bobbyeshleman@gmail.com>
> CC: Alistair Francis <alistair.francis@wdc.com>
> CC: Connor Davis <connojdavis@gmail.com>
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> Pulled out of the very start of my work to try and unify the handling of
> xen_phys_addr across architectures.
> ---
>  xen/arch/arm/arm32/head.S | 2 +-
>  xen/arch/arm/arm64/head.S | 2 +-
>  xen/arch/arm/xen.lds.S    | 2 ++
>  xen/arch/riscv/xen.lds.S  | 4 ++++
>  4 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index df51550baa8a..f9f7be9588b1 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -301,7 +301,7 @@ ENDPROC(check_cpu_mode)
>  zero_bss:
>          PRINT("- Zero BSS -\r\n")
>          mov_w r0, __bss_start        /* r0 := vaddr(__bss_start) */
> -        mov_w r1, __bss_end          /* r1 := vaddr(__bss_start) */
> +        mov_w r1, __bss_end          /* r1 := vaddr(__bss_end)   */
>
>          mov   r2, #0
>  1:      str   r2, [r0], #4
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 4a3f87117c83..8a4dd64c99ad 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -437,7 +437,7 @@ zero_bss:
>
>          PRINT("- Zero BSS -\r\n")
>          ldr   x0, =__bss_start       /* x0 := vaddr(__bss_start) */
> -        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_start) */
> +        ldr   x1, =__bss_end         /* x1 := vaddr(__bss_end)   */
>
>  1:      str   xzr, [x0], #8
>          cmp   x0, x1
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1b392345bc3b..6ca3caefe607 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -240,3 +240,5 @@ ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is larger t
>   */
>  ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
>  ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
> +ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is misaligned")
> +ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
> diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
> index ca57cce75cba..2ed70eccc62a 100644
> --- a/xen/arch/riscv/xen.lds.S
> +++ b/xen/arch/riscv/xen.lds.S
> @@ -1,3 +1,4 @@
> +#include <xen/lib.h>
>  #include <xen/xen.lds.h>
>
>  #undef ENTRY
> @@ -156,3 +157,6 @@ SECTIONS
>
>      ELF_DETAILS_SECTIONS
>  }
> +
> +ASSERT(IS_ALIGNED(__bss_start,      POINTER_ALIGN), "__bss_start is misaligned")
> +ASSERT(IS_ALIGNED(__bss_end,        POINTER_ALIGN), "__bss_end is misaligned")
> --
> 2.30.2
>
>
Re: [PATCH] ARM+RISC-V: BSS handling improvements
Posted by Oleksii 1 year ago
Hi Andrew,

Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii
Re: [PATCH] ARM+RISC-V: BSS handling improvements
Posted by Julien Grall 1 year ago
Hi Andrew,

On 24/03/2023 22:24, Andrew Cooper wrote:
>   * Correct comments in arm{32,64}/head.S
>   * Provide Linker assertions to check the safety of the zeroing loops
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall