Re: [PATCH v5 10/17] KVM: Implement KVM_CAP_USERFAULT_ON_MISSING by atomizing __gfn_to_pfn_memslot() calls

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

 



On Wed, Nov 1, 2023 at 3:03 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Nov 01, 2023, Anish Moorthy wrote:
> > On Wed, Oct 4, 2023 at 6:44 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > >
> > > Eh, the shortlog basically says "do work" with a lot of fancy words.  It really
> > > just boils down to:
> > >
> > >   KVM: Let callers of __gfn_to_pfn_memslot() opt-out of USERFAULT_ON_MISSING
> >
> > Proposed commit message for v6:
> >
> > > KVM: Implement KVM_CAP EXIT_ON_MISSING by checking memslot flag in __gfn_to_pfn_memslot()
> > >
> > > When the slot flag is enabled, forbid __gfn_to_pfn_memslot() from
> > > faulting in pages for which mappings are absent. However, some callers of
> > > __gfn_to_pfn_memslot() (such as kvm_vcpu_map()) must be able to opt out
> > > of this behavior: allow doing so via the new can_exit_on_missing
> > > parameter.
> >
> > Although separately, I don't think the parameter should be named
> > can_exit_on_missing (or, as you suggested, can_do_userfault)-
> > __gfn_to_pfn_memslot() shouldn't know or care how its callers are
> > setting up KVM exits, after all.
>
> Why not?  __gfn_to_pfn_memslot() gets passed all kinds of constraints, I don't
> see how "I can't handle exits to userspace" is any different.

Well the thing is that __gfn_to_pfn_memslot() is orthogonal to KVM
exits. Its callers are just using it to try resolving a pfn, and what
they do with the results is up to them.

Put more concretely, __gfn_to_pfn_memslot() has many callers of which
only two (the stage-2 fault handlers) actually use it to set up a KVM
exit- how does a parameter named "can_exit_on_missing" make sense to
its callers in general? If it were __gfn_to_pfn_memslot() itself that
was populating the run struct in response to absent mappings then I
would agree that the name was appropriate- but that's not what's going
on here.

(side note, I'll assume that aside from the current naming discussion
the commit message I proposed is fine)





[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