Re: [PATCH 02/10] nEPT: MMU context for nested EPT

[Avi Kivity <avi@redhat.com>]

On 11/13/2011 08:26 PM, Orit Wasserman wrote:
> > 
> >>  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.
> > 
>
> That what I did first but than I was afraid that the MASK will be changed for mmu.c too.
> so I decided on ifdefs.
> The more I think about it I think we need rapper function for mask checking (at least for this file).
> What do you think ?

Either should work, as long as the main logic is clean.

> >>  	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.
>
> Is it because translate_nested_gpa always set USER_MASK ? 

Yes... maybe that function needs to do something like

   access |= mmu->default_access;



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