From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction Date: Thu, 30 Jul 2015 10:38:56 +0100 Message-ID: <55B9F0B0.1020209@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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZKkJc-0006ov-3V for xen-devel@lists.xenproject.org; Thu, 30 Jul 2015 09:39:52 +0000 In-Reply-To: <1438248776.11600.270.camel@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: Ian Campbell , 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 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. -George