On 06/15/2018 06:14 PM, Junaid Shahid wrote: > On 06/13/2018 04:39 AM, Paolo Bonzini wrote: >>> + >>> +union kvm_mmu_page_role >>> +kvm_mmu_calc_root_page_role(struct kvm_vcpu *vcpu); >>> + >>> +union kvm_mmu_page_role >>> +kvm_calc_shadow_mmu_root_page_role(struct kvm_vcpu *vcpu); >>> + >>> +union kvm_mmu_page_role >>> +kvm_mmu_calc_shadow_ept_root_page_role(struct kvm_vcpu *vcpu, >>> + bool accessed_dirty); >> Having these public is a little ugly. >> >> Really only kvm_mmu_calc_root_page_role is needed but perhaps it's even >> better if you have something like >> >> void kvm_mmu_new_cr3(struct kvm_vcpu *vcpu, gpa_t cr3) >> { >> __kvm_mmu_new_cr3(vcpu, cr3, kvm_mmu_calc_root_page_role(vcpu)); >> } >> >> Then vmx.c can initialize the vcpu->arch.mmu.* function pointers >> *before* calling kvm_init_shadow_ept_mmu, and kvm_init_shadow_ept_mmu >> can call __kvm_mmu_new_cr3 itself because it can get the eptp. This >> also avoids computing the role twice, once in vmx.c and once in >> kvm_init_shadow_ept_mmu. > > Sounds good. I actually don't think that we even need to move the initialization of the function pointers before the kvm_init_shadow_ept_mmu() call. Just doing the __kvm_mmu_new_cr3() call (along with the role calculation) at the start of kvm_init_shadow_ept_mmu() should be enough. > Sorry, just realized that we do indeed need to move the function pointer initialization earlier. Otherwise, we won't get the correct EPTP value inside kvm_init_shadow_ept_mmu(). Thanks, Junaid