Re: [PATCH v3 07/22] KVM: Annotate -EFAULTs from kvm_vcpu_write_guest_page()

[Peter Xu <peterx@redhat.com>]

On Thu, Apr 20, 2023 at 04:29:38PM -0700, Anish Moorthy wrote:
> On Thu, Apr 20, 2023 at 1:52 PM Peter Xu <peterx@redhat.com> wrote:
> >
> > On Wed, Apr 12, 2023 at 09:34:55PM +0000, Anish Moorthy wrote:
> > > Implement KVM_CAP_MEMORY_FAULT_INFO for efaults from
> > > kvm_vcpu_write_guest_page()
> > >
> > > Signed-off-by: Anish Moorthy <amoorthy@google.com>
> > > ---
> > >  virt/kvm/kvm_main.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > index 63b4285d858d1..b29a38af543f0 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -3119,8 +3119,11 @@ int kvm_vcpu_write_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn,
> > >                             const void *data, int offset, int len)
> > >  {
> > >       struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> > > +     int ret = __kvm_write_guest_page(vcpu->kvm, slot, gfn, data, offset, len);
> > >
> > > -     return __kvm_write_guest_page(vcpu->kvm, slot, gfn, data, offset, len);
> > > +     if (ret == -EFAULT)
> > > +             kvm_populate_efault_info(vcpu, gfn * PAGE_SIZE + offset, len);
> > > +     return ret;
> > >  }
> > >  EXPORT_SYMBOL_GPL(kvm_vcpu_write_guest_page);
> >
> > Why need to trap this?  Is this -EFAULT part of the "scalable userfault"
> > plan or not?
> >
> > My previous memory was one can still leave things like copy_to_user() to go
> > via the userfaults channels which should work in parallel with the new vcpu
> > MEMORY_FAULT exit.  But maybe the plan changed?
> 
> This commit isn't really part of the "scalable uffd" changes, which
> basically correspond to KVM_CAP_ABSENT_MAPPING_FAULT. There should be
> more details in the cover letter, but basically my v1 just included
> KVM_CAP_ABSENT_MAPPING_FAULT: Sean argued that the API there ("return
> to userspace whenever KVM fails a guest memory access to a page
> fault") was problematic, and so I reworked the series to include a
> general capability for reporting extra information for failed guest
> memory accesses (KVM_CAP_MEMORY_FAULT_INFO) and
> KVM_CAP_ABSENT_MAPPING_FAULT (which is meant to be used in combination
> with the other cap) for the "scalable userfaultfd" changes.
> 
> As such most of the commits in this series are unrelated to
> KVM_CAP_ABSENT_MAPPING_FAULT, and this is one of those commits. It
> doesn't affect page faults generated by copy_to_user (which should
> still be delivered via uffd).

Indeed it'll be an improvement itself to report more details for such an
error already.  Makes sense to me, thanks,

-- 
Peter Xu