Re: [PATCH] kvm: vfio: fix unregister kvm_device_ops of vfio

[Paolo Bonzini <pbonzini@redhat.com>]

Il 09/10/2014 12:30, Wanpeng Li ha scritto:
> WARNING: CPU: 1 PID: 10358 at /root/cathy/kvm/arch/x86/kvm/../../../virt/kvm/kvm_main.c:3289 kvm_init+0x234/0x282 [kvm]()
> Modules linked in: kvm_intel(O+) kvm(O) nfsv3 nfs_acl auth_rpcgss oid_registry nfsv4 dns_resolver nfs fscache lockd sunrpc pci_stub bridge stp llc autofs4 8021q cpufreq_ondemand ipv6 joydev microcode pcspkr igb i2c_algo_bit ehci_pci ehci_hcd e1000e i2c_i801 ixgbe ptp pps_core hwmon mdio tpm_tis tpm ipmi_si ipmi_msghandler acpi_cpufreq isci libsas scsi_transport_sas button dm_mirror dm_region_hash dm_log dm_mod [last unloaded: kvm_intel]
> CPU: 1 PID: 10358 Comm: insmod Tainted: G        W  O   3.17.0-rc1 #2
> Hardware name: Intel Corporation S2600CP/S2600CP, BIOS RMLSDP.86I.00.29.D696.1311111329 11/11/2013
>  0000000000000cd9 ffff880ff08cfd18 ffffffff814a61d9 0000000000000cd9
>  0000000000000000 ffff880ff08cfd58 ffffffff810417b7 ffff880ff08cfd48
>  ffffffffa045bcac ffffffffa049c420 0000000000000040 00000000000000ff
> Call Trace:
>  [<ffffffff814a61d9>] dump_stack+0x49/0x60
>  [<ffffffff810417b7>] warn_slowpath_common+0x7c/0x96
>  [<ffffffffa045bcac>] ? kvm_init+0x234/0x282 [kvm]
>  [<ffffffff810417e6>] warn_slowpath_null+0x15/0x17
>  [<ffffffffa045bcac>] kvm_init+0x234/0x282 [kvm]
>  [<ffffffffa016e995>] vmx_init+0x1bf/0x42a [kvm_intel]
>  [<ffffffffa016e7d6>] ? vmx_check_processor_compat+0x64/0x64 [kvm_intel]
>  [<ffffffff810002ab>] do_one_initcall+0xe3/0x170
>  [<ffffffff811168a9>] ? __vunmap+0xad/0xb8
>  [<ffffffff8109c58f>] do_init_module+0x2b/0x174
>  [<ffffffff8109d414>] load_module+0x43e/0x569
>  [<ffffffff8109c6d8>] ? do_init_module+0x174/0x174
>  [<ffffffff8109c75a>] ? copy_module_from_user+0x39/0x82
>  [<ffffffff8109b7dd>] ? module_sect_show+0x20/0x20
>  [<ffffffff8109d65f>] SyS_init_module+0x54/0x81
>  [<ffffffff814a9a12>] system_call_fastpath+0x16/0x1b
> ---[ end trace 0626f4a3ddea56f3 ]---
> 
> After commit 803e163 (KVM: VFIO: register kvm_device_ops dynamically), kvm_device_ops
> of vfio can be registered dynamically. Commit 3c3c29fd (kvm-vfio: do not use module_init)
> move the dynamic register invoked by kvm_init in order to fix broke unloading of the 
> kvm module. However, kvm_device_ops of vfio is unregistered after rmmod kvm-intel module 
> which lead to device type collision detection warning after kvm-intel module reinsmod.
> 
> The bug can be reproduced by:
> rmmod kvm_intel.ko
> insmod kvm_intel.ko 
> w/o rmmod/insmod kvm.ko
> 
> This patch fix the bug by unregister kvm_device_ops of vfio dynamically during rmmod 
> kvm-intel module.
> 
> Reported-by: Liu Rongrong <rongrongx.liu@intel.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
>  include/linux/kvm_host.h |    1 +
>  virt/kvm/kvm_main.c      |    7 +++++++
>  virt/kvm/vfio.c          |    5 +++++
>  virt/kvm/vfio.h          |    4 ++++
>  4 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 28be31f..ea53b04 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1080,6 +1080,7 @@ void kvm_device_get(struct kvm_device *dev);
>  void kvm_device_put(struct kvm_device *dev);
>  struct kvm_device *kvm_device_from_filp(struct file *filp);
>  int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type);
> +void kvm_unregister_device_ops(u32 type);
>  
>  extern struct kvm_device_ops kvm_mpic_ops;
>  extern struct kvm_device_ops kvm_xics_ops;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 39a02fb..5837e93 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2354,6 +2354,12 @@ int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type)
>  	return 0;
>  }
>  
> +void kvm_unregister_device_ops(u32 type)
> +{
> +	if (kvm_device_ops_table[type] != NULL)
> +		kvm_device_ops_table[type] = NULL;
> +}
> +
>  static int kvm_ioctl_create_device(struct kvm *kvm,
>  				   struct kvm_create_device *cd)
>  {
> @@ -3328,5 +3334,6 @@ void kvm_exit(void)
>  	kvm_arch_exit();
>  	kvm_irqfd_exit();
>  	free_cpumask_var(cpus_hardware_enabled);
> +	kvm_vfio_ops_exit();
>  }
>  EXPORT_SYMBOL_GPL(kvm_exit);
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index 281e7cf..620e37f 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -283,3 +283,8 @@ int kvm_vfio_ops_init(void)
>  {
>  	return kvm_register_device_ops(&kvm_vfio_ops, KVM_DEV_TYPE_VFIO);
>  }
> +
> +void kvm_vfio_ops_exit(void)
> +{
> +	kvm_unregister_device_ops(KVM_DEV_TYPE_VFIO);
> +}
> diff --git a/virt/kvm/vfio.h b/virt/kvm/vfio.h
> index 92eac75..ab88c7d 100644
> --- a/virt/kvm/vfio.h
> +++ b/virt/kvm/vfio.h
> @@ -3,11 +3,15 @@
>  
>  #ifdef CONFIG_KVM_VFIO
>  int kvm_vfio_ops_init(void);
> +void kvm_vfio_ops_exit(void);
>  #else
>  static inline int kvm_vfio_ops_init(void)
>  {
>  	return 0;
>  }
> +static inline void kvm_vfio_ops_exit(void)
> +{
> +}
>  #endif
>  
>  #endif
> 

Thanks, applying (for now locally).

Paolo