From mboxrd@z Thu Jan  1 00:00:00 1970
From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Subject: Re: [PATCH v5 07/14] nEPT: Add EPT tables support to paging_tmpl.h
Date: Thu, 01 Aug 2013 15:31:01 +0800
Message-ID: <51FA0EB5.9030802@linux.vnet.ibm.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>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Cc: Gleb Natapov <gleb@redhat.com>, kvm@vger.kernel.org,
	Jun Nakajima <jun.nakajima@intel.com>,
	Yang Zhang <yang.z.zhang@intel.com>, pbonzini@redhat.com
To: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from e23smtp05.au.ibm.com ([202.81.31.147]:53747 "EHLO
	e23smtp05.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753835Ab3HAHbJ (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 1 Aug 2013 03:31:09 -0400
Received: from /spool/local
	by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted
	for <kvm@vger.kernel.org> from <xiaoguangrong@linux.vnet.ibm.com>;
	Thu, 1 Aug 2013 17:24:20 +1000
Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21])
	by d23dlp01.au.ibm.com (Postfix) with ESMTP id 967FC2CE8054
	for <kvm@vger.kernel.org>; Thu,  1 Aug 2013 17:31:05 +1000 (EST)
Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139])
	by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r717UtGX19267646
	for <kvm@vger.kernel.org>; Thu, 1 Aug 2013 17:30:55 +1000
Received: from d23av04.au.ibm.com (loopback [127.0.0.1])
	by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r717V47L015659
	for <kvm@vger.kernel.org>; Thu, 1 Aug 2013 17:31:04 +1000
In-Reply-To: <51FA0BBA.7090206@linux.vnet.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.