On Tue, Sep 04, 2018 at 11:21:09AM -0700, Sean Christopherson wrote:
>On Tue, Sep 04, 2018 at 03:24:13PM +0000, kvm-owner@xxxxxxxxxxxxxxx wrote:
>> On Tue, Sep 04, 2018 at 04:21:40AM -0700, Liran Alon wrote:
>> >
>> >----- richard.weiyang@xxxxxxxxx wrote:
>> >
>> >> Here is the code path which shows kvm_mmu_setup() is invoked after
>> >> kvm_mmu_create(). Since kvm_mmu_setup() is only invoked in this code
>> >> path,
>> >> this means the root_hpa and prev_roots are setup properly. And it is
>> >> not
>> >> necessary to reset it again.
>> >>
>> >> kvm_vm_ioctl_create_vcpu()
>> >> kvm_arch_vcpu_create()
>> >> vmx_create_vcpu()
>> >> kvm_vcpu_init()
>> >> kvm_arch_vcpu_init()
>> >> kvm_mmu_create()
>> >> kvm_arch_vcpu_setup()
>> >> kvm_mmu_setup()
>> >> kvm_init_mmu()
>> >>
>> >> This patch set reset_roots to false in kmv_mmu_setup().
>> >>
>> >> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>> >> ---
>> >> arch/x86/kvm/mmu.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> >> index a282321329b5..28da8a9bd37b 100644
>> >> --- a/arch/x86/kvm/mmu.c
>> >> +++ b/arch/x86/kvm/mmu.c
>> >> @@ -5413,7 +5413,7 @@ void kvm_mmu_setup(struct kvm_vcpu *vcpu)
>> >> {
>> >> MMU_WARN_ON(VALID_PAGE(vcpu->arch.mmu.root_hpa));
>> >>
>> >> - kvm_init_mmu(vcpu, true);
>> >> + kvm_init_mmu(vcpu, false);
>> >> }
>> >>
>> >> static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm,
>> >> --
>> >> 2.15.1
>> >
>> >I would also add a comment in code.
>> >Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
>>
>> Thanks for your comment.
>>
>> Hmm... do you have any suggestion about the comment? Like the changelog?
>
>I don't think a comment is warranted, IMO the MMU_WARN_ON right above
>the call to kvm_init_mmu() is sufficient clarification as to why we
>don't need to reset roots. Though I would reword the changelog to use
>"invalid" instead of "setup properly". To me, the latter implies the
>roots are valid. E.g. "... the root_hpa and prev_roots are guaranteed
>to be invalid."
Ok, that's fine to me.
--
Wei Yang
Help you, Help me
