| Message ID | 20200321202603.19355-3-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Move x86 init ops to separate struct | expand |
Sean Christopherson <sean.j.christopherson@intel.com> writes: > Move the kvm_x86_ops functions that are used only within the scope of > kvm_init() into a separate struct, kvm_x86_init_ops. In addition to > identifying the init-only functions without restorting to code comments, > this also sets the stage for waiting until after ->hardware_setup() to > set kvm_x86_ops. Setting kvm_x86_ops after ->hardware_setup() is > desirable as many of the hooks are not usable until ->hardware_setup() > completes. > > No functional change intended. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 13 +++++++++---- > arch/x86/kvm/svm.c | 15 ++++++++++----- > arch/x86/kvm/vmx/vmx.c | 16 +++++++++++----- > arch/x86/kvm/x86.c | 10 ++++++---- > 4 files changed, 36 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 9a183e9d4cb1..f4c5b49299ff 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1054,12 +1054,8 @@ static inline u16 kvm_lapic_irq_dest_mode(bool dest_mode_logical) > } > > struct kvm_x86_ops { > - int (*cpu_has_kvm_support)(void); /* __init */ > - int (*disabled_by_bios)(void); /* __init */ > int (*hardware_enable)(void); > void (*hardware_disable)(void); > - int (*check_processor_compatibility)(void);/* __init */ > - int (*hardware_setup)(void); /* __init */ > void (*hardware_unsetup)(void); /* __exit */ > bool (*cpu_has_accelerated_tpr)(void); > bool (*has_emulated_msr)(int index); > @@ -1260,6 +1256,15 @@ struct kvm_x86_ops { > int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu); > }; > > +struct kvm_x86_init_ops { > + int (*cpu_has_kvm_support)(void); > + int (*disabled_by_bios)(void); > + int (*check_processor_compatibility)(void); > + int (*hardware_setup)(void); > + > + struct kvm_x86_ops *runtime_ops; > +}; > + > struct kvm_arch_async_pf { > u32 token; > gfn_t gfn; > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 2125c6ae5951..33e67c3389c2 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -7351,11 +7351,7 @@ static void svm_pre_update_apicv_exec_ctrl(struct kvm *kvm, bool activate) > } > > static struct kvm_x86_ops svm_x86_ops __ro_after_init = { > - .cpu_has_kvm_support = has_svm, > - .disabled_by_bios = is_disabled, > - .hardware_setup = svm_hardware_setup, > .hardware_unsetup = svm_hardware_teardown, > - .check_processor_compatibility = svm_check_processor_compat, > .hardware_enable = svm_hardware_enable, > .hardware_disable = svm_hardware_disable, > .cpu_has_accelerated_tpr = svm_cpu_has_accelerated_tpr, > @@ -7480,9 +7476,18 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = { > .check_nested_events = svm_check_nested_events, > }; > > +static struct kvm_x86_init_ops svm_init_ops __initdata = { > + .cpu_has_kvm_support = has_svm, > + .disabled_by_bios = is_disabled, > + .hardware_setup = svm_hardware_setup, > + .check_processor_compatibility = svm_check_processor_compat, > + > + .runtime_ops = &svm_x86_ops, > +}; Unrelated to your patch but I think we can make the naming of some of these functions more consistend on SVM/VMX, in particular I'd suggest has_svm() -> cpu_has_svm_support() is_disabled -> svm_disabled_by_bios() ... (see below for VMX) > + > static int __init svm_init(void) > { > - return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm), > + return kvm_init(&svm_init_ops, sizeof(struct vcpu_svm), > __alignof__(struct vcpu_svm), THIS_MODULE); > } > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 07299a957d4a..ffcdcc86f5b7 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -7842,11 +7842,8 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit) > } > > static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > - .cpu_has_kvm_support = cpu_has_kvm_support, > - .disabled_by_bios = vmx_disabled_by_bios, > - .hardware_setup = hardware_setup, > .hardware_unsetup = hardware_unsetup, > - .check_processor_compatibility = vmx_check_processor_compat, > + > .hardware_enable = hardware_enable, > .hardware_disable = hardware_disable, > .cpu_has_accelerated_tpr = report_flexpriority, > @@ -7981,6 +7978,15 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > .apic_init_signal_blocked = vmx_apic_init_signal_blocked, > }; > > +static struct kvm_x86_init_ops vmx_init_ops __initdata = { > + .cpu_has_kvm_support = cpu_has_kvm_support, > + .disabled_by_bios = vmx_disabled_by_bios, > + .check_processor_compatibility = vmx_check_processor_compat, > + .hardware_setup = hardware_setup, cpu_has_kvm_support() -> cpu_has_vmx_support() hardware_setup() -> vmx_hardware_setup() > + > + .runtime_ops = &vmx_x86_ops, > +}; > + > static void vmx_cleanup_l1d_flush(void) > { > if (vmx_l1d_flush_pages) { > @@ -8065,7 +8071,7 @@ static int __init vmx_init(void) > } > #endif > > - r = kvm_init(&vmx_x86_ops, sizeof(struct vcpu_vmx), > + r = kvm_init(&vmx_init_ops, sizeof(struct vcpu_vmx), > __alignof__(struct vcpu_vmx), THIS_MODULE); > if (r) > return r; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 0f08e1b4e762..20f989d1bba8 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -7298,8 +7298,8 @@ static struct notifier_block pvclock_gtod_notifier = { > > int kvm_arch_init(void *opaque) > { > + struct kvm_x86_init_ops *ops = opaque; > int r; > - struct kvm_x86_ops *ops = opaque; > > if (kvm_x86_ops) { > printk(KERN_ERR "kvm: already loaded the other module\n"); > @@ -7354,7 +7354,7 @@ int kvm_arch_init(void *opaque) > if (r) > goto out_free_percpu; > > - kvm_x86_ops = ops; > + kvm_x86_ops = ops->runtime_ops; > > kvm_mmu_set_mask_ptes(PT_USER_MASK, PT_ACCESSED_MASK, > PT_DIRTY_MASK, PT64_NX_MASK, 0, > @@ -9623,6 +9623,7 @@ void kvm_arch_hardware_disable(void) > > int kvm_arch_hardware_setup(void *opaque) > { > + struct kvm_x86_init_ops *ops = opaque; > int r; > > rdmsrl_safe(MSR_EFER, &host_efer); > @@ -9630,7 +9631,7 @@ int kvm_arch_hardware_setup(void *opaque) > if (boot_cpu_has(X86_FEATURE_XSAVES)) > rdmsrl(MSR_IA32_XSS, host_xss); > > - r = kvm_x86_ops->hardware_setup(); > + r = ops->hardware_setup(); > if (r != 0) > return r; > > @@ -9665,13 +9666,14 @@ void kvm_arch_hardware_unsetup(void) > int kvm_arch_check_processor_compat(void *opaque) > { > struct cpuinfo_x86 *c = &cpu_data(smp_processor_id()); > + struct kvm_x86_init_ops *ops = opaque; > > WARN_ON(!irqs_disabled()); > > if (kvm_host_cr4_reserved_bits(c) != cr4_reserved_bits) > return -EIO; > > - return kvm_x86_ops->check_processor_compatibility(); > + return ops->check_processor_compatibility(); > } > > bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu) The patch itself looks good, Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
On Mon, Mar 23, 2020 at 01:10:40PM +0100, Vitaly Kuznetsov wrote: > Sean Christopherson <sean.j.christopherson@intel.com> writes: > > > + > > + .runtime_ops = &svm_x86_ops, > > +}; > > Unrelated to your patch but I think we can make the naming of some of > these functions more consistend on SVM/VMX, in particular I'd suggest > > has_svm() -> cpu_has_svm_support() > is_disabled -> svm_disabled_by_bios() > ... > (see below for VMX) > > > + > > static int __init svm_init(void) > > { > > - return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm), > > + return kvm_init(&svm_init_ops, sizeof(struct vcpu_svm), > > __alignof__(struct vcpu_svm), THIS_MODULE); > > } > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > index 07299a957d4a..ffcdcc86f5b7 100644 > > --- a/arch/x86/kvm/vmx/vmx.c > > +++ b/arch/x86/kvm/vmx/vmx.c > > @@ -7842,11 +7842,8 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit) > > } > > > > static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > > - .cpu_has_kvm_support = cpu_has_kvm_support, > > - .disabled_by_bios = vmx_disabled_by_bios, > > - .hardware_setup = hardware_setup, > > .hardware_unsetup = hardware_unsetup, > > - .check_processor_compatibility = vmx_check_processor_compat, > > + > > .hardware_enable = hardware_enable, > > .hardware_disable = hardware_disable, > > .cpu_has_accelerated_tpr = report_flexpriority, > > @@ -7981,6 +7978,15 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > > .apic_init_signal_blocked = vmx_apic_init_signal_blocked, > > }; > > > > +static struct kvm_x86_init_ops vmx_init_ops __initdata = { > > + .cpu_has_kvm_support = cpu_has_kvm_support, > > + .disabled_by_bios = vmx_disabled_by_bios, > > + .check_processor_compatibility = vmx_check_processor_compat, > > + .hardware_setup = hardware_setup, > > cpu_has_kvm_support() -> cpu_has_vmx_support() > hardware_setup() -> vmx_hardware_setup() Preaching to the choir on this one. The VMX functions without prefixes in in particular annoy me to no end, e.g. hardware_setup(). Though the worst is probably ".vcpu_create = vmx_create_vcpu", if I had a nickel for every time I've tried to find vmx_vcpu_create()... What if we added a macro to auto-generate the common/required hooks? E.g.: static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { MANDATORY_KVM_X86_OPS(vmx), .pmu_ops = &intel_pmu_ops, ... }; That'd enforce consistent naming, and would provide a bit of documentation as to which hooks are optional, e.g. many of the nested hooks, and which must be defined for KVM to function.
Sean Christopherson <sean.j.christopherson@intel.com> writes: > On Mon, Mar 23, 2020 at 01:10:40PM +0100, Vitaly Kuznetsov wrote: >> Sean Christopherson <sean.j.christopherson@intel.com> writes: >> >> > + >> > + .runtime_ops = &svm_x86_ops, >> > +}; >> >> Unrelated to your patch but I think we can make the naming of some of >> these functions more consistend on SVM/VMX, in particular I'd suggest >> >> has_svm() -> cpu_has_svm_support() >> is_disabled -> svm_disabled_by_bios() >> ... >> (see below for VMX) >> >> > + >> > static int __init svm_init(void) >> > { >> > - return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm), >> > + return kvm_init(&svm_init_ops, sizeof(struct vcpu_svm), >> > __alignof__(struct vcpu_svm), THIS_MODULE); >> > } >> > >> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >> > index 07299a957d4a..ffcdcc86f5b7 100644 >> > --- a/arch/x86/kvm/vmx/vmx.c >> > +++ b/arch/x86/kvm/vmx/vmx.c >> > @@ -7842,11 +7842,8 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit) >> > } >> > >> > static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { >> > - .cpu_has_kvm_support = cpu_has_kvm_support, >> > - .disabled_by_bios = vmx_disabled_by_bios, >> > - .hardware_setup = hardware_setup, >> > .hardware_unsetup = hardware_unsetup, >> > - .check_processor_compatibility = vmx_check_processor_compat, >> > + >> > .hardware_enable = hardware_enable, >> > .hardware_disable = hardware_disable, >> > .cpu_has_accelerated_tpr = report_flexpriority, >> > @@ -7981,6 +7978,15 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { >> > .apic_init_signal_blocked = vmx_apic_init_signal_blocked, >> > }; >> > >> > +static struct kvm_x86_init_ops vmx_init_ops __initdata = { >> > + .cpu_has_kvm_support = cpu_has_kvm_support, >> > + .disabled_by_bios = vmx_disabled_by_bios, >> > + .check_processor_compatibility = vmx_check_processor_compat, >> > + .hardware_setup = hardware_setup, >> >> cpu_has_kvm_support() -> cpu_has_vmx_support() >> hardware_setup() -> vmx_hardware_setup() > > Preaching to the choir on this one. The VMX functions without prefixes in > in particular annoy me to no end, e.g. hardware_setup(). Though the worst > is probably ".vcpu_create = vmx_create_vcpu", if I had a nickel for every > time I've tried to find vmx_vcpu_create()... > > What if we added a macro to auto-generate the common/required hooks? E.g.: > > static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > MANDATORY_KVM_X86_OPS(vmx), > > .pmu_ops = &intel_pmu_ops, > > ... > }; > > That'd enforce consistent naming, and would provide a bit of documentation > as to which hooks are optional, e.g. many of the nested hooks, and which > must be defined for KVM to function. Sounds cool! (not sure that with only two implementations people won't call it 'over-engineered' but cool). My personal wish would just be that function names in function implementations are not auto-generated so e.g. a simple 'git grep vmx_hardware_setup' works but the way how we fill vmx_x86_ops in can be macroed I guess.
On Mon, Mar 23, 2020 at 05:24:56PM +0100, Vitaly Kuznetsov wrote: > Sean Christopherson <sean.j.christopherson@intel.com> writes: > > > On Mon, Mar 23, 2020 at 01:10:40PM +0100, Vitaly Kuznetsov wrote: > >> Sean Christopherson <sean.j.christopherson@intel.com> writes: > >> > >> > + > >> > + .runtime_ops = &svm_x86_ops, > >> > +}; > >> > >> Unrelated to your patch but I think we can make the naming of some of > >> these functions more consistend on SVM/VMX, in particular I'd suggest > >> > >> has_svm() -> cpu_has_svm_support() > >> is_disabled -> svm_disabled_by_bios() > >> ... > >> (see below for VMX) > >> > >> > + > >> > static int __init svm_init(void) > >> > { > >> > - return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm), > >> > + return kvm_init(&svm_init_ops, sizeof(struct vcpu_svm), > >> > __alignof__(struct vcpu_svm), THIS_MODULE); > >> > } > >> > > >> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > >> > index 07299a957d4a..ffcdcc86f5b7 100644 > >> > --- a/arch/x86/kvm/vmx/vmx.c > >> > +++ b/arch/x86/kvm/vmx/vmx.c > >> > @@ -7842,11 +7842,8 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit) > >> > } > >> > > >> > static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > >> > - .cpu_has_kvm_support = cpu_has_kvm_support, > >> > - .disabled_by_bios = vmx_disabled_by_bios, > >> > - .hardware_setup = hardware_setup, > >> > .hardware_unsetup = hardware_unsetup, > >> > - .check_processor_compatibility = vmx_check_processor_compat, > >> > + > >> > .hardware_enable = hardware_enable, > >> > .hardware_disable = hardware_disable, > >> > .cpu_has_accelerated_tpr = report_flexpriority, > >> > @@ -7981,6 +7978,15 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > >> > .apic_init_signal_blocked = vmx_apic_init_signal_blocked, > >> > }; > >> > > >> > +static struct kvm_x86_init_ops vmx_init_ops __initdata = { > >> > + .cpu_has_kvm_support = cpu_has_kvm_support, > >> > + .disabled_by_bios = vmx_disabled_by_bios, > >> > + .check_processor_compatibility = vmx_check_processor_compat, > >> > + .hardware_setup = hardware_setup, > >> > >> cpu_has_kvm_support() -> cpu_has_vmx_support() > >> hardware_setup() -> vmx_hardware_setup() > > > > Preaching to the choir on this one. The VMX functions without prefixes in > > in particular annoy me to no end, e.g. hardware_setup(). Though the worst > > is probably ".vcpu_create = vmx_create_vcpu", if I had a nickel for every > > time I've tried to find vmx_vcpu_create()... > > > > What if we added a macro to auto-generate the common/required hooks? E.g.: > > > > static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { > > MANDATORY_KVM_X86_OPS(vmx), > > > > .pmu_ops = &intel_pmu_ops, > > > > ... > > }; > > > > That'd enforce consistent naming, and would provide a bit of documentation > > as to which hooks are optional, e.g. many of the nested hooks, and which > > must be defined for KVM to function. > > Sounds cool! (not sure that with only two implementations people won't > call it 'over-engineered' but cool). My personal wish would just be that > function names in function implementations are not auto-generated so > e.g. a simple 'git grep vmx_hardware_setup' works but the way how we > fill vmx_x86_ops in can be macroed I guess. Ya, I was thinking of just the macro. Even that has downsides though, e.g. chasing kvm_x86_ops.hardware_setup() to find VMX's hardware_setup() becomes a bit kludgy. On the other hand, _if_ you know how the fill macro works, getting to the implementation should be easier.
On 23/03/20 17:24, Vitaly Kuznetsov wrote: > Sounds cool! (not sure that with only two implementations people won't > call it 'over-engineered' but cool). Yes, something like #define KVM_X86_OP(name) .name = vmx_##name (svm_##name for svm.c) and then KVM_X86_OP(check_nested_events) etc. > My personal wish would just be that > function names in function implementations are not auto-generated so > e.g. a simple 'git grep vmx_hardware_setup' works Yes, absolutely; the function names would still be written by hand. Paolo > but the way how we > fill vmx_x86_ops in can be macroed I guess.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 9a183e9d4cb1..f4c5b49299ff 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1054,12 +1054,8 @@ static inline u16 kvm_lapic_irq_dest_mode(bool dest_mode_logical) } struct kvm_x86_ops { - int (*cpu_has_kvm_support)(void); /* __init */ - int (*disabled_by_bios)(void); /* __init */ int (*hardware_enable)(void); void (*hardware_disable)(void); - int (*check_processor_compatibility)(void);/* __init */ - int (*hardware_setup)(void); /* __init */ void (*hardware_unsetup)(void); /* __exit */ bool (*cpu_has_accelerated_tpr)(void); bool (*has_emulated_msr)(int index); @@ -1260,6 +1256,15 @@ struct kvm_x86_ops { int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu); }; +struct kvm_x86_init_ops { + int (*cpu_has_kvm_support)(void); + int (*disabled_by_bios)(void); + int (*check_processor_compatibility)(void); + int (*hardware_setup)(void); + + struct kvm_x86_ops *runtime_ops; +}; + struct kvm_arch_async_pf { u32 token; gfn_t gfn; diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 2125c6ae5951..33e67c3389c2 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -7351,11 +7351,7 @@ static void svm_pre_update_apicv_exec_ctrl(struct kvm *kvm, bool activate) } static struct kvm_x86_ops svm_x86_ops __ro_after_init = { - .cpu_has_kvm_support = has_svm, - .disabled_by_bios = is_disabled, - .hardware_setup = svm_hardware_setup, .hardware_unsetup = svm_hardware_teardown, - .check_processor_compatibility = svm_check_processor_compat, .hardware_enable = svm_hardware_enable, .hardware_disable = svm_hardware_disable, .cpu_has_accelerated_tpr = svm_cpu_has_accelerated_tpr, @@ -7480,9 +7476,18 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = { .check_nested_events = svm_check_nested_events, }; +static struct kvm_x86_init_ops svm_init_ops __initdata = { + .cpu_has_kvm_support = has_svm, + .disabled_by_bios = is_disabled, + .hardware_setup = svm_hardware_setup, + .check_processor_compatibility = svm_check_processor_compat, + + .runtime_ops = &svm_x86_ops, +}; + static int __init svm_init(void) { - return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm), + return kvm_init(&svm_init_ops, sizeof(struct vcpu_svm), __alignof__(struct vcpu_svm), THIS_MODULE); } diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 07299a957d4a..ffcdcc86f5b7 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7842,11 +7842,8 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit) } static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { - .cpu_has_kvm_support = cpu_has_kvm_support, - .disabled_by_bios = vmx_disabled_by_bios, - .hardware_setup = hardware_setup, .hardware_unsetup = hardware_unsetup, - .check_processor_compatibility = vmx_check_processor_compat, + .hardware_enable = hardware_enable, .hardware_disable = hardware_disable, .cpu_has_accelerated_tpr = report_flexpriority, @@ -7981,6 +7978,15 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { .apic_init_signal_blocked = vmx_apic_init_signal_blocked, }; +static struct kvm_x86_init_ops vmx_init_ops __initdata = { + .cpu_has_kvm_support = cpu_has_kvm_support, + .disabled_by_bios = vmx_disabled_by_bios, + .check_processor_compatibility = vmx_check_processor_compat, + .hardware_setup = hardware_setup, + + .runtime_ops = &vmx_x86_ops, +}; + static void vmx_cleanup_l1d_flush(void) { if (vmx_l1d_flush_pages) { @@ -8065,7 +8071,7 @@ static int __init vmx_init(void) } #endif - r = kvm_init(&vmx_x86_ops, sizeof(struct vcpu_vmx), + r = kvm_init(&vmx_init_ops, sizeof(struct vcpu_vmx), __alignof__(struct vcpu_vmx), THIS_MODULE); if (r) return r; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 0f08e1b4e762..20f989d1bba8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7298,8 +7298,8 @@ static struct notifier_block pvclock_gtod_notifier = { int kvm_arch_init(void *opaque) { + struct kvm_x86_init_ops *ops = opaque; int r; - struct kvm_x86_ops *ops = opaque; if (kvm_x86_ops) { printk(KERN_ERR "kvm: already loaded the other module\n"); @@ -7354,7 +7354,7 @@ int kvm_arch_init(void *opaque) if (r) goto out_free_percpu; - kvm_x86_ops = ops; + kvm_x86_ops = ops->runtime_ops; kvm_mmu_set_mask_ptes(PT_USER_MASK, PT_ACCESSED_MASK, PT_DIRTY_MASK, PT64_NX_MASK, 0, @@ -9623,6 +9623,7 @@ void kvm_arch_hardware_disable(void) int kvm_arch_hardware_setup(void *opaque) { + struct kvm_x86_init_ops *ops = opaque; int r; rdmsrl_safe(MSR_EFER, &host_efer); @@ -9630,7 +9631,7 @@ int kvm_arch_hardware_setup(void *opaque) if (boot_cpu_has(X86_FEATURE_XSAVES)) rdmsrl(MSR_IA32_XSS, host_xss); - r = kvm_x86_ops->hardware_setup(); + r = ops->hardware_setup(); if (r != 0) return r; @@ -9665,13 +9666,14 @@ void kvm_arch_hardware_unsetup(void) int kvm_arch_check_processor_compat(void *opaque) { struct cpuinfo_x86 *c = &cpu_data(smp_processor_id()); + struct kvm_x86_init_ops *ops = opaque; WARN_ON(!irqs_disabled()); if (kvm_host_cr4_reserved_bits(c) != cr4_reserved_bits) return -EIO; - return kvm_x86_ops->check_processor_compatibility(); + return ops->check_processor_compatibility(); } bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu)
Move the kvm_x86_ops functions that are used only within the scope of kvm_init() into a separate struct, kvm_x86_init_ops. In addition to identifying the init-only functions without restorting to code comments, this also sets the stage for waiting until after ->hardware_setup() to set kvm_x86_ops. Setting kvm_x86_ops after ->hardware_setup() is desirable as many of the hooks are not usable until ->hardware_setup() completes. No functional change intended. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/include/asm/kvm_host.h | 13 +++++++++---- arch/x86/kvm/svm.c | 15 ++++++++++----- arch/x86/kvm/vmx/vmx.c | 16 +++++++++++----- arch/x86/kvm/x86.c | 10 ++++++---- 4 files changed, 36 insertions(+), 18 deletions(-)