[PATCH] usb: gadget: functions: add ftrace export over USB

[PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

Allow for ftrace data to be exported over a USB Gadget
Controller. With this, we have a potentially very fast pipe for
transmitting ftrace data to a Host PC for further analysis.

Note that in order to decode the data, one needs access to kernel
symbols in order to convert binary data into function names and what
not.

Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---

I wanted to take this through the gadget tree, but there is a
dependency with a previous patch of mine adding and extra argument to
the ->write() function. Hoping someone else will take it.

 drivers/usb/gadget/Kconfig            |  15 ++
 drivers/usb/gadget/function/Makefile  |   2 +
 drivers/usb/gadget/function/f-trace.c | 400 ++++++++++++++++++++++++++++++++++
 3 files changed, 417 insertions(+)
 create mode 100644 drivers/usb/gadget/function/f-trace.c

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index c164d6b788c3..617921f19b5e 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -188,6 +188,9 @@ config USB_F_MASS_STORAGE
 config USB_F_FS
 	tristate
 
+config USB_F_TRACE
+	tristate
+
 config USB_F_UAC1
 	tristate
 
@@ -362,6 +365,18 @@ config USB_CONFIGFS_F_FS
 	  implemented in kernel space (for instance Ethernet, serial or
 	  mass storage) and other are implemented in user space.
 
+config USB_CONFIGFS_F_TRACE
+	bool "Linux FTrace Export Over USB"
+	depends on USB_CONFIGFS
+	select USB_F_TRACE
+	help
+	  The Linux FTrace Export Over USB lets one export ftrace buffer
+	  over a USB cable to a host computer for further processing.
+
+	  If you want support for that, say Y or M here. Otherwise say N.
+
+	  If unsure, say N.
+
 config USB_CONFIGFS_F_UAC1
 	bool "Audio Class 1.0"
 	depends on USB_CONFIGFS
diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile
index cb8c225e8549..1433e8ad7675 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -46,3 +46,5 @@ usb_f_printer-y			:= f_printer.o
 obj-$(CONFIG_USB_F_PRINTER)	+= usb_f_printer.o
 usb_f_tcm-y			:= f_tcm.o
 obj-$(CONFIG_USB_F_TCM)		+= usb_f_tcm.o
+usb_f_trace-y			:= f-trace.o
+obj-$(CONFIG_USB_F_TRACE)	+= usb_f_trace.o
diff --git a/drivers/usb/gadget/function/f-trace.c b/drivers/usb/gadget/function/f-trace.c
new file mode 100644
index 000000000000..7de92950c0e7
--- /dev/null
+++ b/drivers/usb/gadget/function/f-trace.c
@@ -0,0 +1,400 @@
+/*
+ * f_trace.c -- USB FTrace Export
+ *
+ * Copyright (C) 2017 Intel Corporation
+ * Author: Felipe Balbi <felipe.balbi@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License v2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/trace.h>
+#include <linux/usb.h>
+#include <linux/usb/composite.h>
+#include <linux/usb/gadget.h>
+#include <linux/workqueue.h>
+
+struct usb_ftrace {
+	struct trace_export ftrace;
+	struct usb_function function;
+	struct work_struct queue_work;
+	spinlock_t lock;
+
+	struct list_head list;
+	struct list_head pending;
+	struct list_head queued;
+
+	struct usb_ep *in;
+
+	u8 intf_id;
+};
+#define ftrace_to_trace(f)	(container_of((f), struct usb_ftrace, ftrace))
+#define work_to_trace(w)	(container_of((w), struct usb_ftrace, queue_work))
+#define to_trace(f)		(container_of((f), struct usb_ftrace, function))
+
+#define FTRACE_REQUEST_QUEUE_LENGTH	250
+
+static inline struct usb_request *next_request(struct list_head *list)
+{
+	return list_first_entry_or_null(list, struct usb_request, list);
+}
+
+struct usb_ftrace_opts {
+	struct usb_function_instance func_inst;
+};
+#define to_opts(fi)	(container_of((fi), struct usb_ftrace_opts, func_inst))
+
+static struct usb_interface_descriptor ftrace_intf_desc = {
+	.bLength		= USB_DT_INTERFACE_SIZE,
+	.bDescriptorType	= USB_DT_INTERFACE,
+
+	.bAlternateSetting	= 0,
+	.bNumEndpoints		= 1,
+	.bInterfaceClass	= USB_CLASS_VENDOR_SPEC,
+	.bInterfaceSubClass	= USB_SUBCLASS_VENDOR_SPEC,
+};
+
+/* Super-Speed Support */
+static struct usb_endpoint_descriptor ftrace_ss_in_desc = {
+	.bLength		= USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType	= USB_DT_ENDPOINT,
+
+	.bEndpointAddress	= USB_DIR_IN,
+	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize		= cpu_to_le16(1024),
+};
+
+static struct usb_ss_ep_comp_descriptor ftrace_ss_in_comp_desc = {
+	.bLength		= USB_DT_SS_EP_COMP_SIZE,
+	.bDescriptorType	= USB_DT_SS_ENDPOINT_COMP,
+
+	.bMaxBurst		= 15,
+};
+
+static struct usb_descriptor_header *ftrace_ss_function[] = {
+	(struct usb_descriptor_header *) &ftrace_intf_desc,
+	(struct usb_descriptor_header *) &ftrace_ss_in_desc,
+	(struct usb_descriptor_header *) &ftrace_ss_in_comp_desc,
+	NULL,
+};
+
+/* High-Speed Support */
+static struct usb_endpoint_descriptor ftrace_hs_in_desc = {
+	.bLength		= USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType	= USB_DT_ENDPOINT,
+
+	.bEndpointAddress	= USB_DIR_IN,
+	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize		= cpu_to_le16(512),
+};
+
+static struct usb_descriptor_header *ftrace_hs_function[] = {
+	(struct usb_descriptor_header *) &ftrace_intf_desc,
+	(struct usb_descriptor_header *) &ftrace_hs_in_desc,
+	NULL,
+};
+
+/* Full-Speed Support */
+static struct usb_endpoint_descriptor ftrace_fs_in_desc = {
+	.bLength		= USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType	= USB_DT_ENDPOINT,
+
+	.bEndpointAddress	= USB_DIR_IN,
+	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize		= cpu_to_le16(64),
+};
+
+static struct usb_descriptor_header *ftrace_fs_function[] = {
+	(struct usb_descriptor_header *) &ftrace_intf_desc,
+	(struct usb_descriptor_header *) &ftrace_fs_in_desc,
+	NULL,
+};
+
+static struct usb_string ftrace_string_defs[] = {
+	[0].s = "Linux Ftrace Export",
+	{ },
+};
+
+static struct usb_gadget_strings ftrace_string_table = {
+	.language		= 0x0409, /* en-US */
+	.strings		= ftrace_string_defs,
+};
+
+static struct usb_gadget_strings *ftrace_strings[] = {
+	&ftrace_string_table,
+	NULL,
+};
+
+/* ------------------------------------------------------------------------ */
+
+static void ftrace_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct usb_ftrace		*trace = req->context;
+
+	kfree(req->buf);
+	list_move_tail(&req->list, &trace->list);
+}
+
+static void ftrace_queue_work(struct work_struct *work)
+{
+	struct usb_ftrace		*trace = work_to_trace(work);
+	struct usb_request		*req;
+	struct usb_request		*tmp;
+	struct list_head		local_list;
+
+	spin_lock_irq(&trace->lock);
+restart:
+	list_replace_init(&trace->pending, &local_list);
+	spin_unlock_irq(&trace->lock);
+
+	list_for_each_entry_safe(req, tmp, &local_list, list) {
+		int			ret;
+
+		ret = usb_ep_queue(trace->in, req, GFP_KERNEL);
+		if (!ret)
+			list_move_tail(&req->list, &trace->queued);
+	}
+
+	spin_lock_irq(&trace->lock);
+	if (!list_empty(&trace->pending))
+		goto restart;
+	spin_unlock_irq(&trace->lock);
+}
+
+static void notrace ftrace_write(struct trace_export *ftrace, const void *buf,
+				 unsigned int len)
+{
+	struct usb_ftrace		*trace = ftrace_to_trace(ftrace);
+	struct usb_request		*req = next_request(&trace->list);
+
+	if (!req)
+		return;
+
+	if (!trace->in->enabled)
+		return;
+
+	req->buf = kmemdup(buf, len, GFP_ATOMIC);
+	req->length = len;
+	req->context = trace;
+	req->complete = ftrace_complete;
+	list_move_tail(&req->list, &trace->pending);
+
+	schedule_work(&trace->queue_work);
+}
+
+/* ------------------------------------------------------------------------ */
+
+static void ftrace_disable_endpoint(struct usb_ftrace *trace)
+{
+	if (trace->in->enabled)
+		WARN_ON(usb_ep_disable(trace->in));
+}
+
+static int ftrace_enable_endpoint(struct usb_ftrace *trace)
+{
+	if (trace->in->enabled)
+		return 0;
+
+	return usb_ep_enable(trace->in);
+}
+
+static int ftrace_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
+{
+	struct usb_ftrace		*trace = to_trace(f);
+	struct usb_composite_dev	*cdev = f->config->cdev;
+	int				ret;
+
+	if (alt != 0)
+		goto fail;
+
+	if (intf != trace->intf_id)
+		goto fail;
+
+	ftrace_disable_endpoint(trace);
+
+	if (!trace->in->desc) {
+		ret = config_ep_by_speed(cdev->gadget, f, trace->in);
+		if (ret) {
+			trace->in->desc = NULL;
+			goto fail;
+		}
+	}
+
+	ret = ftrace_enable_endpoint(trace);
+	if (ret)
+		goto fail;
+
+	return 0;
+
+fail:
+	return -EINVAL;
+}
+
+static int ftrace_bind(struct usb_configuration *c, struct usb_function *f)
+{
+	struct usb_composite_dev	*cdev = c->cdev;
+	struct usb_ftrace		*trace = to_trace(f);
+	struct usb_string		*us;
+	struct usb_ep			*ep;
+
+	int				ret;
+	int				i;
+
+	us = usb_gstrings_attach(cdev, ftrace_strings,
+				 ARRAY_SIZE(ftrace_string_defs));
+	if (IS_ERR(us))
+		return PTR_ERR(us);
+
+	ftrace_intf_desc.iInterface = us[0].id;
+
+	ret = usb_interface_id(c, f);
+	if (ret < 0)
+		goto err0;
+	trace->intf_id = ret;
+	ftrace_intf_desc.bInterfaceNumber = ret;
+
+	ep = usb_ep_autoconfig(cdev->gadget, &ftrace_fs_in_desc);
+	if (!ep)
+		goto err0;
+	trace->in = ep;
+
+	ftrace_hs_in_desc.bEndpointAddress = ftrace_fs_in_desc.bEndpointAddress;
+	ftrace_ss_in_desc.bEndpointAddress = ftrace_fs_in_desc.bEndpointAddress;
+
+	trace->ftrace.write = ftrace_write;
+
+	spin_lock_init(&trace->lock);
+	INIT_WORK(&trace->queue_work, ftrace_queue_work);
+	INIT_LIST_HEAD(&trace->list);
+	INIT_LIST_HEAD(&trace->pending);
+	INIT_LIST_HEAD(&trace->queued);
+
+	ret = usb_assign_descriptors(f, ftrace_fs_function, ftrace_hs_function,
+				     ftrace_ss_function, NULL);
+	if (ret)
+		goto err0;
+
+	for (i = 0; i < FTRACE_REQUEST_QUEUE_LENGTH; i++) {
+		struct usb_request *req;
+
+		req = usb_ep_alloc_request(trace->in, GFP_KERNEL);
+		if (!req)
+			goto err1;
+
+		list_add_tail(&req->list, &trace->list);
+	}
+
+	ret = register_ftrace_export(&trace->ftrace);
+	if (ret)
+		goto err1;
+
+	return 0;
+
+err1:
+	while (!list_empty(&trace->list)) {
+		struct usb_request *req = next_request(&trace->list);
+
+		usb_ep_free_request(trace->in, req);
+		list_del(&req->list);
+	}
+
+	usb_free_all_descriptors(f);
+
+err0:
+	ERROR(cdev, "%s: can't bind --> err %d\n", f->name, ret);
+
+	return ret;
+}
+
+static void ftrace_unbind(struct usb_configuration *c, struct usb_function *f)
+{
+	struct usb_ftrace		*trace = to_trace(f);
+	struct usb_request		*req;
+	struct usb_request		*tmp;
+
+	unregister_ftrace_export(&trace->ftrace);
+	cancel_work_sync(&trace->queue_work);
+	usb_free_all_descriptors(f);
+
+	list_for_each_entry(req, &trace->queued, list)
+		usb_ep_dequeue(trace->in, req);
+
+	list_for_each_entry_safe(req, tmp, &trace->pending, list) {
+		usb_ep_free_request(trace->in, req);
+		list_del(&req->list);
+	}
+
+	list_for_each_entry_safe(req, tmp, &trace->list, list) {
+		usb_ep_free_request(trace->in, req);
+		list_del(&req->list);
+	}
+}
+
+static void ftrace_disable(struct usb_function *f)
+{
+	struct usb_ftrace		*trace = to_trace(f);
+
+	ftrace_disable_endpoint(trace);
+}
+
+static void ftrace_free_func(struct usb_function *f)
+{
+	kfree(to_trace(f));
+}
+
+static struct config_item_type ftrace_func_type = {
+	.ct_owner		= THIS_MODULE,
+};
+
+static void ftrace_free_inst(struct usb_function_instance *fi)
+{
+	struct usb_ftrace_opts		*opts = to_opts(fi);
+
+	kfree(opts);
+}
+
+static struct usb_function_instance *ftrace_alloc_inst(void)
+{
+	struct usb_ftrace_opts		*opts;
+
+	opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+	if (!opts)
+		return ERR_PTR(-ENOMEM);
+
+	opts->func_inst.free_func_inst = ftrace_free_inst;
+
+	config_group_init_type_name(&opts->func_inst.group, "",
+				    &ftrace_func_type);
+
+	return &opts->func_inst;
+}
+
+static struct usb_function *ftrace_alloc(struct usb_function_instance *fi)
+{
+	struct usb_ftrace		*trace;
+
+	trace = kzalloc(sizeof(*trace), GFP_KERNEL);
+	if (!trace)
+		return NULL;
+
+	trace->function.name = "ftrace";
+	trace->function.bind = ftrace_bind;
+	trace->function.unbind = ftrace_unbind;
+	trace->function.set_alt = ftrace_set_alt;
+	trace->function.disable = ftrace_disable;
+	trace->function.strings = ftrace_strings;
+	trace->function.free_func = ftrace_free_func;
+
+	return &trace->function;
+}
+
+DECLARE_USB_FUNCTION_INIT(ftrace, ftrace_alloc_inst, ftrace_alloc);
+MODULE_AUTHOR("Felipe Balbi <felipe.balbi@linux.intel.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0.295.gd7dffce1ce

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

Felipe Balbi <felipe.balbi@linux.intel.com> writes:

> Allow for ftrace data to be exported over a USB Gadget
> Controller. With this, we have a potentially very fast pipe for
> transmitting ftrace data to a Host PC for further analysis.
>
> Note that in order to decode the data, one needs access to kernel
> symbols in order to convert binary data into function names and what
> not.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---
>
> I wanted to take this through the gadget tree, but there is a
> dependency with a previous patch of mine adding and extra argument to
> the ->write() function. Hoping someone else will take it.

just as an extra note here. In order for this to be really useful, it
would be nice to be able to control what is going to be traced over USB
as well, but that means exporting a few extra functions to GPL drivers.

Would that be okay? I could have a set of vendor-specific control
requests to set buffer size and to read/write ftrace filter functions.

The idea is that things like e.g. Android SDK could rely on this on
debug builds and the SDK itself would make sure to keep a copy of
vmlinux around to processing of the data coming through USB.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 5227 bytes --]

Felipe Balbi <felipe.balbi@linux.intel.com> writes:

> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
>
>> Allow for ftrace data to be exported over a USB Gadget
>> Controller. With this, we have a potentially very fast pipe for
>> transmitting ftrace data to a Host PC for further analysis.
>>
>> Note that in order to decode the data, one needs access to kernel
>> symbols in order to convert binary data into function names and what
>> not.
>>
>> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>> ---
>>
>> I wanted to take this through the gadget tree, but there is a
>> dependency with a previous patch of mine adding and extra argument to
>> the ->write() function. Hoping someone else will take it.
>
> just as an extra note here. In order for this to be really useful, it
> would be nice to be able to control what is going to be traced over USB
> as well, but that means exporting a few extra functions to GPL drivers.
>
> Would that be okay? I could have a set of vendor-specific control
> requests to set buffer size and to read/write ftrace filter functions.
>
> The idea is that things like e.g. Android SDK could rely on this on
> debug builds and the SDK itself would make sure to keep a copy of
> vmlinux around to processing of the data coming through USB.

something along these lines (although I think trace buffer size doesn't
matter for trace export, but it serves well enough to illustrate a
point):

modified   drivers/usb/gadget/function/f-trace.c
@@ -33,6 +33,8 @@ struct usb_ftrace {
 
 	struct usb_ep *in;
 
+	u32 buffer_size;
+	u16 version;
 	u8 intf_id;
 };
 #define ftrace_to_trace(f)	(container_of((f), struct usb_ftrace, ftrace))
@@ -40,6 +42,12 @@ struct usb_ftrace {
 #define to_trace(f)		(container_of((f), struct usb_ftrace, function))
 
 #define FTRACE_REQUEST_QUEUE_LENGTH	250
+#define FTRACE_VERSION			0x0100 /* bcd 1.00 */
+
+/* FTrace vendor-specific requests */
+#define USB_FTRACE_GET_VERSION		0x00
+#define USB_FTRACE_GET_TRACE_BUF_SIZE	0x01
+#define USB_FTRACE_SET_TRACE_BUF_SIZE	0x02
 
 static inline struct usb_request *next_request(struct list_head *list)
 {
@@ -142,6 +150,13 @@ static void ftrace_complete(struct usb_ep *ep, struct usb_request *req)
 	list_move_tail(&req->list, &trace->list);
 }
 
+static void ftrace_set_trace_buf_size_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct usb_ftrace		*trace = req->context;
+
+	trace_set_buf_size(le32_to_cpu(trace->buffer_size));
+}
+
 static void ftrace_queue_work(struct work_struct *work)
 {
 	struct usb_ftrace		*trace = work_to_trace(work);
@@ -237,6 +252,71 @@ static int ftrace_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	return -EINVAL;
 }
 
+extern unsigned long trace_get_buf_size(void);
+
+static int ftrace_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
+{
+	struct usb_configuration	*c = f->config;
+	struct usb_request		*req = c->cdev->req;
+	struct usb_ftrace		*trace = to_trace(f);
+
+	int				ret;
+
+	u16				index = le16_to_cpu(ctrl->wIndex);
+	u16				value = le16_to_cpu(ctrl->wValue);
+	u16				length = le16_to_cpu(ctrl->wLength);
+
+	if (value != 0 || index != 0)
+		return -EINVAL;
+
+	switch (ctrl->bRequest) {
+	case USB_FTRACE_GET_VERSION:
+		if (ctrl->bRequestType != (USB_DIR_IN | USB_TYPE_VENDOR |
+					   USB_RECIP_INTERFACE))
+			return -EINVAL;
+
+		if (length != 2)
+			return -EINVAL;
+
+		req->zero = 0;
+		req->length = 2;
+		req->buf = &trace->version;
+		break;
+	case USB_FTRACE_GET_TRACE_BUF_SIZE:
+		if (ctrl->bRequestType != (USB_DIR_IN | USB_TYPE_VENDOR |
+					   USB_RECIP_INTERFACE))
+			return -EINVAL;
+
+		if (length != 2)
+			return -EINVAL;
+
+		trace->buffer_size = cpu_to_le32(trace_get_buf_size());
+
+		req->zero = 0;
+		req->length = 2;
+		req->buf = &trace->buffer_size;
+		break;
+	case USB_FTRACE_SET_TRACE_BUF_SIZE:
+		if (ctrl->bRequestType != (USB_DIR_OUT | USB_TYPE_VENDOR |
+					   USB_RECIP_INTERFACE))
+			return -EINVAL;
+
+		if (length != 4)
+			return -EINVAL;
+
+		req->zero = 0;
+		req->length = 4;
+		req->context = trace;
+		req->complete = ftrace_set_trace_buf_size_complete;
+		req->buf = &trace->buffer_size;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return usb_ep_queue(c->cdev->gadget->ep0, req, GFP_ATOMIC);
+}
+
 static int ftrace_bind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct usb_composite_dev	*cdev = c->cdev;
@@ -247,6 +327,8 @@ static int ftrace_bind(struct usb_configuration *c, struct usb_function *f)
 	int				ret;
 	int				i;
 
+	trace->version = cpu_to_le16(FTRACE_VERSION);
+
 	us = usb_gstrings_attach(cdev, ftrace_strings,
 				 ARRAY_SIZE(ftrace_string_defs));
 	if (IS_ERR(us))
modified   kernel/trace/trace.c
@@ -618,6 +618,12 @@ int tracing_is_enabled(void)
 
 static unsigned long		trace_buf_size = TRACE_BUF_SIZE_DEFAULT;
 
+unsigned long trace_get_buf_size(void)
+{
+	return trace_buf_size;
+}
+EXPORT_SYMBOL_GPL(trace_get_buf_size);
+
 /* trace_types holds a link list of available tracers. */
 static struct tracer		*trace_types __read_mostly;
 

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Steven Rostedt]

On Fri,  9 Jun 2017 09:13:27 +0300
Felipe Balbi <felipe.balbi@linux.intel.com> wrote:

> Allow for ftrace data to be exported over a USB Gadget
> Controller. With this, we have a potentially very fast pipe for
> transmitting ftrace data to a Host PC for further analysis.
> 
> Note that in order to decode the data, one needs access to kernel
> symbols in order to convert binary data into function names and what
> not.
> 

Can you please explain what this is in a bit more detail. I have no
idea what you are trying to accomplish.

Also, do you mean ftrace as the internal Linux tracer (which should
only be called "ftrace" or sometimes "Ftrace" but not "f_trace" or
"FTrace", that just leads to more confusion.

Or is this to do with http://www.ftrace.com/en/gb, a way to trace
produce ;-)

-- Steve


> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---
> 
> I wanted to take this through the gadget tree, but there is a
> dependency with a previous patch of mine adding and extra argument to
> the ->write() function. Hoping someone else will take it.
> 
>  drivers/usb/gadget/Kconfig            |  15 ++
>  drivers/usb/gadget/function/Makefile  |   2 +
>  drivers/usb/gadget/function/f-trace.c | 400 ++++++++++++++++++++++++++++++++++
>  3 files changed, 417 insertions(+)
>  create mode 100644 drivers/usb/gadget/function/f-trace.c
> 
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index c164d6b788c3..617921f19b5e 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -188,6 +188,9 @@ config USB_F_MASS_STORAGE
>  config USB_F_FS
>  	tristate
>  
> +config USB_F_TRACE
> +	tristate
> +
>  config USB_F_UAC1
>  	tristate
>  
> @@ -362,6 +365,18 @@ config USB_CONFIGFS_F_FS
>  	  implemented in kernel space (for instance Ethernet, serial or
>  	  mass storage) and other are implemented in user space.
>  
> +config USB_CONFIGFS_F_TRACE
> +	bool "Linux FTrace Export Over USB"
> +	depends on USB_CONFIGFS
> +	select USB_F_TRACE
> +	help
> +	  The Linux FTrace Export Over USB lets one export ftrace buffer
> +	  over a USB cable to a host computer for further processing.
> +
> +	  If you want support for that, say Y or M here. Otherwise say N.
> +
> +	  If unsure, say N.
> +
>  config USB_CONFIGFS_F_UAC1
>  	bool "Audio Class 1.0"
>  	depends on USB_CONFIGFS
> diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile
> index cb8c225e8549..1433e8ad7675 100644
> --- a/drivers/usb/gadget/function/Makefile
> +++ b/drivers/usb/gadget/function/Makefile
> @@ -46,3 +46,5 @@ usb_f_printer-y			:= f_printer.o
>  obj-$(CONFIG_USB_F_PRINTER)	+= usb_f_printer.o
>  usb_f_tcm-y			:= f_tcm.o
>  obj-$(CONFIG_USB_F_TCM)		+= usb_f_tcm.o
> +usb_f_trace-y			:= f-trace.o
> +obj-$(CONFIG_USB_F_TRACE)	+= usb_f_trace.o
> diff --git a/drivers/usb/gadget/function/f-trace.c b/drivers/usb/gadget/function/f-trace.c
> new file mode 100644
> index 000000000000..7de92950c0e7
> --- /dev/null
> +++ b/drivers/usb/gadget/function/f-trace.c
> @@ -0,0 +1,400 @@
> +/*
> + * f_trace.c -- USB FTrace Export
> + *
> + * Copyright (C) 2017 Intel Corporation
> + * Author: Felipe Balbi <felipe.balbi@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License v2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/trace.h>
> +#include <linux/usb.h>
> +#include <linux/usb/composite.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/workqueue.h>
> +
> +struct usb_ftrace {
> +	struct trace_export ftrace;
> +	struct usb_function function;
> +	struct work_struct queue_work;
> +	spinlock_t lock;
> +
> +	struct list_head list;
> +	struct list_head pending;
> +	struct list_head queued;
> +
> +	struct usb_ep *in;
> +
> +	u8 intf_id;
> +};
> +#define ftrace_to_trace(f)	(container_of((f), struct usb_ftrace, ftrace))
> +#define work_to_trace(w)	(container_of((w), struct usb_ftrace, queue_work))
> +#define to_trace(f)		(container_of((f), struct usb_ftrace, function))
> +
> +#define FTRACE_REQUEST_QUEUE_LENGTH	250
> +
> +static inline struct usb_request *next_request(struct list_head *list)
> +{
> +	return list_first_entry_or_null(list, struct usb_request, list);
> +}
> +
> +struct usb_ftrace_opts {
> +	struct usb_function_instance func_inst;
> +};
> +#define to_opts(fi)	(container_of((fi), struct usb_ftrace_opts, func_inst))
> +
> +static struct usb_interface_descriptor ftrace_intf_desc = {
> +	.bLength		= USB_DT_INTERFACE_SIZE,
> +	.bDescriptorType	= USB_DT_INTERFACE,
> +
> +	.bAlternateSetting	= 0,
> +	.bNumEndpoints		= 1,
> +	.bInterfaceClass	= USB_CLASS_VENDOR_SPEC,
> +	.bInterfaceSubClass	= USB_SUBCLASS_VENDOR_SPEC,
> +};
> +
> +/* Super-Speed Support */
> +static struct usb_endpoint_descriptor ftrace_ss_in_desc = {
> +	.bLength		= USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType	= USB_DT_ENDPOINT,
> +
> +	.bEndpointAddress	= USB_DIR_IN,
> +	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize		= cpu_to_le16(1024),
> +};
> +
> +static struct usb_ss_ep_comp_descriptor ftrace_ss_in_comp_desc = {
> +	.bLength		= USB_DT_SS_EP_COMP_SIZE,
> +	.bDescriptorType	= USB_DT_SS_ENDPOINT_COMP,
> +
> +	.bMaxBurst		= 15,
> +};
> +
> +static struct usb_descriptor_header *ftrace_ss_function[] = {
> +	(struct usb_descriptor_header *) &ftrace_intf_desc,
> +	(struct usb_descriptor_header *) &ftrace_ss_in_desc,
> +	(struct usb_descriptor_header *) &ftrace_ss_in_comp_desc,
> +	NULL,
> +};
> +
> +/* High-Speed Support */
> +static struct usb_endpoint_descriptor ftrace_hs_in_desc = {
> +	.bLength		= USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType	= USB_DT_ENDPOINT,
> +
> +	.bEndpointAddress	= USB_DIR_IN,
> +	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize		= cpu_to_le16(512),
> +};
> +
> +static struct usb_descriptor_header *ftrace_hs_function[] = {
> +	(struct usb_descriptor_header *) &ftrace_intf_desc,
> +	(struct usb_descriptor_header *) &ftrace_hs_in_desc,
> +	NULL,
> +};
> +
> +/* Full-Speed Support */
> +static struct usb_endpoint_descriptor ftrace_fs_in_desc = {
> +	.bLength		= USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType	= USB_DT_ENDPOINT,
> +
> +	.bEndpointAddress	= USB_DIR_IN,
> +	.bmAttributes		= USB_ENDPOINT_XFER_BULK,
> +	.wMaxPacketSize		= cpu_to_le16(64),
> +};
> +
> +static struct usb_descriptor_header *ftrace_fs_function[] = {
> +	(struct usb_descriptor_header *) &ftrace_intf_desc,
> +	(struct usb_descriptor_header *) &ftrace_fs_in_desc,
> +	NULL,
> +};
> +
> +static struct usb_string ftrace_string_defs[] = {
> +	[0].s = "Linux Ftrace Export",
> +	{ },
> +};
> +
> +static struct usb_gadget_strings ftrace_string_table = {
> +	.language		= 0x0409, /* en-US */
> +	.strings		= ftrace_string_defs,
> +};
> +
> +static struct usb_gadget_strings *ftrace_strings[] = {
> +	&ftrace_string_table,
> +	NULL,
> +};
> +
> +/* ------------------------------------------------------------------------ */
> +
> +static void ftrace_complete(struct usb_ep *ep, struct usb_request *req)
> +{
> +	struct usb_ftrace		*trace = req->context;
> +
> +	kfree(req->buf);
> +	list_move_tail(&req->list, &trace->list);
> +}
> +
> +static void ftrace_queue_work(struct work_struct *work)
> +{
> +	struct usb_ftrace		*trace = work_to_trace(work);
> +	struct usb_request		*req;
> +	struct usb_request		*tmp;
> +	struct list_head		local_list;
> +
> +	spin_lock_irq(&trace->lock);
> +restart:
> +	list_replace_init(&trace->pending, &local_list);
> +	spin_unlock_irq(&trace->lock);
> +
> +	list_for_each_entry_safe(req, tmp, &local_list, list) {
> +		int			ret;
> +
> +		ret = usb_ep_queue(trace->in, req, GFP_KERNEL);
> +		if (!ret)
> +			list_move_tail(&req->list, &trace->queued);
> +	}
> +
> +	spin_lock_irq(&trace->lock);
> +	if (!list_empty(&trace->pending))
> +		goto restart;
> +	spin_unlock_irq(&trace->lock);
> +}
> +
> +static void notrace ftrace_write(struct trace_export *ftrace, const void *buf,
> +				 unsigned int len)
> +{
> +	struct usb_ftrace		*trace = ftrace_to_trace(ftrace);
> +	struct usb_request		*req = next_request(&trace->list);
> +
> +	if (!req)
> +		return;
> +
> +	if (!trace->in->enabled)
> +		return;
> +
> +	req->buf = kmemdup(buf, len, GFP_ATOMIC);
> +	req->length = len;
> +	req->context = trace;
> +	req->complete = ftrace_complete;
> +	list_move_tail(&req->list, &trace->pending);
> +
> +	schedule_work(&trace->queue_work);
> +}
> +
> +/* ------------------------------------------------------------------------ */
> +
> +static void ftrace_disable_endpoint(struct usb_ftrace *trace)
> +{
> +	if (trace->in->enabled)
> +		WARN_ON(usb_ep_disable(trace->in));
> +}
> +
> +static int ftrace_enable_endpoint(struct usb_ftrace *trace)
> +{
> +	if (trace->in->enabled)
> +		return 0;
> +
> +	return usb_ep_enable(trace->in);
> +}
> +
> +static int ftrace_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
> +{
> +	struct usb_ftrace		*trace = to_trace(f);
> +	struct usb_composite_dev	*cdev = f->config->cdev;
> +	int				ret;
> +
> +	if (alt != 0)
> +		goto fail;
> +
> +	if (intf != trace->intf_id)
> +		goto fail;
> +
> +	ftrace_disable_endpoint(trace);
> +
> +	if (!trace->in->desc) {
> +		ret = config_ep_by_speed(cdev->gadget, f, trace->in);
> +		if (ret) {
> +			trace->in->desc = NULL;
> +			goto fail;
> +		}
> +	}
> +
> +	ret = ftrace_enable_endpoint(trace);
> +	if (ret)
> +		goto fail;
> +
> +	return 0;
> +
> +fail:
> +	return -EINVAL;
> +}
> +
> +static int ftrace_bind(struct usb_configuration *c, struct usb_function *f)
> +{
> +	struct usb_composite_dev	*cdev = c->cdev;
> +	struct usb_ftrace		*trace = to_trace(f);
> +	struct usb_string		*us;
> +	struct usb_ep			*ep;
> +
> +	int				ret;
> +	int				i;
> +
> +	us = usb_gstrings_attach(cdev, ftrace_strings,
> +				 ARRAY_SIZE(ftrace_string_defs));
> +	if (IS_ERR(us))
> +		return PTR_ERR(us);
> +
> +	ftrace_intf_desc.iInterface = us[0].id;
> +
> +	ret = usb_interface_id(c, f);
> +	if (ret < 0)
> +		goto err0;
> +	trace->intf_id = ret;
> +	ftrace_intf_desc.bInterfaceNumber = ret;
> +
> +	ep = usb_ep_autoconfig(cdev->gadget, &ftrace_fs_in_desc);
> +	if (!ep)
> +		goto err0;
> +	trace->in = ep;
> +
> +	ftrace_hs_in_desc.bEndpointAddress = ftrace_fs_in_desc.bEndpointAddress;
> +	ftrace_ss_in_desc.bEndpointAddress = ftrace_fs_in_desc.bEndpointAddress;
> +
> +	trace->ftrace.write = ftrace_write;
> +
> +	spin_lock_init(&trace->lock);
> +	INIT_WORK(&trace->queue_work, ftrace_queue_work);
> +	INIT_LIST_HEAD(&trace->list);
> +	INIT_LIST_HEAD(&trace->pending);
> +	INIT_LIST_HEAD(&trace->queued);
> +
> +	ret = usb_assign_descriptors(f, ftrace_fs_function, ftrace_hs_function,
> +				     ftrace_ss_function, NULL);
> +	if (ret)
> +		goto err0;
> +
> +	for (i = 0; i < FTRACE_REQUEST_QUEUE_LENGTH; i++) {
> +		struct usb_request *req;
> +
> +		req = usb_ep_alloc_request(trace->in, GFP_KERNEL);
> +		if (!req)
> +			goto err1;
> +
> +		list_add_tail(&req->list, &trace->list);
> +	}
> +
> +	ret = register_ftrace_export(&trace->ftrace);
> +	if (ret)
> +		goto err1;
> +
> +	return 0;
> +
> +err1:
> +	while (!list_empty(&trace->list)) {
> +		struct usb_request *req = next_request(&trace->list);
> +
> +		usb_ep_free_request(trace->in, req);
> +		list_del(&req->list);
> +	}
> +
> +	usb_free_all_descriptors(f);
> +
> +err0:
> +	ERROR(cdev, "%s: can't bind --> err %d\n", f->name, ret);
> +
> +	return ret;
> +}
> +
> +static void ftrace_unbind(struct usb_configuration *c, struct usb_function *f)
> +{
> +	struct usb_ftrace		*trace = to_trace(f);
> +	struct usb_request		*req;
> +	struct usb_request		*tmp;
> +
> +	unregister_ftrace_export(&trace->ftrace);
> +	cancel_work_sync(&trace->queue_work);
> +	usb_free_all_descriptors(f);
> +
> +	list_for_each_entry(req, &trace->queued, list)
> +		usb_ep_dequeue(trace->in, req);
> +
> +	list_for_each_entry_safe(req, tmp, &trace->pending, list) {
> +		usb_ep_free_request(trace->in, req);
> +		list_del(&req->list);
> +	}
> +
> +	list_for_each_entry_safe(req, tmp, &trace->list, list) {
> +		usb_ep_free_request(trace->in, req);
> +		list_del(&req->list);
> +	}
> +}
> +
> +static void ftrace_disable(struct usb_function *f)
> +{
> +	struct usb_ftrace		*trace = to_trace(f);
> +
> +	ftrace_disable_endpoint(trace);
> +}
> +
> +static void ftrace_free_func(struct usb_function *f)
> +{
> +	kfree(to_trace(f));
> +}
> +
> +static struct config_item_type ftrace_func_type = {
> +	.ct_owner		= THIS_MODULE,
> +};
> +
> +static void ftrace_free_inst(struct usb_function_instance *fi)
> +{
> +	struct usb_ftrace_opts		*opts = to_opts(fi);
> +
> +	kfree(opts);
> +}
> +
> +static struct usb_function_instance *ftrace_alloc_inst(void)
> +{
> +	struct usb_ftrace_opts		*opts;
> +
> +	opts = kzalloc(sizeof(*opts), GFP_KERNEL);
> +	if (!opts)
> +		return ERR_PTR(-ENOMEM);
> +
> +	opts->func_inst.free_func_inst = ftrace_free_inst;
> +
> +	config_group_init_type_name(&opts->func_inst.group, "",
> +				    &ftrace_func_type);
> +
> +	return &opts->func_inst;
> +}
> +
> +static struct usb_function *ftrace_alloc(struct usb_function_instance *fi)
> +{
> +	struct usb_ftrace		*trace;
> +
> +	trace = kzalloc(sizeof(*trace), GFP_KERNEL);
> +	if (!trace)
> +		return NULL;
> +
> +	trace->function.name = "ftrace";
> +	trace->function.bind = ftrace_bind;
> +	trace->function.unbind = ftrace_unbind;
> +	trace->function.set_alt = ftrace_set_alt;
> +	trace->function.disable = ftrace_disable;
> +	trace->function.strings = ftrace_strings;
> +	trace->function.free_func = ftrace_free_func;
> +
> +	return &trace->function;
> +}
> +
> +DECLARE_USB_FUNCTION_INIT(ftrace, ftrace_alloc_inst, ftrace_alloc);
> +MODULE_AUTHOR("Felipe Balbi <felipe.balbi@linux.intel.com>");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]


Hi,

Steven Rostedt <rostedt@goodmis.org> writes:
> On Fri,  9 Jun 2017 09:13:27 +0300
> Felipe Balbi <felipe.balbi@linux.intel.com> wrote:
>
>> Allow for ftrace data to be exported over a USB Gadget
>> Controller. With this, we have a potentially very fast pipe for
>> transmitting ftrace data to a Host PC for further analysis.
>> 
>> Note that in order to decode the data, one needs access to kernel
>> symbols in order to convert binary data into function names and what
>> not.
>> 
>
> Can you please explain what this is in a bit more detail. I have no
> idea what you are trying to accomplish.

this is just another ftrace export. Just like STM
(drivers/hwtracing/stm/ftrace.c), but I'm making use of a USB Peripheral
Controller that may be available.

> Also, do you mean ftrace as the internal Linux tracer (which should
> only be called "ftrace" or sometimes "Ftrace" but not "f_trace" or
> "FTrace", that just leads to more confusion.

heh, internal linux ftrace ;-) The driver name (f-trace.c) is just to
follow the convention of USB functions being name f_*.c or f-*.c. I
could call it f-ftrace.c, but seemed unnecessary.

> Or is this to do with http://www.ftrace.com/en/gb, a way to trace
> produce ;-)

heh :-) unlikely

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Steven Rostedt]

On Fri, 09 Jun 2017 17:05:52 +0300
Felipe Balbi <felipe.balbi@linux.intel.com> wrote:

> Hi,
> 
> Steven Rostedt <rostedt@goodmis.org> writes:
> > On Fri,  9 Jun 2017 09:13:27 +0300
> > Felipe Balbi <felipe.balbi@linux.intel.com> wrote:
> >  
> >> Allow for ftrace data to be exported over a USB Gadget
> >> Controller. With this, we have a potentially very fast pipe for
> >> transmitting ftrace data to a Host PC for further analysis.
> >> 
> >> Note that in order to decode the data, one needs access to kernel
> >> symbols in order to convert binary data into function names and what
> >> not.
> >>   
> >
> > Can you please explain what this is in a bit more detail. I have no
> > idea what you are trying to accomplish.  
> 
> this is just another ftrace export. Just like STM
> (drivers/hwtracing/stm/ftrace.c), but I'm making use of a USB Peripheral
> Controller that may be available.
> 
> > Also, do you mean ftrace as the internal Linux tracer (which should
> > only be called "ftrace" or sometimes "Ftrace" but not "f_trace" or
> > "FTrace", that just leads to more confusion.  
> 
> heh, internal linux ftrace ;-) The driver name (f-trace.c) is just to
> follow the convention of USB functions being name f_*.c or f-*.c. I
> could call it f-ftrace.c, but seemed unnecessary.

OK, looking at the other files and functions in
drivers/usb/gadget/function, I see that F there is part of usb process.
OK, although it does make it somewhat confusing.

-- Steve

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

Hi Felipe,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.12-rc4 next-20170609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Felipe-Balbi/usb-gadget-functions-add-ftrace-export-over-USB/20170610-060059
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/usb/gadget/function/f-trace.c: In function 'ftrace_bind':
>> drivers/usb/gadget/function/f-trace.c:271:22: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
     trace->ftrace.write = ftrace_write;
                         ^
   cc1: some warnings being treated as errors

vim +271 drivers/usb/gadget/function/f-trace.c

   265			goto err0;
   266		trace->in = ep;
   267	
   268		ftrace_hs_in_desc.bEndpointAddress = ftrace_fs_in_desc.bEndpointAddress;
   269		ftrace_ss_in_desc.bEndpointAddress = ftrace_fs_in_desc.bEndpointAddress;
   270	
 > 271		trace->ftrace.write = ftrace_write;
   272	
   273		spin_lock_init(&trace->lock);
   274		INIT_WORK(&trace->queue_work, ftrace_queue_work);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60009 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9154 bytes --]

Hi Felipe,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.12-rc4 next-20170609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Felipe-Balbi/usb-gadget-functions-add-ftrace-export-over-USB/20170610-060059
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All error/warnings (new ones prefixed by >>):

>> drivers/usb/gadget/function/f-trace.c:25:22: error: field 'ftrace' has incomplete type
     struct trace_export ftrace;
                         ^~~~~~
   In file included from include/linux/list.h:8:0,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:17,
                    from drivers/usb/gadget/function/f-trace.c:12:
   drivers/usb/gadget/function/f-trace.c: In function 'ftrace_write':
   include/linux/kernel.h:854:48: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     const typeof( ((type *)0)->member ) *__mptr = (ptr); \
                                                   ^
>> drivers/usb/gadget/function/f-trace.c:38:29: note: in expansion of macro 'container_of'
    #define ftrace_to_trace(f) (container_of((f), struct usb_ftrace, ftrace))
                                ^~~~~~~~~~~~
>> drivers/usb/gadget/function/f-trace.c:174:30: note: in expansion of macro 'ftrace_to_trace'
     struct usb_ftrace  *trace = ftrace_to_trace(ftrace);
                                 ^~~~~~~~~~~~~~~
   include/linux/kernel.h:854:48: note: (near initialization for 'trace')
     const typeof( ((type *)0)->member ) *__mptr = (ptr); \
                                                   ^
>> drivers/usb/gadget/function/f-trace.c:38:29: note: in expansion of macro 'container_of'
    #define ftrace_to_trace(f) (container_of((f), struct usb_ftrace, ftrace))
                                ^~~~~~~~~~~~
>> drivers/usb/gadget/function/f-trace.c:174:30: note: in expansion of macro 'ftrace_to_trace'
     struct usb_ftrace  *trace = ftrace_to_trace(ftrace);
                                 ^~~~~~~~~~~~~~~
   drivers/usb/gadget/function/f-trace.c: In function 'ftrace_bind':
>> drivers/usb/gadget/function/f-trace.c:294:8: error: implicit declaration of function 'register_ftrace_export' [-Werror=implicit-function-declaration]
     ret = register_ftrace_export(&trace->ftrace);
           ^~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/gadget/function/f-trace.c: In function 'ftrace_unbind':
>> drivers/usb/gadget/function/f-trace.c:322:2: error: implicit declaration of function 'unregister_ftrace_export' [-Werror=implicit-function-declaration]
     unregister_ftrace_export(&trace->ftrace);
     ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/ftrace +25 drivers/usb/gadget/function/f-trace.c

     6	 *
     7	 * This program is free software; you can redistribute it and/or
     8	 * modify it under the terms of the GNU General Public License v2 as
     9	 * published by the Free Software Foundation.
    10	 */
    11	
  > 12	#include <linux/device.h>
    13	#include <linux/kernel.h>
    14	#include <linux/list.h>
    15	#include <linux/module.h>
    16	#include <linux/slab.h>
    17	#include <linux/spinlock.h>
    18	#include <linux/trace.h>
    19	#include <linux/usb.h>
    20	#include <linux/usb/composite.h>
    21	#include <linux/usb/gadget.h>
    22	#include <linux/workqueue.h>
    23	
    24	struct usb_ftrace {
  > 25		struct trace_export ftrace;
    26		struct usb_function function;
    27		struct work_struct queue_work;
    28		spinlock_t lock;
    29	
    30		struct list_head list;
    31		struct list_head pending;
    32		struct list_head queued;
    33	
    34		struct usb_ep *in;
    35	
    36		u8 intf_id;
    37	};
  > 38	#define ftrace_to_trace(f)	(container_of((f), struct usb_ftrace, ftrace))
    39	#define work_to_trace(w)	(container_of((w), struct usb_ftrace, queue_work))
    40	#define to_trace(f)		(container_of((f), struct usb_ftrace, function))
    41	
    42	#define FTRACE_REQUEST_QUEUE_LENGTH	250
    43	
    44	static inline struct usb_request *next_request(struct list_head *list)
    45	{
    46		return list_first_entry_or_null(list, struct usb_request, list);
    47	}
    48	
    49	struct usb_ftrace_opts {
    50		struct usb_function_instance func_inst;
    51	};
    52	#define to_opts(fi)	(container_of((fi), struct usb_ftrace_opts, func_inst))
    53	
    54	static struct usb_interface_descriptor ftrace_intf_desc = {
    55		.bLength		= USB_DT_INTERFACE_SIZE,
    56		.bDescriptorType	= USB_DT_INTERFACE,
    57	
    58		.bAlternateSetting	= 0,
    59		.bNumEndpoints		= 1,
    60		.bInterfaceClass	= USB_CLASS_VENDOR_SPEC,
    61		.bInterfaceSubClass	= USB_SUBCLASS_VENDOR_SPEC,
    62	};
    63	
    64	/* Super-Speed Support */
    65	static struct usb_endpoint_descriptor ftrace_ss_in_desc = {
    66		.bLength		= USB_DT_ENDPOINT_SIZE,
    67		.bDescriptorType	= USB_DT_ENDPOINT,
    68	
    69		.bEndpointAddress	= USB_DIR_IN,
    70		.bmAttributes		= USB_ENDPOINT_XFER_BULK,
    71		.wMaxPacketSize		= cpu_to_le16(1024),
    72	};
    73	
    74	static struct usb_ss_ep_comp_descriptor ftrace_ss_in_comp_desc = {
    75		.bLength		= USB_DT_SS_EP_COMP_SIZE,
    76		.bDescriptorType	= USB_DT_SS_ENDPOINT_COMP,
    77	
    78		.bMaxBurst		= 15,
    79	};
    80	
    81	static struct usb_descriptor_header *ftrace_ss_function[] = {
    82		(struct usb_descriptor_header *) &ftrace_intf_desc,
    83		(struct usb_descriptor_header *) &ftrace_ss_in_desc,
    84		(struct usb_descriptor_header *) &ftrace_ss_in_comp_desc,
    85		NULL,
    86	};
    87	
    88	/* High-Speed Support */
    89	static struct usb_endpoint_descriptor ftrace_hs_in_desc = {
    90		.bLength		= USB_DT_ENDPOINT_SIZE,
    91		.bDescriptorType	= USB_DT_ENDPOINT,
    92	
    93		.bEndpointAddress	= USB_DIR_IN,
    94		.bmAttributes		= USB_ENDPOINT_XFER_BULK,
    95		.wMaxPacketSize		= cpu_to_le16(512),
    96	};
    97	
    98	static struct usb_descriptor_header *ftrace_hs_function[] = {
    99		(struct usb_descriptor_header *) &ftrace_intf_desc,
   100		(struct usb_descriptor_header *) &ftrace_hs_in_desc,
   101		NULL,
   102	};
   103	
   104	/* Full-Speed Support */
   105	static struct usb_endpoint_descriptor ftrace_fs_in_desc = {
   106		.bLength		= USB_DT_ENDPOINT_SIZE,
   107		.bDescriptorType	= USB_DT_ENDPOINT,
   108	
   109		.bEndpointAddress	= USB_DIR_IN,
   110		.bmAttributes		= USB_ENDPOINT_XFER_BULK,
   111		.wMaxPacketSize		= cpu_to_le16(64),
   112	};
   113	
   114	static struct usb_descriptor_header *ftrace_fs_function[] = {
   115		(struct usb_descriptor_header *) &ftrace_intf_desc,
   116		(struct usb_descriptor_header *) &ftrace_fs_in_desc,
   117		NULL,
   118	};
   119	
   120	static struct usb_string ftrace_string_defs[] = {
   121		[0].s = "Linux Ftrace Export",
   122		{ },
   123	};
   124	
   125	static struct usb_gadget_strings ftrace_string_table = {
   126		.language		= 0x0409, /* en-US */
   127		.strings		= ftrace_string_defs,
   128	};
   129	
   130	static struct usb_gadget_strings *ftrace_strings[] = {
   131		&ftrace_string_table,
   132		NULL,
   133	};
   134	
   135	/* ------------------------------------------------------------------------ */
   136	
   137	static void ftrace_complete(struct usb_ep *ep, struct usb_request *req)
   138	{
   139		struct usb_ftrace		*trace = req->context;
   140	
   141		kfree(req->buf);
   142		list_move_tail(&req->list, &trace->list);
   143	}
   144	
   145	static void ftrace_queue_work(struct work_struct *work)
   146	{
   147		struct usb_ftrace		*trace = work_to_trace(work);
   148		struct usb_request		*req;
   149		struct usb_request		*tmp;
   150		struct list_head		local_list;
   151	
   152		spin_lock_irq(&trace->lock);
   153	restart:
   154		list_replace_init(&trace->pending, &local_list);
   155		spin_unlock_irq(&trace->lock);
   156	
   157		list_for_each_entry_safe(req, tmp, &local_list, list) {
   158			int			ret;
   159	
   160			ret = usb_ep_queue(trace->in, req, GFP_KERNEL);
   161			if (!ret)
   162				list_move_tail(&req->list, &trace->queued);
   163		}
   164	
   165		spin_lock_irq(&trace->lock);
   166		if (!list_empty(&trace->pending))
   167			goto restart;
   168		spin_unlock_irq(&trace->lock);
   169	}
   170	
   171	static void notrace ftrace_write(struct trace_export *ftrace, const void *buf,
   172					 unsigned int len)
   173	{
 > 174		struct usb_ftrace		*trace = ftrace_to_trace(ftrace);
   175		struct usb_request		*req = next_request(&trace->list);
   176	
   177		if (!req)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50314 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 439 bytes --]


Hi,

kbuild test robot <lkp@intel.com> writes:
> Hi Felipe,
>
> [auto build test ERROR on balbi-usb/next]
> [also build test ERROR on v4.12-rc4 next-20170609]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

this patch depends on another patch [1] in order to compile properly :-)

[1] https://marc.info/?i=20170602102025.5140-1-felipe.balbi@linux.intel.com

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

Hi,

Felipe Balbi <felipe.balbi@linux.intel.com> writes:
> Allow for ftrace data to be exported over a USB Gadget
> Controller. With this, we have a potentially very fast pipe for
> transmitting ftrace data to a Host PC for further analysis.
>
> Note that in order to decode the data, one needs access to kernel
> symbols in order to convert binary data into function names and what
> not.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---

Looks like this and its depedency missed the merge window. Steven, how
do you want me to handle them? Should I rebase them once -rc1 is tagged
and resend?

-- 
balbi

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Pratyush Anand]

Hi Felipe,

On Friday 09 June 2017 11:43 AM, Felipe Balbi wrote:
> +static void notrace ftrace_write(struct trace_export *ftrace, const void *buf,
> +				 unsigned int len)
> +{
> +	struct usb_ftrace		*trace = ftrace_to_trace(ftrace);
> +	struct usb_request		*req = next_request(&trace->list);
> +
> +	if (!req)
> +		return;
> +
> +	if (!trace->in->enabled)
> +		return;
> +
> +	req->buf = kmemdup(buf, len, GFP_ATOMIC);

Probably we can avoid the copy of trace data.

We can make write() call of "struct trace_export" as posted. Can have a 
write_complete() callback function implemented in  struct trace_export,which 
can be called from your ftrace_complete().

We need to execute __buffer_unlock_commit() only in write_complete() in case 
of ftrace_export is enabled.


> +	req->length = len;
> +	req->context = trace;
> +	req->complete = ftrace_complete;
> +	list_move_tail(&req->list, &trace->pending);
> +
> +	schedule_work(&trace->queue_work);
> +}
> +

--
Pratyush

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Pratyush Anand]

Hi Felipe,

On Friday 09 June 2017 03:58 PM, Felipe Balbi wrote:
> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
>
>> Allow for ftrace data to be exported over a USB Gadget
>> Controller. With this, we have a potentially very fast pipe for
>> transmitting ftrace data to a Host PC for further analysis.
>>
>> Note that in order to decode the data, one needs access to kernel
>> symbols in order to convert binary data into function names and what
>> not.
>>
>> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>> ---
>>
>> I wanted to take this through the gadget tree, but there is a
>> dependency with a previous patch of mine adding and extra argument to
>> the ->write() function. Hoping someone else will take it.
>
> just as an extra note here. In order for this to be really useful, it
> would be nice to be able to control what is going to be traced over USB

Probably you will also need to export *atleast* symbol information. In future, 
if this framework is extended to export tracepoint/kprobe/uprobe event data, 
then the information like event format etc will also need to be exported.

What tool do you use on host machine in order to extract information from this 
exported target ring buffer data?
IMHO, standard tools like trace-cmd will not be able to use it as it is.

> as well, but that means exporting a few extra functions to GPL drivers.
>
> Would that be okay? I could have a set of vendor-specific control
> requests to set buffer size and to read/write ftrace filter functions.
>
> The idea is that things like e.g. Android SDK could rely on this on
> debug builds and the SDK itself would make sure to keep a copy of
> vmlinux around to processing of the data coming through USB.
>

--
Pratyush

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]


Hi,

Pratyush Anand <panand@redhat.com> writes:
> On Friday 09 June 2017 03:58 PM, Felipe Balbi wrote:
>> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
>>
>>> Allow for ftrace data to be exported over a USB Gadget
>>> Controller. With this, we have a potentially very fast pipe for
>>> transmitting ftrace data to a Host PC for further analysis.
>>>
>>> Note that in order to decode the data, one needs access to kernel
>>> symbols in order to convert binary data into function names and what
>>> not.
>>>
>>> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>>> ---
>>>
>>> I wanted to take this through the gadget tree, but there is a
>>> dependency with a previous patch of mine adding and extra argument to
>>> the ->write() function. Hoping someone else will take it.
>>
>> just as an extra note here. In order for this to be really useful, it
>> would be nice to be able to control what is going to be traced over USB
>
> Probably you will also need to export *atleast* symbol information. In future, 

just keep your vmlinux around. This is not supposed to be shipped on end
products, only debug builds.

> if this framework is extended to export tracepoint/kprobe/uprobe event data, 
> then the information like event format etc will also need to be exported.
>
> What tool do you use on host machine in order to extract information from this 
> exported target ring buffer data?
> IMHO, standard tools like trace-cmd will not be able to use it as it is.

I hacked something together with libusb for testing. Proper tooling is
needed, indeed.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]


Hi,

Pratyush Anand <panand@redhat.com> writes:
> Hi Felipe,
>
> On Friday 09 June 2017 11:43 AM, Felipe Balbi wrote:
>> +static void notrace ftrace_write(struct trace_export *ftrace, const void *buf,
>> +				 unsigned int len)
>> +{
>> +	struct usb_ftrace		*trace = ftrace_to_trace(ftrace);
>> +	struct usb_request		*req = next_request(&trace->list);
>> +
>> +	if (!req)
>> +		return;
>> +
>> +	if (!trace->in->enabled)
>> +		return;
>> +
>> +	req->buf = kmemdup(buf, len, GFP_ATOMIC);
>
> Probably we can avoid the copy of trace data.

not with current setup. I have no control over data's lifetime. If I
did, then this would be zero-copy.

> We can make write() call of "struct trace_export" as posted. Can have a 

that would require further patching in trace core which is not part of
$subject. Also, if we turn this into asynchronous calls, we could run
out of trace buffer before the first chunk of data is transferred over
to the other side.

> write_complete() callback function implemented in  struct trace_export,which 
> can be called from your ftrace_complete().

well, you're welcome to propose a patch :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Steven Rostedt]

On Fri, 14 Jul 2017 11:22:51 +0530
Pratyush Anand <panand@redhat.com> wrote:

> Hi Felipe,
> 
> On Friday 09 June 2017 03:58 PM, Felipe Balbi wrote:
> > Felipe Balbi <felipe.balbi@linux.intel.com> writes:
> >  
> >> Allow for ftrace data to be exported over a USB Gadget
> >> Controller. With this, we have a potentially very fast pipe for
> >> transmitting ftrace data to a Host PC for further analysis.
> >>
> >> Note that in order to decode the data, one needs access to kernel
> >> symbols in order to convert binary data into function names and what
> >> not.
> >>
> >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> >> ---
> >>
> >> I wanted to take this through the gadget tree, but there is a
> >> dependency with a previous patch of mine adding and extra argument to
> >> the ->write() function. Hoping someone else will take it.  
> >
> > just as an extra note here. In order for this to be really useful, it
> > would be nice to be able to control what is going to be traced over USB  
> 
> Probably you will also need to export *atleast* symbol information. In future, 
> if this framework is extended to export tracepoint/kprobe/uprobe event data, 
> then the information like event format etc will also need to be exported.
> 
> What tool do you use on host machine in order to extract information from this 
> exported target ring buffer data?
> IMHO, standard tools like trace-cmd will not be able to use it as it is.
>

Have you looked at how trace-cmd does network transfers? Perhaps this
can do something similar. I'm also working on having trace-cmd work
through sockets for guest to host tracing.

See trace-cmd listen.

-- Steve

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]


Hi,

Felipe Balbi <felipe.balbi@linux.intel.com> writes:
> Allow for ftrace data to be exported over a USB Gadget
> Controller. With this, we have a potentially very fast pipe for
> transmitting ftrace data to a Host PC for further analysis.
>
> Note that in order to decode the data, one needs access to kernel
> symbols in order to convert binary data into function names and what
> not.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---
>
> I wanted to take this through the gadget tree, but there is a
> dependency with a previous patch of mine adding and extra argument to
> the ->write() function. Hoping someone else will take it.

This is still not upstream, I'll take it through my tree this time around.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Steven Rostedt]

On Thu, 21 Mar 2019 11:38:56 +0200
Felipe Balbi <felipe.balbi@linux.intel.com> wrote:

> Hi,
> 
> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
> > Allow for ftrace data to be exported over a USB Gadget
> > Controller. With this, we have a potentially very fast pipe for
> > transmitting ftrace data to a Host PC for further analysis.
> >
> > Note that in order to decode the data, one needs access to kernel
> > symbols in order to convert binary data into function names and what
> > not.
> >
> > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > ---
> >
> > I wanted to take this through the gadget tree, but there is a
> > dependency with a previous patch of mine adding and extra argument to
> > the ->write() function. Hoping someone else will take it.  
> 
> This is still not upstream, I'll take it through my tree this time around.
> 

Can you post the latest to the list one more time. At least to make
sure you got the naming correct ;-)

  Not FTrace or f_trace.

-- Steve

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]


Hi,

Steven Rostedt <rostedt@goodmis.org> writes:
>> Felipe Balbi <felipe.balbi@linux.intel.com> writes:
>> > Allow for ftrace data to be exported over a USB Gadget
>> > Controller. With this, we have a potentially very fast pipe for
>> > transmitting ftrace data to a Host PC for further analysis.
>> >
>> > Note that in order to decode the data, one needs access to kernel
>> > symbols in order to convert binary data into function names and what
>> > not.
>> >
>> > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>> > ---
>> >
>> > I wanted to take this through the gadget tree, but there is a
>> > dependency with a previous patch of mine adding and extra argument to
>> > the ->write() function. Hoping someone else will take it.  
>> 
>> This is still not upstream, I'll take it through my tree this time around.
>> 
>
> Can you post the latest to the list one more time. At least to make
> sure you got the naming correct ;-)
>
>   Not FTrace or f_trace.

The reason for the name was discussed already. See [1] and [2]

[1] https://marc.info/?i=878tl1qmrj.fsf@linux.intel.com
[2] https://marc.info/?i=20170609110717.3304f6b9@gandalf.local.home

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Steven Rostedt]

On Fri, 22 Mar 2019 13:49:55 +0200
Felipe Balbi <felipe.balbi@linux.intel.com> wrote:

> > Can you post the latest to the list one more time. At least to make
> > sure you got the naming correct ;-)
> >
> >   Not FTrace or f_trace.  
> 
> The reason for the name was discussed already. See [1] and [2]
> 
> [1] https://marc.info/?i=878tl1qmrj.fsf@linux.intel.com
> [2] https://marc.info/?i=20170609110717.3304f6b9@gandalf.local.home

Ah, I forgot (was two years ago) But not doing FTrace still stands!

-- Steve

Re: [PATCH] usb: gadget: functions: add ftrace export over USB

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

Steven Rostedt <rostedt@goodmis.org> writes:

> On Fri, 22 Mar 2019 13:49:55 +0200
> Felipe Balbi <felipe.balbi@linux.intel.com> wrote:
>
>> > Can you post the latest to the list one more time. At least to make
>> > sure you got the naming correct ;-)
>> >
>> >   Not FTrace or f_trace.  
>> 
>> The reason for the name was discussed already. See [1] and [2]
>> 
>> [1] https://marc.info/?i=878tl1qmrj.fsf@linux.intel.com
>> [2] https://marc.info/?i=20170609110717.3304f6b9@gandalf.local.home
>
> Ah, I forgot (was two years ago) But not doing FTrace still stands!

Replaced with ftrace everywhere now. I'll wait a little before a resend
(see [1] for some questions I have)

[1] https://marc.info/?i=87h8bvffth.fsf@linux.intel.com

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]