Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 1 Sep 2021 23:15:39 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        isaku.yamahata@intel.com, David Matlack <dmatlack@google.com>,
        peterx@redhat.com
Subject: Re: [PATCH 08/16] KVM: MMU: change handle_abnormal_pfn() arguments
 to kvm_page_fault
Message-ID: <YTAJm7c1a37N4adR@google.com>
References: <20210807134936.3083984-1-pbonzini@redhat.com>
 <20210807134936.3083984-9-pbonzini@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210807134936.3083984-9-pbonzini@redhat.com>
Precedence: bulk

On Sat, Aug 07, 2021, Paolo Bonzini wrote:
> Pass struct kvm_page_fault to handle_abnormal_pfn() instead of
> extracting the arguments from the struct.
> 
> Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/mmu/mmu.c         | 17 ++++++++---------
>  arch/x86/kvm/mmu/paging_tmpl.h |  2 +-
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index a6366f1c4197..cec59ac2e1cd 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3024,18 +3024,18 @@ static int kvm_handle_bad_page(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
>  	return -EFAULT;
>  }
>  
> -static bool handle_abnormal_pfn(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn,
> -				kvm_pfn_t pfn, unsigned int access,
> -				int *ret_val)
> +static bool handle_abnormal_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
> +				unsigned int access, int *ret_val)
>  {
>  	/* The pfn is invalid, report the error! */
> -	if (unlikely(is_error_pfn(pfn))) {
> -		*ret_val = kvm_handle_bad_page(vcpu, gfn, pfn);
> +	if (unlikely(is_error_pfn(fault->pfn))) {
> +		*ret_val = kvm_handle_bad_page(vcpu, fault->gfn, fault->pfn);
>  		return true;
>  	}
>  
> -	if (unlikely(is_noslot_pfn(pfn))) {
> -		vcpu_cache_mmio_info(vcpu, gva, gfn,
> +	if (unlikely(is_noslot_pfn(fault->pfn))) {
> +		gva_t gva = fault->is_tdp ? 0 : fault->addr;

Checkpatch wants a newline.  I'm also surprised you didn't abuse bitwise math:

		gva_t gva = fault->addr & ((u64)fault->is_tdp - 1);

I am _not_ suggesting you actually do that ;-)

> +		vcpu_cache_mmio_info(vcpu, gva, fault->gfn,
>  				     access & shadow_mmio_access_mask);
>  		/*
>  		 * If MMIO caching is disabled, emulate immediately without