On Mon, Sep 11, 2023, David Stevens wrote:
> @@ -2681,24 +2668,22 @@ static int hva_to_pfn_remapped(struct vm_area_struct *vma,
> * 2): @write_fault = false && @writable, @writable will tell the caller
> * whether the mapping is writable.
> */
> -kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool interruptible,
> - bool *async, bool write_fault, bool *writable)
> +kvm_pfn_t hva_to_pfn(struct kvm_follow_pfn *foll)
> {
> struct vm_area_struct *vma;
> kvm_pfn_t pfn;
> int npages, r;
>
> /* we can do it either atomically or asynchronously, not both */
Can you change this comment? Not your fault, as it's already wierd, but it ends
being really confusing after the conversion to FOLL_NOWAIT, because not waiting
in atomic context seems completely sane.
> - BUG_ON(atomic && async);
> + BUG_ON(foll->atomic && (foll->flags & FOLL_NOWAIT));
Side topic, a BUG_ON() here is ridiculous overkill. Can you add a patch somewhere
in the series to convert this to a WARN_ON_ONCE()? The check is there purely to
guard against incorrect usage in KVM, the absolutely worst case scenario is that
KVM simply doesn't go down the slow path when it should and effectively DoS's the
guest.
