On Mon, Oct 17, 2022 at 11:31:19AM +0100, Fuad Tabba wrote: > Hi, > > > > > > > Actually, for pKVM, there is no need for the guest memory to be > > > GUP'able at all if we use the new inaccessible_get_pfn(). > > > > If pKVM can use inaccessible_get_pfn() to get pfn and can avoid GUP (I > > think that is the major concern?), do you see any other gap from > > existing API? > > Actually for this part no, there aren't any gaps and > inaccessible_get_pfn() is sufficient. Thanks for the confirmation. > > > > This of > > > course goes back to what I'd mentioned before in v7; it seems that > > > representing the memslot memory as a file descriptor should be > > > orthogonal to whether the memory is shared or private, rather than a > > > private_fd for private memory and the userspace_addr for shared > > > memory. The host can then map or unmap the shared/private memory using > > > the fd, which allows it more freedom in even choosing to unmap shared > > > memory when not needed, for example. > > > > Using both private_fd and userspace_addr is only needed in TDX and other > > confidential computing scenarios, pKVM may only use private_fd if the fd > > can also be mmaped as a whole to userspace as Sean suggested. > > That does work in practice, for now at least, and is what I do in my > current port. However, the naming and how the API is defined as > implied by the name and the documentation. By calling the field > private_fd, it does imply that it should not be mapped, which is also > what api.rst says in PATCH v8 5/8. My worry is that in that case pKVM > would be mis/ab-using this interface, and that future changes could > cause unforeseen issues for pKVM. That is fairly enough. We can change the naming and the documents. > > Maybe renaming this to something like "guest_fp", and specifying in > the documentation that it can be restricted, e.g., instead of "the > content of the private memory is invisible to userspace" something > along the lines of "the content of the guest memory may be restricted > to userspace". Some other candidates in my mind: - restricted_fd: to pair with the mm side restricted_memfd - protected_fd: as Sean suggested before - fd: how it's explained relies on the memslot.flag. Thanks, Chao > > What do you think? > > Cheers, > /fuad > > > > > Thanks, > > Chao > > > > > > Cheers, > > > /fuad