[PATCH v15 08/10] x86/shadow: make log-dirty mode enable/disable properly preemptable

[Jan Beulich <jbeulich@xxxxxxxx>]

Their calls to shadow_set_allocation() are the last missing piece.

While it may seem a little odd, it actually turns out easier to deal
with the continuation a level up from where the need for it was first
recognized.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I think the point was raised before: It's questionable whether
shadow_one_bit_enable(), upon error, should bring the allocation back
down to zero. This is going to be especially bad for a domain which
previously had shadow enabled already (which could be HVM or L1TF-
affected PV). Even in shadow_one_bit_disable() it's not clear this is
the best possible behavior - the pool may have been set to something
larger than the default by the admin. For now I'm maintaining prior
behavior, but of course things would end up simpler if we could just get
rid of those set-to-zero operations (and then perhaps also on
shadow_enable()'s similar error path); the possible caveat there would
be that overall memory consumption may then appear to grow for people
monitoring a system.

Originally I was considering to further qualify the
d->arch.paging.preempt.drop_allocation checks by passing further down
the "resuming" flag, but for a well-behaved tool stack (which allows one
shadow-op to finish before starting another one) there shouldn't be a
difference. Thoughts?
---
v14: New.

--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -236,6 +236,7 @@ struct paging_domain {
     struct {
         const struct domain *dom;
         unsigned int op;
+        bool drop_allocation:1;
         union {
             struct {
                 unsigned long done:PADDR_BITS - PAGE_SHIFT;
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -215,9 +215,10 @@ static int paging_log_dirty_enable(struc
 
     paging_lock(d);
 
-    if ( d->arch.paging.preempt.dom &&
-         (d->arch.paging.preempt.dom != current->domain ||
-          d->arch.paging.preempt.op != op) )
+    if ( !d->arch.paging.preempt.dom )
+        d->arch.paging.preempt.drop_allocation = false;
+    else if ( d->arch.paging.preempt.dom != current->domain ||
+              d->arch.paging.preempt.op != op )
     {
         paging_unlock(d);
         if ( !resuming )
@@ -259,9 +260,10 @@ static int paging_log_dirty_disable(stru
 
     paging_lock(d);
 
-    if ( d->arch.paging.preempt.dom &&
-         (d->arch.paging.preempt.dom != current->domain ||
-          d->arch.paging.preempt.op != XEN_DOMCTL_SHADOW_OP_OFF) )
+    if ( !d->arch.paging.preempt.dom )
+        d->arch.paging.preempt.drop_allocation = false;
+    else if ( d->arch.paging.preempt.dom != current->domain ||
+              d->arch.paging.preempt.op != XEN_DOMCTL_SHADOW_OP_OFF )
     {
         paging_unlock(d);
         if ( !resuming )
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2440,12 +2440,22 @@ static int shadow_one_bit_enable(struct
 
     if ( d->arch.paging.total_pages < sh_min_allocation(d) )
     {
+        bool preempted = false;
+
         /* Init the shadow memory allocation if the user hasn't done so */
-        if ( shadow_set_allocation(d, 1, NULL) != 0 )
+        if ( shadow_set_allocation(d, 1,
+                                   mode & PG_log_dirty ? &preempted
+                                                       : NULL) != 0 )
         {
-            shadow_set_allocation(d, 0, NULL);
-            return -ENOMEM;
+            shadow_set_allocation(d, 0,
+                                  mode & PG_log_dirty ? &preempted : NULL);
+            if ( !preempted )
+                return -ENOMEM;
+            d->arch.paging.preempt.drop_allocation = true;
         }
+
+        if ( preempted )
+            return -ERESTART;
     }
 
     /* Allow p2m and log-dirty code to borrow shadow memory */
@@ -2484,6 +2494,8 @@ static int shadow_one_bit_disable(struct
     sh_new_mode(d, mode);
     if ( d->arch.paging.mode == 0 )
     {
+        bool preempted = false;
+
         /* Get this domain off shadows */
         SHADOW_PRINTK("un-shadowing of domain %u starts."
                        "  Shadow pages total = %u, free = %u, p2m=%u\n",
@@ -2511,8 +2523,16 @@ static int shadow_one_bit_disable(struct
         }
 
         /* Pull down the memory allocation */
-        if ( shadow_set_allocation(d, 0, NULL) != 0 )
+        if ( shadow_set_allocation(d, 0,
+                                   mode & PG_log_dirty ? &preempted
+                                                       : NULL) != 0 )
             BUG(); /* In fact, we will have BUG()ed already */
+        if ( preempted )
+        {
+            d->arch.paging.preempt.drop_allocation = true;
+            return -ERESTART;
+        }
+
         shadow_hash_teardown(d);
         SHADOW_PRINTK("un-shadowing of domain %u done."
                        "  Shadow pages total = %u, free = %u, p2m=%u\n",
@@ -2558,14 +2578,20 @@ static int shadow_test_disable(struct do
  */
 static int cf_check sh_enable_log_dirty(struct domain *d)
 {
+    bool preempted = false;
     int ret;
 
     ASSERT(paging_locked_by_me(d));
 
-    if ( shadow_mode_enabled(d) )
+    if ( d->arch.paging.preempt.drop_allocation )
     {
-        bool preempted = false;
+        shadow_set_allocation(d, 0, &preempted);
 
+        return preempted ? -ERESTART : -ENOMEM;
+    }
+
+    if ( shadow_mode_enabled(d) )
+    {
         /*
          * This domain already has some shadows: need to clear them out
          * of the way to make sure that all references to guest memory are
@@ -2598,6 +2624,21 @@ static int cf_check sh_disable_log_dirty
 
     ASSERT(paging_locked_by_me(d));
 
+    if ( d->arch.paging.preempt.drop_allocation )
+    {
+        shadow_set_allocation(d, 0, &preempted);
+
+        if ( preempted )
+            return -ERESTART;
+
+        shadow_hash_teardown(d);
+        SHADOW_PRINTK("un-shadowing of domain %u done."
+                       "  Shadow pages total = %u, free = %u, p2m=%u\n",
+                       d->domain_id, d->arch.paging.total_pages,
+                       d->arch.paging.free_pages, d->arch.paging.p2m_pages);
+        return 0;
+    }
+
     /*
      * Limit the amount of work to do from sh_detach_old_tables() (called from
      * shadow_one_bit_disable() via sh_new_mode() -> sh_update_paging_modes()),