Re: [PATCH 1/2] KVM: device: add simple registration mechanism for kvm_device_ops

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

 



Il 30/06/2014 11:21, Cornelia Huck ha scritto:
On Thu, 26 Jun 2014 18:30:16 +0100
Will Deacon <will.deacon@xxxxxxx> wrote:

kvm_ioctl_create_device currently has knowledge of all the device types
and their associated ops. This is fairly inflexible when adding support
for new in-kernel device emulations, so move what we currently have out
into a table, which can support dynamic registration of ops by new
drivers for virtual hardware.

I didn't try to port all current drivers over, as it's not always clear
which initialisation hook the ops should be registered from.

I like the general idea of registering the ops dynamically, some
comments below.


Cc: Gleb Natapov <gleb@xxxxxxxxxx>
Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
Cc: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
---

Hi guys,

I've just started writing a virtual IOMMU for the ARM SMMU and figured a
registration mechanism for kvm_device_ops would be a nice cleanup for
that.

Will

 include/linux/kvm_host.h |  1 +
 include/uapi/linux/kvm.h |  1 +
 virt/kvm/kvm_main.c      | 65 ++++++++++++++++++++++++++++--------------------
 3 files changed, 40 insertions(+), 27 deletions(-)


diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index e11d8f170a62..3b368166286f 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -949,6 +949,7 @@ struct kvm_device_attr {
 #define   KVM_DEV_VFIO_GROUP_DEL			2
 #define KVM_DEV_TYPE_ARM_VGIC_V2	5
 #define KVM_DEV_TYPE_FLIC		6
+#define KVM_DEV_TYPE_MAX		7

This means we always need to move this value once we introduce a new
kvm device type. Can't you keep it in a dynamic list instead of a
table? We just need to do the lookup during device creation anyway.

There's also this wonderful thing called enum. ;)

It would let Will keep the simpler code with an array, and autogenerate KVM_DEV_TYPE_MAX.


 /*
  * ioctls for VM fds

+int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type)
+{
+	if (type >= KVM_DEV_TYPE_MAX)

... then you can make this ARRAY_SIZE, which makes the code quite nice & obvious.

Paolo

+		return -ENOSPC;
+
+	if (kvm_device_ops_table[type] != NULL)
+		return -EEXIST;

Checking for type collisions would be a bit more expensive with a list,
but I don't think it matters.

+
+	kvm_device_ops_table[type] = ops;
+	return 0;
+}


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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