Re: [PATCH 08/11] nEPT: Use minimal permission for nested p2m.

[Tim Deegan <tim@xen.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.