[RFCv2 1/2] virtio_console: Add support for DMA memory allocation

[RFCv2 1/2] virtio_console: Add support for DMA memory allocation

[sjur.brandeland]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Add feature VIRTIO_CONSOLE_F_DMA_MEM. If the architecture has
DMA support and this feature bit is set, the virtio data buffers
will be allocated from DMA memory. If the device requests
the feature VIRTIO_CONSOLE_F_DMA_MEM, but the architecture
don't support DMA the driver's probe function will fail.

This is needed for using virtio_console from the remoteproc
framework.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
cc: Rusty Russell <rusty@rustcorp.com.au>
cc: Michael S. Tsirkin <mst@redhat.com>
cc: Amit Shah <amit.shah@redhat.com>
cc: Ohad Ben-Cohen <ohad@wizery.com>
cc: Linus Walleij <linus.walleij@linaro.org>
cc: virtualization@lists.linux-foundation.org
cc: linux-kernel@vger.kernel.org
---
 drivers/char/virtio_console.c  |   91 +++++++++++++++++++++++++++++++++-------
 include/linux/virtio_console.h |    1 +
 2 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cdf2f54..469c05f 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -35,8 +35,15 @@
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <linux/module.h>
+#include <linux/dma-mapping.h>
 #include "../tty/hvc/hvc_console.h"
 
+#ifdef CONFIG_HAS_DMA
+#define VIRTIO_CONSOLE_HAS_DMA (1)
+#else
+#define VIRTIO_CONSOLE_HAS_DMA (0)
+#endif
+
 /*
  * This is a global struct for storing common data for all the devices
  * this driver handles.
@@ -334,20 +341,56 @@ static inline bool use_multiport(struct ports_device *portdev)
 	return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
-static void free_buf(struct port_buffer *buf)
+/* Allocate data buffer from DMA memory if requested */
+static inline void *
+alloc_databuf(struct virtio_device *vdev, size_t size, gfp_t flag)
+{
+	if (VIRTIO_CONSOLE_HAS_DMA &&
+	    virtio_has_feature(vdev, VIRTIO_CONSOLE_F_DMA_MEM)) {
+		struct device *dev = &vdev->dev;
+		dma_addr_t dma;
+		/*
+		 * Allocate DMA memory from ancestors. Finding the ancestor
+		 * is a bit quirky when DMA_MEMORY_INCLUDES_CHILDREN is not
+		 * implemented.
+		 */
+		dev = dev->parent ? dev->parent : dev;
+		dev = dev->parent ? dev->parent : dev;
+		return dma_alloc_coherent(dev, size, &dma, flag);
+	}
+	return kzalloc(size, flag);
+}
+
+static inline void
+free_databuf(struct virtio_device *vdev, size_t size, void *cpu_addr)
+{
+	if (VIRTIO_CONSOLE_HAS_DMA &&
+	    virtio_has_feature(vdev, VIRTIO_CONSOLE_F_DMA_MEM)) {
+		struct device *dev = &vdev->dev;
+		dma_addr_t dma = virt_to_bus(cpu_addr);
+		dev = dev->parent ? dev->parent : dev;
+		dev = dev->parent ? dev->parent : dev;
+		dma_free_coherent(dev, size, cpu_addr, dma);
+		return;
+	}
+	kfree(cpu_addr);
+}
+
+static void
+free_buf(struct virtqueue *vq, struct port_buffer *buf, size_t buf_size)
 {
-	kfree(buf->buf);
+	free_databuf(vq->vdev, buf_size, buf);
 	kfree(buf);
 }
 
-static struct port_buffer *alloc_buf(size_t buf_size)
+static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size)
 {
 	struct port_buffer *buf;
 
 	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
 	if (!buf)
 		goto fail;
-	buf->buf = kzalloc(buf_size, GFP_KERNEL);
+	buf->buf = alloc_databuf(vq->vdev, buf_size, GFP_KERNEL);
 	if (!buf->buf)
 		goto free_buf;
 	buf->len = 0;
@@ -414,7 +457,7 @@ static void discard_port_data(struct port *port)
 		port->stats.bytes_discarded += buf->len - buf->offset;
 		if (add_inbuf(port->in_vq, buf) < 0) {
 			err++;
-			free_buf(buf);
+			free_buf(port->in_vq, buf, PAGE_SIZE);
 		}
 		port->inbuf = NULL;
 		buf = get_inbuf(port);
@@ -485,7 +528,7 @@ static void reclaim_consumed_buffers(struct port *port)
 		return;
 	}
 	while ((buf = virtqueue_get_buf(port->out_vq, &len))) {
-		kfree(buf);
+		free_databuf(port->portdev->vdev, len, buf);
 		port->outvq_full = false;
 	}
 }
@@ -672,6 +715,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 	char *buf;
 	ssize_t ret;
 	bool nonblock;
+	struct virtio_device *vdev;
 
 	/* Userspace could be out to fool us */
 	if (!count)
@@ -694,9 +738,10 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 	if (!port->guest_connected)
 		return -ENODEV;
 
+	vdev = port->portdev->vdev;
 	count = min((size_t)(32 * 1024), count);
 
-	buf = kmalloc(count, GFP_KERNEL);
+	buf = alloc_databuf(vdev, count, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -720,7 +765,8 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 		goto out;
 
 free_buf:
-	kfree(buf);
+	free_databuf(vdev, count, buf);
+
 out:
 	return ret;
 }
@@ -1102,7 +1148,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
 
 	nr_added_bufs = 0;
 	do {
-		buf = alloc_buf(PAGE_SIZE);
+		buf = alloc_buf(vq, PAGE_SIZE);
 		if (!buf)
 			break;
 
@@ -1110,7 +1156,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
 		ret = add_inbuf(vq, buf);
 		if (ret < 0) {
 			spin_unlock_irq(lock);
-			free_buf(buf);
+			free_buf(vq, buf, PAGE_SIZE);
 			break;
 		}
 		nr_added_bufs++;
@@ -1234,7 +1280,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 
 free_inbufs:
 	while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
-		free_buf(buf);
+		free_buf(port->in_vq, buf, PAGE_SIZE);
 free_device:
 	device_destroy(pdrvdata.class, port->dev->devt);
 free_cdev:
@@ -1276,7 +1322,7 @@ static void remove_port_data(struct port *port)
 
 	/* Remove buffers we queued up for the Host to send us data in. */
 	while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
-		free_buf(buf);
+		free_buf(port->in_vq, buf, PAGE_SIZE);
 }
 
 /*
@@ -1478,7 +1524,7 @@ static void control_work_handler(struct work_struct *work)
 		if (add_inbuf(portdev->c_ivq, buf) < 0) {
 			dev_warn(&portdev->vdev->dev,
 				 "Error adding buffer to queue\n");
-			free_buf(buf);
+			free_buf(portdev->c_ivq, buf, PAGE_SIZE);
 		}
 	}
 	spin_unlock(&portdev->cvq_lock);
@@ -1674,10 +1720,10 @@ static void remove_controlq_data(struct ports_device *portdev)
 		return;
 
 	while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
-		free_buf(buf);
+		free_buf(portdev->c_ivq, buf, PAGE_SIZE);
 
 	while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
-		free_buf(buf);
+		free_buf(portdev->c_ivq, buf, PAGE_SIZE);
 }
 
 /*
@@ -1698,6 +1744,17 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	/* Ensure to read early_put_chars now */
 	barrier();
 
+	/* Refuse to bind if F_DMA_MEM request cannot be met */
+	if (!VIRTIO_CONSOLE_HAS_DMA &&
+	    (vdev->config->get_features(vdev) &
+		(1 << VIRTIO_CONSOLE_F_DMA_MEM))) {
+
+		dev_err(&vdev->dev,
+			"DMA_MEM requested but arch does not support DMA\n");
+		err = -ENODEV;
+		goto fail;
+	}
+
 	portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
 	if (!portdev) {
 		err = -ENOMEM;
@@ -1836,6 +1893,10 @@ static struct virtio_device_id id_table[] = {
 static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_SIZE,
 	VIRTIO_CONSOLE_F_MULTIPORT,
+#if VIRTIO_CONSOLE_HAS_DMA
+	VIRTIO_CONSOLE_F_DMA_MEM,
+#endif
+
 };
 
 #ifdef CONFIG_PM
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index bdf4b00..b27f7fa 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -38,6 +38,7 @@
 /* Feature bits */
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
 #define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
+#define VIRTIO_CONSOLE_F_DMA_MEM 2	/* Use DMA memory in vrings */
 
 #define VIRTIO_CONSOLE_BAD_ID		(~(u32)0)
 
-- 
1.7.9.5

[PATCHv2] virtio: Don't access device data after unregistration.

[sjur.brandeland]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Fix panic in virtio.c when CONFIG_DEBUG_SLAB is set.
device_unregister() drops reference to device so put_device()
could invoke release callback. In this case the release
callback will free the device. Make sure we don't access
device after unregister by fetching the device index
before calling unregister.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
cc: Guzman Lugo, Fernadndo <fernando.lugo@ti.com>
cc: Michael S. Tsirkin <mst@redhat.com>
cc: Rusty Russell <rusty@rustcorp.com.au>
cc: Ohad Ben-Cohen <ohad@wizery.com>
cc: virtualization@lists.linux-foundation.org
cc: linux-kernel@vger.kernel.org
---
 drivers/virtio/virtio.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index c3b3f7f..faee112 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -225,8 +225,15 @@ EXPORT_SYMBOL_GPL(register_virtio_device);
 
 void unregister_virtio_device(struct virtio_device *dev)
 {
+	/*
+	 * device_unregister() drops reference to device so put_device could
+	 * invoke release callback. In case that callback will free the device,
+	 * make sure we don't access device after this call.
+	 */
+
+	int index = dev->index;
 	device_unregister(&dev->dev);
-	ida_simple_remove(&virtio_index_ida, dev->index);
+	ida_simple_remove(&virtio_index_ida, index);
 }
 EXPORT_SYMBOL_GPL(unregister_virtio_device);
 
-- 
1.7.5.4

[RFCv2 2/2] virtio_console: Add feature to disable console port

[sjur.brandeland]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Add the feature VIRTIO_CONSOLE_F_NO_HVC. With this bit set
only port-devices are created. The console port and
port control virtio-queues are not created.

The console port is not suited for communicating
to a remote processor because of it's blocking behavior.
But the port-device supports efficient non-blocking IO
to a remote processor.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
cc: Rusty Russell <rusty@rustcorp.com.au>
cc: Michael S. Tsirkin <mst@redhat.com>
cc: Amit Shah <amit.shah@redhat.com>
cc: Ohad Ben-Cohen <ohad@wizery.com>
cc: Linus Walleij <linus.walleij@linaro.org>
cc: virtualization@lists.linux-foundation.org
cc: linux-kernel@vger.kernel.org
---
 drivers/char/virtio_console.c  |    6 +++++-
 include/linux/virtio_console.h |    1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 469c05f..7408c00 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1244,10 +1244,13 @@ static int add_port(struct ports_device *portdev, u32 id)
 		goto free_device;
 	}
 
+	/* Don't initialize the port_console if F_NO_HVC is set*/
+	if (virtio_has_feature(port->portdev->vdev, VIRTIO_CONSOLE_F_NO_HVC))
+		port->host_connected = true;
 	/*
 	 * If we're not using multiport support, this has to be a console port
 	 */
-	if (!use_multiport(port->portdev)) {
+	else if (!use_multiport(port->portdev)) {
 		err = init_port_console(port);
 		if (err)
 			goto free_inbufs;
@@ -1896,6 +1899,7 @@ static unsigned int features[] = {
 #if VIRTIO_CONSOLE_HAS_DMA
 	VIRTIO_CONSOLE_F_DMA_MEM,
 #endif
+	VIRTIO_CONSOLE_F_NO_HVC,
 
 };
 
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index b27f7fa..a7c8974 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -39,6 +39,7 @@
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
 #define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
 #define VIRTIO_CONSOLE_F_DMA_MEM 2	/* Use DMA memory in vrings */
+#define VIRTIO_CONSOLE_F_NO_HVC 3	/* Disable use of HVC */
 
 #define VIRTIO_CONSOLE_BAD_ID		(~(u32)0)
 
-- 
1.7.9.5

Re: [RFCv2 1/2] virtio_console: Add support for DMA memory allocation

[Rusty Russell]

sjur.brandeland@stericsson.com writes:

> From: Sjur Brændeland <sjur.brandeland@stericsson.com>
>
> Add feature VIRTIO_CONSOLE_F_DMA_MEM. If the architecture has
> DMA support and this feature bit is set, the virtio data buffers
> will be allocated from DMA memory. If the device requests
> the feature VIRTIO_CONSOLE_F_DMA_MEM, but the architecture
> don't support DMA the driver's probe function will fail.
>
> This is needed for using virtio_console from the remoteproc
> framework.

Sorry for the back and forth, I've been pondering MST's points.

If we make a new dma-multiport device (eg. ID 11), how ugly is the code?

It would be a virtio console with DMA buffers and no console, just the
multiport stuff.  This would have no impact on the current spec for
virtio console.

Thanks,
Rusty.

[RFCv3] virtio_console: Add support for virtio remoteproc serial

[sjur.brandeland]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Add a virtio remoteproc serial driver: VIRTIO_ID_RPROC_SERIAL (0xB)
for communicating with a remote processor in an asymmetric
multi-processing configuration.

The virtio remoteproc serial driver reuses the existing virtio_console
implementation, and adds support for DMA allocation of data buffers
but disables support for tty console, mutiple ports, and the control queue.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
cc: Rusty Russell <rusty@rustcorp.com.au>
cc: Michael S. Tsirkin <mst@redhat.com>
cc: Amit Shah <amit.shah@redhat.com>
cc: Ohad Ben-Cohen <ohad@wizery.com>
cc: Linus Walleij <linus.walleij@linaro.org>
cc: virtualization@lists.linux-foundation.org
cc: linux-kernel@vger.kernel.org
---

Hi Rusty,

...

>Sorry for the back and forth, I've been pondering MST's points.
>If we make a new dma-multiport device (eg. ID 11), how ugly is the code?

I don't think it's too bad.
It's a few more code lines - as I have added a new virtio_driver
definition and feature table. The rest is just replacing the check
on feature bits with driver type. I no longer need to check on
feature bits in the probe function, as the match on device type is
done automatically by the driver framework.

I actually like this new approach better.
It solves the issues Michael has pointed out, and we don't have to
think through side effects of weired combination of feature bits.

>It would be a virtio console with DMA buffers and no console, just the
>multiport stuff.  This would have no impact on the current spec for
>virtio console.

Yes, and this way it will also be easier to explain what this new
feature does.

Regards,
Sjur

 drivers/char/virtio_console.c |  133 +++++++++++++++++++++++++++++++++--------
 include/linux/virtio_ids.h    |    1 +
 2 files changed, 109 insertions(+), 25 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cdf2f54..08593aa 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -35,6 +35,7 @@
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <linux/module.h>
+#include <linux/dma-mapping.h>
 #include "../tty/hvc/hvc_console.h"
 
 /*
@@ -323,6 +324,52 @@ static bool is_console_port(struct port *port)
 	return false;
 }
 
+#ifdef CONFIG_HAS_DMA
+static inline bool is_rproc_serial(struct virtio_device *vdev)
+{
+	return vdev->id.device == VIRTIO_ID_RPROC_SERIAL;
+}
+#else
+static inline bool is_rproc_serial(struct virtio_device *vdev)
+{
+	return false;
+}
+#endif
+
+/* Allocate data buffer from DMA memory if requested */
+static inline void *
+alloc_databuf(struct virtio_device *vdev, size_t size, gfp_t flag)
+{
+	if (is_rproc_serial(vdev)) {
+		dma_addr_t dma;
+		struct device *dev = &vdev->dev;
+		/*
+		 * Allocate DMA memory from ancestors. Finding the ancestor
+		 * is a bit quirky when DMA_MEMORY_INCLUDES_CHILDREN is not
+		 * implemented.
+		 */
+		dev = dev->parent ? dev->parent : dev;
+		dev = dev->parent ? dev->parent : dev;
+		return dma_alloc_coherent(dev, size, &dma, flag);
+	}
+	return kzalloc(size, flag);
+}
+
+static inline void
+free_databuf(struct virtio_device *vdev, size_t size, void *cpu_addr)
+{
+
+	if (is_rproc_serial(vdev)) {
+		struct device *dev = &vdev->dev;
+		dma_addr_t dma_handle = virt_to_bus(cpu_addr);
+		dev = dev->parent ? dev->parent : dev;
+		dev = dev->parent ? dev->parent : dev;
+		dma_free_coherent(dev, size, cpu_addr, dma_handle);
+		return;
+	}
+	kfree(cpu_addr);
+}
+
 static inline bool use_multiport(struct ports_device *portdev)
 {
 	/*
@@ -334,20 +381,21 @@ static inline bool use_multiport(struct ports_device *portdev)
 	return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
-static void free_buf(struct port_buffer *buf)
+static void
+free_buf(struct virtqueue *vq, struct port_buffer *buf, size_t buf_size)
 {
-	kfree(buf->buf);
+	free_databuf(vq->vdev, buf_size, buf->buf);
 	kfree(buf);
 }
 
-static struct port_buffer *alloc_buf(size_t buf_size)
+static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size)
 {
 	struct port_buffer *buf;
 
 	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
 	if (!buf)
 		goto fail;
-	buf->buf = kzalloc(buf_size, GFP_KERNEL);
+	buf->buf = alloc_databuf(vq->vdev, buf_size, GFP_KERNEL);
 	if (!buf->buf)
 		goto free_buf;
 	buf->len = 0;
@@ -414,7 +462,7 @@ static void discard_port_data(struct port *port)
 		port->stats.bytes_discarded += buf->len - buf->offset;
 		if (add_inbuf(port->in_vq, buf) < 0) {
 			err++;
-			free_buf(buf);
+			free_buf(port->in_vq, buf, PAGE_SIZE);
 		}
 		port->inbuf = NULL;
 		buf = get_inbuf(port);
@@ -485,7 +533,7 @@ static void reclaim_consumed_buffers(struct port *port)
 		return;
 	}
 	while ((buf = virtqueue_get_buf(port->out_vq, &len))) {
-		kfree(buf);
+		free_databuf(port->portdev->vdev, len, buf);
 		port->outvq_full = false;
 	}
 }
@@ -672,6 +720,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 	char *buf;
 	ssize_t ret;
 	bool nonblock;
+	struct virtio_device *vdev;
 
 	/* Userspace could be out to fool us */
 	if (!count)
@@ -694,9 +743,10 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 	if (!port->guest_connected)
 		return -ENODEV;
 
+	vdev = port->portdev->vdev;
 	count = min((size_t)(32 * 1024), count);
 
-	buf = kmalloc(count, GFP_KERNEL);
+	buf = alloc_databuf(vdev, count, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -720,7 +770,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 		goto out;
 
 free_buf:
-	kfree(buf);
+	free_databuf(vdev, count, buf);
 out:
 	return ret;
 }
@@ -918,7 +968,8 @@ static void resize_console(struct port *port)
 		return;
 
 	vdev = port->portdev->vdev;
-	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
+	if (!is_rproc_serial(vdev) &&
+	    virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
 		hvc_resize(port->cons.hvc, port->cons.ws);
 }
 
@@ -1102,7 +1153,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
 
 	nr_added_bufs = 0;
 	do {
-		buf = alloc_buf(PAGE_SIZE);
+		buf = alloc_buf(vq, PAGE_SIZE);
 		if (!buf)
 			break;
 
@@ -1110,7 +1161,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
 		ret = add_inbuf(vq, buf);
 		if (ret < 0) {
 			spin_unlock_irq(lock);
-			free_buf(buf);
+			free_buf(vq, buf, PAGE_SIZE);
 			break;
 		}
 		nr_added_bufs++;
@@ -1198,10 +1249,18 @@ static int add_port(struct ports_device *portdev, u32 id)
 		goto free_device;
 	}
 
-	/*
-	 * If we're not using multiport support, this has to be a console port
-	 */
-	if (!use_multiport(port->portdev)) {
+	if (is_rproc_serial(port->portdev->vdev))
+		/*
+		 * For rproc_serial assume remote processor is connected.
+		 * rproc_serial does not want the console port, but
+		 * the generic port implementation.
+		 */
+		port->host_connected = true;
+	else if (!use_multiport(port->portdev)) {
+		/*
+		 * If we're not using multiport support,
+		 * this has to be a console port.
+		 */
 		err = init_port_console(port);
 		if (err)
 			goto free_inbufs;
@@ -1234,7 +1293,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 
 free_inbufs:
 	while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
-		free_buf(buf);
+		free_buf(port->in_vq, buf, PAGE_SIZE);
 free_device:
 	device_destroy(pdrvdata.class, port->dev->devt);
 free_cdev:
@@ -1276,7 +1335,7 @@ static void remove_port_data(struct port *port)
 
 	/* Remove buffers we queued up for the Host to send us data in. */
 	while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
-		free_buf(buf);
+		free_buf(port->in_vq, buf, PAGE_SIZE);
 }
 
 /*
@@ -1478,7 +1537,7 @@ static void control_work_handler(struct work_struct *work)
 		if (add_inbuf(portdev->c_ivq, buf) < 0) {
 			dev_warn(&portdev->vdev->dev,
 				 "Error adding buffer to queue\n");
-			free_buf(buf);
+			free_buf(portdev->c_ivq, buf, PAGE_SIZE);
 		}
 	}
 	spin_unlock(&portdev->cvq_lock);
@@ -1674,10 +1733,10 @@ static void remove_controlq_data(struct ports_device *portdev)
 		return;
 
 	while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
-		free_buf(buf);
+		free_buf(portdev->c_ivq, buf, PAGE_SIZE);
 
 	while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
-		free_buf(buf);
+		free_buf(portdev->c_ivq, buf, PAGE_SIZE);
 }
 
 /*
@@ -1724,10 +1783,12 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 
 	multiport = false;
 	portdev->config.max_nr_ports = 1;
-	if (virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
-			      offsetof(struct virtio_console_config,
-				       max_nr_ports),
-			      &portdev->config.max_nr_ports) == 0)
+	if (is_rproc_serial(vdev))
+		multiport = false;
+	else if (virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
+				  offsetof(struct virtio_console_config,
+					   max_nr_ports),
+				  &portdev->config.max_nr_ports) == 0)
 		multiport = true;
 
 	err = init_vqs(portdev);
@@ -1838,6 +1899,16 @@ static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_MULTIPORT,
 };
 
+static struct virtio_device_id rproc_serial_id_table[] = {
+#ifdef CONFIG_HAS_DMA
+	{ VIRTIO_ID_RPROC_SERIAL, VIRTIO_DEV_ANY_ID },
+#endif
+	{ 0 },
+};
+
+static unsigned int rproc_serial_features[] = {
+};
+
 #ifdef CONFIG_PM
 static int virtcons_freeze(struct virtio_device *vdev)
 {
@@ -1922,6 +1993,16 @@ static struct virtio_driver virtio_console = {
 #endif
 };
 
+static struct virtio_driver virtio_rproc_serial = {
+	.feature_table = rproc_serial_features,
+	.feature_table_size = ARRAY_SIZE(rproc_serial_features),
+	.driver.name =	"virtio_rproc_serial",
+	.driver.owner =	THIS_MODULE,
+	.id_table =	rproc_serial_id_table,
+	.probe =	virtcons_probe,
+	.remove =	virtcons_remove,
+};
+
 static int __init init(void)
 {
 	int err;
@@ -1941,12 +2022,14 @@ static int __init init(void)
 	INIT_LIST_HEAD(&pdrvdata.consoles);
 	INIT_LIST_HEAD(&pdrvdata.portdevs);
 
-	return register_virtio_driver(&virtio_console);
+	return register_virtio_driver(&virtio_console) &&
+		register_virtio_driver(&virtio_rproc_serial);
 }
 
 static void __exit fini(void)
 {
 	unregister_virtio_driver(&virtio_console);
+	unregister_virtio_driver(&virtio_rproc_serial);
 
 	class_destroy(pdrvdata.class);
 	if (pdrvdata.debugfs_dir)
diff --git a/include/linux/virtio_ids.h b/include/linux/virtio_ids.h
index 270fb22..07cf6f7 100644
--- a/include/linux/virtio_ids.h
+++ b/include/linux/virtio_ids.h
@@ -37,5 +37,6 @@
 #define VIRTIO_ID_RPMSG		7 /* virtio remote processor messaging */
 #define VIRTIO_ID_SCSI		8 /* virtio scsi */
 #define VIRTIO_ID_9P		9 /* 9p virtio console */
+#define VIRTIO_ID_RPROC_SERIAL	0xB /* virtio remoteproc serial link */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
1.7.5.4

Re: [RFCv3] virtio_console: Add support for virtio remoteproc serial

[Rusty Russell]

sjur.brandeland@stericsson.com writes:
> I actually like this new approach better.
> It solves the issues Michael has pointed out, and we don't have to
> think through side effects of weired combination of feature bits.

Agreed.

Just one thing, should it depend on CONFIG_REMOTEPROC?  And have
OMAP_REMOTEPROC depend on CONFIG_HAS_DMA (if it doesn't already).

Thanks,
Rusty.
PS.  I've reserved 11 for you in the latest virtio spec draft.