Hi Jan
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Friday, July 8, 2022 9:06 PM
> To: Penny Zheng <penny.zh...@arm.com>
> Cc: Wei Chen <wei.c...@arm.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>;
> Julien Grall <jul...@xen.org>; Stefano Stabellini <sstabell...@kernel.org>;
> Wei Liu <w...@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v8 9/9] xen: retrieve reserved pages on
> populate_physmap
>
> On 07.07.2022 11:22, Penny Zheng wrote:
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args
> > *a)
> >
> > mfn = _mfn(gpfn);
> > }
> > + else if ( is_domain_using_staticmem(d) )
> > + {
> > + /*
> > + * No easy way to guarantee the retrieved pages are
> > contiguous,
> > + * so forbid non-zero-order requests here.
> > + */
> > + if ( a->extent_order != 0 )
> > + {
> > + gdprintk(XENLOG_WARNING,
> > + "Cannot allocate static order-%u pages for
> > static %pd\n",
> > + a->extent_order, d);
>
> I'm probably wrong in thinking that I did point out before that there's no
> real
> reason to have "static" twice in the message. Or am I mistaken in my
> understanding that only static domains can ever have static pages?
>
Sorry for omitting the comment, I'll only keep one static here.
You're right, only static domains can have static pages at the moment.
> > @@ -2818,6 +2805,55 @@ int __init acquire_domstatic_pages(struct
> > domain *d, mfn_t smfn,
> >
> > return 0;
> > }
> > +
> > +/*
> > + * Acquire nr_mfns contiguous pages, starting at #smfn, of static
> > +memory,
> > + * then assign them to one specific domain #d.
> > + */
> > +int __init acquire_domstatic_pages(struct domain *d, mfn_t smfn,
> > + unsigned int nr_mfns, unsigned int
> > +memflags) {
> > + struct page_info *pg;
> > +
> > + ASSERT_ALLOC_CONTEXT();
> > +
> > + pg = acquire_staticmem_pages(smfn, nr_mfns, memflags);
> > + if ( !pg )
> > + return -ENOENT;
> > +
> > + if ( assign_domstatic_pages(d, pg, nr_mfns, memflags) )
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * 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;
>
> Use ASSERT_ALLOC_CONTEXT() here as well?
>
Sure,
> > + /* 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;
>
> Don't you need to undo what this did if ...
>
> > + if ( assign_domstatic_pages(d, page, 1, memflags) )
> > + goto fail;
>
> ... this fails?
Yes, thanks for pointing out.
free_staticmem_pages is needed to do the reversal when it fails
>
> Jan
