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]> 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. > >>>>> > >>>>> 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/[email protected]/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 > >>>>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >>> > >> >
