linux-kernel - RE: [RFC PATCH 08/18] KVM: x86: Add KVM Userfault support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DS0PR11MB637372143BBA7424D2E2EDAEDCAD2@DS0PR11MB6373.namprd11.prod.outlook.com>
Date: Fri, 19 Jul 2024 14:44:19 +0000
From: "Wang, Wei W" <wei.w.wang@...el.com>
To: James Houghton <jthoughton@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, Marc Zyngier <maz@...nel.org>, Oliver
 Upton <oliver.upton@...ux.dev>, James Morse <james.morse@....com>, Suzuki K
 Poulose <suzuki.poulose@....com>, Zenghui Yu <yuzenghui@...wei.com>, Sean
 Christopherson <seanjc@...gle.com>, Shuah Khan <shuah@...nel.org>, Axel
 Rasmussen <axelrasmussen@...gle.com>, "David Matlack" <dmatlack@...gle.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "kvmarm@...ts.linux.dev"
	<kvmarm@...ts.linux.dev>, Peter Xu <peterx@...hat.com>
Subject: RE: [RFC PATCH 08/18] KVM: x86: Add KVM Userfault support

On Friday, July 19, 2024 1:09 AM, James Houghton wrote:
> On Wed, Jul 17, 2024 at 8:34 AM Wang, Wei W <wei.w.wang@...el.com>
> wrote:
> >
> > On Thursday, July 11, 2024 7:42 AM, James Houghton wrote:
> > > The first prong for enabling KVM Userfault support for x86 is to be
> > > able to inform userspace of userfaults. We know when userfaults
> > > occurs when
> > > fault->pfn comes back as KVM_PFN_ERR_FAULT, so in
> > > kvm_mmu_prepare_memory_fault_exit(), simply check if fault->pfn is
> > > indeed KVM_PFN_ERR_FAULT. This means always setting fault->pfn to a
> > > known value (I have chosen KVM_PFN_ERR_FAULT) before calling
> > > kvm_mmu_prepare_memory_fault_exit().
> > >
> > > The next prong is to unmap pages that are newly userfault-enabled.
> > > Do this in kvm_arch_pre_set_memory_attributes().
> >
> > Why is there a need to unmap it?
> > I think a userfault is triggered on a page during postcopy when its
> > data has not yet been fetched from the source, that is, the page is
> > never accessed by guest on the destination and the page table leaf entry is
> empty.
> >
> 
> You're right that it's not strictly necessary for implementing post-copy. This just
> comes down to the UAPI we want: does ATTRIBUTE_USERFAULT mean "KVM
> will be unable to access this memory; any attempt to access it will generate a
> userfault" or does it mean "accesses to never-accessed, non-prefaulted
> memory will generate a userfault."
> 
> I think the former (i.e., the one implemented in this RFC) is slightly clearer and
> slightly more useful.
> 
> Userfaultfd does the latter:
> 1. MAP_PRIVATE|MAP_ANONYMOUS + UFFDIO_REGISTER_MODE_MISSING: if
> nothing is mapped (i.e., major page fault) 2. non-anonymous VMA +
> UFFDIO_REGISTER_MODE_MISSING: if the page cache does not contain a page
> 3. MAP_SHARED + UFFDIO_REGISTER_MODE_MINOR: if the page cache
> *contains* a page, but we got a fault anyway
> 

Yeah, you pointed a good reference here. I think it's beneficial to have different
"modes" for a feature, so we don’t need to force everybody to carry on the
unnecessary burden (e.g. unmap()).

The design is targeted for postcopy support currently, we could start with what
postcopy needs (similar to UFFDIO_REGISTER_MODE_MISSING), while leaving
space for incremental addition of new modes for new usages. When there is a
clear need for unmap(), it could be added under a new flag.

> But in all of these cases, we have a way to start getting userfaults for already-
> accessed memory: for (1) and (3), MADV_DONTNEED, and for (2),
> fallocate(FALLOC_FL_PUNCH_HOLE).

Those cases don't seem related to postcopy (i.e., faults are not brand new and they
need to be handled locally). They could be added under a new flag later when the
related usage is clear.

> 
> Even if we didn't have MADV_DONTNEED (as used to be the case with
> HugeTLB), we can use PROT_NONE to prevent anyone from mapping anything
> in between an mmap() and a UFFDIO_REGISTER. This has been useful for me.

Same for this case as well, plus gmem doesn't support mmap() at present.

> 
> With KVM, we have neither of these tools (unless we include them here),
> AFAIA.