Re: [WIP Patch v2 00/14] Avoiding slow get-user-pages via memory fault exit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 17, 2023 at 12:00 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
>
> On Fri, Mar 17, 2023 at 11:54 AM Oliver Upton <oliver.upton@xxxxxxxxx> wrote:
> >
> > David,
> >
> > On Fri, Mar 17, 2023 at 11:46:58AM -0700, David Matlack wrote:
> > > On Fri, Mar 17, 2023 at 11:13 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > >
> > > > On Fri, Mar 17, 2023, Oliver Upton wrote:
> > > > > On Wed, Mar 15, 2023 at 02:17:24AM +0000, Anish Moorthy wrote:
> > > > > > Hi Sean, here's what I'm planing to send up as v2 of the scalable
> > > > > > userfaultfd series.
> > > > >
> > > > > I don't see a ton of value in sending a targeted posting of a series to the
> > > > > list.
> > >
> > > But isn't it already generating value as you were able to weigh in and
> > > provide feedback on technical aspects that you would not have been
> > > otherwise able to if Anish had just messaged Sean?
> >
> > No, I only happened upon this series looking at lore. My problem is that
> > none of the affected maintainers or reviewers were cc'ed on the series.
> >
> > > > > IOW, just CC all of the appropriate reviewers+maintainers. I promise,
> > > > > we won't bite.
> > >
> > > I disagree. While I think it's fine to reach out to someone off-list
> > > to discuss a specific question, if you're going to message all
> > > reviewers and maintainers, you should also CC the mailing list. That
> > > allows more people to follow along and weigh in if necessary.
> >
> > I think there may be a slight disconnect here :) I'm in no way encouraging
> > off-list discussion and instead asking that mail on the list arrives in
> > the right folks' inboxes.
> >
> > Posting an RFC on the list was absolutely the right thing to do.
>
> Doh. I misunderstood what you meant. We are in violent agreement!

Noted. Also, thanks Oliver and Isaku for paying attention to the
series despite it being obscure.

On Fri, Mar 17, 2023 at 11:13 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Fri, Mar 17, 2023, Oliver Upton wrote:
> > > Still unsure if needs conversion
> > > --------------------------------
> > > * __kvm_read_guest_atomic
> > >   The EFAULT might be propagated though FNAME(sync_page)?
> > > * kvm_write_guest_offset_cached (virt/kvm/kvm_main.c:3226)
> > > * __kvm_write_guest_page
> > >   Called from kvm_write_guest_offset_cached: if that needs change, this does too
> >
> > The low-level accessors are common across architectures and can be called from
> > other contexts besides a vCPU. Is it possible for the caller to catch -EFAULT
> > and convert that into an exit?
>
> Ya, as things stand today, the conversions _must_ be performed at the caller, as
> there are (sadly) far too many flows where KVM squashes the error.  E.g. almost
> all of x86's paravirt code just suppresses user memory faults :-(
>
> Anish, when we discussed this off-list, what I meant by limiting the intial support
> to existing -EFAULT cases was limiting support to existing cases where KVM directly
> returns -EFAULT to userspace, not to all existing cases where -EFAULT is ever
> returned _within KVM_ while handling KVM_RUN.  My apologies if I didn't make that clear.

Don't worry, we eventually got there off-list :)

This brings us back to my original set of questions. As has already
been pointed out, I'll have to revisit my "Confident that needs
conversion" changes and tweak them so that the vCPU exit is populated
only for the call sites where the -EFAULT makes it to userspace. I
still want feedback on if I've mis-identified any of the functions in
my "EFAULT does not propagate to userspace" list and whether there are
functions/callers in the "Still unsure if needs conversion" which do
have return paths to KVM_RUN.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux