Re: [PATCH] KVM: x86: Fix names of implemented kvm_x86_ops in VMX and SVM modules

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Mon, Jul 20, 2020 at 06:07:28PM -0400, Krish Sadhukhan wrote:
> Some of the names do not have a corresponding 'vmx_' or 'svm_' prefix. Also,
> the order of the words in some of the names is not the same as that in the
> kvm_x86_ops structure. Fixing the naming will help in better readability of
> the code and maintenance.

If we're going to do a massive rename, I would strongly prefer to
simultaneously enforce the "correct" names by adding a macro to generate the
kvm_x86_ops hooks[*] (sample patch below).  I'd like to realize long term
benefits if we're going to incur merge conflicts on everyone's in-flight
development.

I had a series for this, but it got derailed when svm.c was fractured and I
haven't found time to get back to it.  Code is on
https://github.com/sean-jc/linux/tree/vmx/x86_ops_macros if you have bandwidth
to pick it up.

[*] https://lkml.kernel.org/r/30b847cf-98db-145f-8aa0-a847146d5649@redhat.com


commit 83b835803554f6288af438a519be2c2559d77d89
Author: Sean Christopherson <sean.j.christopherson@intel.com>
Date:   Fri Apr 3 14:12:28 2020 -0700

    KVM: VMX: Fill in conforming vmx_x86_ops via macro

    Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
    Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
    Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 4a8b5a21dcd2e..adb75e21fd021 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6279,14 +6279,16 @@ void nested_vmx_hardware_unsetup(void)
        }
 }

+#define KVM_X86_NESTED_OP(name) .name = nested_vmx_##name
+
 static struct kvm_x86_nested_ops vmx_nested_ops __initdata = {
-       .check_events = nested_vmx_check_events,
-       .get_state = nested_vmx_get_state,
-       .set_state = nested_vmx_set_state,
-       .get_vmcs12_pages = nested_vmx_get_vmcs12_pages,
-       .enable_evmcs = nested_vmx_enable_evmcs,
-       .get_evmcs_version = nested_vmx_get_evmcs_version,
-       .write_log_dirty = nested_vmx_write_log_dirty,
+       KVM_X86_NESTED_OP(check_events),
+       KVM_X86_NESTED_OP(get_state),
+       KVM_X86_NESTED_OP(set_state),
+       KVM_X86_NESTED_OP(get_vmcs12_pages),
+       KVM_X86_NESTED_OP(enable_evmcs),
+       KVM_X86_NESTED_OP(get_evmcs_version),
+       KVM_X86_NESTED_OP(write_log_dirty),
 };

 __init int nested_vmx_hardware_setup(struct kvm_x86_nested_ops *nested_ops,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 8453c5b6f090c..447acda22af17 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7626,134 +7626,136 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit)
        return supported & BIT(bit);
 }

+#define KVM_X86_OP(name) .name = vmx_##name
+
 static struct kvm_x86_ops vmx_x86_ops __initdata = {
-       .hardware_unsetup = vmx_hardware_unsetup,
+       KVM_X86_OP(hardware_unsetup),

-       .hardware_enable = vmx_hardware_enable,
-       .hardware_disable = vmx_hardware_disable,
-       .cpu_has_accelerated_tpr = vmx_cpu_has_accelerated_tpr,
-       .has_emulated_msr = vmx_has_emulated_msr,
+       KVM_X86_OP(hardware_enable),
+       KVM_X86_OP(hardware_disable),
+       KVM_X86_OP(cpu_has_accelerated_tpr),
+       KVM_X86_OP(has_emulated_msr),

...