* [PATCH] kvm-vfio: do not use module_init
@ 2014-09-24 11:29 Paolo Bonzini
2014-09-24 11:44 ` Will Deacon
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2014-09-24 11:29 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 | 6 ++++++
virt/kvm/vfio.c | 4 ++--
virt/kvm/vfio.h | 6 ++++++
3 files changed, 14 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..69f90fb7899f 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,11 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
goto out_undebugfs;
}
+ if (kvm_vfio_ops_init) {
+ 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..8f7418186d7d 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 __weak 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..f70b384d0ef8
--- /dev/null
+++ b/virt/kvm/vfio.h
@@ -0,0 +1,6 @@
+#ifndef __KVM_VFIO_H
+#define __KVM_VFIO_H
+
+int __weak kvm_vfio_ops_init(void);
+
+#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm-vfio: do not use module_init
2014-09-24 11:29 [PATCH] kvm-vfio: do not use module_init Paolo Bonzini
@ 2014-09-24 11:44 ` Will Deacon
2014-09-24 11:45 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Will Deacon @ 2014-09-24 11:44 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, Gleb Natapov, Alex Williamson
On Wed, Sep 24, 2014 at 12:29:09PM +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.
Happy for you to take the blame, but I think this one's my fault!
> 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.
I forget kvm builds as a module for other architectures (ie. not arm/arm64).
> Repair it by making kvm_vfio_ops_init weak, and checking it in
> kvm_init.
Hehe, if only there was a kconfig option for kvm-vfio.c...
Will
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm-vfio: do not use module_init
2014-09-24 11:44 ` Will Deacon
@ 2014-09-24 11:45 ` Paolo Bonzini
2014-09-24 11:48 ` Will Deacon
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2014-09-24 11:45 UTC (permalink / raw)
To: Will Deacon; +Cc: linux-kernel, kvm, Gleb Natapov, Alex Williamson
Il 24/09/2014 13:44, Will Deacon ha scritto:
> On Wed, Sep 24, 2014 at 12:29:09PM +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.
>
> Happy for you to take the blame, but I think this one's my fault!
That's why you were CCed! ;)
>> 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.
>
> I forget kvm builds as a module for other architectures (ie. not arm/arm64).
>
>> Repair it by making kvm_vfio_ops_init weak, and checking it in
>> kvm_init.
>
> Hehe, if only there was a kconfig option for kvm-vfio.c...
Yeah, I was tempted to put it back. What do you think?
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm-vfio: do not use module_init
2014-09-24 11:45 ` Paolo Bonzini
@ 2014-09-24 11:48 ` Will Deacon
0 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2014-09-24 11:48 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, Gleb Natapov, Alex Williamson
On Wed, Sep 24, 2014 at 12:45:53PM +0100, Paolo Bonzini wrote:
> Il 24/09/2014 13:44, Will Deacon ha scritto:
> > On Wed, Sep 24, 2014 at 12:29:09PM +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.
> >
> > Happy for you to take the blame, but I think this one's my fault!
>
> That's why you were CCed! ;)
>
> >> 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.
> >
> > I forget kvm builds as a module for other architectures (ie. not arm/arm64).
> >
> >> Repair it by making kvm_vfio_ops_init weak, and checking it in
> >> kvm_init.
> >
> > Hehe, if only there was a kconfig option for kvm-vfio.c...
>
> Yeah, I was tempted to put it back. What do you think?
I think it's nicer than the __weak symbol and I don't see a downside to
having the option (the reason for removing it was lack of users, which
lasted a couple of hours).
Will
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-24 11:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-24 11:29 [PATCH] kvm-vfio: do not use module_init Paolo Bonzini
2014-09-24 11:44 ` Will Deacon
2014-09-24 11:45 ` Paolo Bonzini
2014-09-24 11:48 ` Will Deacon
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).