linux-kernel - Re: [PATCH v5 3/7] KVM: x86: hyper-v: Move the remote TLB flush logic out of vmx

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]

Message-ID: <87k0n0yij7.fsf@vitty.brq.redhat.com>
Date:   Fri, 11 Jun 2021 09:20:12 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Vineeth Pillai <viremana@...ux.microsoft.com>
Cc:     "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "K. Y. Srinivasan" <kys@...rosoft.com>, x86@...nel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-hyperv@...r.kernel.org,
        Lan Tianyu <Tianyu.Lan@...rosoft.com>,
        Michael Kelley <mikelley@...rosoft.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, Wei Liu <wei.liu@...nel.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>
Subject: Re: [PATCH v5 3/7] KVM: x86: hyper-v: Move the remote TLB flush
 logic out of vmx

Paolo Bonzini <pbonzini@...hat.com> writes:

> On 10/06/21 13:20, Vitaly Kuznetsov wrote:
>
>>> +static inline void hv_track_root_tdp(struct kvm_vcpu *vcpu, hpa_t root_tdp)
>>> +{
>>> +	struct kvm_arch *kvm_arch = &vcpu->kvm->arch;
>>> +
>>> +	if (kvm_x86_ops.tlb_remote_flush == hv_remote_flush_tlb) {
>>> +		spin_lock(&kvm_arch->hv_root_tdp_lock);
>>> +		vcpu->arch.hv_root_tdp = root_tdp;
>>> +		if (root_tdp != kvm_arch->hv_root_tdp)
>>> +			kvm_arch->hv_root_tdp = INVALID_PAGE;
>>> +		spin_unlock(&kvm_arch->hv_root_tdp_lock);
>>> +	}
>>> +}
>>> +#else
>>> +static inline void hv_track_root_tdp(struct kvm_vcpu *vcpu, hpa_t root_tdp)
>>> +{
>>> +}
>>> +#endif
>>> +#endif
>> 
>> Super-nitpick: I'd suggest adding /* __ARCH_X86_KVM_KVM_ONHYPERV_H__ */
>> to the second '#endif' and /* IS_ENABLED(CONFIG_HYPERV) */ to '#else'
>> and the first one: files/functions tend to grow and it becomes hard to
>> see where the particular '#endif/#else' belongs.
>
> Done, thanks.  I've also changed the #if to just "#ifdef CONFIG_HYPERV", 
> since IS_ENABLED is only needed in C statements.

kvm/queue fails to compile and I blame this change:

In file included from arch/x86/kvm/svm/svm_onhyperv.c:16:
arch/x86/kvm/svm/svm_onhyperv.h: In function ‘svm_hv_hardware_setup’:
arch/x86/kvm/svm/svm_onhyperv.h:56:34: error: ‘hv_remote_flush_tlb’ undeclared (first use in this function); did you mean ‘svm_flush_tlb’?
   56 |   svm_x86_ops.tlb_remote_flush = hv_remote_flush_tlb;
      |                                  ^~~~~~~~~~~~~~~~~~~
      |                                  svm_flush_tlb
arch/x86/kvm/svm/svm_onhyperv.h:56:34: note: each undeclared identifier is reported only once for each function it appears in
arch/x86/kvm/svm/svm_onhyperv.h:58:5: error: ‘hv_remote_flush_tlb_with_range’ undeclared (first use in this function)
   58 |     hv_remote_flush_tlb_with_range;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: arch/x86/kvm/svm/svm_onhyperv.o] Error 1
make[2]: *** Waiting for unfinished jobs....
In file included from arch/x86/kvm/svm/svm.c:47:
arch/x86/kvm/svm/svm_onhyperv.h: In function ‘svm_hv_hardware_setup’:
arch/x86/kvm/svm/svm_onhyperv.h:56:34: error: ‘hv_remote_flush_tlb’ undeclared (first use in this function); did you mean ‘svm_flush_tlb’?
   56 |   svm_x86_ops.tlb_remote_flush = hv_remote_flush_tlb;
      |                                  ^~~~~~~~~~~~~~~~~~~
      |                                  svm_flush_tlb
arch/x86/kvm/svm/svm_onhyperv.h:56:34: note: each undeclared identifier is reported only once for each function it appears in
arch/x86/kvm/svm/svm_onhyperv.h:58:5: error: ‘hv_remote_flush_tlb_with_range’ undeclared (first use in this function)
   58 |     hv_remote_flush_tlb_with_range;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: arch/x86/kvm/svm/svm.o] Error 1
arch/x86/kvm/vmx/vmx.c: In function ‘hardware_setup’:
arch/x86/kvm/vmx/vmx.c:7752:34: error: ‘hv_remote_flush_tlb’ undeclared (first use in this function)
 7752 |   vmx_x86_ops.tlb_remote_flush = hv_remote_flush_tlb;
      |                                  ^~~~~~~~~~~~~~~~~~~
arch/x86/kvm/vmx/vmx.c:7752:34: note: each undeclared identifier is reported only once for each function it appears in
arch/x86/kvm/vmx/vmx.c:7754:5: error: ‘hv_remote_flush_tlb_with_range’ undeclared (first use in this function)
 7754 |     hv_remote_flush_tlb_with_range;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


(Note: CONFIG_HYPERV can be 'm'.)

The following:

index 96da53edfe83..1c67abf2eba9 100644
--- a/arch/x86/kvm/kvm_onhyperv.h
+++ b/arch/x86/kvm/kvm_onhyperv.h
@@ -6,7 +6,7 @@
 #ifndef __ARCH_X86_KVM_KVM_ONHYPERV_H__
 #define __ARCH_X86_KVM_KVM_ONHYPERV_H__
 
-#ifdef CONFIG_HYPERV
+#if IS_ENABLED(CONFIG_HYPERV)
 int hv_remote_flush_tlb_with_range(struct kvm *kvm,
                struct kvm_tlb_range *range);
 int hv_remote_flush_tlb(struct kvm *kvm);

saves the day for me.

-- 
Vitaly