On Tue, Jul 30, 2024, Paolo Bonzini wrote: > On 7/27/24 01:51, Sean Christopherson wrote: > > Provide the "struct page" associated with a guest_memfd pfn as an output > > from __kvm_gmem_get_pfn() so that KVM guest page fault handlers can > ^^^^^^^^^^^^^^^^^^^^ > > Just "kvm_gmem_get_pfn()". > > > directly put the page instead of having to rely on > > kvm_pfn_to_refcounted_page(). > > This will conflict with my series, where I'm introducing > folio_file_pfn() and using it here: > > - page = folio_file_page(folio, index); > > + *page = folio_file_page(folio, index); > > - *pfn = page_to_pfn(page); > > + *pfn = page_to_pfn(*page); > > if (max_order) > > *max_order = 0; > > That said, I think it's better to turn kvm_gmem_get_pfn() into > kvm_gmem_get_page() here, and pull the page_to_pfn() or page_to_phys() > to the caller as applicable. This highlights that the caller always > gets a refcounted page with guest_memfd.
I have mixed feelings on this. On one hand, it's silly/confusing to return a pfn+page pair and thus imply that guest_memfd can return a pfn without a page. On the other hand, if guest_memfd does ever serve pfns without a struct page, it could be quite painful to unwind all of the arch arch code we'll accrue that assumes guest_memfd only ever returns a refcounted page (as evidenced by this series). The probability of guest_memfd not having struct page for mapped pfns is likely very low, but at the same time, providing a pfn+page pair doesn't cost us much. And if it turns out that not having struct page is nonsensical, deferring the kvm_gmem_get_pfn() => kvm_gmem_get_page() conversion could be annoying, but highly unlikely to be painful since it should be 100% mechanical. Whereas reverting back to kvm_gmem_get_pfn() if we make the wrong decision now could mean doing surgery on a pile of arch code.
