Sean Christopherson <[email protected]> writes: > On Thu, Aug 13, 2026, Ackerley Tng wrote: >> 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? > > No, because that's bleeding x86 details into common code, which defeats the > purpose > of arch hooks.
Ah okay yup maybe it should have been kvm_arch_has_gmem_make_shared() that defaults to false and x86 will return !!kvm_x86_ops.gmem_make_shared. > That's why I think it's worth analyzing the cost: if it's in the > noise, leave it alone. If it's meaningful, figure out a not-too-gross way to > skip > the entire thing if kvm_arch_gmem_make_shared() is a glorified nop in the end. Is noise defined relative to the entire conversion process? Would this benchmark look like 1. Convert 4G to shared on TDX with CONFIG_AMD_SEV defined 2. Convert 4G to shared on TDX without CONFIG_AMD_SEV defined and then compare the difference in time taken?
