Re: [PATCH v10 12/41] KVM: guest_memfd: Call arch make_shared callback for to-shared conversion
Ackerley Tng <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,dev.linux.lists.linux-coco,org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.linux-trace-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <CAEvNRgGpCVCGVjc9jCt9Yr+XypF=71h6eBahkXNkrXhfrtGY+g@mail.gmail.com> |
Sean Christopherson <[email protected]> writes: > On Thu, Aug 13, 2026, Binbin Wu wrote: >> On 8/8/2026 5:52 AM, Ackerley Tng via B4 Relay wrote: >> > +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT >> > +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end) >> > +{ >> > + struct folio_batch fbatch; >> > + pgoff_t next = start; >> > + int i; >> > + >> > + folio_batch_init(&fbatch); >> > + while (filemap_get_folios(inode->i_mapping, &next, end - 1, &fbatch)) { >> > + for (i = 0; i < folio_batch_count(&fbatch); ++i) { >> > + struct folio *folio = fbatch.folios[i]; >> > + pgoff_t start_index, end_index; >> > + kvm_pfn_t start_pfn; >> > + kvm_pfn_t nr_pages; >> > + >> > + start_index = max(start, folio->index); >> > + end_index = min(end, folio_next_index(folio)); >> > + /* >> > + * end_index is either in folio or points to >> > + * the first page of the next folio. Hence, >> > + * all pages in range [start_index, end_index) >> > + * are contiguous. >> > + */ >> > + start_pfn = folio_file_pfn(folio, start_index); >> > + nr_pages = end_index - start_index; >> > + >> > + kvm_arch_gmem_make_shared(start_pfn, nr_pages); >> > + } >> > + >> > + folio_batch_release(&fbatch); >> > + cond_resched(); >> > + } >> > +} >> > +#else >> > +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end) {} >> > +#endif >> > + >> > static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start, >> > size_t nr_pages, uint64_t attrs, >> > pgoff_t *err_index) >> > @@ -599,7 +636,12 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start, >> > >> > filter = to_private ? KVM_FILTER_SHARED : KVM_FILTER_PRIVATE; >> > kvm_gmem_invalidate_start(inode, start, end, filter); >> > + >> > + if (!to_private) Should I make this condition if (!to_private && kvm_x86_ops.gmem_make_shared) instead? Will that help? >> > + kvm_gmem_make_shared(inode, start, end); >> >> If both KVM_AMD_SEV and KVM_INTEL_TDX are enabled, HAVE_KVM_ARCH_GMEM_CONVERT >> will be enabled and the logic in kvm_gmem_make_shared() introduces unnecessary >> overhead for TDX. >> Not sure about CSPs, but in a standard distribution kernel, it's very likely >> that both are enabled, right? > > Yep. > >> Should kvm_gmem_make_shared() do some optimization or the overhead is relative >> small in the conversion to shared path so that the optimization is not worth it? > > I assume it's relatively small, but it's probably a good idea to gets numbers so > that we can make a semi-informed decision. E.g. if processing 1GiB work of 4KiB > folios is <100 cycles, I think we generally don't care. But if it's more like > tens of microseconds, then I think we do care enough to optimize the walking for > TDX.