On 8/9/24 21:03, Sean Christopherson wrote:
Move the event re-injection unprotect+retry logic into kvm_mmu_write_protect_fault(), i.e. unprotect and retry if and only if the #PF actually hit a write-protected gfn. Note, there is a small possibility that the gfn was unprotected by a different tasking between hitting the #PF and acquiring mmu_lock, but in that case, KVM will resume the guest immediately anyways because KVM will treat the fault as spurious. As a bonus, unprotecting _after_ handling the page fault also addresses the case where the installing a SPTE to handle fault encounters a shadowed PTE, i.e. *creates* a read-only SPTE. Opportunstically add a comment explaining what on earth the intent of the code is, as based on the changelog from commit 577bdc496614 ("KVM: Avoid instruction emulation when event delivery is pending"). Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> --- arch/x86/kvm/mmu/mmu.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index f64ad36ca9e0..d3c0220ff7ee 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -2753,23 +2753,6 @@ bool kvm_mmu_unprotect_gfn_and_retry(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa) return r; }-static int kvm_mmu_unprotect_page_virt(struct kvm_vcpu *vcpu, gva_t gva)-{ - gpa_t gpa; - int r; - - if (vcpu->arch.mmu->root_role.direct) - return 0; - - gpa = kvm_mmu_gva_to_gpa_write(vcpu, gva, NULL); - if (gpa == INVALID_GPA) - return 0; - - r = kvm_mmu_unprotect_page(vcpu->kvm, gpa >> PAGE_SHIFT); - - return r; -} - static void kvm_unsync_page(struct kvm *kvm, struct kvm_mmu_page *sp) { trace_kvm_mmu_unsync_page(sp); @@ -4640,8 +4623,6 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code, if (!flags) { trace_kvm_page_fault(vcpu, fault_address, error_code);- if (kvm_event_needs_reinjection(vcpu))- kvm_mmu_unprotect_page_virt(vcpu, fault_address); r = kvm_mmu_page_fault(vcpu, fault_address, error_code, insn, insn_len); } else if (flags & KVM_PV_REASON_PAGE_NOT_PRESENT) { @@ -6037,8 +6018,15 @@ static int kvm_mmu_write_protect_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, * execute the instruction. If no shadow pages were zapped, then the * write-fault is due to something else entirely, i.e. KVM needs to * emulate, as resuming the guest will put it into an infinite loop. + * + * For indirect MMUs, i.e. if KVM is shadowing the current MMU, try to + * unprotect the gfn and retry if an event is awaiting reinjection. If + * KVM emulates multiple instructions before completing even injection, + * the event could be delayed beyond what is architecturally allowed, + * e.g. KVM could inject an IRQ after the TPR has been raised.
This paragraph should go before the description of kvm_mmu_unprotect_gfn_and_retry: * There are two cases in which we try to unprotect the page here * preemptively, i.e. zap any shadow pages, before emulating the * instruction. * * First, the access may be due to L1 accessing nested NPT/EPT entries * used for L2, i.e. if the gfn being written is for gPTEs that KVM is * shadowing and has write-protected. In this case, because AMD CPUs * walk nested page table using a write operation, walking NPT entries * in L1 can trigger write faults even when L1 isn't modifying PTEs. * KVM would then emulate an excessive number of L1 instructions * without triggering KVM's write-flooding detection, i.e. without * unprotecting the gfn. This is detected as a RO violation while * translating the guest page when the current MMU is direct. * * Second, for indirect MMUs, i.e. if KVM is shadowing the current MMU, * unprotect the gfn and reenter the guest if an event is awaiting * reinjection. If KVM emulates multiple instructions before completing * event injection, the event could be delayed beyond what is * architecturally allowed, e.g. KVM could inject an IRQ after the * TPR has been raised. * * In both cases, if one or more shadow pages were zapped, skip * emulation and resume L1 to let it natively execute the instruction. * If no shadow pages were zapped, then the write-fault is due to * something else entirely and KVM needs to emulate, as resuming * the guest will put it into an infinite loop. Thanks, Paolo
*/ - if (direct && (is_write_to_guest_page_table(error_code)) && + if (((direct && is_write_to_guest_page_table(error_code)) || + (!direct && kvm_event_needs_reinjection(vcpu))) && kvm_mmu_unprotect_gfn_and_retry(vcpu, cr2_or_gpa)) return RET_PF_FIXED;
