Hi Jan > -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Tuesday, May 18, 2021 6:43 PM > To: Penny Zheng <penny.zh...@arm.com> > Cc: Bertrand Marquis <bertrand.marq...@arm.com>; Wei Chen > <wei.c...@arm.com>; nd <n...@arm.com>; xen-devel@lists.xenproject.org; > sstabell...@kernel.org; jul...@xen.org > Subject: Re: [PATCH 04/10] xen/arm: static memory initialization > > On 18.05.2021 11:51, Penny Zheng wrote: > >> From: Jan Beulich <jbeul...@suse.com> > >> Sent: Tuesday, May 18, 2021 3:16 PM > >> > >> On 18.05.2021 07:21, Penny Zheng wrote: > >>> This patch introduces static memory initialization, during system > >>> RAM boot > >> up. > >>> > >>> New func init_staticmem_pages is the equivalent of init_heap_pages, > >>> responsible for static memory initialization. > >>> > >>> Helper func free_staticmem_pages is the equivalent of > >>> free_heap_pages, to free nr_pfns pages of static memory. > >>> For each page, it includes the following steps: > >>> 1. change page state from in-use(also initialization state) to free > >>> state and grant PGC_reserved. > >>> 2. set its owner NULL and make sure this page is not a guest frame > >>> any more > >> > >> But isn't the goal (as per the previous patch) to associate such > >> pages with a _specific_ domain? > >> > > > > Free_staticmem_pages are alike free_heap_pages, are not used only for > initialization. > > Freeing used pages to unused are also included. > > Here, setting its owner NULL is to set owner in used field NULL. > > I'm afraid I still don't understand. >
When initializing heap, xen is using free_heap_pages to do the initialization. And also when normal domain get destroyed/rebooted, xen is using free_domheap_pages, which calls free_heap_pages to free the pages. So here, since free_staticmem_pages is the equivalent of the free_heap_pages for static memory, I'm also considering both two scenarios here. And if it is domain get destroyed/rebooted, Page state is in inuse state(PGC_inuse), and the page_info.v.inuse.domain is holding the domain owner. When freeing then, we need to switch the page state to free state(PGC_free) and set its inuse owner to NULL. I'll clarify it more clearly in commit message. > > Still, I need to add more explanation here. > > Yes please. > > >>> @@ -1512,6 +1515,49 @@ static void free_heap_pages( > >>> spin_unlock(&heap_lock); > >>> } > >>> > >>> +/* Equivalent of free_heap_pages to free nr_pfns pages of static > >>> +memory. */ static void free_staticmem_pages(struct page_info *pg, > >> unsigned long nr_pfns, > >>> + bool need_scrub) > >> > >> Right now this function gets called only from an __init one. Unless > >> it is intended to gain further callers, it should be marked __init itself > >> then. > >> Otherwise it should be made sure that other architectures don't > >> include this (dead there) code. > >> > > > > Sure, I'll add __init. Thx. > > Didn't I see a 2nd call to the function later in the series? That one doesn't > permit the function to be __init, iirc. > > Jan Cheers Penny
