Re: [PATCH v4 6/6] xen: retrieve reserved pages on populate_physmap

[Julien Grall <julien@xen.org>]

On 17/05/2022 07:24, Penny Zheng wrote:
> Hi Julien

Hi Penny,

>>> +    if ( unlikely(!page) )
>>> +        return INVALID_MFN;
>>> +
>>> +    smfn = page_to_mfn(page);
>>> +
>>> +    if ( acquire_domstatic_pages(d, smfn, 1, memflags) )
>>
>> I am OK if we call acquire_domstatic_pages() for now. But long term, I think we
>> should consider to optimize it because we know the page is valid and belong
>> to the guest. So there are a lot of pointless work (checking mfn_valid(),
>> scrubbing in the free part, cleaning the cache...).
>>
> 
> I'm willing to fix it here since this fix is not blocking any other patch serie~~
> I'm considering that maybe we could add a new memflag MEMF_xxx, (oh,
> Naming something is really "killing" me), then all these pointless work, checking
> mfn_valid, flushing TLB and cache, we could exclude them by checking
> memflags & MEMF_xxxx.
> Wdyt?

I don't think we need a new flag because the decision is internal to the 
page allocator. Instead, acquire_staticmem_pages() could be split in two 
parts. Something like (function names are random):


static bool foo(struct page_info *pg,
	        unsigned long nr,
	        unsigned long memflags)
{
	spin_lock(&heap_lock);

	for ( i = 0; i < nr; i++ )
		...

	spin_unlock(&heap_lock);

	if ( need_tlbflush )
	  filtered...

  	return true;

out_err:
	for ( ... )
	  ...
	return false;
}

static struct page_info *bar(mfn_t smfn,
			     unsigned long mfn,
			     unsigned int memflags)
{
	ASSERT(nr_mfns);
	for ( i = 0; i < nr_mfns; i++ )
	    if ( !mfn_valid(mfn_add(smfn, i)) )
		return NULL;

	pg = mfn_to_page(mfn);
	if ( !foo(...) )
	  return NULL;

	for ( i = 0; i < nr_mfns; i++ )
		flush_page_to_ram(...);
}


acquire_reserved_page() would then only call foo() and assign_pages().

Cheers,

-- 
Julien Grall