RE: [PATCH V3 08/10] xen/arm: introduce acquire_staticmem_pages and acquire_domstatic_pages

[Penny Zheng <Penny.Zheng@xxxxxxx>]

Hi Julien 

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Monday, July 19, 2021 6:00 PM
> To: Jan Beulich <jbeulich@xxxxxxxx>; Penny Zheng <Penny.Zheng@xxxxxxx>
> Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
> <Wei.Chen@xxxxxxx>; nd <nd@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx
> Subject: Re: [PATCH V3 08/10] xen/arm: introduce acquire_staticmem_pages
> and acquire_domstatic_pages
> 
> Hi Jan,
> 
> On 19/07/2021 10:26, Jan Beulich wrote:
> > On 15.07.2021 07:18, Penny Zheng wrote:
> >> @@ -1065,6 +1069,73 @@ static struct page_info *alloc_heap_pages(
> >>       return pg;
> >>   }
> >>
> >> +#ifdef CONFIG_STATIC_MEMORY
> >> +/*
> >> + * Acquire nr_mfns contiguous reserved pages, starting at #smfn, of
> >> + * static memory.
> >> + */
> >> +static struct page_info *acquire_staticmem_pages(unsigned long nr_mfns,
> >> +                                                 mfn_t smfn,
> >> +                                                 unsigned int
> >> +memflags)
> >
> > This and the other function not being __init, and there neither being
> > any caller nor any freeing, a question is whether __init wants adding;
> > patch 10 suggests so. The lack of freeing in particular means that
> > domains created using static memory won't be restartable, requiring a
> > host reboot instead.
> 
> I am open to request an host reboot in case of an issue. Although, it would be
> good to have code in place for that.
> 
> Regardless the reboot part, I would still expect the domain to balloon in/out
> memory. This is pretty broken today because Xen would end up to give the
> memory to the heap allocator and the next allocation would be through the
> heap allocate.
> 
> For runtime "free/allocate", we may want to follow the same behavior as
> direct-mapped domain (i.e. GFN == MFN): the page will not be given back to
> any allocator and we only check if the page belongs to the domain on
> allocation.
> 
> So adding __init for acquire_staticmem_pages() is probably fine.
> 
> On a side node, on v2, I have requested to keep track of the list of missing
> pieces. @Penny, where can I find the list?
> 

Oh, sorry... 

I thought you were requesting a new mail list issue to track all missing pieces 
in ARM...
And a second though here, you shall only mean the missing pieces for this patch 
serie.

I'll do a quick sum-up here and put in the Patch v4 cover letter:

TODO:
- reboot domain on static allocation.
- All memory-ops(hypercalls) regarding domain on static allocation to balloon 
in/out memory
- asynchronously scrubbing PGC_reserved pages
- consider domain on static allocation on NUMA-support scenario

> Cheers,
> 
> --
> Julien Grall

Cheers,

--
Penny Zheng