On 7/16/2026 10:48 AM, Mathieu Poirier wrote:
> On Wed, 15 Jul 2026 at 11:28, Shah, Tanmay <[email protected]> wrote:
>>
>> 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 <[email protected]>
>>>> ---
>>>> 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.
>>
>
> How is that scenario different from today's 512 byte buffer size?
> Most users don't use all 512 bytes and we don't run in the problem
> described above?
>
512 buffer size is hardcoded, so it is enforced on the protocol by the
framework. But by allowing the configuration of the buffer size we are
allowing the protocol to decide what the buffer size should be. So, when
user request the buffer via rpmsg_get_mtu() API, then that should be the
original buffer size which is expected by the protocol, which may not be
same as the aligned buffer size.
If for internal management we want to treat buffer size = aligned buffer
size, I am okay. But rpmsg_get_mtu() must give unaligned buffer size
which is expected by the protocol.
Thanks,
Tanmay
>> 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.
>>
>
> The same as in my previous email to Arnaud applies here - is this an
> immediate requirement of something we think may be happening in the
> future?
>
IMHO, vendors will use it if the feature is available, otherwise the
need to optimize alignment is not easily encountered.
>
>> 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
>>>>
>>
