On 1/16/25 10:19, Fuad Tabba wrote: > Hi Ackerley, > > On Thu, 16 Jan 2025 at 00:35, Ackerley Tng <ackerleytng@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx/ >> >> [2] https://lore.kernel.org/all/20241108162040.159038-1-tabba@xxxxxxxxxx/ >> >> >> >> 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 >> >>
