> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Tuesday, May 31, 2022 4:37 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 v5 2/9] xen: do not free reserved memory into heap > > On 31.05.2022 05:12, 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. > > > > free_staticmem_pages will be called by free_heap_pages in runtime for > > static domain freeing memory resource, so let's drop the __init flag. > > > > Signed-off-by: Penny Zheng <penny.zh...@arm.com> > > --- > > v5 changes: > > - In order to avoid stub functions, we #define PGC_staticmem to > > non-zero only when CONFIG_STATIC_MEMORY > > - use "unlikely()" around pg->count_info & PGC_staticmem > > - remove pointless "if", since mark_page_free() is going to set > > count_info to PGC_state_free and by consequence clear PGC_staticmem > > - move #define PGC_staticmem 0 to mm.h > > --- > > v4 changes: > > - no changes > > --- > > v3 changes: > > - fix possible racy issue in free_staticmem_pages() > > - introduce a stub free_staticmem_pages() for the > > !CONFIG_STATIC_MEMORY case > > - move the change to free_heap_pages() to cover other potential call > > sites > > - fix the indentation > > --- > > v2 changes: > > - new commit > > --- > > xen/arch/arm/include/asm/mm.h | 2 ++ > > xen/common/page_alloc.c | 16 +++++++++------- > > xen/include/xen/mm.h | 6 +++++- > > 3 files changed, 16 insertions(+), 8 deletions(-) > > > > diff --git a/xen/arch/arm/include/asm/mm.h > > b/xen/arch/arm/include/asm/mm.h index 1226700085..56d0939318 100644 > > --- a/xen/arch/arm/include/asm/mm.h > > +++ b/xen/arch/arm/include/asm/mm.h > > @@ -108,9 +108,11 @@ struct page_info > > /* Page is Xen heap? */ > > #define _PGC_xen_heap PG_shift(2) > > #define PGC_xen_heap PG_mask(1, 2) > > +#ifdef CONFIG_STATIC_MEMORY > > /* Page is static memory */ > > #define _PGC_staticmem PG_shift(3) > > #define PGC_staticmem PG_mask(1, 3) > > +#endif > > /* ... */ > > /* Page is broken? */ > > #define _PGC_broken PG_shift(7) > > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index > > 44600dd9cd..6425761116 100644 > > --- a/xen/common/page_alloc.c > > +++ b/xen/common/page_alloc.c > > @@ -151,10 +151,6 @@ > > #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL) > > #endif > > > > -#ifndef PGC_staticmem > > -#define PGC_staticmem 0 > > -#endif > > - > > Is the moving of this into the header really a necessary part of this change? > Afaics the symbol is still only ever used in this one C file.
Later, in commit "xen/arm: unpopulate memory when domain is static", we will use this flag in xen/arch/arm/include/asm/mm.h > > --- a/xen/include/xen/mm.h > > +++ b/xen/include/xen/mm.h > > @@ -85,10 +85,10 @@ 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); > > +#ifdef CONFIG_STATIC_MEMORY > > int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int > nr_mfns, > > unsigned int memflags); #endif > > Is the #ifdef really worth retaining at this point? Code is generally better > readable without. > Sure, will remove > Jan
