Re: [PATCH 02/11] nEPT: Add EPT tables support to paging_tmpl.h

[Paolo Bonzini <pbonzini@redhat.com>]

Il 26/04/2013 08:43, Jun Nakajima ha scritto:
> This is the first patch in a series which adds nested EPT support to KVM's
> nested VMX. Nested EPT means emulating EPT for an L1 guest so that L1 can use
> EPT when running a nested guest L2. When L1 uses EPT, it allows the L2 guest
> to set its own cr3 and take its own page faults without either of L0 or L1
> getting involved. This often significanlty improves L2's performance over the
> previous two alternatives (shadow page tables over EPT, and shadow page
> tables over shadow page tables).
> 
> This patch adds EPT support to paging_tmpl.h.
> 
> paging_tmpl.h contains the code for reading and writing page tables. The code
> for 32-bit and 64-bit tables is very similar, but not identical, so
> paging_tmpl.h is #include'd twice in mmu.c, once with PTTTYPE=32 and once
> with PTTYPE=64, and this generates the two sets of similar functions.
> 
> There are subtle but important differences between the format of EPT tables
> and that of ordinary x86 64-bit page tables, so for nested EPT we need a
> third set of functions to read the guest EPT table and to write the shadow
> EPT table.
> 
> So this patch adds third PTTYPE, PTTYPE_EPT, which creates functions (prefixed
> with "EPT") which correctly read and write EPT tables.
> 
> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
> ---
>  arch/x86/kvm/mmu.c         |  35 ++----------
>  arch/x86/kvm/paging_tmpl.h | 133 ++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 130 insertions(+), 38 deletions(-)

I would split this patch so that first prefetch_invalid_gpte and
gpte_access are moved to paging_tmpl.h (adding the FNAME everywhere).
The second patch then can add the EPT special cases.

> 
> +static inline int FNAME(check_write_user_access)(struct kvm_vcpu *vcpu,
> +					   bool write_fault, bool user_fault,
> +					   unsigned long pte)
> +{
> +#if PTTYPE == PTTYPE_EPT
> +	if (unlikely(write_fault && !(pte & VMX_EPT_WRITABLE_MASK)
> +				 && (user_fault || is_write_protection(vcpu))))
> +		return false;
> +	return true;
> +#else
> +	u32 access = ((kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0)
> +                | (write_fault ? PFERR_WRITE_MASK : 0);
>  
> +	return !permission_fault(vcpu->arch.walk_mmu, vcpu->arch.access, access);
> +#endif
> +}
> +

I think check_write_user_access doesn't exist anymore?  Perhaps a wrong
conflict resolution.

Paolo