On Mon, Apr 19, 2021 at 06:09:29PM +0000, Sean Christopherson wrote: > On Mon, Apr 19, 2021, Kirill A. Shutemov wrote: > > On Mon, Apr 19, 2021 at 04:01:46PM +0000, Sean Christopherson wrote: > > > But fundamentally the private pages, are well, private. They can't be shared > > > across processes, so I think we could (should?) require the VMA to always be > > > MAP_PRIVATE. Does that buy us enough to rely on the VMA alone? I.e. is that > > > enough to prevent userspace and unaware kernel code from acquiring a reference > > > to the underlying page? > > > > Shared pages should be fine too (you folks wanted tmpfs support). > > Is that a conflict though? If the private->shared conversion request is kicked > out to userspace, then userspace can re-mmap() the files as MAP_SHARED, no? > > Allowing MAP_SHARED for guest private memory feels wrong. The data can't be > shared, and dirty data can't be written back to the file. It can be remapped, but faulting in the page would produce hwpoison entry. I don't see other way to make Google's use-case with tmpfs-backed guest memory work. > > The poisoned pages must be useless outside of the process with the blessed > > struct kvm. See kvm_pfn_map in the patch. > > The big requirement for kernel TDX support is that the pages are useless in the > host. Regarding the guest, for TDX, the TDX Module guarantees that at most a > single KVM guest can have access to a page at any given time. I believe the RMP > provides the same guarantees for SEV-SNP. > > SEV/SEV-ES could still end up with corruption if multiple guests map the same > private page, but that's obviously not the end of the world since it's the status > quo today. Living with that shortcoming might be a worthy tradeoff if punting > mutual exclusion between guests to firmware/hardware allows us to simplify the > kernel implementation. The critical question is whether we ever need to translate hva->pfn after the page is added to the guest private memory. I believe we do, but I never checked. And that's the reason we need to keep hwpoison entries around, which encode pfn. If we don't, it would simplify the solution: kvm_pfn_map is not needed. Single bit-per page would be enough. > > > > - Add a new GUP flag to retrive such pages from the userspace mapping. > > > > Used only for private mapping population. > > > > > > > - Shared gfn ranges managed by userspace, based on hypercalls from the > > > > guest. > > > > > > > > - Shared mappings get populated via normal VMA. Any poisoned pages here > > > > would lead to SIGBUS. > > > > > > > > So far it looks pretty straight-forward. > > > > > > > > The only thing that I don't understand is at way point the page gets tied > > > > to the KVM instance. Currently we do it just before populating shadow > > > > entries, but it would not work with the new scheme: as we poison pages > > > > on fault it they may never get inserted into shadow entries. That's not > > > > good as we rely on the info to unpoison page on free. > > > > > > Can you elaborate on what you mean by "unpoison"? If the page is never actually > > > mapped into the guest, then its poisoned status is nothing more than a software > > > flag, i.e. nothing extra needs to be done on free. > > > > Normally, poisoned flag preserved for freed pages as it usually indicate > > hardware issue. In this case we need return page to the normal circulation. > > So we need a way to differentiate two kinds of page poison. Current patch > > does this by adding page's pfn to kvm_pfn_map. But this will not work if > > we uncouple poisoning and adding to shadow PTE. > > Why use PG_hwpoison then? Page flags are scarce. I don't want to take occupy a new one until I'm sure I must. And we can re-use existing infrastructure to SIGBUS on access to such pages. -- Kirill A. Shutemov
