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

[Felipe Balbi <felipe.balbi@linux.intel.com>]

[-- 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 --]