Re: [PATCH v2 4/4] xen/arm: do not give memory back to static heap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.11.2024 09:58, Luca Fancellu wrote:
> From: Penny Zheng <Penny.Zheng@xxxxxxx>
> 
> If Xenheap is statically configured in Device Tree, its size is
> definite. So, the memory shall not be given back into static heap, like
> it's normally done in free_init_memory, etc, once the initialization
> is finished.

I'm somewhat confused by the use of "back" here? Isn't the change all
about init-time behavior, i.e. memory that's handed to the allocator for
the very first time? Else I may be misunderstanding something here, in
which case I'd like to ask for the description to be refined.

> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -132,7 +132,6 @@ struct bootinfo {
>  #ifdef CONFIG_STATIC_SHM
>      struct shared_meminfo shmem;
>  #endif
> -    bool static_heap;
>  };
>  
>  #ifdef CONFIG_ACPI
> @@ -157,6 +156,10 @@ struct bootinfo {
>  
>  extern struct bootinfo bootinfo;
>  
> +#ifdef CONFIG_STATIC_MEMORY
> +extern bool static_heap;
> +#endif

Just to double check Misra-wise: Is there a guarantee that this header will
always be included by page-alloc.c, so that the definition of the symbol
has an earlier declaration already visible? I ask because this header
doesn't look like one where symbols defined in page-alloc.c would normally
be declared. And I sincerely hope that this header isn't one of those that
end up being included virtually everywhere.

> @@ -206,4 +209,13 @@ static inline struct shmem_membank_extra 
> *bootinfo_get_shmem_extra(void)
>  }
>  #endif
>  
> +static inline bool xen_is_using_staticheap(void)
> +{
> +#ifdef CONFIG_STATIC_MEMORY
> +    return static_heap;
> +#else
> +    return false;
> +#endif
> +}

As to naming: How about using_static_heap()? The xen_ prefix doesn't look to
be carrying any useful information, and the then remaining is_ prefix would
be largely redundant with "using". Plus there surely wants to be a separator
between "static" and "heap".

Jan