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;
Message-ID: <94cb12a0-ba01-57b0-0aeb-9b179d89c874@redhat.com>
Date:   Fri, 11 Feb 2022 12:18:07 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 04/12] KVM: MMU: WARN if PAE roots linger after
 kvm_mmu_unload
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        vkuznets@redhat.com, mlevitsk@redhat.com, dmatlack@google.com
References: <20220209170020.1775368-1-pbonzini@redhat.com>
 <20220209170020.1775368-5-pbonzini@redhat.com> <YgWdyN3uarajuLdG@google.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <YgWdyN3uarajuLdG@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/11/22 00:20, Sean Christopherson wrote:
> On Wed, Feb 09, 2022, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   arch/x86/kvm/mmu/mmu.c | 17 +++++++++++++----
>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
>> index e0c0f0bc2e8b..7b5765ced928 100644
>> --- a/arch/x86/kvm/mmu/mmu.c
>> +++ b/arch/x86/kvm/mmu/mmu.c
>> @@ -5065,12 +5065,21 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
>>   	return r;
>>   }
>>   
>> +static void __kvm_mmu_unload(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
>> +{
>> +	int i;
>> +	kvm_mmu_free_roots(vcpu, mmu, KVM_MMU_ROOTS_ALL);
>> +	WARN_ON(VALID_PAGE(mmu->root_hpa));
>> +	if (mmu->pae_root) {
>> +		for (i = 0; i < 4; ++i)
>> +			WARN_ON(IS_VALID_PAE_ROOT(mmu->pae_root[i]));
>> +	}
> 
> I'm somewhat ambivalent, but if you're at all on the fence, I vote to drop this
> one.  I've always viewed the WARN on root_hpa as gratuitous.
> 
> But, if it helped during development, then why not...

Well, it was not really helping in that the WARN triggered, but rather 
it was ruling out the more blatant violations of invariants.  The one in 
patch 5 triggered a lot, though.

Paolo