diff mbox series

[v2,5/6] rpmsg: virtio_rpmsg: don't allocate buffer if provided by low level driver

Message ID 1485867097-6960-6-git-send-email-loic.pallardy@st.com
State New
Headers show
Series virtio_rpmsg: make rpmsg channel configurable | expand

Commit Message

Loic Pallardy Jan. 31, 2017, 12:51 p.m. UTC 
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
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 55 +++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 18 deletions(-)

-- 
1.9.1

Comments

kernel test robot Jan. 31, 2017, 6:29 p.m. UTC | #1 
Hi Loic,

[auto build test WARNING on v4.9-rc8]
[also build test WARNING on next-20170130]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Loic-Pallardy/virtio_rpmsg-make-rpmsg-channel-configurable/20170131-212156
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from arch/arm/include/asm/page.h:171:0,
                    from arch/arm/include/asm/thread_info.h:17,
                    from include/linux/thread_info.h:58,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/arm/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/stat.h:18,
                    from include/linux/module.h:10,
                    from drivers/rpmsg/virtio_rpmsg_bus.c:23:
   drivers/rpmsg/virtio_rpmsg_bus.c: In function 'rpmsg_probe':
>> include/asm-generic/getorder.h:17:6: warning: 'total_buf_space' may be used uninitialized in this function [-Wmaybe-uninitialized]

     size--;
     ~~~~^~
   drivers/rpmsg/virtio_rpmsg_bus.c:934:9: note: 'total_buf_space' was declared here
     size_t total_buf_space;
            ^~~~~~~~~~~~~~~
   In file included from drivers/rpmsg/virtio_rpmsg_bus.c:28:0:
>> include/linux/dma-mapping.h:481:6: warning: 'bufs_va' may be used uninitialized in this function [-Wmaybe-uninitialized]

     if (dma_release_from_coherent(dev, get_order(size), cpu_addr))
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/rpmsg/virtio_rpmsg_bus.c:932:8: note: 'bufs_va' was declared here
     void *bufs_va;
           ^~~~~~~

vim +/total_buf_space +17 include/asm-generic/getorder.h

5b17e1cd include/asm-generic/getorder.h Arnd Bergmann    2009-05-13   1  #ifndef __ASM_GENERIC_GETORDER_H
5b17e1cd include/asm-generic/getorder.h Arnd Bergmann    2009-05-13   2  #define __ASM_GENERIC_GETORDER_H
fd4fd5aa include/asm-generic/page.h     Stephen Rothwell 2005-09-03   3  
fd4fd5aa include/asm-generic/page.h     Stephen Rothwell 2005-09-03   4  #ifndef __ASSEMBLY__
fd4fd5aa include/asm-generic/page.h     Stephen Rothwell 2005-09-03   5  
38f33230 include/asm-generic/page.h     Linus Torvalds   2007-03-06   6  #include <linux/compiler.h>
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20   7  #include <linux/log2.h>
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20   8  
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20   9  /*
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  10   * Runtime evaluation of get_order()
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  11   */
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  12  static inline __attribute_const__
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  13  int __get_order(unsigned long size)
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  14  {
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  15  	int order;
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  16  
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20 @17  	size--;
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  18  	size >>= PAGE_SHIFT;
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  19  #if BITS_PER_LONG == 32
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  20  	order = fls(size);
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  21  #else
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  22  	order = fls64(size);
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  23  #endif
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  24  	return order;
d66acc39 include/asm-generic/getorder.h David Howells    2012-02-20  25  }

:::::: The code at line 17 was first introduced by commit
:::::: d66acc39c7cee323733c8503b9de1821a56dff7e bitops: Optimise get_order()

:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: H. Peter Anvin <hpa@zytor.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 080ee07..7c89210 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:
@@ -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);
 }