From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ed White Subject: Re: [PATCH 10/11] x86/altp2m: fix log-dirty handling. Date: Thu, 15 Jan 2015 12:49:25 -0800 Message-ID: <54B827D5.7000906@intel.com> References: <1420838801-11704-1-git-send-email-edmund.h.white@intel.com> <1420838801-11704-11-git-send-email-edmund.h.white@intel.com> <20150115172021.GI57240@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150115172021.GI57240@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: keir@xen.org, ian.jackson@eu.citrix.com, ian.campbell@citrix.com, jbeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 01/15/2015 09:20 AM, Tim Deegan wrote: > Hi, > > The locking chages look OK at first glance, but... > > At 13:26 -0800 on 09 Jan (1420806400), Ed White wrote: >> @@ -793,6 +793,10 @@ int p2m_change_type_one(struct domain *d, unsigned long gfn, >> >> gfn_unlock(p2m, gfn, 0); >> >> + if ( pt == ot && altp2mhvm_active(d) ) >> + /* make sure this page isn't valid in any alternate p2m */ >> + p2m_remove_altp2m_page(d, gfn); >> + >> return rc; >> } > > ...this is the wrong level to be making this change at. The hook needs > to be right at the bottom, in atomic_write_ept_entry() (and > hap_write_p2m_entry() for AMD, I think), to catch _every_ update of a > p2m entry in the host p2m. > > Otherwise a guest frame could be removed entirely and the altp2m would > still map it. Or am I missing some other path that handles that case? > nested-p2m handles this by failry aggressively flushing nested p2m > tabvles but that doesn't sounds suitable for this since there's state > in the alt-p2m that needs to be retained. Hmm. Is that going to give me even more locking order problems? I don't want to go down the nested p2m route. That is seriously bad for performance, and I've also seen plenty of cases where a flush on one vcpu breaks instruction emulation on another. Also, as you say, I don't have enough information to rebuild the alt p2m. Ed