[PATCH 3/3] xen/evtchn: Clean up teardown handling

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>
Subject: [PATCH 3/3] xen/evtchn: Clean up teardown handling
Date: Mon, 21 Dec 2020 18:14:46 +0000	[thread overview]
Message-ID: <20201221181446.7791-4-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20201221181446.7791-1-andrew.cooper3@citrix.com>

First of all, rename the evtchn APIs:
 * evtchn_destroy       => evtchn_teardown
 * evtchn_destroy_final => evtchn_destroy

Move both calls into appropriate positions in domain_teardown() and
_domain_destroy(), which avoids having different cleanup logic depending on
the the cause of the cleanup.

In particular, this avoids evtchn_teardown() (previously named
evtchn_destroy()) being called redundantly thousands of times on a typical
XEN_DOMCTL_destroydomain hypercall.

No net change in behaviour.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>

RFC.  While testing this, I observed this, after faking up an -ENOMEM in
dom0's construction:

  (XEN) [2020-12-21 16:31:20] NX (Execute Disable) protection active
  (XEN) [2020-12-21 16:33:04]
  (XEN) [2020-12-21 16:33:04] ****************************************
  (XEN) [2020-12-21 16:33:04] Panic on CPU 0:
  (XEN) [2020-12-21 16:33:04] Error creating domain 0
  (XEN) [2020-12-21 16:33:04] ****************************************

XSA-344 appears to have added nearly 2 minutes of wallclock time into the
domain_create() error path, which isn't ok.

Considering that event channels haven't even been initialised in this
particular scenario, it ought to take ~0 time.  Even if event channels have
been initalised, none can be active as the domain isn't visible to the system.
---
 xen/common/domain.c        | 17 ++++++++---------
 xen/common/event_channel.c |  8 ++++----
 xen/include/xen/sched.h    |  4 ++--
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index ef1987335b..701747b9d9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -284,6 +284,8 @@ custom_param("extra_guest_irqs", parse_extra_guest_irqs);
  */
 static int domain_teardown(struct domain *d)
 {
+    int rc;
+
     BUG_ON(!d->is_dying);
 
     /*
@@ -313,6 +315,10 @@ static int domain_teardown(struct domain *d)
         };
 
     case 0:
+        rc = evtchn_teardown(d);
+        if ( rc )
+            return rc;
+
     PROGRESS(done):
         break;
 
@@ -335,6 +341,8 @@ static void _domain_destroy(struct domain *d)
     BUG_ON(!d->is_dying);
     BUG_ON(atomic_read(&d->refcnt) != DOMAIN_DESTROYED);
 
+    evtchn_destroy(d);
+
     xfree(d->pbuf);
 
     argo_destroy(d);
@@ -598,11 +606,7 @@ struct domain *domain_create(domid_t domid,
     if ( init_status & INIT_gnttab )
         grant_table_destroy(d);
     if ( init_status & INIT_evtchn )
-    {
-        evtchn_destroy(d);
-        evtchn_destroy_final(d);
         radix_tree_destroy(&d->pirq_tree, free_pirq_struct);
-    }
     if ( init_status & INIT_watchdog )
         watchdog_domain_destroy(d);
 
@@ -792,9 +796,6 @@ int domain_kill(struct domain *d)
         rc = domain_teardown(d);
         if ( rc )
             break;
-        rc = evtchn_destroy(d);
-        if ( rc )
-            break;
         rc = domain_relinquish_resources(d);
         if ( rc != 0 )
             break;
@@ -987,8 +988,6 @@ static void complete_domain_destroy(struct rcu_head *head)
     if ( d->target != NULL )
         put_domain(d->target);
 
-    evtchn_destroy_final(d);
-
     radix_tree_destroy(&d->pirq_tree, free_pirq_struct);
 
     xfree(d->vcpu);
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 4a48094356..c1af54eed5 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1401,7 +1401,7 @@ void free_xen_event_channel(struct domain *d, int port)
     {
         /*
          * Make sure ->is_dying is read /after/ ->valid_evtchns, pairing
-         * with the spin_barrier() and BUG_ON() in evtchn_destroy().
+         * with the spin_barrier() and BUG_ON() in evtchn_teardown().
          */
         smp_rmb();
         BUG_ON(!d->is_dying);
@@ -1421,7 +1421,7 @@ void notify_via_xen_event_channel(struct domain *ld, int lport)
     {
         /*
          * Make sure ->is_dying is read /after/ ->valid_evtchns, pairing
-         * with the spin_barrier() and BUG_ON() in evtchn_destroy().
+         * with the spin_barrier() and BUG_ON() in evtchn_teardown().
          */
         smp_rmb();
         ASSERT(ld->is_dying);
@@ -1499,7 +1499,7 @@ int evtchn_init(struct domain *d, unsigned int max_port)
     return 0;
 }
 
-int evtchn_destroy(struct domain *d)
+int evtchn_teardown(struct domain *d)
 {
     unsigned int i;
 
@@ -1534,7 +1534,7 @@ int evtchn_destroy(struct domain *d)
 }
 
 
-void evtchn_destroy_final(struct domain *d)
+void evtchn_destroy(struct domain *d)
 {
     unsigned int i, j;
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3f35c537b8..bb22eeca38 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -142,8 +142,8 @@ struct evtchn
 } __attribute__((aligned(64)));
 
 int  evtchn_init(struct domain *d, unsigned int max_port);
-int  evtchn_destroy(struct domain *d); /* from domain_kill */
-void evtchn_destroy_final(struct domain *d); /* from complete_domain_destroy */
+int  evtchn_teardown(struct domain *d);
+void evtchn_destroy(struct domain *d);
 
 struct waitqueue_vcpu;
 
-- 
2.11.0


