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@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <pdurrant@amazon.com>

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 12f13f8c3cdb9c90d49fbc6be6eb5d0040890b1e..9fbdc5302bbccd50af03de3fe282875110eef160 100644 (file)
--- 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 0b93609a8201297de2c7f9a0474a301b8eed730f..ea99417f08eff9a24eee1dc20cdbd92802288b9c 100644 (file)
--- 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 90202bdcecef891bb8ac0bee631d6dd4ba529395..0976a992adf3a89525f9614889c5ef5814b8755b 100644 (file)
--- 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 d347144096ce7c60e0c35fe0c9cfe93cb7a147e5..e51aebd69a26fab7bbc9529c2c67248a7aa5441e 100644 (file)
--- 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 402bc8e6a27e5dff6968030bb9340eba45bd2afe..21ebb933315c17ec23adb0fbefcf6319c4c83034 100644 (file)
--- 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 bb41efe84a996f202078a587d9143310d5fb9ff0..ce603b8cf8fc2c357491858afa6064ff97ebc11d 100644 (file)
--- 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 3de5665c0225239c6cb0d03bb50aa104f8c08103..24ff117edb20ddad7de0e4478532bbb85c0bfdc7 100644 (file)
--- 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 7ceb119b642d86d7ffcf5a80bfaf2856b8761867..f5a8813b33f5e516f2f45fb59ab7b9a88ee98ec6 100644 (file)
--- 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);