On Thu, 23 Jul 2026 at 18:28, Shah, Tanmay <[email protected]> wrote: > > > > On 7/23/2026 12:27 PM, Arnaud POULIQUEN 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. > > > > 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. > > Ack. > > > - 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. > > Here I have a question. Currently we do not have any mechanism (such as > dt property) that allows the driver (linux) side alignment. In such case > why even mention it in the documentation? Or if we do, then we also need > to make it clear that Linux provides no such mechanism today. >
I agree. Alignment is chosen by the device and should be documented with the device. > > The effective RX/TX size remains the one requested by the device. > > IIUC, does this mean rpmsg_get_mtu() API should return an aligned buffer > size from device not from the driver ? > Correct. > > - The virtio device should specify sizes that respect its own alignment > > constraints. > > Ack. This means, we don't need extra alignment field at all in the > config space. The firmware directly mention aligned size only in the > txbuf_size and rxbuf_size fields. > That is what I think. > Please let me know if my understanding is missing something. I am okay > to document above points. > > Thanks. > > > > > 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 > >>>>>>>>>>> > >>>>>>>>> > >>>>>>> > >>>>> > >>>> > >>> > > >
