Re: [PATCH 04/16] xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 20 May 2022, Julien Grall wrote:
> From: Julien Grall <julien.grall@xxxxxxx>
> 
> Now that xen_pt_update_entry() is able to deal with different mapping
> size, we can replace the open-coding of the page-tables update by a call
> to modify_xen_mappings().
> 
> As the function is not meant to fail, a BUG_ON() is added to check the
> return.
> 
> Note that we don't use destroy_xen_mappings() because the helper doesn't
> allow us to pass a flags. In theory we could add an extra parameter to
> the function, however there are no other expected users. Hence why
> modify_xen_mappings() is used.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> Reviewed-by: Hongda Deng <Hongda.Heng@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>     Changes in v4:
>         - Add Hongda's reviewed-by
>         - Add a comment to explain what modify_xen_mappings() does.
>         - Clarify in the commit message hwy modify_xen_mappings() is
>           used rather than destroy_xen_mappings().
> 
>     Changes in v2:
>         - Stay consistent with how function name are used in the commit
>         message
>         - Add my AWS signed-off-by
> ---
>  xen/arch/arm/mm.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 747083d820dd..64a79d45b38c 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -614,11 +614,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
>  
>  void __init remove_early_mappings(void)
>  {
> -    lpae_t pte = {0};
> -    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
> -    write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START + SZ_2M),
> -              pte);
> -    flush_xen_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE);
> +    int rc;
> +
> +    /* destroy the _PAGE_BLOCK mapping */
> +    rc = modify_xen_mappings(BOOT_FDT_VIRT_START, BOOT_FDT_VIRT_END,
> +                             _PAGE_BLOCK);
> +    BUG_ON(rc);
>  }
>  
>  /*
> -- 
> 2.32.0
>