On Tue, Apr 16, 2024 at 04:22:35PM +0800,
Chao Gao <chao.gao@xxxxxxxxx> wrote:
>
> >This patch makes the emulation_type always set irrelevant to the return
> >code. kvm_mmu_page_fault() is the only caller of kvm_mmu_do_page_fault(),
> >and references the value only when PF_RET_EMULATE is returned. Therefore,
> >this adjustment doesn't affect functionality.
>
> This is benign. But what's the benefit of doing this?
To avoid increment vcpu->stat. Because originally this was VM ioctl, I wanted
to avoid touch vCPU stat. Now it's vCPU ioctl, it's fine to increment them.
Probably we can drop this patch and use kvm_mmu_do_page_fault().
>
> >+static inline int __kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> >+ u64 err, bool prefetch, int *emulation_type)
> > {
> > struct kvm_page_fault fault = {
> > .addr = cr2_or_gpa,
> >@@ -318,14 +318,6 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> > fault.slot = kvm_vcpu_gfn_to_memslot(vcpu, fault.gfn);
> > }
> >
> >- /*
> >- * Async #PF "faults", a.k.a. prefetch faults, are not faults from the
> >- * guest perspective and have already been counted at the time of the
> >- * original fault.
> >- */
> >- if (!prefetch)
> >- vcpu->stat.pf_taken++;
> >-
> > if (IS_ENABLED(CONFIG_MITIGATION_RETPOLINE) && fault.is_tdp)
> > r = kvm_tdp_page_fault(vcpu, &fault);
> > else
> >@@ -333,12 +325,30 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> >
> > if (r == RET_PF_EMULATE && fault.is_private) {
> > kvm_mmu_prepare_memory_fault_exit(vcpu, &fault);
> >- return -EFAULT;
> >+ r = -EFAULT;
> > }
> >
> > if (fault.write_fault_to_shadow_pgtable && emulation_type)
> > *emulation_type |= EMULTYPE_WRITE_PF_TO_SP;
> >
> >+ return r;
> >+}
> >+
> >+static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> >+ u64 err, bool prefetch, int *emulation_type)
> >+{
> >+ int r;
> >+
> >+ /*
> >+ * Async #PF "faults", a.k.a. prefetch faults, are not faults from the
> >+ * guest perspective and have already been counted at the time of the
> >+ * original fault.
> >+ */
> >+ if (!prefetch)
> >+ vcpu->stat.pf_taken++;
> >+
> >+ r = __kvm_mmu_do_page_fault(vcpu, cr2_or_gpa, err, prefetch, emulation_type);
>
> bail out if r < 0?
The following if clauses checks RET_PF_xxx > 0.
--
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
