Enhance rpmsg buf config#684
Conversation
|
we can review this one after the release. |
tnmysh
force-pushed
the
enhance_rpmsg_buf_config
branch
2 times, most recently
from
7310369 to
3474c42
Compare
tnmysh
force-pushed
the
enhance_rpmsg_buf_config
branch
2 times, most recently
from
6ddcf11 to
7c0b658
Compare
| return status; | ||
|
|
||
| if (features & (1 << VIRTIO_RPMSG_F_BUFSZ)) | ||
| rvdev->config = *config; |
There was a problem hiding this comment.
Here you should use virtio_read_config to retrieve the config from the transport layer
Did you have a look to @xiaoxiang781216 work in #155 ?
Seems to me the good approach
tnmysh
force-pushed
the
enhance_rpmsg_buf_config
branch
2 times, most recently
from
24613a3 to
8289cfd
Compare
Introduce new feature bit that allows rpmsg buffer size via virtio device config space. If the feature is available then, driver will set single rpmsg buffer size from virtio device config space in the resource table. New virtio feature allows to configure rpmsg single buffer size via virtio device config space. Hence, rx buffer size can be provided by vdev device config space as well. If BUFSZ feature bit is set, then use rx buffer size from the virtio config space in the resource table. Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
tnmysh
force-pushed
the
enhance_rpmsg_buf_config
branch
from
8289cfd to
a361586
Compare
|
|
||
| if (features & (1 << VIRTIO_RPMSG_F_BUFSZ)) | ||
| virtio_write_config(rvdev->vdev, 0, (void *)config, | ||
| sizeof(*config)); |
There was a problem hiding this comment.
The config could be read by the virtio driver before this code is executed. For instance the Linux probes the rpmsg in parallel of the start of the remoteproc firmware based on the resource table that should contain the config.
For my here we should only read the config if VIRTIO_RPMSG_F_BUFSZ feature bit is set else we should rely on *config.
One question is: should we also mange the config write or should we consider that it is part of the resource table.
I propose to discuss this in next OpenAMP meeting
There was a problem hiding this comment.
7/1 OpenAMP System Reference call:
- Arnaud to confirm it is a standard pattern w/ MMIO
- Tanmay to confirm would work to have read instead of write for current use case & update accordingly.
Allow write only for use case when it is compiled as driver.
There was a problem hiding this comment.
Extracted from virtio spec 1 .4:
2.5.2 Device Requirements: Device Configuration Space
The device MUST allow reading of any device-specific configuration field before FEATURES_OK is set by the driver. This includes fields which are conditional on feature bits, as long as those feature bits are offered by the device.
3.2 Device Operation
When operating the device, each field in the device configuration space can be changed by either the driver or the device.
Whenever such a configuration change is triggered by the device, driver is notified. This makes it possible for drivers to cache device configuration, avoiding expensive configuration reads unless notified.
So it seems that having a static configuration as first step is quite simple and reliable
- the device provides a static configuration space (in the resource table).
- the driver can update it until FEATURES_OK is set
- the device read the configuration space when DRIVER_OK is set
4 participants
Corresponding Linux kernel patch series: https://lore.kernel.org/all/20251114184640.3020427-1-tanmay.shah@amd.com/