Re: [PATCH v9 2/8] xen: do not free reserved memory into heap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.07.2022 07:46, Penny Zheng wrote:
> Pages used as guest RAM for static domain, shall be reserved to this
> domain only.
> So in case reserved pages being used for other purpose, users
> shall not free them back to heap, even when last ref gets dropped.
> 
> This commit introduces a new helper free_domstatic_page to free
> static page in runtime, and free_staticmem_pages will be called by it
> in runtime, so let's drop the __init flag.
> 
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>

Technically
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Nevertheless two remarks:

> +void free_domstatic_page(struct page_info *page)
> +{
> +    struct domain *d = page_get_owner(page);
> +    bool drop_dom_ref;
> +
> +    ASSERT(d);

I wonder whether

    if ( unlikely(!d) )
    {
        ASSERT_UNREACHABLE();
        return;
    }

wouldn't be more robust looking forward.

> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -85,13 +85,12 @@ bool scrub_free_pages(void);
>  } while ( false )
>  #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
>  
> -#ifdef CONFIG_STATIC_MEMORY
>  /* These functions are for static memory */
>  void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
>                            bool need_scrub);
> +void free_domstatic_page(struct page_info *page);
>  int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int 
> nr_mfns,
>                              unsigned int memflags);
> -#endif
>  
>  /* Map machine page range in Xen virtual address space. */
>  int map_pages_to_xen(
> @@ -212,6 +211,10 @@ extern struct domain *dom_cow;
>  
>  #include <asm/mm.h>
>  
> +#ifndef PGC_static
> +#define PGC_static 0
> +#endif

This disconnect from all other PGC_* values isn't very nice. I wonder
as how bad it would be seen if Arm kept its #define to 0 private, with
the generic fallback remaining in page_alloc.c.

Jan