[Xen-changelog] [xen master] x86/HVM: relinquish resources also from hvm_domain_destroy()

[patchbot@xxxxxxx]

commit b3344bb1cae0c9ac22a57db8ecca488ad0e4a66d
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Fri Jan 31 16:47:29 2020 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Jan 31 16:47:29 2020 +0100

    x86/HVM: relinquish resources also from hvm_domain_destroy()
    
    Domain creation failure paths don't call domain_relinquish_resources(),
    yet allocations and alike done from hvm_domain_initialize() need to be
    undone nevertheless. Call the function also from hvm_domain_destroy(),
    after making sure all descendants are idempotent.
    
    Note that while viridian_{domain,vcpu}_deinit() were already used in
    ways suggesting they're idempotent, viridian_time_vcpu_deinit() actually
    wasn't: One can't kill a timer that was never initialized.
    
    For hvm_destroy_all_ioreq_servers()'s purposes make
    relocate_portio_handler() return whether the to be relocated port range
    was actually found. This seems cheaper than introducing a flag into
    struct hvm_domain's ioreq_server sub-structure.
    
    In hvm_domain_initialise() additionally
    - use XFREE() also to replace adjacent xfree(),
    - use hvm_domain_relinquish_resources() as being idempotent now.
    There as well as in hvm_domain_destroy() the explicit call to
    rtc_deinit() isn't needed anymore.
    
    In hvm_domain_relinquish_resources() additionally drop a no longer
    relevant if().
    
    Fixes: e7a9b5e72f26 ("viridian: separately allocate domain and vcpu 
structures")
    Fixes: 26fba3c85571 ("viridian: add implementation of synthetic timers")
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Paul Durrant <pdurrant@xxxxxxxxxx>
---
 xen/arch/x86/hvm/hpet.c          |  4 +++-
 xen/arch/x86/hvm/hvm.c           | 27 +++++++++++++++------------
 xen/arch/x86/hvm/intercept.c     |  6 ++++--
 xen/arch/x86/hvm/ioreq.c         |  3 +++
 xen/arch/x86/hvm/pmtimer.c       |  2 +-
 xen/arch/x86/hvm/rtc.c           |  3 ++-
 xen/arch/x86/hvm/viridian/time.c |  2 ++
 xen/include/asm-x86/hvm/io.h     |  2 +-
 8 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 12f13f8c3c..9fbdc5302b 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -751,7 +751,7 @@ void hpet_deinit(struct domain *d)
     int i;
     HPETState *h = domain_vhpet(d);
 
-    if ( !has_vhpet(d) )
+    if ( !has_vhpet(d) || !d->arch.hvm.pl_time || !h->stime_freq )
         return;
 
     write_lock(&h->lock);
@@ -763,6 +763,8 @@ void hpet_deinit(struct domain *d)
         for ( i = 0; i < HPET_TIMER_NUM; i++ )
             if ( timer_enabled(h, i) )
                 hpet_stop_timer(h, i, guest_time);
+
+        h->hpet.config = 0;
     }
 
     write_unlock(&h->lock);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0b93609a82..ea99417f08 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -696,24 +696,24 @@ int hvm_domain_initialise(struct domain *d)
     return 0;
 
  fail2:
-    rtc_deinit(d);
     stdvga_deinit(d);
     vioapic_deinit(d);
  fail1:
     if ( is_hardware_domain(d) )
         xfree(d->arch.hvm.io_bitmap);
-    xfree(d->arch.hvm.io_handler);
-    xfree(d->arch.hvm.params);
-    xfree(d->arch.hvm.pl_time);
-    xfree(d->arch.hvm.irq);
+    XFREE(d->arch.hvm.io_handler);
+    XFREE(d->arch.hvm.params);
+    XFREE(d->arch.hvm.pl_time);
+    XFREE(d->arch.hvm.irq);
  fail0:
     hvm_destroy_cacheattr_region_list(d);
     destroy_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0);
  fail:
-    viridian_domain_deinit(d);
+    hvm_domain_relinquish_resources(d);
     return rc;
 }
 
+/* This function and all its descendants need to be to be idempotent. */
 void hvm_domain_relinquish_resources(struct domain *d)
 {
     if ( hvm_funcs.domain_relinquish_resources )
@@ -730,11 +730,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
 
     /* Stop all asynchronous timer actions. */
     rtc_deinit(d);
-    if ( d->vcpu != NULL && d->vcpu[0] != NULL )
-    {
-        pmtimer_deinit(d);
-        hpet_deinit(d);
-    }
+    pmtimer_deinit(d);
+    hpet_deinit(d);
 }
 
 void hvm_domain_destroy(struct domain *d)
@@ -742,6 +739,13 @@ void hvm_domain_destroy(struct domain *d)
     struct list_head *ioport_list, *tmp;
     struct g2m_ioport *ioport;
 
+    /*
+     * This function would not be called when domain initialization fails
+     * (late enough), so do so here. This requires the function and all its
+     * descendants to be idempotent.
+     */
+    hvm_domain_relinquish_resources(d);
+
     XFREE(d->arch.hvm.io_handler);
     XFREE(d->arch.hvm.params);
 
@@ -750,7 +754,6 @@ void hvm_domain_destroy(struct domain *d)
     if ( hvm_funcs.domain_destroy )
         alternative_vcall(hvm_funcs.domain_destroy, d);
 
-    rtc_deinit(d);
     stdvga_deinit(d);
     vioapic_deinit(d);
 
diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c
index 90202bdcec..0976a992ad 100644
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -300,7 +300,7 @@ void register_portio_handler(struct domain *d, unsigned int 
port,
     handler->portio.action = action;
 }
 
-void relocate_portio_handler(struct domain *d, unsigned int old_port,
+bool relocate_portio_handler(struct domain *d, unsigned int old_port,
                              unsigned int new_port, unsigned int size)
 {
     unsigned int i;
@@ -317,9 +317,11 @@ void relocate_portio_handler(struct domain *d, unsigned 
int old_port,
              (handler->portio.size = size) )
         {
             handler->portio.port = new_port;
-            break;
+            return true;
         }
     }
+
+    return false;
 }
 
 bool_t hvm_mmio_internal(paddr_t gpa)
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index d347144096..e51aebd69a 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -1228,6 +1228,9 @@ void hvm_destroy_all_ioreq_servers(struct domain *d)
     struct hvm_ioreq_server *s;
     unsigned int id;
 
+    if ( !relocate_portio_handler(d, 0xcf8, 0xcf8, 4) )
+        return;
+
     spin_lock_recursive(&d->arch.hvm.ioreq_server.lock);
 
     /* No need to domain_pause() as the domain is being torn down */
diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 402bc8e6a2..21ebb93331 100644
--- a/xen/arch/x86/hvm/pmtimer.c
+++ b/xen/arch/x86/hvm/pmtimer.c
@@ -373,7 +373,7 @@ void pmtimer_deinit(struct domain *d)
 {
     PMTState *s = &d->arch.hvm.pl_time->vpmt;
 
-    if ( !has_vpm(d) )
+    if ( !has_vpm(d) || !d->arch.hvm.pl_time || !s->vcpu )
         return;
 
     kill_timer(&s->timer);
diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c
index bb41efe84a..ce603b8cf8 100644
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -844,7 +844,8 @@ void rtc_deinit(struct domain *d)
 {
     RTCState *s = domain_vrtc(d);
 
-    if ( !has_vrtc(d) )
+    if ( !has_vrtc(d) || !d->arch.hvm.pl_time ||
+         s->update_timer.status == TIMER_STATUS_invalid )
         return;
 
     spin_barrier(&s->lock);
diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
index 3de5665c02..24ff117edb 100644
--- a/xen/arch/x86/hvm/viridian/time.c
+++ b/xen/arch/x86/hvm/viridian/time.c
@@ -524,6 +524,8 @@ void viridian_time_vcpu_deinit(const struct vcpu *v)
     {
         struct viridian_stimer *vs = &vv->stimer[i];
 
+        if ( !vs->v )
+            continue;
         kill_timer(&vs->timer);
         vs->v = NULL;
     }
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index 7ceb119b64..f5a8813b33 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -112,7 +112,7 @@ void register_portio_handler(
     struct domain *d, unsigned int port, unsigned int size,
     portio_action_t action);
 
-void relocate_portio_handler(
+bool relocate_portio_handler(
     struct domain *d, unsigned int old_port, unsigned int new_port,
     unsigned int size);
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog