On Thu, Oct 05, 2023, Anish Moorthy wrote: > On Wed, Oct 4, 2023 at 6:26 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > ...isn't helpful because it doesn't provide any > > insight into the actual impact of the change, i.e. requires the reader to already > > know exactly how and when kvm_handle_error_pfn() is used. > > Ah, true. Hopefully the following is better? > > > KVM: x86: Annotate hva->hpa translation failures in stage-2 fault path > > > > Set up a KVM_EXIT_MEMORY_FAULT in cases where the stage-2 fault exit > > handler successfully translates a GPA to an HVA, but fails to translate > > the HVA to an HPA via GUP. This provides information to userspace which it > > could potentially use, for instance to resolve the failure. > > Maybe the "via GUP" isn't really doing anything there Yeah, drop GUP. Not all pfns are resolved via gup(), e.g. see hva_to_pfn_remapped(). Actually, I would hedge on the "HVA" part too. It's an accurate statement for both ARM and x86, but only because writes to readonly slots get treated as MMIO (stupid option ROMs). And what part of the translation fails isn't important, what's important is that KVM is exiting to userspace with an error. Using "can't resolve the pfn" is fine and desirable, but the changelog should first focus on the important outcome, which is that KVM is now providing more information when throwing an -EFAULT at userspace. Something like this? When handling a stage-2 fault, set up a KVM_EXIT_MEMORY_FAULT exit if KVM returns -EFAULT to userspace, e.g. if resolving the pfn fails for whatever reason. Providing information about the unhandled fault will allow userspace to potentially fix the source of the error and avoid terminating the guest.
