Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751724AbdFILQ1 (ORCPT ); Fri, 9 Jun 2017 07:16:27 -0400 Received: from mga01.intel.com ([192.55.52.88]:7167 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751536AbdFILQZ (ORCPT ); Fri, 9 Jun 2017 07:16:25 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,317,1493708400"; d="asc'?scan'208";a="1158522485" From: Felipe Balbi To: Alexander Shishkin , Steven Rostedt , Ingo Molnar Cc: Linux USB , linux-kernel@vger.kernel.org, Chunyan Zhang Subject: Re: [PATCH] usb: gadget: functions: add ftrace export over USB In-Reply-To: <87mv9hqwt8.fsf@linux.intel.com> References: <20170609061327.17899-1-felipe.balbi@linux.intel.com> <87mv9hqwt8.fsf@linux.intel.com> Date: Fri, 09 Jun 2017 14:15:37 +0300 Message-ID: <87h8zpquna.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6149 Lines: 199 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Felipe Balbi writes: > Felipe Balbi 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 >> --- >> >> 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 { =20 struct usb_ep *in; =20 + 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)) =20 #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 =20 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); } =20 +static void ftrace_set_trace_buf_size_complete(struct usb_ep *ep, struct u= sb_request *req) +{ + struct usb_ftrace *trace =3D 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 =3D work_to_trace(work); @@ -237,6 +252,71 @@ static int ftrace_set_alt(struct usb_function *f, unsi= gned intf, unsigned alt) return -EINVAL; } =20 +extern unsigned long trace_get_buf_size(void); + +static int ftrace_setup(struct usb_function *f, const struct usb_ctrlreque= st *ctrl) +{ + struct usb_configuration *c =3D f->config; + struct usb_request *req =3D c->cdev->req; + struct usb_ftrace *trace =3D to_trace(f); + + int ret; + + u16 index =3D le16_to_cpu(ctrl->wIndex); + u16 value =3D le16_to_cpu(ctrl->wValue); + u16 length =3D le16_to_cpu(ctrl->wLength); + + if (value !=3D 0 || index !=3D 0) + return -EINVAL; + + switch (ctrl->bRequest) { + case USB_FTRACE_GET_VERSION: + if (ctrl->bRequestType !=3D (USB_DIR_IN | USB_TYPE_VENDOR | + USB_RECIP_INTERFACE)) + return -EINVAL; + + if (length !=3D 2) + return -EINVAL; + + req->zero =3D 0; + req->length =3D 2; + req->buf =3D &trace->version; + break; + case USB_FTRACE_GET_TRACE_BUF_SIZE: + if (ctrl->bRequestType !=3D (USB_DIR_IN | USB_TYPE_VENDOR | + USB_RECIP_INTERFACE)) + return -EINVAL; + + if (length !=3D 2) + return -EINVAL; + + trace->buffer_size =3D cpu_to_le32(trace_get_buf_size()); + + req->zero =3D 0; + req->length =3D 2; + req->buf =3D &trace->buffer_size; + break; + case USB_FTRACE_SET_TRACE_BUF_SIZE: + if (ctrl->bRequestType !=3D (USB_DIR_OUT | USB_TYPE_VENDOR | + USB_RECIP_INTERFACE)) + return -EINVAL; + + if (length !=3D 4) + return -EINVAL; + + req->zero =3D 0; + req->length =3D 4; + req->context =3D trace; + req->complete =3D ftrace_set_trace_buf_size_complete; + req->buf =3D &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 =3D c->cdev; @@ -247,6 +327,8 @@ static int ftrace_bind(struct usb_configuration *c, str= uct usb_function *f) int ret; int i; =20 + trace->version =3D cpu_to_le16(FTRACE_VERSION); + us =3D 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) =20 static unsigned long trace_buf_size =3D TRACE_BUF_SIZE_DEFAULT; =20 +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; =20 =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlk6g1kACgkQzL64meEa mQaanA//deckJNdUx/K2P2FiXwCXgZj6ZtgZiWl2/rri1FR1u2bZ85Hd+7AW/hAR h0y4KeQLTrdfRyCUje0WcRyKkmRr6qmIMvSn2USP4VgaKkvxpMiwrLeh7vnFCchT qbv8DQPp+cs4UBtGO4pUGpQSryB1tmqcIs7wDhOdfd5YWI/MMHH5hiU7ubU70Ri9 ucuJjL7aw9kRbzDA/u7T7koymENvj/K7jSOI5BWb9rxGg5AidZnovBOx8MNfuCMW EsUtYd7YulDmNS0jVlsnUI1E62Cy89aYUxLEoC1kNHk/jZU6XHd6ThYt9j9FJpFZ eHqIzvcmcurIjAZ6gJw1USr6hvUE8EhGCdD4MGcoSImBfph25bg5tq3OcgtCGoF+ eJxHUFNgnlamu1N4vqZw2T1uWLL6IR5WdsHL/53MahdEOr6Em7O55DpYTEDnCzB8 my/I2ewLsmVI3fh0AHBKTAcvSJevhQbBGsinQdF/P3vLJsaLbzvPQVRlMOwZzHLv PTUfVu8oBkAQoWqlZmuJKoOmYAqgk8WNWhhka48uDwspcUxdm7bUYDLnEQFplfDD 7kw9h249tyn4ellVMaUX+IeYnOV6Atf7WxGE8B4IdZ+7FpI1CSysSkkBCbnFwCgq iLyWK/BPVTVqmbCECAzjF2XPQrwn9qxupVLmtdJnY4calJJKfUo= =G2Ex -----END PGP SIGNATURE----- --=-=-=--