Paolo Bonzini <pbonzini@xxxxxxxxxx> 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
