On Mon, 17 Feb 2025 at 11:21, Vlastimil Babka <vbabka@xxxxxxx> wrote: > > 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> Thank you! /fuad
