Linux Virtualization - Mar 2013 - [PATCH vringh] virtio: Introduce vringh wrappers in virtio_config

From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>

Add wrappers for the host vrings to support loose
coupling between the virtio device and driver.

The functions find_vrhs() and del_vrhs() are added to
struct virtio_config_ops to manage the host vrings.
The function vringh_notify() is added so the guest
can be kicked when buffers are added to the used-ring.

This enables the virtio drivers to manage the virtio rings
without knowledge of how the host vrings are managed.

Cc: Ohad Ben-Cohen <ohad at wizery.com>
Signed-off-by: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
---
 include/linux/virtio_config.h |   13 +++++++++++++
 include/linux/vringh.h        |   13 +++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..88dd5ae 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -51,7 +51,17 @@
  *      This returns a pointer to the bus name a la pci_name from which
  *      the caller can then copy.
  * @set_vq_affinity: set the affinity for a virtqueue.
+ * @find_vrhs: find the host vrings and instantiate them
+ *	vdev: the virtio_device
+ *	nhvrs: the number of host vrings to find
+ *	hvrs: on success, includes new host vrings
+ *	callbacks: array of driver callbacks, for each host vring
+ *		include a NULL entry for vqs that do not need a callback
+ *	Returns 0 on success or error status
+ * @del_vrhs: free the host vrings found by find_vrhs().
  */
+struct vringh;
+typedef void vrh_callback_t(struct virtio_device *, struct vringh *);
 typedef void vq_callback_t(struct virtqueue *);
 struct virtio_config_ops {
 	void (*get)(struct virtio_device *vdev, unsigned offset,
@@ -70,6 +80,9 @@ struct virtio_config_ops {
 	void (*finalize_features)(struct virtio_device *vdev);
 	const char *(*bus_name)(struct virtio_device *vdev);
 	int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
+	int (*find_vrhs)(struct virtio_device *vdev, unsigned nhvrs,
+			 struct vringh *vrhs[], vrh_callback_t *callbacks[]);
+	void (*del_vrhs)(struct virtio_device *vdev);
 };
 
 /* If driver didn't advertise the feature, it will never appear. */
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index ab41185..8156f51 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -50,6 +50,12 @@ struct vringh {
 
 	/* The vring (note: it may contain user pointers!) */
 	struct vring vring;
+
+	/* The function to call when buffers are available */
+	void (*notify)(struct vringh *);
+
+	/* A pointer for the vringh clients to use. */
+	void *priv;
 };
 
 /* The memory the vring can access, and what offset to apply. */
@@ -182,4 +188,11 @@ void vringh_notify_disable_kern(struct vringh *vrh);
 
 int vringh_need_notify_kern(struct vringh *vrh);
 
+/* Notify the guest about buffers added to the used ring */
+static inline void vringh_notify(struct vringh *vrh)
+{
+	if (vrh->notify)
+		vrh->notify(vrh);
+}
+
 #endif /* _LINUX_VRINGH_H */
-- 
1.7.5.4

sjur.brandeland at stericsson.com writes:
> From: Sjur Br?ndeland <sjur.brandeland at stericsson.com>
>
> Add wrappers for the host vrings to support loose
> coupling between the virtio device and driver.
>
> The functions find_vrhs() and del_vrhs() are added to
> struct virtio_config_ops to manage the host vrings.
> The function vringh_notify() is added so the guest
> can be kicked when buffers are added to the used-ring.
>
> This enables the virtio drivers to manage the virtio rings
> without knowledge of how the host vrings are managed.
Hmm, this is a bit weird.
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 29b9104..88dd5ae 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -51,7 +51,17 @@
>   *      This returns a pointer to the bus name a la pci_name from which
>   *      the caller can then copy.
>   * @set_vq_affinity: set the affinity for a virtqueue.
> + * @find_vrhs: find the host vrings and instantiate them
> + *	vdev: the virtio_device
> + *	nhvrs: the number of host vrings to find
> + *	hvrs: on success, includes new host vrings
> + *	callbacks: array of driver callbacks, for each host vring
> + *		include a NULL entry for vqs that do not need a callback
> + *	Returns 0 on success or error status
> + * @del_vrhs: free the host vrings found by find_vrhs().
>   */
> +struct vringh;
> +typedef void vrh_callback_t(struct virtio_device *, struct vringh *);
>  typedef void vq_callback_t(struct virtqueue *);
>  struct virtio_config_ops {
>  	void (*get)(struct virtio_device *vdev, unsigned offset,
> @@ -70,6 +80,9 @@ struct virtio_config_ops {
>  	void (*finalize_features)(struct virtio_device *vdev);
>  	const char *(*bus_name)(struct virtio_device *vdev);
>  	int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
> +	int (*find_vrhs)(struct virtio_device *vdev, unsigned nhvrs,
> +			 struct vringh *vrhs[], vrh_callback_t *callbacks[]);
> +	void (*del_vrhs)(struct virtio_device *vdev);
>  };
>  
>  /* If driver didn't advertise the feature, it will never appear. */
It's weird that you conflate the host and guest ring sides in rpmsg, but
that might make sense if they're really bound together.  However, in
general they are not: it's normal to be a guest or host, not both.

This implies that you need a struct vringh_config, to put this in.
> diff --git a/include/linux/vringh.h b/include/linux/vringh.h
> index ab41185..8156f51 100644
> --- a/include/linux/vringh.h
> +++ b/include/linux/vringh.h
> @@ -50,6 +50,12 @@ struct vringh {
>  
>  	/* The vring (note: it may contain user pointers!) */
>  	struct vring vring;
> +
> +	/* The function to call when buffers are available */
> +	void (*notify)(struct vringh *);
> +
> +	/* A pointer for the vringh clients to use. */
> +	void *priv;
>  };
Since the caller allocates the vringh, can it not use container_of() 
instead of a priv pointer?

Thanks,
Rusty.

Hi all,

More benchmarking using vringh_test, and I finally figured out that my
changes to virtio_ring had no effect because we were actually
bottlenecked on the vringh side...

Here are the changes I've ended up with, for your reading pleasure.
They are all in my pending-rebases git tree, and I'll fold them together
before putting them into virtio-next (I'm waiting for Sjur's notify
changes).

Cheers,
Rusty.