Hi Jan Sorry for the delayed response.
> -----Original Message----- > From: Xen-devel <[email protected]> On Behalf Of Jan > Beulich > Sent: Monday, July 25, 2022 11:44 PM > To: Penny Zheng <[email protected]> > Cc: Wei Chen <[email protected]>; Andrew Cooper > <[email protected]>; George Dunlap <[email protected]>; > Julien Grall <[email protected]>; Stefano Stabellini <[email protected]>; > Wei Liu <[email protected]>; [email protected] > Subject: Re: [PATCH v9 8/8] xen: retrieve reserved pages on > populate_physmap > > On 20.07.2022 07:46, Penny Zheng wrote: > > When a static domain populates memory through populate_physmap at > > runtime, it shall retrieve reserved pages from resv_page_list to make > > sure that guest RAM is still restricted in statically configured memory > regions. > > This commit also introduces a new helper acquire_reserved_page to make > it work. > > > > Signed-off-by: Penny Zheng <[email protected]> > > --- > > v9 changes: > > - Use ASSERT_ALLOC_CONTEXT() in acquire_reserved_page > > - Add free_staticmem_pages to undo prepare_staticmem_pages when > > assign_domstatic_pages fails > > May I suggest to re-consider naming of the various functions? Undoing what > "prepare" did by "free" is, well, counterintuitive. > How about change the name "prepare_staticmem_pages" to "allocate_staticmem_pages"? > > +/* > > + * Acquire a page from reserved page list(resv_page_list), when > > +populating > > + * memory for static domain on runtime. > > + */ > > +mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags) > > +{ > > + struct page_info *page; > > + > > + ASSERT_ALLOC_CONTEXT(); > > + > > + /* Acquire a page from reserved page list(resv_page_list). */ > > + spin_lock(&d->page_alloc_lock); > > + page = page_list_remove_head(&d->resv_page_list); > > + spin_unlock(&d->page_alloc_lock); > > + if ( unlikely(!page) ) > > + return INVALID_MFN; > > + > > + if ( !prepare_staticmem_pages(page, 1, memflags) ) > > + goto fail; > > + > > + if ( assign_domstatic_pages(d, page, 1, memflags) ) > > + goto fail_assign; > > + > > + return page_to_mfn(page); > > + > > + fail_assign: > > + free_staticmem_pages(page, 1, memflags & MEMF_no_scrub); > > + fail: > > + page_list_add_tail(page, &d->resv_page_list); > > + return INVALID_MFN; > > +} > > What about locking on this error path? > Right, locking is needed here too: fail: spin_lock(&d->page_alloc_lock); page_list_add_tail(page, &d->resv_page_list); spin_unlock(&d->page_alloc_lock); > Jan
