linux-kernel - Re: [RFC Patch 3/3] KVM/x86: Add tlb_remote

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jun 2018 10:13:46 +0000
From:   Tianyu Lan <Tianyu.Lan@...rosoft.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>
CC:     "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>
Subject: Re: [RFC Patch 3/3] KVM/x86: Add tlb_remote_flush callback support
 for vmcs

Hi Vitaly:
	Thanks for your review.

On 6/12/2018 11:12 PM, Vitaly Kuznetsov wrote:
> Tianyu Lan <Tianyu.Lan@...rosoft.com> writes:
> 
>> Register tlb_remote_flush callback for vmcs when hyperv capability of
>> nested guest mapping flush is detected. The interface can help to reduce
>> overhead when flush ept table among vcpus for nested VM. The tradition way
>> is to send IPIs to all affected vcpus and executes INVEPT on each vcpus.
>> It will trigger several vmexits for IPI and INVEPT emulation. Hyperv provides
>> such hypercall to do flush for all vcpus.
>>
>> Signed-off-by: Lan Tianyu <Tianyu.Lan@...rosoft.com>
>> ---
>>   arch/x86/kvm/vmx.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index e50beb76d846..6cb241c05690 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -4737,6 +4737,17 @@ static inline void __vmx_flush_tlb(struct kvm_vcpu *vcpu, int vpid,
>>   	}
>>   }
>>
>> +static int vmx_remote_flush_tlb(struct kvm *kvm)
>> +{
>> +	struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0);
>> +
>> +	if (!VALID_PAGE(vcpu->arch.mmu.root_hpa))
>> +		return -1;
> 
> Why vcpu0? Can arch.mmu.root_hpa-s differ across vCPUs? What happens if
> they do?

Yes, it may take place that arch.mmu.root_hpa is differ across vCPUs.
We may check all vcpu root_hpa and use the hypercall when there is only 
one validated ept table. If not, go back to current way.

static int hv_remote_flush_tlb()
{
         struct kvm_vcpu *vcpu;
         u64 root_hpa = INVALID_PAGE;
         int i;

         kvm_for_each_vcpu(i, vcpu, kvm) {
                 if (!VALID_PAGE(vcpu->arch.mmu.root_hpa))
                         continue;

                 if (!VALID_PAGE(root_hpa))
                         root_hpa = vcpu->arch.mmu.root_hpa;
                 else if (root_hpa != vcpu->arch.mmu.root_hpa) {
                         return -1;
                 }
         }

         if (!VALID_PAGE(root_hpa))
                 return -1;

         return hyperv_flush_guest_mapping(construct_eptp(vcpu,
                 root_hpa));
}


> 
>> +
>> +	return hyperv_flush_guest_mapping(construct_eptp(vcpu,
>> +		vcpu->arch.mmu.root_hpa));
>> +}
> 
> The 'vmx_remote_flush_tlb' name looks generic enough but it is actually
> Hyper-V-specific. I'd suggest renaming to something like
> hv_remote_flush_tlb().
> 
>> +
>>   static void vmx_flush_tlb(struct kvm_vcpu *vcpu, bool invalidate_gpa)
>>   {
>>   	__vmx_flush_tlb(vcpu, to_vmx(vcpu)->vpid, invalidate_gpa);
>> @@ -7495,6 +7506,10 @@ static __init int hardware_setup(void)
>>   	if (enable_ept && !cpu_has_vmx_ept_2m_page())
>>   		kvm_disable_largepages();
>>
>> +	if (ms_hyperv.nested_features & HV_X64_NESTED_GUSET_MAPPING_FLUSH
>> +	    && enable_ept)
>> +		kvm_x86_ops->tlb_remote_flush = vmx_remote_flush_tlb;
>> +
>>   	if (!cpu_has_vmx_ple()) {
>>   		ple_gap = 0;
>>   		ple_window = 0;
>