openSUSE Build Service

Overview Repositories Revisions Requests Users Attributes Meta

File xsa380-1.patch of Package xen

gnttab: add preemption check to gnttab_release_mappings()

A guest may die with many grant mappings still in place, or simply with
a large maptrack table. Iterating through this may take more time than
is reasonable without intermediate preemption (to run softirqs and
perhaps the scheduler).

Move the invocation of the function to the section where other
restartable functions get invoked, and have the function itself check
for preemption every once in a while. Have it iterate the table
backwards, such that decreasing the maptrack limit is all it takes to
convey restart information.

In domain_teardown() introduce PROG_none such that inserting at the
front will be easier going forward.

This is part of XSA-380.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>

--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -631,13 +631,15 @@ int domain_kill(struct domain *d)
         if ( d->is_dying != DOMDYING_alive )
             return domain_kill(d);
         d->is_dying = DOMDYING_dying;
-        gnttab_release_mappings(d);
         tmem_destroy(d->tmem_client);
         vnuma_destroy(d->vnuma);
         domain_set_outstanding_pages(d, 0);
         d->tmem_client = NULL;
         /* fallthrough */
     case DOMDYING_dying:
+        rc = gnttab_release_mappings(d);
+        if ( rc )
+            break;
         rc = evtchn_destroy(d);
         if ( rc )
             break;
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3451,9 +3451,7 @@ grant_table_create(
     return -ENOMEM;
 }
 
-void
-gnttab_release_mappings(
-    struct domain *d)
+int gnttab_release_mappings(struct domain *d)
 {
     struct grant_table   *gt = d->grant_table, *rgt;
     struct grant_mapping *map;
@@ -3467,8 +3465,32 @@ gnttab_release_mappings(
 
     BUG_ON(!d->is_dying);
 
-    for ( handle = 0; handle < gt->maptrack_limit; handle++ )
+    if ( !gt || !gt->maptrack )
+        return 0;
+
+    for ( handle = gt->maptrack_limit; handle; )
     {
+        /*
+         * Deal with full pages such that their freeing (in the body of the
+         * if()) remains simple.
+         */
+        if ( handle < gt->maptrack_limit && !(handle % MAPTRACK_PER_PAGE) )
+        {
+            /*
+             * Changing maptrack_limit alters nr_maptrack_frames()'es return
+             * value. Free the then excess trailing page right here, rather
+             * than leaving it to grant_table_destroy() (and in turn requiring
+             * to leave gt->maptrack_limit unaltered).
+             */
+            gt->maptrack_limit = handle;
+            FREE_XENHEAP_PAGE(gt->maptrack[nr_maptrack_frames(gt)]);
+
+            if ( hypercall_preempt_check() )
+                return -ERESTART;
+        }
+
+        --handle;
+
         map = &maptrack_entry(gt, handle);
         if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) )
             continue;
@@ -3556,6 +3578,11 @@ gnttab_release_mappings(
 
         map->flags = 0;
     }
+
+    gt->maptrack_limit = 0;
+    FREE_XENHEAP_PAGE(gt->maptrack[0]);
+
+    return 0;
 }
 
 void grant_table_warn_active_grants(struct domain *d)
@@ -3608,8 +3635,7 @@ grant_table_destroy(
         free_xenheap_page(t->shared_raw[i]);
     xfree(t->shared_raw);
 
-    for ( i = 0; i < nr_maptrack_frames(t); i++ )
-        free_xenheap_page(t->maptrack[i]);
+    ASSERT(!t->maptrack_limit);
     vfree(t->maptrack);
 
     for ( i = 0; i < nr_active_grant_frames(t); i++ )
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -77,6 +77,13 @@ struct grant_table {
     struct active_grant_entry **active;
     /* Mapping tracking table per vcpu. */
     struct grant_mapping **maptrack;
+    /*
+     * Number of available maptrack entries.  For cleanup purposes it is
+     * important to realize that this field and @maptrack further down will
+     * only ever be accessed by the local domain.  Thus it is okay to clean
+     * up early, and to shrink the limit for the purpose of tracking cleanup
+     * progress.
+     */
     unsigned int          maptrack_limit;
     /* Lock protecting the maptrack limit */
     spinlock_t            maptrack_lock;
@@ -118,9 +125,7 @@ void grant_table_init_vcpu(struct vcpu *
 void grant_table_warn_active_grants(struct domain *d);
 
 /* Domain death release of granted mappings of other domains' memory. */
-void
-gnttab_release_mappings(
-    struct domain *d);
+int gnttab_release_mappings(struct domain *d);
 
 /* Increase the size of a domain's grant table.
  * Caller must hold d's grant table write lock.

Places

File xsa380-1.patch of Package xen

Places