Re: [Discussion]: Enhance virtio rpmsg bus driver buffer allocation

[Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>]

On 11/28/23 12:19, Divin Raj wrote:
> On 11/28/23 8:34 AM, Arnaud POULIQUEN wrote:
>>
>>
>> On 11/24/23 17:45, Divin Raj wrote:
>>> Hi Arnaud,
>>> Please find my comments inline.
>>>
>>> On 11/20/23 10:14 AM, Arnaud POULIQUEN wrote:
>>>> Hi Divin,
>>>>
>>>> On 11/17/23 23:24, Divin Raj wrote:
>>>>> On 10/23/23 11:44 AM, Divin Raj wrote:
>>>>>> Hello all,
>>>>>>
>>>>>> I am reaching out with reference to the patch discussed here: Enhanced
>>>>>> virtio rpmsg bus driver buffer allocation.
>>>>>> <https://lore.kernel.org/all/CAH2Cfb-sv3SAL8bcczC-Dc3_r58MYZCS7s7zGtn1Qfo3mmBqVg@mail.gmail.com/>
>>>>>>
>>>>>> I've been keenly following the developments around enhancing buffer
>>>>>> allocation strategies, especially those focused on dynamic buffer sizing
>>>>>> and the considerations for systems under varying memory constraints.This
>>>>>> work is highly relevant to several projects I am involved in, and I am
>>>>>> quite interested in its progression. May I kindly request an update on
>>>>>> the current phase of these initiatives? Additionally, I am eager to know
>>>>>> if there would be an opportunity for me to contribute to enhancing the
>>>>>> patch, possibly by working on improvements or assisting in verification
>>>>>> processes.
>>>>>>
>>>>>> Furthermore, if there are any condensed resources, summaries, or
>>>>>> specific threads that encapsulate recent advancements or discussions on
>>>>>> this topic, I would be grateful to receive directions to them.
>>>>>>
>>>>>> I appreciate everyone's dedicated efforts and invaluable contributions
>>>>>> to this area of development. Looking forward to the updates.
>>>>>>
>>>>>> Regards Divin
>>>>>>
>>>>> Hello Linux Community,
>>>>>
>>>>> In one of our internal projects, we encountered a challenge with RPMSG
>>>>> buffer allocation. Our goal is to optimize memory allocation for an
>>>>> out-of-tree RPMSG Ethernet device driver using virtio. This is to ensure
>>>>> support for packet sizes matching the standard MTU (Maximum Transmission
>>>>> Unit) size of 1500 bytes.
>>>>>
>>>>> To mitigate this issue, There are few possible solutions:
>>>>>
>>>>> 1. Configure buffer size and number through Kconfig.
>>>>> 2. Permit the firmware creator to determine the most suitable value from
>>>>>     the resource table.
>>>>> 3. Enable independent configurations on both ends. This approach would
>>>>> support both dynamic and fixed buffer configurations using a generic
>>>>> allocator.
>>>>>
>>>>> Reference:
>>>>>
>>>>> [1]:
>>>>> https://lore.kernel.org/all/1548949280-31794-4-git-send-email-xiaoxiang@xiaomi.com/
>>>>> [2]: https://lore.kernel.org/all/20190701061353.GE1263@builder/
>>>>>
>>>>>
>>>>> Draft Design Overview:
>>>>>
>>>>> Based on the reference patch and the discussions, we have outlined the
>>>>> following key points for the belw design:
>>>>>
>>>>> 1. Assure compatibility, enabling both Linux and the remote system to
>>>>> interchangeably transmit and receive messages, irrespective of size.
>>>>> 2. For systems with constrained shared memory:
>>>>> Systems with small, shared memory, we need to deal with a
>>>>> limited/optimized memory chunk. To avoid memory fragmentation, the
>>>>> allocator should have a pre-reserved buffer pool
>>>>> 3. The implementation should ensure that the remote side does not
>>>>> receive messages based on its allocation parameters.
>>>>>
>>>>> do you think it could make sense?
>>>>>
>>>>> High level view:
>>>>> +------------------+                               +------------------+
>>>>> |                  |                               |                  |
>>>>> |      Linux       |                               |      Remote      |
>>>>> |                  |                               |                  |
>>>>> |   +----------+   |       +-----------------+     |   +----------+   |
>>>>> |   |   RPMSG  |   | <---> | Buffer Allocator|<--->|   | RPMSG    |   |
>>>>> |   +----------+   |       | (Dynamic/Static)|     |   +----------+   |
>>>>> |                  |       +-----------------+     |                  |
>>>>> +------------------+                               +------------------+
>>>>>
>>>>>
>>>>> Detailed view:
>>>>>
>>>>>                     +-------------------------+
>>>>>                     |  Message Creation       |
>>>>>                     |  (Both Linux/Remote)    |
>>>>>                     +------------+------------+
>>>>>                                  |
>>>>>                                  v
>>>>>                     +-------------------------+
>>>>>                     | Determine the allocation|
>>>>>                     | strategy                |
>>>>>                     +------------+------------+
>>>>>                                  |
>>>>>                   +--------------+--------------+
>>>>>                   |                             |
>>>>> +-------------------------------+  +-------------------------------+
>>>>> | Dynamic allocation            |  | Static allocation             |
>>>>> | (Buffer allocator allocates   |  | (Pre-reserved memory          |
>>>>> | memory space as needed,       |  | space)                        |
>>>>> | based on the current          |  |                               |
>>>>> | message requirement )         |  |                               |
>>>>> +-------------------------------+  +-------------------------------+
>>>>
>>>> Do you have a proposal for dynamic allocation?
>>>>
>>>> RPMSG is based on the virtio protocol. The virtio driver in the Linux kernel
>>>> is responsible for allocating buffers for the virtio device on the remote
>>>> processor.
>>>>
>>>> In the current implementation (static allocation) the Linux
>>>> kernel allocates predefined buffers for the remote processor.
>>>>
>>>> How would you manage the fact that the sender allocates its own buffers and
>>>> references
>>>> them in the vring descriptor? This would require each core to have
>>>> a dual role, right?
>>>> - a virtio driver role on its TX vring
>>>> - a virtio device role on its RX vring."
>>>>
>>> I'm unsure if a dual role is feasible under the Virtio specification.
>>
>> At least, it does not seem to align with the philosophy of VirtIO.
>>
>>
>>> However, would it make sense to set the size of the outbuf based on the
>>> Maximum Transmission Unit (MTU) size that is supported? Additionally,
>>> the size of the inbuf could be set by the firmware, suggesting that it
>>> should be derived from the resource table. With this approach, I believe
>>> the sender can decide the maximum size.
>>
>> It is not clear to me what your proposal is.
>> Are you speaking about a pre-allocated buffers as proposed in [1],
>> or are you speaking about dynamic allocation of the RPMsg in a pool?
> 
> we are at the initial phase of this investigation. As we previously
> discussed, option 3 is not feasible in accordance with the virtio
> specification.The above proposed solution aligns with [1], suggesting
> preallocated in_buf and out_buf, with sizes determined from the resource
> table and MTU. By allowing Linux to decide the out_buf size and the
> remote to decide the in_buf size, I believe we can avoid conflicts. If
> everyone agrees on a common idea, then it would be a good starting point

Thanks for the clarification. It seems reasonable to me to start with a
pre-allocated buffer with a fixed size specified by the remote firmware.

Bjorn, Mathieu,
Please, could you share you point of view on the topic?

Thanks,
Arnaud

> 
> Regards
> Divin
> 
>> Regards,
>> Arnaud
>>
>>>
>>> Regards
>>> Divin
>>>
>>>>
>>>> Regards,
>>>> Arnaud
>>>>
>>>
>>>>
>>>>>
>>>>> We would greatly appreciate any feedback, suggestions, or improvements
>>>>> you could provide.
>>>>>
>>>>> Thank you for your time and consideration.
>>>>>
>>>>> Regards
>>>>> Divin
>>>>> IMPORTANT NOTICE: The contents of this email and any attachments are
>>>>> confidential and may also be privileged. If you are not the intended
>>>>> recipient,
>>>>> please notify the sender immediately and do not disclose the contents to any
>>>>> other person, use it for any purpose, or store or copy the information in any
>>>>> medium. Thank you.
>>>
>>> IMPORTANT NOTICE: The contents of this email and any attachments are
>>> confidential and may also be privileged. If you are not the intended recipient,
>>> please notify the sender immediately and do not disclose the contents to any
>>> other person, use it for any purpose, or store or copy the information in any
>>> medium. Thank you.
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.