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

Re: [Xen-devel] [PATCH v03 01/10] xen: implement guest_physmap_pin_range



Hi Andrii,

On 02/09/14 08:46, Andrii Tseglytskyi wrote:
  int guest_physmap_mark_populate_on_demand(struct domain *d,
                                            unsigned long gfn,
                                            unsigned int order)
@@ -478,10 +551,18 @@ static int apply_one_level(struct domain *d,
      struct p2m_domain *p2m = &d->arch.p2m;
      lpae_t pte;
      const lpae_t orig_pte = *entry;
+    struct page_info *page = NULL;
      int rc;

      BUG_ON(level > 3);

+    if ( guest_physmap_pinned_range(d, orig_pte.p2m.base, 0) )

This change is wrong, orig_pte.p2m.base may not be valid. I think you have to do this check only on REMOVE and INSERT op.

Also few general questions about this patch:
- What about the destruction of the domain? Shouldn't you remove the flag? - In case of REMOVE, if the page is pinned, the error value will be ignored (this is because guest_physmap_remove_page is returning void). So the upper code (see guest_remove_page in common/memory.c) will think the mapping has effectively been removed and will put back the page to the memory allocator... This is because we don't take a reference when is mapped.

Overall, AFIU your usage in this patch, I don't think we care if the guest decides to remove the page from the P2M. The most important things is to avoid Xen using the page for another guest. I suspect this could be done by taking a reference on the page.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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