On 12/20/24 10:35 AM, Simona Vetter wrote: > On Fri, Dec 20, 2024 at 06:04:09PM +0800, Honglei Huang wrote: >> From: Honglei Huang <Honglei1.Huang@xxxxxxx> >> >> A virtio-gpu userptr is based on HMM notifier. >> Used for let host access guest userspace memory and >> notice the change of userspace memory. >> This series patches are in very beginning state, >> User space are pinned currently to ensure the host >> device memory operations are correct. >> The free and unmap operations for userspace can be >> handled by MMU notifier this is a simple and basice >> SVM feature for this series patches. >> The physical PFNS update operations is splited into >> two OPs in here. The evicted memories won't be used >> anymore but remap into host again to achieve same >> effect with hmm_rang_fault. > > So in my opinion there are two ways to implement userptr that make sense: > > - pinned userptr with pin_user_pages(FOLL_LONGTERM). there is not mmu > notifier > > - unpinnned userptr where you entirely rely on userptr and do not hold any > page references or page pins at all, for full SVM integration. This > should use hmm_range_fault ideally, since that's the version that > doesn't ever grab any page reference pins. > > All the in-between variants are imo really bad hacks, whether they hold a > page reference or a temporary page pin (which seems to be what you're > doing here). In much older kernels there was some justification for them, > because strange stuff happened over fork(), but with FOLL_LONGTERM this is > now all sorted out. So there's really only fully pinned, or true svm left > as clean design choices imo. > > With that background, why does pin_user_pages(FOLL_LONGTERM) not work for > you? +1 on using FOLL_LONGTERM. Fully dynamic memory management has a huge cost in complexity that pinning everything avoids. Furthermore, this avoids the host having to take action in response to guest memory reclaim requests. This avoids additional complexity (and thus attack surface) on the host side. Furthermore, since this is for ROCm and not for graphics, I am less concerned about supporting systems that require swappable GPU VRAM. -- Sincerely, Demi Marie Obenour (she/her/hers)
