Re: [v7 2/3] vfio: support notifier chain in vfio_group

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

 




On 11/22/2016 11:39 AM, Jike Song wrote:
> Beyond vfio_iommu events, users might also be interested in
> vfio_group events. For example, if a vfio_group is used along
> with Qemu/KVM, whenever kvm pointer is set to/cleared from the
> vfio_group, users could be notified.
> 
> Currently only VFIO_GROUP_NOTIFY_SET_KVM supported.
> 
> 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>
> ---
>  drivers/vfio/vfio.c  | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/vfio.h |  6 ++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 493bbad..3ab5edb 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -86,6 +86,8 @@ struct vfio_group {
>  	struct mutex			unbound_lock;
>  	atomic_t			opened;
>  	bool				noiommu;
> +	struct kvm			*kvm;
> +	struct blocking_notifier_head	notifier;
>  };
>  
>  struct vfio_device {
> @@ -339,6 +341,7 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
>  #ifdef CONFIG_VFIO_NOIOMMU
>  	group->noiommu = (iommu_group_get_iommudata(iommu_group) == &noiommu);
>  #endif
> +	BLOCKING_INIT_NOTIFIER_HEAD(&group->notifier);
>  
>  	group->nb.notifier_call = vfio_iommu_group_notifier;
>  
> @@ -1581,6 +1584,9 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep)
>  
>  	filep->private_data = NULL;
>  
> +	/* Any user didn't unregister? */
> +	WARN_ON(group->notifier.head);
> +
>  	vfio_group_try_dissolve_container(group);
>  
>  	atomic_dec(&group->opened);
> @@ -2069,6 +2075,76 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
>  	return ret;
>  }
>  
> +void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
> +{
> +	group->kvm = kvm;
> +	blocking_notifier_call_chain(&group->notifier,
> +				VFIO_GROUP_NOTIFY_SET_KVM, kvm);
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_set_kvm);
> +
> +static int vfio_register_group_notifier(struct vfio_group *group,
> +					unsigned long *events,
> +					struct notifier_block *nb)
> +{
> +	struct vfio_container *container;
> +	int ret;
> +	bool set_kvm = false;
> +
> +	if (*events & VFIO_GROUP_NOTIFY_SET_KVM)
> +		set_kvm = true;
> +
> +	/* clear known events */
> +	*events &= ~VFIO_GROUP_NOTIFY_SET_KVM;
> +
> +	/* refuse to continue if still events remaining */
> +	if (*events)
> +		return -EINVAL;
> +
> +	ret = vfio_group_add_container_user(group);
> +	if (ret)
> +		return -EINVAL;
> +
> +	container = group->container;
> +	down_read(&container->group_lock);
> +
> +	ret = blocking_notifier_chain_register(&group->notifier, nb);
> +
> +	/*
> +	 * The attaching of kvm and vfio_group might already happen, so
> +	 * here we replay once upon registration.
> +	 */
> +	if (!ret && set_kvm && group->kvm)
> +		blocking_notifier_call_chain(&group->notifier,
> +					VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
> +
> +	up_read(&container->group_lock);
> +	vfio_group_try_dissolve_container(group);
> +
> +	return ret;
> +}
> +
> +static int vfio_unregister_group_notifier(struct vfio_group *group,
> +					 struct notifier_block *nb)
> +{
> +	struct vfio_container *container;
> +	int ret;
> +
> +	ret = vfio_group_add_container_user(group);
> +	if (ret)
> +		return -EINVAL;
> +
> +	container = group->container;
> +	down_read(&container->group_lock);
> +
> +	ret = blocking_notifier_chain_unregister(&group->notifier, nb);
> +
> +	up_read(&container->group_lock);
> +	vfio_group_try_dissolve_container(group);
> +
> +	return ret;
> +}
> +
>  int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
>  			   unsigned long *events,
>  			   struct notifier_block *nb)
> @@ -2087,6 +2163,9 @@ int vfio_register_notifier(struct device *dev, vfio_notify_type_t type,
>  	case VFIO_IOMMU_NOTIFY:
>  		ret = vfio_register_iommu_notifier(group, events, nb);
>  		break;
> +	case VFIO_GROUP_NOTIFY:
> +		ret = vfio_register_group_notifier(group, events, nb);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -2113,6 +2192,9 @@ int vfio_unregister_notifier(struct device *dev, vfio_notify_type_t type,
>  	case VFIO_IOMMU_NOTIFY:
>  		ret = vfio_unregister_iommu_notifier(group, nb);
>  		break;
> +	case VFIO_GROUP_NOTIFY:
> +		ret = vfio_unregister_group_notifier(group, nb);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 6f3ff31..5d46e3c 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -119,11 +119,17 @@ extern int vfio_unregister_notifier(struct device *dev,
>  /* each type has independent events */
>  enum vfio_notify_type {
>  	VFIO_IOMMU_NOTIFY = (__force vfio_notify_type_t)0,
> +	VFIO_GROUP_NOTIFY = (__force vfio_notify_type_t)1,
>  };
>  
>  /* events for VFIO_IOMMU_NOTIFY */
>  #define VFIO_IOMMU_NOTIFY_DMA_UNMAP	BIT(0)
>  
> +/* events for VFIO_GROUP_NOTIFY */
> +#define VFIO_GROUP_NOTIFY_SET_KVM	BIT(0)
> +
> +struct kvm;
> +extern void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
>  

If I understand correctly, notifier for backend iommu driver and
notifier for group should be registered with different notifier blocks,
as per your current implementation:

unsigned long iommu_event = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
struct notifier_block iommu_nb;

iommu_nb.notifier_call = vfio_iommu_notifier_cb;
vfio_register_notifier(dev, VFIO_IOMMU_NOTIFY, &iommu_event, &iommu_nb);


unsigned long group_event = VFIO_GROUP_NOTIFY_SET_KVM;
struct notifier_block group_nb;

group_nb.notifier_call = vfio_group_notifier_cb;
vfio_register_notifier(dev, VFIO_GROUP_NOTIFY, &group_event, &group_nb);

Then what is the use of having 'events' bitmask as input argument?

Its clear that caller should set:
-  VFIO_IOMMU_NOTIFY_DMA_UNMAP when vfio_notify_type_t is VFIO_IOMMU_NOTIFY
-  VFIO_GROUP_NOTIFY_SET_KVM when vfio_notify_type_t is VFIO_GROUP_NOTIFY

Notifier action could be a integer as earlier and indexing of action
would be different for both notifiers.

Then caller don't have to sent event bitmask, for caller it would look like:

struct notifier_block iommu_nb;

iommu_nb.notifier_call = vfio_iommu_notifier_cb;
vfio_register_notifier(dev, VFIO_IOMMU_NOTIFY, &iommu_nb);


struct notifier_block group_nb;

group_nb.notifier_call = vfio_group_notifier_cb;
vfio_register_notifier(dev, VFIO_GROUP_NOTIFY, &group_nb);

Thanks,
Kirti
--
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