Re: [PATCH v9 0/6] KVM: allow mapping non-refcounted pages

[Sean Christopherson]

On Mon, Feb 05, 2024, Sean Christopherson wrote:
> On Tue, Dec 19, 2023, Sean Christopherson wrote:
> > On Tue, Dec 12, 2023, David Stevens wrote:
> > > On Tue, Oct 31, 2023 at 11:30 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > >
> > > > On Tue, Oct 31, 2023, David Stevens wrote:
> > > > > Sean, have you been waiting for a new patch series with responses to
> > > > > Maxim's comments? I'm not really familiar with kernel contribution
> > > > > etiquette, but I was hoping to get your feedback before spending the
> > > > > time to put together another patch series.
> > > >
> > > > No, I'm working my way back toward it.  The guest_memfd series took precedence
> > > > over everything that I wasn't confident would land in 6.7, i.e. larger series
> > > > effectively got put on the back burner.  Sorry :-(
> > > 
> > > Is this series something that may be able to make it into 6.8 or 6.9?
> > 
> > 6.8 isn't realistic.  Between LPC, vacation, and non-upstream stuff, I've done
> > frustratingly little code review since early November.  Sorry :-(
> > 
> > I haven't paged this series back into memory, so take this with a grain of salt,
> > but IIRC there was nothing that would block this from landing in 6.9.  Timing will
> > likely be tight though, especially for getting testing on all architectures.
> 
> I did a quick-ish pass today.  If you can hold off on v10 until later this week,
> I'll try to take a more in-depth look by EOD Thursday.

So I took a "deeper" look, but honestly it wasn't really any more in-depth than
the previous look.  I think I was somewhat surprised at the relatively small amount
of churn this ended up requiring.

Anywho, no major complaints.  This might be fodder for 6.9?  Maybe.  It'll be
tight.  At the very least though, I expect to shove v10 in a branch and start
beating on it in anticipation of landing it no later than 6.10.

One question though: what happened to the !FOLL_GET logic in kvm_follow_refcounted_pfn()?

In a previous version, I suggested:

  static kvm_pfn_t kvm_follow_refcounted_pfn(struct kvm_follow_pfn *foll,
                                             struct page *page)
  {
       kvm_pfn_t pfn = page_to_pfn(page);

       foll->is_refcounted_page = true;

       /*
        * FIXME: Ideally, KVM wouldn't pass FOLL_GET to gup() when the caller
        * doesn't want to grab a reference, but gup() doesn't support getting
        * just the pfn, i.e. FOLL_GET is effectively mandatory.  If that ever
        * changes, drop this and simply don't pass FOLL_GET to gup().
        */
       if (!(foll->flags & FOLL_GET))
               put_page(page);

       return pfn;
  }

but in v9 it's simply:

  static kvm_pfn_t kvm_follow_refcounted_pfn(struct kvm_follow_pfn *foll,
					     struct page *page)
  {
	kvm_pfn_t pfn = page_to_pfn(page);

	foll->is_refcounted_page = true;
	return pfn;
  }

And instead the x86 page fault handlers manually drop the reference.  Why is that?