[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains



Hi,

On 03/01/2020 10:44, Julien Grall wrote:
Hi Andrew,

On 02/01/2020 17:49, Andrew Cooper wrote:
On 23/12/2019 18:23, Julien Grall wrote:
Hi,

On 17/12/2019 21:15, Andrew Cooper wrote:
xc_dom_p2m() and dom->p2m_host[] implement a linear transform for
translated
domains, but waste a substantial chunk of RAM doing so.

ARM literally never reads dom->p2m_host[] (because of the
xc_dom_translated()
short circuit in xc_dom_p2m()).  Drop it all.

x86 HVM does use dom->p2m_host[] for
xc_domain_populate_physmap_exact() calls
when populating 4k pages.  Reuse the same tactic from 2M/1G ranges
and use an
on-stack array instead.  Drop the memory allocation.

x86 PV guests do use dom->p2m_host[] as a non-identity transform.
Rename the
field to pv_p2m to make it clear it is PV-only.

Nice cleanup! This will probably make slightly faster guest boot :).

@@ -359,7 +356,6 @@ static int populate_guest_memory(struct
xc_dom_image *dom,
   static int meminit(struct xc_dom_image *dom)
   {
       int i, rc;
-    xen_pfn_t pfn;
       uint64_t modbase;
         uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
@@ -423,11 +419,6 @@ static int meminit(struct xc_dom_image *dom)
       assert(ramsize == 0); /* Too much RAM is rejected above */
         dom->p2m_size = p2m_size;

Do we need to keep p2m_size?

Yes, I think so.

There are some common checks which would fail if it becomes 0, and
importantly, it is used to locate safe gfns for temporary physmap mappings.

Do you mean the scratch page? If so, on Arm we use a fix address in the memory map rather than p2m_size.

I have looked at the usage of the p2m_size in the common code and I didn't spot any path that can be used by Arm and using p2m_size.

Also, I don't think p2m_size is calculated correctly on Arm. It only englobe the RAM and doesn't take into account the rest of the mappings (e.g MMIO...). So I am not sure how this could be used in common code.

Thinking a bit more of this. As the p2m_size was IHMO already buggy, it should not make any difference after this series. Therefore removing p2m_size can be probably done on a follow-up patch.

So I am happy with the patch as-is:

Acked-by: Julien Grall <julien@xxxxxxx>

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.