Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Apr 19, 2023 at 05:49:55PM -0700, Sean Christopherson wrote: > On Wed, Apr 19, 2023, Christian Brauner wrote: > > On Thu, Apr 13, 2023 at 03:28:43PM -0700, Sean Christopherson wrote: > > > > But if you want to preserve the inode number and device number of the > > > > relevant tmpfs instance but still report memfd restricted as your > > > > filesystem type > > > > > > Unless I missed something along the way, reporting memfd_restricted as a > > > distinct > > > filesystem is very much a non-goal. AFAIK it's purely a side effect of > > > the > > > proposed implementation. > > > > In the current implementation you would have to put in effort to fake > > this. For example, you would need to also implement ->statfs > > super_operation where you'd need to fill in the details of the tmpfs > > instance. At that point all that memfd_restricted fs code that you've > > written is nothing but deadweight, I would reckon. > > After digging a bit, I suspect the main reason Kirill implemented an overlay > to > inode_operations was to prevent modifying the file size via ->setattr(). > Relying > on shmem_setattr() to unmap entries in KVM's MMU wouldn't work because, by > design, > the memory can't be mmap()'d into host userspace. > > if (attr->ia_valid & ATTR_SIZE) { > if (memfd->f_inode->i_size) > return -EPERM; > > if (!PAGE_ALIGNED(attr->ia_size)) > return -EINVAL; > } > > But I think we can solve this particular problem by using > F_SEAL_{GROW,SHRINK} or > SHMEM_LONGPIN. For a variety of reasons, I'm leaning more and more toward > making > this a KVM ioctl() instead of a dedicated syscall, at which point we can be > both > more flexible and more draconian, e.g. let userspace provide the file size at > the > time of creation, but make the size immutable, at least by default. > > > > After giving myself a bit of a crash course in file systems, would > > > something like > > > the below have any chance of (a) working, (b) getting merged, and (c) > > > being > > > maintainable? > > > > > > The idea is similar to a stacking filesystem, but instead of stacking, > > > restrictedmem > > > hijacks a f_ops and a_ops to create a lightweight shim around tmpfs. > > > There are > > > undoubtedly issues and edge cases, I'm just looking for a quick "yes, > > > this might > > > be doable" or a "no, that's absolutely bonkers, don't try it". > > > > Maybe, but I think it's weird. > > Yeah, agreed. > > > _Replacing_ f_ops isn't something that's unprecedented. It happens everytime > > a character device is opened (see fs/char_dev.c:chrdev_open()). And debugfs > > does a similar (much more involved) thing where it replaces it's proxy f_ops > > with the relevant subsystem's f_ops. The difference is that in both cases > > the > > replace happens at ->open() time; and the replace is done once. Afterwards > > only the newly added f_ops are relevant. > > > > In your case you'd be keeping two sets of {f,a}_ops; one usable by > > userspace and another only usable by in-kernel consumers. And there are > > some concerns (non-exhaustive list), I think: > > > > * {f,a}_ops weren't designed for this. IOW, one set of {f,a}_ops is > > authoritative per @file and it is left to the individual subsystems to > > maintain driver specific ops (see the sunrpc stuff or sockets). > > * lifetime management for the two sets of {f,a}_ops: If the ops belong > > to a module then you need to make sure that the module can't get > > unloaded while you're using the fops. Might not be a concern in this > > case. > > Ah, whereas I assume the owner of inode_operations is pinned by ??? (dentry?) > holding a reference to the inode? I don't think it would be possible to safely replace inode_operations after the inode's been made visible in caches. It works with file_operations because when a file is opened a new struct file is allocated which isn't reachable anywhere before fd_install() is called. So it is possible to replace f_ops in the default f->f_op->open() method (which is what devices do as the inode is located on e.g., ext4/xfs/tmpfs but the functionality of the device usually provided by some driver/module through its file_operations). The default f_ops are taken from i_fop of the inode. The lifetime of the file_/inode_operations will be aligned with the lifetime of the module they're originating from. If only file_/inode_operations are used from within the same module then there should never be any lifetime concerns. So an inode doesn't explictly pin file_/inode_operations because there's usually no need to do that and it be weird if each new inode would take a reference on the f_ops/i_ops on the off-chance that someone _might_ open the file. Let alone the overhead of calling try_module_get() everytime a new inode is added to the cache. There are various fs objects - the superblock which is pinning the filesystem/module - that exc
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Apr 19, 2023, Christian Brauner wrote: > On Thu, Apr 13, 2023 at 03:28:43PM -0700, Sean Christopherson wrote: > > > But if you want to preserve the inode number and device number of the > > > relevant tmpfs instance but still report memfd restricted as your > > > filesystem type > > > > Unless I missed something along the way, reporting memfd_restricted as a > > distinct > > filesystem is very much a non-goal. AFAIK it's purely a side effect of the > > proposed implementation. > > In the current implementation you would have to put in effort to fake > this. For example, you would need to also implement ->statfs > super_operation where you'd need to fill in the details of the tmpfs > instance. At that point all that memfd_restricted fs code that you've > written is nothing but deadweight, I would reckon. After digging a bit, I suspect the main reason Kirill implemented an overlay to inode_operations was to prevent modifying the file size via ->setattr(). Relying on shmem_setattr() to unmap entries in KVM's MMU wouldn't work because, by design, the memory can't be mmap()'d into host userspace. if (attr->ia_valid & ATTR_SIZE) { if (memfd->f_inode->i_size) return -EPERM; if (!PAGE_ALIGNED(attr->ia_size)) return -EINVAL; } But I think we can solve this particular problem by using F_SEAL_{GROW,SHRINK} or SHMEM_LONGPIN. For a variety of reasons, I'm leaning more and more toward making this a KVM ioctl() instead of a dedicated syscall, at which point we can be both more flexible and more draconian, e.g. let userspace provide the file size at the time of creation, but make the size immutable, at least by default. > > After giving myself a bit of a crash course in file systems, would > > something like > > the below have any chance of (a) working, (b) getting merged, and (c) being > > maintainable? > > > > The idea is similar to a stacking filesystem, but instead of stacking, > > restrictedmem > > hijacks a f_ops and a_ops to create a lightweight shim around tmpfs. There > > are > > undoubtedly issues and edge cases, I'm just looking for a quick "yes, this > > might > > be doable" or a "no, that's absolutely bonkers, don't try it". > > Maybe, but I think it's weird. Yeah, agreed. > _Replacing_ f_ops isn't something that's unprecedented. It happens everytime > a character device is opened (see fs/char_dev.c:chrdev_open()). And debugfs > does a similar (much more involved) thing where it replaces it's proxy f_ops > with the relevant subsystem's f_ops. The difference is that in both cases the > replace happens at ->open() time; and the replace is done once. Afterwards > only the newly added f_ops are relevant. > > In your case you'd be keeping two sets of {f,a}_ops; one usable by > userspace and another only usable by in-kernel consumers. And there are > some concerns (non-exhaustive list), I think: > > * {f,a}_ops weren't designed for this. IOW, one set of {f,a}_ops is > authoritative per @file and it is left to the individual subsystems to > maintain driver specific ops (see the sunrpc stuff or sockets). > * lifetime management for the two sets of {f,a}_ops: If the ops belong > to a module then you need to make sure that the module can't get > unloaded while you're using the fops. Might not be a concern in this > case. Ah, whereas I assume the owner of inode_operations is pinned by ??? (dentry?) holding a reference to the inode? > * brittleness: Not all f_ops for example deal with userspace > functionality some deal with cleanup when the file is closed like > ->release(). So it's delicate to override that functionality with > custom f_ops. Restricted memfds could easily forget to cleanup > resources. > * Potential for confusion why there's two sets of {f,a}_ops. > * f_ops specifically are generic across a vast amount of consumers and > are subject to change. If memfd_restricted() has specific requirements > because of this weird double-use they won't be taken into account. > > I find this hard to navigate tbh and it feels like taking a shortcut to > avoid building a proper api. Agreed. At the very least, it would be better to take an explicit dependency on whatever APIs are being used instead of somewhat blindly bouncing through ->fallocate(). I think that gives us a clearer path to getting something merged too, as we'll need Acks on making specific functions visible, i.e. will give MM maintainers something concrete to react too. > If you only care about a specific set of operations specific to memfd > restricte that needs to be available to in-kernel consumers, I wonder if you > shouldn't just go one step further then your proposal below and build a > dedicated minimal ops api. This is actually very doable for shmem. Unless I'm missing something, because our use case doesn't allow mmap(), swap, or migration, a good chunk of shmem_fallocate() is simply irr
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Apr 13, 2023 at 03:28:43PM -0700, Sean Christopherson wrote: > On Thu, Apr 13, 2023, Christian Brauner wrote: > > On Thu, Aug 18, 2022 at 04:24:21PM +0300, Kirill A . Shutemov wrote: > > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > > Here's what I would prefer, and imagine much easier for you to maintain; > > > > but I'm no system designer, and may be misunderstanding throughout. > > > > > > > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > > > > the fallocate syscall interface itself) to allocate and free the memory, > > > > ioctl for initializing some of it too. KVM in control of whether that > > > > fd can be read or written or mmap'ed or whatever, no need to prevent it > > > > in shmem.c, no need for flags, seals, notifications to and fro because > > > > KVM is already in control and knows the history. If shmem actually has > > > > value, call into it underneath - somewhat like SysV SHM, and /dev/zero > > > > mmap, and i915/gem make use of it underneath. If shmem has nothing to > > > > add, just allocate and free kernel memory directly, recorded in your > > > > own xarray. > > > > > > I guess shim layer on top of shmem *can* work. I don't see immediately why > > > it would not. But I'm not sure it is right direction. We risk creating yet > > > another parallel VM with own rules/locking/accounting that opaque to > > > core-mm. > > > > Sorry for necrobumping this thread but I've been reviewing the > > No worries, I'm just stoked someone who actually knows what they're doing is > chiming in :-) It's a dangerous business, going out of your subsystem. You step into code, and if you don't watch your hands, there is no knowing where you might be swept off to. That saying goes for me here specifically... > > > memfd_restricted() extension that Ackerley is currently working on. I > > was pointed to this thread as this is what the extension is building > > on but I'll reply to both threads here. > > > > From a glance at v10, memfd_restricted() is currently implemented as an > > in-kernel stacking filesystem. A call to memfd_restricted() creates a > > new restricted memfd file and a new unlinked tmpfs file and stashes the > > tmpfs file into the memfd file's private data member. It then uses the > > tmpfs file's f_ops and i_ops to perform the relevant file and inode > > operations. So it has the same callstack as a general stacking > > filesystem like overlayfs in some cases: > > > > memfd_restricted->getattr() > > -> tmpfs->getattr() > > ... > > > Since you're effectively acting like a stacking filesystem you should > > really use the device number of your memfd restricted filesystem. IOW, > > sm like: > > > > stat->dev = memfd_restricted_dentry->d_sb->s_dev; > > > > But then you run into trouble if you want to go forward with Ackerley's > > extension that allows to explicitly pass in tmpfs fds to > > memfd_restricted(). Afaict, two tmpfs instances might allocate the same > > inode number. So now the inode and device number pair isn't unique > > anymore. > > > > So you might best be served by allocating and reporting your own inode > > numbers as well. > > > > But if you want to preserve the inode number and device number of the > > relevant tmpfs instance but still report memfd restricted as your > > filesystem type > > Unless I missed something along the way, reporting memfd_restricted as a > distinct > filesystem is very much a non-goal. AFAIK it's purely a side effect of the > proposed implementation. In the current implementation you would have to put in effort to fake this. For example, you would need to also implement ->statfs super_operation where you'd need to fill in the details of the tmpfs instance. At that point all that memfd_restricted fs code that you've written is nothing but deadweight, I would reckon. > > > then I think it's reasonable to ask whether a stacking implementation really > > makes sense here. > > > > If you extend memfd_restricted() or even consider extending it in the > > future to take tmpfs file descriptors as arguments to identify the tmpfs > > instance in which to allocate the underlying tmpfs file for the new > > restricted memfd file you should really consider a tmpfs based > > implementation. > > > > Because at that point it just feels like a pointless wrapper to get > > custom f_ops and i_ops. Plus it's wasteful because you allocate dentries > > and inodes that you don't really care about at all. > > > > Just off the top of my hat you might be better served: > > * by a new ioctl() on tmpfs instances that > > yield regular tmpfs file descriptors with restricted f_ops and i_ops. > > That's not that different from btrfs subvolumes which effectively are > > directories but are created through an ioctl(). > > I think this is more or less what we want to do, except via a dedicated > syscall > instead of an ioctl() so that the primary interface isn't strictly tie
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Apr 14, 2023, Sean Christopherson wrote: > On Fri, Apr 14, 2023, Ackerley Tng wrote: > > Sean Christopherson writes: > > > if (WARN_ON_ONCE(file->private_data)) { > > > err = -EEXIST; > > > goto err_fd; > > > } > > > > Did you intend this as a check that the backing filesystem isn't using > > the private_data field in the mapping? > > > > I think you meant file->f_mapping->private_data. > > Ya, sounds right. I should have added disclaimers that (a) I wrote this quite > quickly and (b) it's compile tested only at this point. FWIW, here's a very lightly tested version that doesn't explode on a basic selftest. https://github.com/sean-jc/linux/tree/x86/upm_base_support
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Apr 14, 2023, Ackerley Tng wrote: > Sean Christopherson writes: > > > On Thu, Apr 13, 2023, Christian Brauner wrote: > > > * by a mount option to tmpfs that makes it act > > >in this restricted manner then you don't need an ioctl() and can get > > >away with regular open calls. Such a tmpfs instance would only create > > >regular, restricted memfds. > > > I'd prefer to not go this route, becuase IIUC, it would require relatively > > invasive changes to shmem code, and IIUC would require similar changes to > > other support backings in the future, e.g. hugetlbfs? And as above, I > > don't think any of the potential use cases need restrictedmem to be a > > uniquely identifiable mount. > > FWIW, I'm starting to look at extending restrictedmem to hugetlbfs and > the separation that the current implementation has is very helpful. Also > helps that hugetlbfs and tmpfs are structured similarly, I guess. > > > One of the goals (hopefully not a pipe dream) is to design restrictmem in > > such a way that extending it to support other backing types isn't terribly > > difficult. In case it's not obvious, most of us working on this stuff > > aren't filesystems experts, and many of us aren't mm experts either. The > > more we (KVM folks for the most part) can leverage existing code to do the > > heavy lifting, the better. > > > After giving myself a bit of a crash course in file systems, would > > something like the below have any chance of (a) working, (b) getting > > merged, and (c) being maintainable? > > > The idea is similar to a stacking filesystem, but instead of stacking, > > restrictedmem hijacks a f_ops and a_ops to create a lightweight shim around > > tmpfs. There are undoubtedly issues and edge cases, I'm just looking for a > > quick "yes, this might be doable" or a "no, that's absolutely bonkers, > > don't try it". > > Not an FS expert by any means, but I did think of approaching it this > way as well! > > "Hijacking" perhaps gives this approach a bit of a negative connotation. Heh, commandeer then. > I thought this is pretty close to subclassing (as in Object > Oriented Programming). When some methods (e.g. fallocate) are called, > restrictedmem does some work, and calls the same method in the > superclass. > > The existing restrictedmem code is a more like instantiating an shmem > object and keeping that object as a field within the restrictedmem > object. > > Some (maybe small) issues I can think of now: > > (1) > > One difficulty with this approach is that other functions may make > assumptions about private_data being of a certain type, or functions may > use private_data. > > I checked and IIUC neither shmem nor hugetlbfs use the private_data > field in the inode's i_mapping (also file's f_mapping). > > But there's fs/buffer.c which uses private_data, although those > functions seem to be used by FSes like ext4 and fat, not memory-backed > FSes. > > We can probably fix this if any backing filesystems of restrictedmem, > like tmpfs and future ones use private_data. Ya, if we go the route of poking into f_ops and stuff, I would want to add WARN_ON_ONCE() hardening of everything that restrictemem wants to "commandeer" ;-) > > static int restrictedmem_file_create(struct file *file) > > { > > struct address_space *mapping = file->f_mapping; > > struct restrictedmem *rm; > > > rm = kzalloc(sizeof(*rm), GFP_KERNEL); > > if (!rm) > > return -ENOMEM; > > > rm->backing_f_ops = file->f_op; > > rm->backing_a_ops = mapping->a_ops; > > rm->file = file; > > We don't really need to do this, since rm->file is already the same as > file, we could just pass the file itself when it's needed Aha! I was working on getting rid of it, but forgot to go back and do another pass. > > init_rwsem(&rm->lock); > > xa_init(&rm->bindings); > > > file->f_flags |= O_LARGEFILE; > > > file->f_op = &restrictedmem_fops; > > mapping->a_ops = &restrictedmem_aops; > > I think we probably have to override inode_operations as well, because > otherwise other methods would become available to a restrictedmem file > (like link, unlink, mkdir, tmpfile). Or maybe that's a feature instead > of a bug. I think we want those? What we want to restrict are operations that require read/write/execute access to the file, everything else should be ok. fallocate() is a special case because restrictmem needs to tell KVM to unmap the memory when a hole is punched. I assume ->setattr() needs similar treatment to handle ftruncate()? I'd love to hear Christian's input on this aspect of things. > > if (WARN_ON_ONCE(file->private_data)) { > > err = -EEXIST; > > goto err_fd; > > } > > Did you intend this as a check that the backing filesystem isn't using > the private_data field in the mapping? > > I think you meant file->f_mapping->private_data. Ya, sounds right. I should have added disclaimers that (a) I wrote this
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
Sean Christopherson writes: On Thu, Apr 13, 2023, Christian Brauner wrote: On Thu, Aug 18, 2022 at 04:24:21PM +0300, Kirill A . Shutemov wrote: > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > Here's what I would prefer, and imagine much easier for you to maintain; > > but I'm no system designer, and may be misunderstanding throughout. > > > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > > the fallocate syscall interface itself) to allocate and free the memory, > > ioctl for initializing some of it too. KVM in control of whether that > > fd can be read or written or mmap'ed or whatever, no need to prevent it > > in shmem.c, no need for flags, seals, notifications to and fro because > > KVM is already in control and knows the history. If shmem actually has > > value, call into it underneath - somewhat like SysV SHM, and /dev/zero > > mmap, and i915/gem make use of it underneath. If shmem has nothing to > > add, just allocate and free kernel memory directly, recorded in your > > own xarray. > > I guess shim layer on top of shmem *can* work. I don't see immediately why > it would not. But I'm not sure it is right direction. We risk creating yet > another parallel VM with own rules/locking/accounting that opaque to > core-mm. Sorry for necrobumping this thread but I've been reviewing the No worries, I'm just stoked someone who actually knows what they're doing is chiming in :-) +1, thanks Christian! memfd_restricted() extension that Ackerley is currently working on. I was pointed to this thread as this is what the extension is building on but I'll reply to both threads here. From a glance at v10, memfd_restricted() is currently implemented as an in-kernel stacking filesystem. A call to memfd_restricted() creates a new restricted memfd file and a new unlinked tmpfs file and stashes the tmpfs file into the memfd file's private data member. It then uses the tmpfs file's f_ops and i_ops to perform the relevant file and inode operations. So it has the same callstack as a general stacking filesystem like overlayfs in some cases: memfd_restricted->getattr() -> tmpfs->getattr() ... Since you're effectively acting like a stacking filesystem you should really use the device number of your memfd restricted filesystem. IOW, sm like: stat->dev = memfd_restricted_dentry->d_sb->s_dev; But then you run into trouble if you want to go forward with Ackerley's extension that allows to explicitly pass in tmpfs fds to memfd_restricted(). Afaict, two tmpfs instances might allocate the same inode number. So now the inode and device number pair isn't unique anymore. So you might best be served by allocating and reporting your own inode numbers as well. But if you want to preserve the inode number and device number of the relevant tmpfs instance but still report memfd restricted as your filesystem type Unless I missed something along the way, reporting memfd_restricted as a distinct filesystem is very much a non-goal. AFAIK it's purely a side effect of the proposed implementation. then I think it's reasonable to ask whether a stacking implementation really makes sense here. If you extend memfd_restricted() or even consider extending it in the future to take tmpfs file descriptors as arguments to identify the tmpfs instance in which to allocate the underlying tmpfs file for the new restricted memfd file you should really consider a tmpfs based implementation. Because at that point it just feels like a pointless wrapper to get custom f_ops and i_ops. Plus it's wasteful because you allocate dentries and inodes that you don't really care about at all. Just off the top of my hat you might be better served: * by a new ioctl() on tmpfs instances that yield regular tmpfs file descriptors with restricted f_ops and i_ops. That's not that different from btrfs subvolumes which effectively are directories but are created through an ioctl(). I think this is more or less what we want to do, except via a dedicated syscall instead of an ioctl() so that the primary interface isn't strictly tied to tmpfs, e.g. so that it can be extended to other backing types in the future. * by a mount option to tmpfs that makes it act in this restricted manner then you don't need an ioctl() and can get away with regular open calls. Such a tmpfs instance would only create regular, restricted memfds. I'd prefer to not go this route, becuase IIUC, it would require relatively invasive changes to shmem code, and IIUC would require similar changes to other support backings in the future, e.g. hugetlbfs? And as above, I don't think any of the potential use cases need restrictedmem to be a uniquely identifiable mount. FWIW, I'm starting to look at extending restrictedmem to hugetlbfs and the separation that the current implementation has is very helpful. Also helps that hugetl
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Apr 13, 2023, Christian Brauner wrote: > On Thu, Aug 18, 2022 at 04:24:21PM +0300, Kirill A . Shutemov wrote: > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > Here's what I would prefer, and imagine much easier for you to maintain; > > > but I'm no system designer, and may be misunderstanding throughout. > > > > > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > > > the fallocate syscall interface itself) to allocate and free the memory, > > > ioctl for initializing some of it too. KVM in control of whether that > > > fd can be read or written or mmap'ed or whatever, no need to prevent it > > > in shmem.c, no need for flags, seals, notifications to and fro because > > > KVM is already in control and knows the history. If shmem actually has > > > value, call into it underneath - somewhat like SysV SHM, and /dev/zero > > > mmap, and i915/gem make use of it underneath. If shmem has nothing to > > > add, just allocate and free kernel memory directly, recorded in your > > > own xarray. > > > > I guess shim layer on top of shmem *can* work. I don't see immediately why > > it would not. But I'm not sure it is right direction. We risk creating yet > > another parallel VM with own rules/locking/accounting that opaque to > > core-mm. > > Sorry for necrobumping this thread but I've been reviewing the No worries, I'm just stoked someone who actually knows what they're doing is chiming in :-) > memfd_restricted() extension that Ackerley is currently working on. I > was pointed to this thread as this is what the extension is building > on but I'll reply to both threads here. > > From a glance at v10, memfd_restricted() is currently implemented as an > in-kernel stacking filesystem. A call to memfd_restricted() creates a > new restricted memfd file and a new unlinked tmpfs file and stashes the > tmpfs file into the memfd file's private data member. It then uses the > tmpfs file's f_ops and i_ops to perform the relevant file and inode > operations. So it has the same callstack as a general stacking > filesystem like overlayfs in some cases: > > memfd_restricted->getattr() > -> tmpfs->getattr() ... > Since you're effectively acting like a stacking filesystem you should > really use the device number of your memfd restricted filesystem. IOW, > sm like: > > stat->dev = memfd_restricted_dentry->d_sb->s_dev; > > But then you run into trouble if you want to go forward with Ackerley's > extension that allows to explicitly pass in tmpfs fds to > memfd_restricted(). Afaict, two tmpfs instances might allocate the same > inode number. So now the inode and device number pair isn't unique > anymore. > > So you might best be served by allocating and reporting your own inode > numbers as well. > > But if you want to preserve the inode number and device number of the > relevant tmpfs instance but still report memfd restricted as your > filesystem type Unless I missed something along the way, reporting memfd_restricted as a distinct filesystem is very much a non-goal. AFAIK it's purely a side effect of the proposed implementation. > then I think it's reasonable to ask whether a stacking implementation really > makes sense here. > > If you extend memfd_restricted() or even consider extending it in the > future to take tmpfs file descriptors as arguments to identify the tmpfs > instance in which to allocate the underlying tmpfs file for the new > restricted memfd file you should really consider a tmpfs based > implementation. > > Because at that point it just feels like a pointless wrapper to get > custom f_ops and i_ops. Plus it's wasteful because you allocate dentries > and inodes that you don't really care about at all. > > Just off the top of my hat you might be better served: > * by a new ioctl() on tmpfs instances that > yield regular tmpfs file descriptors with restricted f_ops and i_ops. > That's not that different from btrfs subvolumes which effectively are > directories but are created through an ioctl(). I think this is more or less what we want to do, except via a dedicated syscall instead of an ioctl() so that the primary interface isn't strictly tied to tmpfs, e.g. so that it can be extended to other backing types in the future. > * by a mount option to tmpfs that makes it act > in this restricted manner then you don't need an ioctl() and can get > away with regular open calls. Such a tmpfs instance would only create > regular, restricted memfds. I'd prefer to not go this route, becuase IIUC, it would require relatively invasive changes to shmem code, and IIUC would require similar changes to other support backings in the future, e.g. hugetlbfs? And as above, I don't think any of the potential use cases need restrictedmem to be a uniquely identifiable mount. One of the goals (hopefully not a pipe dream) is to design restrictmem in such a way that extending it to support other backing types isn't terribly di
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Aug 18, 2022 at 04:24:21PM +0300, Kirill A . Shutemov wrote: > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > On Wed, 6 Jul 2022, Chao Peng wrote: > > > This is the v7 of this series which tries to implement the fd-based KVM > > > guest private memory. > > > > Here at last are my reluctant thoughts on this patchset. > > > > fd-based approach for supporting KVM guest private memory: fine. > > > > Use or abuse of memfd and shmem.c: mistaken. > > > > memfd_create() was an excellent way to put together the initial prototype. > > > > But since then, TDX in particular has forced an effort into preventing > > (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. > > > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. > > > > What use do you have for a filesystem here? Almost none. > > IIUC, what you want is an fd through which QEMU can allocate kernel > > memory, selectively free that memory, and communicate fd+offset+length > > to KVM. And perhaps an interface to initialize a little of that memory > > from a template (presumably copied from a real file on disk somewhere). > > > > You don't need shmem.c or a filesystem for that! > > > > If your memory could be swapped, that would be enough of a good reason > > to make use of shmem.c: but it cannot be swapped; and although there > > are some references in the mailthreads to it perhaps being swappable > > in future, I get the impression that will not happen soon if ever. > > > > If your memory could be migrated, that would be some reason to use > > filesystem page cache (because page migration happens to understand > > that type of memory): but it cannot be migrated. > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping > theoretically possible, but I'm not aware of any plans as of now. > > [1] > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html > > > Some of these impressions may come from earlier iterations of the > > patchset (v7 looks better in several ways than v5). I am probably > > underestimating the extent to which you have taken on board other > > usages beyond TDX and SEV private memory, and rightly want to serve > > them all with similar interfaces: perhaps there is enough justification > > for shmem there, but I don't see it. There was mention of userfaultfd > > in one link: does that provide the justification for using shmem? > > > > I'm afraid of the special demands you may make of memory allocation > > later on - surprised that huge pages are not mentioned already; > > gigantic contiguous extents? secretmem removed from direct map? > > The design allows for extension to hugetlbfs if needed. Combination of > MFD_INACCESSIBLE | MFD_HUGETLB should route this way. There should be zero > implications for shmem. It is going to be separate struct > memfile_backing_store. > > I'm not sure secretmem is a fit here as we want to extend MFD_INACCESSIBLE > to be movable if platform supports it and secretmem is not migratable by > design (without direct mapping fragmentations). > > > Here's what I would prefer, and imagine much easier for you to maintain; > > but I'm no system designer, and may be misunderstanding throughout. > > > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > > the fallocate syscall interface itself) to allocate and free the memory, > > ioctl for initializing some of it too. KVM in control of whether that > > fd can be read or written or mmap'ed or whatever, no need to prevent it > > in shmem.c, no need for flags, seals, notifications to and fro because > > KVM is already in control and knows the history. If shmem actually has > > value, call into it underneath - somewhat like SysV SHM, and /dev/zero > > mmap, and i915/gem make use of it underneath. If shmem has nothing to > > add, just allocate and free kernel memory directly, recorded in your > > own xarray. > > I guess shim layer on top of shmem *can* work. I don't see immediately why > it would not. But I'm not sure it is right direction. We risk creating yet > another parallel VM with own rules/locking/accounting that opaque to > core-mm. Sorry for necrobumping this thread but I've been reviewing the memfd_restricted() extension that Ackerley is currently working on. I was pointed to this thread as this is what the extension is building on but I'll reply to both threads here. >From a glance at v10, memfd_restricted() is currently implemented as an in-kernel stacking filesystem. A call to memfd_restricted() creates a new restricted memfd file and a new unlinked tmpfs file and stashes the tmpfs file into the memfd file's private data member. It then uses the tmpfs file's f_ops and i_ops to perform the relevant file and inode operations. So it has the same callstack as a general stacking filesystem like overlayfs in
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Sep 13, 2022, Kirill A. Shutemov wrote: > On Tue, Sep 13, 2022 at 02:53:25PM +, Sean Christopherson wrote: > > > > Switching topics, what actually prevents mmapp() on the shim? I tried > > > > to follow, > > > > but I don't know these areas well enough. > > > > > > It has no f_op->mmap, so mmap() will fail with -ENODEV. See do_mmap(). > > > (I did not read the switch statement correctly at first. Note there are > > > two 'fallthrough' there.) > > > > Ah, validate_mmap_request(). Thought not implementing ->mmap() was the > > key, but > > couldn't find the actual check. > > validate_mmap_request() is in mm/nommu.c which is not relevant for real > computers. > > I was talking about this check: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/mmap.c#n1495 Hence the comment about 'fallthrough'. Thanks again!
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Sep 13, 2022 at 02:53:25PM +, Sean Christopherson wrote: > > > Switching topics, what actually prevents mmapp() on the shim? I tried to > > > follow, > > > but I don't know these areas well enough. > > > > It has no f_op->mmap, so mmap() will fail with -ENODEV. See do_mmap(). > > (I did not read the switch statement correctly at first. Note there are > > two 'fallthrough' there.) > > Ah, validate_mmap_request(). Thought not implementing ->mmap() was the key, > but > couldn't find the actual check. validate_mmap_request() is in mm/nommu.c which is not relevant for real computers. I was talking about this check: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/mmap.c#n1495 -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Sep 13, 2022, Kirill A. Shutemov wrote: > On Tue, Sep 13, 2022 at 09:44:27AM +, Sean Christopherson wrote: > > On Thu, Sep 08, 2022, Kirill A. Shutemov wrote: > > > On Wed, Aug 31, 2022 at 05:24:39PM +0300, Kirill A . Shutemov wrote: > > > > On Sat, Aug 20, 2022 at 10:15:32PM -0700, Hugh Dickins wrote: > > > > > > I will try next week to rework it as shim to top of shmem. Does it > > > > > > work > > > > > > for you? > > > > > > > > > > Yes, please do, thanks. It's a compromise between us: the initial TDX > > > > > case has no justification to use shmem at all, but doing it that way > > > > > will help you with some of the infrastructure, and will probably be > > > > > easiest for KVM to extend to other more relaxed fd cases later. > > > > > > > > Okay, below is my take on the shim approach. > > > > > > > > I don't hate how it turned out. It is easier to understand without > > > > callback exchange thing. > > > > > > > > The only caveat is I had to introduce external lock to protect against > > > > race between lookup and truncate. > > > > As before, I think this lock is unnecessary. Or at least it's unnessary to > > hold > > the lock across get/put. The ->invalidate() call will ensure that the pfn > > is > > never actually used if get() races with truncation. > > The updated version you replying to does not use the lock to protect > against truncation anymore. The lock protect notifier list. Gah, grabbed the patch when applying. > > Switching topics, what actually prevents mmapp() on the shim? I tried to > > follow, > > but I don't know these areas well enough. > > It has no f_op->mmap, so mmap() will fail with -ENODEV. See do_mmap(). > (I did not read the switch statement correctly at first. Note there are > two 'fallthrough' there.) Ah, validate_mmap_request(). Thought not implementing ->mmap() was the key, but couldn't find the actual check. Thanks much!
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Sep 13, 2022 at 09:44:27AM +, Sean Christopherson wrote: > On Thu, Sep 08, 2022, Kirill A. Shutemov wrote: > > On Wed, Aug 31, 2022 at 05:24:39PM +0300, Kirill A . Shutemov wrote: > > > On Sat, Aug 20, 2022 at 10:15:32PM -0700, Hugh Dickins wrote: > > > > > I will try next week to rework it as shim to top of shmem. Does it > > > > > work > > > > > for you? > > > > > > > > Yes, please do, thanks. It's a compromise between us: the initial TDX > > > > case has no justification to use shmem at all, but doing it that way > > > > will help you with some of the infrastructure, and will probably be > > > > easiest for KVM to extend to other more relaxed fd cases later. > > > > > > Okay, below is my take on the shim approach. > > > > > > I don't hate how it turned out. It is easier to understand without > > > callback exchange thing. > > > > > > The only caveat is I had to introduce external lock to protect against > > > race between lookup and truncate. > > As before, I think this lock is unnecessary. Or at least it's unnessary to > hold > the lock across get/put. The ->invalidate() call will ensure that the pfn is > never actually used if get() races with truncation. The updated version you replying to does not use the lock to protect against truncation anymore. The lock protect notifier list. > Switching topics, what actually prevents mmapp() on the shim? I tried to > follow, > but I don't know these areas well enough. It has no f_op->mmap, so mmap() will fail with -ENODEV. See do_mmap(). (I did not read the switch statement correctly at first. Note there are two 'fallthrough' there.) -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Sep 08, 2022, Kirill A. Shutemov wrote: > On Wed, Aug 31, 2022 at 05:24:39PM +0300, Kirill A . Shutemov wrote: > > On Sat, Aug 20, 2022 at 10:15:32PM -0700, Hugh Dickins wrote: > > > > I will try next week to rework it as shim to top of shmem. Does it work > > > > for you? > > > > > > Yes, please do, thanks. It's a compromise between us: the initial TDX > > > case has no justification to use shmem at all, but doing it that way > > > will help you with some of the infrastructure, and will probably be > > > easiest for KVM to extend to other more relaxed fd cases later. > > > > Okay, below is my take on the shim approach. > > > > I don't hate how it turned out. It is easier to understand without > > callback exchange thing. > > > > The only caveat is I had to introduce external lock to protect against > > race between lookup and truncate. As before, I think this lock is unnecessary. Or at least it's unnessary to hold the lock across get/put. The ->invalidate() call will ensure that the pfn is never actually used if get() races with truncation. Switching topics, what actually prevents mmapp() on the shim? I tried to follow, but I don't know these areas well enough.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Sep 09, 2022 at 12:11:05PM -0700, Andy Lutomirski wrote: > > > On Fri, Sep 9, 2022, at 7:32 AM, Kirill A . Shutemov wrote: > > On Thu, Sep 08, 2022 at 09:48:35PM -0700, Andy Lutomirski wrote: > >> On 8/19/22 17:27, Kirill A. Shutemov wrote: > >> > On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: > >> > > On Thu, 18 Aug 2022, Kirill A . Shutemov wrote: > >> > > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > >> > > > > > >> > > > > If your memory could be swapped, that would be enough of a good > >> > > > > reason > >> > > > > to make use of shmem.c: but it cannot be swapped; and although > >> > > > > there > >> > > > > are some references in the mailthreads to it perhaps being > >> > > > > swappable > >> > > > > in future, I get the impression that will not happen soon if ever. > >> > > > > > >> > > > > If your memory could be migrated, that would be some reason to use > >> > > > > filesystem page cache (because page migration happens to understand > >> > > > > that type of memory): but it cannot be migrated. > >> > > > > >> > > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And > >> > > > swapping > >> > > > theoretically possible, but I'm not aware of any plans as of now. > >> > > > > >> > > > [1] > >> > > > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html > >> > > > >> > > I always forget, migration means different things to different > >> > > audiences. > >> > > As an mm person, I was meaning page migration, whereas a virtualization > >> > > person thinks VM live migration (which that reference appears to be > >> > > about), > >> > > a scheduler person task migration, an ornithologist bird migration, > >> > > etc. > >> > > > >> > > But you're an mm person too: you may have cited that reference in the > >> > > knowledge that TDX 1.5 Live Migration will entail page migration of the > >> > > kind I'm thinking of. (Anyway, it's not important to clarify that > >> > > here.) > >> > > >> > TDX 1.5 brings both. > >> > > >> > In TDX speak, mm migration called relocation. See TDH.MEM.PAGE.RELOCATE. > >> > > >> > >> This seems to be a pretty bad fit for the way that the core mm migrates > >> pages. The core mm unmaps the page, then moves (in software) the contents > >> to a new address, then faults it in. TDH.MEM.PAGE.RELOCATE doesn't fit > >> into > >> that workflow very well. I'm not saying it can't be done, but it won't > >> just > >> work. > > > > Hm. From what I see we have all necessary infrastructure in place. > > > > Unmaping is NOP for inaccessible pages as it is never mapped and we have > > mapping->a_ops->migrate_folio() callback that allows to replace software > > copying with whatever is needed, like TDH.MEM.PAGE.RELOCATE. > > > > What do I miss? > > Hmm, maybe this isn't as bad as I thought. > > Right now, unless I've missed something, the migration workflow is to > unmap (via try_to_migrate) all mappings, then migrate the backing store > (with ->migrate_folio(), although it seems like most callers expect the > actual copy to happen outside of ->migrate_folio(), Most? I guess you are talking about MIGRATE_SYNC_NO_COPY, right? AFAICS, it is HMM thing and not a common thing. > and then make new > mappings. With the *current* (vma-based, not fd-based) model for KVM > memory, this won't work -- we can't unmap before calling > TDH.MEM.PAGE.RELOCATE. We don't need to unmap. The page is not mapped from core-mm PoV. > But maybe it's actually okay with some care or maybe mild modifications > with the fd-based model. We don't have any mmaps, per se, to unmap for > secret / INACCESSIBLE memory. So maybe we can get all the way to > ->migrate_folio() without zapping anything in the secure EPT and just > call TDH-MEM.PAGE.RELOCATE from inside migrate_folio(). And there will > be nothing to fault back in. From the core code's perspective, it's > like migrating a memfd that doesn't happen to have my mappings at the > time. Modifications needed if we want to initiate migation from userspace. IIRC, we don't have any API that can initiate page migration for file ranges, without mapping the file. But kernel can do it fine for own housekeeping, like compaction doesn't need any VMA. And we need compaction working for long term stability of the system. -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Sep 9, 2022, at 7:32 AM, Kirill A . Shutemov wrote: > On Thu, Sep 08, 2022 at 09:48:35PM -0700, Andy Lutomirski wrote: >> On 8/19/22 17:27, Kirill A. Shutemov wrote: >> > On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: >> > > On Thu, 18 Aug 2022, Kirill A . Shutemov wrote: >> > > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: >> > > > > >> > > > > If your memory could be swapped, that would be enough of a good >> > > > > reason >> > > > > to make use of shmem.c: but it cannot be swapped; and although there >> > > > > are some references in the mailthreads to it perhaps being swappable >> > > > > in future, I get the impression that will not happen soon if ever. >> > > > > >> > > > > If your memory could be migrated, that would be some reason to use >> > > > > filesystem page cache (because page migration happens to understand >> > > > > that type of memory): but it cannot be migrated. >> > > > >> > > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And >> > > > swapping >> > > > theoretically possible, but I'm not aware of any plans as of now. >> > > > >> > > > [1] >> > > > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html >> > > >> > > I always forget, migration means different things to different audiences. >> > > As an mm person, I was meaning page migration, whereas a virtualization >> > > person thinks VM live migration (which that reference appears to be >> > > about), >> > > a scheduler person task migration, an ornithologist bird migration, etc. >> > > >> > > But you're an mm person too: you may have cited that reference in the >> > > knowledge that TDX 1.5 Live Migration will entail page migration of the >> > > kind I'm thinking of. (Anyway, it's not important to clarify that here.) >> > >> > TDX 1.5 brings both. >> > >> > In TDX speak, mm migration called relocation. See TDH.MEM.PAGE.RELOCATE. >> > >> >> This seems to be a pretty bad fit for the way that the core mm migrates >> pages. The core mm unmaps the page, then moves (in software) the contents >> to a new address, then faults it in. TDH.MEM.PAGE.RELOCATE doesn't fit into >> that workflow very well. I'm not saying it can't be done, but it won't just >> work. > > Hm. From what I see we have all necessary infrastructure in place. > > Unmaping is NOP for inaccessible pages as it is never mapped and we have > mapping->a_ops->migrate_folio() callback that allows to replace software > copying with whatever is needed, like TDH.MEM.PAGE.RELOCATE. > > What do I miss? Hmm, maybe this isn't as bad as I thought. Right now, unless I've missed something, the migration workflow is to unmap (via try_to_migrate) all mappings, then migrate the backing store (with ->migrate_folio(), although it seems like most callers expect the actual copy to happen outside of ->migrate_folio(), and then make new mappings. With the *current* (vma-based, not fd-based) model for KVM memory, this won't work -- we can't unmap before calling TDH.MEM.PAGE.RELOCATE. But maybe it's actually okay with some care or maybe mild modifications with the fd-based model. We don't have any mmaps, per se, to unmap for secret / INACCESSIBLE memory. So maybe we can get all the way to ->migrate_folio() without zapping anything in the secure EPT and just call TDH-MEM.PAGE.RELOCATE from inside migrate_folio(). And there will be nothing to fault back in. From the core code's perspective, it's like migrating a memfd that doesn't happen to have my mappings at the time. --Andy
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Jul 06, 2022 at 04:20:02PM +0800, Chao Peng wrote: > This is the v7 of this series which tries to implement the fd-based KVM > guest private memory. The patches are based on latest kvm/queue branch > commit: > > b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > split_desc_cache only by default capacity > > Introduction > > In general this patch series introduce fd-based memslot which provides > guest memory through memory file descriptor fd[offset,size] instead of > hva/size. The fd can be created from a supported memory filesystem > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > and the the memory backing store exchange callbacks when such memslot > gets created. At runtime KVM will call into callbacks provided by the > backing store to get the pfn with the fd+offset. Memory backing store > will also call into KVM callbacks when userspace punch hole on the fd > to notify KVM to unmap secondary MMU page table entries. > > Comparing to existing hva-based memslot, this new type of memslot allows > guest memory unmapped from host userspace like QEMU and even the kernel > itself, therefore reduce attack surface and prevent bugs. > > Based on this fd-based memslot, we can build guest private memory that > is going to be used in confidential computing environments such as Intel > TDX and AMD SEV. When supported, the memory backing store can provide > more enforcement on the fd and KVM can use a single memslot to hold both > the private and shared part of the guest memory. Hi everyone, Just wanted to let you all know that I reserved a slot at the LPC Confidential Computing Microconference to discuss some topics related to unmapped/inaccessible private memory support: "Unmapped Private Memory for Confidential Guests" Tuesday, Sep 13th, 10:00am (Dublin time) https://lpc.events/event/16/sessions/133/#20220913 The discussion agenda is still a bit in flux, but one topic I really wanted to cover is how we intend to deal with the kernel directmap for TDX/SNP, where there is a need to either remove or split mappings so that KVM or other kernel threads writing to non-private pages don't run into issues due mappings overlapping with private pages.[1] Other possible discussion topics: - guarding against shared->private conversions while KVM is attempting to access a shared page (separate PFN pools for shared/private seems to resolve this nicely, but may not be compatible with things like pKVM where the underlying PFN is the same for shared/private)[2] - extending KVM_EXIT_MEMORY_FAULT to handle batched requests to better handle things like explicit batched conversions initiated by the guest It's a short session so not sure how much time we'll actually have to discuss things in detail, but maybe this can at least be a good jumping off point for other discussions. Thanks, and hope to see you there! [1] https://lore.kernel.org/all/ywb8wg6ravbs1...@google.com/ [2] https://lore.kernel.org/lkml/CA+EHjTy6NF=bkcqk0vhxldtkzmahp55jumsfxn96-nt3yim...@mail.gmail.com/
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Sep 08, 2022 at 09:48:35PM -0700, Andy Lutomirski wrote: > On 8/19/22 17:27, Kirill A. Shutemov wrote: > > On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: > > > On Thu, 18 Aug 2022, Kirill A . Shutemov wrote: > > > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > > > > > > > > If your memory could be swapped, that would be enough of a good reason > > > > > to make use of shmem.c: but it cannot be swapped; and although there > > > > > are some references in the mailthreads to it perhaps being swappable > > > > > in future, I get the impression that will not happen soon if ever. > > > > > > > > > > If your memory could be migrated, that would be some reason to use > > > > > filesystem page cache (because page migration happens to understand > > > > > that type of memory): but it cannot be migrated. > > > > > > > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And > > > > swapping > > > > theoretically possible, but I'm not aware of any plans as of now. > > > > > > > > [1] > > > > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html > > > > > > I always forget, migration means different things to different audiences. > > > As an mm person, I was meaning page migration, whereas a virtualization > > > person thinks VM live migration (which that reference appears to be > > > about), > > > a scheduler person task migration, an ornithologist bird migration, etc. > > > > > > But you're an mm person too: you may have cited that reference in the > > > knowledge that TDX 1.5 Live Migration will entail page migration of the > > > kind I'm thinking of. (Anyway, it's not important to clarify that here.) > > > > TDX 1.5 brings both. > > > > In TDX speak, mm migration called relocation. See TDH.MEM.PAGE.RELOCATE. > > > > This seems to be a pretty bad fit for the way that the core mm migrates > pages. The core mm unmaps the page, then moves (in software) the contents > to a new address, then faults it in. TDH.MEM.PAGE.RELOCATE doesn't fit into > that workflow very well. I'm not saying it can't be done, but it won't just > work. Hm. From what I see we have all necessary infrastructure in place. Unmaping is NOP for inaccessible pages as it is never mapped and we have mapping->a_ops->migrate_folio() callback that allows to replace software copying with whatever is needed, like TDH.MEM.PAGE.RELOCATE. What do I miss? -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 8/24/22 02:41, Chao Peng wrote: On Tue, Aug 23, 2022 at 04:05:27PM +, Sean Christopherson wrote: On Tue, Aug 23, 2022, David Hildenbrand wrote: On 19.08.22 05:38, Hugh Dickins wrote: On Fri, 19 Aug 2022, Sean Christopherson wrote: On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: On Wed, 6 Jul 2022, Chao Peng wrote: But since then, TDX in particular has forced an effort into preventing (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. Are any of the shmem.c mods useful to existing users of shmem.c? No. Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. But QEMU and other VMMs are users of shmem and memfd. The new features certainly aren't useful for _all_ existing users, but I don't think it's fair to say that they're not useful for _any_ existing users. Okay, I stand corrected: there exist some users of memfd_create() who will also have use for "INACCESSIBLE" memory. As raised in reply to the relevant patch, I'm not sure if we really have to/want to expose MFD_INACCESSIBLE to user space. I feel like this is a requirement of specific memfd_notifer (memfile_notifier) implementations -- such as TDX that will convert the memory and MCE-kill the machine on ordinary write access. We might be able to set/enforce this when registering a notifier internally instead, and fail notifier registration if a condition isn't met (e.g., existing mmap). So I'd be curious, which other users of shmem/memfd would benefit from (MMU)-"INACCESSIBLE" memory obtained via memfd_create()? I agree that there's no need to expose the inaccessible behavior via uAPI. Making it a kernel-internal thing that's negotiated/resolved when KVM binds to the fd would align INACCESSIBLE with the UNMOVABLE and UNRECLAIMABLE flags (and any other flags that get added in the future). AFAICT, the user-visible flag is a holdover from the early RFCs and doesn't provide any unique functionality. That's also what I'm thinking. And I don't see problem immediately if user has populated the fd at the binding time. Actually that looks an advantage for previously discussed guest payload pre-loading. I think this gets awkward. Trying to define sensible semantics for what happens if a shmem or similar fd gets used as secret guest memory and that fd isn't utterly and completely empty can get quite nasty. For example: If there are already mmaps, then TDX (much more so than SEV) really doesn't want to also use it as guest memory. If there is already data in the fd, then maybe some technologies can use this for pre-population, but TDX needs explicit instructions in order to get the guest's hash right. In general, it seems like it will be much more likely to actually work well if the user (uAPI) is required to declare to the kernel exactly what the fd is for (e.g. TDX secret memory, software-only secret memory, etc) before doing anything at all with it other than binding it to KVM. INACCESSIBLE is a way to achieve this. Maybe it's not the prettiest in the world -- I personally would rather see an explicit request for, say, TDX or SEV memory or maybe the memory that works for a particular KVM instance instead of something generic like INACCESSIBLE, but this is a pretty weak preference. But I think that just starting with a plain memfd is a can of worms.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 8/19/22 17:27, Kirill A. Shutemov wrote: On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: On Thu, 18 Aug 2022, Kirill A . Shutemov wrote: On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: If your memory could be swapped, that would be enough of a good reason to make use of shmem.c: but it cannot be swapped; and although there are some references in the mailthreads to it perhaps being swappable in future, I get the impression that will not happen soon if ever. If your memory could be migrated, that would be some reason to use filesystem page cache (because page migration happens to understand that type of memory): but it cannot be migrated. Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping theoretically possible, but I'm not aware of any plans as of now. [1] https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html I always forget, migration means different things to different audiences. As an mm person, I was meaning page migration, whereas a virtualization person thinks VM live migration (which that reference appears to be about), a scheduler person task migration, an ornithologist bird migration, etc. But you're an mm person too: you may have cited that reference in the knowledge that TDX 1.5 Live Migration will entail page migration of the kind I'm thinking of. (Anyway, it's not important to clarify that here.) TDX 1.5 brings both. In TDX speak, mm migration called relocation. See TDH.MEM.PAGE.RELOCATE. This seems to be a pretty bad fit for the way that the core mm migrates pages. The core mm unmaps the page, then moves (in software) the contents to a new address, then faults it in. TDH.MEM.PAGE.RELOCATE doesn't fit into that workflow very well. I'm not saying it can't be done, but it won't just work.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 8/18/22 06:24, Kirill A . Shutemov wrote: On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: On Wed, 6 Jul 2022, Chao Peng wrote: This is the v7 of this series which tries to implement the fd-based KVM guest private memory. Here at last are my reluctant thoughts on this patchset. fd-based approach for supporting KVM guest private memory: fine. Use or abuse of memfd and shmem.c: mistaken. memfd_create() was an excellent way to put together the initial prototype. But since then, TDX in particular has forced an effort into preventing (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. Are any of the shmem.c mods useful to existing users of shmem.c? No. Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. What use do you have for a filesystem here? Almost none. IIUC, what you want is an fd through which QEMU can allocate kernel memory, selectively free that memory, and communicate fd+offset+length to KVM. And perhaps an interface to initialize a little of that memory from a template (presumably copied from a real file on disk somewhere). You don't need shmem.c or a filesystem for that! If your memory could be swapped, that would be enough of a good reason to make use of shmem.c: but it cannot be swapped; and although there are some references in the mailthreads to it perhaps being swappable in future, I get the impression that will not happen soon if ever. If your memory could be migrated, that would be some reason to use filesystem page cache (because page migration happens to understand that type of memory): but it cannot be migrated. Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping theoretically possible, but I'm not aware of any plans as of now. [1] https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html This thing? https://cdrdv2.intel.com/v1/dl/getContent/733578 That looks like migration between computers, not between NUMA nodes. Or am I missing something?
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Aug 31, 2022 at 05:24:39PM +0300, Kirill A . Shutemov wrote: > On Sat, Aug 20, 2022 at 10:15:32PM -0700, Hugh Dickins wrote: > > > I will try next week to rework it as shim to top of shmem. Does it work > > > for you? > > > > Yes, please do, thanks. It's a compromise between us: the initial TDX > > case has no justification to use shmem at all, but doing it that way > > will help you with some of the infrastructure, and will probably be > > easiest for KVM to extend to other more relaxed fd cases later. > > Okay, below is my take on the shim approach. > > I don't hate how it turned out. It is easier to understand without > callback exchange thing. > > The only caveat is I had to introduce external lock to protect against > race between lookup and truncate. Otherwise, looks pretty reasonable to me. > > I did very limited testing. And it lacks integration with KVM, but API > changed not substantially, any it should be easy to adopt. > > Any comments? Updated version below. Nothing major. Some simplification and cleanups. diff --git a/include/linux/memfd.h b/include/linux/memfd.h index 4f1600413f91..334ddff08377 100644 --- a/include/linux/memfd.h +++ b/include/linux/memfd.h @@ -3,6 +3,7 @@ #define __LINUX_MEMFD_H #include +#include #ifdef CONFIG_MEMFD_CREATE extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg); @@ -13,4 +14,27 @@ static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned long a) } #endif +struct inaccessible_notifier; + +struct inaccessible_notifier_ops { + void (*invalidate)(struct inaccessible_notifier *notifier, + pgoff_t start, pgoff_t end); +}; + +struct inaccessible_notifier { + struct list_head list; + const struct inaccessible_notifier_ops *ops; +}; + +void inaccessible_register_notifier(struct file *file, + struct inaccessible_notifier *notifier); +void inaccessible_unregister_notifier(struct file *file, + struct inaccessible_notifier *notifier); + +int inaccessible_get_pfn(struct file *file, pgoff_t offset, pfn_t *pfn, +int *order); +void inaccessible_put_pfn(struct file *file, pfn_t pfn); + +struct file *memfd_mkinaccessible(struct file *memfd); + #endif /* __LINUX_MEMFD_H */ diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h index 6325d1d0e90f..9d066be3d7e8 100644 --- a/include/uapi/linux/magic.h +++ b/include/uapi/linux/magic.h @@ -101,5 +101,6 @@ #define DMA_BUF_MAGIC 0x444d4142 /* "DMAB" */ #define DEVMEM_MAGIC 0x454d444d /* "DMEM" */ #define SECRETMEM_MAGIC0x5345434d /* "SECM" */ +#define INACCESSIBLE_MAGIC 0x494e4143 /* "INAC" */ #endif /* __LINUX_MAGIC_H__ */ diff --git a/include/uapi/linux/memfd.h b/include/uapi/linux/memfd.h index 7a8a26751c23..48750474b904 100644 --- a/include/uapi/linux/memfd.h +++ b/include/uapi/linux/memfd.h @@ -8,6 +8,7 @@ #define MFD_CLOEXEC0x0001U #define MFD_ALLOW_SEALING 0x0002U #define MFD_HUGETLB0x0004U +#define MFD_INACCESSIBLE 0x0008U /* * Huge page size encoding when MFD_HUGETLB is specified, and a huge page diff --git a/mm/Makefile b/mm/Makefile index 9a564f836403..f82e5d4b4388 100644 --- a/mm/Makefile +++ b/mm/Makefile @@ -126,7 +126,7 @@ obj-$(CONFIG_HARDENED_USERCOPY) += usercopy.o obj-$(CONFIG_PERCPU_STATS) += percpu-stats.o obj-$(CONFIG_ZONE_DEVICE) += memremap.o obj-$(CONFIG_HMM_MIRROR) += hmm.o -obj-$(CONFIG_MEMFD_CREATE) += memfd.o +obj-$(CONFIG_MEMFD_CREATE) += memfd.o memfd_inaccessible.o obj-$(CONFIG_MAPPING_DIRTY_HELPERS) += mapping_dirty_helpers.o obj-$(CONFIG_PTDUMP_CORE) += ptdump.o obj-$(CONFIG_PAGE_REPORTING) += page_reporting.o diff --git a/mm/memfd.c b/mm/memfd.c index 08f5f8304746..1853a90f49ff 100644 --- a/mm/memfd.c +++ b/mm/memfd.c @@ -261,7 +261,8 @@ long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg) #define MFD_NAME_PREFIX_LEN (sizeof(MFD_NAME_PREFIX) - 1) #define MFD_NAME_MAX_LEN (NAME_MAX - MFD_NAME_PREFIX_LEN) -#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB) +#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB | \ + MFD_INACCESSIBLE) SYSCALL_DEFINE2(memfd_create, const char __user *, uname, @@ -283,6 +284,14 @@ SYSCALL_DEFINE2(memfd_create, return -EINVAL; } + /* Disallow sealing when MFD_INACCESSIBLE is set. */ + if ((flags & MFD_INACCESSIBLE) && (flags & MFD_ALLOW_SEALING)) + return -EINVAL; + + /* TODO: add hugetlb support */ + if ((flags & MFD_INACCESSIBLE) && (flags & MFD_HUGETLB)) + return -EINVAL; + /* length includes terminating zero */ len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1); if (len <= 0) @@ -331,10 +340,24 @@ SYSCALL_DEFINE2(memfd_create,
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Sep 02, 2022 at 06:27:57PM +0800, Chao Peng wrote: > > + if (flags & MFD_INACCESSIBLE) { > > + struct file *inaccessible_file; > > + > > + inaccessible_file = memfd_mkinaccessible(file); > > + if (IS_ERR(inaccessible_file)) { > > + error = PTR_ERR(inaccessible_file); > > + goto err_file; > > + } > > The new file should alse be marked as O_LARGEFILE otherwise setting the > initial size greater than 2^31 on the fd will be refused by ftruncate(). > > + inaccessible_file->f_flags |= O_LARGEFILE; > + Good catch. Thanks. I will modify memfd_mkinaccessible() to do this. -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Aug 31, 2022 at 05:24:39PM +0300, Kirill A . Shutemov wrote: > On Sat, Aug 20, 2022 at 10:15:32PM -0700, Hugh Dickins wrote: > > > I will try next week to rework it as shim to top of shmem. Does it work > > > for you? > > > > Yes, please do, thanks. It's a compromise between us: the initial TDX > > case has no justification to use shmem at all, but doing it that way > > will help you with some of the infrastructure, and will probably be > > easiest for KVM to extend to other more relaxed fd cases later. > > Okay, below is my take on the shim approach. > > I don't hate how it turned out. It is easier to understand without > callback exchange thing. > > The only caveat is I had to introduce external lock to protect against > race between lookup and truncate. Otherwise, looks pretty reasonable to me. > > I did very limited testing. And it lacks integration with KVM, but API > changed not substantially, any it should be easy to adopt. I have integrated this patch with other KVM patches and verified the functionality works well in TDX environment with a minor fix below. > > Any comments? > ... > diff --git a/mm/memfd.c b/mm/memfd.c > index 08f5f8304746..1853a90f49ff 100644 > --- a/mm/memfd.c > +++ b/mm/memfd.c > @@ -261,7 +261,8 @@ long memfd_fcntl(struct file *file, unsigned int cmd, > unsigned long arg) > #define MFD_NAME_PREFIX_LEN (sizeof(MFD_NAME_PREFIX) - 1) > #define MFD_NAME_MAX_LEN (NAME_MAX - MFD_NAME_PREFIX_LEN) > > -#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB) > +#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB | \ > +MFD_INACCESSIBLE) > > SYSCALL_DEFINE2(memfd_create, > const char __user *, uname, > @@ -283,6 +284,14 @@ SYSCALL_DEFINE2(memfd_create, > return -EINVAL; > } > > + /* Disallow sealing when MFD_INACCESSIBLE is set. */ > + if ((flags & MFD_INACCESSIBLE) && (flags & MFD_ALLOW_SEALING)) > + return -EINVAL; > + > + /* TODO: add hugetlb support */ > + if ((flags & MFD_INACCESSIBLE) && (flags & MFD_HUGETLB)) > + return -EINVAL; > + > /* length includes terminating zero */ > len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1); > if (len <= 0) > @@ -331,10 +340,24 @@ SYSCALL_DEFINE2(memfd_create, > *file_seals &= ~F_SEAL_SEAL; > } > > + if (flags & MFD_INACCESSIBLE) { > + struct file *inaccessible_file; > + > + inaccessible_file = memfd_mkinaccessible(file); > + if (IS_ERR(inaccessible_file)) { > + error = PTR_ERR(inaccessible_file); > + goto err_file; > + } The new file should alse be marked as O_LARGEFILE otherwise setting the initial size greater than 2^31 on the fd will be refused by ftruncate(). + inaccessible_file->f_flags |= O_LARGEFILE; + > + > + file = inaccessible_file; > + } > + > fd_install(fd, file); > kfree(name); > return fd; > > +err_file: > + fput(file); > err_fd: > put_unused_fd(fd); > err_name:
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Aug 31, 2022 at 10:12:12AM +0100, Fuad Tabba wrote: > > > Moreover, something which was discussed here before [3], is the > > > ability to share in-place. For pKVM/arm64, the conversion between > > > shared and private involves only changes to the stage-2 page tables, > > > which are controlled by the hypervisor. Android supports this in-place > > > conversion already, and I think that the cost of copying for many > > > use-cases that would involve large amounts of data would be big. We > > > will measure the relative costs in due course, but in the meantime > > > we’re nervous about adopting a new user ABI which doesn’t appear to > > > cater for in-place conversion; having just the fd would simplify that > > > somewhat > > > > I understand there is difficulty to achieve that with the current > > private_fd + userspace_addr (they basically in two separate fds), but is > > it possible for pKVM to extend this? Brainstorming for example, pKVM can > > ignore userspace_addr and only use private_fd to cover both shared and > > private memory, or pKVM introduce new KVM memslot flag? > > It's not that there's anything blocking pKVM from doing that. It's > that the disconnect of using a memory address for the shared memory, > and a file descriptor for the private memory doesn't really make sense > for pKVM. I see how it makes sense for TDX and the Intel-specific > implementation. It just seems that this is baking in an > implementation-specific aspect as a part of the KVM general api, and > the worry is that this might have some unintended consequences in the > future. It's true this API originates from supporting TDX and probably other similar confidential computing(CC) technologies. But if we ever get chance to make it more common to cover more usages like pKVM, I would also like to. The challenge on this point is pKVM diverges a lot from CC usages, putting both shared and private memory in the same fd complicates CC usages. If two things are different enough, I'm also thinking implementation-specific may not be that bad. Chao
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Sat, Aug 20, 2022 at 10:15:32PM -0700, Hugh Dickins wrote: > > I will try next week to rework it as shim to top of shmem. Does it work > > for you? > > Yes, please do, thanks. It's a compromise between us: the initial TDX > case has no justification to use shmem at all, but doing it that way > will help you with some of the infrastructure, and will probably be > easiest for KVM to extend to other more relaxed fd cases later. Okay, below is my take on the shim approach. I don't hate how it turned out. It is easier to understand without callback exchange thing. The only caveat is I had to introduce external lock to protect against race between lookup and truncate. Otherwise, looks pretty reasonable to me. I did very limited testing. And it lacks integration with KVM, but API changed not substantially, any it should be easy to adopt. Any comments? diff --git a/include/linux/memfd.h b/include/linux/memfd.h index 4f1600413f91..aec04a0f8b7b 100644 --- a/include/linux/memfd.h +++ b/include/linux/memfd.h @@ -3,6 +3,7 @@ #define __LINUX_MEMFD_H #include +#include #ifdef CONFIG_MEMFD_CREATE extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg); @@ -13,4 +14,27 @@ static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned long a) } #endif +struct inaccessible_notifier; + +struct inaccessible_notifier_ops { + void (*invalidate)(struct inaccessible_notifier *notifier, + pgoff_t start, pgoff_t end); +}; + +struct inaccessible_notifier { + struct list_head list; + const struct inaccessible_notifier_ops *ops; +}; + +int inaccessible_register_notifier(struct file *file, + struct inaccessible_notifier *notifier); +void inaccessible_unregister_notifier(struct file *file, + struct inaccessible_notifier *notifier); + +int inaccessible_get_pfn(struct file *file, pgoff_t offset, pfn_t *pfn, +int *order); +void inaccessible_put_pfn(struct file *file, pfn_t pfn); + +struct file *memfd_mkinaccessible(struct file *memfd); + #endif /* __LINUX_MEMFD_H */ diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h index 6325d1d0e90f..9d066be3d7e8 100644 --- a/include/uapi/linux/magic.h +++ b/include/uapi/linux/magic.h @@ -101,5 +101,6 @@ #define DMA_BUF_MAGIC 0x444d4142 /* "DMAB" */ #define DEVMEM_MAGIC 0x454d444d /* "DMEM" */ #define SECRETMEM_MAGIC0x5345434d /* "SECM" */ +#define INACCESSIBLE_MAGIC 0x494e4143 /* "INAC" */ #endif /* __LINUX_MAGIC_H__ */ diff --git a/include/uapi/linux/memfd.h b/include/uapi/linux/memfd.h index 7a8a26751c23..48750474b904 100644 --- a/include/uapi/linux/memfd.h +++ b/include/uapi/linux/memfd.h @@ -8,6 +8,7 @@ #define MFD_CLOEXEC0x0001U #define MFD_ALLOW_SEALING 0x0002U #define MFD_HUGETLB0x0004U +#define MFD_INACCESSIBLE 0x0008U /* * Huge page size encoding when MFD_HUGETLB is specified, and a huge page diff --git a/mm/Makefile b/mm/Makefile index 9a564f836403..f82e5d4b4388 100644 --- a/mm/Makefile +++ b/mm/Makefile @@ -126,7 +126,7 @@ obj-$(CONFIG_HARDENED_USERCOPY) += usercopy.o obj-$(CONFIG_PERCPU_STATS) += percpu-stats.o obj-$(CONFIG_ZONE_DEVICE) += memremap.o obj-$(CONFIG_HMM_MIRROR) += hmm.o -obj-$(CONFIG_MEMFD_CREATE) += memfd.o +obj-$(CONFIG_MEMFD_CREATE) += memfd.o memfd_inaccessible.o obj-$(CONFIG_MAPPING_DIRTY_HELPERS) += mapping_dirty_helpers.o obj-$(CONFIG_PTDUMP_CORE) += ptdump.o obj-$(CONFIG_PAGE_REPORTING) += page_reporting.o diff --git a/mm/memfd.c b/mm/memfd.c index 08f5f8304746..1853a90f49ff 100644 --- a/mm/memfd.c +++ b/mm/memfd.c @@ -261,7 +261,8 @@ long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg) #define MFD_NAME_PREFIX_LEN (sizeof(MFD_NAME_PREFIX) - 1) #define MFD_NAME_MAX_LEN (NAME_MAX - MFD_NAME_PREFIX_LEN) -#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB) +#define MFD_ALL_FLAGS (MFD_CLOEXEC | MFD_ALLOW_SEALING | MFD_HUGETLB | \ + MFD_INACCESSIBLE) SYSCALL_DEFINE2(memfd_create, const char __user *, uname, @@ -283,6 +284,14 @@ SYSCALL_DEFINE2(memfd_create, return -EINVAL; } + /* Disallow sealing when MFD_INACCESSIBLE is set. */ + if ((flags & MFD_INACCESSIBLE) && (flags & MFD_ALLOW_SEALING)) + return -EINVAL; + + /* TODO: add hugetlb support */ + if ((flags & MFD_INACCESSIBLE) && (flags & MFD_HUGETLB)) + return -EINVAL; + /* length includes terminating zero */ len = strnlen_user(uname, MFD_NAME_MAX_LEN + 1); if (len <= 0) @@ -331,10 +340,24 @@ SYSCALL_DEFINE2(memfd_create, *file_seals &= ~F_SEAL_SEAL; } + if (flags & MFD_INACCESSIBLE) { + struct file *inaccessible_file; + + inaccessible_file = memfd_mkina
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
Hi Chao, Thank you for your reply. On Mon, Aug 29, 2022 at 4:23 PM Chao Peng wrote: > > On Fri, Aug 26, 2022 at 04:19:25PM +0100, Fuad Tabba wrote: > > Hi, > > > > On Wed, Jul 6, 2022 at 9:24 AM Chao Peng > > wrote: > > > > > > This is the v7 of this series which tries to implement the fd-based KVM > > > guest private memory. The patches are based on latest kvm/queue branch > > > commit: > > > > > > b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > > > split_desc_cache only by default capacity > > > > > > Introduction > > > > > > In general this patch series introduce fd-based memslot which provides > > > guest memory through memory file descriptor fd[offset,size] instead of > > > hva/size. The fd can be created from a supported memory filesystem > > > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > > > and the the memory backing store exchange callbacks when such memslot > > > gets created. At runtime KVM will call into callbacks provided by the > > > backing store to get the pfn with the fd+offset. Memory backing store > > > will also call into KVM callbacks when userspace punch hole on the fd > > > to notify KVM to unmap secondary MMU page table entries. > > > > > > Comparing to existing hva-based memslot, this new type of memslot allows > > > guest memory unmapped from host userspace like QEMU and even the kernel > > > itself, therefore reduce attack surface and prevent bugs. > > > > > > Based on this fd-based memslot, we can build guest private memory that > > > is going to be used in confidential computing environments such as Intel > > > TDX and AMD SEV. When supported, the memory backing store can provide > > > more enforcement on the fd and KVM can use a single memslot to hold both > > > the private and shared part of the guest memory. > > > > > > mm extension > > > - > > > Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file > > > created with these flags cannot read(), write() or mmap() etc via normal > > > MMU operations. The file content can only be used with the newly > > > introduced memfile_notifier extension. > > > > > > The memfile_notifier extension provides two sets of callbacks for KVM to > > > interact with the memory backing store: > > > - memfile_notifier_ops: callbacks for memory backing store to notify > > > KVM when memory gets invalidated. > > > - backing store callbacks: callbacks for KVM to call into memory > > > backing store to request memory pages for guest private memory. > > > > > > The memfile_notifier extension also provides APIs for memory backing > > > store to register/unregister itself and to trigger the notifier when the > > > bookmarked memory gets invalidated. > > > > > > The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to > > > prevent double allocation caused by unintentional guest when we only > > > have a single side of the shared/private memfds effective. > > > > > > memslot extension > > > - > > > Add the private fd and the fd offset to existing 'shared' memslot so > > > that both private/shared guest memory can live in one single memslot. > > > A page in the memslot is either private or shared. Whether a guest page > > > is private or shared is maintained through reusing existing SEV ioctls > > > KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. > > > > > > > I'm on the Android pKVM team at Google, and we've been looking into > > how this approach fits with what we've been doing with pkvm/arm64. > > I've had a go at porting your patches, along with some fixes and > > additions so it would go on top of our latest pkvm patch series [1] to > > see how well this proposal fits with what we’re doing. You can find > > the ported code at this link [2]. > > > > In general, an fd-based approach fits very well with pKVM for the > > reasons you mention. It means that we don't necessarily need to map > > the guest memory, and with the new extensions it allows the host > > kernel to control whether to restrict migration and swapping. > > Good to hear that. > > > > > For pKVM, we would also need the guest private memory not to be > > GUP’able by the kernel so that userspace can’t trick the kernel into > > accessing guest private memory in a context where it isn’t prepared to > > handle the fault injected by the hypervisor. We’re looking at whether > > we could use memfd_secret to achieve this, or maybe whether extending > > your work might solve the problem. > > This is interesting and can be a valuable addition to this series. I'll keep you posted as it goes. I think with the work that you've already put in, it wouldn't require that much more. > > > > However, during the porting effort, the main issue we've encountered > > is that many of the details of this approach seem to be targeted at > > TDX/SEV and don’t readily align with the design of pKVM. My knowledge > > on TDX is very rudimentary, so please bear with me if I get things > > wrong. > > No do
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Aug 26, 2022 at 04:19:25PM +0100, Fuad Tabba wrote: > Hi, > > On Wed, Jul 6, 2022 at 9:24 AM Chao Peng wrote: > > > > This is the v7 of this series which tries to implement the fd-based KVM > > guest private memory. The patches are based on latest kvm/queue branch > > commit: > > > > b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > > split_desc_cache only by default capacity > > > > Introduction > > > > In general this patch series introduce fd-based memslot which provides > > guest memory through memory file descriptor fd[offset,size] instead of > > hva/size. The fd can be created from a supported memory filesystem > > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > > and the the memory backing store exchange callbacks when such memslot > > gets created. At runtime KVM will call into callbacks provided by the > > backing store to get the pfn with the fd+offset. Memory backing store > > will also call into KVM callbacks when userspace punch hole on the fd > > to notify KVM to unmap secondary MMU page table entries. > > > > Comparing to existing hva-based memslot, this new type of memslot allows > > guest memory unmapped from host userspace like QEMU and even the kernel > > itself, therefore reduce attack surface and prevent bugs. > > > > Based on this fd-based memslot, we can build guest private memory that > > is going to be used in confidential computing environments such as Intel > > TDX and AMD SEV. When supported, the memory backing store can provide > > more enforcement on the fd and KVM can use a single memslot to hold both > > the private and shared part of the guest memory. > > > > mm extension > > - > > Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file > > created with these flags cannot read(), write() or mmap() etc via normal > > MMU operations. The file content can only be used with the newly > > introduced memfile_notifier extension. > > > > The memfile_notifier extension provides two sets of callbacks for KVM to > > interact with the memory backing store: > > - memfile_notifier_ops: callbacks for memory backing store to notify > > KVM when memory gets invalidated. > > - backing store callbacks: callbacks for KVM to call into memory > > backing store to request memory pages for guest private memory. > > > > The memfile_notifier extension also provides APIs for memory backing > > store to register/unregister itself and to trigger the notifier when the > > bookmarked memory gets invalidated. > > > > The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to > > prevent double allocation caused by unintentional guest when we only > > have a single side of the shared/private memfds effective. > > > > memslot extension > > - > > Add the private fd and the fd offset to existing 'shared' memslot so > > that both private/shared guest memory can live in one single memslot. > > A page in the memslot is either private or shared. Whether a guest page > > is private or shared is maintained through reusing existing SEV ioctls > > KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. > > > > I'm on the Android pKVM team at Google, and we've been looking into > how this approach fits with what we've been doing with pkvm/arm64. > I've had a go at porting your patches, along with some fixes and > additions so it would go on top of our latest pkvm patch series [1] to > see how well this proposal fits with what we’re doing. You can find > the ported code at this link [2]. > > In general, an fd-based approach fits very well with pKVM for the > reasons you mention. It means that we don't necessarily need to map > the guest memory, and with the new extensions it allows the host > kernel to control whether to restrict migration and swapping. Good to hear that. > > For pKVM, we would also need the guest private memory not to be > GUP’able by the kernel so that userspace can’t trick the kernel into > accessing guest private memory in a context where it isn’t prepared to > handle the fault injected by the hypervisor. We’re looking at whether > we could use memfd_secret to achieve this, or maybe whether extending > your work might solve the problem. This is interesting and can be a valuable addition to this series. > > However, during the porting effort, the main issue we've encountered > is that many of the details of this approach seem to be targeted at > TDX/SEV and don’t readily align with the design of pKVM. My knowledge > on TDX is very rudimentary, so please bear with me if I get things > wrong. No doubt this series is initially designed for confidential computing usages, but pKVM can definitely extend it if it finds useful. > > The idea of the memslot having two references to the backing memory, > the (new) private_fd (a file descriptor) as well as the userspace_addr > (a memory address), with the meaning changing depending on whether the > memory is private or shared. Both can potential
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
Hi, On Wed, Jul 6, 2022 at 9:24 AM Chao Peng wrote: > > This is the v7 of this series which tries to implement the fd-based KVM > guest private memory. The patches are based on latest kvm/queue branch > commit: > > b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > split_desc_cache only by default capacity > > Introduction > > In general this patch series introduce fd-based memslot which provides > guest memory through memory file descriptor fd[offset,size] instead of > hva/size. The fd can be created from a supported memory filesystem > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > and the the memory backing store exchange callbacks when such memslot > gets created. At runtime KVM will call into callbacks provided by the > backing store to get the pfn with the fd+offset. Memory backing store > will also call into KVM callbacks when userspace punch hole on the fd > to notify KVM to unmap secondary MMU page table entries. > > Comparing to existing hva-based memslot, this new type of memslot allows > guest memory unmapped from host userspace like QEMU and even the kernel > itself, therefore reduce attack surface and prevent bugs. > > Based on this fd-based memslot, we can build guest private memory that > is going to be used in confidential computing environments such as Intel > TDX and AMD SEV. When supported, the memory backing store can provide > more enforcement on the fd and KVM can use a single memslot to hold both > the private and shared part of the guest memory. > > mm extension > - > Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file > created with these flags cannot read(), write() or mmap() etc via normal > MMU operations. The file content can only be used with the newly > introduced memfile_notifier extension. > > The memfile_notifier extension provides two sets of callbacks for KVM to > interact with the memory backing store: > - memfile_notifier_ops: callbacks for memory backing store to notify > KVM when memory gets invalidated. > - backing store callbacks: callbacks for KVM to call into memory > backing store to request memory pages for guest private memory. > > The memfile_notifier extension also provides APIs for memory backing > store to register/unregister itself and to trigger the notifier when the > bookmarked memory gets invalidated. > > The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to > prevent double allocation caused by unintentional guest when we only > have a single side of the shared/private memfds effective. > > memslot extension > - > Add the private fd and the fd offset to existing 'shared' memslot so > that both private/shared guest memory can live in one single memslot. > A page in the memslot is either private or shared. Whether a guest page > is private or shared is maintained through reusing existing SEV ioctls > KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. > I'm on the Android pKVM team at Google, and we've been looking into how this approach fits with what we've been doing with pkvm/arm64. I've had a go at porting your patches, along with some fixes and additions so it would go on top of our latest pkvm patch series [1] to see how well this proposal fits with what we’re doing. You can find the ported code at this link [2]. In general, an fd-based approach fits very well with pKVM for the reasons you mention. It means that we don't necessarily need to map the guest memory, and with the new extensions it allows the host kernel to control whether to restrict migration and swapping. For pKVM, we would also need the guest private memory not to be GUP’able by the kernel so that userspace can’t trick the kernel into accessing guest private memory in a context where it isn’t prepared to handle the fault injected by the hypervisor. We’re looking at whether we could use memfd_secret to achieve this, or maybe whether extending your work might solve the problem. However, during the porting effort, the main issue we've encountered is that many of the details of this approach seem to be targeted at TDX/SEV and don’t readily align with the design of pKVM. My knowledge on TDX is very rudimentary, so please bear with me if I get things wrong. The idea of the memslot having two references to the backing memory, the (new) private_fd (a file descriptor) as well as the userspace_addr (a memory address), with the meaning changing depending on whether the memory is private or shared. Both can potentially be live at the same time, but only one is used by the guest depending on whether the memory is shared or private. For pKVM, the memory region is the same, and whether the underlying physical page is shared or private is determined by the hypervisor based on the initial configuration of the VM and also in response to hypercalls from the guest. So at least from our side, having a private_fd isn't the best fit, but rather just having an fd instead of a userspace_add
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Sun, Aug 21, 2022 at 11:27:44AM +0100, Matthew Wilcox wrote: > On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: > > tmpfs and hugetlbfs and page cache are designed around sharing memory: > > TDX is designed around absolutely not sharing memory; and the further > > uses which Sean foresees appear not to need it as page cache either. > > > > Except perhaps for page migration reasons. It's somewhat incidental, > > but of course page migration knows how to migrate page cache, so > > masquerading as page cache will give a short cut to page migration, > > when page migration becomes at all possible. > > I haven't read the patch series, and I'm not taking a position one way > or the other on whether this is better implemented as a shmem addition > or a shim that asks shmem for memory. Page migration can be done for > driver memory by using PageMovable. I just rewrote how it works, so > the details are top of my mind at the moment if anyone wants something > explained. Commit 68f2736a8583 is the key one to look at. Thanks Matthew. That is helpful to understand the current code. Chao
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Aug 23, 2022 at 04:05:27PM +, Sean Christopherson wrote: > On Tue, Aug 23, 2022, David Hildenbrand wrote: > > On 19.08.22 05:38, Hugh Dickins wrote: > > > On Fri, 19 Aug 2022, Sean Christopherson wrote: > > >> On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: > > >>> On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > On Wed, 6 Jul 2022, Chao Peng wrote: > > But since then, TDX in particular has forced an effort into preventing > > (by flags, seals, notifiers) almost everything that makes it > > shmem/tmpfs. > > > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? > > No. > > >> > > >> But QEMU and other VMMs are users of shmem and memfd. The new features > > >> certainly > > >> aren't useful for _all_ existing users, but I don't think it's fair to > > >> say that > > >> they're not useful for _any_ existing users. > > > > > > Okay, I stand corrected: there exist some users of memfd_create() > > > who will also have use for "INACCESSIBLE" memory. > > > > As raised in reply to the relevant patch, I'm not sure if we really have > > to/want to expose MFD_INACCESSIBLE to user space. I feel like this is a > > requirement of specific memfd_notifer (memfile_notifier) implementations > > -- such as TDX that will convert the memory and MCE-kill the machine on > > ordinary write access. We might be able to set/enforce this when > > registering a notifier internally instead, and fail notifier > > registration if a condition isn't met (e.g., existing mmap). > > > > So I'd be curious, which other users of shmem/memfd would benefit from > > (MMU)-"INACCESSIBLE" memory obtained via memfd_create()? > > I agree that there's no need to expose the inaccessible behavior via uAPI. > Making > it a kernel-internal thing that's negotiated/resolved when KVM binds to the fd > would align INACCESSIBLE with the UNMOVABLE and UNRECLAIMABLE flags (and any > other > flags that get added in the future). > > AFAICT, the user-visible flag is a holdover from the early RFCs and doesn't > provide > any unique functionality. That's also what I'm thinking. And I don't see problem immediately if user has populated the fd at the binding time. Actually that looks an advantage for previously discussed guest payload pre-loading. > > If we go that route, we might want to have shmem/memfd require INACCESSIBLE > to be > set for the initial implementation. I.e. disallow binding without > INACCESSIBLE > until there's a use case. I can do that. Chao
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
Actually the current version allows you to delay the allocation to a later time (e.g. page fault time) if you don't call fallocate() on the private fd. fallocate() is necessary in previous versions because we treat the existense in the fd as 'private' but in this version we track private/shared info in KVM so we don't rely on that fact from memory backstores. Does this also mean reservation of guest physical memory with secure processor (both for SEV-SNP & TDX) will also happen at page fault time? Do we plan to keep it this way? If you are talking about accepting memory by the guest, it is initiated by the guest and has nothing to do with page fault time vs fallocate() allocation of host memory. I mean acceptance happens after host memory allocation but they are not in lockstep, acceptance can happen much later. No, I meant reserving guest physical memory range from hypervisor e.g with RMPUpdate for SEV-SNP or equivalent at TDX side (PAMTs?). As proposed, RMP/PAMT updates will occur in the fault path, i.e. there is no way for userspace to pre-map guest memory. I think the best approach is to turn KVM_TDX_INIT_MEM_REGION into a generic vCPU-scoped ioctl() that allows userspace to pre-map guest memory. Supporting initializing guest private memory with a source page can be implemented via a flag. That also gives KVM line of sight to in-place "conversion", e.g. another flag could be added to say that the dest is also the source. Questions to clarify *my* understanding here: - Do you suggest to use KVM_TDX_INIT_MEM_REGION into a generic ioctl to pre-map guest private memory in addition to initialize the payload (in-place encryption or just copy page to guest private memory)? - Want to clarify "pre-map": Are you suggesting to use the ioctl to avoid the RMP/PAMT registration at guest page fault time? instead pre-map guest private memory i.e to allocate and do RMP/PAMT registration before running the actual guest vCPU's? Thanks, Pankaj The TDX and SNP restrictions would then become addition restrictions on when initializing with a source is allowed (and VMs that don't have guest private memory wouldn't allow the flag at all).
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Aug 23, 2022, David Hildenbrand wrote: > On 19.08.22 05:38, Hugh Dickins wrote: > > On Fri, 19 Aug 2022, Sean Christopherson wrote: > >> On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: > >>> On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > On Wed, 6 Jul 2022, Chao Peng wrote: > But since then, TDX in particular has forced an effort into preventing > (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. > >> > >> But QEMU and other VMMs are users of shmem and memfd. The new features > >> certainly > >> aren't useful for _all_ existing users, but I don't think it's fair to say > >> that > >> they're not useful for _any_ existing users. > > > > Okay, I stand corrected: there exist some users of memfd_create() > > who will also have use for "INACCESSIBLE" memory. > > As raised in reply to the relevant patch, I'm not sure if we really have > to/want to expose MFD_INACCESSIBLE to user space. I feel like this is a > requirement of specific memfd_notifer (memfile_notifier) implementations > -- such as TDX that will convert the memory and MCE-kill the machine on > ordinary write access. We might be able to set/enforce this when > registering a notifier internally instead, and fail notifier > registration if a condition isn't met (e.g., existing mmap). > > So I'd be curious, which other users of shmem/memfd would benefit from > (MMU)-"INACCESSIBLE" memory obtained via memfd_create()? I agree that there's no need to expose the inaccessible behavior via uAPI. Making it a kernel-internal thing that's negotiated/resolved when KVM binds to the fd would align INACCESSIBLE with the UNMOVABLE and UNRECLAIMABLE flags (and any other flags that get added in the future). AFAICT, the user-visible flag is a holdover from the early RFCs and doesn't provide any unique functionality. If we go that route, we might want to have shmem/memfd require INACCESSIBLE to be set for the initial implementation. I.e. disallow binding without INACCESSIBLE until there's a use case.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 19.08.22 05:38, Hugh Dickins wrote: > On Fri, 19 Aug 2022, Sean Christopherson wrote: >> On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: >>> On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: On Wed, 6 Jul 2022, Chao Peng wrote: But since then, TDX in particular has forced an effort into preventing (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. Are any of the shmem.c mods useful to existing users of shmem.c? No. Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. >> >> But QEMU and other VMMs are users of shmem and memfd. The new features >> certainly >> aren't useful for _all_ existing users, but I don't think it's fair to say >> that >> they're not useful for _any_ existing users. > > Okay, I stand corrected: there exist some users of memfd_create() > who will also have use for "INACCESSIBLE" memory. As raised in reply to the relevant patch, I'm not sure if we really have to/want to expose MFD_INACCESSIBLE to user space. I feel like this is a requirement of specific memfd_notifer (memfile_notifier) implementations -- such as TDX that will convert the memory and MCE-kill the machine on ordinary write access. We might be able to set/enforce this when registering a notifier internally instead, and fail notifier registration if a condition isn't met (e.g., existing mmap). So I'd be curious, which other users of shmem/memfd would benefit from (MMU)-"INACCESSIBLE" memory obtained via memfd_create()? -- Thanks, David / dhildenb
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Aug 17, 2022 at 10:27:19AM -0500, Michael Roth wrote: > > I think the best approach is to turn KVM_TDX_INIT_MEM_REGION into a generic > > vCPU-scoped ioctl() that allows userspace to pre-map guest memory. > > Supporting > > initializing guest private memory with a source page can be implemented via > > a > > flag. That also gives KVM line of sight to in-place "conversion", e.g. > > another > > flag could be added to say that the dest is also the source. > > So is this proposed ioctl only intended to handle the initial encrypted > payload, and the KVM_MEMORY_ENCRYPT_{REG,UNREG}_REGION ioctls would > still be used for conversions post-boot? Yes. It is called before running any vcpu. At run time (after running vcpus), KVM_MEMORY_ENCRYPT_{REG,UNREG}_REGION is used. -- Isaku Yamahata
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: > tmpfs and hugetlbfs and page cache are designed around sharing memory: > TDX is designed around absolutely not sharing memory; and the further > uses which Sean foresees appear not to need it as page cache either. > > Except perhaps for page migration reasons. It's somewhat incidental, > but of course page migration knows how to migrate page cache, so > masquerading as page cache will give a short cut to page migration, > when page migration becomes at all possible. I haven't read the patch series, and I'm not taking a position one way or the other on whether this is better implemented as a shmem addition or a shim that asks shmem for memory. Page migration can be done for driver memory by using PageMovable. I just rewrote how it works, so the details are top of my mind at the moment if anyone wants something explained. Commit 68f2736a8583 is the key one to look at.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Sat, 20 Aug 2022, Kirill A. Shutemov wrote: > > Yes, INACCESSIBLE is increase of complexity which you do not want to deal > with in shmem.c. It get it. It's not so much that INACCESSIBLE increases the complexity of memfd/shmem/tmpfs, as that it is completely foreign to it. And by handling all those foreign needs at the KVM end (where you can be sure that the mem attached to the fd is INACCESSIBLE because you have given nobody access to it - no handshaking with 3rd party required). > > I will try next week to rework it as shim to top of shmem. Does it work > for you? Yes, please do, thanks. It's a compromise between us: the initial TDX case has no justification to use shmem at all, but doing it that way will help you with some of the infrastructure, and will probably be easiest for KVM to extend to other more relaxed fd cases later. > > But I think it is wrong to throw it over the fence to KVM folks and say it > is your problem. Core MM has to manage it. We disagree on who is throwing over the fence to whom :) Core MM should manage the core MM parts and KVM should manage the KVM parts. What makes this rather different from most driver usage of MM, is that KVM seems likely to use a great proportion of memory this way. With great memory usage comes great responsibility: I don't think all those flags and seals and notifiers let KVM escape from that. Hugh
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote: > On Thu, 18 Aug 2022, Kirill A . Shutemov wrote: > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > > > > If your memory could be swapped, that would be enough of a good reason > > > to make use of shmem.c: but it cannot be swapped; and although there > > > are some references in the mailthreads to it perhaps being swappable > > > in future, I get the impression that will not happen soon if ever. > > > > > > If your memory could be migrated, that would be some reason to use > > > filesystem page cache (because page migration happens to understand > > > that type of memory): but it cannot be migrated. > > > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping > > theoretically possible, but I'm not aware of any plans as of now. > > > > [1] > > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html > > I always forget, migration means different things to different audiences. > As an mm person, I was meaning page migration, whereas a virtualization > person thinks VM live migration (which that reference appears to be about), > a scheduler person task migration, an ornithologist bird migration, etc. > > But you're an mm person too: you may have cited that reference in the > knowledge that TDX 1.5 Live Migration will entail page migration of the > kind I'm thinking of. (Anyway, it's not important to clarify that here.) TDX 1.5 brings both. In TDX speak, mm migration called relocation. See TDH.MEM.PAGE.RELOCATE. > > > Some of these impressions may come from earlier iterations of the > > > patchset (v7 looks better in several ways than v5). I am probably > > > underestimating the extent to which you have taken on board other > > > usages beyond TDX and SEV private memory, and rightly want to serve > > > them all with similar interfaces: perhaps there is enough justification > > > for shmem there, but I don't see it. There was mention of userfaultfd > > > in one link: does that provide the justification for using shmem? > > > > > > I'm afraid of the special demands you may make of memory allocation > > > later on - surprised that huge pages are not mentioned already; > > > gigantic contiguous extents? secretmem removed from direct map? > > > > The design allows for extension to hugetlbfs if needed. Combination of > > MFD_INACCESSIBLE | MFD_HUGETLB should route this way. There should be zero > > implications for shmem. It is going to be separate struct > > memfile_backing_store. > > Last year's MFD_HUGEPAGE proposal would have allowed you to do it with > memfd via tmpfs without needing to involve hugetlbfs; but you may prefer > the determinism of hugetlbfs, relying on /proc/sys/vm/nr_hugepages etc. > > But I've yet to see why you want to involve this or that filesystem > (with all its filesystem-icity suppressed) at all. The backing store > is host memory, and tmpfs and hugetlbfs just impose their own > idiosyncrasies on how that memory is allocated; but I think you would > do better to choose your own idiosyncrasies in allocation directly - > you don't need a different "backing store" to choose between 4k or 2M > or 1G or whatever allocations. These idiosyncrasies are well known: user who used hugetlbfs may want to get direct replacement that would tap into the same hugetlb reserves and get the same allocation guarantees. Admins know where to look if ENOMEM happens. For THP, admin may know how to tweak allocation/defrag policy for his liking and how to track if they are allocated. > tmpfs and hugetlbfs and page cache are designed around sharing memory: > TDX is designed around absolutely not sharing memory; and the further > uses which Sean foresees appear not to need it as page cache either. > > Except perhaps for page migration reasons. It's somewhat incidental, > but of course page migration knows how to migrate page cache, so > masquerading as page cache will give a short cut to page migration, > when page migration becomes at all possible. > > > > > I'm not sure secretmem is a fit here as we want to extend MFD_INACCESSIBLE > > to be movable if platform supports it and secretmem is not migratable by > > design (without direct mapping fragmentations). > > > > > Here's what I would prefer, and imagine much easier for you to maintain; > > > but I'm no system designer, and may be misunderstanding throughout. > > > > > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > > > the fallocate syscall interface itself) to allocate and free the memory, > > > ioctl for initializing some of it too. KVM in control of whether that > > > fd can be read or written or mmap'ed or whatever, no need to prevent it > > > in shmem.c, no need for flags, seals, notifications to and fro because > > > KVM is already in control and knows the history. If shmem actually has > > > value, call into it underneath - somewhat like SysV SHM,
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Aug 18, 2022, Hugh Dickins wrote: > On Fri, 19 Aug 2022, Sean Christopherson wrote: > > On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: > > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > > If your memory could be migrated, that would be some reason to use > > > > filesystem page cache (because page migration happens to understand > > > > that type of memory): but it cannot be migrated. > > > > > > Migration support is in pipeline. It is part of TDX 1.5 [1]. > > > > And this isn't intended for just TDX (or SNP, or pKVM). We're not _that_ > > far off > > from being able to use UPM for "regular" VMs as a way to provide > > defense-in-depth > > UPM? That's an acronym from your side of the fence, I spy references to > it in the mail threads, but haven't tracked down a definition. I'll > just take it to mean the fd-based memory we're discussing. Ya, sorry, UPM is what we came up with as shorthand for "Unmapping guest Private Memory". Your assumption is spot on, it's just a fancy way of saying "guest is backed with inaccessible fd-based memory". > > without having to take on the overhead of confidential VMs. At that point, > > migration and probably even swap are on the table. > > Good, the more "flexible" that memory is, the better for competing users > of memory. But an fd supplied by KVM gives you freedom to change to a > better implementation of allocation underneath, whenever it suits you. > Maybe shmem beneath is good from the start, maybe not. The main flaw with KVM providing the fd is that it forces KVM to get into the memory management business, which us KVM folks really, really do not want to do. And based on the types of bugs KVM has had in the past related to memory management, it's a safe bet to say the mm folks don't want us getting involved either :-) The combination of gup()/follow_pte() and mmu_notifiers has worked very well. KVM gets a set of (relatively) simple rules to follow and doesn't have to be taught new things every time a new backing type comes along. And from the other side, KVM has very rarely had to go poke into other subsystems' code to support exposing a new type of memory to guests. What we're trying to do with UPM/fd-based memory is establish a similar contract between mm and KVM, but without requiring mm to also map memory into host userspace. The only way having KVM provide the fd works out in the long run is if KVM is the only subsystem that ever wants to make use of memory that isn't accessible from userspace and isn't tied to a specific backing type, _and_ if the set of backing types that KVM ever supports is kept to an absolute minimum.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, 19 Aug 2022, Sean Christopherson wrote: > On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: > > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > On Wed, 6 Jul 2022, Chao Peng wrote: > > > But since then, TDX in particular has forced an effort into preventing > > > (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. > > > > > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > > > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. > > But QEMU and other VMMs are users of shmem and memfd. The new features > certainly > aren't useful for _all_ existing users, but I don't think it's fair to say > that > they're not useful for _any_ existing users. Okay, I stand corrected: there exist some users of memfd_create() who will also have use for "INACCESSIBLE" memory. > > > > What use do you have for a filesystem here? Almost none. > > > IIUC, what you want is an fd through which QEMU can allocate kernel > > > memory, selectively free that memory, and communicate fd+offset+length > > > to KVM. And perhaps an interface to initialize a little of that memory > > > from a template (presumably copied from a real file on disk somewhere). > > > > > > You don't need shmem.c or a filesystem for that! > > > > > > If your memory could be swapped, that would be enough of a good reason > > > to make use of shmem.c: but it cannot be swapped; and although there > > > are some references in the mailthreads to it perhaps being swappable > > > in future, I get the impression that will not happen soon if ever. > > > > > > If your memory could be migrated, that would be some reason to use > > > filesystem page cache (because page migration happens to understand > > > that type of memory): but it cannot be migrated. > > > > Migration support is in pipeline. It is part of TDX 1.5 [1]. > > And this isn't intended for just TDX (or SNP, or pKVM). We're not _that_ far > off > from being able to use UPM for "regular" VMs as a way to provide > defense-in-depth UPM? That's an acronym from your side of the fence, I spy references to it in the mail threads, but haven't tracked down a definition. I'll just take it to mean the fd-based memory we're discussing. > without having to take on the overhead of confidential VMs. At that point, > migration and probably even swap are on the table. Good, the more "flexible" that memory is, the better for competing users of memory. But an fd supplied by KVM gives you freedom to change to a better implementation of allocation underneath, whenever it suits you. Maybe shmem beneath is good from the start, maybe not. Hugh
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, 18 Aug 2022, Kirill A . Shutemov wrote: > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > > > If your memory could be swapped, that would be enough of a good reason > > to make use of shmem.c: but it cannot be swapped; and although there > > are some references in the mailthreads to it perhaps being swappable > > in future, I get the impression that will not happen soon if ever. > > > > If your memory could be migrated, that would be some reason to use > > filesystem page cache (because page migration happens to understand > > that type of memory): but it cannot be migrated. > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping > theoretically possible, but I'm not aware of any plans as of now. > > [1] > https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html I always forget, migration means different things to different audiences. As an mm person, I was meaning page migration, whereas a virtualization person thinks VM live migration (which that reference appears to be about), a scheduler person task migration, an ornithologist bird migration, etc. But you're an mm person too: you may have cited that reference in the knowledge that TDX 1.5 Live Migration will entail page migration of the kind I'm thinking of. (Anyway, it's not important to clarify that here.) > > > Some of these impressions may come from earlier iterations of the > > patchset (v7 looks better in several ways than v5). I am probably > > underestimating the extent to which you have taken on board other > > usages beyond TDX and SEV private memory, and rightly want to serve > > them all with similar interfaces: perhaps there is enough justification > > for shmem there, but I don't see it. There was mention of userfaultfd > > in one link: does that provide the justification for using shmem? > > > > I'm afraid of the special demands you may make of memory allocation > > later on - surprised that huge pages are not mentioned already; > > gigantic contiguous extents? secretmem removed from direct map? > > The design allows for extension to hugetlbfs if needed. Combination of > MFD_INACCESSIBLE | MFD_HUGETLB should route this way. There should be zero > implications for shmem. It is going to be separate struct > memfile_backing_store. Last year's MFD_HUGEPAGE proposal would have allowed you to do it with memfd via tmpfs without needing to involve hugetlbfs; but you may prefer the determinism of hugetlbfs, relying on /proc/sys/vm/nr_hugepages etc. But I've yet to see why you want to involve this or that filesystem (with all its filesystem-icity suppressed) at all. The backing store is host memory, and tmpfs and hugetlbfs just impose their own idiosyncrasies on how that memory is allocated; but I think you would do better to choose your own idiosyncrasies in allocation directly - you don't need a different "backing store" to choose between 4k or 2M or 1G or whatever allocations. tmpfs and hugetlbfs and page cache are designed around sharing memory: TDX is designed around absolutely not sharing memory; and the further uses which Sean foresees appear not to need it as page cache either. Except perhaps for page migration reasons. It's somewhat incidental, but of course page migration knows how to migrate page cache, so masquerading as page cache will give a short cut to page migration, when page migration becomes at all possible. > > I'm not sure secretmem is a fit here as we want to extend MFD_INACCESSIBLE > to be movable if platform supports it and secretmem is not migratable by > design (without direct mapping fragmentations). > > > Here's what I would prefer, and imagine much easier for you to maintain; > > but I'm no system designer, and may be misunderstanding throughout. > > > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > > the fallocate syscall interface itself) to allocate and free the memory, > > ioctl for initializing some of it too. KVM in control of whether that > > fd can be read or written or mmap'ed or whatever, no need to prevent it > > in shmem.c, no need for flags, seals, notifications to and fro because > > KVM is already in control and knows the history. If shmem actually has > > value, call into it underneath - somewhat like SysV SHM, and /dev/zero > > mmap, and i915/gem make use of it underneath. If shmem has nothing to > > add, just allocate and free kernel memory directly, recorded in your > > own xarray. > > I guess shim layer on top of shmem *can* work. I don't see immediately why > it would not. But I'm not sure it is right direction. We risk creating yet > another parallel VM with own rules/locking/accounting that opaque to > core-mm. You are already proposing a new set of rules, foreign to how tmpfs works for others. You're right that KVM allocating large amounts of memory, opaque to core-mm, carries risk: and you'd be right to say that shmem.c provides some clues (s
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Aug 18, 2022, Kirill A . Shutemov wrote: > On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > > On Wed, 6 Jul 2022, Chao Peng wrote: > > But since then, TDX in particular has forced an effort into preventing > > (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. > > > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. But QEMU and other VMMs are users of shmem and memfd. The new features certainly aren't useful for _all_ existing users, but I don't think it's fair to say that they're not useful for _any_ existing users. > > What use do you have for a filesystem here? Almost none. > > IIUC, what you want is an fd through which QEMU can allocate kernel > > memory, selectively free that memory, and communicate fd+offset+length > > to KVM. And perhaps an interface to initialize a little of that memory > > from a template (presumably copied from a real file on disk somewhere). > > > > You don't need shmem.c or a filesystem for that! > > > > If your memory could be swapped, that would be enough of a good reason > > to make use of shmem.c: but it cannot be swapped; and although there > > are some references in the mailthreads to it perhaps being swappable > > in future, I get the impression that will not happen soon if ever. > > > > If your memory could be migrated, that would be some reason to use > > filesystem page cache (because page migration happens to understand > > that type of memory): but it cannot be migrated. > > Migration support is in pipeline. It is part of TDX 1.5 [1]. And this isn't intended for just TDX (or SNP, or pKVM). We're not _that_ far off from being able to use UPM for "regular" VMs as a way to provide defense-in-depth without having to take on the overhead of confidential VMs. At that point, migration and probably even swap are on the table. > And swapping theoretically possible, but I'm not aware of any plans as of > now. Ya, I highly doubt confidential VMs will ever bother with swap. > > I'm afraid of the special demands you may make of memory allocation > > later on - surprised that huge pages are not mentioned already; > > gigantic contiguous extents? secretmem removed from direct map? > > The design allows for extension to hugetlbfs if needed. Combination of > MFD_INACCESSIBLE | MFD_HUGETLB should route this way. There should be zero > implications for shmem. It is going to be separate struct > memfile_backing_store. > > I'm not sure secretmem is a fit here as we want to extend MFD_INACCESSIBLE > to be movable if platform supports it and secretmem is not migratable by > design (without direct mapping fragmentations). But secretmem _could_ be a fit. If a use case wants to unmap guest private memory from both userspace and the kernel then KVM should absolutely be able to support that, but at the same time I don't want to have to update KVM to enable secretmem (and I definitely don't want KVM poking into the directmap itself). MFD_INACCESSIBLE should only say "this memory can't be mapped into userspace", any other properties should be completely separate, e.g. the inability to migrate pages is effective a restriction from KVM (acting on behalf of TDX/SNP), it's not a fundamental property of MFD_INACCESSIBLE.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote: > On Wed, 6 Jul 2022, Chao Peng wrote: > > This is the v7 of this series which tries to implement the fd-based KVM > > guest private memory. > > Here at last are my reluctant thoughts on this patchset. > > fd-based approach for supporting KVM guest private memory: fine. > > Use or abuse of memfd and shmem.c: mistaken. > > memfd_create() was an excellent way to put together the initial prototype. > > But since then, TDX in particular has forced an effort into preventing > (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. > > Are any of the shmem.c mods useful to existing users of shmem.c? No. > Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. > > What use do you have for a filesystem here? Almost none. > IIUC, what you want is an fd through which QEMU can allocate kernel > memory, selectively free that memory, and communicate fd+offset+length > to KVM. And perhaps an interface to initialize a little of that memory > from a template (presumably copied from a real file on disk somewhere). > > You don't need shmem.c or a filesystem for that! > > If your memory could be swapped, that would be enough of a good reason > to make use of shmem.c: but it cannot be swapped; and although there > are some references in the mailthreads to it perhaps being swappable > in future, I get the impression that will not happen soon if ever. > > If your memory could be migrated, that would be some reason to use > filesystem page cache (because page migration happens to understand > that type of memory): but it cannot be migrated. Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping theoretically possible, but I'm not aware of any plans as of now. [1] https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html > Some of these impressions may come from earlier iterations of the > patchset (v7 looks better in several ways than v5). I am probably > underestimating the extent to which you have taken on board other > usages beyond TDX and SEV private memory, and rightly want to serve > them all with similar interfaces: perhaps there is enough justification > for shmem there, but I don't see it. There was mention of userfaultfd > in one link: does that provide the justification for using shmem? > > I'm afraid of the special demands you may make of memory allocation > later on - surprised that huge pages are not mentioned already; > gigantic contiguous extents? secretmem removed from direct map? The design allows for extension to hugetlbfs if needed. Combination of MFD_INACCESSIBLE | MFD_HUGETLB should route this way. There should be zero implications for shmem. It is going to be separate struct memfile_backing_store. I'm not sure secretmem is a fit here as we want to extend MFD_INACCESSIBLE to be movable if platform supports it and secretmem is not migratable by design (without direct mapping fragmentations). > Here's what I would prefer, and imagine much easier for you to maintain; > but I'm no system designer, and may be misunderstanding throughout. > > QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps > the fallocate syscall interface itself) to allocate and free the memory, > ioctl for initializing some of it too. KVM in control of whether that > fd can be read or written or mmap'ed or whatever, no need to prevent it > in shmem.c, no need for flags, seals, notifications to and fro because > KVM is already in control and knows the history. If shmem actually has > value, call into it underneath - somewhat like SysV SHM, and /dev/zero > mmap, and i915/gem make use of it underneath. If shmem has nothing to > add, just allocate and free kernel memory directly, recorded in your > own xarray. I guess shim layer on top of shmem *can* work. I don't see immediately why it would not. But I'm not sure it is right direction. We risk creating yet another parallel VM with own rules/locking/accounting that opaque to core-mm. Note that on machines that run TDX guests such memory would likely be the bulk of memory use. Treating it as a fringe case may bite us one day. -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Aug 16, 2022 at 03:38:08PM +, Sean Christopherson wrote: > On Tue, Aug 16, 2022, Gupta, Pankaj wrote: > > > > > > > Actually the current version allows you to delay the allocation to a > > > > > later time (e.g. page fault time) if you don't call fallocate() on the > > > > > private fd. fallocate() is necessary in previous versions because we > > > > > treat the existense in the fd as 'private' but in this version we > > > > > track > > > > > private/shared info in KVM so we don't rely on that fact from memory > > > > > backstores. > > > > > > > > Does this also mean reservation of guest physical memory with secure > > > > processor (both for SEV-SNP & TDX) will also happen at page fault time? > > > > > > > > Do we plan to keep it this way? > > > > > > If you are talking about accepting memory by the guest, it is initiated by > > > the guest and has nothing to do with page fault time vs fallocate() > > > allocation of host memory. I mean acceptance happens after host memory > > > allocation but they are not in lockstep, acceptance can happen much later. > > > > No, I meant reserving guest physical memory range from hypervisor e.g with > > RMPUpdate for SEV-SNP or equivalent at TDX side (PAMTs?). > > As proposed, RMP/PAMT updates will occur in the fault path, i.e. there is no > way > for userspace to pre-map guest memory. Hi Sean, Currently I have the rmpupdate hook in KVM_MEMORY_ENCRYPT_{REG,UNREG}_REGION ioctls, so that when the pages actually get faulted in they are already in the expected state. I have userspace set up to call KVM_MEMORY_ENCRYPT_* in response to explicit page state changes issued by the guest, as well as in response to MEMORY_FAULT exits for implicit page state changes. Initially the private backing store may or may not be pre-fallocate()'d depending on how userspace wants to handle it. If it's not pre-fallocate()'d, then the pages don't get faulted in until the guest does explicit page state changes (currently SNP guests will do this for all memory at boot time, but with unaccepted memory patches for guest/ovmf this will happen during guest run-time, would still allow us to make efficient use of lazy-pinning support for shorter boot times). If userspaces wants to pre-allocate, it can issue the fallocate() for all the ranges up-front so it doesn't incur the cost during run-time. Is that compatible with the proposed design? Of course, for the initial encrypted payload, we would need to to issue the KVM_MEMORY_ENCRYPT_{REG,UNREG}_REGION up-front. I'm doing that in conjunction with the hack to allow pwrite() to memfd to pre-populate the private pages before the in-place encryption that occurs when SNP_LAUNCH_UPDATE is issued... In the past you and Vishal suggested doing the copy from within SNP_LAUNCH_UPDATE, which seems like a workable solution and something we've been meaning to implement... > > I think the best approach is to turn KVM_TDX_INIT_MEM_REGION into a generic > vCPU-scoped ioctl() that allows userspace to pre-map guest memory. Supporting > initializing guest private memory with a source page can be implemented via a > flag. That also gives KVM line of sight to in-place "conversion", e.g. > another > flag could be added to say that the dest is also the source. So is this proposed ioctl only intended to handle the initial encrypted payload, and the KVM_MEMORY_ENCRYPT_{REG,UNREG}_REGION ioctls would still be used for conversions post-boot? If so, that seems reasonable, but I thought there was some consensus that just handling it per-platform in, e.g., SNP_LAUNCH_UPDATE, was sufficient for now until some additional need arose for a new interface. Has something changed in the regard? Just want to understand the motivations so we can plan accordingly. Thanks! -Mike > > The TDX and SNP restrictions would then become addition restrictions on when > initializing with a source is allowed (and VMs that don't have guest private > memory wouldn't allow the flag at all).
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, 6 Jul 2022, Chao Peng wrote: > This is the v7 of this series which tries to implement the fd-based KVM > guest private memory. Here at last are my reluctant thoughts on this patchset. fd-based approach for supporting KVM guest private memory: fine. Use or abuse of memfd and shmem.c: mistaken. memfd_create() was an excellent way to put together the initial prototype. But since then, TDX in particular has forced an effort into preventing (by flags, seals, notifiers) almost everything that makes it shmem/tmpfs. Are any of the shmem.c mods useful to existing users of shmem.c? No. Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No. What use do you have for a filesystem here? Almost none. IIUC, what you want is an fd through which QEMU can allocate kernel memory, selectively free that memory, and communicate fd+offset+length to KVM. And perhaps an interface to initialize a little of that memory from a template (presumably copied from a real file on disk somewhere). You don't need shmem.c or a filesystem for that! If your memory could be swapped, that would be enough of a good reason to make use of shmem.c: but it cannot be swapped; and although there are some references in the mailthreads to it perhaps being swappable in future, I get the impression that will not happen soon if ever. If your memory could be migrated, that would be some reason to use filesystem page cache (because page migration happens to understand that type of memory): but it cannot be migrated. Some of these impressions may come from earlier iterations of the patchset (v7 looks better in several ways than v5). I am probably underestimating the extent to which you have taken on board other usages beyond TDX and SEV private memory, and rightly want to serve them all with similar interfaces: perhaps there is enough justification for shmem there, but I don't see it. There was mention of userfaultfd in one link: does that provide the justification for using shmem? I'm afraid of the special demands you may make of memory allocation later on - surprised that huge pages are not mentioned already; gigantic contiguous extents? secretmem removed from direct map? Here's what I would prefer, and imagine much easier for you to maintain; but I'm no system designer, and may be misunderstanding throughout. QEMU gets fd from opening /dev/kvm_something, uses ioctls (or perhaps the fallocate syscall interface itself) to allocate and free the memory, ioctl for initializing some of it too. KVM in control of whether that fd can be read or written or mmap'ed or whatever, no need to prevent it in shmem.c, no need for flags, seals, notifications to and fro because KVM is already in control and knows the history. If shmem actually has value, call into it underneath - somewhat like SysV SHM, and /dev/zero mmap, and i915/gem make use of it underneath. If shmem has nothing to add, just allocate and free kernel memory directly, recorded in your own xarray. With that /dev/kvm_something subject to access controls and LSMs - which I cannot find for memfd_create(). Full marks for including the MFD_INACCESSIBLE manpage update, and for Cc'ing linux-api: but I'd have expected some doubts from that direction already. Hugh
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Aug 16, 2022, Gupta, Pankaj wrote: > > > > > Actually the current version allows you to delay the allocation to a > > > > later time (e.g. page fault time) if you don't call fallocate() on the > > > > private fd. fallocate() is necessary in previous versions because we > > > > treat the existense in the fd as 'private' but in this version we track > > > > private/shared info in KVM so we don't rely on that fact from memory > > > > backstores. > > > > > > Does this also mean reservation of guest physical memory with secure > > > processor (both for SEV-SNP & TDX) will also happen at page fault time? > > > > > > Do we plan to keep it this way? > > > > If you are talking about accepting memory by the guest, it is initiated by > > the guest and has nothing to do with page fault time vs fallocate() > > allocation of host memory. I mean acceptance happens after host memory > > allocation but they are not in lockstep, acceptance can happen much later. > > No, I meant reserving guest physical memory range from hypervisor e.g with > RMPUpdate for SEV-SNP or equivalent at TDX side (PAMTs?). As proposed, RMP/PAMT updates will occur in the fault path, i.e. there is no way for userspace to pre-map guest memory. I think the best approach is to turn KVM_TDX_INIT_MEM_REGION into a generic vCPU-scoped ioctl() that allows userspace to pre-map guest memory. Supporting initializing guest private memory with a source page can be implemented via a flag. That also gives KVM line of sight to in-place "conversion", e.g. another flag could be added to say that the dest is also the source. The TDX and SNP restrictions would then become addition restrictions on when initializing with a source is allowed (and VMs that don't have guest private memory wouldn't allow the flag at all).
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
Actually the current version allows you to delay the allocation to a later time (e.g. page fault time) if you don't call fallocate() on the private fd. fallocate() is necessary in previous versions because we treat the existense in the fd as 'private' but in this version we track private/shared info in KVM so we don't rely on that fact from memory backstores. Does this also mean reservation of guest physical memory with secure processor (both for SEV-SNP & TDX) will also happen at page fault time? Do we plan to keep it this way? If you are talking about accepting memory by the guest, it is initiated by the guest and has nothing to do with page fault time vs fallocate() allocation of host memory. I mean acceptance happens after host memory allocation but they are not in lockstep, acceptance can happen much later. No, I meant reserving guest physical memory range from hypervisor e.g with RMPUpdate for SEV-SNP or equivalent at TDX side (PAMTs?). Thanks, Pankaj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Tue, Aug 16, 2022 at 01:33:00PM +0200, Gupta, Pankaj wrote: > Hi Chao, > > > > > Actually the current version allows you to delay the allocation to a > > later time (e.g. page fault time) if you don't call fallocate() on the > > private fd. fallocate() is necessary in previous versions because we > > treat the existense in the fd as 'private' but in this version we track > > private/shared info in KVM so we don't rely on that fact from memory > > backstores. > > Does this also mean reservation of guest physical memory with secure > processor (both for SEV-SNP & TDX) will also happen at page fault time? > > Do we plan to keep it this way? If you are talking about accepting memory by the guest, it is initiated by the guest and has nothing to do with page fault time vs fallocate() allocation of host memory. I mean acceptance happens after host memory allocation but they are not in lockstep, acceptance can happen much later. -- Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
Hi Chao, Actually the current version allows you to delay the allocation to a later time (e.g. page fault time) if you don't call fallocate() on the private fd. fallocate() is necessary in previous versions because we treat the existense in the fd as 'private' but in this version we track private/shared info in KVM so we don't rely on that fact from memory backstores. Does this also mean reservation of guest physical memory with secure processor (both for SEV-SNP & TDX) will also happen at page fault time? Do we plan to keep it this way? Thanks, Pankaj Definitely the page will still be pinned once it's allocated, there is no way to swap it out for example just with the current code. That kind of support, if desirable, can be extended through MOVABLE flag and some other callbacks to let feature-specific code to involve.
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 15/08/22 18:34, Chao Peng wrote: > On Fri, Aug 12, 2022 at 02:18:43PM +0530, Nikunj A. Dadhania wrote: >> >> >> On 12/08/22 12:48, Gupta, Pankaj wrote: >>> However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? >>> >>> Isn't the pinning being handled by the corresponding host memory >>> backend with mmu > notifier and architecture support while doing the >>> memory operations e.g page> migration and swapping/reclaim (not >>> supported currently AFAIU). But yes, we need> to allocate entire guest >>> memory with the new flags MEMFILE_F_{UNMOVABLE, UNRECLAIMABLE etc}. >> >> That is correct, but the question is when does the memory allocated, as >> these flags are set, >> memory is neither moved nor reclaimed. In current scenario, if I start a >> 32GB guest, all 32GB is >> allocated. > > I guess so if guest memory is private by default. > > Other option would be to allocate memory as shared by default and > handle on demand allocation and RMPUPDATE with page state change event. > But still that would be done at guest boot time, IIUC. Sorry! Don't want to hijack the other thread so replying here. I thought the question is for SEV SNP. For SEV, maybe the hypercall with the page state information can be used to allocate memory as we use it or something like quota based memory allocation (just thinking). >>> >>> But all this would have considerable performance overhead (if by default >>> memory is shared) and used mostly at boot time. >> >>> So, preallocating memory (default memory private) seems better approach for >>> both SEV & SEV SNP with later page management (pinning, reclaim) taken care >>> by host memory backend & architecture together. >> >> I am not sure how will pre-allocating memory help, even if guest would not >> use full memory it will be pre-allocated. Which if I understand correctly is >> not expected. > > Actually the current version allows you to delay the allocation to a > later time (e.g. page fault time) if you don't call fallocate() on the > private fd. fallocate() is necessary in previous versions because we > treat the existense in the fd as 'private' but in this version we track > private/shared info in KVM so we don't rely on that fact from memory > backstores. Thanks for confirming Chao, in that case we can drop fallocate() from qemu in both the case * Once while creating the memfd private object * During ram_block_convert_range() for shared->private and vice versa. > Definitely the page will still be pinned once it's allocated, there is > no way to swap it out for example just with the current code. Agree, at present once the page is brought in, page will remain till VM shutdown. > That kind of support, if desirable, can be extended through MOVABLE flag and > some > other callbacks to let feature-specific code to involve. Sure, that could be future work. Regards Nikunj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Fri, Aug 12, 2022 at 02:18:43PM +0530, Nikunj A. Dadhania wrote: > > > On 12/08/22 12:48, Gupta, Pankaj wrote: > > > >> > >> However, fallocate() preallocates full guest memory before starting > >> the guest. > >> With this behaviour guest memory is *not* demand pinned. Is there a > >> way to > >> prevent fallocate() from reserving full guest memory? > > > > Isn't the pinning being handled by the corresponding host memory > > backend with mmu > notifier and architecture support while doing the > > memory operations e.g page> migration and swapping/reclaim (not > > supported currently AFAIU). But yes, we need> to allocate entire guest > > memory with the new flags MEMFILE_F_{UNMOVABLE, UNRECLAIMABLE etc}. > > That is correct, but the question is when does the memory allocated, as > these flags are set, > memory is neither moved nor reclaimed. In current scenario, if I start a > 32GB guest, all 32GB is > allocated. > >>> > >>> I guess so if guest memory is private by default. > >>> > >>> Other option would be to allocate memory as shared by default and > >>> handle on demand allocation and RMPUPDATE with page state change event. > >>> But still that would be done at guest boot time, IIUC. > >> > >> Sorry! Don't want to hijack the other thread so replying here. > >> > >> I thought the question is for SEV SNP. For SEV, maybe the hypercall with > >> the page state information can be used to allocate memory as we use it or > >> something like quota based memory allocation (just thinking). > > > > But all this would have considerable performance overhead (if by default > > memory is shared) and used mostly at boot time. > > > So, preallocating memory (default memory private) seems better approach for > > both SEV & SEV SNP with later page management (pinning, reclaim) taken care > > by host memory backend & architecture together. > > I am not sure how will pre-allocating memory help, even if guest would not > use full memory it will be pre-allocated. Which if I understand correctly is > not expected. Actually the current version allows you to delay the allocation to a later time (e.g. page fault time) if you don't call fallocate() on the private fd. fallocate() is necessary in previous versions because we treat the existense in the fd as 'private' but in this version we track private/shared info in KVM so we don't rely on that fact from memory backstores. Definitely the page will still be pinned once it's allocated, there is no way to swap it out for example just with the current code. That kind of support, if desirable, can be extended through MOVABLE flag and some other callbacks to let feature-specific code to involve. Chao > > Regards > Nikunj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? Isn't the pinning being handled by the corresponding host memory backend with mmu > notifier and architecture support while doing the memory operations e.g page> migration and swapping/reclaim (not supported currently AFAIU). But yes, we need> to allocate entire guest memory with the new flags MEMFILE_F_{UNMOVABLE, UNRECLAIMABLE etc}. That is correct, but the question is when does the memory allocated, as these flags are set, memory is neither moved nor reclaimed. In current scenario, if I start a 32GB guest, all 32GB is allocated. I guess so if guest memory is private by default. Other option would be to allocate memory as shared by default and handle on demand allocation and RMPUPDATE with page state change event. But still that would be done at guest boot time, IIUC. Sorry! Don't want to hijack the other thread so replying here. I thought the question is for SEV SNP. For SEV, maybe the hypercall with the page state information can be used to allocate memory as we use it or something like quota based memory allocation (just thinking). But all this would have considerable performance overhead (if by default memory is shared) and used mostly at boot time. So, preallocating memory (default memory private) seems better approach for both SEV & SEV SNP with later page management (pinning, reclaim) taken care by host memory backend & architecture together. I am not sure how will pre-allocating memory help, even if guest would not use full memory it will be pre-allocated. Which if I understand correctly is not expected. For SEV I am also not very sure what would be the best way. There could be a tradeoff between memory pinning and performance. As I was also thinking about "Async page fault" aspect of guest in my previous reply. Details needs to be figure out. Quoting my previous reply here: "Or maybe later we can think of something like allowing direct page fault on host memory access for *SEV* guest as there is no strict requirement for memory integrity guarantee and the performance overhead." Thanks, Pankaj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 12/08/22 12:48, Gupta, Pankaj wrote: > >> >> However, fallocate() preallocates full guest memory before starting the >> guest. >> With this behaviour guest memory is *not* demand pinned. Is there a way >> to >> prevent fallocate() from reserving full guest memory? > > Isn't the pinning being handled by the corresponding host memory backend > with mmu > notifier and architecture support while doing the memory > operations e.g page> migration and swapping/reclaim (not supported > currently AFAIU). But yes, we need> to allocate entire guest memory with > the new flags MEMFILE_F_{UNMOVABLE, UNRECLAIMABLE etc}. That is correct, but the question is when does the memory allocated, as these flags are set, memory is neither moved nor reclaimed. In current scenario, if I start a 32GB guest, all 32GB is allocated. >>> >>> I guess so if guest memory is private by default. >>> >>> Other option would be to allocate memory as shared by default and >>> handle on demand allocation and RMPUPDATE with page state change event. But >>> still that would be done at guest boot time, IIUC. >> >> Sorry! Don't want to hijack the other thread so replying here. >> >> I thought the question is for SEV SNP. For SEV, maybe the hypercall with the >> page state information can be used to allocate memory as we use it or >> something like quota based memory allocation (just thinking). > > But all this would have considerable performance overhead (if by default > memory is shared) and used mostly at boot time. > So, preallocating memory (default memory private) seems better approach for > both SEV & SEV SNP with later page management (pinning, reclaim) taken care > by host memory backend & architecture together. I am not sure how will pre-allocating memory help, even if guest would not use full memory it will be pre-allocated. Which if I understand correctly is not expected. Regards Nikunj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM and the the memory backing store exchange callbacks when such memslot gets created. At runtime KVM will call into callbacks provided by the backing store to get the pfn with the fd+offset. Memory backing store will also call into KVM callbacks when userspace punch hole on the fd to notify KVM to unmap secondary MMU page table entries. Comparing to existing hva-based memslot, this new type of memslot allows guest memory unmapped from host userspace like QEMU and even the kernel itself, therefore reduce attack surface and prevent bugs. Based on this fd-based memslot, we can build guest private memory that is going to be used in confidential computing environments such as Intel TDX and AMD SEV. When supported, the memory backing store can provide more enforcement on the fd and KVM can use a single memslot to hold both the private and shared part of the guest memory. mm extension - Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file created with these flags cannot read(), write() or mmap() etc via normal MMU operations. The file content can only be used with the newly introduced memfile_notifier extension. The memfile_notifier extension provides two sets of callbacks for KVM to interact with the memory backing store: - memfile_notifier_ops: callbacks for memory backing store to notify KVM when memory gets invalidated. - backing store callbacks: callbacks for KVM to call into memory backing store to request memory pages for guest private memory. The memfile_notifier extension also provides APIs for memory backing store to register/unregister itself and to trigger the notifier when the bookmarked memory gets invalidated. The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to prevent double allocation caused by unintentional guest when we only have a single side of the shared/private memfds effective. memslot extension - Add the private fd and the fd offset to existing 'shared' memslot so that both private/shared guest memory can live in one single memslot. A page in the memslot is either private or shared. Whether a guest page is private or shared is maintained through reusing existing SEV ioctls KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 While debugging an issue with SEV+UPM, found that fallocate() returns an error in QEMU which is not handled (EINTR). With the below handling of EINTR subsequent fallocate() succeeds: diff --git a/backends/hostmem-memfd-private.c b/backends/hostmem-memfd-private.c index af8fb0c957..e8597ed28d 100644 --- a/backends/hostmem-memfd-private.c +++ b/backends/hostmem-memfd-private.c @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) MachineState *machine = MACHINE(qdev_get_machine()); uint32_t ram_flags; char *name; - int fd, priv_fd; + int fd, priv_fd, ret; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) backend->size, ram_flags, fd, 0, errp); g_free(name); - fallocate(priv_fd, 0, 0, backend->size); +again: + ret = fallocate(priv_fd, 0, 0, backend->size); + if (ret) { + perror("Fallocate failed: \n"); + if (errno == EINTR) + goto again; + else + exit(1); + } However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? Isn't the pinning being handled by the corresponding host memory backend with mmu > notifier and architecture support while doing the memory op
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM and the the memory backing store exchange callbacks when such memslot gets created. At runtime KVM will call into callbacks provided by the backing store to get the pfn with the fd+offset. Memory backing store will also call into KVM callbacks when userspace punch hole on the fd to notify KVM to unmap secondary MMU page table entries. Comparing to existing hva-based memslot, this new type of memslot allows guest memory unmapped from host userspace like QEMU and even the kernel itself, therefore reduce attack surface and prevent bugs. Based on this fd-based memslot, we can build guest private memory that is going to be used in confidential computing environments such as Intel TDX and AMD SEV. When supported, the memory backing store can provide more enforcement on the fd and KVM can use a single memslot to hold both the private and shared part of the guest memory. mm extension - Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file created with these flags cannot read(), write() or mmap() etc via normal MMU operations. The file content can only be used with the newly introduced memfile_notifier extension. The memfile_notifier extension provides two sets of callbacks for KVM to interact with the memory backing store: - memfile_notifier_ops: callbacks for memory backing store to notify KVM when memory gets invalidated. - backing store callbacks: callbacks for KVM to call into memory backing store to request memory pages for guest private memory. The memfile_notifier extension also provides APIs for memory backing store to register/unregister itself and to trigger the notifier when the bookmarked memory gets invalidated. The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to prevent double allocation caused by unintentional guest when we only have a single side of the shared/private memfds effective. memslot extension - Add the private fd and the fd offset to existing 'shared' memslot so that both private/shared guest memory can live in one single memslot. A page in the memslot is either private or shared. Whether a guest page is private or shared is maintained through reusing existing SEV ioctls KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 While debugging an issue with SEV+UPM, found that fallocate() returns an error in QEMU which is not handled (EINTR). With the below handling of EINTR subsequent fallocate() succeeds: diff --git a/backends/hostmem-memfd-private.c b/backends/hostmem-memfd-private.c index af8fb0c957..e8597ed28d 100644 --- a/backends/hostmem-memfd-private.c +++ b/backends/hostmem-memfd-private.c @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) MachineState *machine = MACHINE(qdev_get_machine()); uint32_t ram_flags; char *name; - int fd, priv_fd; + int fd, priv_fd, ret; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) backend->size, ram_flags, fd, 0, errp); g_free(name); - fallocate(priv_fd, 0, 0, backend->size); +again: + ret = fallocate(priv_fd, 0, 0, backend->size); + if (ret) { + perror("Fallocate failed: \n"); + if (errno == EINTR) + goto again; + else + exit(1); + } However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? Isn't the pinning being handled by the corresponding host memory backend with mmu > notifier and architecture support while doing the memory opera
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 11/08/22 19:02, Chao Peng wrote: > On Thu, Aug 11, 2022 at 01:30:06PM +0200, Gupta, Pankaj wrote: >>> >>> While debugging an issue with SEV+UPM, found that fallocate() returns >>> an error in QEMU which is not handled (EINTR). With the below handling >>> of EINTR subsequent fallocate() succeeds: > > QEMU code has not well-tested so it's not strange you met problem. But > from the man page, there is signal was caught for EINTR, do you know > the signal number? > > Thanks for you patch but before we change it in QEMU I want to make sure > it's indeed a QEMU issue (e.g. not a kernel isssue). > >>> >>> >>> diff --git a/backends/hostmem-memfd-private.c >>> b/backends/hostmem-memfd-private.c >>> index af8fb0c957..e8597ed28d 100644 >>> --- a/backends/hostmem-memfd-private.c >>> +++ b/backends/hostmem-memfd-private.c >>> @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend >>> *backend, Error **errp) >>> MachineState *machine = MACHINE(qdev_get_machine()); >>> uint32_t ram_flags; >>> char *name; >>> -int fd, priv_fd; >>> +int fd, priv_fd, ret; >>> if (!backend->size) { >>> error_setg(errp, "can't create backend with size 0"); >>> @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend >>> *backend, Error **errp) >>> backend->size, ram_flags, fd, 0, errp); >>> g_free(name); >>> -fallocate(priv_fd, 0, 0, backend->size); >>> +again: >>> +ret = fallocate(priv_fd, 0, 0, backend->size); >>> +if (ret) { >>> + perror("Fallocate failed: \n"); >>> + if (errno == EINTR) >>> + goto again; >>> + else >>> + exit(1); >>> +} >>> >>> However, fallocate() preallocates full guest memory before starting the >>> guest. >>> With this behaviour guest memory is *not* demand pinned. This is with reference to the SEV demand pinning patches that I was working on. The understanding was UPM will not reserve memory for SEV/TDX guest in the beginning similar to normal guest. Here is the relevant quote from the discussion with Sean[1]: "I think we should abandon this approach in favor of committing all our resources to fd-based private memory[*], which (if done right) will provide on-demand pinning for "free". " >>> Is there a way to prevent fallocate() from reserving full guest memory? Regards Nikunj [1] https://lore.kernel.org/kvm/ykih8zm7xfhsf...@google.com/
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 8/11/2022 7:18 PM, Nikunj A. Dadhania wrote: On 11/08/22 17:00, Gupta, Pankaj wrote: This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM and the the memory backing store exchange callbacks when such memslot gets created. At runtime KVM will call into callbacks provided by the backing store to get the pfn with the fd+offset. Memory backing store will also call into KVM callbacks when userspace punch hole on the fd to notify KVM to unmap secondary MMU page table entries. Comparing to existing hva-based memslot, this new type of memslot allows guest memory unmapped from host userspace like QEMU and even the kernel itself, therefore reduce attack surface and prevent bugs. Based on this fd-based memslot, we can build guest private memory that is going to be used in confidential computing environments such as Intel TDX and AMD SEV. When supported, the memory backing store can provide more enforcement on the fd and KVM can use a single memslot to hold both the private and shared part of the guest memory. mm extension - Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file created with these flags cannot read(), write() or mmap() etc via normal MMU operations. The file content can only be used with the newly introduced memfile_notifier extension. The memfile_notifier extension provides two sets of callbacks for KVM to interact with the memory backing store: - memfile_notifier_ops: callbacks for memory backing store to notify KVM when memory gets invalidated. - backing store callbacks: callbacks for KVM to call into memory backing store to request memory pages for guest private memory. The memfile_notifier extension also provides APIs for memory backing store to register/unregister itself and to trigger the notifier when the bookmarked memory gets invalidated. The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to prevent double allocation caused by unintentional guest when we only have a single side of the shared/private memfds effective. memslot extension - Add the private fd and the fd offset to existing 'shared' memslot so that both private/shared guest memory can live in one single memslot. A page in the memslot is either private or shared. Whether a guest page is private or shared is maintained through reusing existing SEV ioctls KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 While debugging an issue with SEV+UPM, found that fallocate() returns an error in QEMU which is not handled (EINTR). With the below handling of EINTR subsequent fallocate() succeeds: diff --git a/backends/hostmem-memfd-private.c b/backends/hostmem-memfd-private.c index af8fb0c957..e8597ed28d 100644 --- a/backends/hostmem-memfd-private.c +++ b/backends/hostmem-memfd-private.c @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) MachineState *machine = MACHINE(qdev_get_machine()); uint32_t ram_flags; char *name; - int fd, priv_fd; + int fd, priv_fd, ret; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) backend->size, ram_flags, fd, 0, errp); g_free(name); - fallocate(priv_fd, 0, 0, backend->size); +again: + ret = fallocate(priv_fd, 0, 0, backend->size); + if (ret) { + perror("Fallocate failed: \n"); + if (errno == EINTR) + goto again; + else + exit(1); + } However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? Isn't the pinning being handled by the corresponding host memory backend with mmu > notifier
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 11/08/22 19:02, Chao Peng wrote: > On Thu, Aug 11, 2022 at 01:30:06PM +0200, Gupta, Pankaj wrote: >> Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 >>> >>> While debugging an issue with SEV+UPM, found that fallocate() returns >>> an error in QEMU which is not handled (EINTR). With the below handling >>> of EINTR subsequent fallocate() succeeds: > > QEMU code has not well-tested so it's not strange you met problem. But > from the man page, there is signal was caught for EINTR, do you know > the signal number? I haven't check that, but that should be fairly straight forward to get. I presume that you are referring to signal_pending() in the shmem_fallocate() > Thanks for you patch but before we change it in QEMU I want to make sure > it's indeed a QEMU issue (e.g. not a kernel isssue). As per the manual fallocate() can return EINTR, and this should be handled by the user space. Regards Nikunj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 11/08/22 17:00, Gupta, Pankaj wrote: > >>> This is the v7 of this series which tries to implement the fd-based KVM >>> guest private memory. The patches are based on latest kvm/queue branch >>> commit: >>> >>> b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU >>> split_desc_cache only by default capacity >>> >>> Introduction >>> >>> In general this patch series introduce fd-based memslot which provides >>> guest memory through memory file descriptor fd[offset,size] instead of >>> hva/size. The fd can be created from a supported memory filesystem >>> like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM >>> and the the memory backing store exchange callbacks when such memslot >>> gets created. At runtime KVM will call into callbacks provided by the >>> backing store to get the pfn with the fd+offset. Memory backing store >>> will also call into KVM callbacks when userspace punch hole on the fd >>> to notify KVM to unmap secondary MMU page table entries. >>> >>> Comparing to existing hva-based memslot, this new type of memslot allows >>> guest memory unmapped from host userspace like QEMU and even the kernel >>> itself, therefore reduce attack surface and prevent bugs. >>> >>> Based on this fd-based memslot, we can build guest private memory that >>> is going to be used in confidential computing environments such as Intel >>> TDX and AMD SEV. When supported, the memory backing store can provide >>> more enforcement on the fd and KVM can use a single memslot to hold both >>> the private and shared part of the guest memory. >>> >>> mm extension >>> - >>> Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file >>> created with these flags cannot read(), write() or mmap() etc via normal >>> MMU operations. The file content can only be used with the newly >>> introduced memfile_notifier extension. >>> >>> The memfile_notifier extension provides two sets of callbacks for KVM to >>> interact with the memory backing store: >>> - memfile_notifier_ops: callbacks for memory backing store to notify >>> KVM when memory gets invalidated. >>> - backing store callbacks: callbacks for KVM to call into memory >>> backing store to request memory pages for guest private memory. >>> >>> The memfile_notifier extension also provides APIs for memory backing >>> store to register/unregister itself and to trigger the notifier when the >>> bookmarked memory gets invalidated. >>> >>> The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to >>> prevent double allocation caused by unintentional guest when we only >>> have a single side of the shared/private memfds effective. >>> >>> memslot extension >>> - >>> Add the private fd and the fd offset to existing 'shared' memslot so >>> that both private/shared guest memory can live in one single memslot. >>> A page in the memslot is either private or shared. Whether a guest page >>> is private or shared is maintained through reusing existing SEV ioctls >>> KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. >>> >>> Test >>> >>> To test the new functionalities of this patch TDX patchset is needed. >>> Since TDX patchset has not been merged so I did two kinds of test: >>> >>> - Regresion test on kvm/queue (this patchset) >>> Most new code are not covered. Code also in below repo: >>> https://github.com/chao-p/linux/tree/privmem-v7 >>> >>> - New Funational test on latest TDX code >>> The patch is rebased to latest TDX code and tested the new >>> funcationalities. See below repos: >>> Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx >>> QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 >> >> While debugging an issue with SEV+UPM, found that fallocate() returns >> an error in QEMU which is not handled (EINTR). With the below handling >> of EINTR subsequent fallocate() succeeds: >> >> >> diff --git a/backends/hostmem-memfd-private.c >> b/backends/hostmem-memfd-private.c >> index af8fb0c957..e8597ed28d 100644 >> --- a/backends/hostmem-memfd-private.c >> +++ b/backends/hostmem-memfd-private.c >> @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend >> *backend, Error **errp) >> MachineState *machine = MACHINE(qdev_get_machine()); >> uint32_t ram_flags; >> char *name; >> - int fd, priv_fd; >> + int fd, priv_fd, ret; >> if (!backend->size) { >> error_setg(errp, "can't create backend with size 0"); >> @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend >> *backend, Error **errp) >> backend->size, ram_flags, fd, 0, errp); >> g_free(name); >> - fallocate(priv_fd, 0, 0, backend->size); >> +again: >> + ret = fallocate(priv_fd, 0, 0, backend->size); >> + if (ret) { >> + perror("Fallocate failed: \n"); >> + if (errno == EINTR) >> + goto again; >> + else >> + exit(1); >> +
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Aug 11, 2022 at 01:30:06PM +0200, Gupta, Pankaj wrote: > > > > This is the v7 of this series which tries to implement the fd-based KVM > > > guest private memory. The patches are based on latest kvm/queue branch > > > commit: > > > > > >b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > > > split_desc_cache only by default capacity > > > > > > Introduction > > > > > > In general this patch series introduce fd-based memslot which provides > > > guest memory through memory file descriptor fd[offset,size] instead of > > > hva/size. The fd can be created from a supported memory filesystem > > > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > > > and the the memory backing store exchange callbacks when such memslot > > > gets created. At runtime KVM will call into callbacks provided by the > > > backing store to get the pfn with the fd+offset. Memory backing store > > > will also call into KVM callbacks when userspace punch hole on the fd > > > to notify KVM to unmap secondary MMU page table entries. > > > > > > Comparing to existing hva-based memslot, this new type of memslot allows > > > guest memory unmapped from host userspace like QEMU and even the kernel > > > itself, therefore reduce attack surface and prevent bugs. > > > > > > Based on this fd-based memslot, we can build guest private memory that > > > is going to be used in confidential computing environments such as Intel > > > TDX and AMD SEV. When supported, the memory backing store can provide > > > more enforcement on the fd and KVM can use a single memslot to hold both > > > the private and shared part of the guest memory. > > > > > > mm extension > > > - > > > Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file > > > created with these flags cannot read(), write() or mmap() etc via normal > > > MMU operations. The file content can only be used with the newly > > > introduced memfile_notifier extension. > > > > > > The memfile_notifier extension provides two sets of callbacks for KVM to > > > interact with the memory backing store: > > >- memfile_notifier_ops: callbacks for memory backing store to notify > > > KVM when memory gets invalidated. > > >- backing store callbacks: callbacks for KVM to call into memory > > > backing store to request memory pages for guest private memory. > > > > > > The memfile_notifier extension also provides APIs for memory backing > > > store to register/unregister itself and to trigger the notifier when the > > > bookmarked memory gets invalidated. > > > > > > The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to > > > prevent double allocation caused by unintentional guest when we only > > > have a single side of the shared/private memfds effective. > > > > > > memslot extension > > > - > > > Add the private fd and the fd offset to existing 'shared' memslot so > > > that both private/shared guest memory can live in one single memslot. > > > A page in the memslot is either private or shared. Whether a guest page > > > is private or shared is maintained through reusing existing SEV ioctls > > > KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. > > > > > > Test > > > > > > To test the new functionalities of this patch TDX patchset is needed. > > > Since TDX patchset has not been merged so I did two kinds of test: > > > > > > - Regresion test on kvm/queue (this patchset) > > > Most new code are not covered. Code also in below repo: > > > https://github.com/chao-p/linux/tree/privmem-v7 > > > > > > - New Funational test on latest TDX code > > > The patch is rebased to latest TDX code and tested the new > > > funcationalities. See below repos: > > > Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx > > > QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 > > > > While debugging an issue with SEV+UPM, found that fallocate() returns > > an error in QEMU which is not handled (EINTR). With the below handling > > of EINTR subsequent fallocate() succeeds: QEMU code has not well-tested so it's not strange you met problem. But from the man page, there is signal was caught for EINTR, do you know the signal number? Thanks for you patch but before we change it in QEMU I want to make sure it's indeed a QEMU issue (e.g. not a kernel isssue). > > > > > > diff --git a/backends/hostmem-memfd-private.c > > b/backends/hostmem-memfd-private.c > > index af8fb0c957..e8597ed28d 100644 > > --- a/backends/hostmem-memfd-private.c > > +++ b/backends/hostmem-memfd-private.c > > @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend > > *backend, Error **errp) > > MachineState *machine = MACHINE(qdev_get_machine()); > > uint32_t ram_flags; > > char *name; > > -int fd, priv_fd; > > +int fd, priv_fd, ret; > > if (!backend->size) { > > error_setg(errp, "can't create backend with size 0"); > > @@ -65,7 +65,15 @@
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM and the the memory backing store exchange callbacks when such memslot gets created. At runtime KVM will call into callbacks provided by the backing store to get the pfn with the fd+offset. Memory backing store will also call into KVM callbacks when userspace punch hole on the fd to notify KVM to unmap secondary MMU page table entries. Comparing to existing hva-based memslot, this new type of memslot allows guest memory unmapped from host userspace like QEMU and even the kernel itself, therefore reduce attack surface and prevent bugs. Based on this fd-based memslot, we can build guest private memory that is going to be used in confidential computing environments such as Intel TDX and AMD SEV. When supported, the memory backing store can provide more enforcement on the fd and KVM can use a single memslot to hold both the private and shared part of the guest memory. mm extension - Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file created with these flags cannot read(), write() or mmap() etc via normal MMU operations. The file content can only be used with the newly introduced memfile_notifier extension. The memfile_notifier extension provides two sets of callbacks for KVM to interact with the memory backing store: - memfile_notifier_ops: callbacks for memory backing store to notify KVM when memory gets invalidated. - backing store callbacks: callbacks for KVM to call into memory backing store to request memory pages for guest private memory. The memfile_notifier extension also provides APIs for memory backing store to register/unregister itself and to trigger the notifier when the bookmarked memory gets invalidated. The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to prevent double allocation caused by unintentional guest when we only have a single side of the shared/private memfds effective. memslot extension - Add the private fd and the fd offset to existing 'shared' memslot so that both private/shared guest memory can live in one single memslot. A page in the memslot is either private or shared. Whether a guest page is private or shared is maintained through reusing existing SEV ioctls KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 While debugging an issue with SEV+UPM, found that fallocate() returns an error in QEMU which is not handled (EINTR). With the below handling of EINTR subsequent fallocate() succeeds: diff --git a/backends/hostmem-memfd-private.c b/backends/hostmem-memfd-private.c index af8fb0c957..e8597ed28d 100644 --- a/backends/hostmem-memfd-private.c +++ b/backends/hostmem-memfd-private.c @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) MachineState *machine = MACHINE(qdev_get_machine()); uint32_t ram_flags; char *name; -int fd, priv_fd; +int fd, priv_fd, ret; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) backend->size, ram_flags, fd, 0, errp); g_free(name); -fallocate(priv_fd, 0, 0, backend->size); +again: +ret = fallocate(priv_fd, 0, 0, backend->size); +if (ret) { + perror("Fallocate failed: \n"); + if (errno == EINTR) + goto again; + else + exit(1); +} However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? Isn't the pinning being handled by the corresponding host memory backend with mmu notifier and architecture support while doing the memory operations e.g page migration and swapping/reclaim (not s
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On 06/07/22 13:50, Chao Peng wrote: > This is the v7 of this series which tries to implement the fd-based KVM > guest private memory. The patches are based on latest kvm/queue branch > commit: > > b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > split_desc_cache only by default capacity > > Introduction > > In general this patch series introduce fd-based memslot which provides > guest memory through memory file descriptor fd[offset,size] instead of > hva/size. The fd can be created from a supported memory filesystem > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > and the the memory backing store exchange callbacks when such memslot > gets created. At runtime KVM will call into callbacks provided by the > backing store to get the pfn with the fd+offset. Memory backing store > will also call into KVM callbacks when userspace punch hole on the fd > to notify KVM to unmap secondary MMU page table entries. > > Comparing to existing hva-based memslot, this new type of memslot allows > guest memory unmapped from host userspace like QEMU and even the kernel > itself, therefore reduce attack surface and prevent bugs. > > Based on this fd-based memslot, we can build guest private memory that > is going to be used in confidential computing environments such as Intel > TDX and AMD SEV. When supported, the memory backing store can provide > more enforcement on the fd and KVM can use a single memslot to hold both > the private and shared part of the guest memory. > > mm extension > - > Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file > created with these flags cannot read(), write() or mmap() etc via normal > MMU operations. The file content can only be used with the newly > introduced memfile_notifier extension. > > The memfile_notifier extension provides two sets of callbacks for KVM to > interact with the memory backing store: > - memfile_notifier_ops: callbacks for memory backing store to notify > KVM when memory gets invalidated. > - backing store callbacks: callbacks for KVM to call into memory > backing store to request memory pages for guest private memory. > > The memfile_notifier extension also provides APIs for memory backing > store to register/unregister itself and to trigger the notifier when the > bookmarked memory gets invalidated. > > The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to > prevent double allocation caused by unintentional guest when we only > have a single side of the shared/private memfds effective. > > memslot extension > - > Add the private fd and the fd offset to existing 'shared' memslot so > that both private/shared guest memory can live in one single memslot. > A page in the memslot is either private or shared. Whether a guest page > is private or shared is maintained through reusing existing SEV ioctls > KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. > > Test > > To test the new functionalities of this patch TDX patchset is needed. > Since TDX patchset has not been merged so I did two kinds of test: > > - Regresion test on kvm/queue (this patchset) >Most new code are not covered. Code also in below repo: >https://github.com/chao-p/linux/tree/privmem-v7 > > - New Funational test on latest TDX code >The patch is rebased to latest TDX code and tested the new >funcationalities. See below repos: >Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx >QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 While debugging an issue with SEV+UPM, found that fallocate() returns an error in QEMU which is not handled (EINTR). With the below handling of EINTR subsequent fallocate() succeeds: diff --git a/backends/hostmem-memfd-private.c b/backends/hostmem-memfd-private.c index af8fb0c957..e8597ed28d 100644 --- a/backends/hostmem-memfd-private.c +++ b/backends/hostmem-memfd-private.c @@ -39,7 +39,7 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) MachineState *machine = MACHINE(qdev_get_machine()); uint32_t ram_flags; char *name; -int fd, priv_fd; +int fd, priv_fd, ret; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -65,7 +65,15 @@ priv_memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) backend->size, ram_flags, fd, 0, errp); g_free(name); -fallocate(priv_fd, 0, 0, backend->size); +again: +ret = fallocate(priv_fd, 0, 0, backend->size); +if (ret) { + perror("Fallocate failed: \n"); + if (errno == EINTR) + goto again; + else + exit(1); +} However, fallocate() preallocates full guest memory before starting the guest. With this behaviour guest memory is *not* demand pinned. Is there a way to prevent fallocate() from reserving full guest memory? > An example QEMU command line for TDX t
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM Thinking a bit, As host side fd on tmpfs or shmem will store memory on host page cache instead of mapping pages into userspace address space. Can we hit double (un-coordinated) page cache problem with this when guest page cache is also used? This is my understanding: in host it will be indeed in page cache (in current shmem implementation) but that's just the way it allocates and provides the physical memory for the guest. In guest, guest OS will not see this fd (absolutely), it only sees guest memory, on top of which it can build its own page cache system for its own file-mapped content but that is unrelated to host page cache. yes. If guest fills its page cache with file backed memory, this at host side(on shmem fd backend) will also fill the host page cache fast. This can have an impact on performance of guest VM's if host goes to memory pressure situation sooner. Or else we end up utilizing way less System RAM. Is this in any meaningful way different from a regular VM? After thinking a bit, Seems 'No'. Except the reclaim decisions system would take under memory pressure and also will have to see how well this gets stitched with memory tiers in future. But all these are future topics. Sorry! for the noise. Thanks, Pankaj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM Thinking a bit, As host side fd on tmpfs or shmem will store memory on host page cache instead of mapping pages into userspace address space. Can we hit double (un-coordinated) page cache problem with this when guest page cache is also used? This is my understanding: in host it will be indeed in page cache (in current shmem implementation) but that's just the way it allocates and provides the physical memory for the guest. In guest, guest OS will not see this fd (absolutely), it only sees guest memory, on top of which it can build its own page cache system for its own file-mapped content but that is unrelated to host page cache. yes. If guest fills its page cache with file backed memory, this at host side(on shmem fd backend) will also fill the host page cache fast. This can have an impact on performance of guest VM's if host goes to memory pressure situation sooner. Or else we end up utilizing way less System RAM. (Currently), the file backed guest private memory is long-term pinned and not reclaimable, it's in page cache anyway once we allocated it for guest. This does not depend on how guest use it (e.g. use it for guest page cache or not). Even if host shmem backed memory always be always un-reclaimable, we end up utilizing double RAM (both in guest & host page cache) for guest disk accesses? Answering my own question: We wont use double RAM, just view of guest & host structures would change as per the code path taken. If we we don't care about reclaim situations we should be good, else we have to think something to coordinate page cache between guest & host (that could be an optimization for later). Thanks, Pankaj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM Thinking a bit, As host side fd on tmpfs or shmem will store memory on host page cache instead of mapping pages into userspace address space. Can we hit double (un-coordinated) page cache problem with this when guest page cache is also used? This is my understanding: in host it will be indeed in page cache (in current shmem implementation) but that's just the way it allocates and provides the physical memory for the guest. In guest, guest OS will not see this fd (absolutely), it only sees guest memory, on top of which it can build its own page cache system for its own file-mapped content but that is unrelated to host page cache. yes. If guest fills its page cache with file backed memory, this at host side(on shmem fd backend) will also fill the host page cache fast. This can have an impact on performance of guest VM's if host goes to memory pressure situation sooner. Or else we end up utilizing way less System RAM. (Currently), the file backed guest private memory is long-term pinned and not reclaimable, it's in page cache anyway once we allocated it for guest. This does not depend on how guest use it (e.g. use it for guest page cache or not). Even if host shmem backed memory always be always un-reclaimable, we end up utilizing double RAM (both in guest & host page cache) for guest disk accesses? I am considering this a serious design decision before we commit to this approach. Happy to be enlightened on this and know the thoughts from others as well. Thanks, Pankaj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Jul 13, 2022, at 3:35 AM, Gupta, Pankaj wrote: This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM >>> >>> Thinking a bit, As host side fd on tmpfs or shmem will store memory on host >>> page cache instead of mapping pages into userspace address space. Can we hit >>> double (un-coordinated) page cache problem with this when guest page cache >>> is also used? >> >> This is my understanding: in host it will be indeed in page cache (in >> current shmem implementation) but that's just the way it allocates and >> provides the physical memory for the guest. In guest, guest OS will not >> see this fd (absolutely), it only sees guest memory, on top of which it >> can build its own page cache system for its own file-mapped content but >> that is unrelated to host page cache. > > yes. If guest fills its page cache with file backed memory, this at host > side(on shmem fd backend) will also fill the host page cache fast. This > can have an impact on performance of guest VM's if host goes to memory > pressure situation sooner. Or else we end up utilizing way less System > RAM. Is this in any meaningful way different from a regular VM? --Andy
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Jul 13, 2022 at 12:35:56PM +0200, Gupta, Pankaj wrote: > > > > > This is the v7 of this series which tries to implement the fd-based KVM > > > > guest private memory. The patches are based on latest kvm/queue branch > > > > commit: > > > > > > > > b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > > > > split_desc_cache only by default capacity > > > > > > > > Introduction > > > > > > > > In general this patch series introduce fd-based memslot which provides > > > > guest memory through memory file descriptor fd[offset,size] instead of > > > > hva/size. The fd can be created from a supported memory filesystem > > > > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > > > > > > Thinking a bit, As host side fd on tmpfs or shmem will store memory on > > > host > > > page cache instead of mapping pages into userspace address space. Can we > > > hit > > > double (un-coordinated) page cache problem with this when guest page cache > > > is also used? > > > > This is my understanding: in host it will be indeed in page cache (in > > current shmem implementation) but that's just the way it allocates and > > provides the physical memory for the guest. In guest, guest OS will not > > see this fd (absolutely), it only sees guest memory, on top of which it > > can build its own page cache system for its own file-mapped content but > > that is unrelated to host page cache. > > yes. If guest fills its page cache with file backed memory, this at host > side(on shmem fd backend) will also fill the host page cache fast. This can > have an impact on performance of guest VM's if host goes to memory pressure > situation sooner. Or else we end up utilizing way less System RAM. (Currently), the file backed guest private memory is long-term pinned and not reclaimable, it's in page cache anyway once we allocated it for guest. This does not depend on how guest use it (e.g. use it for guest page cache or not). Chao > > Thanks, > Pankaj >
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM Thinking a bit, As host side fd on tmpfs or shmem will store memory on host page cache instead of mapping pages into userspace address space. Can we hit double (un-coordinated) page cache problem with this when guest page cache is also used? This is my understanding: in host it will be indeed in page cache (in current shmem implementation) but that's just the way it allocates and provides the physical memory for the guest. In guest, guest OS will not see this fd (absolutely), it only sees guest memory, on top of which it can build its own page cache system for its own file-mapped content but that is unrelated to host page cache. yes. If guest fills its page cache with file backed memory, this at host side(on shmem fd backend) will also fill the host page cache fast. This can have an impact on performance of guest VM's if host goes to memory pressure situation sooner. Or else we end up utilizing way less System RAM. Thanks, Pankaj
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
On Wed, Jul 13, 2022 at 05:58:32AM +0200, Gupta, Pankaj wrote: > > > This is the v7 of this series which tries to implement the fd-based KVM > > guest private memory. The patches are based on latest kvm/queue branch > > commit: > > > >b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU > > split_desc_cache only by default capacity > > > > Introduction > > > > In general this patch series introduce fd-based memslot which provides > > guest memory through memory file descriptor fd[offset,size] instead of > > hva/size. The fd can be created from a supported memory filesystem > > like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM > > Thinking a bit, As host side fd on tmpfs or shmem will store memory on host > page cache instead of mapping pages into userspace address space. Can we hit > double (un-coordinated) page cache problem with this when guest page cache > is also used? This is my understanding: in host it will be indeed in page cache (in current shmem implementation) but that's just the way it allocates and provides the physical memory for the guest. In guest, guest OS will not see this fd (absolutely), it only sees guest memory, on top of which it can build its own page cache system for its own file-mapped content but that is unrelated to host page cache. Chao > > Thanks, > Pankaj >
Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM Thinking a bit, As host side fd on tmpfs or shmem will store memory on host page cache instead of mapping pages into userspace address space. Can we hit double (un-coordinated) page cache problem with this when guest page cache is also used? Thanks, Pankaj and the the memory backing store exchange callbacks when such memslot gets created. At runtime KVM will call into callbacks provided by the backing store to get the pfn with the fd+offset. Memory backing store will also call into KVM callbacks when userspace punch hole on the fd to notify KVM to unmap secondary MMU page table entries. Comparing to existing hva-based memslot, this new type of memslot allows guest memory unmapped from host userspace like QEMU and even the kernel itself, therefore reduce attack surface and prevent bugs. Based on this fd-based memslot, we can build guest private memory that is going to be used in confidential computing environments such as Intel TDX and AMD SEV. When supported, the memory backing store can provide more enforcement on the fd and KVM can use a single memslot to hold both the private and shared part of the guest memory. mm extension - Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file created with these flags cannot read(), write() or mmap() etc via normal MMU operations. The file content can only be used with the newly introduced memfile_notifier extension. The memfile_notifier extension provides two sets of callbacks for KVM to interact with the memory backing store: - memfile_notifier_ops: callbacks for memory backing store to notify KVM when memory gets invalidated. - backing store callbacks: callbacks for KVM to call into memory backing store to request memory pages for guest private memory. The memfile_notifier extension also provides APIs for memory backing store to register/unregister itself and to trigger the notifier when the bookmarked memory gets invalidated. The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to prevent double allocation caused by unintentional guest when we only have a single side of the shared/private memfds effective. memslot extension - Add the private fd and the fd offset to existing 'shared' memslot so that both private/shared guest memory can live in one single memslot. A page in the memslot is either private or shared. Whether a guest page is private or shared is maintained through reusing existing SEV ioctls KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 An example QEMU command line for TDX test: -object tdx-guest,id=tdx,debug=off,sept-ve-disable=off \ -machine confidential-guest-support=tdx \ -object memory-backend-memfd-private,id=ram1,size=${mem} \ -machine memory-backend=ram1 Changelog -- v7: - Move the private/shared info from backing store to KVM. - Introduce F_SEAL_AUTO_ALLOCATE to avoid double allocation. - Rework on the sync mechanism between zap/page fault paths. - Addressed other comments in v6. v6: - Re-organzied patch for both mm/KVM parts. - Added flags for memfile_notifier so its consumers can state their features and memory backing store can check against these flags. - Put a backing store reference in the memfile_notifier and move pfn_ops into backing store. - Only support boot time backing store register. - Overall KVM part improvement suggested by Sean and some others. v5: - Removed userspace visible F_SEAL_INACCESSIBLE, instead using an in-kernel flag (SHM_F_INACCESSIBLE for shmem). Private fd can only be created by MFD_INACCESSIBLE. - Introduced new APIs for backing store to register itself to memfile_notifier instead of direct function call. - Added the accounting and restriction for MFD_INACCESSIBLE memory. - Added KVM API doc for new memslot extensions and man page for the new
[PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory
This is the v7 of this series which tries to implement the fd-based KVM guest private memory. The patches are based on latest kvm/queue branch commit: b9b71f43683a (kvm/queue) KVM: x86/mmu: Buffer nested MMU split_desc_cache only by default capacity Introduction In general this patch series introduce fd-based memslot which provides guest memory through memory file descriptor fd[offset,size] instead of hva/size. The fd can be created from a supported memory filesystem like tmpfs/hugetlbfs etc. which we refer as memory backing store. KVM and the the memory backing store exchange callbacks when such memslot gets created. At runtime KVM will call into callbacks provided by the backing store to get the pfn with the fd+offset. Memory backing store will also call into KVM callbacks when userspace punch hole on the fd to notify KVM to unmap secondary MMU page table entries. Comparing to existing hva-based memslot, this new type of memslot allows guest memory unmapped from host userspace like QEMU and even the kernel itself, therefore reduce attack surface and prevent bugs. Based on this fd-based memslot, we can build guest private memory that is going to be used in confidential computing environments such as Intel TDX and AMD SEV. When supported, the memory backing store can provide more enforcement on the fd and KVM can use a single memslot to hold both the private and shared part of the guest memory. mm extension - Introduces new MFD_INACCESSIBLE flag for memfd_create(), the file created with these flags cannot read(), write() or mmap() etc via normal MMU operations. The file content can only be used with the newly introduced memfile_notifier extension. The memfile_notifier extension provides two sets of callbacks for KVM to interact with the memory backing store: - memfile_notifier_ops: callbacks for memory backing store to notify KVM when memory gets invalidated. - backing store callbacks: callbacks for KVM to call into memory backing store to request memory pages for guest private memory. The memfile_notifier extension also provides APIs for memory backing store to register/unregister itself and to trigger the notifier when the bookmarked memory gets invalidated. The patchset also introduces a new memfd seal F_SEAL_AUTO_ALLOCATE to prevent double allocation caused by unintentional guest when we only have a single side of the shared/private memfds effective. memslot extension - Add the private fd and the fd offset to existing 'shared' memslot so that both private/shared guest memory can live in one single memslot. A page in the memslot is either private or shared. Whether a guest page is private or shared is maintained through reusing existing SEV ioctls KVM_MEMORY_ENCRYPT_{UN,}REG_REGION. Test To test the new functionalities of this patch TDX patchset is needed. Since TDX patchset has not been merged so I did two kinds of test: - Regresion test on kvm/queue (this patchset) Most new code are not covered. Code also in below repo: https://github.com/chao-p/linux/tree/privmem-v7 - New Funational test on latest TDX code The patch is rebased to latest TDX code and tested the new funcationalities. See below repos: Linux: https://github.com/chao-p/linux/tree/privmem-v7-tdx QEMU: https://github.com/chao-p/qemu/tree/privmem-v7 An example QEMU command line for TDX test: -object tdx-guest,id=tdx,debug=off,sept-ve-disable=off \ -machine confidential-guest-support=tdx \ -object memory-backend-memfd-private,id=ram1,size=${mem} \ -machine memory-backend=ram1 Changelog -- v7: - Move the private/shared info from backing store to KVM. - Introduce F_SEAL_AUTO_ALLOCATE to avoid double allocation. - Rework on the sync mechanism between zap/page fault paths. - Addressed other comments in v6. v6: - Re-organzied patch for both mm/KVM parts. - Added flags for memfile_notifier so its consumers can state their features and memory backing store can check against these flags. - Put a backing store reference in the memfile_notifier and move pfn_ops into backing store. - Only support boot time backing store register. - Overall KVM part improvement suggested by Sean and some others. v5: - Removed userspace visible F_SEAL_INACCESSIBLE, instead using an in-kernel flag (SHM_F_INACCESSIBLE for shmem). Private fd can only be created by MFD_INACCESSIBLE. - Introduced new APIs for backing store to register itself to memfile_notifier instead of direct function call. - Added the accounting and restriction for MFD_INACCESSIBLE memory. - Added KVM API doc for new memslot extensions and man page for the new MFD_INACCESSIBLE flag. - Removed the overlap check for mapping the same file+offset into multiple gfns due to perf consideration, warned in document. - Addressed other comments in v4. v4: - Decoupled the callbacks between KVM/mm from memfd and use new name 'memfile_notifier'.