On 2/17/25 11:12, Fuad Tabba wrote: > Hi Vlastimil, > > On Mon, 17 Feb 2025 at 09:49, Vlastimil Babka <vbabka@xxxxxxx> wrote: >> >> On 2/11/25 13:11, Fuad Tabba wrote: >> > Before transitioning a guest_memfd folio to unshared, thereby >> > disallowing access by the host and allowing the hypervisor to >> > transition its view of the guest page as private, we need to be >> > sure that the host doesn't have any references to the folio. >> > >> > This patch introduces a new type for guest_memfd folios, which >> > isn't activated in this series but is here as a placeholder and >> > to facilitate the code in the next patch. This will be used in >> >> It's not clear to me how the code in the next page is facilitated as it >> doesn't use any of this? > > I'm sorry about that, I'm missing the word "series". i.e., > >> > This patch introduces a new type for guest_memfd folios, which >> > isn't activated in this series but is here as a placeholder and >> > to facilitate the code in the next patch *series*. > > I'm referring to this series: > https://lore.kernel.org/all/20250117163001.2326672-1-tabba@xxxxxxxxxx/ > >> > the future to register a callback that informs the guest_memfd >> > subsystem when the last reference is dropped, therefore knowing >> > that the host doesn't have any remaining references. >> > >> > Signed-off-by: Fuad Tabba <tabba@xxxxxxxxxx> >> > --- >> > include/linux/kvm_host.h | 9 +++++++++ >> > include/linux/page-flags.h | 17 +++++++++++++++++ >> > mm/debug.c | 1 + >> > mm/swap.c | 9 +++++++++ >> > virt/kvm/guest_memfd.c | 7 +++++++ >> > 5 files changed, 43 insertions(+) >> > >> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> > index f34f4cfaa513..8b5f28f6efff 100644 >> > --- a/include/linux/kvm_host.h >> > +++ b/include/linux/kvm_host.h >> > @@ -2571,4 +2571,13 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, >> > struct kvm_pre_fault_memory *range); >> > #endif >> > >> > +#ifdef CONFIG_KVM_GMEM_SHARED_MEM >> > +void kvm_gmem_handle_folio_put(struct folio *folio); >> > +#else >> > +static inline void kvm_gmem_handle_folio_put(struct folio *folio) >> > +{ >> > + WARN_ON_ONCE(1); >> > +} >> >> Since the caller is guarded by CONFIG_KVM_GMEM_SHARED_MEM, do we need the >> CONFIG_KVM_GMEM_SHARED_MEM=n variant at all? > > No. I'll remove it. > >> > +#endif >> > + >> > #endif >> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h >> > index 6dc2494bd002..734afda268ab 100644 >> > --- a/include/linux/page-flags.h >> > +++ b/include/linux/page-flags.h >> > @@ -933,6 +933,17 @@ enum pagetype { >> > PGTY_slab = 0xf5, >> > PGTY_zsmalloc = 0xf6, >> > PGTY_unaccepted = 0xf7, >> > + /* >> > + * guestmem folios are used to back VM memory as managed by guest_memfd. >> > + * Once the last reference is put, instead of freeing these folios back >> > + * to the page allocator, they are returned to guest_memfd. >> > + * >> > + * For now, guestmem will only be set on these folios as long as they >> > + * cannot be mapped to user space ("private state"), with the plan of >> >> Might be a bit misleading as I don't think it's set yet as of this series. >> But I guess we can keep it to avoid another update later. > > You're right, it's not in this series. But as you said, the idea is to > have the least amount of churn in the core mm code. > >> > + * always setting that type once typed folios can be mapped to user >> > + * space cleanly. >> > + */ >> > + PGTY_guestmem = 0xf8, >> > >> > PGTY_mapcount_underflow = 0xff >> > }; >> > @@ -1082,6 +1093,12 @@ FOLIO_TYPE_OPS(hugetlb, hugetlb) >> > FOLIO_TEST_FLAG_FALSE(hugetlb) >> > #endif >> > >> > +#ifdef CONFIG_KVM_GMEM_SHARED_MEM >> > +FOLIO_TYPE_OPS(guestmem, guestmem) >> > +#else >> > +FOLIO_TEST_FLAG_FALSE(guestmem) >> > +#endif >> > + >> > PAGE_TYPE_OPS(Zsmalloc, zsmalloc, zsmalloc) >> > >> > /* >> > diff --git a/mm/debug.c b/mm/debug.c >> > index 8d2acf432385..08bc42c6cba8 100644 >> > --- a/mm/debug.c >> > +++ b/mm/debug.c >> > @@ -56,6 +56,7 @@ static const char *page_type_names[] = { >> > DEF_PAGETYPE_NAME(table), >> > DEF_PAGETYPE_NAME(buddy), >> > DEF_PAGETYPE_NAME(unaccepted), >> > + DEF_PAGETYPE_NAME(guestmem), >> > }; >> > >> > static const char *page_type_name(unsigned int page_type) >> > diff --git a/mm/swap.c b/mm/swap.c >> > index 47bc1bb919cc..241880a46358 100644 >> > --- a/mm/swap.c >> > +++ b/mm/swap.c >> > @@ -38,6 +38,10 @@ >> > #include <linux/local_lock.h> >> > #include <linux/buffer_head.h> >> > >> > +#ifdef CONFIG_KVM_GMEM_SHARED_MEM >> > +#include <linux/kvm_host.h> >> > +#endif >> >> Do we need to guard the include? > > Yes, otherwise allnoconfig complains due to many of the x86 things it > drags along if included but KVM isn't configured. I could put it in a > different header that doesn't have this problem, but I couldn't think > of a better header for this. Ok, you can add Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
