Skip to content

Conversation

@luckyyaojin
Copy link

@luckyyaojin luckyyaojin commented Dec 20, 2024

support set trace callback to trace buffer status

Adding this is to track the application and release of rpmsg buffer,then connect to the OS trace system

users can enable RPMSG_TRACE to monitor application and release of txbuffer

option (WITH_DOC "Build with documentation" OFF)
option (WITH_RPMSG_TRACE "Enable Rpmsg Device Trace" OFF)

if (WITH_RPMSG_TRACE)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain commit mesasge in detail what this option is for.

Copy link
Author

@luckyyaojin luckyyaojin Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this is to track the application and release of rpmsg buffer,then connect to the OS trace system

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked virtqueue_dump functionality? Is it not enough for above purpose?

Copy link
Author

@luckyyaojin luckyyaojin Dec 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Virtqueue_dump is not enough for record historical information of buffer application and release.
it just record the info of virtqueue

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look to me too complex just for some specific traces...
What about using RPMSG_DEBUG in open-amp/lib/rpmsg/rpmsg_internal.h?
Then you could define RPMSG_LOG macro to add debug traces in RPMSG

* @release_tx_buffer: trace callback, called when release a tx buffer
*/
struct rpmsg_device_trace {
void (*get_tx_buffer)(struct rpmsg_device *rvdev, void *hdr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What these callbacks are expected to trace ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpmsg_get_tx_payload_buffer and rpmsg_release_tx_buffer

Adding this is to track the application and release of rpmsg buffer,
then connect to the OS trace system

Signed-off-by: Bowen Wang <[email protected]>
@luckyyaojin
Copy link
Author

luckyyaojin commented Dec 23, 2024

@arnopo hi, please help me to solve this ci problem, think you very much: open-amp Heath Check / Zephyr build from latest on main (pull_request)

CMake Error at /github/workspace/zephyrproject/zephyr/cmake/modules/boards.cmake:228 (message):
Invalid BOARD; see above.
Call Stack (most recent call first):
cmake/modules/sysbuild_default.cmake:15 (include)
/github/workspace/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:75 (include)
/github/workspace/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate)
/github/workspace/zephyrproject/zephyr/share/sysbuild-package/cmake/SysbuildConfig.cmake:8 (include)
template/CMakeLists.txt:10 (find_package)

-- Configuring incomplete, errors occurred!
FATAL ERROR: command exited with status 1: /github/workspace/.venv/bin/cmake -DWEST_PYTHON=/github/workspace/.venv/bin/python3 -B/github/workspace/zephyrproject/zephyr/build -GNinja -DBOARD=lpcxpresso54114_m4 -S/github/workspace/zephyrproject/zephyr/share/sysbuild -DAPP_DIR:PATH=/github/workspace/zephyrproject/zephyr/samples/subsys/ipc/openamp

@luckyyaojin luckyyaojin requested a review from tnmysh December 25, 2024 11:58
@arnopo
Copy link
Collaborator

arnopo commented Jan 2, 2025

@arnopo hi, please help me to solve this ci problem, think you very much: open-amp Heath Check / Zephyr build from latest on main (pull_request)

CMake Error at /github/workspace/zephyrproject/zephyr/cmake/modules/boards.cmake:228 (message): Invalid BOARD; see above. Call Stack (most recent call first): cmake/modules/sysbuild_default.cmake:15 (include) /github/workspace/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:75 (include) /github/workspace/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate) /github/workspace/zephyrproject/zephyr/share/sysbuild-package/cmake/SysbuildConfig.cmake:8 (include) template/CMakeLists.txt:10 (find_package)

-- Configuring incomplete, errors occurred! FATAL ERROR: command exited with status 1: /github/workspace/.venv/bin/cmake -DWEST_PYTHON=/github/workspace/.venv/bin/python3 -B/github/workspace/zephyrproject/zephyr/build -GNinja -DBOARD=lpcxpresso54114_m4 -S/github/workspace/zephyrproject/zephyr/share/sysbuild -DAPP_DIR:PATH=/github/workspace/zephyrproject/zephyr/samples/subsys/ipc/openamp

I will have a look , you can ignore it for the moment , only the required checks are mandatory

@github-actions
Copy link

This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity.

@github-actions github-actions bot added the Stale label Feb 17, 2025
@CV-Bowen
Copy link
Contributor

@arnopo @tnmysh After some thinking, about the trace functions for the rpmsg virtio, I wonder if we can add a new api metal_trace in Libmetal, so this api can be used for not only the rpmsg virtio but also the virtio drivers.

@github-actions github-actions bot removed the Stale label Aug 18, 2025
@arnopo
Copy link
Collaborator

arnopo commented Aug 26, 2025

@arnopo @tnmysh After some thinking, about the trace functions for the rpmsg virtio, I wonder if we can add a new api metal_trace in Libmetal, so this api can be used for not only the rpmsg virtio but also the virtio drivers.

The trace API is already defined in libmetal: https://github.com/OpenAMP/libmetal/blob/main/lib/log.h,
What about using RPMSG_DEBUG defined in https://github.com/OpenAMP/open-amp/blob/main/lib/rpmsg/rpmsg_internal.h#L17 as proposed in a previous comment ?

#ifdef RPMSG_DEBUG
#define RPMSG_LOG_DBG(fmt, args...)  metal_dbg(fmt, ##args)
#endif

@CV-Bowen
Copy link
Contributor

CV-Bowen commented Aug 30, 2025

@arnopo I mean add a new API named metal_trace() or something, because the log system is still different with the trace system in many OS.
Or your suggestion is adding a new log level to represent the trace message?

@arnopo
Copy link
Collaborator

arnopo commented Sep 2, 2025

@arnopo I mean add a new API named metal_trace() or something, because the log system is still different with the trace system in many OS. Or your suggestion is adding a new log level to represent the trace message?

I better understand your request. You would have two types of information: logs and traces.My concern is that these concepts could differ depending on the OS.
Please first detail which kind of information you expect to have in traces versus logs.
We should have a clear definition of Open-AMP traces and logs as the first step, to be able to discuss how to implement them.

@CV-Bowen
Copy link
Contributor

CV-Bowen commented Sep 2, 2025

@arnopo I mean add a new API named metal_trace() or something, because the log system is still different with the trace system in many OS. Or your suggestion is adding a new log level to represent the trace message?

I better understand your request. You would have two types of information: logs and traces.My concern is that these concepts could differ depending on the OS. Please first detail which kind of information you expect to have in traces versus logs. We should have a clear definition of Open-AMP traces and logs as the first step, to be able to discuss how to implement them.

Firstly, in many embedded projects, the log is always output the UART for the convenience of debugging, and the trace information is always output the ram and can be dumped manually or triggered by some special system event (e.g. PANIC). So the log is a always a slow process but trace has small impact on performance.

So I think the log is used to print some infrequent information such as the error information and initialization information.
Instead, trace can be used to profile the performance issue and debug the bug that requires historical sending or receiving information. So I would like to add a trace log every time a message is sent or received. And the trace log should also can be used to record the send and receive data to debug more complicated bug.

@arnopo
Copy link
Collaborator

arnopo commented Sep 5, 2025

@arnopo I mean add a new API named metal_trace() or something, because the log system is still different with the trace system in many OS. Or your suggestion is adding a new log level to represent the trace message?

I better understand your request. You would have two types of information: logs and traces.My concern is that these concepts could differ depending on the OS. Please first detail which kind of information you expect to have in traces versus logs. We should have a clear definition of Open-AMP traces and logs as the first step, to be able to discuss how to implement them.

Firstly, in many embedded projects, the log is always output the UART for the convenience of debugging, and the trace information is always output the ram and can be dumped manually or triggered by some special system event (e.g. PANIC). So the log is a always a slow process but trace has small impact on performance.

So I think the log is used to print some infrequent information such as the error information and initialization information. Instead, trace can be used to profile the performance issue and debug the bug that requires historical sending or receiving information. So I would like to add a trace log every time a message is sent or received. And the trace log should also can be used to record the send and receive data to debug more complicated bug.

This is probably a good topic to discuss in the next OpenAMP technical meeting...

Could you please detail what you expect to output in trace logs?

  • API calls (possibly with a new metal_log type)
  • Current metal_log, with some implementations in libmetal to redirect prints to log and/or trace outputs
  • Other options?

@tnmysh
Copy link
Collaborator

tnmysh commented Sep 5, 2025

Firstly, in many embedded projects, the log is always output the UART for the convenience of debugging, and the trace information is always output the ram and can be dumped manually or triggered by some special system event (e.g. PANIC). So the log is a always a slow process but trace has small impact on performance.

open-amp can have a trace buffer type of resource where you can copy all the logs in RAM when you use metal_log API. In Heterogeneous system it's recommended to not throw logs on UART usually because number of UARTs are mostly less than number of cores and possibility of corruption of logs. The recommended way is to use trace buffer and from linux you can get those logs via debugfs or memory dump etc.

trace can be used to profile the performance

We do have debug level or many other levels of log for this. Besides it's not good idea to keep any logs enabled to measure performance of actual software for long time. We use logs only for notification purpose if performance goes down or varies a lot.

And I agree it varies from project to project, and that is why should not be introduced in a library, but probably some higher level framework.

@arnopo
Copy link
Collaborator

arnopo commented Sep 17, 2025

We discussed this point during the OpenAMP meeting.
To move forward on this topic the proposal is:

  • propose a PR in open-amp to add traces that would miss. Use existing metal_log as a first step using RPMSG_DEBUG to enable traces
  • propose PR in Lib metal to allow to output traces to a ri,g buffer in parallel of add traces. we could have 2 levels of traces one for the treace on serial one for the trace in ring buffer

@CV-Bowen
Copy link
Contributor

CV-Bowen commented Oct 9, 2025

@arnopo @tnmysh Thank you for the update.

@github-actions
Copy link

This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity.

@github-actions github-actions bot added the Stale label Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants