From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v5 07/14] nEPT: Add EPT tables support to paging_tmpl.h
Date: Thu, 1 Aug 2013 13:05:47 +0200
Message-ID: <20130801110547.GA5245@mail.corp.redhat.com>
References: <1375282131-9713-1-git-send-email-gleb@redhat.com>
 <1375282131-9713-8-git-send-email-gleb@redhat.com>
 <51FA077C.4020700@linux.vnet.ibm.com>
 <20130801071023.GL7484@redhat.com>
 <51FA0BBA.7090206@linux.vnet.ibm.com>
 <51FA0EB5.9030802@linux.vnet.ibm.com>
 <20130801074229.GN7484@redhat.com>
 <51FA1387.3010603@linux.vnet.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: Gleb Natapov <gleb@redhat.com>, kvm@vger.kernel.org,
	Jun Nakajima <jun.nakajima@intel.com>,
	Yang Zhang <yang.z.zhang@intel.com>
To: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:7524 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754658Ab3HALF5 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 1 Aug 2013 07:05:57 -0400
Content-Disposition: inline
In-Reply-To: <51FA1387.3010603@linux.vnet.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

 On Aug 01 2013, Xiao Guangrong wrote:
> On 08/01/2013 03:42 PM, Gleb Natapov wrote:
> > On Thu, Aug 01, 2013 at 03:31:01PM +0800, Xiao Guangrong wrote:
> >> On 08/01/2013 03:18 PM, Xiao Guangrong wrote:
> >>  +#endif
> >>>>>
> >>>>> Hmm, why not use shadow_x_mask, shadow_user_mask instead? PT_WRITABLE_MASK
> >>>>> is also suitable for ept, i guess we can remove the "#if/#else/#endif" after
> >>>>> that.
> >>>>>
> >>>> shadow_x_mask and shadow_user_mask do not depend on guest paging mode,
> >>>> so cannot be used here. Since we have to use ifdefs anyway relying on
> >>>> VMX_EPT_WRITABLE_MASK == PT_WRITABLE_MASK is not necessary. Makes code
> >>>> easier to read.
> >>>
> >>> Oh, yes, you are right.
> >>>
> >>> Reviewed-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> >>
> >> BTW, i notice the code in mmu.c uses PT64_NX_MASK to check the permission,
> >> i.e:
> >>
> >> static bool need_remote_flush(u64 old, u64 new)
> >> {
> >> 	if (!is_shadow_present_pte(old))
> >> 		return false;
> >> 	if (!is_shadow_present_pte(new))
> >> 		return true;
> >> 	if ((old ^ new) & PT64_BASE_ADDR_MASK)
> >> 		return true;
> >> 	old ^= PT64_NX_MASK;
> >> 	new ^= PT64_NX_MASK;
> >> 	return (old & ~new & PT64_PERM_MASK) != 0;
> >> }
> >>
> >> It checks shadow page table and the mask is wrong one nest ept spte.
> > So shadow_x_mask need to be used here, correct?
> 
> Yes. The code checks shadow page table which does not depend on guest mode. :)

The XOR should be with shadow_nx_mask, no?  And PT64_PERM_MASK
should include both shadow_x_mask and shadow_nx_mask, I think.

Paolo