linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] kvm-vfio: do not use module_init
@ 2014-09-24 12:09 Paolo Bonzini
  2014-09-24 12:14 ` Will Deacon
  2014-09-24 15:41 ` Alex Williamson
  0 siblings, 2 replies; 3+ messages in thread
From: Paolo Bonzini @ 2014-09-24 12:09 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Will Deacon, Gleb Natapov, Alex Williamson

/me got confused between the kernel and QEMU.  In the kernel, you can
only have one module_init function, and it will prevent unloading the
module unless you also have the corresponding module_exit function.

So, commit 80ce1639727e (KVM: VFIO: register kvm_device_ops dynamically,
2014-09-02) broke unloading of the kvm module, by adding a module_init
function and no module_exit.

Repair it by making kvm_vfio_ops_init weak, and checking it in
kvm_init.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Alex Williamson <Alex.Williamson@redhat.com>
Fixes: 80ce1639727e9d38729c34f162378508c307ca25
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/kvm_main.c |  4 ++++
 virt/kvm/vfio.c     |  4 ++--
 virt/kvm/vfio.h     | 13 +++++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 virt/kvm/vfio.h

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index db57363cc287..499db0977f3c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -57,6 +57,7 @@
 
 #include "coalesced_mmio.h"
 #include "async_pf.h"
+#include "vfio.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/kvm.h>
@@ -3226,6 +3227,9 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 		goto out_undebugfs;
 	}
 
+	r = kvm_vfio_ops_init();
+	WARN_ON(r);
+
 	return 0;
 
 out_undebugfs:
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index bb11b36ee8a2..281e7cf2b8e5 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -18,6 +18,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/vfio.h>
+#include "vfio.h"
 
 struct kvm_vfio_group {
 	struct list_head node;
@@ -278,8 +279,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
 	return 0;
 }
 
-static int __init kvm_vfio_ops_init(void)
+int kvm_vfio_ops_init(void)
 {
 	return kvm_register_device_ops(&kvm_vfio_ops, KVM_DEV_TYPE_VFIO);
 }
-module_init(kvm_vfio_ops_init);
diff --git a/virt/kvm/vfio.h b/virt/kvm/vfio.h
new file mode 100644
index 000000000000..92eac75d6b62
--- /dev/null
+++ b/virt/kvm/vfio.h
@@ -0,0 +1,13 @@
+#ifndef __KVM_VFIO_H
+#define __KVM_VFIO_H
+
+#ifdef CONFIG_KVM_VFIO
+int kvm_vfio_ops_init(void);
+#else
+static inline int kvm_vfio_ops_init(void)
+{
+	return 0;
+}
+#endif
+
+#endif
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] kvm-vfio: do not use module_init
  2014-09-24 12:09 [PATCH v3] kvm-vfio: do not use module_init Paolo Bonzini
@ 2014-09-24 12:14 ` Will Deacon
  2014-09-24 15:41 ` Alex Williamson
  1 sibling, 0 replies; 3+ messages in thread
From: Will Deacon @ 2014-09-24 12:14 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, Gleb Natapov, Alex Williamson

On Wed, Sep 24, 2014 at 01:09:28PM +0100, Paolo Bonzini wrote:
> /me got confused between the kernel and QEMU.  In the kernel, you can
> only have one module_init function, and it will prevent unloading the
> module unless you also have the corresponding module_exit function.
> 
> So, commit 80ce1639727e (KVM: VFIO: register kvm_device_ops dynamically,
> 2014-09-02) broke unloading of the kvm module, by adding a module_init
> function and no module_exit.
> 
> Repair it by making kvm_vfio_ops_init weak, and checking it in
> kvm_init.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Alex Williamson <Alex.Williamson@redhat.com>
> Fixes: 80ce1639727e9d38729c34f162378508c307ca25
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  virt/kvm/kvm_main.c |  4 ++++
>  virt/kvm/vfio.c     |  4 ++--
>  virt/kvm/vfio.h     | 13 +++++++++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
>  create mode 100644 virt/kvm/vfio.h

Looks good to me, thanks for fixing this:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index db57363cc287..499db0977f3c 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -57,6 +57,7 @@
>  
>  #include "coalesced_mmio.h"
>  #include "async_pf.h"
> +#include "vfio.h"
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/kvm.h>
> @@ -3226,6 +3227,9 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
>  		goto out_undebugfs;
>  	}
>  
> +	r = kvm_vfio_ops_init();
> +	WARN_ON(r);
> +
>  	return 0;
>  
>  out_undebugfs:
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index bb11b36ee8a2..281e7cf2b8e5 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/vfio.h>
> +#include "vfio.h"
>  
>  struct kvm_vfio_group {
>  	struct list_head node;
> @@ -278,8 +279,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>  	return 0;
>  }
>  
> -static int __init kvm_vfio_ops_init(void)
> +int kvm_vfio_ops_init(void)
>  {
>  	return kvm_register_device_ops(&kvm_vfio_ops, KVM_DEV_TYPE_VFIO);
>  }
> -module_init(kvm_vfio_ops_init);
> diff --git a/virt/kvm/vfio.h b/virt/kvm/vfio.h
> new file mode 100644
> index 000000000000..92eac75d6b62
> --- /dev/null
> +++ b/virt/kvm/vfio.h
> @@ -0,0 +1,13 @@
> +#ifndef __KVM_VFIO_H
> +#define __KVM_VFIO_H
> +
> +#ifdef CONFIG_KVM_VFIO
> +int kvm_vfio_ops_init(void);
> +#else
> +static inline int kvm_vfio_ops_init(void)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif
> -- 
> 1.8.3.1
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] kvm-vfio: do not use module_init
  2014-09-24 12:09 [PATCH v3] kvm-vfio: do not use module_init Paolo Bonzini
  2014-09-24 12:14 ` Will Deacon
@ 2014-09-24 15:41 ` Alex Williamson
  1 sibling, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2014-09-24 15:41 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, Will Deacon, Gleb Natapov

On Wed, 2014-09-24 at 14:09 +0200, Paolo Bonzini wrote:
> /me got confused between the kernel and QEMU.  In the kernel, you can
> only have one module_init function, and it will prevent unloading the
> module unless you also have the corresponding module_exit function.
> 
> So, commit 80ce1639727e (KVM: VFIO: register kvm_device_ops dynamically,
> 2014-09-02) broke unloading of the kvm module, by adding a module_init
> function and no module_exit.
> 
> Repair it by making kvm_vfio_ops_init weak, and checking it in
> kvm_init.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Alex Williamson <Alex.Williamson@redhat.com>
> Fixes: 80ce1639727e9d38729c34f162378508c307ca25
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  virt/kvm/kvm_main.c |  4 ++++
>  virt/kvm/vfio.c     |  4 ++--
>  virt/kvm/vfio.h     | 13 +++++++++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
>  create mode 100644 virt/kvm/vfio.h

Thanks Paolo

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index db57363cc287..499db0977f3c 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -57,6 +57,7 @@
>  
>  #include "coalesced_mmio.h"
>  #include "async_pf.h"
> +#include "vfio.h"
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/kvm.h>
> @@ -3226,6 +3227,9 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
>  		goto out_undebugfs;
>  	}
>  
> +	r = kvm_vfio_ops_init();
> +	WARN_ON(r);
> +
>  	return 0;
>  
>  out_undebugfs:
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index bb11b36ee8a2..281e7cf2b8e5 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/vfio.h>
> +#include "vfio.h"
>  
>  struct kvm_vfio_group {
>  	struct list_head node;
> @@ -278,8 +279,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>  	return 0;
>  }
>  
> -static int __init kvm_vfio_ops_init(void)
> +int kvm_vfio_ops_init(void)
>  {
>  	return kvm_register_device_ops(&kvm_vfio_ops, KVM_DEV_TYPE_VFIO);
>  }
> -module_init(kvm_vfio_ops_init);
> diff --git a/virt/kvm/vfio.h b/virt/kvm/vfio.h
> new file mode 100644
> index 000000000000..92eac75d6b62
> --- /dev/null
> +++ b/virt/kvm/vfio.h
> @@ -0,0 +1,13 @@
> +#ifndef __KVM_VFIO_H
> +#define __KVM_VFIO_H
> +
> +#ifdef CONFIG_KVM_VFIO
> +int kvm_vfio_ops_init(void);
> +#else
> +static inline int kvm_vfio_ops_init(void)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-09-24 15:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-24 12:09 [PATCH v3] kvm-vfio: do not use module_init Paolo Bonzini
2014-09-24 12:14 ` Will Deacon
2014-09-24 15:41 ` Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).