Re: [PATCH v1 2/5] xen/arm: introduce CDF_staticmem

[Julien Grall <julien@xxxxxxx>]

Hi Penny,

On 30/03/2022 10:36, Penny Zheng wrote:
> In order to have an easy and quick way to find out whether this domain is
> on static allocation, this commit introduces a new flag CDF_staticmem and a
> new helper is_domain_on_static_allocation.
>
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
>   xen/arch/arm/domain_build.c       | 5 ++++-
>   xen/arch/arm/include/asm/domain.h | 2 ++
>   xen/include/xen/domain.h          | 2 ++
>   3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 8be01678de..4e62fd0bf1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3191,9 +3191,12 @@ void __init create_domUs(void)
>           if ( !dt_device_is_compatible(node, "xen,domain") )
>               continue;
>   
> +        if ( dt_find_property(node, "xen,static-mem", NULL) )
> +            flags |= CDF_staticmem;
> +
>           if ( dt_property_read_bool(node, "direct-map") )
>           {
> -            if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, 
> "xen,static-mem", NULL) )
> +            if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !(flags & CDF_staticmem) 
> )
>                   panic("direct-map is not valid for domain %s without static 
> allocation.\n",
>                         dt_node_name(node));
>   
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index 95fef29111..4379f20a12 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -31,6 +31,8 @@ enum domain_type {
>   
>   #define is_domain_direct_mapped(d) (((d)->arch.flags) & CDF_directmap)
>   
> +#define is_domain_on_static_allocation(d) (((d)->arch.flags) & CDF_staticmem)
"on" sounds strange to me and the name is bit long. How about 
"is_domain_using_staticmem()" or maybe "is_domain_static()"?
I will have a look at the rest of the series before giving my ack on 
this patch.
> +
>   /*
>    * Is the domain using the host memory layout?
>    *
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 1c3c88a14d..35dc7143a4 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -34,6 +34,8 @@ void arch_get_domain_info(const struct domain *d,
>   #ifdef CONFIG_ARM
>   /* Should domain memory be directly mapped? */
>   #define CDF_directmap            (1U << 1)
> +/* Is domain memory on static allocation? */
> +#define CDF_staticmem            (1U << 2)
>   #endif
>   
>   /*
Cheers,

--
Julien Grall