Re: [RFC PATCH v4 00/14] KVM: Restricted mapping of guest_memfd at the host and arm64 support

Vlastimil Babka
2025-01-20T09:27:00Z

On 1/16/25 10:19, Fuad Tabba wrote:
> Hi Ackerley,
>
> On Thu, 16 Jan 2025 at 00:35, Ackerley Tng <ackerleytng@google.com> wrote:
>>
>> Registration of the folio_put() callback only happens if the VMM
>> actually tries to do vcpu_run(). For 4K folios I think this is okay
>> since the 4K folio can be freed via the transition state K -> state I,
>> but for hugetlb folios that have been split for sharing with userspace,
>> not getting a folio_put() callback means never putting the hugetlb folio
>> together. Hence, relying on vcpu_run() to add the folio_put() callback
>> leaves a way that hugetlb pages can be removed from the system.
>>
>> I think we should try and find a path forward that works for both 4K and
>> hugetlb folios.
>
> I agree, this could be an issue, but we could find other ways to
> trigger the callback for huge folios. The important thing I was trying
> to get to is how to have the callback and be able to register it.
>
>> IIUC page._mapcount and page.page_type works as a union because
>> page_type is only set for page types that are never mapped to userspace,
>> like PGTY_slab, PGTY_offline, etc.
>
> In the last guest_memfd sync, David Hildenbrand mentioned that that
> would be a temporary restriction since the two structures would
> eventually be decoupled, work being done by Matthew Wilcox I believe.

Note the "temporary" might be few years still, it's a long-term project.

>> Technically PGTY_guest_memfd is only set once the page can never be
>> mapped to userspace, but PGTY_guest_memfd can only be set once mapcount
>> reaches 0. Since mapcount is added in the faulting process, could gmem
>> perhaps use some kind of .unmap/.unfault callback, so that gmem gets
>> notified of all unmaps and will know for sure that the mapcount gets to
>> 0?
>
> I'm not sure if there is such a callback. If there were, I'm not sure
> what that would buy us really. The main pain point is the refcount
> going down to zero. The mapcount part is pretty straightforard and
> likely to be only temporary as mentioned, i.e., when it get decoupled,
> we could register the callback earlier and simplify the transition
> altogether.
>
>> Alternatively, I took a look at the folio_is_zone_device()
>> implementation, and page.flags is used to identify the page's type. IIUC
>> a ZONE_DEVICE page also falls in the intersection of needing a
>> folio_put() callback and can be mapped to userspace. Could we use a
>> similar approach, using page.flags to identify a page as a guest_memfd
>> page? That way we don't need to know when unmapping happens, and will
>> always be able to get a folio_put() callback.
>
> Same as above, with this being temporary, adding a new page flag might
> not be something that the rest of the community might be too excited
> about :)

Yeah, adding a page flag is very difficult these days. Also while it's
technically true that being a ZONE_DEVICE page is recorded in the page flags
field, it's not really a separate flag - just that some bits of the flags
field encode the zonenum. But zonenum is a number, not independent flags -
i.e. there can be up to 8 zones, using up to 3 flag bits. And page's zonenum
also has to match the zone's pfn range, so we couldn't just change the zone
of a page to some hypothetical ZONE_MEMFD when it becomes used for memfd.

> Thanks for your comments!
> /fuad
>
>> [1] https://lpc.events/event/18/contributions/1758/attachments/1457/3699/Guestmemfd%20folio%20state%20page_type.pdf
>> [2] https://android-kvm.googlesource.com/linux/+/764360863785ba16d974253a572c87abdd9fdf0b%5E%21/#F0
>>
>> > This patch series doesn't necessarily impose all these transitions,
>> > many of them would be a matter of policy. This just happens to be the
>> > current way I've done it with pKVM/arm64.
>> >
>> > Cheers,
>> > /fuad
>> >
>> > On Fri, 13 Dec 2024 at 16:48, Fuad Tabba <tabba@google.com> wrote:
>> >>
>> >> This series adds restricted mmap() support to guest_memfd, as
>> >> well as support for guest_memfd on arm64. It is based on Linux
>> >> 6.13-rc2. Please refer to v3 for the context [1].
>> >>
>> >> Main changes since v3:
>> >> - Added a new folio type for guestmem, used to register a
>> >> callback when a folio's reference count reaches 0 (Matthew
>> >> Wilcox, DavidH) [2]
>> >> - Introduce new mappability states for folios, where a folio can
>> >> be mappable by the host and the guest, only the guest, or by no
>> >> one (transient state)
>> >> - Rebased on Linux 6.13-rc2
>> >> - Refactoring and tidying up
>> >>
>> >> Cheers,
>> >> /fuad
>> >>
>> >> [1] https://lore.kernel.org/all/20241010085930.1546800-1-tabba@google.com/
>> >> [2] https://lore.kernel.org/all/20241108162040.159038-1-tabba@google.com/
>> >>
>> >> Ackerley Tng (2):
>> >> KVM: guest_memfd: Make guest mem use guest mem inodes instead of
>> >> anonymous inodes
>> >> KVM: guest_memfd: Track mappability within a struct kvm_gmem_private
>> >>
>> >> Fuad Tabba (12):
>> >> mm: Consolidate freeing of typed folios on final folio_put()
>> >> KVM: guest_memfd: Introduce kvm_gmem_get_pfn_locked(), which retains
>> >> the folio lock
>> >> KVM: guest_memfd: Folio mappability states and functions that manage
>> >> their transition
>> >> KVM: guest_memfd: Handle final folio_put() of guestmem pages
>> >> KVM: guest_memfd: Allow host to mmap guest_memfd() pages when shared
>> >> KVM: guest_memfd: Add guest_memfd support to
>> >> kvm_(read|/write)_guest_page()
>> >> KVM: guest_memfd: Add KVM capability to check if guest_memfd is host
>> >> mappable
>> >> KVM: guest_memfd: Add a guest_memfd() flag to initialize it as
>> >> mappable
>> >> KVM: guest_memfd: selftests: guest_memfd mmap() test when mapping is
>> >> allowed
>> >> KVM: arm64: Skip VMA checks for slots without userspace address
>> >> KVM: arm64: Handle guest_memfd()-backed guest page faults
>> >> KVM: arm64: Enable guest_memfd private memory when pKVM is enabled
>> >>
>> >> Documentation/virt/kvm/api.rst | 4 +
>> >> arch/arm64/include/asm/kvm_host.h | 3 +
>> >> arch/arm64/kvm/Kconfig | 1 +
>> >> arch/arm64/kvm/mmu.c | 119 +++-
>> >> include/linux/kvm_host.h | 75 +++
>> >> include/linux/page-flags.h | 22 +
>> >> include/uapi/linux/kvm.h | 2 +
>> >> include/uapi/linux/magic.h | 1 +
>> >> mm/debug.c | 1 +
>> >> mm/swap.c | 28 +-
>> >> tools/testing/selftests/kvm/Makefile | 1 +
>> >> .../testing/selftests/kvm/guest_memfd_test.c | 64 +-
>> >> virt/kvm/Kconfig | 4 +
>> >> virt/kvm/guest_memfd.c | 579 +++++++++++++++++-
>> >> virt/kvm/kvm_main.c | 229 ++++++-
>> >> 15 files changed, 1074 insertions(+), 59 deletions(-)
>> >>
>> >>
>> >> base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
>> >> --
>> >> 2.47.1.613.gc27f4b7a9f-goog
>> >>