Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Sep 2022 14:48:24 +0800
From:   Yuan Yao <yuan.yao@linux.intel.com>
To:     isaku.yamahata@intel.com
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        isaku.yamahata@gmail.com, Paolo Bonzini <pbonzini@redhat.com>,
        erdemaktas@google.com, Sean Christopherson <seanjc@google.com>,
        Sagi Shahar <sagis@google.com>
Subject: Re: [PATCH v8 038/103] KVM: x86/tdp_mmu: refactor kvm_tdp_mmu_map()
Message-ID: <20220901064824.mpjd3xpgal3d3ynu@yy-desk-7060>
References: <cover.1659854790.git.isaku.yamahata@intel.com>
 <021cf72b904933f23743d74b2a67341298ae5328.1659854790.git.isaku.yamahata@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <021cf72b904933f23743d74b2a67341298ae5328.1659854790.git.isaku.yamahata@intel.com>
User-Agent: NeoMutt/20171215
Precedence: bulk

On Sun, Aug 07, 2022 at 03:01:23PM -0700, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Factor out non-leaf SPTE population logic from kvm_tdp_mmu_map().  MapGPA
> hypercall needs to populate non-leaf SPTE to record which GPA, private or
> shared, is allowed in the leaf EPT entry.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 8bc3a8d1803e..90b468a3a1a2 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1145,6 +1145,24 @@ static int tdp_mmu_link_sp(struct kvm *kvm, struct tdp_iter *iter,
>  	return 0;
>  }
>
> +static int tdp_mmu_populate_nonleaf(
> +	struct kvm_vcpu *vcpu, struct tdp_iter *iter, bool account_nx)
> +{
> +	struct kvm_mmu_page *sp;
> +	int ret;
> +
> +	WARN_ON(is_shadow_present_pte(iter->old_spte));
> +	WARN_ON(is_removed_spte(iter->old_spte));

Why these 2 WARN_ON are necessary here ?

In TPD MMU the changes of PTE with shared lock is not surprised and
should be handle properly (e.g. the page is freed below for this
case), or this function will be called without checking the present
and removed state of the pte ?

> +
> +	sp = tdp_mmu_alloc_sp(vcpu);
> +	tdp_mmu_init_child_sp(sp, iter);
> +
> +	ret = tdp_mmu_link_sp(vcpu->kvm, iter, sp, account_nx, true);
> +	if (ret)
> +		tdp_mmu_free_sp(sp);
> +	return ret;
> +}
> +
>  /*
>   * Handle a TDP page fault (NPT/EPT violation/misconfiguration) by installing
>   * page tables and SPTEs to translate the faulting guest physical address.
> @@ -1153,7 +1171,6 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>  {
>  	struct kvm_mmu *mmu = vcpu->arch.mmu;
>  	struct tdp_iter iter;
> -	struct kvm_mmu_page *sp;
>  	int ret;
>
>  	kvm_mmu_hugepage_adjust(vcpu, fault);
> @@ -1199,13 +1216,8 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>  			if (is_removed_spte(iter.old_spte))
>  				break;
>
> -			sp = tdp_mmu_alloc_sp(vcpu);
> -			tdp_mmu_init_child_sp(sp, &iter);
> -
> -			if (tdp_mmu_link_sp(vcpu->kvm, &iter, sp, account_nx, true)) {
> -				tdp_mmu_free_sp(sp);
> +			if (tdp_mmu_populate_nonleaf(vcpu, &iter, account_nx))
>  				break;
> -			}
>  		}
>  	}
>
> --
> 2.25.1
>