On Fri, Jun 02, 2023, Anish Moorthy wrote:
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 69a221f71914..abbc5dd72292 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2297,4 +2297,10 @@ static inline void kvm_account_pgtable_pages(void *virt, int nr)
> */
> inline void kvm_populate_efault_info(struct kvm_vcpu *vcpu,
> uint64_t gpa, uint64_t len, uint64_t flags);
> +
> +static inline bool kvm_slot_nowait_on_fault(
> + const struct kvm_memory_slot *slot)
Just when I was starting to think that we had beat all of the Google3 out of you :-)
And predicate helpers in KVM typically have "is" or "has" in the name, so that it's
clear the helper queries, versus e.g. sets the flag or something.
> +{
> + return slot->flags & KVM_MEM_NOWAIT_ON_FAULT;
KVM is anything but consistent, but if the helper is likely to be called without
a known good memslot, it probably makes sense to have the helper check for NULL,
e.g.
static inline bool kvm_is_slot_nowait_on_fault(const struct kvm_memory_slot *slot)
{
return slot && slot->flags & KVM_MEM_NOWAIT_ON_FAULT;
}
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);
}
Ah, crud. The above highlights something I missed in v3. The memslot NOWAIT
flag isn't tied to FOLL_NOWAIT, it's really truly a "fast-only" flag. And even
more confusingly, KVM does set FOLL_NOWAIT, but for the async #PF case, which will
get even more confusing if/when KVM uses FOLL_NOWAIT internally.
Drat. I really like the NOWAIT name, but unfortunately it doesn't do what as the
name says.
I still don't love "fast-only" as that bleeds kernel internals to userspace.
Anyone have ideas? Maybe something about not installing new mappings?
