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."
