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

On Thu, 23 Jul 2026 at 11:27, Arnaud POULIQUEN
<[email protected]> wrote:
>
>
>
> On 7/23/26 16:13, Mathieu Poirier wrote:
> > On Thu, 23 Jul 2026 at 03:11, Arnaud POULIQUEN
> > <[email protected]> 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 <[email protected]> wrot=
e:
> >>>>>>>>
> >>>>>>>> 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 not=
ifications */
> >>>>>>>>>> +#define VIRTIO_RPMSG_F_BUFSZ        1 /* RP get buffer size f=
rom config space */
> >>>>>>>>>> +
> >>>>>>>>>> +/* Version of struct virtio_rpmsg_config understood by this d=
river */
> >>>>>>>>>> +#define RPMSG_VDEV_CONFIG_V1        1
> >>>>>>>>>> +
> >>>>>>>>>> +/**
> >>>>>>>>>> + * struct virtio_rpmsg_config - config space for rpmsg virtio=
 device
> >>>>>>>>>> + *
> >>>>>>>>>> + * @version:        version of this structure, currently %RPM=
SG_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. Th=
is alignment
> >>>>>>>>>> + *               will not decide actual size of the buffer bu=
t will be
> >>>>>>>>>> + *               used to decided the start address of the buf=
fer. The
> >>>>>>>>>> + *               actual size of the buffer can be different t=
han 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 alig=
nment will be
> >>>>>>>>> used for something else.  I'm fine with a buffer alignment requ=
irement 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 main=
tain and prone to
> >>>>>>>>> bugs.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> I had the same concern before. However, following example change=
d my mind:
> >>>>>>>>
> >>>>>>>> So, a single buffer size is the MTU size of a packet for the pro=
tocol
> >>>>>>>> supported by the firmware. Now that can be different than the al=
igned
> >>>>>>>> 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 do=
esn'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 proble=
m
> >>>>>>> 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 som=
e arbitrary
> >>>>> value by the remote processsor's firmware, there is a possibility o=
f 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 tha=
t 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 proces=
sor, it
> >>> should choose the buffer size accordingly.  rpmsg_get_mtu() should re=
turn 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 I=
nbox.
> >
> >>   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=E2=80=
=99s
> >> 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 si=
ze
>    remains the one requested by the device.

Although hidden by the memory allocation API, this is often the case in Lin=
ux.

> - 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/[email protected]/msg264339=
8.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 proto=
col.
> >>>
> >>> In that case and assuming alignment is required, the buffer size shou=
ld be 4096
> >>> and rpmsg_get_mtu() should also return 4096.  How a higher protocol u=
ses 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 configurab=
le buffer
> >>> size.
> >>>
> >>>>
> >>>>>>
> >>>>>> If for internal management we want to treat buffer size =3D aligne=
d buffer
> >>>>>> size, I am okay. But rpmsg_get_mtu() must give unaligned buffer si=
ze
> >>>>>> 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 siz=
e 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 wort=
h 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 t=
he
> >>>>>>> future?
> >>>>>>>
> >>>>>>
> >>>>>> IMHO, vendors will use it if the feature is available, otherwise t=
he
> >>>>>> 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 a=
nd 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_BU=
FSZ) */
> >>>>>>>>>> +    __virtio32 txbuf_size;
> >>>>>>>>>> +    __virtio32 rxbuf_size;
> >>>>>>>>>> +} __packed;
> >>>>>>>>>> +
> >>>>>>>>>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
> >>>>>>>>>> --
> >>>>>>>>>> 2.34.1
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> >>
>