From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Krish Sadhukhan <krish.sadhukhan@oracle.com>, kvm@vger.kernel.org
Cc: jmattson@google.com, sean.j.christopherson@intel.com,
pbonzini@redhat.com
Subject: Re: [PATCH 1/5 v2] KVM: x86: Change names of some of the kvm_x86_ops functions to make them more semantical and readable
Date: Mon, 27 Jul 2020 11:39:22 +0200 [thread overview]
Message-ID: <87wo2pmh4l.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <1595647598-53208-2-git-send-email-krish.sadhukhan@oracle.com>
Krish Sadhukhan <krish.sadhukhan@oracle.com> writes:
> Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
> arch/x86/include/asm/kvm_host.h | 12 ++++++------
> arch/x86/kvm/svm/svm.c | 12 ++++++------
> arch/x86/kvm/vmx/vmx.c | 8 ++++----
> arch/x86/kvm/x86.c | 22 +++++++++++-----------
> include/linux/kvm_host.h | 2 +-
> virt/kvm/kvm_main.c | 4 ++--
> 6 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index be5363b..ccad66d 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1080,7 +1080,7 @@ static inline u16 kvm_lapic_irq_dest_mode(bool dest_mode_logical)
> struct kvm_x86_ops {
> int (*hardware_enable)(void);
> void (*hardware_disable)(void);
> - void (*hardware_unsetup)(void);
> + void (*hardware_teardown)(void);
I definitely welcome the change but we may want to fix other arches as
well:
git grep hardware_unsetup HEAD
HEAD:arch/arm64/include/asm/kvm_host.h:static inline void kvm_arch_hardware_unsetup(void) {}
HEAD:arch/mips/include/asm/kvm_host.h:static inline void kvm_arch_hardware_unsetup(void) {}
HEAD:arch/powerpc/include/asm/kvm_host.h:static inline void kvm_arch_hardware_unsetup(void) {}
HEAD:arch/s390/kvm/kvm-s390.c:void kvm_arch_hardware_unsetup(void)
> bool (*cpu_has_accelerated_tpr)(void);
> bool (*has_emulated_msr)(u32 index);
> void (*cpuid_update)(struct kvm_vcpu *vcpu);
> @@ -1141,7 +1141,7 @@ struct kvm_x86_ops {
> */
> void (*tlb_flush_guest)(struct kvm_vcpu *vcpu);
>
> - enum exit_fastpath_completion (*run)(struct kvm_vcpu *vcpu);
> + enum exit_fastpath_completion (*vcpu_run)(struct kvm_vcpu *vcpu);
> int (*handle_exit)(struct kvm_vcpu *vcpu,
> enum exit_fastpath_completion exit_fastpath);
> int (*skip_emulated_instruction)(struct kvm_vcpu *vcpu);
> @@ -1150,8 +1150,8 @@ struct kvm_x86_ops {
> u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu);
> void (*patch_hypercall)(struct kvm_vcpu *vcpu,
> unsigned char *hypercall_addr);
> - void (*set_irq)(struct kvm_vcpu *vcpu);
> - void (*set_nmi)(struct kvm_vcpu *vcpu);
> + void (*inject_irq)(struct kvm_vcpu *vcpu);
> + void (*inject_nmi)(struct kvm_vcpu *vcpu);
> void (*queue_exception)(struct kvm_vcpu *vcpu);
> void (*cancel_injection)(struct kvm_vcpu *vcpu);
> int (*interrupt_allowed)(struct kvm_vcpu *vcpu, bool for_injection);
> @@ -1258,8 +1258,8 @@ struct kvm_x86_ops {
> void (*enable_smi_window)(struct kvm_vcpu *vcpu);
>
> int (*mem_enc_op)(struct kvm *kvm, void __user *argp);
> - int (*mem_enc_reg_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> - int (*mem_enc_unreg_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> + int (*mem_enc_register_region)(struct kvm *kvm, struct kvm_enc_region *argp);
> + int (*mem_enc_unregister_region)(struct kvm *kvm, struct kvm_enc_region *argp);
These two should probably pair with
KVM_MEMORY_ENCRYPT_REG_REGION
KVM_MEMORY_ENCRYPT_UNREG_REGION
shortening "memory" as "mem" and "encrypt" as "enc" is probably OK
because I can't think of any other meaning but shortening "register" as
"reg" may be percieved as CPU register.
What if we change ioctls too:
KVM_MEM_ENC_REGISTER_REGION
KVM_MEM_ENC_UNREGISTER_REGION
to make this all consistent?
>
> int (*get_msr_feature)(struct kvm_msr_entry *entry);
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index c0da4dd..24755eb 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3969,7 +3969,7 @@ static int svm_vm_init(struct kvm *kvm)
> }
>
> static struct kvm_x86_ops svm_x86_ops __initdata = {
> - .hardware_unsetup = svm_hardware_teardown,
> + .hardware_teardown = svm_hardware_teardown,
> .hardware_enable = svm_hardware_enable,
> .hardware_disable = svm_hardware_disable,
> .cpu_has_accelerated_tpr = svm_cpu_has_accelerated_tpr,
> @@ -4016,15 +4016,15 @@ static int svm_vm_init(struct kvm *kvm)
> .tlb_flush_gva = svm_flush_tlb_gva,
> .tlb_flush_guest = svm_flush_tlb,
>
> - .run = svm_vcpu_run,
> + .vcpu_run = svm_vcpu_run,
> .handle_exit = handle_exit,
> .skip_emulated_instruction = skip_emulated_instruction,
> .update_emulated_instruction = NULL,
> .set_interrupt_shadow = svm_set_interrupt_shadow,
> .get_interrupt_shadow = svm_get_interrupt_shadow,
> .patch_hypercall = svm_patch_hypercall,
> - .set_irq = svm_set_irq,
> - .set_nmi = svm_inject_nmi,
> + .inject_irq = svm_set_irq,
> + .inject_nmi = svm_inject_nmi,
> .queue_exception = svm_queue_exception,
> .cancel_injection = svm_cancel_injection,
> .interrupt_allowed = svm_interrupt_allowed,
> @@ -4080,8 +4080,8 @@ static int svm_vm_init(struct kvm *kvm)
> .enable_smi_window = enable_smi_window,
>
> .mem_enc_op = svm_mem_enc_op,
> - .mem_enc_reg_region = svm_register_enc_region,
> - .mem_enc_unreg_region = svm_unregister_enc_region,
> + .mem_enc_register_region = svm_register_enc_region,
> + .mem_enc_unregister_region = svm_unregister_enc_region,
>
> .need_emulation_on_page_fault = svm_need_emulation_on_page_fault,
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index cb22f33..90d91524 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7844,7 +7844,7 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit)
> }
>
> static struct kvm_x86_ops vmx_x86_ops __initdata = {
> - .hardware_unsetup = hardware_unsetup,
> + .hardware_teardown = hardware_unsetup,
>
> .hardware_enable = hardware_enable,
> .hardware_disable = hardware_disable,
> @@ -7889,15 +7889,15 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit)
> .tlb_flush_gva = vmx_flush_tlb_gva,
> .tlb_flush_guest = vmx_flush_tlb_guest,
>
> - .run = vmx_vcpu_run,
> + .vcpu_run = vmx_vcpu_run,
> .handle_exit = vmx_handle_exit,
> .skip_emulated_instruction = vmx_skip_emulated_instruction,
> .update_emulated_instruction = vmx_update_emulated_instruction,
> .set_interrupt_shadow = vmx_set_interrupt_shadow,
> .get_interrupt_shadow = vmx_get_interrupt_shadow,
> .patch_hypercall = vmx_patch_hypercall,
> - .set_irq = vmx_inject_irq,
> - .set_nmi = vmx_inject_nmi,
> + .inject_irq = vmx_inject_irq,
> + .inject_nmi = vmx_inject_nmi,
> .queue_exception = vmx_queue_exception,
> .cancel_injection = vmx_cancel_injection,
> .interrupt_allowed = vmx_interrupt_allowed,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 3b92db4..e850fb3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5270,8 +5270,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> goto out;
>
> r = -ENOTTY;
> - if (kvm_x86_ops.mem_enc_reg_region)
> - r = kvm_x86_ops.mem_enc_reg_region(kvm, ®ion);
> + if (kvm_x86_ops.mem_enc_register_region)
> + r = kvm_x86_ops.mem_enc_register_region(kvm, ®ion);
> break;
> }
> case KVM_MEMORY_ENCRYPT_UNREG_REGION: {
> @@ -5282,8 +5282,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> goto out;
>
> r = -ENOTTY;
> - if (kvm_x86_ops.mem_enc_unreg_region)
> - r = kvm_x86_ops.mem_enc_unreg_region(kvm, ®ion);
> + if (kvm_x86_ops.mem_enc_unregister_region)
> + r = kvm_x86_ops.mem_enc_unregister_region(kvm, ®ion);
> break;
> }
> case KVM_HYPERV_EVENTFD: {
> @@ -7788,10 +7788,10 @@ static void inject_pending_event(struct kvm_vcpu *vcpu, bool *req_immediate_exit
> */
> else if (!vcpu->arch.exception.pending) {
> if (vcpu->arch.nmi_injected) {
> - kvm_x86_ops.set_nmi(vcpu);
> + kvm_x86_ops.inject_nmi(vcpu);
> can_inject = false;
> } else if (vcpu->arch.interrupt.injected) {
> - kvm_x86_ops.set_irq(vcpu);
> + kvm_x86_ops.inject_irq(vcpu);
> can_inject = false;
> }
> }
> @@ -7867,7 +7867,7 @@ static void inject_pending_event(struct kvm_vcpu *vcpu, bool *req_immediate_exit
> if (r) {
> --vcpu->arch.nmi_pending;
> vcpu->arch.nmi_injected = true;
> - kvm_x86_ops.set_nmi(vcpu);
> + kvm_x86_ops.inject_nmi(vcpu);
> can_inject = false;
> WARN_ON(kvm_x86_ops.nmi_allowed(vcpu, true) < 0);
> }
> @@ -7881,7 +7881,7 @@ static void inject_pending_event(struct kvm_vcpu *vcpu, bool *req_immediate_exit
> goto busy;
> if (r) {
> kvm_queue_interrupt(vcpu, kvm_cpu_get_interrupt(vcpu), false);
> - kvm_x86_ops.set_irq(vcpu);
> + kvm_x86_ops.inject_irq(vcpu);
> WARN_ON(kvm_x86_ops.interrupt_allowed(vcpu, true) < 0);
> }
> if (kvm_cpu_has_injectable_intr(vcpu))
> @@ -8517,7 +8517,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> vcpu->arch.switch_db_regs &= ~KVM_DEBUGREG_RELOAD;
> }
>
> - exit_fastpath = kvm_x86_ops.run(vcpu);
> + exit_fastpath = kvm_x86_ops.vcpu_run(vcpu);
>
> /*
> * Do this here before restoring debug registers on the host. And
> @@ -9793,9 +9793,9 @@ int kvm_arch_hardware_setup(void *opaque)
> return 0;
> }
>
> -void kvm_arch_hardware_unsetup(void)
> +void kvm_arch_hardware_teardown(void)
> {
> - kvm_x86_ops.hardware_unsetup();
> + kvm_x86_ops.hardware_teardown();
> }
>
> int kvm_arch_check_processor_compat(void *opaque)
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index d564855..b49312c 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -894,7 +894,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> int kvm_arch_hardware_enable(void);
> void kvm_arch_hardware_disable(void);
> int kvm_arch_hardware_setup(void *opaque);
> -void kvm_arch_hardware_unsetup(void);
> +void kvm_arch_hardware_teardown(void);
> int kvm_arch_check_processor_compat(void *opaque);
> int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
> bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index a852af5..4625f3a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4786,7 +4786,7 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
> unregister_reboot_notifier(&kvm_reboot_notifier);
> cpuhp_remove_state_nocalls(CPUHP_AP_KVM_STARTING);
> out_free_2:
> - kvm_arch_hardware_unsetup();
> + kvm_arch_hardware_teardown();
> out_free_1:
> free_cpumask_var(cpus_hardware_enabled);
> out_free_0:
> @@ -4808,7 +4808,7 @@ void kvm_exit(void)
> unregister_reboot_notifier(&kvm_reboot_notifier);
> cpuhp_remove_state_nocalls(CPUHP_AP_KVM_STARTING);
> on_each_cpu(hardware_disable_nolock, NULL, 1);
> - kvm_arch_hardware_unsetup();
> + kvm_arch_hardware_teardown();
And this probably means we'll have to take care of non-x86 arches after
all.
> kvm_arch_exit();
> kvm_irqfd_exit();
> free_cpumask_var(cpus_hardware_enabled);
--
Vitaly
next prev parent reply other threads:[~2020-07-27 9:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-25 3:26 [PATCH 0/5 v2] KVM: x86: Fill in conforming {vmx|svm}_x86_ops and {vmx|svm}_nested_ops via macros Krish Sadhukhan
2020-07-25 3:26 ` [PATCH 1/5 v2] KVM: x86: Change names of some of the kvm_x86_ops functions to make them more semantical and readable Krish Sadhukhan
2020-07-25 18:41 ` kernel test robot
2020-07-25 18:41 ` kernel test robot
2020-07-27 9:39 ` Vitaly Kuznetsov [this message]
2020-07-27 17:30 ` Krish Sadhukhan
2020-07-25 3:26 ` [PATCH 2/5 v2] KVM: SVM: Fill in conforming svm_x86_ops via macro Krish Sadhukhan
2020-07-25 3:26 ` [PATCH 3/5 v2] KVM: nSVM: Fill in conforming svm_nested_ops " Krish Sadhukhan
2020-07-25 3:26 ` [PATCH 4/5 v2] KVM: VMX: Fill in conforming vmx_x86_ops " Krish Sadhukhan
2020-07-25 3:26 ` [PATCH 5/5 v2] KVM: nVMX: Fill in conforming vmx_nested_ops " Krish Sadhukhan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wo2pmh4l.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=jmattson@google.com \
--cc=krish.sadhukhan@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=sean.j.christopherson@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.