Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <5443b630-d2c8-b0c3-14f5-2b6b3f71221c@redhat.com>
Date:   Tue, 5 Apr 2022 15:55:50 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [RFC PATCH v5 033/104] KVM: x86: Add infrastructure for stolen
 GPA bits
Content-Language: en-US
To:     Kai Huang <kai.huang@intel.com>, isaku.yamahata@intel.com,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     isaku.yamahata@gmail.com, Jim Mattson <jmattson@google.com>,
        erdemaktas@google.com, Connor Kuehl <ckuehl@redhat.com>,
        Sean Christopherson <seanjc@google.com>
References: <cover.1646422845.git.isaku.yamahata@intel.com>
 <a21c1f9065cf27db54820b2b504db4e507835584.1646422845.git.isaku.yamahata@intel.com>
 <2b8038c17b85658a054191b362840240bd66e46b.camel@intel.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <2b8038c17b85658a054191b362840240bd66e46b.camel@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/31/22 13:16, Kai Huang wrote:
>> +	if (range && kvm_available_flush_tlb_with_range()) {
>> +		/* Callback should flush both private GFN and shared GFN. */
>> +		range->start_gfn = kvm_gfn_unalias(kvm, range->start_gfn);
> This seems wrong.  It seems the intention of this function is to flush TLB for
> all aliases for a given GFN range.  Here it seems you are unconditionally change
> to range to always exclude the stolen bits.

He passes the "low" range with bits cleared, and expects the callback to 
take care of both.  That seems okay (apart from the incorrect 
fallthrough that you pointed out).

>>
>>  
>> -		gfn = gpte_to_gfn(gpte);
>> +		gfn = gpte_to_gfn(vcpu, gpte);
>>  		pte_access = sp->role.access;
>>  		pte_access &= FNAME(gpte_access)(gpte);
>>  		FNAME(protect_clean_gpte)(vcpu->arch.mmu, &pte_access, gpte);
> 
> In commit message you mentioned "Don't support stolen bits for shadow EPT" (you
> actually mean shadow MMU I suppose), yet there's bunch of code change to shadow
> MMU.

It's a bit ugly, but it's uglier to keep two versions of gpte_to_gfn.

Perhaps the commit message can be rephrased to "Stolen bits are not 
supported in the shadow MMU; they will be used only for TDX which uses 
the TDP MMU exclusively as it does not support nested virtualization. 
Therefore, the gfn_shared_mask will always be zero in that case".

Paolo