Re: bogus gfn - mfn - gfn - mfn checks in guest_physmap_add_entry

[Olaf Hering <olaf@aepfle.de>]

On Wed, Nov 24, Tim Deegan wrote:

> The problem is that PV guests set their own m2p entries and can't be
> relied on to tear them down.  

What needs to happen for PV guests?
Dont they use the machine_to_phys_mapping[] array like HVM guests?

> The guest_physmap_add_entry code, and the p2m audit code, would be made
> more reliable if, say, alloc_domheap_pages and/or free_domheap_pages
> zapped the m2p entries for MFNs they touched.
> 
> I think originally that wasn't done because the alloc is quickly
> followed by another write of the m2p but that's probably over-keen
> optimization.

Could it be done like that? (not yet compile-tested)
The mfn is probably always valid.

I see memory_exchange uses assign_pages() to move mfns from one domain
to another (havent studied the whole function yet). I think thats
another place that needs an audit wether the machine_to_phys_mapping[]
array is maintained properly.

--- xen-4.0.1-testing.orig/xen/common/page_alloc.c
+++ xen-4.0.1-testing/xen/common/page_alloc.c
@@ -1146,6 +1146,8 @@ struct page_info *alloc_domheap_pages(
     struct page_info *pg = NULL;
     unsigned int bits = memflags >> _MEMF_bits, zone_hi = NR_ZONES - 1;
     unsigned int node = (uint8_t)((memflags >> _MEMF_node) - 1), dma_zone;
+    int i;
+    mfn_t mfn;
 
     ASSERT(!in_irq());
 
@@ -1170,6 +1172,13 @@ struct page_info *alloc_domheap_pages(
         free_heap_pages(pg, order);
         return NULL;
     }
+    /* this page is not yet a gfn */
+    mfn = page_to_mfn(pg);
+    if (mfn_valid(mfn))
+    {
+        for ( i = 0; i < (1 << order); i++ )
+            set_gpfn_from_mfn(mfn_x(mfn) + j, INVALID_M2P_ENTRY);
+    }
     
     return pg;
 }
@@ -1178,9 +1187,18 @@ void free_domheap_pages(struct page_info
 {
     int            i, drop_dom_ref;
     struct domain *d = page_get_owner(pg);
+    mfn_t mfn;
 
     ASSERT(!in_irq());
 
+    /* this page is not a gfn anymore */
+    mfn = page_to_mfn(pg);
+    if (mfn_valid(mfn))
+    {
+        for ( i = 0; i < (1 << order); i++ )
+            set_gpfn_from_mfn(mfn_x(mfn) + j, INVALID_M2P_ENTRY);
+    }
+
     if ( unlikely(is_xen_heap_page(pg)) )
     {
         /* NB. May recursively lock from relinquish_memory(). */