On Tue, 22 Nov 2016 14:09:33 +0800
Jike Song <jike.song@xxxxxxxxx> wrote:
> Sometimes users need to be aware when a vfio_group attaches to a
> KVM or detaches from it. KVM already calls get/put method from vfio to
> manipulate the vfio_group reference, it can notify vfio_group in
> a similar way.
>
> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Cc: Kirti Wankhede <kwankhede@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
> Signed-off-by: Jike Song <jike.song@xxxxxxxxx>
> ---
> virt/kvm/vfio.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index 1dd087d..ea5b4bb 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -60,6 +60,32 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
> symbol_put(vfio_group_put_external_user);
> }
>
> +static void kvm_vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
> +{
> + void (*fn)(struct vfio_group *, struct kvm *);
> +
> + fn = symbol_get(vfio_group_set_kvm);
> + if (!fn)
> + return;
> +
> + fn(group, kvm);
> +
> + symbol_put(vfio_group_set_kvm);
> +}
> +
> +static void kvm_vfio_group_clear_kvm(struct vfio_group *group, struct kvm *kvm)
> +{
> + void (*fn)(struct vfio_group *, struct kvm *);
> +
> + fn = symbol_get(vfio_group_set_kvm);
> + if (!fn)
> + return;
> +
> + fn(group, NULL);
> +
> + symbol_put(vfio_group_set_kvm);
> +}
Why do we need this function? Can't we simply call
kvm_vfio_group_set_kvm(vfio_group, NULL)? Not to mention that the
struct kvm arg is unused here otherwise.
> +
> static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group)
> {
> long (*fn)(struct vfio_group *, unsigned long);
> @@ -155,6 +181,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
> list_add_tail(&kvg->node, &kv->group_list);
> kvg->vfio_group = vfio_group;
>
> + kvm_vfio_group_set_kvm(vfio_group, dev->kvm);
> +
> kvm_arch_start_assignment(dev->kvm);
>
> mutex_unlock(&kv->lock);
> @@ -196,6 +224,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>
> mutex_unlock(&kv->lock);
>
> + kvm_vfio_group_clear_kvm(vfio_group, dev->kvm);
> +
Why did we set_kvm within kv->lock, but clear it outside of kv->lock?
I'm not sure kv->lock is particularly relevant to us but we might as
well be consistent.
> kvm_vfio_group_put_external_user(vfio_group);
>
> kvm_vfio_update_coherency(dev);
> @@ -240,6 +270,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev)
> struct kvm_vfio_group *kvg, *tmp;
>
> list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) {
> + kvm_vfio_group_clear_kvm(kvg->vfio_group, dev->kvm);
> kvm_vfio_group_put_external_user(kvg->vfio_group);
> list_del(&kvg->node);
> kfree(kvg);
--
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
