Re: [PATCH 2/3] KVM: x86/mmu: Fix pf_fixed count in tdp_mmu_map_handle_target_level()

[Ben Gardon <bgardon@google.com>]

On Wed, May 5, 2021 at 2:38 AM Kai Huang <kai.huang@intel.com> wrote:
>
> Currently pf_fixed is increased even when page fault requires emulation,
> or fault is spurious.  Fix by only increasing it when return value is
> RET_PF_FIXED.

Revisiting __direct_map and mmu_set_spte, there are cases in the
legacy MMU where RET_PF_EMULATE is returned but pf_fixed is still
incremented.
Perhaps it would make more sense to do the increment in the success
case of tdp_mmu_set_spte_atomic as you suggested before. Sorry I
didn't catch that earlier.

It would probably also be worth putting a comment on pf_fixed so that
people in the future know what it's supposed to mean and we don't get
into archeology, reverse engineering the meaning of the stat again.

>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 1cad4c9f7c34..debe8c3ec844 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -942,7 +942,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu, int write,
>                                        rcu_dereference(iter->sptep));
>         }
>
> -       if (!prefault)
> +       if (!prefault && ret == RET_PF_FIXED)
>                 vcpu->stat.pf_fixed++;
>
>         return ret;
> --
> 2.31.1
>