Re: [RFC PATCH v2 02/11] KVM: guest_memfd: Handle final folio_put() of guest_memfd pages

[Fuad Tabba <tabba@google.com>]

Hi David,

On Thu, 30 Jan 2025 at 17:16, David Hildenbrand <david@redhat.com> wrote:
>
> On 29.01.25 18:23, 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
> > 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@google.com>
> > ---
> >   include/linux/page-flags.h | 7 +++++++
> >   mm/debug.c                 | 1 +
> >   mm/swap.c                  | 5 +++++
> >   3 files changed, 13 insertions(+)
> >
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 6615f2f59144..bab3cac1f93b 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -942,6 +942,7 @@ enum pagetype {
> >       PGTY_slab       = 0xf5,
> >       PGTY_zsmalloc   = 0xf6,
> >       PGTY_unaccepted = 0xf7,
> > +     PGTY_guestmem   = 0xf8,
> >
> >       PGTY_mapcount_underflow = 0xff
> >   };
> > @@ -1091,6 +1092,12 @@ FOLIO_TYPE_OPS(hugetlb, hugetlb)
> >   FOLIO_TEST_FLAG_FALSE(hugetlb)
> >   #endif
> >
>
> Some short doc would be nice, to at least hint that this is related to
> guest_memfd, and that these are otherwise folios.
>
>
> /*
>   * guestmem folios are folios that are used to back VM memory as managed
>   * 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
>   * always setting that type once typed folios can be mapped to user
>   * space cleanly.
>   */

Will add this.

> > +#ifdef CONFIG_KVM_GMEM_MAPPABLE
> > +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 95b6ab809c0e..db93be385ed9 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 8a66cd9cb9da..73d61c7f8edd 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -37,6 +37,7 @@
> >   #include <linux/page_idle.h>
> >   #include <linux/local_lock.h>
> >   #include <linux/buffer_head.h>
> > +#include <linux/kvm_host.h>
> >
> >   #include "internal.h"
> >
> > @@ -101,6 +102,10 @@ static void free_typed_folio(struct folio *folio)
> >               if (IS_ENABLED(CONFIG_HUGETLBFS))
> >                       free_huge_folio(folio);
> >               return;
> > +     case PGTY_guestmem:
> > +             if (IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM))
> > +                     WARN_ONCE(1, "A placeholder that shouldn't trigger.");
>
>
> Does it make sense to directly introduce the callback into guest_memfd
> and handle the WARN_ONCE() in there? Then, we don't have tot ouch this
> core code later again.

Yes. That would simplify things.

Thanks,
/fuad

> --
> Cheers,
>
> David / dhildenb
>