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

[Mathieu Poirier]

On Thu, 23 Jul 2026 at 11:27, Arnaud POULIQUEN
<arnaud.pouliquen@xxxxxxxxxxx> wrote:
>
>
>
> On 7/23/26 16:13, Mathieu Poirier wrote:
> > On Thu, 23 Jul 2026 at 03:11, Arnaud POULIQUEN
> > <arnaud.pouliquen@xxxxxxxxxxx> wrote:
> >>
> >> Hello Mathieu,
> >>
> >> On 7/22/26 16:49, Mathieu Poirier wrote:
> >>> On Tue, Jul 21, 2026 at 11:02:13AM -0500, Shah, Tanmay wrote:
> >>>>
> >>>>
> >>>> On 7/21/2026 10:50 AM, Mathieu Poirier wrote:
> >>>>> On Thu, Jul 16, 2026 at 11:12:55AM -0500, Shah, Tanmay wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 7/16/2026 10:48 AM, Mathieu Poirier wrote:
> >>>>>>> On Wed, 15 Jul 2026 at 11:28, Shah, Tanmay <tanmays@xxxxxxx> 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 <tanmay.shah@xxxxxxx>
> >>>>>>>>>> ---
> >>>>>>>>>> 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.
> >>>>>
> >>>>> Regardless of the buffer size, whether it is set to 512 byte or some arbitrary
> >>>>> value by the remote processsor's firmware, there is a possibility of a
> >>>>> discrepancy with what is expected by the protocol.  Right now rpmsg_get_mtu()
> >>>>> returns 512 regardless of what a protocol uses.  The only thing that should be
> >>>>> important to the protocol is not to exceed that limit.
> >>>>>
> >>>>
> >>>> I think I am missing something. Are you saying that buffer size can not
> >>>> be configured greater than 512 bytes?
> >>>
> >>> I am not.
> >>>
> >>> What I am saying is that if alignment is important to a remote processor, it
> >>> should choose the buffer size accordingly.  rpmsg_get_mtu() should return the
> >>> value of the buffer size, exactly the way it is today.
> >>
> >> Did you get a chance to look to my previous reply [1]?
> >>
> >
> > Unfortunately I haven't had the opportunity to reach that point in my Inbox.
> >
> >>   From my understanding, due to the Virtio specification, we cannot
> >> guarantee that
> >> the buffer size will not be updated by the main processor. Having an
> >> alignment
> >> field ensures that the main processor takes the remote processor’s
> >> constraints
> >> into account if it updates the RPMsg buffer size.
> >>
> >
> > Where does the main processor change the buffer size?
>
> Linux does not, but any main processor may do so, consistent with the
> Virtio specification. If requested in OpenAMP with a valid usecase, no
> reason to reject if it is not specified that it is not possible.

"Linux does not" - that is the important part.  This patchset is for
Linux and RPMSG is part of the Linux code base.

>
> What we define here will probably become the reference for all RPMsg
> implementations...
>
> >
> >> That said, if we decide to remove the alignment field, we must clearly
> >> document the constraints for the RPMsg Virtio config space:
> >>
> >> - virtio_rpmsg_config cannot be changed by the Virtio driver.
> >> - the sizes must respect the device and driver alignment constraint.
> >>
> >
> > If the size chosen by the device isn't compatible with what the driver
> > can accommodate, then initialization simply fails.
> >
> >> This is not my preferred solution,because that means that the remote
> >> processor has
> >> to know the alignment constraints of the main processor.
> >>
> >
> > It would have to anyway wouldn't it?
>
> I think I was wrong about this last argument. Since the main processor
> allocates the buffers, it can apply its own alignment constraints,so the
> remote processor does not need to know them.
>
> That makes the option without an alignment field acceptable to me,
> if restrictions are documented.
>
> Here are the constraints that I would like to see documented (not
> necessary using the exact terms) in Linux and OpenAMP to make the usage
> clear for the RPMsg virtio config space:
> - The virtio driver must not update the config space.

Currently, there is no chance of that happening in Linux.

> - The virtio driver can allocate a memory area equal to or greater than
>    the RX/TX message size specified by the device. The effective RX/TX size
>    remains the one requested by the device.

Although hidden by the memory allocation API, this is often the case in Linux.

> - The virtio device should specify sizes that respect its own alignment
>    constraints.
>

Yes, that has been my position.

> Does this would be acceptable to you?
> Regards,
> Arnaud
>
> >
> >   And as I said from the beginning, this is not something that has come
> > up in the 20 years this subsystem existed.  There is no point in
> > adding all this complexity for a something that _may_ happen.
> >
> >> [1]https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg2643398.html
> >>
> >> Regards,
> >>
> >> Arnaud
> >>>
> >>>>
> >>>> If the higher level protocol (not RPMsg) wants to use 4030 bytes for
> >>>> single packet payload then that is what the MTU size should be. And so
> >>>> the firmware will configure 4030 bytes as single buffer size in the vdev
> >>>> config space. That is why alignment should be treated separately.
> >>>> Because it is not equal to payload size needed by higher level protocol.
> >>>
> >>> In that case and assuming alignment is required, the buffer size should be 4096
> >>> and rpmsg_get_mtu() should also return 4096.  How a higher protocol uses the
> >>> buffer space is none of our concern.
> >>>
> >>> Currently, the buffer size is set to 512 and users don't always fill the entire
> >>> buffer.  I don't see why things should be different with a configurable 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.
> >>>>>
> >>>>> I agree with the first sentence but not the second.  The only thing protocols
> >>>>> should care about is the start address of a buffer and that its size is
> >>>>> sufficient for what it needs.
> >>>>>
> >>>>>>
> >>>>>> 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
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> >>
>