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
