[PATCH v4 0/5] virtio_rpmsg: make rpmsg channel configurable

[PATCH v4 0/5] virtio_rpmsg: make rpmsg channel configurable

[Loic Pallardy]

This goal of this series is to offer more flexibility for virtio_rpmsg
communication link configuration.

It should be possible to tune rpmsg buffer size per Rpmsg link, to provide
selected configuration to coprocessor, to take into account coprocessor
specificities like memory region.

This series introduces an optional virtio rpmsg configuration structure, which
will be managed as an extension of struct fw_rsc_vdev present in coprocessor firmware
resource table.
Structure access is possible thanks to virtio get and set API.
This structure contains the different information to tune the link between the
host and the coprocessor.

Patch "rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid
kernel address" has the same goal as Wendy's RFC [1]. I don't have tested
Wendy's series with level driver buffer allocation.

Last patch "rpmsg: virtio_rpmsg: set buffer configuration to virtio" has a
dependency with remoteproc subdevice boot sequence as coprocessor resource
table must be updated before coprocessor start. See [2]

[1] http://www.spinics.net/lists/linux-remoteproc/msg00852.html
[2] http://www.spinics.net/lists/linux-remoteproc/msg00826.html

---
Changes from v1:
- No more dependency with [2]. Coprocessor firmware should wait for first kick
indicating virtio rpmsg link ready before accessing resource table information.
- Correct issues reported by Suman
- Fix warnings found by checkpatch with --strict option

Changes from v2:
- Fix warnings reported by kbuild test robot

Changes from v3:
After discussions with Bjorn, virtual address sharing between low level driver
and virtio_rpmsg could be replaced by associated a dedicated memory region to
vdev and changing virtio_rpmsg allocation from dev->parent->parent to
dev->parent, i.e. vdev and not rproc device.
The advantage of the solution is that there will be only few changes in
virtio_rpmsg layer and it will be common with virtio_console driver.
These will be part of a dedicated patch series.
So v4 drops previous patch 5 "rpmsg: virtio_rpmsg: don't allocate buffer if
provided by low level driver" dealing with buffer previously allocated and va
fields from struct virtio_rpmsg_cfg.
In addition, patch series has been rebased on kernel 4.11-rc4.


Loic Pallardy (5):
  rpmsg: virtio_rpmsg: set rpmsg_buf_size customizable
  rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid
    kernel address
  include: virtio_rpmsg: add virtio rpmsg configuration structure
  rpmsg: virtio_rpmsg: get buffer configuration from virtio
  rpmsg: virtio_rpmsg: set buffer configuration to virtio

 drivers/rpmsg/virtio_rpmsg_bus.c   | 123 +++++++++++++++++++++++++++++++++----
 include/linux/rpmsg/virtio_rpmsg.h |  30 +++++++++
 2 files changed, 141 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/rpmsg/virtio_rpmsg.h

-- 
1.9.1

[PATCH v4 1/5] rpmsg: virtio_rpmsg: set rpmsg_buf_size customizable

[Loic Pallardy]

Rpmsg buffer size is currently fixed to 512 bytes.
This patch introduces a new capability in struct virtproc_info
to tune shared buffer size between host and coprocessor
according to the needs.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes since V1:
- Initialize vrp->num_bufs before use.
- Alignment with open parenthesis.

No change since v2.
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 5e66e08..3f44a03 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -45,6 +45,7 @@
  * @rbufs:	kernel address of rx buffers
  * @sbufs:	kernel address of tx buffers
  * @num_bufs:	total number of buffers for rx and tx
+ * @buf_size:   size of one rx or tx buffer
  * @last_sbuf:	index of last tx buffer used
  * @bufs_dma:	dma base addr of the buffers
  * @tx_lock:	protects svq, sbufs and sleepers, to allow concurrent senders.
@@ -65,6 +66,7 @@ struct virtproc_info {
 	struct virtqueue *rvq, *svq;
 	void *rbufs, *sbufs;
 	unsigned int num_bufs;
+	unsigned int buf_size;
 	int last_sbuf;
 	dma_addr_t bufs_dma;
 	struct mutex tx_lock;
@@ -158,7 +160,7 @@ struct virtio_rpmsg_channel {
  * processor.
  */
 #define MAX_RPMSG_NUM_BUFS	(512)
-#define RPMSG_BUF_SIZE		(512)
+#define MAX_RPMSG_BUF_SIZE	(512)
 
 /*
  * Local addresses are dynamically allocated on-demand.
@@ -429,7 +431,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
 	 * (half of our buffers are used for sending messages)
 	 */
 	if (vrp->last_sbuf < vrp->num_bufs / 2)
-		ret = vrp->sbufs + RPMSG_BUF_SIZE * vrp->last_sbuf++;
+		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
 	/* or recycle a used one */
 	else
 		ret = virtqueue_get_buf(vrp->svq, &len);
@@ -555,7 +557,7 @@ static int rpmsg_send_offchannel_raw(struct rpmsg_device *rpdev,
 	 * messaging), or to improve the buffer allocator, to support
 	 * variable-length buffer sizes.
 	 */
-	if (len > RPMSG_BUF_SIZE - sizeof(struct rpmsg_hdr)) {
+	if (len > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
 		dev_err(dev, "message is too big (%d)\n", len);
 		return -EMSGSIZE;
 	}
@@ -696,7 +698,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 	 * We currently use fixed-sized buffers, so trivially sanitize
 	 * the reported payload length.
 	 */
-	if (len > RPMSG_BUF_SIZE ||
+	if (len > vrp->buf_size ||
 	    msg->len > (len - sizeof(struct rpmsg_hdr))) {
 		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg->len);
 		return -EINVAL;
@@ -729,7 +731,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 		dev_warn(dev, "msg received with no recipient\n");
 
 	/* publish the real size of the buffer */
-	sg_init_one(&sg, msg, RPMSG_BUF_SIZE);
+	sg_init_one(&sg, msg, vrp->buf_size);
 
 	/* add the buffer back to the remote processor's virtqueue */
 	err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
@@ -886,7 +888,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	else
 		vrp->num_bufs = MAX_RPMSG_NUM_BUFS;
 
-	total_buf_space = vrp->num_bufs * RPMSG_BUF_SIZE;
+	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
+
+	total_buf_space = vrp->num_bufs * vrp->buf_size;
 
 	/* allocate coherent memory for the buffers */
 	bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
@@ -909,9 +913,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	/* set up the receive buffers */
 	for (i = 0; i < vrp->num_bufs / 2; i++) {
 		struct scatterlist sg;
-		void *cpu_addr = vrp->rbufs + i * RPMSG_BUF_SIZE;
+		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
 
-		sg_init_one(&sg, cpu_addr, RPMSG_BUF_SIZE);
+		sg_init_one(&sg, cpu_addr, vrp->buf_size);
 
 		err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
 					  GFP_KERNEL);
@@ -976,7 +980,7 @@ static int rpmsg_remove_device(struct device *dev, void *data)
 static void rpmsg_remove(struct virtio_device *vdev)
 {
 	struct virtproc_info *vrp = vdev->priv;
-	size_t total_buf_space = vrp->num_bufs * RPMSG_BUF_SIZE;
+	size_t total_buf_space = vrp->num_bufs * vrp->buf_size;
 	int ret;
 
 	vdev->config->reset(vdev);
-- 
1.9.1

Re: [PATCH v4 1/5] rpmsg: virtio_rpmsg: set rpmsg_buf_size customizable

[Suman Anna]

On 03/28/2017 06:49 AM, Loic Pallardy wrote:
> Rpmsg buffer size is currently fixed to 512 bytes.
> This patch introduces a new capability in struct virtproc_info
> to tune shared buffer size between host and coprocessor
> according to the needs.
> 
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

Acked-by: Suman Anna <s-anna@ti.com>

regards
Suman

[PATCH v4 2/5] rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid kernel address

[Loic Pallardy]

To specify memory for remoteproc, we declare (dma_declare_coherent_memory())
an area which is ioremap'ed to the vmalloc area.  However, this address is
not a kernel address so virt_addr_valid(buf) fails.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
Acked-by: Patrice Chotard <patrice.chotard@st.com>
Tested-by: Suman Anna <s-anna@ti.com>
---
Changes since V1:
- Remove extra line.

No change since v2.

---
 drivers/rpmsg/virtio_rpmsg_bus.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 3f44a03..1c7cde9 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -195,6 +195,28 @@ static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
 };
 
 /**
+ * rpmsg_sg_init - initialize scatterlist according to cpu address location
+ * @sg: scatterlist to fill
+ * @cpu_addr: virtual address of the buffer
+ * @len: buffer length
+ *
+ * An internal function filling scatterlist according to virtual address
+ * location (in vmalloc or in kernel).
+ */
+static void
+rpmsg_sg_init(struct scatterlist *sg, void *cpu_addr, unsigned int len)
+{
+	if (is_vmalloc_addr(cpu_addr)) {
+		sg_init_table(sg, 1);
+		sg_set_page(sg, vmalloc_to_page(cpu_addr), len,
+			    offset_in_page(cpu_addr));
+	} else {
+		WARN_ON(!virt_addr_valid(cpu_addr));
+		sg_init_one(sg, cpu_addr, len);
+	}
+}
+
+/**
  * __ept_release() - deallocate an rpmsg endpoint
  * @kref: the ept's reference count
  *
@@ -606,7 +628,7 @@ static int rpmsg_send_offchannel_raw(struct rpmsg_device *rpdev,
 			 msg, sizeof(*msg) + msg->len, true);
 #endif
 
-	sg_init_one(&sg, msg, sizeof(*msg) + len);
+	rpmsg_sg_init(&sg, msg, sizeof(*msg) + len);
 
 	mutex_lock(&vrp->tx_lock);
 
@@ -731,7 +753,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 		dev_warn(dev, "msg received with no recipient\n");
 
 	/* publish the real size of the buffer */
-	sg_init_one(&sg, msg, vrp->buf_size);
+	rpmsg_sg_init(&sg, msg, vrp->buf_size);
 
 	/* add the buffer back to the remote processor's virtqueue */
 	err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
@@ -915,7 +937,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
 		struct scatterlist sg;
 		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
 
-		sg_init_one(&sg, cpu_addr, vrp->buf_size);
+		rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
 
 		err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
 					  GFP_KERNEL);
-- 
1.9.1

Re: [PATCH v4 2/5] rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid kernel address

[Suman Anna]

Hi Bjorn,

On 03/28/2017 06:49 AM, Loic Pallardy wrote:
> To specify memory for remoteproc, we declare (dma_declare_coherent_memory())
> an area which is ioremap'ed to the vmalloc area.  However, this address is
> not a kernel address so virt_addr_valid(buf) fails.
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> Acked-by: Patrice Chotard <patrice.chotard@st.com>
> Tested-by: Suman Anna <s-anna@ti.com>

Can you pick up the first 2 patches from this series for v4.14? This
does enable us to use HighMem CMA pools or carveout pools with
virtio_rpmsg_bus, and have been sitting on the lists for some time in
Acked/Tested status.

regards
Suman

> ---
> Changes since V1:
> - Remove extra line.
> 
> No change since v2.
> 
> ---
>  drivers/rpmsg/virtio_rpmsg_bus.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 3f44a03..1c7cde9 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -195,6 +195,28 @@ static int virtio_rpmsg_trysend_offchannel(struct rpmsg_endpoint *ept, u32 src,
>  };
>  
>  /**
> + * rpmsg_sg_init - initialize scatterlist according to cpu address location
> + * @sg: scatterlist to fill
> + * @cpu_addr: virtual address of the buffer
> + * @len: buffer length
> + *
> + * An internal function filling scatterlist according to virtual address
> + * location (in vmalloc or in kernel).
> + */
> +static void
> +rpmsg_sg_init(struct scatterlist *sg, void *cpu_addr, unsigned int len)
> +{
> +	if (is_vmalloc_addr(cpu_addr)) {
> +		sg_init_table(sg, 1);
> +		sg_set_page(sg, vmalloc_to_page(cpu_addr), len,
> +			    offset_in_page(cpu_addr));
> +	} else {
> +		WARN_ON(!virt_addr_valid(cpu_addr));
> +		sg_init_one(sg, cpu_addr, len);
> +	}
> +}
> +
> +/**
>   * __ept_release() - deallocate an rpmsg endpoint
>   * @kref: the ept's reference count
>   *
> @@ -606,7 +628,7 @@ static int rpmsg_send_offchannel_raw(struct rpmsg_device *rpdev,
>  			 msg, sizeof(*msg) + msg->len, true);
>  #endif
>  
> -	sg_init_one(&sg, msg, sizeof(*msg) + len);
> +	rpmsg_sg_init(&sg, msg, sizeof(*msg) + len);
>  
>  	mutex_lock(&vrp->tx_lock);
>  
> @@ -731,7 +753,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>  		dev_warn(dev, "msg received with no recipient\n");
>  
>  	/* publish the real size of the buffer */
> -	sg_init_one(&sg, msg, vrp->buf_size);
> +	rpmsg_sg_init(&sg, msg, vrp->buf_size);
>  
>  	/* add the buffer back to the remote processor's virtqueue */
>  	err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
> @@ -915,7 +937,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
>  		struct scatterlist sg;
>  		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>  
> -		sg_init_one(&sg, cpu_addr, vrp->buf_size);
> +		rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
>  
>  		err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>  					  GFP_KERNEL);
> 

[PATCH v4 3/5] include: virtio_rpmsg: add virtio rpmsg configuration structure

[Loic Pallardy]

Rpmsg channel configuration should be identical on host and
coprocessor side.
This patch proposes a new structure named struct virtio_rpmsg_cfg
to gather all configuration information to characterize the
communication link and.
This structure will be exchanged with coprocessor via the resource
table, expanding struct fw_rsc_vdev. It will guarantee that host
and coprocessor will share the same information.
virtio_rpmsg will access it thanks to virtio get and set features.
Presence of struct virtio_rpmsg_cfg is optional.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes since V3:
- remove va field from struct virtio_rpmsg_cfg

---
 include/linux/rpmsg/virtio_rpmsg.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 include/linux/rpmsg/virtio_rpmsg.h

diff --git a/include/linux/rpmsg/virtio_rpmsg.h b/include/linux/rpmsg/virtio_rpmsg.h
new file mode 100644
index 0000000..2d31636
--- /dev/null
+++ b/include/linux/rpmsg/virtio_rpmsg.h
@@ -0,0 +1,30 @@
+
+#ifndef _LINUX_VIRTIO_RPMSG_H
+#define _LINUX_VIRTIO_RPMSG_H
+
+/* Offset in struct fw_rsc_vdev */
+#define RPMSG_CONFIG_OFFSET	0
+
+/**
+ * struct virtio_rpmsg_cfg - optional configuration field for virtio rpmsg
+ * provided at probe time by virtio (get/set)
+ * @id: virtio cfg id (as in virtio_ids.h)
+ * @version: virtio_rpmsg_cfg structure version number
+ * @da: device address
+ * @pa: physical address
+ * @len: length (in bytes)
+ * @buf_size: size of rpmsg buffer size (defined by firmware else default value
+ *	      used)
+ * @reserved: reserved (must be zero)
+ */
+struct virtio_rpmsg_cfg {
+	u32 id;
+	u32 version;
+	u32 da;
+	u32 pa;
+	u32 len;
+	u32 buf_size;
+	u32 reserved;
+} __packed;
+
+#endif
-- 
1.9.1

[PATCH v4 4/5] rpmsg: virtio_rpmsg: get buffer configuration from virtio

[Loic Pallardy]

Some coprocessors have memory mapping constraints which require
predefined buffer location or specific buffer size different from
default definition.
Coprocessor resources are defined in associated firmware resource table.
Remoteproc offers access to firmware resource table via virtio get
interface.

This patch modifies rpmsg_probe sequence to:
- retrieve rpmsg buffer configuration (if any)
- verify and apply configuration
- allocate buffer according to requested configuration

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes since V1:
- Move Rpmsg buffer physical address initialization in patch 5 "rpmsg: virtio_rpmsg:
don't allocate buffer if provided by low level driver"
- Remove extra lines

No change since v2.

---
 drivers/rpmsg/virtio_rpmsg_bus.c | 48 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 1c7cde9..69285c1 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -32,6 +32,7 @@
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/rpmsg.h>
+#include <linux/rpmsg/virtio_rpmsg.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
 
@@ -870,6 +871,44 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
 	return 0;
 }
 
+static int virtio_rpmsg_get_config(struct virtio_device *vdev)
+{
+	struct virtio_rpmsg_cfg virtio_cfg;
+	struct virtproc_info *vrp = vdev->priv;
+	size_t total_buf_space;
+	int ret = 0;
+
+	memset(&virtio_cfg, 0, sizeof(virtio_cfg));
+	vdev->config->get(vdev, RPMSG_CONFIG_OFFSET, &virtio_cfg,
+			  sizeof(virtio_cfg));
+
+	if (virtio_cfg.id == VIRTIO_ID_RPMSG && virtio_cfg.version == 1 &&
+	    virtio_cfg.reserved == 0) {
+		if (virtio_cfg.buf_size <= MAX_RPMSG_BUF_SIZE) {
+			vrp->buf_size = virtio_cfg.buf_size;
+		} else {
+			WARN_ON(1);
+			dev_warn(&vdev->dev, "Requested RPMsg buffer size too big: %d\n",
+				 vrp->buf_size);
+			ret = -EINVAL;
+			goto out;
+		}
+
+		/* Check rpmsg buffer length */
+		total_buf_space = vrp->num_bufs * vrp->buf_size;
+		if ((virtio_cfg.len != -1) &&
+		    (total_buf_space > virtio_cfg.len)) {
+			dev_warn(&vdev->dev, "Not enough memory for buffers: %d\n",
+				 total_buf_space);
+			ret = -ENOMEM;
+			goto out;
+		}
+		return !ret;
+	}
+out:
+	return ret;
+}
+
 static int rpmsg_probe(struct virtio_device *vdev)
 {
 	vq_callback_t *vq_cbs[] = { rpmsg_recv_done, rpmsg_xmit_done };
@@ -900,6 +939,8 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	vrp->rvq = vqs[0];
 	vrp->svq = vqs[1];
 
+	vdev->priv = vrp;
+
 	/* we expect symmetric tx/rx vrings */
 	WARN_ON(virtqueue_get_vring_size(vrp->rvq) !=
 		virtqueue_get_vring_size(vrp->svq));
@@ -912,6 +953,11 @@ static int rpmsg_probe(struct virtio_device *vdev)
 
 	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
 
+	/* Try to get rpmsg configuration if any */
+	err = virtio_rpmsg_get_config(vdev);
+	if (err < 0)
+		goto free_vrp;
+
 	total_buf_space = vrp->num_bufs * vrp->buf_size;
 
 	/* allocate coherent memory for the buffers */
@@ -947,8 +993,6 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	/* suppress "tx-complete" interrupts */
 	virtqueue_disable_cb(vrp->svq);
 
-	vdev->priv = vrp;
-
 	/* if supported by the remote processor, enable the name service */
 	if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_NS)) {
 		/* a dedicated endpoint handles the name service msgs */
-- 
1.9.1

[PATCH v4 5/5] rpmsg: virtio_rpmsg: set buffer configuration to virtio

[Loic Pallardy]

Rpmsg is allocating buffer one dedicated communication link
with some specificity like number of buffers, size of one buffer...
These characteristics should be shared with remote coprocessor to
guarantee communication link coherency.

Proposal is to update rpmsg configuration fields in coprocessor firmware
resource table if it exists.

This is possible thanks to virtio set interface which allows to update
cfg fields of struct fw_rsc_vdev.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes since V1:
- Set virtio_cfg.da to -1 (any address) by default
- Add comment about IOMMU support

No change since v2.

---
 drivers/rpmsg/virtio_rpmsg_bus.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 69285c1..3e41322 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -909,6 +909,27 @@ static int virtio_rpmsg_get_config(struct virtio_device *vdev)
 	return ret;
 }
 
+static void virtio_rpmsg_set_config(struct virtio_device *vdev)
+{
+	struct virtio_rpmsg_cfg virtio_cfg;
+	struct virtproc_info *vrp = vdev->priv;
+
+	/* fill virtio_cfg struct */
+	memset(&virtio_cfg, 0, sizeof(virtio_cfg));
+	virtio_cfg.id = VIRTIO_ID_RPMSG;
+	/*
+	 * IOMMU not managed at the time being, set device address to (-1)
+	 * meaning any address value.
+	 */
+	virtio_cfg.da = -1;
+	virtio_cfg.pa =	vrp->bufs_dma;
+	virtio_cfg.len = vrp->num_bufs * vrp->buf_size;
+	virtio_cfg.buf_size = vrp->buf_size;
+
+	vdev->config->set(vdev, RPMSG_CONFIG_OFFSET, &virtio_cfg,
+			  sizeof(virtio_cfg));
+}
+
 static int rpmsg_probe(struct virtio_device *vdev)
 {
 	vq_callback_t *vq_cbs[] = { rpmsg_recv_done, rpmsg_xmit_done };
@@ -919,6 +940,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	int err = 0, i;
 	size_t total_buf_space;
 	bool notify;
+	bool has_cfg = false;
 
 	vrp = kzalloc(sizeof(*vrp), GFP_KERNEL);
 	if (!vrp)
@@ -958,6 +980,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	if (err < 0)
 		goto free_vrp;
 
+	if (err)
+		has_cfg = true;
+
 	total_buf_space = vrp->num_bufs * vrp->buf_size;
 
 	/* allocate coherent memory for the buffers */
@@ -978,6 +1003,10 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	/* and half is dedicated for TX */
 	vrp->sbufs = bufs_va + total_buf_space / 2;
 
+	/* Notify configuration to coprocessor */
+	if (has_cfg)
+		virtio_rpmsg_set_config(vdev);
+
 	/* set up the receive buffers */
 	for (i = 0; i < vrp->num_bufs / 2; i++) {
 		struct scatterlist sg;
-- 
1.9.1

Re: [PATCH v4 0/5] virtio_rpmsg: make rpmsg channel configurable

[Loic Pallardy]

On 03/28/2017 01:49 PM, Loic Pallardy wrote:
> This goal of this series is to offer more flexibility for virtio_rpmsg
> communication link configuration.
>
> It should be possible to tune rpmsg buffer size per Rpmsg link, to provide
> selected configuration to coprocessor, to take into account coprocessor
> specificities like memory region.
>
> This series introduces an optional virtio rpmsg configuration structure, which
> will be managed as an extension of struct fw_rsc_vdev present in coprocessor firmware
> resource table.
> Structure access is possible thanks to virtio get and set API.
> This structure contains the different information to tune the link between the
> host and the coprocessor.
>
> Patch "rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid
> kernel address" has the same goal as Wendy's RFC [1]. I don't have tested
> Wendy's series with level driver buffer allocation.
>
> Last patch "rpmsg: virtio_rpmsg: set buffer configuration to virtio" has a
> dependency with remoteproc subdevice boot sequence as coprocessor resource
> table must be updated before coprocessor start. See [2]
>
> [1] http://www.spinics.net/lists/linux-remoteproc/msg00852.html
> [2] http://www.spinics.net/lists/linux-remoteproc/msg00826.html
>
> ---
> Changes from v1:
> - No more dependency with [2]. Coprocessor firmware should wait for first kick
> indicating virtio rpmsg link ready before accessing resource table information.
> - Correct issues reported by Suman
> - Fix warnings found by checkpatch with --strict option
>
> Changes from v2:
> - Fix warnings reported by kbuild test robot
>
> Changes from v3:
> After discussions with Bjorn, virtual address sharing between low level driver
> and virtio_rpmsg could be replaced by associated a dedicated memory region to
> vdev and changing virtio_rpmsg allocation from dev->parent->parent to
> dev->parent, i.e. vdev and not rproc device.
> The advantage of the solution is that there will be only few changes in
> virtio_rpmsg layer and it will be common with virtio_console driver.
> These will be part of a dedicated patch series.
> So v4 drops previous patch 5 "rpmsg: virtio_rpmsg: don't allocate buffer if
> provided by low level driver" dealing with buffer previously allocated and va
> fields from struct virtio_rpmsg_cfg.
> In addition, patch series has been rebased on kernel 4.11-rc4.
>
Hi Bjorn,

Any time to review this series?

Regards,
Loic

>
> Loic Pallardy (5):
>   rpmsg: virtio_rpmsg: set rpmsg_buf_size customizable
>   rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid
>     kernel address
>   include: virtio_rpmsg: add virtio rpmsg configuration structure
>   rpmsg: virtio_rpmsg: get buffer configuration from virtio
>   rpmsg: virtio_rpmsg: set buffer configuration to virtio
>
>  drivers/rpmsg/virtio_rpmsg_bus.c   | 123 +++++++++++++++++++++++++++++++++----
>  include/linux/rpmsg/virtio_rpmsg.h |  30 +++++++++
>  2 files changed, 141 insertions(+), 12 deletions(-)
>  create mode 100644 include/linux/rpmsg/virtio_rpmsg.h
>