From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction Date: Thu, 30 Jul 2015 10:50:59 +0100 Message-ID: <1438249859.11600.271.camel@citrix.com> References: <1438187962-4047-1-git-send-email-ravi.sahita@intel.com> <55B902F6.3040005@citrix.com> <1438248061.11600.265.camel@citrix.com> <1438248776.11600.270.camel@citrix.com> <55B9F0B0.1020209@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZKkUS-0007K9-J2 for xen-devel@lists.xenproject.org; Thu, 30 Jul 2015 09:51:04 +0000 In-Reply-To: <55B9F0B0.1020209@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap , Andrew Cooper , Ravi Sahita , xen-devel@lists.xenproject.org Cc: george.dunlap@eu.citrix.com, tim@xen.org, wei.liu2@citrix.com, edmund.h.white@intel.com, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On Thu, 2015-07-30 at 10:38 +0100, George Dunlap wrote: > On 07/30/2015 10:32 AM, Ian Campbell wrote: > > On Thu, 2015-07-30 at 10:21 +0100, Ian Campbell wrote: > > > > > which I have applied with. I still don't think the commit message is > > > very > > > satisfactory, but I'm not maintainer of any of this code so meh. > > > > For the benefit of the archives perhaps someone could explain why > > gating a > > per-vcpu teardown on a host level feature setting is correct? > > > > In particular what ensures that altp2m_vcpu_initialise has been called, > > given that this is only called from HVMOP_altp2m_set_domain_state. What > > happens if that HVMOP is never touched? > > > > Do things work both for altp2m disabled on the Xen command line and > > disabled/enabled in the guest config? If so how? > > > > Also how come HVMOP_altp2m_set_domain_state does not have a > > hvm_altp2m_supported check? > > So this was all acked & stuff before I had much of a chance to comment > on it, but on my to-do list for 4.7 is to rework a lot of the > initialization / teardown stuff. In particular: > > - Always and only check for whether something has been initialized > (e.g., non-NULL, non-INVALID_MFN) before tearing it down > > - Do *all* of the initialization for both altp2m and nestedhvm when > they're actually enabled for the domain, rather than doing a bunch of > the initialization unconditionally up front. > > This is all part of the "technical debt" we were talking about when we > considered giving it a freeze exception. Thanks, I'm inferring that everything I asked about in the second from last paragraph is somehow fine, just confusingly achieved in the current code... I'm done grumping now... Ian.