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

From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>, Penny Zheng <penny.zheng@arm.com>
Cc: Bertrand.Marquis@arm.com, Wei.Chen@arm.com, nd@arm.com,
	xen-devel@lists.xenproject.org, sstabellini@kernel.org
Subject: Re: [PATCH V3 08/10] xen/arm: introduce acquire_staticmem_pages and acquire_domstatic_pages
Date: Mon, 19 Jul 2021 11:00:10 +0100	[thread overview]
Message-ID: <7753fe1f-7a5c-6205-a184-25190ad43286@xen.org> (raw)
In-Reply-To: <48c59068-296e-7060-1352-f07338833378@suse.com>

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?

Cheers,

-- 
Julien Grall