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: <cover.1618914692.git.isaku.yamahata@intel.com>
 <YK65V++S2Kt1OLTu@google.com> <936b00e2-1bcc-d5cc-5ae1-59f43ab5325f@redhat.com>
 <20210610220056.GA642297@private.email.ne.jp>
In-Reply-To: <20210610220056.GA642297@private.email.ne.jp>
From:   David Matlack <dmatlack@google.com>
Date:   Thu, 29 Jul 2021 09:48:31 -0700
Message-ID: <CALzav=d2m+HffSLu5e3gz0cYk=MZ2uc1a3K+vP8VRVvLRiwd9g@mail.gmail.com>
Subject: Re: [RFC PATCH 00/10] KVM: x86/mmu: simplify argument to kvm page
 fault handler
To:     Isaku Yamahata <isaku.yamahata@gmail.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Isaku Yamahata <isaku.yamahata@intel.com>,
        kvm list <kvm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jun 10, 2021 at 3:05 PM Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
>
> Thanks for feedback. Let me respin it.

Hi Isaku,

I'm working on a series to plumb the memslot backing the faulting gfn
through the page fault handling stack to avoid redundant lookups. This
would be much cleaner to implement on top of your struct
kvm_page_fault series than the existing code.

Are you still planning to send another version of this series? Or if
you have decided to drop it or go in a different direction?

>
> On Thu, Jun 10, 2021 at 02:45:55PM +0200,
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> > On 26/05/21 23:10, Sean Christopherson wrote:
> > >    - Have kvm_mmu_do_page_fault() handle initialization of the struct.  That
> > >      will allow making most of the fields const, and will avoid the rather painful
> > >      kvm_page_fault_init().
> > >
> > >    - Pass @vcpu separately.  Yes, it's associated with the fault, but literally
> > >      the first line in every consumer is "struct kvm_vcpu *vcpu = kpf->vcpu;".
> > >
> > >    - Use "fault" instead of "kpf", mostly because it reads better for people that
> > >      aren't intimately familiar with the code, but also to avoid having to refactor
> > >      a huge amount of code if we decide to rename kvm_page_fault, e.g. if we decide
> > >      to use that name to return fault information to userspace.
> > >
> > >    - Snapshot anything that is computed in multiple places, even if it is
> > >      derivative of existing info.  E.g. it probably makes sense to grab
> >
> > I agree with all of these (especially it was a bit weird not to see vcpu in
> > the prototypes).  Thanks Sean for the review!
> >
> > Paolo
> >
>
> --
> Isaku Yamahata <isaku.yamahata@gmail.com>