Re: [PATCH v3 17/18] xen/arm: linker: The identitymap check should cover the whole .text.header

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 12 Dec 2022, Julien Grall wrote:
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> At the moment, we are only checking that only some part of .text.header
> is part of the identity mapping. However, this doesn't take into account
> the litteral pool which will be located at the end of the section.
      ^ literal


> While we could try to avoid using a literal pool, in the near future we
> will also want to use an identity mapping for switch_ttbr().
> 
> Not everything in .text.header requires to be part of the identity
> mapping. But it is below a page size (i.e. 4KB) so take a shortcut and
> check that .text.header is smaller than a page size.
> 
> With that _end_boot can be removed as it is now unused. Take the
> pportunity to avoid assuming that a page size is always 4KB in the
  ^ opportunity

> error message and comment.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> 
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/arm32/head.S |  2 --
>  xen/arch/arm/arm64/head.S |  2 --
>  xen/arch/arm/xen.lds.S    | 10 +++++++---
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 2658625bc775..e47f90f15b3d 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -730,8 +730,6 @@ fail:   PRINT("- Boot failed -\r\n")
>          b     1b
>  ENDPROC(fail)
>  
> -GLOBAL(_end_boot)
> -
>  /*
>   * Switch TTBR
>   * r1:r0       ttbr
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 23c2c7491db2..663f5813b12e 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -812,8 +812,6 @@ fail:   PRINT("- Boot failed -\r\n")
>          b     1b
>  ENDPROC(fail)
>  
> -GLOBAL(_end_boot)
> -
>  /*
>   * Switch TTBR
>   *
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index ae8c3b4c6c5f..3f7ebd19f3ed 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -32,7 +32,9 @@ SECTIONS
>    _start = .;
>    .text : {
>         _stext = .;             /* Text section */
> +       _idmap_start = .;
>         *(.text.header)
> +       _idmap_end = .;
>  
>         *(.text.cold)
>         *(.text.unlikely .text.*_unlikely .text.unlikely.*)
> @@ -225,10 +227,12 @@ SECTIONS
>  }
>  
>  /*
> - * We require that Xen is loaded at a 4K boundary, so this ensures that any
> - * code running on the boot time identity map cannot cross a section 
> boundary.
> + * We require that Xen is loaded at a page boundary, so this ensures that any
> + * code running on the identity map cannot cross a section boundary.
>   */
> -ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
> +ASSERT(IS_ALIGNED(_idmap_start, PAGE_SIZE), "_idmap_start should be 
> page-aligned")
> +ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is 
> larger than a page size")
> +
>  /*
>   * __init_[begin|end] MUST be at word size boundary otherwise we cannot
>   * write fault instructions in the space properly.
> -- 
> 2.38.1
>