> -----Original Message----- > From: Suman Anna [mailto:s-anna@xxxxxx] > Sent: Saturday, January 14, 2017 2:40 AM > To: Loic PALLARDY <loic.pallardy@xxxxxx>; bjorn.andersson@xxxxxxxxxx; > ohad@xxxxxxxxxx; lee.jones@xxxxxxxxxx; Patrice CHOTARD > <patrice.chotard@xxxxxx> > Cc: linux-remoteproc@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxx > Subject: Re: [PATCH v1 1/6] rpmsg: virtio_rpmsg: set rpmsg_buf_size > customizable > > On 12/07/2016 02:35 PM, 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. > > This patch breaks rpmsg functionality w.r.t bisectability. The new buf_size > variable is not initialized (is 0) in this patch while you replace the > RPMSG_BUF_SIZE with vrp->buf_size in most call sites. You introduce the > initialization in Patch 4, that should be moved up. The ops to override can > stay in Patch 4. Exact vrp->buf_size should be forced to MAX_RPMSG_BUF_SIZE (512) to preserve virtio_rpmsg behavior. Thanks, Loic > > regards > Suman > > > > > Signed-off-by: Loic Pallardy <loic.pallardy@xxxxxx> > > --- > > drivers/rpmsg/virtio_rpmsg_bus.c | 22 ++++++++++++---------- > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c > > b/drivers/rpmsg/virtio_rpmsg_bus.c > > index 3090b0d..1f6dfc6 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,8 +698,8 @@ 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 || > > - msg->len > (len - sizeof(struct rpmsg_hdr))) { > > + 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,7 @@ 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; > > + 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 +911,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 +978,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); > > -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
