On Mon, Mar 20, 2023 at 8:13 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Fri, Mar 17, 2023, Anish Moorthy wrote: > > On Fri, Mar 17, 2023 at 1:17 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > > > And as I argued in the last version[*], I am _strongly_ opposed to KVM speculating > > > on why KVM is exiting to userspace. I.e. KVM should not set a special flag if > > > the memslot has "fast only" behavior. The only thing the flag should do is control > > > whether or not KVM tries slow paths, what KVM does in response to an unresolved > > > fault should be an orthogonal thing. > > > > I'm guessing you would want changes to patch 10 of this series [*] > > then, right? Setting a bit/exit reason in kvm_run::memory_fault.flags > > depending on whether the failure originated from a "fast only" fault > > is... exactly what I'm doing :/ I'm not totally clear on your usages > > of the word "flag" above though, the "KVM should not set a special > > flag... the only thing *the* flag should do" part is throwing me off a > > bit. What I think you're saying is > > Heh, the second "the flag" is referring to the memslot flag. Rewriting the above: > > KVM should not set a special flag in kvm_run::memory_fault.flags ... the > only thing KVM_MEM_FAST_FAULT_ONLY should do is ..." > > > "KVM should not set a special bit in kvm_run::memory_fault.flags if > > the memslot has fast-only behavior. The only thing > > KVM_MEM_ABSENT_MAPPING_FAULT should do is..." > > > > [1] https://lore.kernel.org/all/20230315021738.1151386-11-amoorthy@xxxxxxxxxx/ Ok so, just to be clear, you are not opposed to (a) all -EFAULTs from kvm_faultin_pfn populating the kvm_run.memory_fault and setting kvm_run.memory_fault.flags to, say, FAULTIN_FAILURE if/when kvm_cap_memory_fault_exit is enabled but *are* opposed to (b) the combination of the memslot flag and kvm_cap_memory_fault_exit providing any additional information on top of: for instance, a kvm_run.memory_fault.flags of FAULTIN_FAILURE & FAST_FAULT_ONLY. Is that right? > > On Fri, Mar 17, 2023 at 1:54 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > > > Strictly speaking, if y'all buy my argument that the flag shouldn't control the > > > gup behavior, there won't be semantic differences for the memslot flag. KVM will > > > (obviously) behavior differently if KVM_CAP_MEMORY_FAULT_EXIT is not set, but that > > > will hold true for x86 as well. The only difference is that x86 will also support > > > an orthogonal flag that makes the fast-only memslot flag useful in practice. > > > > > > So yeah, there will be an arch dependency, but only because arch code needs to > > > actually handle perform the exit, and that's true no matter what. > > > > > > That said, there's zero reason to put X86 in the name. Just add the capability > > > as KVM_CAP_MEMORY_FAULT_EXIT or whatever and mark it as x86 in the documentation. > > > > > > That said, there's zero reason to put X86 in the name. Just add the capability > > > as KVM_CAP_MEMORY_FAULT_EXIT or whatever and mark it as x86 in the documentation. > > > > Again, a little confused on your first "flag" usage here. I figure you > > can't mean the memslot flag because the whole point of that is to > > control the GUP behavior, but I'm not sure what else you'd be > > referring to. > > > > Anyways the idea of having orthogonal features, one to -EFAULTing > > early before a slow path and another to transform/augment -EFAULTs > > into/with useful information does make sense to me. But I think the > > issue here is that we want the fast-only memslot flag to be useful on > > Arm as well, and with KVM_CAP_MEMORY_FAULT_NOWAIT written as it is now > > there is a semantic differences between x86 and Arm. > > If and only if userspace enables the capability that transforms -EFAULT. > > > I don't see a way to keep the two features here orthogonal on x86 and > > linked on arm without keeping that semantic difference. Perhaps the > > solution here is a bare-bones implementation of > > KVM_CAP_MEMORY_FAULT_EXIT for Arm? All that actually *needs* to be > > covered to resolve this difference is the one call site in > > user_mem_abort. since KVM_CAP_MEMORY_FAULT_EXIT will be allowed to > > have holes anyways. > > As above, so long as userspace must opt into transforming -EFAULT, and can do > so independent of KVM_MEM_FAST_FAULT_ONLY (or whatever we call it), the behavior > of KVM_MEM_FAST_FAULT_ONLY itself is semantically identical across all > architectures. > > KVM_MEM_FAST_FAULT_ONLY is obviously not very useful without precise information > about the failing address, but IMO that's not reason enough to tie the two > together.
