KVM ARM Archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Stevens <stevensd@chromium.org>
Cc: Yu Zhang <yu.c.zhang@linux.intel.com>,
	Isaku Yamahata <isaku.yamahata@gmail.com>,
	 Zhi Wang <zhi.wang.linux@gmail.com>,
	kvmarm@lists.linux.dev,  linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [PATCH v9 2/6] KVM: mmu: Introduce __kvm_follow_pfn function
Date: Mon, 5 Feb 2024 19:16:33 -0800	[thread overview]
Message-ID: <ZcGkkVAaT_Lz_ZVQ@google.com> (raw)
In-Reply-To: <20230911021637.1941096-3-stevensd@google.com>

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.

  parent reply	other threads:[~2024-02-06  3:16 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-11  2:16 [PATCH v9 0/6] KVM: allow mapping non-refcounted pages David Stevens
2023-09-11  2:16 ` [PATCH v9 1/6] KVM: Assert that a page's refcount is elevated when marking accessed/dirty David Stevens
2023-09-11  2:16 ` [PATCH v9 2/6] KVM: mmu: Introduce __kvm_follow_pfn function David Stevens
2023-10-03 16:54   ` Maxim Levitsky
2024-02-06  1:25     ` Sean Christopherson
2024-02-06  3:16   ` Sean Christopherson [this message]
2024-02-13  3:27   ` Sean Christopherson
2024-02-13  3:44   ` Sean Christopherson
2023-09-11  2:16 ` [PATCH v9 3/6] KVM: mmu: Improve handling of non-refcounted pfns David Stevens
2023-10-03 16:54   ` Maxim Levitsky
2024-02-06  2:54     ` Sean Christopherson
2024-02-13  3:44   ` Sean Christopherson
2023-09-11  2:16 ` [PATCH v9 4/6] KVM: Migrate kvm_vcpu_map to __kvm_follow_pfn David Stevens
2023-10-03 16:54   ` Maxim Levitsky
2023-09-11  2:16 ` [PATCH v9 5/6] KVM: x86: Migrate " David Stevens
2023-10-03 16:54   ` Maxim Levitsky
2023-10-03 20:58     ` Sean Christopherson
2023-09-11  2:16 ` [PATCH v9 6/6] KVM: x86/mmu: Handle non-refcounted pages David Stevens
2023-09-18  9:53   ` Dmitry Osipenko
2023-09-19  2:25     ` David Stevens
2023-09-30 13:34       ` Dmitry Osipenko
2023-09-18  9:58   ` Dmitry Osipenko
2023-09-18 11:19     ` Dmitry Osipenko
2023-09-19  2:59       ` David Stevens
2023-09-21 20:06         ` Dmitry Osipenko
2023-09-30 13:34           ` Dmitry Osipenko
2023-09-19  2:31     ` David Stevens
2023-09-21 20:04       ` Dmitry Osipenko
2024-02-06  3:02       ` Sean Christopherson
2023-10-03 16:54   ` Maxim Levitsky
2024-02-06  3:23   ` Sean Christopherson
2023-09-29  5:19 ` [PATCH v9 0/6] KVM: allow mapping " Christoph Hellwig
2023-09-29 16:06   ` Sean Christopherson
2023-10-02  6:25     ` Christoph Hellwig
2024-02-06  3:29       ` Sean Christopherson
2023-10-31  4:30 ` David Stevens
2023-10-31 14:30   ` Sean Christopherson
2023-12-12  1:59     ` David Stevens
2023-12-20  1:37       ` Sean Christopherson
2024-02-06  3:30         ` Sean Christopherson
2024-02-13  3:39           ` Sean Christopherson
2024-02-21  6:05             ` David Stevens

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZcGkkVAaT_Lz_ZVQ@google.com \
    --to=seanjc@google.com \
    --cc=isaku.yamahata@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stevensd@chromium.org \
    --cc=yu.c.zhang@linux.intel.com \
    --cc=zhi.wang.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).