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

Re: [Xen-devel] [PATCH v12 1/3] xen/mem_sharing: VM forking

On 25/03/2020 16:47, Tamas K Lengyel wrote:
On Wed, Mar 25, 2020 at 10:42 AM Julien Grall <julien@xxxxxxx> wrote:


On 25/03/2020 16:34, Tamas K Lengyel wrote:
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 9f51370327..1ed7d13084 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -509,6 +509,12 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, 
unsigned long gfn_l,

       mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL);

+    /* Check if we need to fork the page */
+    if ( (q & P2M_ALLOC) && p2m_is_hole(*t) &&
+         !mem_sharing_fork_page(p2m->domain, gfn, q & P2M_UNSHARE) )
+        mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL);
+    /* Check if we need to unshare the page */
       if ( (q & P2M_UNSHARE) && p2m_is_shared(*t) )
@@ -588,7 +594,8 @@ struct page_info *p2m_get_page_from_gfn(
               return page;

           /* Error path: not a suitable GFN at all */
-        if ( !p2m_is_ram(*t) && !p2m_is_paging(*t) && !p2m_is_pod(*t) )
+        if ( !p2m_is_ram(*t) && !p2m_is_paging(*t) && !p2m_is_pod(*t) &&
+             !mem_sharing_is_fork(p2m->domain) )
               return NULL;

diff --git a/xen/common/domain.c b/xen/common/domain.c
index b4eb476a9c..62aed53a16 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1270,6 +1270,9 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, 
unsigned offset)

       v->vcpu_info = new_info;
       v->vcpu_info_mfn = page_to_mfn(page);
+    v->vcpu_info_offset = offset;

There's no need to introduce this field, you can just use v->vcpu_info

Just doing what you suggest above results in:

mem_sharing.c:1603:55: error: invalid operands to binary & (have
‘vcpu_info_t * const’ {aka ‘union <anonymous> * const’} and ‘long
                                       d_vcpu->vcpu_info & ~PAGE_MASK);

I can of course cast the vcpu_info pointer to (long int), it's just a
bit ugly. Thoughts?

FWIW, I will also need the offset for liveupdate. I have used (unsigned
long)v->vcpu_info & ~PAGE_MASK so far but this is not really pretty.

So I am all for either a new field or a macro hiding this uglyness.

A macro sounds like a good way to go, no need for an extra field if we
can calculate it based on the currently existing one. How about

#define VCPU_INFO_OFFSET(v) (((unsigned long)v->vcpu_info) & ~PAGE_MASK)

I was more thinking a generic macro to find the offset in a page.

PAGE_OFFSET(ptr) ((unsigned long)(ptr) & ~PAGE_MASK)

Anyway, I am happy either way.


Julien Grall



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