Feel free to tell me to shut up, as it is none of my business, and I might be missing a lot of context. Yet, it never stopped me before. :) > On Apr 24, 2023, at 1:35 PM, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Mon, Apr 24, 2023, Nadav Amit wrote: >> >>> On Apr 24, 2023, at 10:54 AM, Anish Moorthy <amoorthy@xxxxxxxxxx> wrote: >>> Sean did mention that he wanted KVM_CAP_MEMORY_FAULT_INFO in general, >>> so I'm guessing (some version of) that will (eventually :) be merged >>> in any case. >> >> It certainly not my call. But if you ask me, introducing a solution for >> a concrete use-case that requires API changes/enhancements is not >> guaranteed to be the best solution. It may be better first to fully >> understand the existing overheads and agree that there is no alternative >> cleaner and more general solution with similar performance. > > KVM already returns -EFAULT for these situations, the change I really want to land > is to have KVM report detailed information about why the -EFAULT occurred. I'll be > happy to carry the code in KVM even if userspace never does anything beyond dumping > the extra information on failures. I thought that the change is to inform on page-faults through a new interface instead of the existing uffd-file-based one. There is already another interface (signals) and I thought (but did not upstream) io-uring. You now suggest yet another one. I am not sure it is very clean. IIUC, it means that you would still need in userspace to monitor uffd, as qemu (or whatever-kvm-userspace-counterpart-you- use) might also trigger a page-fault. So userspace becomes more complicated, and the ordering of different events/page-faults reporting becomes even more broken. At the same time, you also break various assumptions of userfaultfd. I don’t immediately find some functionality that would break, but I am not very confident about it either. > >> Considering the mess that KVM async-PF introduced, I would be very careful >> before introducing such API changes. I did not look too much on the details, >> but some things anyhow look slightly strange (which might be since I am >> out-of-touch with KVM). For instance, returning -EFAULT on from KVM_RUN? I >> would have assumed -EAGAIN would be more appropriate since the invocation did >> succeed. > > Yeah, returning -EFAULT is somewhat odd, but as above, that's pre-existing > behavior that's been around for many years. Again, it is none of my business, but don’t you want to gradually try to fix the interface so maybe on day you would be able to deprecate it? IOW, if you already introduce a new interface which is enabled with a new flag, which would require userspace change, then you can return the more appropriate error-code.
