On 06/15/2018 06:21 PM, Junaid Shahid wrote:
>>>
>>> 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().
>
Though it is going to be problematic even then, because kvm_mmu_new_cr3() itself calls the get_cr3() handler to get the old CR3. If the get_cr3 has already been updated to the nested version, it won't get the correct old CR3 value. I think we should just pass the new EPTP value to kvm_init_shadow_ept_mmu() as a parameter instead of moving the around the function pointer initialization.
Thanks,
Junaid
