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

[Julien Grall]

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