[RFC PATCH 0/2] Enhance RPMsg buffer management

[RFC PATCH 0/2] Enhance RPMsg buffer management

[Tanmay Shah]

Take rx and tx buffer size from virtio device config space in the
resource table. This allows each firmware to configure RPMsg buffer size
as needed in each direction.

Original seris:
https://lore.kernel.org/all/1548949280-31794-1-git-send-email-xiaoxiang@xiaomi.com/

Following modificaitons are done to the original series:
  - Separate dma allocation is not done for tx and rx buffers. Instead
    allocated chunk of memory is split between tx and rx buffers.
  - If vdev doesn't support VIRTIO_RPMSG_F_BUFSZ feature then use the 
    default size of 512 bytes for buffers
  - Change MAX_RPMSG_BUF_SIZE to DEFAULT_RPMSG_BUF_SIZE
  - move virtio_rpmsg.h from uapi to linux dir
  - RPMsg buffer size must be set to hold rpmsg header at minimum in the
    vdev config space of the firmware.
  - align total buf size to page size when allocating and deallocating
    memory

Xiang Xiao (2):
  rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv
  rpmsg: virtio_rpmsg_bus: get buffer size from config space

 drivers/rpmsg/virtio_rpmsg_bus.c | 105 ++++++++++++++++++++-----------
 include/linux/virtio_rpmsg.h     |  24 +++++++
 2 files changed, 93 insertions(+), 36 deletions(-)
 create mode 100644 include/linux/virtio_rpmsg.h


base-commit: f982fbb1a6ca3553c15763ad9eb2beeae78d3684
-- 
2.34.1

[RFC PATCH 1/2] rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv

[Tanmay Shah]

From: Xiang Xiao <xiaoxiang781216@gmail.com>

it's useful if the communication throughput is different from each side

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 47 ++++++++++++++++----------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 484890b4a6a7..cc26dfcc3e29 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -37,8 +37,9 @@
  * @svq:	tx virtqueue
  * @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
+ * @num_rbufs:	total number of buffers for rx
+ * @num_sbufs:	total number of buffers for 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.
@@ -57,7 +58,8 @@ struct virtproc_info {
 	struct virtio_device *vdev;
 	struct virtqueue *rvq, *svq;
 	void *rbufs, *sbufs;
-	unsigned int num_bufs;
+	unsigned int num_rbufs;
+	unsigned int num_sbufs;
 	unsigned int buf_size;
 	int last_sbuf;
 	dma_addr_t bufs_dma;
@@ -112,7 +114,7 @@ struct virtio_rpmsg_channel {
 /*
  * We're allocating buffers of 512 bytes each for communications. The
  * number of buffers will be computed from the number of buffers supported
- * by the vring, upto a maximum of 512 buffers (256 in each direction).
+ * by the vring, up to a maximum of 256 in each direction.
  *
  * Each buffer will have 16 bytes for the msg header and 496 bytes for
  * the payload.
@@ -127,7 +129,7 @@ struct virtio_rpmsg_channel {
  * can change this without changing anything in the firmware of the remote
  * processor.
  */
-#define MAX_RPMSG_NUM_BUFS	(512)
+#define MAX_RPMSG_NUM_BUFS	(256)
 #define MAX_RPMSG_BUF_SIZE	(512)
 
 /*
@@ -439,11 +441,8 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
 	/* support multiple concurrent senders */
 	mutex_lock(&vrp->tx_lock);
 
-	/*
-	 * either pick the next unused tx buffer
-	 * (half of our buffers are used for sending messages)
-	 */
-	if (vrp->last_sbuf < vrp->num_bufs / 2)
+	/* either pick the next unused tx buffer */
+	if (vrp->last_sbuf < vrp->num_sbufs)
 		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
 	/* or recycle a used one */
 	else
@@ -878,19 +877,20 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	vrp->rvq = vqs[0];
 	vrp->svq = vqs[1];
 
-	/* we expect symmetric tx/rx vrings */
-	WARN_ON(virtqueue_get_vring_size(vrp->rvq) !=
-		virtqueue_get_vring_size(vrp->svq));
-
 	/* we need less buffers if vrings are small */
-	if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS / 2)
-		vrp->num_bufs = virtqueue_get_vring_size(vrp->rvq) * 2;
+	if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS)
+		vrp->num_rbufs = virtqueue_get_vring_size(vrp->rvq);
+	else
+		vrp->num_rbufs = MAX_RPMSG_NUM_BUFS;
+
+	if (virtqueue_get_vring_size(vrp->svq) < MAX_RPMSG_NUM_BUFS)
+		vrp->num_sbufs = virtqueue_get_vring_size(vrp->svq);
 	else
-		vrp->num_bufs = MAX_RPMSG_NUM_BUFS;
+		vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
 
 	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
 
-	total_buf_space = vrp->num_bufs * vrp->buf_size;
+	total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
 
 	/* allocate coherent memory for the buffers */
 	bufs_va = dma_alloc_coherent(vdev->dev.parent,
@@ -904,14 +904,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
 		bufs_va, &vrp->bufs_dma);
 
-	/* half of the buffers is dedicated for RX */
+	/* first part 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 second part is dedicated for TX */
+	vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
 
 	/* set up the receive buffers */
-	for (i = 0; i < vrp->num_bufs / 2; i++) {
+	for (i = 0; i < vrp->num_rbufs; i++) {
 		struct scatterlist sg;
 		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
 
@@ -1001,7 +1001,8 @@ 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 * vrp->buf_size;
+	unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
+	size_t total_buf_space = num_bufs * vrp->buf_size;
 	int ret;
 
 	virtio_reset_device(vdev);
-- 
2.34.1

Re: [RFC PATCH 1/2] rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv

[Arnaud POULIQUEN]

Hi Tanmay,

On 11/14/25 19:46, Tanmay Shah wrote:
> From: Xiang Xiao <xiaoxiang781216@gmail.com>
> 
> it's useful if the communication throughput is different from each side
> 
> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>


Thanks you for reopening of this series!
> ---
>   drivers/rpmsg/virtio_rpmsg_bus.c | 47 ++++++++++++++++----------------
>   1 file changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 484890b4a6a7..cc26dfcc3e29 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -37,8 +37,9 @@
>    * @svq:	tx virtqueue
>    * @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
> + * @num_rbufs:	total number of buffers for rx
> + * @num_sbufs:	total number of buffers for tx

Nitpicking... I wonder if rx, tx term would be more generic
In such case s/num_rxbufs/n_rx_buf/ s/num_sbufs/num_txbufs/

Regards
Arnaud

> + * @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.
> @@ -57,7 +58,8 @@ struct virtproc_info {
>   	struct virtio_device *vdev;
>   	struct virtqueue *rvq, *svq;
>   	void *rbufs, *sbufs;
> -	unsigned int num_bufs;
> +	unsigned int num_rbufs;
> +	unsigned int num_sbufs;
>   	unsigned int buf_size;
>   	int last_sbuf;
>   	dma_addr_t bufs_dma;
> @@ -112,7 +114,7 @@ struct virtio_rpmsg_channel {
>   /*
>    * We're allocating buffers of 512 bytes each for communications. The
>    * number of buffers will be computed from the number of buffers supported
> - * by the vring, upto a maximum of 512 buffers (256 in each direction).
> + * by the vring, up to a maximum of 256 in each direction.
>    *
>    * Each buffer will have 16 bytes for the msg header and 496 bytes for
>    * the payload.
> @@ -127,7 +129,7 @@ struct virtio_rpmsg_channel {
>    * can change this without changing anything in the firmware of the remote
>    * processor.
>    */
> -#define MAX_RPMSG_NUM_BUFS	(512)
> +#define MAX_RPMSG_NUM_BUFS	(256)
>   #define MAX_RPMSG_BUF_SIZE	(512)
>   
>   /*
> @@ -439,11 +441,8 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>   	/* support multiple concurrent senders */
>   	mutex_lock(&vrp->tx_lock);
>   
> -	/*
> -	 * either pick the next unused tx buffer
> -	 * (half of our buffers are used for sending messages)
> -	 */
> -	if (vrp->last_sbuf < vrp->num_bufs / 2)
> +	/* either pick the next unused tx buffer */
> +	if (vrp->last_sbuf < vrp->num_sbufs)
>   		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
>   	/* or recycle a used one */
>   	else
> @@ -878,19 +877,20 @@ static int rpmsg_probe(struct virtio_device *vdev)
>   	vrp->rvq = vqs[0];
>   	vrp->svq = vqs[1];
>   
> -	/* we expect symmetric tx/rx vrings */
> -	WARN_ON(virtqueue_get_vring_size(vrp->rvq) !=
> -		virtqueue_get_vring_size(vrp->svq));
> -
>   	/* we need less buffers if vrings are small */
> -	if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS / 2)
> -		vrp->num_bufs = virtqueue_get_vring_size(vrp->rvq) * 2;
> +	if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS)
> +		vrp->num_rbufs = virtqueue_get_vring_size(vrp->rvq);
> +	else
> +		vrp->num_rbufs = MAX_RPMSG_NUM_BUFS;
> +
> +	if (virtqueue_get_vring_size(vrp->svq) < MAX_RPMSG_NUM_BUFS)
> +		vrp->num_sbufs = virtqueue_get_vring_size(vrp->svq);
>   	else
> -		vrp->num_bufs = MAX_RPMSG_NUM_BUFS;
> +		vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>   
>   	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>   
> -	total_buf_space = vrp->num_bufs * vrp->buf_size;
> +	total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
>   
>   	/* allocate coherent memory for the buffers */
>   	bufs_va = dma_alloc_coherent(vdev->dev.parent,
> @@ -904,14 +904,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>   	dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
>   		bufs_va, &vrp->bufs_dma);
>   
> -	/* half of the buffers is dedicated for RX */
> +	/* first part 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 second part is dedicated for TX */
> +	vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
>   
>   	/* set up the receive buffers */
> -	for (i = 0; i < vrp->num_bufs / 2; i++) {
> +	for (i = 0; i < vrp->num_rbufs; i++) {
>   		struct scatterlist sg;
>   		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>   
> @@ -1001,7 +1001,8 @@ 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 * vrp->buf_size;
> +	unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
> +	size_t total_buf_space = num_bufs * vrp->buf_size;
>   	int ret;
>   
>   	virtio_reset_device(vdev);

Re: [RFC PATCH 1/2] rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv

[Tanmay Shah]

On 11/21/25 3:40 AM, Arnaud POULIQUEN wrote:
> Hi Tanmay,
> 
> On 11/14/25 19:46, Tanmay Shah wrote:
>> From: Xiang Xiao <xiaoxiang781216@gmail.com>
>>
>> it's useful if the communication throughput is different from each side
>>
>> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> 
> 
> Thanks you for reopening of this series!
>> ---
>>   drivers/rpmsg/virtio_rpmsg_bus.c | 47 ++++++++++++++++----------------
>>   1 file changed, 24 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/ 
>> virtio_rpmsg_bus.c
>> index 484890b4a6a7..cc26dfcc3e29 100644
>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>> @@ -37,8 +37,9 @@
>>    * @svq:    tx virtqueue
>>    * @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
>> + * @num_rbufs:    total number of buffers for rx
>> + * @num_sbufs:    total number of buffers for tx
> 
> Nitpicking... I wonder if rx, tx term would be more generic
> In such case s/num_rxbufs/n_rx_buf/ s/num_sbufs/num_txbufs/
> 

Ack. I can change that.

> Regards
> Arnaud
> 
>> + * @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.
>> @@ -57,7 +58,8 @@ struct virtproc_info {
>>       struct virtio_device *vdev;
>>       struct virtqueue *rvq, *svq;
>>       void *rbufs, *sbufs;
>> -    unsigned int num_bufs;
>> +    unsigned int num_rbufs;
>> +    unsigned int num_sbufs;
>>       unsigned int buf_size;
>>       int last_sbuf;
>>       dma_addr_t bufs_dma;
>> @@ -112,7 +114,7 @@ struct virtio_rpmsg_channel {
>>   /*
>>    * We're allocating buffers of 512 bytes each for communications. The
>>    * number of buffers will be computed from the number of buffers 
>> supported
>> - * by the vring, upto a maximum of 512 buffers (256 in each direction).
>> + * by the vring, up to a maximum of 256 in each direction.
>>    *
>>    * Each buffer will have 16 bytes for the msg header and 496 bytes for
>>    * the payload.
>> @@ -127,7 +129,7 @@ struct virtio_rpmsg_channel {
>>    * can change this without changing anything in the firmware of the 
>> remote
>>    * processor.
>>    */
>> -#define MAX_RPMSG_NUM_BUFS    (512)
>> +#define MAX_RPMSG_NUM_BUFS    (256)
>>   #define MAX_RPMSG_BUF_SIZE    (512)
>>   /*
>> @@ -439,11 +441,8 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>>       /* support multiple concurrent senders */
>>       mutex_lock(&vrp->tx_lock);
>> -    /*
>> -     * either pick the next unused tx buffer
>> -     * (half of our buffers are used for sending messages)
>> -     */
>> -    if (vrp->last_sbuf < vrp->num_bufs / 2)
>> +    /* either pick the next unused tx buffer */
>> +    if (vrp->last_sbuf < vrp->num_sbufs)
>>           ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
>>       /* or recycle a used one */
>>       else
>> @@ -878,19 +877,20 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>       vrp->rvq = vqs[0];
>>       vrp->svq = vqs[1];
>> -    /* we expect symmetric tx/rx vrings */
>> -    WARN_ON(virtqueue_get_vring_size(vrp->rvq) !=
>> -        virtqueue_get_vring_size(vrp->svq));
>> -
>>       /* we need less buffers if vrings are small */
>> -    if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS / 2)
>> -        vrp->num_bufs = virtqueue_get_vring_size(vrp->rvq) * 2;
>> +    if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS)
>> +        vrp->num_rbufs = virtqueue_get_vring_size(vrp->rvq);
>> +    else
>> +        vrp->num_rbufs = MAX_RPMSG_NUM_BUFS;
>> +
>> +    if (virtqueue_get_vring_size(vrp->svq) < MAX_RPMSG_NUM_BUFS)
>> +        vrp->num_sbufs = virtqueue_get_vring_size(vrp->svq);
>>       else
>> -        vrp->num_bufs = MAX_RPMSG_NUM_BUFS;
>> +        vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>>       vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>> -    total_buf_space = vrp->num_bufs * vrp->buf_size;
>> +    total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
>>       /* allocate coherent memory for the buffers */
>>       bufs_va = dma_alloc_coherent(vdev->dev.parent,
>> @@ -904,14 +904,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>       dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
>>           bufs_va, &vrp->bufs_dma);
>> -    /* half of the buffers is dedicated for RX */
>> +    /* first part 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 second part is dedicated for TX */
>> +    vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
>>       /* set up the receive buffers */
>> -    for (i = 0; i < vrp->num_bufs / 2; i++) {
>> +    for (i = 0; i < vrp->num_rbufs; i++) {
>>           struct scatterlist sg;
>>           void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>> @@ -1001,7 +1001,8 @@ 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 * vrp->buf_size;
>> +    unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
>> +    size_t total_buf_space = num_bufs * vrp->buf_size;
>>       int ret;
>>       virtio_reset_device(vdev);
> 

Re: [RFC PATCH 1/2] rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv

[Mathieu Poirier]

Hi Tanmay,

On Fri, Nov 14, 2025 at 10:46:39AM -0800, Tanmay Shah wrote:
> From: Xiang Xiao <xiaoxiang781216@gmail.com>
>

This feature has been abandoned a long time ago - you own it now.
 
> it's useful if the communication throughput is different from each side
> 

Please provide a better changelog.

> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
>  drivers/rpmsg/virtio_rpmsg_bus.c | 47 ++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 484890b4a6a7..cc26dfcc3e29 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -37,8 +37,9 @@
>   * @svq:	tx virtqueue
>   * @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
> + * @num_rbufs:	total number of buffers for rx
> + * @num_sbufs:	total number of buffers for 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.
> @@ -57,7 +58,8 @@ struct virtproc_info {
>  	struct virtio_device *vdev;
>  	struct virtqueue *rvq, *svq;
>  	void *rbufs, *sbufs;
> -	unsigned int num_bufs;
> +	unsigned int num_rbufs;
> +	unsigned int num_sbufs;
>  	unsigned int buf_size;
>  	int last_sbuf;
>  	dma_addr_t bufs_dma;
> @@ -112,7 +114,7 @@ struct virtio_rpmsg_channel {
>  /*
>   * We're allocating buffers of 512 bytes each for communications. The
>   * number of buffers will be computed from the number of buffers supported
> - * by the vring, upto a maximum of 512 buffers (256 in each direction).
> + * by the vring, up to a maximum of 256 in each direction.
>   *
>   * Each buffer will have 16 bytes for the msg header and 496 bytes for
>   * the payload.
> @@ -127,7 +129,7 @@ struct virtio_rpmsg_channel {
>   * can change this without changing anything in the firmware of the remote
>   * processor.
>   */
> -#define MAX_RPMSG_NUM_BUFS	(512)
> +#define MAX_RPMSG_NUM_BUFS	(256)
>  #define MAX_RPMSG_BUF_SIZE	(512)
>  
>  /*
> @@ -439,11 +441,8 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>  	/* support multiple concurrent senders */
>  	mutex_lock(&vrp->tx_lock);
>  
> -	/*
> -	 * either pick the next unused tx buffer
> -	 * (half of our buffers are used for sending messages)
> -	 */
> -	if (vrp->last_sbuf < vrp->num_bufs / 2)
> +	/* either pick the next unused tx buffer */
> +	if (vrp->last_sbuf < vrp->num_sbufs)
>  		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
>  	/* or recycle a used one */
>  	else
> @@ -878,19 +877,20 @@ static int rpmsg_probe(struct virtio_device *vdev)
>  	vrp->rvq = vqs[0];
>  	vrp->svq = vqs[1];
>  
> -	/* we expect symmetric tx/rx vrings */
> -	WARN_ON(virtqueue_get_vring_size(vrp->rvq) !=
> -		virtqueue_get_vring_size(vrp->svq));
> -
>  	/* we need less buffers if vrings are small */
> -	if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS / 2)
> -		vrp->num_bufs = virtqueue_get_vring_size(vrp->rvq) * 2;
> +	if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS)
> +		vrp->num_rbufs = virtqueue_get_vring_size(vrp->rvq);
> +	else
> +		vrp->num_rbufs = MAX_RPMSG_NUM_BUFS;
> +
> +	if (virtqueue_get_vring_size(vrp->svq) < MAX_RPMSG_NUM_BUFS)
> +		vrp->num_sbufs = virtqueue_get_vring_size(vrp->svq);
>  	else
> -		vrp->num_bufs = MAX_RPMSG_NUM_BUFS;
> +		vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>  
>  	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>  
> -	total_buf_space = vrp->num_bufs * vrp->buf_size;
> +	total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
>  
>  	/* allocate coherent memory for the buffers */
>  	bufs_va = dma_alloc_coherent(vdev->dev.parent,
> @@ -904,14 +904,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>  	dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n",
>  		bufs_va, &vrp->bufs_dma);
>  
> -	/* half of the buffers is dedicated for RX */
> +	/* first part 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 second part is dedicated for TX */
> +	vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
>  
>  	/* set up the receive buffers */
> -	for (i = 0; i < vrp->num_bufs / 2; i++) {
> +	for (i = 0; i < vrp->num_rbufs; i++) {
>  		struct scatterlist sg;
>  		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>  
> @@ -1001,7 +1001,8 @@ 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 * vrp->buf_size;
> +	unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
> +	size_t total_buf_space = num_bufs * vrp->buf_size;
>  	int ret;
>  
>  	virtio_reset_device(vdev);
> -- 
> 2.34.1
> 

[RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Tanmay Shah]

From: Xiang Xiao <xiaoxiang781216@gmail.com>

512 bytes isn't always suitable for all case, let firmware
maker decide the best value from resource table.
enable by VIRTIO_RPMSG_F_BUFSZ feature bit.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
 include/linux/virtio_rpmsg.h     | 24 +++++++++++
 2 files changed, 74 insertions(+), 18 deletions(-)
 create mode 100644 include/linux/virtio_rpmsg.h

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index cc26dfcc3e29..03dd5535880a 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -25,6 +25,7 @@
 #include <linux/sched.h>
 #include <linux/virtio.h>
 #include <linux/virtio_ids.h>
+#include <linux/virtio_rpmsg.h>
 #include <linux/virtio_config.h>
 #include <linux/wait.h>
 
@@ -39,7 +40,8 @@
  * @sbufs:	kernel address of tx buffers
  * @num_rbufs:	total number of buffers for rx
  * @num_sbufs:	total number of buffers for tx
- * @buf_size:	size of one rx or tx buffer
+ * @rbuf_size:	size of one rx buffer
+ * @sbuf_size:	size of one 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.
@@ -60,7 +62,8 @@ struct virtproc_info {
 	void *rbufs, *sbufs;
 	unsigned int num_rbufs;
 	unsigned int num_sbufs;
-	unsigned int buf_size;
+	unsigned int rbuf_size;
+	unsigned int sbuf_size;
 	int last_sbuf;
 	dma_addr_t bufs_dma;
 	struct mutex tx_lock;
@@ -70,9 +73,6 @@ struct virtproc_info {
 	atomic_t sleepers;
 };
 
-/* The feature bitmap for virtio rpmsg */
-#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
-
 /**
  * struct rpmsg_hdr - common header for all rpmsg messages
  * @src: source address
@@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
  * processor.
  */
 #define MAX_RPMSG_NUM_BUFS	(256)
-#define MAX_RPMSG_BUF_SIZE	(512)
+#define DEFAULT_RPMSG_BUF_SIZE	(512)
 
 /*
  * Local addresses are dynamically allocated on-demand.
@@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
 
 	/* either pick the next unused tx buffer */
 	if (vrp->last_sbuf < vrp->num_sbufs)
-		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
+		ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
 	/* or recycle a used one */
 	else
 		ret = virtqueue_get_buf(vrp->svq, &len);
@@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
+	if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
 		dev_err(dev, "message is too big (%d)\n", len);
 		return -EMSGSIZE;
 	}
@@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct rpmsg_endpoint *ept)
 	struct rpmsg_device *rpdev = ept->rpdev;
 	struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
 
-	return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
+	return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
 }
 
 static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
@@ -706,7 +706,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 > vrp->buf_size ||
+	if (len > vrp->rbuf_size ||
 	    msg_len > (len - sizeof(struct rpmsg_hdr))) {
 		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
 		return -EINVAL;
@@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 		dev_warn_ratelimited(dev, "msg received with no recipient\n");
 
 	/* publish the real size of the buffer */
-	rpmsg_sg_init(&sg, msg, vrp->buf_size);
+	rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
 
 	/* add the buffer back to the remote processor's virtqueue */
 	err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
@@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	else
 		vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
 
-	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
+	/*
+	 * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
+	 * size from virtio device config space from the resource table.
+	 * If the feature is not supported, then assign default buf size.
+	 */
+	if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
+		/* note: virtio_rpmsg_config is defined from remote view */
+		virtio_cread(vdev, struct virtio_rpmsg_config,
+			     txbuf_size, &vrp->rbuf_size);
+		virtio_cread(vdev, struct virtio_rpmsg_config,
+			     rxbuf_size, &vrp->sbuf_size);
+
+		/* The buffers must hold rpmsg header atleast */
+		if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
+		    vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
+			dev_err(&vdev->dev,
+				"vdev config: rx buf sz = %d, tx buf sz = %d\n",
+				vrp->rbuf_size, vrp->sbuf_size);
+			err = -EINVAL;
+			goto vqs_del;
+		}
+
+		dev_dbg(&vdev->dev,
+			"vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
+			vrp->rbuf_size, vrp->sbuf_size);
+	} else {
+		vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
+		vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
+	}
 
-	total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
+	total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
+			  (vrp->num_sbufs * vrp->sbuf_size);
+	total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
 
 	/* allocate coherent memory for the buffers */
 	bufs_va = dma_alloc_coherent(vdev->dev.parent,
@@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
 	vrp->rbufs = bufs_va;
 
 	/* and second part is dedicated for TX */
-	vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
+	vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
 
 	/* set up the receive buffers */
 	for (i = 0; i < vrp->num_rbufs; i++) {
 		struct scatterlist sg;
-		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
+		void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
 
-		rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
+		rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
 
 		err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
 					  GFP_KERNEL);
@@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device *dev, void *data)
 static void rpmsg_remove(struct virtio_device *vdev)
 {
 	struct virtproc_info *vrp = vdev->priv;
-	unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
-	size_t total_buf_space = num_bufs * vrp->buf_size;
+	size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
+				 (vrp->num_sbufs * vrp->sbuf_size);
 	int ret;
 
 	virtio_reset_device(vdev);
@@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
 
 	vdev->config->del_vqs(vrp->vdev);
 
+	total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
 	dma_free_coherent(vdev->dev.parent, total_buf_space,
 			  vrp->rbufs, vrp->bufs_dma);
 
@@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
 
 static unsigned int features[] = {
 	VIRTIO_RPMSG_F_NS,
+	VIRTIO_RPMSG_F_BUFSZ,
 };
 
 static struct virtio_driver virtio_ipc_driver = {
diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
new file mode 100644
index 000000000000..6406bc505383
--- /dev/null
+++ b/include/linux/virtio_rpmsg.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) Pinecone Inc. 2019
+ * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
+ */
+
+#ifndef _LINUX_VIRTIO_RPMSG_H
+#define _LINUX_VIRTIO_RPMSG_H
+
+#include <linux/types.h>
+
+/* The feature bitmap for virtio rpmsg */
+#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
+#define VIRTIO_RPMSG_F_BUFSZ	2 /* RP get buffer size from config space */
+
+struct virtio_rpmsg_config {
+	/* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
+	__u32 txbuf_size;
+	__u32 rxbuf_size;
+	__u32 reserved[14]; /* Reserve for the future use */
+	/* Put the customize config here */
+} __attribute__((packed));
+
+#endif /* _LINUX_VIRTIO_RPMSG_H */
-- 
2.34.1

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Arnaud POULIQUEN]

On 11/14/25 19:46, Tanmay Shah wrote:
> From: Xiang Xiao <xiaoxiang781216@gmail.com>
> 
> 512 bytes isn't always suitable for all case, let firmware
> maker decide the best value from resource table.
> enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
> 
> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
>   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
>   include/linux/virtio_rpmsg.h     | 24 +++++++++++
>   2 files changed, 74 insertions(+), 18 deletions(-)
>   create mode 100644 include/linux/virtio_rpmsg.h
> 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index cc26dfcc3e29..03dd5535880a 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -25,6 +25,7 @@
>   #include <linux/sched.h>
>   #include <linux/virtio.h>
>   #include <linux/virtio_ids.h>
> +#include <linux/virtio_rpmsg.h>
>   #include <linux/virtio_config.h>
>   #include <linux/wait.h>
>   
> @@ -39,7 +40,8 @@
>    * @sbufs:	kernel address of tx buffers
>    * @num_rbufs:	total number of buffers for rx
>    * @num_sbufs:	total number of buffers for tx
> - * @buf_size:	size of one rx or tx buffer
> + * @rbuf_size:	size of one rx buffer
> + * @sbuf_size:	size of one 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.
> @@ -60,7 +62,8 @@ struct virtproc_info {
>   	void *rbufs, *sbufs;
>   	unsigned int num_rbufs;
>   	unsigned int num_sbufs;
> -	unsigned int buf_size;
> +	unsigned int rbuf_size;
> +	unsigned int sbuf_size;
>   	int last_sbuf;
>   	dma_addr_t bufs_dma;
>   	struct mutex tx_lock;
> @@ -70,9 +73,6 @@ struct virtproc_info {
>   	atomic_t sleepers;
>   };
>   
> -/* The feature bitmap for virtio rpmsg */
> -#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
> -
>   /**
>    * struct rpmsg_hdr - common header for all rpmsg messages
>    * @src: source address
> @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
>    * processor.
>    */
>   #define MAX_RPMSG_NUM_BUFS	(256)
> -#define MAX_RPMSG_BUF_SIZE	(512)
> +#define DEFAULT_RPMSG_BUF_SIZE	(512)
>   
>   /*
>    * Local addresses are dynamically allocated on-demand.
> @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>   
>   	/* either pick the next unused tx buffer */
>   	if (vrp->last_sbuf < vrp->num_sbufs)
> -		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
> +		ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
>   	/* or recycle a used one */
>   	else
>   		ret = virtqueue_get_buf(vrp->svq, &len);
> @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
> +	if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
>   		dev_err(dev, "message is too big (%d)\n", len);
>   		return -EMSGSIZE;
>   	}
> @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>   	struct rpmsg_device *rpdev = ept->rpdev;
>   	struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>   
> -	return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
> +	return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
>   }
>   
>   static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
> @@ -706,7 +706,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 > vrp->buf_size ||
> +	if (len > vrp->rbuf_size ||
>   	    msg_len > (len - sizeof(struct rpmsg_hdr))) {
>   		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
>   		return -EINVAL;
> @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>   		dev_warn_ratelimited(dev, "msg received with no recipient\n");
>   
>   	/* publish the real size of the buffer */
> -	rpmsg_sg_init(&sg, msg, vrp->buf_size);
> +	rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
>   
>   	/* add the buffer back to the remote processor's virtqueue */
>   	err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
> @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
>   	else
>   		vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>   
> -	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
> +	/*
> +	 * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
> +	 * size from virtio device config space from the resource table.
> +	 * If the feature is not supported, then assign default buf size.
> +	 */
> +	if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
> +		/* note: virtio_rpmsg_config is defined from remote view */
> +		virtio_cread(vdev, struct virtio_rpmsg_config,
> +			     txbuf_size, &vrp->rbuf_size);
> +		virtio_cread(vdev, struct virtio_rpmsg_config,
> +			     rxbuf_size, &vrp->sbuf_size);
> +
> +		/* The buffers must hold rpmsg header atleast */

typo:  /* The buffers must hold at least the rpmsg header */
> +		if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
> +		    vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
> +			dev_err(&vdev->dev,
> +				"vdev config: rx buf sz = %d, tx buf sz = %d\n",

message could be more explicit: s/"vdev config:/"bad vdev config:/

> +				vrp->rbuf_size, vrp->sbuf_size);
> +			err = -EINVAL;
> +			goto vqs_del;
> +		}
> +
> +		dev_dbg(&vdev->dev,
> +			"vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
> +			vrp->rbuf_size, vrp->sbuf_size);
> +	} else {
> +		vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> +		vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> +	}
>   
> -	total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
> +	total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> +			  (vrp->num_sbufs * vrp->sbuf_size);
> +	total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);

Needed? The allocator does not manage the page alignment?

>   
>   	/* allocate coherent memory for the buffers */
>   	bufs_va = dma_alloc_coherent(vdev->dev.parent,
> @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>   	vrp->rbufs = bufs_va;
>   
>   	/* and second part is dedicated for TX */
> -	vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
> +	vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
>   
>   	/* set up the receive buffers */
>   	for (i = 0; i < vrp->num_rbufs; i++) {
>   		struct scatterlist sg;
> -		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
> +		void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
>   
> -		rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
> +		rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
>   
>   		err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>   					  GFP_KERNEL);
> @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device *dev, void *data)
>   static void rpmsg_remove(struct virtio_device *vdev)
>   {
>   	struct virtproc_info *vrp = vdev->priv;
> -	unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
> -	size_t total_buf_space = num_bufs * vrp->buf_size;
> +	size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> +				 (vrp->num_sbufs * vrp->sbuf_size);
>   	int ret;
>   
>   	virtio_reset_device(vdev);
> @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
>   
>   	vdev->config->del_vqs(vrp->vdev);
>   
> +	total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>   	dma_free_coherent(vdev->dev.parent, total_buf_space,
>   			  vrp->rbufs, vrp->bufs_dma);
>   
> @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
>   
>   static unsigned int features[] = {
>   	VIRTIO_RPMSG_F_NS,
> +	VIRTIO_RPMSG_F_BUFSZ,
>   };
>   
>   static struct virtio_driver virtio_ipc_driver = {
> diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
> new file mode 100644
> index 000000000000..6406bc505383
> --- /dev/null
> +++ b/include/linux/virtio_rpmsg.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */

This licensing seems reserved for the UAPIs

> +/*
> + * Copyright (C) Pinecone Inc. 2019
> + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
> + */
> +
> +#ifndef _LINUX_VIRTIO_RPMSG_H
> +#define _LINUX_VIRTIO_RPMSG_H
> +
> +#include <linux/types.h>
> +
> +/* The feature bitmap for virtio rpmsg */
> +#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
> +#define VIRTIO_RPMSG_F_BUFSZ	2 /* RP get buffer size from config space */
> +
> +struct virtio_rpmsg_config {
> +	/* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
> +	__u32 txbuf_size;
> +	__u32 rxbuf_size;
> +	__u32 reserved[14]; /* Reserve for the future use */
> +	/* Put the customize config here */
> +} __attribute__((packed));

I am still convinced that adding a version field would help with future
improvements, even though a feature bit field can be used to handle new
configurations. Indeed, how can we address the need for more space
beyond the 14 reserved words?
Thanks,
Arnaud

> +
> +#endif /* _LINUX_VIRTIO_RPMSG_H */

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Tanmay Shah]

On 11/21/25 3:44 AM, Arnaud POULIQUEN wrote:
> 
> 
> On 11/14/25 19:46, Tanmay Shah wrote:
>> From: Xiang Xiao <xiaoxiang781216@gmail.com>
>>
>> 512 bytes isn't always suitable for all case, let firmware
>> maker decide the best value from resource table.
>> enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
>>
>> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>> ---
>>   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
>>   include/linux/virtio_rpmsg.h     | 24 +++++++++++
>>   2 files changed, 74 insertions(+), 18 deletions(-)
>>   create mode 100644 include/linux/virtio_rpmsg.h
>>
>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/ 
>> virtio_rpmsg_bus.c
>> index cc26dfcc3e29..03dd5535880a 100644
>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/sched.h>
>>   #include <linux/virtio.h>
>>   #include <linux/virtio_ids.h>
>> +#include <linux/virtio_rpmsg.h>
>>   #include <linux/virtio_config.h>
>>   #include <linux/wait.h>
>> @@ -39,7 +40,8 @@
>>    * @sbufs:    kernel address of tx buffers
>>    * @num_rbufs:    total number of buffers for rx
>>    * @num_sbufs:    total number of buffers for tx
>> - * @buf_size:    size of one rx or tx buffer
>> + * @rbuf_size:    size of one rx buffer
>> + * @sbuf_size:    size of one 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.
>> @@ -60,7 +62,8 @@ struct virtproc_info {
>>       void *rbufs, *sbufs;
>>       unsigned int num_rbufs;
>>       unsigned int num_sbufs;
>> -    unsigned int buf_size;
>> +    unsigned int rbuf_size;
>> +    unsigned int sbuf_size;
>>       int last_sbuf;
>>       dma_addr_t bufs_dma;
>>       struct mutex tx_lock;
>> @@ -70,9 +73,6 @@ struct virtproc_info {
>>       atomic_t sleepers;
>>   };
>> -/* The feature bitmap for virtio rpmsg */
>> -#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service 
>> notifications */
>> -
>>   /**
>>    * struct rpmsg_hdr - common header for all rpmsg messages
>>    * @src: source address
>> @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
>>    * processor.
>>    */
>>   #define MAX_RPMSG_NUM_BUFS    (256)
>> -#define MAX_RPMSG_BUF_SIZE    (512)
>> +#define DEFAULT_RPMSG_BUF_SIZE    (512)
>>   /*
>>    * Local addresses are dynamically allocated on-demand.
>> @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>>       /* either pick the next unused tx buffer */
>>       if (vrp->last_sbuf < vrp->num_sbufs)
>> -        ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
>> +        ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
>>       /* or recycle a used one */
>>       else
>>           ret = virtqueue_get_buf(vrp->svq, &len);
>> @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
>> +    if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
>>           dev_err(dev, "message is too big (%d)\n", len);
>>           return -EMSGSIZE;
>>       }
>> @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct 
>> rpmsg_endpoint *ept)
>>       struct rpmsg_device *rpdev = ept->rpdev;
>>       struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>> -    return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>> +    return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
>>   }
>>   static int rpmsg_recv_single(struct virtproc_info *vrp, struct 
>> device *dev,
>> @@ -706,7 +706,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 > vrp->buf_size ||
>> +    if (len > vrp->rbuf_size ||
>>           msg_len > (len - sizeof(struct rpmsg_hdr))) {
>>           dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
>>           return -EINVAL;
>> @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct virtproc_info 
>> *vrp, struct device *dev,
>>           dev_warn_ratelimited(dev, "msg received with no recipient\n");
>>       /* publish the real size of the buffer */
>> -    rpmsg_sg_init(&sg, msg, vrp->buf_size);
>> +    rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
>>       /* add the buffer back to the remote processor's virtqueue */
>>       err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
>> @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>       else
>>           vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>> -    vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>> +    /*
>> +     * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
>> +     * size from virtio device config space from the resource table.
>> +     * If the feature is not supported, then assign default buf size.
>> +     */
>> +    if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
>> +        /* note: virtio_rpmsg_config is defined from remote view */
>> +        virtio_cread(vdev, struct virtio_rpmsg_config,
>> +                 txbuf_size, &vrp->rbuf_size);
>> +        virtio_cread(vdev, struct virtio_rpmsg_config,
>> +                 rxbuf_size, &vrp->sbuf_size);
>> +
>> +        /* The buffers must hold rpmsg header atleast */
> 
> typo:  /* The buffers must hold at least the rpmsg header */

Ack.

>> +        if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
>> +            vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
>> +            dev_err(&vdev->dev,
>> +                "vdev config: rx buf sz = %d, tx buf sz = %d\n",
> 
> message could be more explicit: s/"vdev config:/"bad vdev config:/
> 

Ack.

>> +                vrp->rbuf_size, vrp->sbuf_size);
>> +            err = -EINVAL;
>> +            goto vqs_del;
>> +        }
>> +
>> +        dev_dbg(&vdev->dev,
>> +            "vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
>> +            vrp->rbuf_size, vrp->sbuf_size);
>> +    } else {
>> +        vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
>> +        vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
>> +    }
>> -    total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
>> +    total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
>> +              (vrp->num_sbufs * vrp->sbuf_size);
>> +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
> 
> Needed? The allocator does not manage the page alignment?
> 

Ack.

Not needed. Now as you pointed, I think it's better to leave the 
alignment to the allocator. This will be fixed.

>>       /* allocate coherent memory for the buffers */
>>       bufs_va = dma_alloc_coherent(vdev->dev.parent,
>> @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>       vrp->rbufs = bufs_va;
>>       /* and second part is dedicated for TX */
>> -    vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
>> +    vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
>>       /* set up the receive buffers */
>>       for (i = 0; i < vrp->num_rbufs; i++) {
>>           struct scatterlist sg;
>> -        void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>> +        void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
>> -        rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
>> +        rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
>>           err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>>                         GFP_KERNEL);
>> @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device 
>> *dev, void *data)
>>   static void rpmsg_remove(struct virtio_device *vdev)
>>   {
>>       struct virtproc_info *vrp = vdev->priv;
>> -    unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
>> -    size_t total_buf_space = num_bufs * vrp->buf_size;
>> +    size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
>> +                 (vrp->num_sbufs * vrp->sbuf_size);
>>       int ret;
>>       virtio_reset_device(vdev);
>> @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device 
>> *vdev)
>>       vdev->config->del_vqs(vrp->vdev);
>> +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>>       dma_free_coherent(vdev->dev.parent, total_buf_space,
>>                 vrp->rbufs, vrp->bufs_dma);
>> @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
>>   static unsigned int features[] = {
>>       VIRTIO_RPMSG_F_NS,
>> +    VIRTIO_RPMSG_F_BUFSZ,
>>   };
>>   static struct virtio_driver virtio_ipc_driver = {
>> diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
>> new file mode 100644
>> index 000000000000..6406bc505383
>> --- /dev/null
>> +++ b/include/linux/virtio_rpmsg.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> 
> This licensing seems reserved for the UAPIs
> 

Ack. I will check other headers in the same directory and update license 
accordingly.

>> +/*
>> + * Copyright (C) Pinecone Inc. 2019
>> + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
>> + */
>> +
>> +#ifndef _LINUX_VIRTIO_RPMSG_H
>> +#define _LINUX_VIRTIO_RPMSG_H
>> +
>> +#include <linux/types.h>
>> +
>> +/* The feature bitmap for virtio rpmsg */
>> +#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service 
>> notifications */
>> +#define VIRTIO_RPMSG_F_BUFSZ    2 /* RP get buffer size from config 
>> space */
>> +
>> +struct virtio_rpmsg_config {
>> +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
>> +    __u32 txbuf_size;
>> +    __u32 rxbuf_size;
>> +    __u32 reserved[14]; /* Reserve for the future use */
>> +    /* Put the customize config here */
>> +} __attribute__((packed));
> 
> I am still convinced that adding a version field would help with future
> improvements, even though a feature bit field can be used to handle new
> configurations. Indeed, how can we address the need for more space
> beyond the 14 reserved words?
> Thanks,
> Arnaud
> 

Ack.
Okay, it's not a deal breaker for me. I will add the version field linux 
side. I will add new structure in open-amp side as well accordingly.

>> +
>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
> 

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Zhongqiu Han]

On 11/15/2025 2:46 AM, Tanmay Shah wrote:
> From: Xiang Xiao <xiaoxiang781216@gmail.com>
> 
> 512 bytes isn't always suitable for all case, let firmware
> maker decide the best value from resource table.
> enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
> 
> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
>   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
>   include/linux/virtio_rpmsg.h     | 24 +++++++++++
>   2 files changed, 74 insertions(+), 18 deletions(-)
>   create mode 100644 include/linux/virtio_rpmsg.h
> 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index cc26dfcc3e29..03dd5535880a 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -25,6 +25,7 @@
>   #include <linux/sched.h>
>   #include <linux/virtio.h>
>   #include <linux/virtio_ids.h>
> +#include <linux/virtio_rpmsg.h>
>   #include <linux/virtio_config.h>
>   #include <linux/wait.h>
>   
> @@ -39,7 +40,8 @@
>    * @sbufs:	kernel address of tx buffers
>    * @num_rbufs:	total number of buffers for rx
>    * @num_sbufs:	total number of buffers for tx
> - * @buf_size:	size of one rx or tx buffer
> + * @rbuf_size:	size of one rx buffer
> + * @sbuf_size:	size of one 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.
> @@ -60,7 +62,8 @@ struct virtproc_info {
>   	void *rbufs, *sbufs;
>   	unsigned int num_rbufs;
>   	unsigned int num_sbufs;
> -	unsigned int buf_size;
> +	unsigned int rbuf_size;
> +	unsigned int sbuf_size;
>   	int last_sbuf;
>   	dma_addr_t bufs_dma;
>   	struct mutex tx_lock;
> @@ -70,9 +73,6 @@ struct virtproc_info {
>   	atomic_t sleepers;
>   };
>   
> -/* The feature bitmap for virtio rpmsg */
> -#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
> -
>   /**
>    * struct rpmsg_hdr - common header for all rpmsg messages
>    * @src: source address
> @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
>    * processor.
>    */
>   #define MAX_RPMSG_NUM_BUFS	(256)
> -#define MAX_RPMSG_BUF_SIZE	(512)
> +#define DEFAULT_RPMSG_BUF_SIZE	(512)
>   
>   /*
>    * Local addresses are dynamically allocated on-demand.
> @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>   
>   	/* either pick the next unused tx buffer */
>   	if (vrp->last_sbuf < vrp->num_sbufs)
> -		ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
> +		ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
>   	/* or recycle a used one */
>   	else
>   		ret = virtqueue_get_buf(vrp->svq, &len);
> @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
> +	if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
>   		dev_err(dev, "message is too big (%d)\n", len);
>   		return -EMSGSIZE;
>   	}
> @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>   	struct rpmsg_device *rpdev = ept->rpdev;
>   	struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>   
> -	return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
> +	return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
>   }
>   
>   static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
> @@ -706,7 +706,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 > vrp->buf_size ||
> +	if (len > vrp->rbuf_size ||
>   	    msg_len > (len - sizeof(struct rpmsg_hdr))) {
>   		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
>   		return -EINVAL;
> @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>   		dev_warn_ratelimited(dev, "msg received with no recipient\n");
>   
>   	/* publish the real size of the buffer */
> -	rpmsg_sg_init(&sg, msg, vrp->buf_size);
> +	rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
>   
>   	/* add the buffer back to the remote processor's virtqueue */
>   	err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
> @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
>   	else
>   		vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>   
> -	vrp->buf_size = MAX_RPMSG_BUF_SIZE;
> +	/*
> +	 * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
> +	 * size from virtio device config space from the resource table.
> +	 * If the feature is not supported, then assign default buf size.
> +	 */
> +	if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
> +		/* note: virtio_rpmsg_config is defined from remote view */
> +		virtio_cread(vdev, struct virtio_rpmsg_config,
> +			     txbuf_size, &vrp->rbuf_size);
> +		virtio_cread(vdev, struct virtio_rpmsg_config,
> +			     rxbuf_size, &vrp->sbuf_size);
> +
> +		/* The buffers must hold rpmsg header atleast */
> +		if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
> +		    vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {


Hello Tanmay,

May I know if the omission of = here is to accommodate the ping/pong/ack
scenarios? mtu will 0


> +			dev_err(&vdev->dev,
> +				"vdev config: rx buf sz = %d, tx buf sz = %d\n",
> +				vrp->rbuf_size, vrp->sbuf_size);
> +			err = -EINVAL;
> +			goto vqs_del;
> +		}
> +
> +		dev_dbg(&vdev->dev,
> +			"vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
> +			vrp->rbuf_size, vrp->sbuf_size);
> +	} else {
> +		vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> +		vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> +	}
>   
> -	total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
> +	total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> +			  (vrp->num_sbufs * vrp->sbuf_size);
> +	total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>   
>   	/* allocate coherent memory for the buffers */
>   	bufs_va = dma_alloc_coherent(vdev->dev.parent,
> @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>   	vrp->rbufs = bufs_va;
>   
>   	/* and second part is dedicated for TX */
> -	vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
> +	vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
>   
>   	/* set up the receive buffers */
>   	for (i = 0; i < vrp->num_rbufs; i++) {
>   		struct scatterlist sg;
> -		void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
> +		void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
>   
> -		rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
> +		rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
>   
>   		err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>   					  GFP_KERNEL);
> @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device *dev, void *data)
>   static void rpmsg_remove(struct virtio_device *vdev)
>   {
>   	struct virtproc_info *vrp = vdev->priv;
> -	unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
> -	size_t total_buf_space = num_bufs * vrp->buf_size;
> +	size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> +				 (vrp->num_sbufs * vrp->sbuf_size);
>   	int ret;
>   
>   	virtio_reset_device(vdev);
> @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
>   
>   	vdev->config->del_vqs(vrp->vdev);
>   
> +	total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>   	dma_free_coherent(vdev->dev.parent, total_buf_space,
>   			  vrp->rbufs, vrp->bufs_dma);
>   
> @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
>   
>   static unsigned int features[] = {
>   	VIRTIO_RPMSG_F_NS,
> +	VIRTIO_RPMSG_F_BUFSZ,
>   };
>   
>   static struct virtio_driver virtio_ipc_driver = {
> diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
> new file mode 100644
> index 000000000000..6406bc505383
> --- /dev/null
> +++ b/include/linux/virtio_rpmsg.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */


Echo Arnaud's comments. If it is intended for UAPI, please keep it in
include/uapi/linux


> +/*
> + * Copyright (C) Pinecone Inc. 2019
> + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
> + */
> +
> +#ifndef _LINUX_VIRTIO_RPMSG_H
> +#define _LINUX_VIRTIO_RPMSG_H
> +
> +#include <linux/types.h>
> +
> +/* The feature bitmap for virtio rpmsg */
> +#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
> +#define VIRTIO_RPMSG_F_BUFSZ	2 /* RP get buffer size from config space */

May I know why skip bit 1?


> +
> +struct virtio_rpmsg_config {
> +	/* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
> +	__u32 txbuf_size;
> +	__u32 rxbuf_size;
> +	__u32 reserved[14]; /* Reserve for the future use */

Should we use __virtio32 instead of __u32 to avoid endianness issues?


> +	/* Put the customize config here */
> +} __attribute__((packed));
> +
> +#endif /* _LINUX_VIRTIO_RPMSG_H */


-- 
Thx and BRs,
Zhongqiu Han

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Tanmay Shah]

Hello,

Thanks for your reviews. Please find the response below.

On 11/22/25 6:05 AM, Zhongqiu Han wrote:
> On 11/15/2025 2:46 AM, Tanmay Shah wrote:
>> From: Xiang Xiao <xiaoxiang781216@gmail.com>
>>
>> 512 bytes isn't always suitable for all case, let firmware
>> maker decide the best value from resource table.
>> enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
>>
>> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>> ---
>>   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
>>   include/linux/virtio_rpmsg.h     | 24 +++++++++++
>>   2 files changed, 74 insertions(+), 18 deletions(-)
>>   create mode 100644 include/linux/virtio_rpmsg.h
>>
>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/ 
>> virtio_rpmsg_bus.c
>> index cc26dfcc3e29..03dd5535880a 100644
>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/sched.h>
>>   #include <linux/virtio.h>
>>   #include <linux/virtio_ids.h>
>> +#include <linux/virtio_rpmsg.h>
>>   #include <linux/virtio_config.h>
>>   #include <linux/wait.h>
>> @@ -39,7 +40,8 @@
>>    * @sbufs:    kernel address of tx buffers
>>    * @num_rbufs:    total number of buffers for rx
>>    * @num_sbufs:    total number of buffers for tx
>> - * @buf_size:    size of one rx or tx buffer
>> + * @rbuf_size:    size of one rx buffer
>> + * @sbuf_size:    size of one 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.
>> @@ -60,7 +62,8 @@ struct virtproc_info {
>>       void *rbufs, *sbufs;
>>       unsigned int num_rbufs;
>>       unsigned int num_sbufs;
>> -    unsigned int buf_size;
>> +    unsigned int rbuf_size;
>> +    unsigned int sbuf_size;
>>       int last_sbuf;
>>       dma_addr_t bufs_dma;
>>       struct mutex tx_lock;
>> @@ -70,9 +73,6 @@ struct virtproc_info {
>>       atomic_t sleepers;
>>   };
>> -/* The feature bitmap for virtio rpmsg */
>> -#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service 
>> notifications */
>> -
>>   /**
>>    * struct rpmsg_hdr - common header for all rpmsg messages
>>    * @src: source address
>> @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
>>    * processor.
>>    */
>>   #define MAX_RPMSG_NUM_BUFS    (256)
>> -#define MAX_RPMSG_BUF_SIZE    (512)
>> +#define DEFAULT_RPMSG_BUF_SIZE    (512)
>>   /*
>>    * Local addresses are dynamically allocated on-demand.
>> @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>>       /* either pick the next unused tx buffer */
>>       if (vrp->last_sbuf < vrp->num_sbufs)
>> -        ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
>> +        ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
>>       /* or recycle a used one */
>>       else
>>           ret = virtqueue_get_buf(vrp->svq, &len);
>> @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
>> +    if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
>>           dev_err(dev, "message is too big (%d)\n", len);
>>           return -EMSGSIZE;
>>       }
>> @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct 
>> rpmsg_endpoint *ept)
>>       struct rpmsg_device *rpdev = ept->rpdev;
>>       struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>> -    return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>> +    return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
>>   }
>>   static int rpmsg_recv_single(struct virtproc_info *vrp, struct 
>> device *dev,
>> @@ -706,7 +706,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 > vrp->buf_size ||
>> +    if (len > vrp->rbuf_size ||
>>           msg_len > (len - sizeof(struct rpmsg_hdr))) {
>>           dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
>>           return -EINVAL;
>> @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct virtproc_info 
>> *vrp, struct device *dev,
>>           dev_warn_ratelimited(dev, "msg received with no recipient\n");
>>       /* publish the real size of the buffer */
>> -    rpmsg_sg_init(&sg, msg, vrp->buf_size);
>> +    rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
>>       /* add the buffer back to the remote processor's virtqueue */
>>       err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
>> @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>       else
>>           vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>> -    vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>> +    /*
>> +     * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
>> +     * size from virtio device config space from the resource table.
>> +     * If the feature is not supported, then assign default buf size.
>> +     */
>> +    if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
>> +        /* note: virtio_rpmsg_config is defined from remote view */
>> +        virtio_cread(vdev, struct virtio_rpmsg_config,
>> +                 txbuf_size, &vrp->rbuf_size);
>> +        virtio_cread(vdev, struct virtio_rpmsg_config,
>> +                 rxbuf_size, &vrp->sbuf_size);
>> +
>> +        /* The buffers must hold rpmsg header atleast */
>> +        if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
>> +            vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
> 
> 
> Hello Tanmay,
> 
> May I know if the omission of = here is to accommodate the ping/pong/ack
> scenarios? mtu will 0
> 
> 

Yes. At minimum RPMsg header is needed to ping the correct endpoint. We 
don't need to have any payload attached to the packet. MTU will be 
sizeof rpmsg_hdr I think.

>> +            dev_err(&vdev->dev,
>> +                "vdev config: rx buf sz = %d, tx buf sz = %d\n",
>> +                vrp->rbuf_size, vrp->sbuf_size);
>> +            err = -EINVAL;
>> +            goto vqs_del;
>> +        }
>> +
>> +        dev_dbg(&vdev->dev,
>> +            "vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
>> +            vrp->rbuf_size, vrp->sbuf_size);
>> +    } else {
>> +        vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
>> +        vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
>> +    }
>> -    total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
>> +    total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
>> +              (vrp->num_sbufs * vrp->sbuf_size);
>> +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>>       /* allocate coherent memory for the buffers */
>>       bufs_va = dma_alloc_coherent(vdev->dev.parent,
>> @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>       vrp->rbufs = bufs_va;
>>       /* and second part is dedicated for TX */
>> -    vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
>> +    vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
>>       /* set up the receive buffers */
>>       for (i = 0; i < vrp->num_rbufs; i++) {
>>           struct scatterlist sg;
>> -        void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>> +        void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
>> -        rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
>> +        rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
>>           err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>>                         GFP_KERNEL);
>> @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device 
>> *dev, void *data)
>>   static void rpmsg_remove(struct virtio_device *vdev)
>>   {
>>       struct virtproc_info *vrp = vdev->priv;
>> -    unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
>> -    size_t total_buf_space = num_bufs * vrp->buf_size;
>> +    size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
>> +                 (vrp->num_sbufs * vrp->sbuf_size);
>>       int ret;
>>       virtio_reset_device(vdev);
>> @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device 
>> *vdev)
>>       vdev->config->del_vqs(vrp->vdev);
>> +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>>       dma_free_coherent(vdev->dev.parent, total_buf_space,
>>                 vrp->rbufs, vrp->bufs_dma);
>> @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
>>   static unsigned int features[] = {
>>       VIRTIO_RPMSG_F_NS,
>> +    VIRTIO_RPMSG_F_BUFSZ,
>>   };
>>   static struct virtio_driver virtio_ipc_driver = {
>> diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
>> new file mode 100644
>> index 000000000000..6406bc505383
>> --- /dev/null
>> +++ b/include/linux/virtio_rpmsg.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> 
> 
> Echo Arnaud's comments. If it is intended for UAPI, please keep it in
> include/uapi/linux
> 
> 

It's not intended for UAPI. I need to fix the license. I will check 
other virtio headers in the same directory and fix the license accordingly.

>> +/*
>> + * Copyright (C) Pinecone Inc. 2019
>> + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
>> + */
>> +
>> +#ifndef _LINUX_VIRTIO_RPMSG_H
>> +#define _LINUX_VIRTIO_RPMSG_H
>> +
>> +#include <linux/types.h>
>> +
>> +/* The feature bitmap for virtio rpmsg */
>> +#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service 
>> notifications */
>> +#define VIRTIO_RPMSG_F_BUFSZ    2 /* RP get buffer size from config 
>> space */
> 
> May I know why skip bit 1?
> 
> 

Thanks, that's a good question. I keept id 2 unmodified from the 
original series. I don't know why ID 2 was chosen in the original 
series. I will have to discuss this with the linux remoteproc/rpmsg 
maintainers and choose the correct ID.

I don't see any problem choosing ID 1, but for some reason if ID 1 was 
assigned and deprecated (I don't think that is the case) then only we 
should use ID 2.


Arnaud, Mathieu, Bjorn any input here?

>> +
>> +struct virtio_rpmsg_config {
>> +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
>> +    __u32 txbuf_size;
>> +    __u32 rxbuf_size;
>> +    __u32 reserved[14]; /* Reserve for the future use */
> 
> Should we use __virtio32 instead of __u32 to avoid endianness issues?
> 
> 

Sure, if that is the standard in other virtio headers I will modify it.

Thanks,
Tanmay

>> +    /* Put the customize config here */
>> +} __attribute__((packed));
>> +
>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
> 
> 

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Michael S. Tsirkin]

On Wed, Dec 03, 2025 at 12:12:46PM -0600, Tanmay Shah wrote:
> Hello,
> 
> Thanks for your reviews. Please find the response below.
> 
> On 11/22/25 6:05 AM, Zhongqiu Han wrote:
> > On 11/15/2025 2:46 AM, Tanmay Shah wrote:
> > > From: Xiang Xiao <xiaoxiang781216@gmail.com>
> > > 
> > > 512 bytes isn't always suitable for all case, let firmware
> > > maker decide the best value from resource table.
> > > enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
> > > 
> > > Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
> > > Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> > > ---
> > >   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
> > >   include/linux/virtio_rpmsg.h     | 24 +++++++++++
> > >   2 files changed, 74 insertions(+), 18 deletions(-)
> > >   create mode 100644 include/linux/virtio_rpmsg.h
> > > 
> > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/
> > > virtio_rpmsg_bus.c
> > > index cc26dfcc3e29..03dd5535880a 100644
> > > --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> > > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> > > @@ -25,6 +25,7 @@
> > >   #include <linux/sched.h>
> > >   #include <linux/virtio.h>
> > >   #include <linux/virtio_ids.h>
> > > +#include <linux/virtio_rpmsg.h>
> > >   #include <linux/virtio_config.h>
> > >   #include <linux/wait.h>
> > > @@ -39,7 +40,8 @@
> > >    * @sbufs:    kernel address of tx buffers
> > >    * @num_rbufs:    total number of buffers for rx
> > >    * @num_sbufs:    total number of buffers for tx
> > > - * @buf_size:    size of one rx or tx buffer
> > > + * @rbuf_size:    size of one rx buffer
> > > + * @sbuf_size:    size of one 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.
> > > @@ -60,7 +62,8 @@ struct virtproc_info {
> > >       void *rbufs, *sbufs;
> > >       unsigned int num_rbufs;
> > >       unsigned int num_sbufs;
> > > -    unsigned int buf_size;
> > > +    unsigned int rbuf_size;
> > > +    unsigned int sbuf_size;
> > >       int last_sbuf;
> > >       dma_addr_t bufs_dma;
> > >       struct mutex tx_lock;
> > > @@ -70,9 +73,6 @@ struct virtproc_info {
> > >       atomic_t sleepers;
> > >   };
> > > -/* The feature bitmap for virtio rpmsg */
> > > -#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service
> > > notifications */
> > > -
> > >   /**
> > >    * struct rpmsg_hdr - common header for all rpmsg messages
> > >    * @src: source address
> > > @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
> > >    * processor.
> > >    */
> > >   #define MAX_RPMSG_NUM_BUFS    (256)
> > > -#define MAX_RPMSG_BUF_SIZE    (512)
> > > +#define DEFAULT_RPMSG_BUF_SIZE    (512)
> > >   /*
> > >    * Local addresses are dynamically allocated on-demand.
> > > @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
> > >       /* either pick the next unused tx buffer */
> > >       if (vrp->last_sbuf < vrp->num_sbufs)
> > > -        ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
> > > +        ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
> > >       /* or recycle a used one */
> > >       else
> > >           ret = virtqueue_get_buf(vrp->svq, &len);
> > > @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
> > > +    if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
> > >           dev_err(dev, "message is too big (%d)\n", len);
> > >           return -EMSGSIZE;
> > >       }
> > > @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct
> > > rpmsg_endpoint *ept)
> > >       struct rpmsg_device *rpdev = ept->rpdev;
> > >       struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
> > > -    return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
> > > +    return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
> > >   }
> > >   static int rpmsg_recv_single(struct virtproc_info *vrp, struct
> > > device *dev,
> > > @@ -706,7 +706,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 > vrp->buf_size ||
> > > +    if (len > vrp->rbuf_size ||
> > >           msg_len > (len - sizeof(struct rpmsg_hdr))) {
> > >           dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
> > >           return -EINVAL;
> > > @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct
> > > virtproc_info *vrp, struct device *dev,
> > >           dev_warn_ratelimited(dev, "msg received with no recipient\n");
> > >       /* publish the real size of the buffer */
> > > -    rpmsg_sg_init(&sg, msg, vrp->buf_size);
> > > +    rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
> > >       /* add the buffer back to the remote processor's virtqueue */
> > >       err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
> > > @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
> > >       else
> > >           vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
> > > -    vrp->buf_size = MAX_RPMSG_BUF_SIZE;
> > > +    /*
> > > +     * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
> > > +     * size from virtio device config space from the resource table.
> > > +     * If the feature is not supported, then assign default buf size.
> > > +     */
> > > +    if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
> > > +        /* note: virtio_rpmsg_config is defined from remote view */
> > > +        virtio_cread(vdev, struct virtio_rpmsg_config,
> > > +                 txbuf_size, &vrp->rbuf_size);
> > > +        virtio_cread(vdev, struct virtio_rpmsg_config,
> > > +                 rxbuf_size, &vrp->sbuf_size);
> > > +
> > > +        /* The buffers must hold rpmsg header atleast */
> > > +        if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
> > > +            vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
> > 
> > 
> > Hello Tanmay,
> > 
> > May I know if the omission of = here is to accommodate the ping/pong/ack
> > scenarios? mtu will 0
> > 
> > 
> 
> Yes. At minimum RPMsg header is needed to ping the correct endpoint. We
> don't need to have any payload attached to the packet. MTU will be sizeof
> rpmsg_hdr I think.
> 
> > > +            dev_err(&vdev->dev,
> > > +                "vdev config: rx buf sz = %d, tx buf sz = %d\n",
> > > +                vrp->rbuf_size, vrp->sbuf_size);
> > > +            err = -EINVAL;
> > > +            goto vqs_del;
> > > +        }
> > > +
> > > +        dev_dbg(&vdev->dev,
> > > +            "vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
> > > +            vrp->rbuf_size, vrp->sbuf_size);
> > > +    } else {
> > > +        vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> > > +        vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> > > +    }
> > > -    total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp->buf_size;
> > > +    total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> > > +              (vrp->num_sbufs * vrp->sbuf_size);
> > > +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
> > >       /* allocate coherent memory for the buffers */
> > >       bufs_va = dma_alloc_coherent(vdev->dev.parent,
> > > @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
> > >       vrp->rbufs = bufs_va;
> > >       /* and second part is dedicated for TX */
> > > -    vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
> > > +    vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
> > >       /* set up the receive buffers */
> > >       for (i = 0; i < vrp->num_rbufs; i++) {
> > >           struct scatterlist sg;
> > > -        void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
> > > +        void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
> > > -        rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
> > > +        rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
> > >           err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
> > >                         GFP_KERNEL);
> > > @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device
> > > *dev, void *data)
> > >   static void rpmsg_remove(struct virtio_device *vdev)
> > >   {
> > >       struct virtproc_info *vrp = vdev->priv;
> > > -    unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
> > > -    size_t total_buf_space = num_bufs * vrp->buf_size;
> > > +    size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> > > +                 (vrp->num_sbufs * vrp->sbuf_size);
> > >       int ret;
> > >       virtio_reset_device(vdev);
> > > @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device
> > > *vdev)
> > >       vdev->config->del_vqs(vrp->vdev);
> > > +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
> > >       dma_free_coherent(vdev->dev.parent, total_buf_space,
> > >                 vrp->rbufs, vrp->bufs_dma);
> > > @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
> > >   static unsigned int features[] = {
> > >       VIRTIO_RPMSG_F_NS,
> > > +    VIRTIO_RPMSG_F_BUFSZ,
> > >   };
> > >   static struct virtio_driver virtio_ipc_driver = {
> > > diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
> > > new file mode 100644
> > > index 000000000000..6406bc505383
> > > --- /dev/null
> > > +++ b/include/linux/virtio_rpmsg.h
> > > @@ -0,0 +1,24 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > 
> > 
> > Echo Arnaud's comments. If it is intended for UAPI, please keep it in
> > include/uapi/linux
> > 
> > 
> 
> It's not intended for UAPI. I need to fix the license. I will check other
> virtio headers in the same directory and fix the license accordingly.
> 
> > > +/*
> > > + * Copyright (C) Pinecone Inc. 2019
> > > + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
> > > + */
> > > +
> > > +#ifndef _LINUX_VIRTIO_RPMSG_H
> > > +#define _LINUX_VIRTIO_RPMSG_H
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +/* The feature bitmap for virtio rpmsg */
> > > +#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service
> > > notifications */
> > > +#define VIRTIO_RPMSG_F_BUFSZ    2 /* RP get buffer size from config
> > > space */
> > 
> > May I know why skip bit 1?
> > 
> > 
> 
> Thanks, that's a good question. I keept id 2 unmodified from the original
> series. I don't know why ID 2 was chosen in the original series. I will have
> to discuss this with the linux remoteproc/rpmsg maintainers and choose the
> correct ID.
> 
> I don't see any problem choosing ID 1, but for some reason if ID 1 was
> assigned and deprecated (I don't think that is the case) then only we should
> use ID 2.
> 
> 
> Arnaud, Mathieu, Bjorn any input here?
> 
> > > +
> > > +struct virtio_rpmsg_config {
> > > +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
> > > +    __u32 txbuf_size;
> > > +    __u32 rxbuf_size;
> > > +    __u32 reserved[14]; /* Reserve for the future use */
> > 
> > Should we use __virtio32 instead of __u32 to avoid endianness issues?
> > 
> > 
> 
> Sure, if that is the standard in other virtio headers I will modify it.
> 
> Thanks,
> Tanmay

rpmsg is still not standardized, sadly. It's really time it was.



Modern virtio devices use __le32.
Accordingly, accessed with virtio_cread_le


__virtioXX and virtio_cread are for legacy compatible parts of config space.


Does rpmsg want to be modern or keep using legacy? I donnu.

Ideally it should finally be documented and at that point we
definitely will want to switch to __le32.


For now, run sparse to make sure you don't introduce new endian-ness
issues.


> > > +    /* Put the customize config here */
> > > +} __attribute__((packed));
> > > +
> > > +#endif /* _LINUX_VIRTIO_RPMSG_H */
> > 
> > 

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Tanmay Shah]

On 12/3/25 1:35 PM, Michael S. Tsirkin wrote:
> On Wed, Dec 03, 2025 at 12:12:46PM -0600, Tanmay Shah wrote:
>> Hello,
>>
>> Thanks for your reviews. Please find the response below.
>>
>> On 11/22/25 6:05 AM, Zhongqiu Han wrote:
>>> On 11/15/2025 2:46 AM, Tanmay Shah wrote:
>>>> From: Xiang Xiao <xiaoxiang781216@gmail.com>
>>>>

[...]

>>
>> Thanks, that's a good question. I keept id 2 unmodified from the original
>> series. I don't know why ID 2 was chosen in the original series. I will have
>> to discuss this with the linux remoteproc/rpmsg maintainers and choose the
>> correct ID.
>>
>> I don't see any problem choosing ID 1, but for some reason if ID 1 was
>> assigned and deprecated (I don't think that is the case) then only we should
>> use ID 2.
>>
>>
>> Arnaud, Mathieu, Bjorn any input here?
>>
>>>> +
>>>> +struct virtio_rpmsg_config {
>>>> +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
>>>> +    __u32 txbuf_size;
>>>> +    __u32 rxbuf_size;
>>>> +    __u32 reserved[14]; /* Reserve for the future use */
>>>
>>> Should we use __virtio32 instead of __u32 to avoid endianness issues?
>>>
>>>
>>
>> Sure, if that is the standard in other virtio headers I will modify it.
>>
>> Thanks,
>> Tanmay
> 
> rpmsg is still not standardized, sadly. It's really time it was.
> 
> 
> 
> Modern virtio devices use __le32.
> Accordingly, accessed with virtio_cread_le
> 
> 
> __virtioXX and virtio_cread are for legacy compatible parts of config space.
> 

As of now, I am using virtio_cread so I think it is legacy compatible.

> 
> Does rpmsg want to be modern or keep using legacy? I donnu.
>

I don't know either. This we have to discuss with the maintainers and 
other vendors.


> Ideally it should finally be documented and at that point we
> definitely will want to switch to __le32.
> 
> 
> For now, run sparse to make sure you don't introduce new endian-ness
> issues.

Ack.

> 
> 



>>>> +    /* Put the customize config here */
>>>> +} __attribute__((packed));
>>>> +
>>>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
>>>
>>>
> 

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Mathieu Poirier]

On Thu, Dec 04, 2025 at 10:55:10AM -0600, Tanmay Shah wrote:
> 
> 
> On 12/3/25 1:35 PM, Michael S. Tsirkin wrote:
> > On Wed, Dec 03, 2025 at 12:12:46PM -0600, Tanmay Shah wrote:
> > > Hello,
> > > 
> > > Thanks for your reviews. Please find the response below.
> > > 
> > > On 11/22/25 6:05 AM, Zhongqiu Han wrote:
> > > > On 11/15/2025 2:46 AM, Tanmay Shah wrote:
> > > > > From: Xiang Xiao <xiaoxiang781216@gmail.com>
> > > > > 
> 
> [...]
> 
> > > 
> > > Thanks, that's a good question. I keept id 2 unmodified from the original
> > > series. I don't know why ID 2 was chosen in the original series. I will have
> > > to discuss this with the linux remoteproc/rpmsg maintainers and choose the
> > > correct ID.
> > > 
> > > I don't see any problem choosing ID 1, but for some reason if ID 1 was
> > > assigned and deprecated (I don't think that is the case) then only we should
> > > use ID 2.
> > > 
> > > 
> > > Arnaud, Mathieu, Bjorn any input here?
> > > 
> > > > > +
> > > > > +struct virtio_rpmsg_config {
> > > > > +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
> > > > > +    __u32 txbuf_size;
> > > > > +    __u32 rxbuf_size;
> > > > > +    __u32 reserved[14]; /* Reserve for the future use */
> > > > 
> > > > Should we use __virtio32 instead of __u32 to avoid endianness issues?
> > > > 
> > > > 
> > > 
> > > Sure, if that is the standard in other virtio headers I will modify it.
> > > 
> > > Thanks,
> > > Tanmay
> > 
> > rpmsg is still not standardized, sadly. It's really time it was.
> > 
> > 
> > 
> > Modern virtio devices use __le32.
> > Accordingly, accessed with virtio_cread_le
> > 
> > 
> > __virtioXX and virtio_cread are for legacy compatible parts of config space.
> > 
> 
> As of now, I am using virtio_cread so I think it is legacy compatible.
> 
> > 
> > Does rpmsg want to be modern or keep using legacy? I donnu.
> > 
> 
> I don't know either. This we have to discuss with the maintainers and other
> vendors.
>

I would certainly like to see a modernization effort, but it needs to be
backward compatible.  Probably outside the scope of this set though.
 
> 
> > Ideally it should finally be documented and at that point we
> > definitely will want to switch to __le32.
> > 
> > 
> > For now, run sparse to make sure you don't introduce new endian-ness
> > issues.
> 
> Ack.
> 
> > 
> > 
> 
> 
> 
> > > > > +    /* Put the customize config here */
> > > > > +} __attribute__((packed));
> > > > > +
> > > > > +#endif /* _LINUX_VIRTIO_RPMSG_H */
> > > > 
> > > > 
> > 
> 

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Arnaud POULIQUEN]

Hi,

On 12/3/25 19:12, Tanmay Shah wrote:
> Hello,
> 
> Thanks for your reviews. Please find the response below.
> 
> On 11/22/25 6:05 AM, Zhongqiu Han wrote:
>> On 11/15/2025 2:46 AM, Tanmay Shah wrote:
>>> From: Xiang Xiao <xiaoxiang781216@gmail.com>
>>>
>>> 512 bytes isn't always suitable for all case, let firmware
>>> maker decide the best value from resource table.
>>> enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
>>>
>>> Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
>>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>>> ---
>>>   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
>>>   include/linux/virtio_rpmsg.h     | 24 +++++++++++
>>>   2 files changed, 74 insertions(+), 18 deletions(-)
>>>   create mode 100644 include/linux/virtio_rpmsg.h
>>>
>>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/ 
>>> virtio_rpmsg_bus.c
>>> index cc26dfcc3e29..03dd5535880a 100644
>>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>>> @@ -25,6 +25,7 @@
>>>   #include <linux/sched.h>
>>>   #include <linux/virtio.h>
>>>   #include <linux/virtio_ids.h>
>>> +#include <linux/virtio_rpmsg.h>
>>>   #include <linux/virtio_config.h>
>>>   #include <linux/wait.h>
>>> @@ -39,7 +40,8 @@
>>>    * @sbufs:    kernel address of tx buffers
>>>    * @num_rbufs:    total number of buffers for rx
>>>    * @num_sbufs:    total number of buffers for tx
>>> - * @buf_size:    size of one rx or tx buffer
>>> + * @rbuf_size:    size of one rx buffer
>>> + * @sbuf_size:    size of one 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.
>>> @@ -60,7 +62,8 @@ struct virtproc_info {
>>>       void *rbufs, *sbufs;
>>>       unsigned int num_rbufs;
>>>       unsigned int num_sbufs;
>>> -    unsigned int buf_size;
>>> +    unsigned int rbuf_size;
>>> +    unsigned int sbuf_size;
>>>       int last_sbuf;
>>>       dma_addr_t bufs_dma;
>>>       struct mutex tx_lock;
>>> @@ -70,9 +73,6 @@ struct virtproc_info {
>>>       atomic_t sleepers;
>>>   };
>>> -/* The feature bitmap for virtio rpmsg */
>>> -#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service 
>>> notifications */
>>> -
>>>   /**
>>>    * struct rpmsg_hdr - common header for all rpmsg messages
>>>    * @src: source address
>>> @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
>>>    * processor.
>>>    */
>>>   #define MAX_RPMSG_NUM_BUFS    (256)
>>> -#define MAX_RPMSG_BUF_SIZE    (512)
>>> +#define DEFAULT_RPMSG_BUF_SIZE    (512)
>>>   /*
>>>    * Local addresses are dynamically allocated on-demand.
>>> @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>>>       /* either pick the next unused tx buffer */
>>>       if (vrp->last_sbuf < vrp->num_sbufs)
>>> -        ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
>>> +        ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
>>>       /* or recycle a used one */
>>>       else
>>>           ret = virtqueue_get_buf(vrp->svq, &len);
>>> @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
>>> +    if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
>>>           dev_err(dev, "message is too big (%d)\n", len);
>>>           return -EMSGSIZE;
>>>       }
>>> @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct 
>>> rpmsg_endpoint *ept)
>>>       struct rpmsg_device *rpdev = ept->rpdev;
>>>       struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>>> -    return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>>> +    return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
>>>   }
>>>   static int rpmsg_recv_single(struct virtproc_info *vrp, struct 
>>> device *dev,
>>> @@ -706,7 +706,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 > vrp->buf_size ||
>>> +    if (len > vrp->rbuf_size ||
>>>           msg_len > (len - sizeof(struct rpmsg_hdr))) {
>>>           dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, 
>>> msg_len);
>>>           return -EINVAL;
>>> @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct virtproc_info 
>>> *vrp, struct device *dev,
>>>           dev_warn_ratelimited(dev, "msg received with no recipient\n");
>>>       /* publish the real size of the buffer */
>>> -    rpmsg_sg_init(&sg, msg, vrp->buf_size);
>>> +    rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
>>>       /* add the buffer back to the remote processor's virtqueue */
>>>       err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
>>> @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>>       else
>>>           vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
>>> -    vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>>> +    /*
>>> +     * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
>>> +     * size from virtio device config space from the resource table.
>>> +     * If the feature is not supported, then assign default buf size.
>>> +     */
>>> +    if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
>>> +        /* note: virtio_rpmsg_config is defined from remote view */
>>> +        virtio_cread(vdev, struct virtio_rpmsg_config,
>>> +                 txbuf_size, &vrp->rbuf_size);
>>> +        virtio_cread(vdev, struct virtio_rpmsg_config,
>>> +                 rxbuf_size, &vrp->sbuf_size);
>>> +
>>> +        /* The buffers must hold rpmsg header atleast */
>>> +        if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
>>> +            vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
>>
>>
>> Hello Tanmay,
>>
>> May I know if the omission of = here is to accommodate the ping/pong/ack
>> scenarios? mtu will 0
>>
>>
> 
> Yes. At minimum RPMsg header is needed to ping the correct endpoint. We 
> don't need to have any payload attached to the packet. MTU will be 
> sizeof rpmsg_hdr I think.
> 
>>> +            dev_err(&vdev->dev,
>>> +                "vdev config: rx buf sz = %d, tx buf sz = %d\n",
>>> +                vrp->rbuf_size, vrp->sbuf_size);
>>> +            err = -EINVAL;
>>> +            goto vqs_del;
>>> +        }
>>> +
>>> +        dev_dbg(&vdev->dev,
>>> +            "vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
>>> +            vrp->rbuf_size, vrp->sbuf_size);
>>> +    } else {
>>> +        vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
>>> +        vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
>>> +    }
>>> -    total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp- 
>>> >buf_size;
>>> +    total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
>>> +              (vrp->num_sbufs * vrp->sbuf_size);
>>> +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>>>       /* allocate coherent memory for the buffers */
>>>       bufs_va = dma_alloc_coherent(vdev->dev.parent,
>>> @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
>>>       vrp->rbufs = bufs_va;
>>>       /* and second part is dedicated for TX */
>>> -    vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
>>> +    vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
>>>       /* set up the receive buffers */
>>>       for (i = 0; i < vrp->num_rbufs; i++) {
>>>           struct scatterlist sg;
>>> -        void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
>>> +        void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
>>> -        rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
>>> +        rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
>>>           err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>>>                         GFP_KERNEL);
>>> @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct device 
>>> *dev, void *data)
>>>   static void rpmsg_remove(struct virtio_device *vdev)
>>>   {
>>>       struct virtproc_info *vrp = vdev->priv;
>>> -    unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
>>> -    size_t total_buf_space = num_bufs * vrp->buf_size;
>>> +    size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
>>> +                 (vrp->num_sbufs * vrp->sbuf_size);
>>>       int ret;
>>>       virtio_reset_device(vdev);
>>> @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct virtio_device 
>>> *vdev)
>>>       vdev->config->del_vqs(vrp->vdev);
>>> +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
>>>       dma_free_coherent(vdev->dev.parent, total_buf_space,
>>>                 vrp->rbufs, vrp->bufs_dma);
>>> @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
>>>   static unsigned int features[] = {
>>>       VIRTIO_RPMSG_F_NS,
>>> +    VIRTIO_RPMSG_F_BUFSZ,
>>>   };
>>>   static struct virtio_driver virtio_ipc_driver = {
>>> diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
>>> new file mode 100644
>>> index 000000000000..6406bc505383
>>> --- /dev/null
>>> +++ b/include/linux/virtio_rpmsg.h
>>> @@ -0,0 +1,24 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>>
>>
>> Echo Arnaud's comments. If it is intended for UAPI, please keep it in
>> include/uapi/linux
>>
>>
> 
> It's not intended for UAPI. I need to fix the license. I will check 
> other virtio headers in the same directory and fix the license accordingly.
> 
>>> +/*
>>> + * Copyright (C) Pinecone Inc. 2019
>>> + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
>>> + */
>>> +
>>> +#ifndef _LINUX_VIRTIO_RPMSG_H
>>> +#define _LINUX_VIRTIO_RPMSG_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +/* The feature bitmap for virtio rpmsg */
>>> +#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service 
>>> notifications */
>>> +#define VIRTIO_RPMSG_F_BUFSZ    2 /* RP get buffer size from config 
>>> space */
>>
>> May I know why skip bit 1?
>>
>>
> 
> Thanks, that's a good question. I keept id 2 unmodified from the 
> original series. I don't know why ID 2 was chosen in the original 
> series. I will have to discuss this with the linux remoteproc/rpmsg 
> maintainers and choose the correct ID.
> 
> I don't see any problem choosing ID 1, but for some reason if ID 1 was 
> assigned and deprecated (I don't think that is the case) then only we 
> should use ID 2.


The ID 1 was proposed in an openamp PR [1]. If we
enter VIRTIO_RPMSG_F_BUFSZ first it makes sense to set its ID to 1.

[1]https://github.com/OpenAMP/open-amp/pull/160/commits/d4a13128f94e46180285c05a20da78fdca54f7d7


Regards,
Arnaud

> 
> 
> Arnaud, Mathieu, Bjorn any input here?
> 
>>> +
>>> +struct virtio_rpmsg_config {
>>> +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
>>> +    __u32 txbuf_size;
>>> +    __u32 rxbuf_size;
>>> +    __u32 reserved[14]; /* Reserve for the future use */
>>
>> Should we use __virtio32 instead of __u32 to avoid endianness issues?
>>
>>
> 
> Sure, if that is the standard in other virtio headers I will modify it.
> 
> Thanks,
> Tanmay
> 
>>> +    /* Put the customize config here */
>>> +} __attribute__((packed));
>>> +
>>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
>>
>>
> 

Re: [RFC PATCH 2/2] rpmsg: virtio_rpmsg_bus: get buffer size from config space

[Mathieu Poirier]

On Fri, Dec 05, 2025 at 05:01:17PM +0100, Arnaud POULIQUEN wrote:
> Hi,
> 
> On 12/3/25 19:12, Tanmay Shah wrote:
> > Hello,
> > 
> > Thanks for your reviews. Please find the response below.
> > 
> > On 11/22/25 6:05 AM, Zhongqiu Han wrote:
> > > On 11/15/2025 2:46 AM, Tanmay Shah wrote:
> > > > From: Xiang Xiao <xiaoxiang781216@gmail.com>
> > > > 
> > > > 512 bytes isn't always suitable for all case, let firmware
> > > > maker decide the best value from resource table.
> > > > enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
> > > > 
> > > > Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
> > > > Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> > > > ---
> > > >   drivers/rpmsg/virtio_rpmsg_bus.c | 68 +++++++++++++++++++++++---------
> > > >   include/linux/virtio_rpmsg.h     | 24 +++++++++++
> > > >   2 files changed, 74 insertions(+), 18 deletions(-)
> > > >   create mode 100644 include/linux/virtio_rpmsg.h
> > > > 
> > > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/
> > > > virtio_rpmsg_bus.c
> > > > index cc26dfcc3e29..03dd5535880a 100644
> > > > --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> > > > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> > > > @@ -25,6 +25,7 @@
> > > >   #include <linux/sched.h>
> > > >   #include <linux/virtio.h>
> > > >   #include <linux/virtio_ids.h>
> > > > +#include <linux/virtio_rpmsg.h>
> > > >   #include <linux/virtio_config.h>
> > > >   #include <linux/wait.h>
> > > > @@ -39,7 +40,8 @@
> > > >    * @sbufs:    kernel address of tx buffers
> > > >    * @num_rbufs:    total number of buffers for rx
> > > >    * @num_sbufs:    total number of buffers for tx
> > > > - * @buf_size:    size of one rx or tx buffer
> > > > + * @rbuf_size:    size of one rx buffer
> > > > + * @sbuf_size:    size of one 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.
> > > > @@ -60,7 +62,8 @@ struct virtproc_info {
> > > >       void *rbufs, *sbufs;
> > > >       unsigned int num_rbufs;
> > > >       unsigned int num_sbufs;
> > > > -    unsigned int buf_size;
> > > > +    unsigned int rbuf_size;
> > > > +    unsigned int sbuf_size;
> > > >       int last_sbuf;
> > > >       dma_addr_t bufs_dma;
> > > >       struct mutex tx_lock;
> > > > @@ -70,9 +73,6 @@ struct virtproc_info {
> > > >       atomic_t sleepers;
> > > >   };
> > > > -/* The feature bitmap for virtio rpmsg */
> > > > -#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service
> > > > notifications */
> > > > -
> > > >   /**
> > > >    * struct rpmsg_hdr - common header for all rpmsg messages
> > > >    * @src: source address
> > > > @@ -130,7 +130,7 @@ struct virtio_rpmsg_channel {
> > > >    * processor.
> > > >    */
> > > >   #define MAX_RPMSG_NUM_BUFS    (256)
> > > > -#define MAX_RPMSG_BUF_SIZE    (512)
> > > > +#define DEFAULT_RPMSG_BUF_SIZE    (512)
> > > >   /*
> > > >    * Local addresses are dynamically allocated on-demand.
> > > > @@ -443,7 +443,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
> > > >       /* either pick the next unused tx buffer */
> > > >       if (vrp->last_sbuf < vrp->num_sbufs)
> > > > -        ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++;
> > > > +        ret = vrp->sbufs + vrp->sbuf_size * vrp->last_sbuf++;
> > > >       /* or recycle a used one */
> > > >       else
> > > >           ret = virtqueue_get_buf(vrp->svq, &len);
> > > > @@ -569,7 +569,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 > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
> > > > +    if (len > vrp->sbuf_size - sizeof(struct rpmsg_hdr)) {
> > > >           dev_err(dev, "message is too big (%d)\n", len);
> > > >           return -EMSGSIZE;
> > > >       }
> > > > @@ -680,7 +680,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct
> > > > rpmsg_endpoint *ept)
> > > >       struct rpmsg_device *rpdev = ept->rpdev;
> > > >       struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
> > > > -    return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
> > > > +    return vch->vrp->sbuf_size - sizeof(struct rpmsg_hdr);
> > > >   }
> > > >   static int rpmsg_recv_single(struct virtproc_info *vrp, struct
> > > > device *dev,
> > > > @@ -706,7 +706,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 > vrp->buf_size ||
> > > > +    if (len > vrp->rbuf_size ||
> > > >           msg_len > (len - sizeof(struct rpmsg_hdr))) {
> > > >           dev_warn(dev, "inbound msg too big: (%d, %d)\n", len,
> > > > msg_len);
> > > >           return -EINVAL;
> > > > @@ -739,7 +739,7 @@ static int rpmsg_recv_single(struct
> > > > virtproc_info *vrp, struct device *dev,
> > > >           dev_warn_ratelimited(dev, "msg received with no recipient\n");
> > > >       /* publish the real size of the buffer */
> > > > -    rpmsg_sg_init(&sg, msg, vrp->buf_size);
> > > > +    rpmsg_sg_init(&sg, msg, vrp->rbuf_size);
> > > >       /* add the buffer back to the remote processor's virtqueue */
> > > >       err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
> > > > @@ -888,9 +888,39 @@ static int rpmsg_probe(struct virtio_device *vdev)
> > > >       else
> > > >           vrp->num_sbufs = MAX_RPMSG_NUM_BUFS;
> > > > -    vrp->buf_size = MAX_RPMSG_BUF_SIZE;
> > > > +    /*
> > > > +     * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
> > > > +     * size from virtio device config space from the resource table.
> > > > +     * If the feature is not supported, then assign default buf size.
> > > > +     */
> > > > +    if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
> > > > +        /* note: virtio_rpmsg_config is defined from remote view */
> > > > +        virtio_cread(vdev, struct virtio_rpmsg_config,
> > > > +                 txbuf_size, &vrp->rbuf_size);
> > > > +        virtio_cread(vdev, struct virtio_rpmsg_config,
> > > > +                 rxbuf_size, &vrp->sbuf_size);
> > > > +
> > > > +        /* The buffers must hold rpmsg header atleast */
> > > > +        if (vrp->rbuf_size < sizeof(struct rpmsg_hdr) ||
> > > > +            vrp->sbuf_size < sizeof(struct rpmsg_hdr)) {
> > > 
> > > 
> > > Hello Tanmay,
> > > 
> > > May I know if the omission of = here is to accommodate the ping/pong/ack
> > > scenarios? mtu will 0
> > > 
> > > 
> > 
> > Yes. At minimum RPMsg header is needed to ping the correct endpoint. We
> > don't need to have any payload attached to the packet. MTU will be
> > sizeof rpmsg_hdr I think.
> > 
> > > > +            dev_err(&vdev->dev,
> > > > +                "vdev config: rx buf sz = %d, tx buf sz = %d\n",
> > > > +                vrp->rbuf_size, vrp->sbuf_size);
> > > > +            err = -EINVAL;
> > > > +            goto vqs_del;
> > > > +        }
> > > > +
> > > > +        dev_dbg(&vdev->dev,
> > > > +            "vdev config: rx buf sz = 0x%x, tx buf sz = 0x%x\n",
> > > > +            vrp->rbuf_size, vrp->sbuf_size);
> > > > +    } else {
> > > > +        vrp->rbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> > > > +        vrp->sbuf_size = DEFAULT_RPMSG_BUF_SIZE;
> > > > +    }
> > > > -    total_buf_space = (vrp->num_rbufs + vrp->num_sbufs) * vrp-
> > > > >buf_size;
> > > > +    total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> > > > +              (vrp->num_sbufs * vrp->sbuf_size);
> > > > +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
> > > >       /* allocate coherent memory for the buffers */
> > > >       bufs_va = dma_alloc_coherent(vdev->dev.parent,
> > > > @@ -908,14 +938,14 @@ static int rpmsg_probe(struct virtio_device *vdev)
> > > >       vrp->rbufs = bufs_va;
> > > >       /* and second part is dedicated for TX */
> > > > -    vrp->sbufs = bufs_va + vrp->num_rbufs * vrp->buf_size;
> > > > +    vrp->sbufs = bufs_va + (vrp->num_rbufs * vrp->rbuf_size);
> > > >       /* set up the receive buffers */
> > > >       for (i = 0; i < vrp->num_rbufs; i++) {
> > > >           struct scatterlist sg;
> > > > -        void *cpu_addr = vrp->rbufs + i * vrp->buf_size;
> > > > +        void *cpu_addr = vrp->rbufs + i * vrp->rbuf_size;
> > > > -        rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
> > > > +        rpmsg_sg_init(&sg, cpu_addr, vrp->rbuf_size);
> > > >           err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
> > > >                         GFP_KERNEL);
> > > > @@ -1001,8 +1031,8 @@ static int rpmsg_remove_device(struct
> > > > device *dev, void *data)
> > > >   static void rpmsg_remove(struct virtio_device *vdev)
> > > >   {
> > > >       struct virtproc_info *vrp = vdev->priv;
> > > > -    unsigned int num_bufs = vrp->num_rbufs + vrp->num_sbufs;
> > > > -    size_t total_buf_space = num_bufs * vrp->buf_size;
> > > > +    size_t total_buf_space = (vrp->num_rbufs * vrp->rbuf_size) +
> > > > +                 (vrp->num_sbufs * vrp->sbuf_size);
> > > >       int ret;
> > > >       virtio_reset_device(vdev);
> > > > @@ -1015,6 +1045,7 @@ static void rpmsg_remove(struct
> > > > virtio_device *vdev)
> > > >       vdev->config->del_vqs(vrp->vdev);
> > > > +    total_buf_space = ALIGN(total_buf_space, PAGE_SIZE);
> > > >       dma_free_coherent(vdev->dev.parent, total_buf_space,
> > > >                 vrp->rbufs, vrp->bufs_dma);
> > > > @@ -1028,6 +1059,7 @@ static struct virtio_device_id id_table[] = {
> > > >   static unsigned int features[] = {
> > > >       VIRTIO_RPMSG_F_NS,
> > > > +    VIRTIO_RPMSG_F_BUFSZ,
> > > >   };
> > > >   static struct virtio_driver virtio_ipc_driver = {
> > > > diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
> > > > new file mode 100644
> > > > index 000000000000..6406bc505383
> > > > --- /dev/null
> > > > +++ b/include/linux/virtio_rpmsg.h
> > > > @@ -0,0 +1,24 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > 
> > > 
> > > Echo Arnaud's comments. If it is intended for UAPI, please keep it in
> > > include/uapi/linux
> > > 
> > > 
> > 
> > It's not intended for UAPI. I need to fix the license. I will check
> > other virtio headers in the same directory and fix the license
> > accordingly.
> > 
> > > > +/*
> > > > + * Copyright (C) Pinecone Inc. 2019
> > > > + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
> > > > + */
> > > > +
> > > > +#ifndef _LINUX_VIRTIO_RPMSG_H
> > > > +#define _LINUX_VIRTIO_RPMSG_H
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +/* The feature bitmap for virtio rpmsg */
> > > > +#define VIRTIO_RPMSG_F_NS    0 /* RP supports name service
> > > > notifications */
> > > > +#define VIRTIO_RPMSG_F_BUFSZ    2 /* RP get buffer size from
> > > > config space */
> > > 
> > > May I know why skip bit 1?
> > > 
> > > 
> > 
> > Thanks, that's a good question. I keept id 2 unmodified from the
> > original series. I don't know why ID 2 was chosen in the original
> > series. I will have to discuss this with the linux remoteproc/rpmsg
> > maintainers and choose the correct ID.
> > 
> > I don't see any problem choosing ID 1, but for some reason if ID 1 was
> > assigned and deprecated (I don't think that is the case) then only we
> > should use ID 2.
> 
> 
> The ID 1 was proposed in an openamp PR [1]. If we
> enter VIRTIO_RPMSG_F_BUFSZ first it makes sense to set its ID to 1.
>

I agree.
 
> [1]https://github.com/OpenAMP/open-amp/pull/160/commits/d4a13128f94e46180285c05a20da78fdca54f7d7
> 
> 
> Regards,
> Arnaud
> 
> > 
> > 
> > Arnaud, Mathieu, Bjorn any input here?
> > 
> > > > +
> > > > +struct virtio_rpmsg_config {
> > > > +    /* The tx/rx individual buffer size(if VIRTIO_RPMSG_F_BUFSZ) */
> > > > +    __u32 txbuf_size;
> > > > +    __u32 rxbuf_size;
> > > > +    __u32 reserved[14]; /* Reserve for the future use */
> > > 
> > > Should we use __virtio32 instead of __u32 to avoid endianness issues?
> > > 
> > > 
> > 
> > Sure, if that is the standard in other virtio headers I will modify it.
> > 
> > Thanks,
> > Tanmay
> > 
> > > > +    /* Put the customize config here */
> > > > +} __attribute__((packed));
> > > > +
> > > > +#endif /* _LINUX_VIRTIO_RPMSG_H */
> > > 
> > > 
> > 
> 

Re: [RFC PATCH 0/2] Enhance RPMsg buffer management

[Mathieu Poirier]

On Fri, Nov 14, 2025 at 10:46:38AM -0800, Tanmay Shah wrote:
> Take rx and tx buffer size from virtio device config space in the
> resource table. This allows each firmware to configure RPMsg buffer size
> as needed in each direction.
> 
> Original seris:
> https://lore.kernel.org/all/1548949280-31794-1-git-send-email-xiaoxiang@xiaomi.com/
> 
> Following modificaitons are done to the original series:
>   - Separate dma allocation is not done for tx and rx buffers. Instead
>     allocated chunk of memory is split between tx and rx buffers.
>   - If vdev doesn't support VIRTIO_RPMSG_F_BUFSZ feature then use the 
>     default size of 512 bytes for buffers
>   - Change MAX_RPMSG_BUF_SIZE to DEFAULT_RPMSG_BUF_SIZE
>   - move virtio_rpmsg.h from uapi to linux dir
>   - RPMsg buffer size must be set to hold rpmsg header at minimum in the
>     vdev config space of the firmware.
>   - align total buf size to page size when allocating and deallocating
>     memory
> 
> Xiang Xiao (2):
>   rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv
>   rpmsg: virtio_rpmsg_bus: get buffer size from config space
> 
>  drivers/rpmsg/virtio_rpmsg_bus.c | 105 ++++++++++++++++++++-----------
>  include/linux/virtio_rpmsg.h     |  24 +++++++
>  2 files changed, 93 insertions(+), 36 deletions(-)
>  create mode 100644 include/linux/virtio_rpmsg.h
>

This set is going somewhere.  Please address the comments that were received and
send a new revision.

Apologies for the long review delay.
Mathieu
 
> 
> base-commit: f982fbb1a6ca3553c15763ad9eb2beeae78d3684
> -- 
> 2.34.1
> 

Re: [RFC PATCH 0/2] Enhance RPMsg buffer management

[Tanmay Shah]

On 12/17/25 3:31 PM, Mathieu Poirier wrote:
> On Fri, Nov 14, 2025 at 10:46:38AM -0800, Tanmay Shah wrote:
>> Take rx and tx buffer size from virtio device config space in the
>> resource table. This allows each firmware to configure RPMsg buffer size
>> as needed in each direction.
>>
>> Original seris:
>> https://lore.kernel.org/all/1548949280-31794-1-git-send-email-xiaoxiang@xiaomi.com/
>>
>> Following modificaitons are done to the original series:
>>   - Separate dma allocation is not done for tx and rx buffers. Instead
>>     allocated chunk of memory is split between tx and rx buffers.
>>   - If vdev doesn't support VIRTIO_RPMSG_F_BUFSZ feature then use the 
>>     default size of 512 bytes for buffers
>>   - Change MAX_RPMSG_BUF_SIZE to DEFAULT_RPMSG_BUF_SIZE
>>   - move virtio_rpmsg.h from uapi to linux dir
>>   - RPMsg buffer size must be set to hold rpmsg header at minimum in the
>>     vdev config space of the firmware.
>>   - align total buf size to page size when allocating and deallocating
>>     memory
>>
>> Xiang Xiao (2):
>>   rpmsg: virtio_rpmsg_bus: allow the different vring size for send/recv
>>   rpmsg: virtio_rpmsg_bus: get buffer size from config space
>>
>>  drivers/rpmsg/virtio_rpmsg_bus.c | 105 ++++++++++++++++++++-----------
>>  include/linux/virtio_rpmsg.h     |  24 +++++++
>>  2 files changed, 93 insertions(+), 36 deletions(-)
>>  create mode 100644 include/linux/virtio_rpmsg.h
>>
> 
> This set is going somewhere.  Please address the comments that were received and
> send a new revision.
> 
> Apologies for the long review delay.
> Mathieu
>  

Sure Mathieu, no problem. I will be on leave in the January, and will start
working on this patch series once I come back.

Thanks,
Tanmay

>>
>> base-commit: f982fbb1a6ca3553c15763ad9eb2beeae78d3684
>> -- 
>> 2.34.1
>>