| Message ID | 20230908222905.1321305-7-amoorthy@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve KVM + userfaultfd live migration via annotated memory faults. | expand |
On Fri, Sep 08, 2023, Anish Moorthy wrote: > Implement KVM_CAP_MEMORY_FAULT_INFO for efaults generated by > kvm_handle_error_pfn(). Rewrite with --verbose please. And avoid function names if possible. Sometimes it's better/useful/necessary to reference a function by name, but in this case, just saying kvm_handle_error_pfn() 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.
On Wed, Oct 4, 2023 at 6:26 PM Sean Christopherson <seanjc@google.com> 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
On Thu, Oct 05, 2023, Anish Moorthy wrote: > On Wed, Oct 4, 2023 at 6:26 PM Sean Christopherson <seanjc@google.com> 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.
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index e1d011c67cc6..deae8ac74d9a 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -3267,6 +3267,8 @@ static void kvm_send_hwpoison_signal(struct kvm_memory_slot *slot, gfn_t gfn) static int kvm_handle_error_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) { + u64 fault_flags; + if (is_sigpending_pfn(fault->pfn)) { kvm_handle_signal_exit(vcpu); return -EINTR; @@ -3285,6 +3287,17 @@ static int kvm_handle_error_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fa return RET_PF_RETRY; } + WARN_ON_ONCE(fault->goal_level != PG_LEVEL_4K); + + fault_flags = 0; + if (fault->write) + fault_flags = KVM_MEMORY_FAULT_FLAG_WRITE; + else if (fault->exec) + fault_flags = KVM_MEMORY_FAULT_FLAG_EXEC; + else + fault_flags = KVM_MEMORY_FAULT_FLAG_READ; + kvm_handle_guest_uaccess_fault(vcpu, gfn_to_gpa(fault->gfn), PAGE_SIZE, + fault_flags); return -EFAULT; }
Implement KVM_CAP_MEMORY_FAULT_INFO for efaults generated by kvm_handle_error_pfn(). Signed-off-by: Anish Moorthy <amoorthy@google.com> --- arch/x86/kvm/mmu/mmu.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)