Hi Jan
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Monday, July 25, 2022 11:30 PM
> To: Penny Zheng <penny.zh...@arm.com>
> Cc: Wei Chen <wei.c...@arm.com>; Stefano Stabellini
> <sstabell...@kernel.org>; Julien Grall <jul...@xen.org>; Bertrand Marquis
> <bertrand.marq...@arm.com>; Volodymyr Babchuk
> <volodymyr_babc...@epam.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>;
> Wei Liu <w...@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v9 2/8] xen: do not free reserved memory into heap
>
> 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.zh...@arm.com>
>
> Technically
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
>
> 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.
>
It, right now, is only used in xen/arch/arm/mm.c and xen/common/page_alloc.c.
It is ok to let Arm keep its #define to 0 private, with the generic
fallback remaining in page_alloc.c.
> Jan
