On Thu, 26 Jun 2014 18:30:16 +0100
Will Deacon <will.dea...@arm.com> 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 <g...@kernel.org>
> Cc: Paolo Bonzini <pbonz...@redhat.com>
> Cc: Marc Zyngier <marc.zyng...@arm.com>
> Cc: Christoffer Dall <christoffer.d...@linaro.org>
> Signed-off-by: Will Deacon <will.dea...@arm.com>
> ---
> 
> 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.

>  
>  /*
>   * ioctls for VM fds

> +int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type)
> +{
> +     if (type >= KVM_DEV_TYPE_MAX)
> +             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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to