[PATCH 2/3] x86/shadow: VRAM last_dirty tagging

[Jan Beulich <jbeulich@xxxxxxxx>]

->last_dirty holding a valid value (one other than -1) is solely an
indication of the bitmap being entirely clean. (The opposite isn't true,
because of _sh_propagate() setting the field to a valid value without
setting a bit in the bitmap.) As a consequence
- setting the field to a valid value right after having allocated zero-
  filled space is pointless,
- copying the the all empty bitmap to the output array is pointless; with
  the output array also having been allocated zero-filled, not even a
  memset() is needed there,
- after restoring bitmap contents when dealing with copy_to_guest() having
  failed, the field needs setting to a valid value again.

Furthermore invoking NOW() in perhaps many loop iterations of the main
loop is wasteful, too. Record whether any bit was set, and record a new
->last_dirty only once, after the loop. Then use the same NOW() value also
for the subsequent check.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -1087,18 +1087,18 @@ int shadow_track_dirty_vram(struct domai
         if ( (dirty_vram->dirty_bitmap = xzalloc_array(uint8_t, dirty_size)) 
== NULL )
             goto out_sl1ma;
 
-        dirty_vram->last_dirty = NOW();
+        dirty_vram->last_dirty = -1;
 
         /* Tell the caller that this time we could not track dirty bits. */
         rc = -ENODATA;
     }
-    else if ( dirty_vram->last_dirty == -1 )
-        /* still completely clean, just copy our empty bitmap */
-        memcpy(dirty_bitmap, dirty_vram->dirty_bitmap, dirty_size);
-    else
+    /* Nothing to do when the bitmap is still completely clean. */
+    else if ( dirty_vram->last_dirty != -1 )
     {
         mfn_t map_mfn = INVALID_MFN;
         void *map_sl1p = NULL;
+        bool any_dirty = false;
+        s_time_t now;
 
         /* Iterate over VRAM to track dirty bits. */
         for ( i = 0; i < nr_frames; i++ )
@@ -1174,16 +1174,20 @@ int shadow_track_dirty_vram(struct domai
             if ( dirty )
             {
                 dirty_vram->dirty_bitmap[i / 8] |= 1 << (i % 8);
-                dirty_vram->last_dirty = NOW();
+                any_dirty = true;
             }
         }
 
+        now = NOW();
+        if ( any_dirty )
+            dirty_vram->last_dirty = now;
+
         if ( map_sl1p )
             unmap_domain_page(map_sl1p);
 
         memcpy(dirty_bitmap, dirty_vram->dirty_bitmap, dirty_size);
         memset(dirty_vram->dirty_bitmap, 0, dirty_size);
-        if ( dirty_vram->last_dirty + SECONDS(2) < NOW() )
+        if ( dirty_vram->last_dirty + SECONDS(2) < now )
         {
             /*
              * Was clean for more than two seconds, try to disable guest
@@ -1216,6 +1220,7 @@ int shadow_track_dirty_vram(struct domai
         paging_lock(d);
         for ( i = 0; i < dirty_size; i++ )
             dirty_vram->dirty_bitmap[i] |= dirty_bitmap[i];
+        dirty_vram->last_dirty = NOW();
         paging_unlock(d);
         rc = -EFAULT;
     }