[PATCH]: fix vram tracking (was Re: [Xen-devel] xen assert in latest 3.3.2-rc bits)

[Gianluca Guida <gianluca.guida@xxxxxxxxxxxxx>]

Mark Johnson wrote:
> I just got the following assertion when shutting down a windows 7 guest on
> the latest 3.3-testing bits.. Has anyone else seen this?
Can you try this patch?

--


Check for writable mappings in ptes before assuming that the type count 
in the page has changed.
Signed-off-by: Gianluca Guida <gianluca.guida@xxxxxxxxxxxxx>

diff -r b249f3e979a5 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c    Mon Mar 09 10:32:24 2009 +0000
+++ b/xen/arch/x86/mm/shadow/multi.c    Tue Mar 10 21:48:42 2009 +0000
@@ -1039,18 +1039,19 @@ static inline void shadow_vram_get_l1e(s
                                        mfn_t sl1mfn,
                                        struct domain *d)
 { 
-    mfn_t mfn;
+    mfn_t mfn = shadow_l1e_get_mfn(new_sl1e);
+    int flags = shadow_l1e_get_flags(new_sl1e);
     unsigned long gfn;
 
-    if ( !d->dirty_vram ) return;
-
-    mfn = shadow_l1e_get_mfn(new_sl1e);
-
-    if ( !mfn_valid(mfn) ) return; /* m2p for mmio_direct may not exist */
+    if ( !d->dirty_vram         /* tracking disabled? */
+         || !(flags & _PAGE_RW) /* read-only mapping? */
+         || !mfn_valid(mfn) )   /* mfn can be invalid in mmio_direct */
+        return;
 
     gfn = mfn_to_gfn(d, mfn);
 
-    if ( (gfn >= d->dirty_vram->begin_pfn) && (gfn < d->dirty_vram->end_pfn) ) 
{
+    if ( (gfn >= d->dirty_vram->begin_pfn) && (gfn < d->dirty_vram->end_pfn) )
+    {
         unsigned long i = gfn - d->dirty_vram->begin_pfn;
         struct page_info *page = mfn_to_page(mfn);
         
@@ -1066,48 +1067,58 @@ static inline void shadow_vram_put_l1e(s
                                        mfn_t sl1mfn,
                                        struct domain *d)
 {
-    mfn_t mfn;
+    mfn_t mfn = shadow_l1e_get_mfn(old_sl1e);
+    int flags = shadow_l1e_get_flags(old_sl1e);
     unsigned long gfn;
 
-    if ( !d->dirty_vram ) return;
-
-    mfn = shadow_l1e_get_mfn(old_sl1e);
-
-    if ( !mfn_valid(mfn) ) return;
+    if ( !d->dirty_vram         /* tracking disabled? */
+         || !(flags & _PAGE_RW) /* read-only mapping? */
+         || !mfn_valid(mfn) )   /* mfn can be invalid in mmio_direct */
+        return;
 
     gfn = mfn_to_gfn(d, mfn);
 
-    if ( (gfn >= d->dirty_vram->begin_pfn) && (gfn < d->dirty_vram->end_pfn) ) 
{
+    if ( (gfn >= d->dirty_vram->begin_pfn) && (gfn < d->dirty_vram->end_pfn) )
+    {
         unsigned long i = gfn - d->dirty_vram->begin_pfn;
         struct page_info *page = mfn_to_page(mfn);
         int dirty = 0;
         paddr_t sl1ma = pfn_to_paddr(mfn_x(sl1mfn))
             | ((unsigned long)sl1e & ~PAGE_MASK);
 
-        if ( (page->u.inuse.type_info & PGT_count_mask) == 1 ) {
+        if ( (page->u.inuse.type_info & PGT_count_mask) == 1 )
+        {
             /* Last reference */
             if ( d->dirty_vram->sl1ma[i] == INVALID_PADDR ) {
                 /* We didn't know it was that one, let's say it is dirty */
                 dirty = 1;
-            } else {
+            }
+            else
+            {
                 ASSERT(d->dirty_vram->sl1ma[i] == sl1ma);
                 d->dirty_vram->sl1ma[i] = INVALID_PADDR;
-                if ( shadow_l1e_get_flags(old_sl1e) & _PAGE_DIRTY )
+                if ( flags & _PAGE_DIRTY )
                     dirty = 1;
             }
-        } else {
+        }
+        else
+        {
             /* We had more than one reference, just consider the page dirty. */
             dirty = 1;
             /* Check that it's not the one we recorded. */
-            if ( d->dirty_vram->sl1ma[i] == sl1ma ) {
+            if ( d->dirty_vram->sl1ma[i] == sl1ma )
+            {
                 /* Too bad, we remembered the wrong one... */
                 d->dirty_vram->sl1ma[i] = INVALID_PADDR;
-            } else {
+            }
+            else
+            {
                 /* Ok, our recorded sl1e is still pointing to this page, let's
                  * just hope it will remain. */
             }
         }
-        if ( dirty ) {
+        if ( dirty )
+        {
             d->dirty_vram->dirty_bitmap[i / 8] |= 1 << (i % 8);
             d->dirty_vram->last_dirty = NOW();
         }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel