On 11/13/2011 01:30 PM, Orit Wasserman wrote:
> Maybe this patch can help, this is roughly what Avi wants (I hope) done very 
> quickly.
> I'm sorry I don't have setup to run nested VMX at the moment so i can't test 
> it.
>
> Orit
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 9335e1b..bbe212f 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3180,6 +3180,10 @@ static bool sync_mmio_spte(u64 *sptep, gfn_t gfn, 
> unsigned access,
>  #include "paging_tmpl.h"
>  #undef PTTYPE
>  
> +#define PTTYPE EPT
> +#include "paging_tmpl.h"
> +#undef PTTYPE
> +

Yes, that's the key.


>  int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 
> sptes[4]);
>  void kvm_mmu_set_mmio_spte_mask(u64 mmio_mask);
>  int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool 
> direct);
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 507e2b8..70d4cfd 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -39,6 +39,21 @@
>       #define CMPXCHG cmpxchg64
>       #define PT_MAX_FULL_LEVELS 2
>       #endif
> +#elif PTTYPE == EPT
> +     #define pt_element_t u64
> +     #define FNAME(name) EPT_##name
> +     #define PT_BASE_ADDR_MASK PT64_BASE_ADDR_MASK
> +     #define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl)
> +     #define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl)
> +     #define PT_INDEX(addr, level) PT64_INDEX(addr, level)
> +     #define PT_LEVEL_BITS PT64_LEVEL_BITS
> +     #ifdef CONFIG_X86_64
> +     #define PT_MAX_FULL_LEVELS 4
> +     #define CMPXCHG cmpxchg
> +     #else
> +     #define CMPXCHG cmpxchg64
> +     #define PT_MAX_FULL_LEVELS 2
> +     #endif

The various masks should be defined here, to avoid lots of #ifdefs later.

>  #elif PTTYPE == 32
>       #define pt_element_t u32
>       #define guest_walker guest_walker32
> @@ -106,14 +121,19 @@ static unsigned FNAME(gpte_access)(struct kvm_vcpu 
> *vcpu, pt_element_t gpte,
>  {
>       unsigned access;
>  
> +#if PTTYPE == EPT       
>       access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
> +#else
> +     access = (gpte & EPT_WRITABLE_MASK) | EPT_EXEC_MASK;
>       if (last && !is_dirty_gpte(gpte))
>               access &= ~ACC_WRITE_MASK;
> +#endif

Like here, you could make is_dirty_gpte() local to paging_tmpl()
returning true for EPT and the dirty bit otherwise.


>  
>  #if PTTYPE == 64
>       if (vcpu->arch.mmu.nx)
>               access &= ~(gpte >> PT64_NX_SHIFT);

The ept X bit is lost.

Could do something like

   access &= (gpte >> PT_X_NX_SHIFT) ^ PT_X_NX_SENSE;


> +#if PTTYPE == EPT
> +     const int write_fault = access & EPT_WRITABLE_MASK;
> +     const int user_fault  = 0;
> +     const int fetch_fault = 0;
> +#else

EPT has fetch permissions (but not user permissions); anyway
translate_nested_gpa() already does this.

>       const int write_fault = access & PFERR_WRITE_MASK;
>       const int user_fault  = access & PFERR_USER_MASK;
>       const int fetch_fault = access & PFERR_FETCH_MASK;
> +#endif
>       u16 errcode = 0;
>  
>       trace_kvm_mmu_pagetable_walk(addr, write_fault, user_fault,
> @@ -174,6 +200,9 @@ retry_walk:
>              (mmu->get_cr3(vcpu) & CR3_NONPAE_RESERVED_BITS) == 0);
>  
>       pt_access = ACC_ALL;
> +#if PTTYPE == EPT 
> +     pt_access =  PT_PRESENT_MASK | EPT_WRITABLE_MASK | EPT_EXEC_MASK;
> +#endif

pt_access is not in EPT or ia32 format - it's our own format (xwu).  So
this doesn't need changing.  Updating gpte_access() is sufficient.

>  
>       for (;;) {
>               gfn_t real_gfn;
> @@ -186,9 +215,14 @@ retry_walk:
>               pte_gpa   = gfn_to_gpa(table_gfn) + offset;
>               walker->table_gfn[walker->level - 1] = table_gfn;
>               walker->pte_gpa[walker->level - 1] = pte_gpa;
> -
> +#if PTTYPE == EPT 
> +             real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
> +                                           EPT_WRITABLE_MASK);
> +#else
>               real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn),
>                                             PFERR_USER_MASK|PFERR_WRITE_MASK);
> +#endif
> +

Unneeded, I think.

>               if (unlikely(real_gfn == UNMAPPED_GVA))
>                       goto error;
>               real_gfn = gpa_to_gfn(real_gfn);
> @@ -221,6 +255,7 @@ retry_walk:
>                       eperm = true;
>  #endif
>  
> +#if PTTYPE != EPT
>               if (!eperm && unlikely(!(pte & PT_ACCESSED_MASK))) {
>                       int ret;
>                       trace_kvm_mmu_set_accessed_bit(table_gfn, index,
> @@ -235,7 +270,7 @@ retry_walk:
>                       mark_page_dirty(vcpu->kvm, table_gfn);
>                       pte |= PT_ACCESSED_MASK;
>               }
> -
> +#endif

If PT_ACCESSED_MASK is 0 for EPT, this goes away without #ifdef.

> +#if PTTYPE != EPT
>                       /* check if the kernel is fetching from user page */
>                       if (unlikely(pte_access & PT_USER_MASK) &&
>                           kvm_read_cr4_bits(vcpu, X86_CR4_SMEP))
>                               if (fetch_fault && !user_fault)
>                                       eperm = true;
> -
> +#endif

Same here.



-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to