mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Ackerley Tng <ackerleytng@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	David Hildenbrand <david@kernel.org>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org,
	Stefan Teodorescu <fane@google.com>,
	 Dennis Tighe <dtighe@google.com>,
	Sashiko Bot <sashiko-bot@kernel.org>,
	 Yan Zhao <yan.y.zhao@intel.com>
Subject: Re: [PATCH v5 4/6] KVM: guest_memfd: Split bind() into prepare()+commit() phases
Date: Thu, 24 Sep 2026 12:40:24 -0700	[thread overview]
Message-ID: <arV8qBw-aC5f2nUX@google.com> (raw)
In-Reply-To: <arV6iiz4kB8LrByl@google.com>

On Thu, Sep 24, 2026, Sean Christopherson wrote:
> On Thu, Sep 24, 2026, Sean Christopherson wrote:
> > On Thu, Sep 24, 2026, Ackerley Tng wrote:
> > But after fiddling with this for an hour or so, I realized that if we fully commit
> > to configuring new.gmem in kvm_set_memory_region(), then we can move the prepare()
> > call into kvm_prepare_memory_region() without needing to plumb extra parameters,
> > and make the gmem stuff look a lot more like the rest of the memslot code.  E.g.
> > 
> > 	if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> > #ifdef CONFIG_KVM_GUEST_MEMFD
> > 		new->gmem.file = fget(mem->guest_memfd);
> > 		if (!new->gmem.file) {
> > 			r = -EBADF;
> > 			goto out;
> > 		}
> > 
> > 		new->gmem.pgoff = mem->guest_memfd_offset >> PAGE_SHIFT;
> > #endif
> > 	}
> > 
> > 	r = kvm_set_memslot(kvm, old, new, change);
> > 
> > 	/*
> > 	 * Drop the reference to the gmem file, even on success.  The file pins
> > 	 * KVM, not the other way 'round.  Active bindings are invalidated if
> > 	 * the file is closed before memslots are destroyed.
> > 	 */
> > #ifdef CONFIG_KVM_GUEST_MEMFD
> > 	if (new->gmem.file)
> > 		fput(new->gmem.file);
> > #endif
> > 
> > Not being able to get rid of the #ifdefs is a shame, but from a control flow
> > perspective, this feels more right than anything else.  Full diff (not fully
> > tested, and would need to be split into 3+ patches):
> 
> Actually, plumbing in @old and @change can wait.  As much as I want to make the
> calls match the other prepare()+commit() hooks, @old and @change aren't needed
> until flags-only updates come along, and adding them at that time provide a better
> git history as the additional plumbing will directly precede their usage (or maybe
> even be in the same patch).

Aaaaand talking to myself again.  I take this back.  Plumbing in @change is
desirable, otherwise both kvm_set_memory_region() and kvm_prepare_memory_region()
need to check KVM_MR_CREATE, which is ugly and unnecessarily fragile.

  reply	other threads:[~2026-09-24 19:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22  0:13 [PATCH v5 0/6] KVM: guest_memfd: Fix binding bugs Sean Christopherson
2026-09-22  0:13 ` [PATCH v5 1/6] KVM: guest_memfd: Gracefully handle xarray errors when binding a memslot Sean Christopherson
2026-09-22  0:13 ` [PATCH v5 2/6] KVM: Use goto to handle errors during memslot preparation Sean Christopherson
2026-09-22  0:13 ` [PATCH v5 3/6] KVM: Only bind memslot to guest_memfd instance for CREATE operations Sean Christopherson
2026-09-22  8:15   ` David Hildenbrand (Arm)
2026-09-22  0:13 ` [PATCH v5 4/6] KVM: guest_memfd: Split bind() into prepare()+commit() phases Sean Christopherson
2026-09-22 12:01   ` David Hildenbrand (Arm)
2026-09-24 16:52   ` Ackerley Tng
2026-09-24 19:21     ` Sean Christopherson
2026-09-24 19:31       ` Sean Christopherson
2026-09-24 19:40         ` Sean Christopherson [this message]
2026-09-24 20:18       ` Ackerley Tng
2026-09-22  0:13 ` [PATCH v5 5/6] KVM: guest_memfd: Establish memslot<=>guest_memfd bindings *after* memslot is ready Sean Christopherson
2026-09-24 16:54   ` Ackerley Tng
2026-09-22  0:13 ` [PATCH v5 6/6] KVM: guest_memfd: Drop superfluous WRITE_ONCE() when binding a memslot Sean Christopherson
2026-09-22 11:53   ` Yan Zhao
2026-09-22 13:47     ` Sean Christopherson
2026-09-23  4:51       ` Yan Zhao
2026-09-23 14:26         ` Sean Christopherson
2026-09-24  9:21           ` Yan Zhao
2026-09-24 21:51 ` [PATCH v5 0/6] KVM: guest_memfd: Fix binding bugs Sean Christopherson

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=arV8qBw-aC5f2nUX@google.com \
    --to=seanjc@google.com \
    --cc=ackerleytng@google.com \
    --cc=david@kernel.org \
    --cc=dtighe@google.com \
    --cc=fane@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=sashiko-bot@kernel.org \
    --cc=yan.y.zhao@intel.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

all inboxes | Powered by JetHome®