Re: [PATCH v3 3/4] xen/arm: mm: Rename xenheap_* variable to directmap_*

[Julien Grall <julien@xxxxxxx>]

Hi Henry,

On 07/09/2022 09:36, Henry Wang wrote:
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 7f5b317d3e..4a70ed2986 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -132,12 +132,12 @@ uint64_t init_ttbr;
>   static paddr_t phys_offset;
>   
>   /* Limits of the Xen heap */
> -mfn_t xenheap_mfn_start __read_mostly = INVALID_MFN_INITIALIZER;
> -mfn_t xenheap_mfn_end __read_mostly;
> -vaddr_t xenheap_virt_end __read_mostly;
> +mfn_t directmap_mfn_start __read_mostly = INVALID_MFN_INITIALIZER;
> +mfn_t directmap_mfn_end __read_mostly;
> +vaddr_t directmap_virt_end __read_mostly;
>   #ifdef CONFIG_ARM_64
> -vaddr_t xenheap_virt_start __read_mostly;
> -unsigned long xenheap_base_pdx __read_mostly;
> +vaddr_t directmap_virt_start __read_mostly;
> +unsigned long directmap_base_pdx __read_mostly;
>   #endif
>   
>   unsigned long frametable_base_pdx __read_mostly;
> @@ -609,7 +609,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
I think the function also want to be renamed to match the code below.

>           panic("Unable to setup the xenheap mappings.\n");
Likely, I think this wants to be s/xenheap/directmap/

>   
>       /* Record where the xenheap is, for translation routines. */
Same here because you set directmap_virt_end.

> -    xenheap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
> +    directmap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
I would be OK to keep "XENHEAP_*" for now.

>   }
>   #else /* CONFIG_ARM_64 */
>   void __init setup_xenheap_mappings(unsigned long base_mfn,
> @@ -618,12 +618,12 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
>       int rc;
>   
>       /* First call sets the xenheap physical and virtual offset. */
s/xenheap/directmap/ I haven't looked if there are other instances in 
the function.
Cheers,

--
Julien Grall