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: <be381bad-3331-495d-d5a5-acf8ae9d6a8e@linux.microsoft.com>
Date:   Wed, 22 Mar 2023 18:07:14 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH] KVM: SVM: Flush Hyper-V TLB when required
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>
Cc:     linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        kvm@vger.kernel.org, Tianyu Lan <ltykernel@gmail.com>,
        Michael Kelley <mikelley@microsoft.com>, stable@vger.kernel.org
References: <20230320185110.1346829-1-jpiotrowski@linux.microsoft.com>
 <ZBsqxeRDh+iV8qmm@google.com> <87355wralt.fsf@redhat.com>
 <ZBs0fTo72LjnR22r@google.com>
From:   Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
In-Reply-To: <ZBs0fTo72LjnR22r@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 22/03/2023 18:01, Sean Christopherson wrote:
> On Wed, Mar 22, 2023, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>>> diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
>>> index cff838f15db5..d91e019fb7da 100644
>>> --- a/arch/x86/kvm/svm/svm_onhyperv.h
>>> +++ b/arch/x86/kvm/svm/svm_onhyperv.h
>>> @@ -15,6 +15,13 @@ static struct kvm_x86_ops svm_x86_ops;
>>>  
>>>  int svm_hv_enable_l2_tlb_flush(struct kvm_vcpu *vcpu);
>>>  
>>> +static inline bool svm_hv_is_enlightened_tlb_enabled(struct kvm_vcpu *vcpu)
>>> +{
>>> +	struct hv_vmcb_enlightenments *hve = &to_svm(vcpu)->vmcb->control.hv_enlightenments;
>>> +
>>> +	return !!hve->hv_enlightenments_control.enlightened_npt_tlb;
>>
>> In theory, we should not look at Hyper-V enlightenments in VMCB control
>> just because our kernel has CONFIG_HYPERV enabled.
> 
> Oooh, right, because hv_enlightenments uses software reserved bits, and in theory
> KVM could be running on a different hypervisor that uses those bits for something
> completely different.
> 
>> I'd suggest we add a
>> real check that we're running on Hyper-V and we can do it the same way
>> it is done in svm_hv_hardware_setup()/svm_hv_init_vmcb():
>>
>> 	return (ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB)
>> 		&& !!hve->hv_enlightenments_control.enlightened_npt_tlb;
> 
> Jeremi, if you grab this, can you put the && on the previous line?  I.e.
> 
> 	return (ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB) &&
> 	       !!hve->hv_enlightenments_control.enlightened_npt_tlb;

Will do. I'll need to read the replies in more detail tomorrow.