On Mon, Nov 14, 2011, Avi Kivity wrote about "Re: [PATCH 02/10] nEPT: MMU context for nested EPT": > > >> +#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; Unless I'm misunderstanding something, translate_nested_gpa, and gva_to_gpa, take as their "access" parameter a bitmask of PFERR_*, so it's fine for PFERR_USER_MASK to be enabled in translate_nested_gpa; It just shouldn't cause PT_USER_MASK to be used. The only additional problem I can find is in walk_addr_generic which does if (!check_write_user_access(vcpu, write_fault, user_fault, pte)) eperm = true; and that checks pte & PT_USER_MASK, which it shouldn't if PTTYPE==PTTYPE_EPT. It's really confusing that we now have in mmu.c no less than 4 (!) access bit schemes, similar in many ways but different in many others: 1. page fault error codes (PFERR_*_MASK) 2. x86 page tables acess bits (PT_*_MASK) 3. KVM private access bits (ACC_*_MASK) 4. EPT access bits (VMX_EPT_*_MASK). I just have to try hard not to confuse them. -- Nadav Har'El | Thursday, Dec 8 2011, nyh@xxxxxxxxxxxxxxxxxxx |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |Sorry, but my karma just ran over your http://nadav.harel.org.il |dogma. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html