Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1571648 > unrolled thread

[PATCH v3 0/6] virtio_rpmsg: make rpmsg channel configurable

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/6] virtio_rpmsg: make rpmsg channel configurable Loic Pallardy <loic.pallardy@st.com> - 2017-02-01 16:10 +0100
    [PATCH v3 5/6] rpmsg: virtio_rpmsg: don't allocate buffer if provided by low level driver Loic Pallardy <loic.pallardy@st.com> - 2017-02-01 16:10 +0100
    [PATCH v3 6/6] rpmsg: virtio_rpmsg: set buffer configuration to virtio Loic Pallardy <loic.pallardy@st.com> - 2017-02-01 16:10 +0100
    [PATCH v3 4/6] rpmsg: virtio_rpmsg: get buffer configuration from virtio Loic Pallardy <loic.pallardy@st.com> - 2017-02-01 16:10 +0100
    [PATCH v3 2/6] rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid kernel address Loic Pallardy <loic.pallardy@st.com> - 2017-02-01 16:10 +0100

#1571648 — [PATCH v3 0/6] virtio_rpmsg: make rpmsg channel configurable

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

Loic Pallardy (6):
  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: don't allocate buffer if provided by low level
    driver
  rpmsg: virtio_rpmsg: set buffer configuration to virtio

 drivers/rpmsg/virtio_rpmsg_bus.c   | 180 ++++++++++++++++++++++++++++++-------
 include/linux/rpmsg/virtio_rpmsg.h |  32 +++++++
 2 files changed, 181 insertions(+), 31 deletions(-)
 create mode 100644 include/linux/rpmsg/virtio_rpmsg.h

-- 
1.9.1

[toc] | [next] | [standalone]

#1571649 — [PATCH v3 5/6] rpmsg: virtio_rpmsg: don't allocate buffer if provided by low level driver

Low level platform specific driver has the knowledge of the different
communication link constraints like fixed or secured memory region to
use for buffer allocation.

If virtual address is defined in virtio_rpmsg_cfg structure, a dedicated
memory pool buffer fitting platform requirements is available.
Rpmsg virtio layer should rely on it if its size is compliant with link
characteristics.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
Changes since V1:
- Alignment with open parenthesis.
- Add vrp->bufs_dma intialization in virtio_rpmsg_get_config function

Changes since V2:
- Fix warnings reported by kbuild test robot
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 59 ++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 080ee07..b347e34 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -57,6 +57,7 @@
  * @sendq:	wait queue of sending contexts waiting for a tx buffers
  * @sleepers:	number of senders that are waiting for a tx buffer
  * @ns_ept:	the bus's name service endpoint
+ * @ext_buffer: buffer allocated by low level driver
  *
  * This structure stores the rpmsg state of a given virtio remote processor
  * device (there might be several virtio proc devices for each physical
@@ -76,6 +77,7 @@ struct virtproc_info {
 	wait_queue_head_t sendq;
 	atomic_t sleepers;
 	struct rpmsg_endpoint *ns_ept;
+	bool ext_buffer;
 };
 
 /* The feature bitmap for virtio rpmsg */
@@ -893,6 +895,7 @@ static int virtio_rpmsg_get_config(struct virtio_device *vdev)
 			ret = -EINVAL;
 			goto out;
 		}
+		vrp->bufs_dma = virtio_cfg.pa;
 
 		/* Check rpmsg buffer length */
 		total_buf_space = vrp->num_bufs * vrp->buf_size;
@@ -903,6 +906,17 @@ static int virtio_rpmsg_get_config(struct virtio_device *vdev)
 			ret = -ENOMEM;
 			goto out;
 		}
+
+		/* Level platform specific buffer driver ? */
+		if (virtio_cfg.va != -1) {
+			vrp->ext_buffer = true;
+			/* half of the buffers is dedicated for RX */
+			vrp->rbufs = (void *)(uintptr_t)virtio_cfg.va;
+
+			/* and half is dedicated for TX */
+			vrp->sbufs = (void *)(uintptr_t)virtio_cfg.va + total_buf_space / 2;
+		}
+
 		return !ret;
 	}
 out:
@@ -915,9 +929,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	static const char * const names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
 	struct virtproc_info *vrp;
-	void *bufs_va;
+	void *bufs_va = NULL;
 	int err = 0, i;
-	size_t total_buf_space;
+	size_t total_buf_space = 0;
 	bool notify;
 
 	vrp = kzalloc(sizeof(*vrp), GFP_KERNEL);
@@ -958,25 +972,28 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	if (err < 0)
 		goto free_vrp;
 
-	total_buf_space = vrp->num_bufs * vrp->buf_size;
+	/* Allocate buffer if none provided by low level platform driver */
+	if (!vrp->ext_buffer) {
+		total_buf_space = vrp->num_bufs * vrp->buf_size;
 
 	/* allocate coherent memory for the buffers */
-	bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
-				     total_buf_space, &vrp->bufs_dma,
-				     GFP_KERNEL);
-	if (!bufs_va) {
-		err = -ENOMEM;
-		goto vqs_del;
-	}
+		bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
+					     total_buf_space, &vrp->bufs_dma,
+					     GFP_KERNEL);
+		if (!bufs_va) {
+			err = -ENOMEM;
+			goto vqs_del;
+		}
 
-	dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
-		bufs_va, &vrp->bufs_dma);
+		dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
+			bufs_va, &vrp->bufs_dma);
 
-	/* half of the buffers is dedicated for RX */
-	vrp->rbufs = bufs_va;
+		/* half of the buffers is dedicated for RX */
+		vrp->rbufs = bufs_va;
 
-	/* and half is dedicated for TX */
-	vrp->sbufs = bufs_va + total_buf_space / 2;
+		/* and half is dedicated for TX */
+		vrp->sbufs = bufs_va + total_buf_space / 2;
+	}
 
 	/* set up the receive buffers */
 	for (i = 0; i < vrp->num_bufs / 2; i++) {
@@ -1027,8 +1044,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	return 0;
 
 free_coherent:
-	dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
-			  bufs_va, vrp->bufs_dma);
+	if (!vrp->ext_buffer)
+		dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
+				  bufs_va, vrp->bufs_dma);
 vqs_del:
 	vdev->config->del_vqs(vrp->vdev);
 free_vrp:
@@ -1062,8 +1080,9 @@ static void rpmsg_remove(struct virtio_device *vdev)
 
 	vdev->config->del_vqs(vrp->vdev);
 
-	dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
-			  vrp->rbufs, vrp->bufs_dma);
+	if (!vrp->ext_buffer)
+		dma_free_coherent(vdev->dev.parent->parent, total_buf_space,
+				  vrp->rbufs, vrp->bufs_dma);
 
 	kfree(vrp);
 }
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1571651 — [PATCH v3 6/6] rpmsg: virtio_rpmsg: set buffer configuration to virtio

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 b347e34..d9af556 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -923,6 +923,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 };
@@ -933,6 +954,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	int err = 0, i;
 	size_t total_buf_space = 0;
 	bool notify;
+	bool has_cfg = false;
 
 	vrp = kzalloc(sizeof(*vrp), GFP_KERNEL);
 	if (!vrp)
@@ -972,6 +994,9 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	if (err < 0)
 		goto free_vrp;
 
+	if (err)
+		has_cfg = true;
+
 	/* Allocate buffer if none provided by low level platform driver */
 	if (!vrp->ext_buffer) {
 		total_buf_space = vrp->num_bufs * vrp->buf_size;
@@ -993,6 +1018,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 */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1571652 — [PATCH v3 4/6] rpmsg: virtio_rpmsg: get buffer configuration from virtio

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 c8d0d6d..080ee07 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

[toc] | [prev] | [next] | [standalone]

#1571656 — [PATCH v3 2/6] rpmsg: virtio_rpmsg_bus: fix sg_set_buf() when addr is not a valid kernel address

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 d387589..c8d0d6d 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web