Re: [PATCH 1/6 v3] KVM: x86: Change names of some of the kvm_x86_ops functions to make them more semantical and readable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This needs a changelog.

I would also split the non-x86 parts, i.e. the kvm_arch_* renames, to a
separate patch.

On Tue, Jul 28, 2020 at 12:10:45AM +0000, Krish Sadhukhan wrote:
> Suggested-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> ---
> @@ -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,

I would strongly prefer these renames to be fully recursive within a single
patch, i.e. rename svm_set_irq() as well.

Ditto for the unsetup->teardown change.

> +	.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/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4fdf303..cb6f153 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1469,15 +1469,15 @@ struct kvm_s390_ucas_mapping {
>  #define KVM_S390_GET_CMMA_BITS      _IOWR(KVMIO, 0xb8, struct kvm_s390_cmma_log)
>  #define KVM_S390_SET_CMMA_BITS      _IOW(KVMIO, 0xb9, struct kvm_s390_cmma_log)
>  /* Memory Encryption Commands */
> -#define KVM_MEMORY_ENCRYPT_OP      _IOWR(KVMIO, 0xba, unsigned long)
> +#define KVM_MEM_ENC_OP	            _IOWR(KVMIO, 0xba, unsigned long)

Renaming macros in uapi headers will break userspace.

We could do

  #define KVM_MEMORY_ENCRYPT_OP	KVM_MEM_ENC_OP

internally, but personally I think it would do more harm than good.

>  struct kvm_enc_region {
>  	__u64 addr;
>  	__u64 size;
>  };
>  
> -#define KVM_MEMORY_ENCRYPT_REG_REGION    _IOR(KVMIO, 0xbb, struct kvm_enc_region)
> -#define KVM_MEMORY_ENCRYPT_UNREG_REGION  _IOR(KVMIO, 0xbc, struct kvm_enc_region)
> +#define KVM_MEM_ENC_REGISTER_REGION    _IOR(KVMIO, 0xbb, struct kvm_enc_region)
> +#define KVM_MEM_ENC_UNREGISTER_REGION  _IOR(KVMIO, 0xbc, struct kvm_enc_region)
>  
>  /* Available with KVM_CAP_HYPERV_EVENTFD */
>  #define KVM_HYPERV_EVENTFD        _IOW(KVMIO,  0xbd, struct kvm_hyperv_eventfd)



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux