On Wed, Feb 07, 2024 at 01:21:05PM -0800, Anish Moorthy wrote: > On Wed, Feb 7, 2024 at 8:41 AM Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > > > > On Wed, Feb 07, 2024 at 07:39:50AM -0800, Sean Christopherson wrote: > > > > Having said that... > > > > > be part of this patch. Because otherwise, advertising KVM_CAP_MEMORY_FAULT_INFO > > > is a lie. Userspace can't catch KVM in the lie, but that doesn't make it right. > > > > > > That should in turn make it easier to write a useful changelog. > > > > The feedback still stands. The capability needs to be squashed into the > > patch that actually introduces the functionality. > > > > -- > > Thanks, > > Oliver > > Hold on, I think there may be confusion here. No, there isn't. > KVM_CAP_MEMORY_FAULT_INFO is the mechanism for reporting annotated > EFAULTs. These are generic in that other things (such as the guest > memfd stuff) may also report information to userspace using annotated > EFAULTs. > > KVM_CAP_EXIT_ON_MISSING is the thing that says "do an annotated EFAULT > when a stage-2 violation would require faulting in host mapping" On > both x86 and arm64, the relevant functionality is added and the cap is > advertised in a single patch. > > I think it makes sense to enable/advertise the two caps separately (as > I've done here). The former, after all, just says that userspace "may > get annotated EFAULTs for whatever reason" (as opposed to the latter > cap, which says that userspace *will* get annotated EFAULTs when the > stage-2 handler is failed). So even if arm64 userspaces never get > annotated EFAULTs as of this patch, I don't think we're "lying" to > them. I don't know about you, but I find describing UAPI in terms of "may" and "whatever reason" quite unsettling. I like to keep my interactions with userspace deterministic. Overall, I find the informational capability to be quite superfluous as it pertains to this feature. Userspace has *explicitly* opted in to a specific behavior, and the side band capability provides no useful information. You can easily document KVM_CAP_MEMORY_FAULT_INFO in such a way that userspace expects to take this sort of exit. Nobody has presented a use case for annotated EFAULTs on arm64 beyond this opt-in and there is zero interest in predefining UAPI for anything else. x86 may've done this a different way, but that's their business. We're not making UAPI out of any of our other EFAULT returns right now. > Consider a related problem: suppose that code is added in core KVM > which also generates annotated EFAULTs, and that later the arm64 > "Enable KVM_CAP_EXIT_ON_MISSING" patch [1] ends up needing to be > reverted for some reason. The single rule we try to uphold in the kernel is to *never break userspace*, so I don't see this being in the realm of possibility. The moment we expose a feature to userspace we're on the hook for it in perpetuity, and if we break that then you're welcome to send a nastygram to Marc or I. -- Thanks, Oliver
