From patchwork Wed Jan 29 12:59:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jan Beulich X-Patchwork-Id: 11356159 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6F8801395 for ; Wed, 29 Jan 2020 13:01:20 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 559F720716 for ; Wed, 29 Jan 2020 13:01:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 559F720716 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iwmwZ-0003o8-8W; Wed, 29 Jan 2020 12:59:43 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iwmwX-0003o3-Ue for xen-devel@lists.xenproject.org; Wed, 29 Jan 2020 12:59:41 +0000 X-Inumbo-ID: 363c5b26-4297-11ea-88b7-12813bfff9fa Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 363c5b26-4297-11ea-88b7-12813bfff9fa; Wed, 29 Jan 2020 12:59:41 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 31CC0ABF6; Wed, 29 Jan 2020 12:59:40 +0000 (UTC) From: Jan Beulich To: "xen-devel@lists.xenproject.org" Message-ID: Date: Wed, 29 Jan 2020 13:59:40 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.4.2 MIME-Version: 1.0 Content-Language: en-US Subject: [Xen-devel] [PATCH v2] x86/HVM: relinquish resources also from hvm_domain_destroy() X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Andrew Cooper , Paul Durrant , Wei Liu , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" 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 Reviewed-by: Roger Pau Monné Acked-by: Andrew Cooper Reviewed-by: Paul Durrant --- v2: Also drop rtc_deinit() from hvm_domain_destroy(). Also drop now pointless if() from hvm_domain_relinquish_resources(). --- 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); --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -696,24 +696,24 @@ int hvm_domain_initialise(struct domain 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(str /* 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); --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -1228,6 +1228,9 @@ void hvm_destroy_all_ioreq_servers(struc 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 */ --- a/xen/arch/x86/hvm/intercept.c +++ b/xen/arch/x86/hvm/intercept.c @@ -300,7 +300,7 @@ void register_portio_handler(struct doma 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 doma (handler->portio.size = size) ) { handler->portio.port = new_port; - break; + return true; } } + + return false; } bool_t hvm_mmio_internal(paddr_t gpa) --- 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); --- 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); --- 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 str { struct viridian_stimer *vs = &vv->stimer[i]; + if ( !vs->v ) + continue; kill_timer(&vs->timer); vs->v = NULL; } --- 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);