Re: [PATCH 7/7] xen/arm: mm: Reduce the area that xen_second covers

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Julien,

On 24.06.2022 11:11, Julien Grall wrote:
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> At the moment, xen_second is used to cover the first 2GB of the
> virtual address space. With the recent rework of the page-tables,
> only the first 1GB region (where Xen resides) is effectively used.
> 
> In addition to that, I would like to reshuffle the memory layout.
> So Xen mappings may not be anymore in the first 2GB of the virtual
> address space.
> 
> Therefore, rework xen_second so it only covers the 1GB region where
> Xen will reside.
> 
> With this change, xen_second doesn't cover anymore the xenheap area
> on arm32. So, we first need to add memory to the boot allocator before
> setting up the xenheap mappings.
> 
> Take the opportunity to update the comments on top of xen_fixmap and
> xen_xenmap.
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> ---
>  xen/arch/arm/mm.c    | 32 +++++++++++---------------------
>  xen/arch/arm/setup.c | 13 +++++++++++--
>  2 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 74666b2e720a..f87a7c32d07d 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -116,17 +116,14 @@ static DEFINE_PAGE_TABLE(cpu0_pgtable);
>  #endif
>  
>  /* Common pagetable leaves */
> -/* Second level page tables.
> - *
> - * The second-level table is 2 contiguous pages long, and covers all
> - * addresses from 0 to 0x7fffffff. Offsets into it are calculated
> - * with second_linear_offset(), not second_table_offset().
> - */
> -static DEFINE_PAGE_TABLES(xen_second, 2);
> -/* First level page table used for fixmap */
> +/* Second level page table used to cover Xen virtual address space */
> +static DEFINE_PAGE_TABLE(xen_second);
> +/* Third level page table used for fixmap */
>  DEFINE_BOOT_PAGE_TABLE(xen_fixmap);
> -/* First level page table used to map Xen itself with the XN bit set
> - * as appropriate. */
> +/*
> + * Third level page table used to map Xen itself with the XN bit set
> + * as appropriate.
> + */
>  static DEFINE_PAGE_TABLE(xen_xenmap);
>  
>  /* Non-boot CPUs use this to find the correct pagetables. */
> @@ -168,7 +165,6 @@ static void __init __maybe_unused build_assertions(void)
>      BUILD_BUG_ON(zeroeth_table_offset(XEN_VIRT_START));
>  #endif
>      BUILD_BUG_ON(first_table_offset(XEN_VIRT_START));
> -    BUILD_BUG_ON(second_linear_offset(XEN_VIRT_START) >= 
> XEN_PT_LPAE_ENTRIES);
Instead of removing this completely, shouldn't you change it to:
BUILD_BUG_ON(second_table_offset(XEN_VIRT_START)); ?

All in all:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Cheers,
Michal