staging: vchiq_arm: move state dump to debugfs
authorStefan Wahren <wahrenst@gmx.net>
Sun, 29 Oct 2023 12:48:37 +0000 (13:48 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 23 Nov 2023 13:12:15 +0000 (13:12 +0000)
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@gmx.net>
Tested-by: "Ricardo B. Marliere" <ricardo@marliere.net>
Link: https://lore.kernel.org/r/20231029124837.119832-4-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c

index 9fb8f657cc781ad41454dc67ef2608d6505db9f7..7978fb6dc4fbd20ac5dd2ffe77b71826746aa7fd 100644 (file)
@@ -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 *
index 7cdc3d70bd2c121078f82bcf047b1dd1656ce591..7844ef765a00e4628814d8c9c88f2f4b9ae8a0f8 100644 (file)
@@ -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;
 
index 75d82a7b510904336ff102d42fe30b8c6be12ef3..0468689a6325b24ec2687de77cdc287be3f18afe 100644 (file)
@@ -3371,8 +3371,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[] = {
@@ -3389,57 +3389,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];
@@ -3459,15 +3439,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;
@@ -3485,130 +3460,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)
index ea8d588447756a7fe6ff20e65c4ef34468bfb1f1..564b5c7072672702ff8ae1abf2cb6e4ce0079af1 100644 (file)
@@ -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);
 
index 58db78a9c8d4c979d5abbae28e2c55ec321a783f..d833e4e2973a7f0b1c276136ddbcaf9dd7f98a85 100644 (file)
@@ -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 */
index 0bc93f48c14cba8ab6a8c7322bb05339cf678fc2..307a2d76cbb74dfc5ee8e93d2687c61185b45b9e 100644 (file)
@@ -1306,26 +1306,6 @@ out:
        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 = {