Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201014182700.2888246-1-bgardon@google.com> <20201014182700.2888246-16-bgardon@google.com>
 <f5e558b2-dab8-ca9e-6870-0c69d683703a@redhat.com>
In-Reply-To: <f5e558b2-dab8-ca9e-6870-0c69d683703a@redhat.com>
From:   Ben Gardon <bgardon@google.com>
Date:   Mon, 19 Oct 2020 10:07:37 -0700
Message-ID: <CANgfPd9UgKK6tr9ArQsDB9ys4Ne=RVkDccY_Za4r4SSHWV46cQ@mail.gmail.com>
Subject: Re: [PATCH v2 15/20] kvm: x86/mmu: Support dirty logging for the TDP MMU
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     LKML <linux-kernel@vger.kernel.org>, kvm <kvm@vger.kernel.org>,
        Cannon Matthews <cannonmatthews@google.com>,
        Peter Xu <peterx@redhat.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Peter Shier <pshier@google.com>,
        Peter Feiner <pfeiner@google.com>,
        Junaid Shahid <junaids@google.com>,
        Jim Mattson <jmattson@google.com>,
        Yulei Zhang <yulei.kernel@gmail.com>,
        Wanpeng Li <kernellwp@gmail.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Oct 16, 2020 at 9:18 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 14/10/20 20:26, Ben Gardon wrote:
> >
> > +     if (kvm->arch.tdp_mmu_enabled)
> > +             kvm_tdp_mmu_clear_dirty_pt_masked(kvm, slot,
> > +                             slot->base_gfn + gfn_offset, mask, true);
>
> This was "false" in v1, I need --verbose for this change. :)

I don't think this changed from v1. Note that there are two callers in
mmu.c - kvm_mmu_write_protect_pt_masked and
kvm_mmu_clear_dirty_pt_masked. One calls with wrprot = true and the
other with wrprot = false.

>
> >       while (mask) {
> >               rmap_head = __gfn_to_rmap(slot->base_gfn + gfn_offset + __ffs(mask),
>
> > +             spte_set = wrprot_gfn_range(kvm, root, slot->base_gfn,
> > +                             slot->base_gfn + slot->npages, min_level) ||
> > +                        spte_set;
>
> A few remaining instances of ||.

Gah, I thought I had gotten all of them. Thanks for catching these.

>
> Paolo
>