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

Re: [Xen-devel] [PATCH 04/18] arm/altp2m: Add altp2m init/teardown routines.



On 04/07/16 17:51, Sergej Proskurin wrote:
On 07/04/2016 06:15 PM, Julien Grall wrote:


On 04/07/16 12:45, Sergej Proskurin wrote:
+static void p2m_teardown_hostp2m(struct domain *d)
+{
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    struct page_info *pg = NULL;
+    mfn_t mfn;
+    unsigned int i;
+
+    spin_lock(&p2m->lock);

-    if ( p2m->root )
-        free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
+    while ( (pg = page_list_remove_head(&p2m->pages)) )
+        if ( pg != p2m->root )
+        {
+            mfn = _mfn(page_to_mfn(pg));
+            clear_domain_page(mfn);

Can you explain why you are cleaning the page here? It was not part of
p2m_teardown before this series.


With the x86-based altp2m implementation, we experienced the problem
that altp2m-teardowns did not clean the pages. As a result, later
re-initialization reused the pages, which subsequently led to faults or
crashes due to reused mappings. We additionally clean the altp2m pages
and for the sake of completeness we clean the hostp2m tables as well.

All the pages allocated for the p2m are cleared before any usage (see p2m_create_table and p2m_allocate_table). So there is no point to zero the page here.

Also, unlike x86 we don't have a pool of page and directly use alloc_domheap_page to allocate a new page. So this would not prevent to get a non-zeroed page.

In any case, a such change should be in a separate patch and not hidden among big changes.

Regards,

--
Julien Grall

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

 


Rackspace

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