Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpmsg/rpmsg_virtio: support set trace callback to trace buffer status #632

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

rpmsg/rpmsg_virtio: support set trace callback to trace buffer status #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmake/options.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,10 @@ if (DEFINED RPMSG_BUFFER_SIZE)
endif (DEFINED RPMSG_BUFFER_SIZE)

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 •
edited
Loading

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 •
edited
Loading

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

add_definitions(-DRPMSG_TRACE)
endif (WITH_RPMSG_TRACE)

message ("-- C_FLAGS : ${CMAKE_C_FLAGS}")
15 changes: 15 additions & 0 deletions lib/include/openamp/rpmsg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ struct rpmsg_device_ops {
int (*get_tx_buffer_size)(struct rpmsg_device *rdev);
};

/**
* struct rpmsg_device_trace - trace operation for debugging
*
* This structure is used by the rpmsg device to trace the various events.
*
* @get_tx_buffer: trace callback, called when get a tx buffer
* @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

void (*release_tx_buffer)(struct rpmsg_device *rvdev, void *hdr);
};

/** @brief Representation of a RPMsg device */
struct rpmsg_device {
/** List of endpoints */
Expand All @@ -153,6 +166,8 @@ struct rpmsg_device {
/** RPMsg device operations */
struct rpmsg_device_ops ops;

struct rpmsg_device_trace trace;

/** Create/destroy namespace message */
bool support_ns;
};
Expand Down
41 changes: 41 additions & 0 deletions lib/rpmsg/rpmsg_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,47 @@ void rpmsg_ept_incref(struct rpmsg_endpoint *ept);
*/
void rpmsg_ept_decref(struct rpmsg_endpoint *ept);

#ifdef RPMSG_TRACE
/**
* @internal
*
* @brief Trace the get tx buffer process
*
* This function is used to trace the get tx buffer process
*
* @param rdev pointer to rpmsg endpoint
* @param hdr pointer to rpmsg header
*
*/
static inline void
rpmsg_device_trace_get_tx_buffer(struct rpmsg_device *rdev, void *hdr)
{
if (rdev->trace.get_tx_buffer && hdr)
rdev->trace.get_tx_buffer(rdev, hdr);
}

/**
* @internal
*
* @brief Trace the release tx buffer process
*
* This function is used to trace the release tx buffer process
*
* @param rdev pointer to rpmsg endpoint
* @param hdr pointer to rpmsg header
*
*/
static inline void
rpmsg_device_trace_release_tx_buffer(struct rpmsg_device *rdev, void *hdr)
{
if (rdev->trace.release_tx_buffer && hdr)
rdev->trace.release_tx_buffer(rdev, hdr);
}
#else
#define rpmsg_device_trace_get_tx_buffer(rdev, hdr)
#define rpmsg_device_trace_release_tx_buffer(rdev, hdr)
#endif

#if defined __cplusplus
}
#endif
Expand Down
13 changes: 11 additions & 2 deletions lib/rpmsg/rpmsg_virtio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ static int rpmsg_virtio_enqueue_buffer(struct rpmsg_virtio_device *rvdev,
void *buffer, uint32_t len,
uint16_t idx)
{
int ret;
BUFFER_FLUSH(buffer, len);

if (VIRTIO_ROLE_IS_DRIVER(rvdev->vdev)) {
Expand All @@ -157,12 +158,17 @@ static int rpmsg_virtio_enqueue_buffer(struct rpmsg_virtio_device *rvdev,
/* Initialize buffer node */
vqbuf.buf = buffer;
vqbuf.len = len;
return virtqueue_add_buffer(rvdev->svq, &vqbuf, 1, 0, buffer);

ret = virtqueue_add_buffer(rvdev->svq, &vqbuf, 1, 0, buffer);
rpmsg_device_trace_release_tx_buffer(&rvdev->rdev, buffer);
return ret;
}

if (VIRTIO_ROLE_IS_DEVICE(rvdev->vdev)) {
(void)buffer;
return virtqueue_add_consumed_buffer(rvdev->svq, idx, len);
ret = virtqueue_add_consumed_buffer(rvdev->svq, idx, len);
rpmsg_device_trace_release_tx_buffer(&rvdev->rdev, buffer);
return ret;
}

return 0;
Expand Down Expand Up @@ -210,6 +216,8 @@ static void *rpmsg_virtio_get_tx_buffer(struct rpmsg_virtio_device *rvdev,
data = virtqueue_get_available_buffer(rvdev->svq, idx, len);
}

rpmsg_device_trace_get_tx_buffer(&rvdev->rdev, data);

return data;
}

Expand Down Expand Up @@ -482,6 +490,7 @@ static int rpmsg_virtio_release_tx_buffer(struct rpmsg_device *rdev, void *txbuf
*/
r_desc->idx = RPMSG_BUF_INDEX(rp_hdr);
metal_list_add_tail(&rvdev->reclaimer, &r_desc->node);
rpmsg_device_trace_release_tx_buffer(&rvdev->rdev, rp_hdr);
}

metal_mutex_release(&rdev->lock);
Expand Down
Loading