From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tim Deegan <tim@xen.org>
Subject: Re: [PATCH 08/11] nEPT: Use minimal permission for
	nested p2m.
Date: Thu, 13 Dec 2012 16:43:38 +0000
Message-ID: <20121213164338.GN75286@ocelot.phlegethon.org>
References: <1355162243-11857-1-git-send-email-xiantao.zhang@intel.com>
	<1355162243-11857-9-git-send-email-xiantao.zhang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <1355162243-11857-9-git-send-email-xiantao.zhang@intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: xiantao.zhang@intel.com
Cc: keir@xen.org, xen-devel@lists.xensource.com, eddie.dong@intel.com, JBeulich@suse.com, jun.nakajima@intel.com
List-Id: xen-devel@lists.xenproject.org

At 01:57 +0800 on 11 Dec (1355191040), xiantao.zhang@intel.com wrote:
> From: Zhang Xiantao <xiantao.zhang@intel.com>
> 
> Emulate permission check for the nested p2m. Current solution is to
> use minimal permission, and once meet permission violation in L0, then
> determin whether it is caused by guest EPT or host EPT
> 
> Signed-off-by: Zhang Xiantao <xiantao.zhang@intel.com>

> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -1177,7 +1177,7 @@ nsvm_vmcb_hap_enabled(struct vcpu *v)
>   */
>  int
>  nsvm_hap_walk_L1_p2m(struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa,
> -                      unsigned int *page_order,
> +                      unsigned int *page_order, uint8_t *p2m_acc,
>                        bool_t access_r, bool_t access_w, bool_t access_x)

I don't like these interface changes (see below) but if we do have them,
at least make the SVM version use p2m_access_rwx, to match the old
behaviour, rather than letting it use an uninitialised stack variable. :)

> @@ -250,10 +251,13 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
>  
>      page_order_20 = min(page_order_21, page_order_10);
>  
> +    if (p2ma_10 > p2m_access_rwx)
> +        p2ma_10 = p2m_access_rwx;

That's plain wrong.  If the access type is p2m_access_rx2rw, this will
give the guest write access to what ought to be a read-only page. 

I think it would be best to leave the p2m-access stuff to the p2m
walkers, and not add all those extra p2ma arguments.  Instead, just use
the _actual_ access permissions of this fault as the p2ma.   That way
you know you have something that's acceptabel to both p2m tables.

I guess that will mean some extra faults on read-then-write behaviour.
If those are measurable, we could look at pulling the p2m-access types
out like this, but you'll have to explicitly handle all the special
types.

Cheers,

Tim.