Re: [Xen-devel] [PATCH] x86: Get rid of p2m_host array allocation for HVM guests

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrew,

On 7/3/19 8:39 PM, Andrew Cooper wrote:
> On 03/07/2019 14:06, Varad Gautam wrote:
> > When allocating the guest memory for an HVM domain, libxc keeps the P2M
> > mapping for the entirety of the guest memory around for the time of the
> > launch as xc_dom_image->p2m_host. For guests that have a large memory
> > (3904 GiB),
> This is slightly awkward phrasing.  How about "For large memory guests
> (e.g. 3904 GiB), "
>
> >   the p2m_host allocation takes more than 7.5 GiB of space, and
> > leaves xl susceptible to getting OOM-killed on guest creation.
> >
> > Convert the p2m_host table lookups to an arch-specific function that
> > returns the mapping on-the-fly for x86 HVM guests to avoid this
> > allocation, bringing down xl's memory usage from > 8GiB to < 70Mib for
> > such launches.
> >
> > Signed-off-by: Varad Gautam <vrd@xxxxxxxxx>
> I can definitely see why you want to avoid this overhead.
>
> In practice, p2m_host looks to be a relic of x86 PV guests.
>
> Now - first things first.  Exactly the same reasoning applies to all ARM
> guests.  (CC'ing the ARM maintainers for visibility)
It is already in my todo list to remove p2m_host for Arm, thanks to a 
discussion with Wei last year :). I have never had the chance to work on 
it so would be happy if someone pick it up!
> However, in attempting to review this, I've got some bigger questions.
>
> All ARM and x86 HVM (and PVH) guests return true for
> xc_dom_translated(), so should take the fastpath out of xc_dom_p2m() and
> never read from dom->p2m_host[].  Therefore, I don't see why the
> majority of this patch is necessary.
I agree that p2m_host will never get used by Arm. So this is a waste of 
memory.
>   On the ARM side, this also means
> that dom->rambase_pfn isn't being used as intended, which suggests there
> is further cleanup/correction to be done here.
I am not sure to follow this. Could you expand it?

> xc_dom_update_guest_p2m() is clearly specific to x86 PV guests.  No
> other guest types set dom->p2m_guest.
>
> I don't see why anything is needed with the vmemranges[].  The result of
> the new p2m_host() hook never has its return value checked, and
> dom->p2m_host is still an identity transform within those ranges.
>
> Unless I'm missing something, I think the result can be rather more
> simple, and strip out a fair amount of code, by moving
> p2m_host/p2m_guest into x86 PV's arch_private area.
>
> ~Andrew
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel