Hi Jan > -----Original Message----- > From: Jan Beulich <[email protected]> > Sent: Monday, July 25, 2022 11:30 PM > To: Penny Zheng <[email protected]> > Cc: Wei Chen <[email protected]>; Stefano Stabellini > <[email protected]>; Julien Grall <[email protected]>; Bertrand Marquis > <[email protected]>; Volodymyr Babchuk > <[email protected]>; Andrew Cooper > <[email protected]>; George Dunlap <[email protected]>; > Wei Liu <[email protected]>; [email protected] > 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 <[email protected]> > > Technically > Reviewed-by: Jan Beulich <[email protected]> > > 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
