> > However, do we actually need to force vendor code to query nowait? At a glance,
> > the only external (relative to kvm_main.c) users of __gfn_to_pfn_memslot() are
> > in flows that play nice with nowait or that don't support it at all. So I *think*
> > we can do this?
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index be06b09e9104..78024318286d 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2403,6 +2403,11 @@ static bool memslot_is_readonly(const struct kvm_memory_slot *slot)
> > return slot->flags & KVM_MEM_READONLY;
> > }
> >
> > +static bool memslot_is_nowait_on_fault(const struct kvm_memory_slot *slot)
> > +{
> > + return slot->flags & KVM_MEM_NOWAIT_ON_FAULT;
> > +}
> > +
> > static unsigned long __gfn_to_hva_many(const struct kvm_memory_slot *slot, gfn_t gfn,
> > gfn_t *nr_pages, bool write)
> > {
> > @@ -2730,6 +2735,11 @@ kvm_pfn_t __gfn_to_pfn_memslot(const struct kvm_memory_slot *slot, gfn_t gfn,
> > writable = NULL;
> > }
> >
> > + if (async && memslot_is_nowait_on_fault(slot)) {
> > + *async = false;
> > + async = NULL;
> > + }
> > +
> > return hva_to_pfn(addr, atomic, interruptible, async, write_fault,
> > writable);
> > }
>
> Hmm, well not having to modify the vendor code would be nice... but
> I'll have to look more at __gfn_to_pfn_memslot()'s callers (and
> probably send more questions your way :). Hopefully it works out more
> like what you suggest.
I took a look of my own, and I don't think moving the nowait query
into __gfn_to_pfn_memslot() would work. At issue is the actual
behavior of KVM_CAP_NOWAIT_ON_FAULT, which I documented as follows:
> The presence of this capability indicates that userspace may pass the
> KVM_MEM_NOWAIT_ON_FAULT flag to KVM_SET_USER_MEMORY_REGION to cause KVM_RUN
> to fail (-EFAULT) in response to page faults for which resolution would require
> the faulting thread to sleep.
Moving the nowait check out of __kvm_faultin_pfn()/user_mem_abort()
and into __gfn_to_pfn_memslot() means that, obviously, other callers
will start to see behavior changes. Some of that is probably actually
necessary for that documentation to be accurate (since any usages of
__gfn_to_pfn_memslot() under KVM_RUN should respect the memslot flag),
but I think there are consumers of __gfn_to_pfn_memslot() from outside
KVM_RUN.
Anyways, after some searching on my end: I think the only caller of
__gfn_to_pfn_memslot() in core kvm/x86/arm64 where moving the "nowait"
check into the function actually changes anything is gfn_to_pfn(). But
that function gets called from vmx_vcpu_create() (through
kvm_alloc_apic_access_page()), and *that* certainly doesn't look like
something KVM_RUN does or would ever call.
