From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [V10 PATCH 3/4] pvh dom0: Add and remove foreign pages Date: Mon, 05 May 2014 08:46:35 +0100 Message-ID: <53675DFB020000780000EDA5@mail.emea.novell.com> References: <1398820008-9005-1-git-send-email-mukesh.rathor@oracle.com> <1398820008-9005-4-git-send-email-mukesh.rathor@oracle.com> <20140501161908.GI86038@deinos.phlegethon.org> <20140501184518.2ebccdb8@mantra.us.oracle.com> <20140502085555.GA64491@deinos.phlegethon.org> <20140502163556.32baa414@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WhDbj-0001um-FP for xen-devel@lists.xenproject.org; Mon, 05 May 2014 07:46:39 +0000 In-Reply-To: <20140502163556.32baa414@mantra.us.oracle.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Mukesh Rathor Cc: George.Dunlap@eu.citrix.com, Tim Deegan , eddie.dong@intel.com, keir.xen@gmail.com, jun.nakajima@intel.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org >>> On 03.05.14 at 01:35, wrote: > On Fri, 2 May 2014 10:55:55 +0200 > Tim Deegan wrote: >> At 18:45 -0700 on 01 May (1398966318), Mukesh Rathor wrote: >> > On Thu, 1 May 2014 18:19:08 +0200 >> > Tim Deegan wrote: >> > > At 18:06 -0700 on 29 Apr (1398791207), Mukesh Rathor wrote: >> > > > diff --git a/xen/arch/x86/mm/p2m-ept.c >> > > > b/xen/arch/x86/mm/p2m-ept.c index c0bfc50..11474e8 100644 >> > > > --- a/xen/arch/x86/mm/p2m-ept.c >> > > > +++ b/xen/arch/x86/mm/p2m-ept.c >> > > > @@ -36,8 +36,6 @@ >> > > > >> > > > #define >> > > > atomic_read_ept_entry(__pepte) \ >> > > > ( (ept_entry_t) { .epte = read_atomic(&(__pepte)->epte) } ) >> > > > -#define atomic_write_ept_entry(__pepte, >> > > > __epte) \ >> > > > - write_atomic(&(__pepte)->epte, (__epte).epte) >> > > > >> > > > #define is_epte_present(ept_entry) ((ept_entry)->epte & >> > > > 0x7) #define is_epte_superpage(ept_entry) ((ept_entry)->sp) >> > > > @@ -46,6 +44,46 @@ static inline bool_t >> > > > is_epte_valid(ept_entry_t *e) return (e->epte != 0 && >> > > > e->sa_p2mt != p2m_invalid); } >> > > > >> > > > +/* returns : 0 for success, -errno otherwise */ >> > > > +static int atomic_write_ept_entry(ept_entry_t *entryptr, >> > > > ept_entry_t new, >> > > > + int level) >> > > > +{ >> > > > + bool_t same_mfn = (new.mfn == entryptr->mfn); >> > > > + unsigned long oldmfn = INVALID_MFN; >> > > > + >> > > > + if ( level ) >> > > > + { >> > > >> > > ASSERT(!(new.sp && p2m_is_foreign(new.sa_p2mt))) here? >> > >> > Yeah, I debated adding p2m_is_foreign ASSERT, but didn't because >> > iirc Jan had said he wanted to use up the non-leaf bits for >> > something else in future. >> >> Well, if new.sp is set it's not non-leaf. Though I suppose maybe the >> test should be for _valid_ + superpage + foreign. > > Ok, so looks like: > > ASSERT(new.sa_p2mt == p2m_invalid || > !(new.sp && p2m_is_foreign(new.sa_p2mt))); > > should do it? Decoding this to ASSERT(new.sa_p2mt == p2m_invalid || !new.sp || !p2m_is_foreign(new.sa_p2mt)); makes already clear that the first check is redundant with the last one. I.e. just ASSERT(!new.sp || !p2m_is_foreign(new.sa_p2mt)); would seem to suffice, but then again wouldn't cover intermediate levels (which right not appears to not be a problem). Jan