| Message ID | 20240809190319.1710470-17-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Fix multiple #PF RO infinite loop bugs | expand |
On 8/9/24 21:03, Sean Christopherson wrote: > Drop the manual pfn look when retrying an instruction that KVM failed to > emulation in response to a #PF due to a write-protected gfn. Now that KVM > sets EMULTYPE_PF if and only if the page fault it a write-protected gfn, Pointing out where this happened will likely help a few years from now: With the introduction of RET_PF_WRITE_PROTECTED, KVM sets EMULTYPE_PF if and only if the page fault it a write-protected gfn, i.e. if and only if there's a writable memslot. KVM will never try to redo an instruction that failed on emulated MMIO (no slot, or a write to a read-only slot), so therefore there's no redo the lookup in reexecute_instruction(). Paolo > i.e. if and only if there's a writable memslot, there's no need to redo > the lookup to avoid retrying an instruction that failed on emulated MMIO > (no slot, or a write to a read-only slot). > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/x86.c | 18 ------------------ > 1 file changed, 18 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 771e67381fce..67f9871990fb 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -8867,7 +8867,6 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, > int emulation_type) > { > gpa_t gpa = cr2_or_gpa; > - kvm_pfn_t pfn; > > if (!(emulation_type & EMULTYPE_ALLOW_RETRY_PF)) > return false; > @@ -8887,23 +8886,6 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, > return true; > } > > - /* > - * Do not retry the unhandleable instruction if it faults on the > - * readonly host memory, otherwise it will goto a infinite loop: > - * retry instruction -> write #PF -> emulation fail -> retry > - * instruction -> ... > - */ > - pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa)); > - > - /* > - * If the instruction failed on the error pfn, it can not be fixed, > - * report the error to userspace. > - */ > - if (is_error_noslot_pfn(pfn)) > - return false; > - > - kvm_release_pfn_clean(pfn); > - > /* > * If emulation may have been triggered by a write to a shadowed page > * table, unprotect the gfn (zap any relevant SPTEs) and re-enter the
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 771e67381fce..67f9871990fb 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8867,7 +8867,6 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, int emulation_type) { gpa_t gpa = cr2_or_gpa; - kvm_pfn_t pfn; if (!(emulation_type & EMULTYPE_ALLOW_RETRY_PF)) return false; @@ -8887,23 +8886,6 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, return true; } - /* - * Do not retry the unhandleable instruction if it faults on the - * readonly host memory, otherwise it will goto a infinite loop: - * retry instruction -> write #PF -> emulation fail -> retry - * instruction -> ... - */ - pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa)); - - /* - * If the instruction failed on the error pfn, it can not be fixed, - * report the error to userspace. - */ - if (is_error_noslot_pfn(pfn)) - return false; - - kvm_release_pfn_clean(pfn); - /* * If emulation may have been triggered by a write to a shadowed page * table, unprotect the gfn (zap any relevant SPTEs) and re-enter the
Drop the manual pfn look when retrying an instruction that KVM failed to emulation in response to a #PF due to a write-protected gfn. Now that KVM sets EMULTYPE_PF if and only if the page fault it a write-protected gfn, i.e. if and only if there's a writable memslot, there's no need to redo the lookup to avoid retrying an instruction that failed on emulated MMIO (no slot, or a write to a read-only slot). Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/x86.c | 18 ------------------ 1 file changed, 18 deletions(-)