Re: [PATCH v5 3/5] rpmsg: virtio_rpmsg_bus: get buffer size from config space

["Shah, Tanmay" <tanmays@amd.com>]

Hi,

Please find my response below:

On 7/15/2026 11:24 AM, Mathieu Poirier wrote:
> On Fri, Jul 10, 2026 at 12:28:29PM -0700, Tanmay Shah wrote:
>> 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: Tanmay Shah <tanmay.shah@amd.com>
>> ---
>> Changes in v5:
>>
>>   - fix documentation about alignment of the buffer size
>>   - change version field from u16 to u8
>>   - remove buffer alignment check
>>   - Separate buffer alignment vs MTU of a single buffer
>>   - Use buffer alignment only to get next buffer address at alignment
>>     boundary
>>

[...]

>> +#ifndef _LINUX_VIRTIO_RPMSG_H
>> +#define _LINUX_VIRTIO_RPMSG_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/virtio_types.h>
>> +
>> +/* The feature bitmap for virtio rpmsg */
>> +#define VIRTIO_RPMSG_F_NS	0 /* RP supports name service notifications */
>> +#define VIRTIO_RPMSG_F_BUFSZ	1 /* RP get buffer size from config space */
>> +
>> +/* Version of struct virtio_rpmsg_config understood by this driver */
>> +#define RPMSG_VDEV_CONFIG_V1	1
>> +
>> +/**
>> + * struct virtio_rpmsg_config - config space for rpmsg virtio device
>> + *
>> + * @version:	version of this structure, currently %RPMSG_VDEV_CONFIG_V1.
>> + * @size:	size of this structure in bytes.
>> + * @rpmsg_buf_align: alignment in bytes for each buffer. Must be a power of
>> + *		     two. If 0 then no alignment will be done. This alignment
>> + *		     will not decide actual size of the buffer but will be
>> + *		     used to decided the start address of the buffer. The
>> + *		     actual size of the buffer can be different than the
>> + *		     aligned size of the buffer.
> 
> Is there really a need to have a buffer size different from its alignment?  It's
> not like the (small) delta between the buffer size and its alignment will be
> used for something else.  I'm fine with a buffer alignment requirement but in
> those cases, the firmware should set the size of the buffer in accordance with
> its alignment requirement.  Otherwise, the complexity needed to manage the
> discrpancy between the two yields a driver that is hard to maintain and prone to
> bugs.
> 

I had the same concern before. However, following example changed my mind:

So, a single buffer size is the MTU size of a packet for the protocol
supported by the firmware. Now that can be different than the aligned
size of the buffer.

For example, the higher level protocol (not rpmsg) has 430 bytes as the
max size of a payload. However, cache line alignment is 64-bytes. Then
in that case, the aligned buffer size is 448 bytes. But, that doesn't
mean we can say protocol's MTU size is 448 bytes. If user end up
treating MTU size 448 bytes and use space beyond 430 bytes, then the
higher level apps might discard that data and communication may fail.

The alignment field is used only to decide where the next buffer start
address is to ease cache operations.

Sure, we need to maintain this complexity, but I think it's worth it.

Thanks,
Tanmay

>> + * @txbuf_size:	Tx buf size from remote's view. For Linux this is rx buf size.
>> + * @rxbuf_size:	Rx buf size from remote's view. For Linux this is tx buf size.
>> + *
>> + * This is the configuration structure shared by the device and the driver,
>> + * read when %VIRTIO_RPMSG_F_BUFSZ is negotiated. The fields are laid out so
>> + * the structure is naturally 32-bit aligned.
>> + */
>> +struct virtio_rpmsg_config {
>> +	u8 version;
>> +	__virtio16 size;
>> +	__virtio16 rpmsg_buf_align;
>> +	/* The tx/rx individual buffer size (if VIRTIO_RPMSG_F_BUFSZ) */
>> +	__virtio32 txbuf_size;
>> +	__virtio32 rxbuf_size;
>> +} __packed;
>> +
>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
>> -- 
>> 2.34.1
>>