Besides the IOCTL interface the VCHIQ character device also provides
a state dump of the whole VCHIQ driver via read. Moving the state dump
function to debugfs has a lot advantages:
- following changes on state dump doesn't break userspace ABI
- debug doesn't depend on VCHIQ_CDEV
- dump code simplifies a lot and reduce the chance of buffer overflows
Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx>
---
.../interface/vchiq_arm/vchiq_arm.c | 94 ++------
.../interface/vchiq_arm/vchiq_arm.h | 7 -
.../interface/vchiq_arm/vchiq_core.c | 220 ++++++------------
.../interface/vchiq_arm/vchiq_core.h | 13 +-
.../interface/vchiq_arm/vchiq_debugfs.c | 10 +
.../interface/vchiq_arm/vchiq_dev.c | 21 --
6 files changed, 107 insertions(+), 258 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 9fb8f657cc78..7978fb6dc4fb 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -659,13 +659,9 @@ vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk)
bulk->actual);
}
-int vchiq_dump_platform_state(void *dump_context)
+void vchiq_dump_platform_state(struct seq_file *f)
{
- char buf[80];
- int len;
-
- len = snprintf(buf, sizeof(buf), " Platform: 2835 (VC master)");
- return vchiq_dump(dump_context, buf, len + 1);
+ seq_puts(f, " Platform: 2835 (VC master)\n");
}
#define VCHIQ_INIT_RETRIES 10
@@ -1190,56 +1186,13 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
bulk_userdata);
}
-int vchiq_dump(void *dump_context, const char *str, int len)
-{
- struct dump_context *context = (struct dump_context *)dump_context;
- int copy_bytes;
-
- if (context->actual >= context->space)
- return 0;
-
- if (context->offset > 0) {
- int skip_bytes = min_t(int, len, context->offset);
-
- str += skip_bytes;
- len -= skip_bytes;
- context->offset -= skip_bytes;
- if (context->offset > 0)
- return 0;
- }
- copy_bytes = min_t(int, len, context->space - context->actual);
- if (copy_bytes == 0)
- return 0;
- if (copy_to_user(context->buf + context->actual, str,
- copy_bytes))
- return -EFAULT;
- context->actual += copy_bytes;
- len -= copy_bytes;
-
- /*
- * If the terminating NUL is included in the length, then it
- * marks the end of a line and should be replaced with a
- * carriage return.
- */
- if ((len == 0) && (str[copy_bytes - 1] == '\0')) {
- char cr = '\n';
-
- if (copy_to_user(context->buf + context->actual - 1,
- &cr, 1))
- return -EFAULT;
- }
- return 0;
-}
-
-int vchiq_dump_platform_instances(void *dump_context)
+void vchiq_dump_platform_instances(struct seq_file *f)
{
struct vchiq_state *state = vchiq_get_state();
- char buf[80];
- int len;
int i;
if (!state)
- return -ENOTCONN;
+ return;
/*
* There is no list of instances, so instead scan all services,
@@ -1264,7 +1217,6 @@ int vchiq_dump_platform_instances(void *dump_context)
for (i = 0; i < state->unused_service; i++) {
struct vchiq_service *service;
struct vchiq_instance *instance;
- int err;
rcu_read_lock();
service = rcu_dereference(state->services[i]);
@@ -1280,43 +1232,35 @@ int vchiq_dump_platform_instances(void *dump_context)
}
rcu_read_unlock();
- len = snprintf(buf, sizeof(buf),
- "Instance %pK: pid %d,%s completions %d/%d",
- instance, instance->pid,
- instance->connected ? " connected, " :
- "",
- instance->completion_insert -
- instance->completion_remove,
- MAX_COMPLETIONS);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+ seq_printf(f, "Instance %pK: pid %d,%s completions %d/%d\n",
+ instance, instance->pid,
+ instance->connected ? " connected, " :
+ "",
+ instance->completion_insert -
+ instance->completion_remove,
+ MAX_COMPLETIONS);
instance->mark = 1;
}
- return 0;
}
-int vchiq_dump_platform_service_state(void *dump_context,
- struct vchiq_service *service)
+void vchiq_dump_platform_service_state(struct seq_file *f,
+ struct vchiq_service *service)
{
struct user_service *user_service =
(struct user_service *)service->base.userdata;
- char buf[80];
- int len;
- len = scnprintf(buf, sizeof(buf), " instance %pK", service->instance);
+ seq_printf(f, " instance %pK", service->instance);
if ((service->base.callback == service_callback) && user_service->is_vchi) {
- len += scnprintf(buf + len, sizeof(buf) - len, ", %d/%d messages",
- user_service->msg_insert - user_service->msg_remove,
- MSG_QUEUE_SIZE);
+ seq_printf(f, ", %d/%d messages",
+ user_service->msg_insert - user_service->msg_remove,
+ MSG_QUEUE_SIZE);
if (user_service->dequeue_pending)
- len += scnprintf(buf + len, sizeof(buf) - len,
- " (dequeue pending)");
+ seq_puts(f, " (dequeue pending)");
}
- return vchiq_dump(dump_context, buf, len + 1);
+ seq_puts(f, "\n");
}
struct vchiq_state *
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
index 7cdc3d70bd2c..7844ef765a00 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -69,13 +69,6 @@ struct vchiq_instance {
struct vchiq_debugfs_node debugfs_node;
};
-struct dump_context {
- char __user *buf;
- size_t actual;
- size_t space;
- loff_t offset;
-};
-
extern spinlock_t msg_queue_spinlock;
extern struct vchiq_state g_state;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 36c742a2f3b2..6ca83f79622e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3372,8 +3372,8 @@ vchiq_set_service_option(struct vchiq_instance *instance, unsigned int handle,
return ret;
}
-static int
-vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
+static void
+vchiq_dump_shared_state(struct seq_file *f, struct vchiq_state *state,
struct vchiq_shared_state *shared, const char *label)
{
static const char *const debug_names[] = {
@@ -3390,57 +3390,37 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
"COMPLETION_QUEUE_FULL_COUNT"
};
int i;
- char buf[80];
- int len;
- int err;
-
- len = scnprintf(buf, sizeof(buf), " %s: slots %d-%d tx_pos=%x recycle=%x",
- label, shared->slot_first, shared->slot_last,
- shared->tx_pos, shared->slot_queue_recycle);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
-
- len = scnprintf(buf, sizeof(buf), " Slots claimed:");
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+
+ seq_printf(f, " %s: slots %d-%d tx_pos=%x recycle=%x\n",
+ label, shared->slot_first, shared->slot_last,
+ shared->tx_pos, shared->slot_queue_recycle);
+
+ seq_puts(f, " Slots claimed:\n");
for (i = shared->slot_first; i <= shared->slot_last; i++) {
struct vchiq_slot_info slot_info =
*SLOT_INFO_FROM_INDEX(state, i);
if (slot_info.use_count != slot_info.release_count) {
- len = scnprintf(buf, sizeof(buf), " %d: %d/%d", i, slot_info.use_count,
- slot_info.release_count);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+ seq_printf(f, " %d: %d/%d\n", i, slot_info.use_count,
+ slot_info.release_count);
}
}
for (i = 1; i < shared->debug[DEBUG_ENTRIES]; i++) {
- len = scnprintf(buf, sizeof(buf), " DEBUG: %s = %d(%x)",
- debug_names[i], shared->debug[i], shared->debug[i]);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+ seq_printf(f, " DEBUG: %s = %d(%x)\n",
+ debug_names[i], shared->debug[i], shared->debug[i]);
}
- return 0;
}
-static int
-vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
+static void
+vchiq_dump_service_state(struct seq_file *f, struct vchiq_service *service)
{
- char buf[80];
- int len;
- int err;
unsigned int ref_count;
/*Don't include the lock just taken*/
ref_count = kref_read(&service->ref_count) - 1;
- len = scnprintf(buf, sizeof(buf), "Service %u: %s (ref %u)",
- service->localport, srvstate_names[service->srvstate],
- ref_count);
+ seq_printf(f, "Service %u: %s (ref %u)", service->localport,
+ srvstate_names[service->srvstate], ref_count);
if (service->srvstate != VCHIQ_SRVSTATE_FREE) {
char remoteport[30];
@@ -3460,15 +3440,10 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
strscpy(remoteport, "n/a", sizeof(remoteport));
}
- len += scnprintf(buf + len, sizeof(buf) - len,
- " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)",
- &fourcc, remoteport,
- quota->message_use_count, quota->message_quota,
- quota->slot_use_count, quota->slot_quota);
-
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+ seq_printf(f, " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)\n",
+ &fourcc, remoteport,
+ quota->message_use_count, quota->message_quota,
+ quota->slot_use_count, quota->slot_quota);
tx_pending = service->bulk_tx.local_insert -
service->bulk_tx.remote_insert;
@@ -3486,130 +3461,79 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
rx_size = service->bulk_rx.bulks[i].size;
}
- len = scnprintf(buf, sizeof(buf),
- " Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
- tx_pending, tx_size, rx_pending, rx_size);
+ seq_printf(f, " Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)\n",
+ tx_pending, tx_size, rx_pending, rx_size);
if (VCHIQ_ENABLE_STATS) {
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+ seq_printf(f, " Ctrl: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu\n",
+ service->stats.ctrl_tx_count,
+ service->stats.ctrl_tx_bytes,
+ service->stats.ctrl_rx_count,
+ service->stats.ctrl_rx_bytes);
- len = scnprintf(buf, sizeof(buf),
- " Ctrl: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu",
- service->stats.ctrl_tx_count, service->stats.ctrl_tx_bytes,
- service->stats.ctrl_rx_count, service->stats.ctrl_rx_bytes);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+ seq_printf(f, " Bulk: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu\n",
+ service->stats.bulk_tx_count,
+ service->stats.bulk_tx_bytes,
+ service->stats.bulk_rx_count,
+ service->stats.bulk_rx_bytes);
- len = scnprintf(buf, sizeof(buf),
- " Bulk: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu",
- service->stats.bulk_tx_count, service->stats.bulk_tx_bytes,
- service->stats.bulk_rx_count, service->stats.bulk_rx_bytes);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
-
- len = scnprintf(buf, sizeof(buf),
- " %d quota stalls, %d slot stalls, %d bulk stalls, %d aborted, %d errors",
- service->stats.quota_stalls, service->stats.slot_stalls,
- service->stats.bulk_stalls,
- service->stats.bulk_aborted_count,
- service->stats.error_count);
+ seq_printf(f, " %d quota stalls, %d slot stalls, %d bulk stalls, %d aborted, %d errors\n",
+ service->stats.quota_stalls,
+ service->stats.slot_stalls,
+ service->stats.bulk_stalls,
+ service->stats.bulk_aborted_count,
+ service->stats.error_count);
}
}
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
-
- if (service->srvstate != VCHIQ_SRVSTATE_FREE)
- err = vchiq_dump_platform_service_state(dump_context, service);
- return err;
+ vchiq_dump_platform_service_state(f, service);
}
-int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
+void vchiq_dump_state(struct seq_file *f, struct vchiq_state *state)
{
- char buf[80];
- int len;
int i;
- int err;
-
- len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
- conn_state_names[state->conn_state]);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
-
- len = scnprintf(buf, sizeof(buf), " tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
- state->local->tx_pos,
- state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
- state->rx_pos,
- state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
-
- len = scnprintf(buf, sizeof(buf), " Version: %d (min %d)",
- VCHIQ_VERSION, VCHIQ_VERSION_MIN);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
+
+ seq_printf(f, "State %d: %s\n", state->id,
+ conn_state_names[state->conn_state]);
+
+ seq_printf(f, " tx_pos=%x(@%pK), rx_pos=%x(@%pK)\n",
+ state->local->tx_pos,
+ state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
+ state->rx_pos,
+ state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
+
+ seq_printf(f, " Version: %d (min %d)\n", VCHIQ_VERSION,
+ VCHIQ_VERSION_MIN);
if (VCHIQ_ENABLE_STATS) {
- len = scnprintf(buf, sizeof(buf),
- " Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
- state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
- state->stats.error_count);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
- }
-
- len = scnprintf(buf, sizeof(buf),
- " Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
- ((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
- state->local_tx_pos) / VCHIQ_SLOT_SIZE,
- state->data_quota - state->data_use_count,
- state->local->slot_queue_recycle - state->slot_queue_available,
- state->stats.slot_stalls, state->stats.data_stalls);
- err = vchiq_dump(dump_context, buf, len + 1);
- if (err)
- return err;
-
- err = vchiq_dump_platform_state(dump_context);
- if (err)
- return err;
-
- err = vchiq_dump_shared_state(dump_context,
- state,
- state->local,
- "Local");
- if (err)
- return err;
- err = vchiq_dump_shared_state(dump_context,
- state,
- state->remote,
- "Remote");
- if (err)
- return err;
-
- err = vchiq_dump_platform_instances(dump_context);
- if (err)
- return err;
+ seq_printf(f, " Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d\n",
+ state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
+ state->stats.error_count);
+ }
+
+ seq_printf(f, " Slots: %d available (%d data), %d recyclable, %d stalls (%d data)\n",
+ ((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
+ state->local_tx_pos) / VCHIQ_SLOT_SIZE,
+ state->data_quota - state->data_use_count,
+ state->local->slot_queue_recycle - state->slot_queue_available,
+ state->stats.slot_stalls, state->stats.data_stalls);
+
+ vchiq_dump_platform_state(f);
+
+ vchiq_dump_shared_state(f, state, state->local, "Local");
+
+ vchiq_dump_shared_state(f, state, state->remote, "Remote");
+
+ vchiq_dump_platform_instances(f);
for (i = 0; i < state->unused_service; i++) {
struct vchiq_service *service = find_service_by_port(state, i);
if (service) {
- err = vchiq_dump_service_state(dump_context, service);
+ vchiq_dump_service_state(f, service);
vchiq_service_put(service);
- if (err)
- return err;
}
}
- return 0;
}
int vchiq_send_remote_use(struct vchiq_state *state)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index ea8d58844775..564b5c707267 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -6,6 +6,7 @@
#include <linux/mutex.h>
#include <linux/completion.h>
+#include <linux/debugfs.h>
#include <linux/dev_printk.h>
#include <linux/kthread.h>
#include <linux/kref.h>
@@ -504,8 +505,8 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
void __user *uoffset, int size, void *userdata, enum vchiq_bulk_mode mode,
enum vchiq_bulk_dir dir);
-extern int
-vchiq_dump_state(void *dump_context, struct vchiq_state *state);
+extern void
+vchiq_dump_state(struct seq_file *f, struct vchiq_state *state);
extern void
vchiq_loud_error_header(void);
@@ -561,13 +562,11 @@ void vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bul
void remote_event_signal(struct remote_event *event);
-int vchiq_dump(void *dump_context, const char *str, int len);
-
-int vchiq_dump_platform_state(void *dump_context);
+void vchiq_dump_platform_state(struct seq_file *f);
-int vchiq_dump_platform_instances(void *dump_context);
+void vchiq_dump_platform_instances(struct seq_file *f);
-int vchiq_dump_platform_service_state(void *dump_context, struct vchiq_service *service);
+void vchiq_dump_platform_service_state(struct seq_file *f, struct vchiq_service *service);
int vchiq_use_service_internal(struct vchiq_service *service);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
index 58db78a9c8d4..d833e4e2973a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
@@ -40,6 +40,13 @@ static int debugfs_trace_show(struct seq_file *f, void *offset)
return 0;
}
+static int vchiq_dump_show(struct seq_file *f, void *offset)
+{
+ vchiq_dump_state(f, &g_state);
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(vchiq_dump);
+
static int debugfs_trace_open(struct inode *inode, struct file *file)
{
return single_open(file, debugfs_trace_show, inode->i_private);
@@ -115,6 +122,9 @@ void vchiq_debugfs_init(void)
{
vchiq_dbg_dir = debugfs_create_dir("vchiq", NULL);
vchiq_dbg_clients = debugfs_create_dir("clients", vchiq_dbg_dir);
+
+ debugfs_create_file("state", S_IFREG | 0444, vchiq_dbg_dir, NULL,
+ &vchiq_dump_fops);
}
/* remove all the debugfs entries */
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index 0bc93f48c14c..307a2d76cbb7 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -1306,26 +1306,6 @@ static int vchiq_release(struct inode *inode, struct file *file)
return ret;
}
-static ssize_t
-vchiq_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
-{
- struct dump_context context;
- int err;
-
- context.buf = buf;
- context.actual = 0;
- context.space = count;
- context.offset = *ppos;
-
- err = vchiq_dump_state(&context, &g_state);
- if (err)
- return err;
-
- *ppos += context.actual;
-
- return context.actual;
-}
-
static const struct file_operations
vchiq_fops = {
.owner = THIS_MODULE,
@@ -1335,7 +1315,6 @@ vchiq_fops = {
#endif
.open = vchiq_open,
.release = vchiq_release,
- .read = vchiq_read
};
static struct miscdevice vchiq_miscdev = {
--
2.34.1
