Linux Documentation
 help / color / mirror / Atom feed
* [PATCH 3/3] MAINTAINERS: add USB CCID Gadget Device
From: Marcus Folkesson @ 2018-05-26 20:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel, Marcus Folkesson
In-Reply-To: <20180526203401.16586-1-marcus.folkesson@gmail.com>

Add MAINTAINERS entry for USB CCID Gadget Device

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 078fd80f664f..e77c3d2bec89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14541,6 +14541,14 @@ L:	linux-scsi@vger.kernel.org
 S:	Maintained
 F:	drivers/usb/storage/uas.c
 
+USB CCID GADGET
+M:	Marcus Folkesson <marcus.folkesson@gmail.com>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	drivers/usb/gadget/function/f_ccid.*
+F:	include/uapi/linux/usb/ccid.h
+F:	Documentation/usb/gadget_ccid.rst
+
 USB CDC ETHERNET DRIVER
 M:	Oliver Neukum <oliver@neukum.org>
 L:	linux-usb@vger.kernel.org
-- 
2.16.2

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
From: Marcus Folkesson @ 2018-05-26 20:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel, Marcus Folkesson

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/usb/gadget/Kconfig           |  17 +
 drivers/usb/gadget/function/Makefile |   1 +
 drivers/usb/gadget/function/f_ccid.c | 988 +++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/function/f_ccid.h |  91 ++++
 include/uapi/linux/usb/ccid.h        |  93 ++++
 5 files changed, 1190 insertions(+)
 create mode 100644 drivers/usb/gadget/function/f_ccid.c
 create mode 100644 drivers/usb/gadget/function/f_ccid.h
 create mode 100644 include/uapi/linux/usb/ccid.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 31cce7805eb2..bdebdf1ffa2b 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -149,6 +149,9 @@ config USB_LIBCOMPOSITE
 config USB_F_ACM
 	tristate
 
+config USB_F_CCID
+	tristate
+
 config USB_F_SS_LB
 	tristate
 
@@ -248,6 +251,20 @@ config USB_CONFIGFS_ACM
 	  ACM serial link.  This function can be used to interoperate with
 	  MS-Windows hosts or with the Linux-USB "cdc-acm" driver.
 
+config USB_CONFIGFS_CCID
+	bool "Chip Card Interface Device (CCID)"
+	depends on USB_CONFIGFS
+	select USB_F_CCID
+	help
+	  The CCID function driver provides generic emulation of a
+	  Chip Card Interface Device (CCID).
+
+	  You will need a user space server talking to /dev/ccidg*,
+	  since the kernel itself does not implement CCID/TPDU/APDU
+	  protocol.
+
+	  For more information, see Documentation/usb/gadget_ccid.rst.
+
 config USB_CONFIGFS_OBEX
 	bool "Object Exchange Model (CDC OBEX)"
 	depends on USB_CONFIGFS
diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile
index 5d3a6cf02218..629851009e1a 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -9,6 +9,7 @@ ccflags-y			+= -I$(srctree)/drivers/usb/gadget/udc/
 # USB Functions
 usb_f_acm-y			:= f_acm.o
 obj-$(CONFIG_USB_F_ACM)		+= usb_f_acm.o
+obj-$(CONFIG_USB_F_CCID)	+= f_ccid.o
 usb_f_ss_lb-y			:= f_loopback.o f_sourcesink.o
 obj-$(CONFIG_USB_F_SS_LB)	+= usb_f_ss_lb.o
 obj-$(CONFIG_USB_U_SERIAL)	+= u_serial.o
diff --git a/drivers/usb/gadget/function/f_ccid.c b/drivers/usb/gadget/function/f_ccid.c
new file mode 100644
index 000000000000..9ff8615ca303
--- /dev/null
+++ b/drivers/usb/gadget/function/f_ccid.c
@@ -0,0 +1,988 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * f_ccid.c -- Chip Card Interface Device (CCID) function Driver
+ *
+ * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
+ *
+ */
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/usb/composite.h>
+#include <uapi/linux/usb/ccid.h>
+
+#include "f_ccid.h"
+#include "u_f.h"
+
+/* Number of tx requests to allocate */
+#define N_TX_REQS 4
+
+/* Maximum number of devices */
+#define CCID_MINORS 4
+
+struct ccidg_bulk_dev {
+	atomic_t is_open;
+	atomic_t rx_req_busy;
+	wait_queue_head_t read_wq;
+	wait_queue_head_t write_wq;
+	struct usb_request *rx_req;
+	atomic_t rx_done;
+	struct list_head tx_idle;
+};
+
+struct f_ccidg {
+	struct usb_function_instance	func_inst;
+	struct usb_function function;
+	spinlock_t lock;
+	atomic_t online;
+
+	/* Character device */
+	struct cdev cdev;
+	int minor;
+
+	/* Dynamic attributes */
+	u32 features;
+	u32 protocols;
+	u8 pinsupport;
+	u8 nslots;
+	u8 lcdlayout;
+
+	/* Endpoints */
+	struct usb_ep *in;
+	struct usb_ep *out;
+	struct ccidg_bulk_dev bulk_dev;
+};
+
+/* Interface Descriptor: */
+static struct usb_interface_descriptor ccid_interface_desc = {
+	.bLength =		USB_DT_INTERFACE_SIZE,
+	.bDescriptorType =	USB_DT_INTERFACE,
+	.bNumEndpoints =	2,
+	.bInterfaceClass =	USB_CLASS_CSCID,
+	.bInterfaceSubClass =	0,
+	.bInterfaceProtocol =	0,
+};
+
+/* CCID Class Descriptor */
+static struct ccid_class_descriptor ccid_class_desc = {
+	.bLength =		sizeof(ccid_class_desc),
+	.bDescriptorType =	CCID_DECRIPTOR_TYPE,
+	.bcdCCID =		CCID1_10,
+	/* .bMaxSlotIndex =	DYNAMIC */
+	.bVoltageSupport =	CCID_VOLTS_3_0,
+	/* .dwProtocols =	DYNAMIC */
+	.dwDefaultClock =	3580,
+	.dwMaximumClock =	3580,
+	.bNumClockSupported =	0,
+	.dwDataRate =		9600,
+	.dwMaxDataRate =	9600,
+	.bNumDataRatesSupported = 0,
+	.dwMaxIFSD =		0,
+	.dwSynchProtocols =	0,
+	.dwMechanical =		0,
+	/* .dwFeatures =	DYNAMIC */
+
+	/* extended APDU level Message Length */
+	.dwMaxCCIDMessageLength = 0x200,
+	.bClassGetResponse =	0x0,
+	.bClassEnvelope =	0x0,
+	/* .wLcdLayout =	DYNAMIC */
+	/* .bPINSupport =	DYNAMIC */
+	.bMaxCCIDBusySlots =	1
+};
+
+/* Full speed support: */
+static struct usb_endpoint_descriptor ccid_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_endpoint_descriptor ccid_fs_out_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT,
+	.bEndpointAddress =	USB_DIR_OUT,
+	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize   =	 cpu_to_le16(64),
+};
+
+static struct usb_descriptor_header *ccid_fs_descs[] = {
+	(struct usb_descriptor_header *) &ccid_interface_desc,
+	(struct usb_descriptor_header *) &ccid_class_desc,
+	(struct usb_descriptor_header *) &ccid_fs_in_desc,
+	(struct usb_descriptor_header *) &ccid_fs_out_desc,
+	NULL,
+};
+
+/* High speed support: */
+static struct usb_endpoint_descriptor ccid_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_endpoint_descriptor ccid_hs_out_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT,
+	.bEndpointAddress =	USB_DIR_OUT,
+	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize =	cpu_to_le16(512),
+};
+
+static struct usb_descriptor_header *ccid_hs_descs[] = {
+	(struct usb_descriptor_header *) &ccid_interface_desc,
+	(struct usb_descriptor_header *) &ccid_class_desc,
+	(struct usb_descriptor_header *) &ccid_hs_in_desc,
+	(struct usb_descriptor_header *) &ccid_hs_out_desc,
+	NULL,
+};
+
+static DEFINE_IDA(ccidg_ida);
+static int major;
+static DEFINE_MUTEX(ccidg_ida_lock); /* protects access to ccidg_ida */
+static struct class *ccidg_class;
+
+static inline struct f_ccidg_opts *to_f_ccidg_opts(struct config_item *item)
+{
+	return container_of(to_config_group(item), struct f_ccidg_opts,
+			    func_inst.group);
+}
+
+static inline struct f_ccidg *func_to_ccidg(struct usb_function *f)
+{
+	return container_of(f, struct f_ccidg, function);
+}
+
+static inline int ccidg_get_minor(void)
+{
+	int ret;
+
+	ret = ida_simple_get(&ccidg_ida, 0, 0, GFP_KERNEL);
+	if (ret >= CCID_MINORS) {
+		ida_simple_remove(&ccidg_ida, ret);
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
+static inline void ccidg_put_minor(int minor)
+{
+	ida_simple_remove(&ccidg_ida, minor);
+}
+
+static int ccidg_setup(void)
+{
+	int ret;
+	dev_t dev;
+
+	ccidg_class = class_create(THIS_MODULE, "ccidg");
+	if (IS_ERR(ccidg_class)) {
+		ccidg_class = NULL;
+		return PTR_ERR(ccidg_class);
+	}
+
+	ret = alloc_chrdev_region(&dev, 0, CCID_MINORS, "ccidg");
+	if (ret) {
+		class_destroy(ccidg_class);
+		ccidg_class = NULL;
+		return ret;
+	}
+
+	major = MAJOR(dev);
+
+	return 0;
+}
+
+static void ccidg_cleanup(void)
+{
+	if (major) {
+		unregister_chrdev_region(MKDEV(major, 0), CCID_MINORS);
+		major = 0;
+	}
+
+	class_destroy(ccidg_class);
+	ccidg_class = NULL;
+}
+
+static void ccidg_attr_release(struct config_item *item)
+{
+	struct f_ccidg_opts *opts = to_f_ccidg_opts(item);
+
+	usb_put_function_instance(&opts->func_inst);
+}
+
+static struct configfs_item_operations ccidg_item_ops = {
+	.release	= ccidg_attr_release,
+};
+
+#define F_CCIDG_OPT(name, prec, limit)					\
+static ssize_t f_ccidg_opts_##name##_show(struct config_item *item, char *page)\
+{									\
+	struct f_ccidg_opts *opts = to_f_ccidg_opts(item);		\
+	int result;							\
+									\
+	mutex_lock(&opts->lock);					\
+	result = sprintf(page, "%x\n", opts->name);			\
+	mutex_unlock(&opts->lock);					\
+									\
+	return result;							\
+}									\
+									\
+static ssize_t f_ccidg_opts_##name##_store(struct config_item *item,	\
+					 const char *page, size_t len)	\
+{									\
+	struct f_ccidg_opts *opts = to_f_ccidg_opts(item);		\
+	int ret;							\
+	u##prec num;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->refcnt) {						\
+		ret = -EBUSY;						\
+		goto end;						\
+	}								\
+									\
+	ret = kstrtou##prec(page, 0, &num);				\
+	if (ret)							\
+		goto end;						\
+									\
+	if (num > limit) {						\
+		ret = -EINVAL;						\
+		goto end;						\
+	}								\
+	opts->name = num;						\
+	ret = len;							\
+									\
+end:									\
+	mutex_unlock(&opts->lock);					\
+	return ret;							\
+}									\
+									\
+CONFIGFS_ATTR(f_ccidg_opts_, name)
+
+F_CCIDG_OPT(features, 32, 0xffffffff);
+F_CCIDG_OPT(protocols, 32, 0x03);
+F_CCIDG_OPT(pinsupport, 8, 0x03);
+F_CCIDG_OPT(lcdlayout, 16, 0xffff);
+F_CCIDG_OPT(nslots, 8, 0xff);
+
+static struct configfs_attribute *ccidg_attrs[] = {
+	&f_ccidg_opts_attr_features,
+	&f_ccidg_opts_attr_protocols,
+	&f_ccidg_opts_attr_pinsupport,
+	&f_ccidg_opts_attr_lcdlayout,
+	&f_ccidg_opts_attr_nslots,
+	NULL,
+};
+
+static struct config_item_type ccidg_func_type = {
+	.ct_item_ops	= &ccidg_item_ops,
+	.ct_attrs	= ccidg_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static void ccidg_req_put(struct f_ccidg *ccidg, struct list_head *head,
+		struct usb_request *req)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ccidg->lock, flags);
+	list_add_tail(&req->list, head);
+	spin_unlock_irqrestore(&ccidg->lock, flags);
+}
+
+static struct usb_request *ccidg_req_get(struct f_ccidg *ccidg,
+					struct list_head *head)
+{
+	unsigned long flags;
+	struct usb_request *req = NULL;
+
+	spin_lock_irqsave(&ccidg->lock, flags);
+	if (!list_empty(head)) {
+		req = list_first_entry(head, struct usb_request, list);
+		list_del(&req->list);
+	}
+	spin_unlock_irqrestore(&ccidg->lock, flags);
+
+	return req;
+}
+
+static void ccidg_bulk_complete_tx(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_ccidg *ccidg = (struct f_ccidg *)ep->driver_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_composite_dev *cdev	= ccidg->function.config->cdev;
+
+	switch (req->status) {
+	default:
+		VDBG(cdev, "ccid: tx err %d\n", req->status);
+		/* FALLTHROUGH */
+	case -ECONNRESET:		/* unlink */
+	case -ESHUTDOWN:		/* disconnect etc */
+		break;
+	case 0:
+		break;
+	}
+
+	ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+	wake_up(&bulk_dev->write_wq);
+}
+
+static void ccidg_bulk_complete_rx(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_ccidg *ccidg = (struct f_ccidg *)ep->driver_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_composite_dev *cdev	= ccidg->function.config->cdev;
+
+	switch (req->status) {
+
+	/* normal completion */
+	case 0:
+		/* We only cares about packets with nonzero length */
+		if (req->actual > 0)
+			atomic_set(&bulk_dev->rx_done, 1);
+		break;
+
+	/* software-driven interface shutdown */
+	case -ECONNRESET:		/* unlink */
+	case -ESHUTDOWN:		/* disconnect etc */
+		VDBG(cdev, "ccid: rx shutdown, code %d\n", req->status);
+		break;
+
+	/* for hardware automagic (such as pxa) */
+	case -ECONNABORTED:		/* endpoint reset */
+		DBG(cdev, "ccid: rx %s reset\n", ep->name);
+		break;
+
+	/* data overrun */
+	case -EOVERFLOW:
+		/* FALLTHROUGH */
+	default:
+		DBG(cdev, "ccid: rx status %d\n", req->status);
+		break;
+	}
+
+	wake_up(&bulk_dev->read_wq);
+}
+
+static struct usb_request *
+ccidg_request_alloc(struct usb_ep *ep, unsigned int len)
+{
+	struct usb_request *req;
+
+	req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+	if (!req)
+		return ERR_PTR(-ENOMEM);
+
+	req->length = len;
+	req->buf = kmalloc(len, GFP_ATOMIC);
+	if (req->buf == NULL) {
+		usb_ep_free_request(ep, req);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	return req;
+}
+
+static void ccidg_request_free(struct usb_request *req, struct usb_ep *ep)
+{
+	if (req) {
+		kfree(req->buf);
+		usb_ep_free_request(ep, req);
+	}
+}
+
+static int ccidg_function_setup(struct usb_function *f,
+		const struct usb_ctrlrequest *ctrl)
+{
+	struct f_ccidg *ccidg = container_of(f, struct f_ccidg, function);
+	struct usb_composite_dev *cdev	= f->config->cdev;
+	struct usb_request *req		= cdev->req;
+	int ret				= -EOPNOTSUPP;
+	u16 w_index			= le16_to_cpu(ctrl->wIndex);
+	u16 w_value			= le16_to_cpu(ctrl->wValue);
+	u16 w_length			= le16_to_cpu(ctrl->wLength);
+
+	if (!atomic_read(&ccidg->online))
+		return -ENOTCONN;
+
+	switch (ctrl->bRequestType & USB_TYPE_MASK) {
+	case USB_TYPE_CLASS:
+		{
+		switch (ctrl->bRequest) {
+		case CCIDGENERICREQ_GET_CLOCK_FREQUENCIES:
+			*(u32 *) req->buf = cpu_to_le32(ccid_class_desc.dwDefaultClock);
+			ret = min_t(u32, w_length,
+					sizeof(ccid_class_desc.dwDefaultClock));
+			break;
+
+		case CCIDGENERICREQ_GET_DATA_RATES:
+			*(u32 *) req->buf = cpu_to_le32(ccid_class_desc.dwDataRate);
+			ret = min_t(u32, w_length, sizeof(ccid_class_desc.dwDataRate));
+			break;
+
+		default:
+			VDBG(f->config->cdev,
+				"ccid: invalid control req%02x.%02x v%04x i%04x l%d\n",
+				ctrl->bRequestType, ctrl->bRequest,
+				w_value, w_index, w_length);
+		}
+		}
+	}
+
+	/* responded with data transfer or status phase? */
+	if (ret >= 0) {
+		VDBG(f->config->cdev, "ccid: req%02x.%02x v%04x i%04x l%d\n",
+			ctrl->bRequestType, ctrl->bRequest,
+			w_value, w_index, w_length);
+
+		req->length = ret;
+		ret = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+		if (ret < 0)
+			ERROR(f->config->cdev,
+				"ccid: ep0 enqueue err %d\n", ret);
+	}
+
+	return ret;
+}
+
+static void ccidg_function_disable(struct usb_function *f)
+{
+	struct f_ccidg *ccidg = func_to_ccidg(f);
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_request *req;
+
+	/* Disable endpoints */
+	usb_ep_disable(ccidg->in);
+	usb_ep_disable(ccidg->out);
+
+	/* Free endpoint related requests */
+	if (!atomic_read(&bulk_dev->rx_req_busy))
+		ccidg_request_free(bulk_dev->rx_req, ccidg->out);
+	while ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle)))
+		ccidg_request_free(req, ccidg->in);
+
+	atomic_set(&ccidg->online, 0);
+
+	/* Wake up threads */
+	wake_up(&bulk_dev->write_wq);
+	wake_up(&bulk_dev->read_wq);
+}
+
+int ccidg_start_ep(struct f_ccidg *ccidg, struct usb_function *f,
+			struct usb_ep *ep)
+{
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int ret;
+
+	usb_ep_disable(ep);
+
+	ret = config_ep_by_speed(cdev->gadget, f, ep);
+	if (ret) {
+		ERROR(cdev, "ccid: can't configure %s: %d\n", ep->name, ret);
+		return ret;
+	}
+
+	ret = usb_ep_enable(ep);
+	if (ret) {
+		ERROR(cdev, "ccid: can't start %s: %d\n", ep->name, ret);
+		return ret;
+	}
+
+	ep->driver_data = ccidg;
+
+	return ret;
+}
+
+static int ccidg_function_set_alt(struct usb_function *f,
+		unsigned int intf, unsigned int alt)
+{
+	struct f_ccidg *ccidg		= func_to_ccidg(f);
+	struct usb_composite_dev *cdev	= f->config->cdev;
+	struct ccidg_bulk_dev *bulk_dev	= &ccidg->bulk_dev;
+	struct usb_request *req;
+	int ret;
+	int i;
+
+	/* Allocate requests for our endpoints */
+	req = ccidg_request_alloc(ccidg->out,
+			sizeof(struct ccidg_bulk_out_header));
+	if (IS_ERR(req)) {
+		ERROR(cdev, "ccid: uname to allocate memory for out req\n");
+		return PTR_ERR(req);
+	}
+	req->complete = ccidg_bulk_complete_rx;
+	req->context = ccidg;
+	bulk_dev->rx_req = req;
+
+	/* Allocate bunch of in requests */
+	for (i = 0; i < N_TX_REQS; i++) {
+		req = ccidg_request_alloc(ccidg->in,
+				sizeof(struct ccidg_bulk_in_header));
+
+		if (IS_ERR(req)) {
+			ret = PTR_ERR(req);
+			ERROR(cdev,
+				"ccid: uname to allocate memory for in req\n");
+			goto free_bulk_out;
+		}
+		req->complete = ccidg_bulk_complete_tx;
+		req->context = ccidg;
+		ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+	}
+
+	/* choose the descriptors and enable endpoints */
+	ret = ccidg_start_ep(ccidg, f, ccidg->in);
+	if (ret)
+		goto free_bulk_in;
+
+	ret = ccidg_start_ep(ccidg, f, ccidg->out);
+	if (ret)
+		goto disable_ep_in;
+
+	atomic_set(&ccidg->online, 1);
+	return ret;
+
+disable_ep_in:
+	usb_ep_disable(ccidg->in);
+free_bulk_in:
+	while ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle)))
+		ccidg_request_free(req, ccidg->in);
+free_bulk_out:
+	ccidg_request_free(bulk_dev->rx_req, ccidg->out);
+	return ret;
+}
+
+static int ccidg_bulk_open(struct inode *inode, struct file *file)
+{
+	struct f_ccidg *ccidg;
+	struct ccidg_bulk_dev *bulk_dev;
+
+	ccidg = container_of(inode->i_cdev, struct f_ccidg, cdev);
+	bulk_dev = &ccidg->bulk_dev;
+
+	if (!atomic_read(&ccidg->online)) {
+		DBG(ccidg->function.config->cdev, "ccid: device not online\n");
+		return -ENODEV;
+	}
+
+	if (atomic_read(&bulk_dev->is_open)) {
+		DBG(ccidg->function.config->cdev,
+				"ccid: device already opened\n");
+		return -EBUSY;
+	}
+
+	atomic_set(&bulk_dev->is_open, 1);
+
+	file->private_data = ccidg;
+
+	return 0;
+}
+
+static int ccidg_bulk_release(struct inode *inode, struct file *file)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+
+	atomic_set(&bulk_dev->is_open, 0);
+	return 0;
+}
+
+static ssize_t ccidg_bulk_read(struct file *file, char __user *buf,
+				size_t count, loff_t *pos)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_request *req;
+	int r = count, xfer;
+	int ret;
+
+	/* Make sure we have enough space for a whole package */
+	if (count < sizeof(struct ccidg_bulk_out_header)) {
+		DBG(ccidg->function.config->cdev,
+				"ccid: too small buffer size. %i provided, need at least %i\n",
+				count, sizeof(struct ccidg_bulk_out_header));
+		return -ENOMEM;
+	}
+
+	if (!atomic_read(&ccidg->online))
+		return -ENODEV;
+
+	/* queue a request */
+	req = bulk_dev->rx_req;
+	req->length = count;
+	atomic_set(&bulk_dev->rx_done, 0);
+
+	ret = usb_ep_queue(ccidg->out, req, GFP_KERNEL);
+	if (ret < 0) {
+		ERROR(ccidg->function.config->cdev,
+				"ccid: usb ep queue failed\n");
+		return -EIO;
+	}
+
+	if (!atomic_read(&bulk_dev->rx_done) &&
+			file->f_flags & (O_NONBLOCK | O_NDELAY))
+		return -EAGAIN;
+
+	/* wait for a request to complete */
+	ret = wait_event_interruptible(bulk_dev->read_wq,
+			atomic_read(&bulk_dev->rx_done) ||
+			!atomic_read(&ccidg->online));
+	if (ret < 0) {
+		usb_ep_dequeue(ccidg->out, req);
+		return -ERESTARTSYS;
+	}
+
+	/* Still online? */
+	if (!atomic_read(&ccidg->online))
+		return -ENODEV;
+
+	atomic_set(&bulk_dev->rx_req_busy, 1);
+	xfer = (req->actual < count) ? req->actual : count;
+
+	if (copy_to_user(buf, req->buf, xfer))
+		r = -EFAULT;
+
+	atomic_set(&bulk_dev->rx_req_busy, 0);
+	if (!atomic_read(&ccidg->online)) {
+		ccidg_request_free(bulk_dev->rx_req, ccidg->out);
+		return -ENODEV;
+	}
+
+	return xfer;
+}
+
+static ssize_t ccidg_bulk_write(struct file *file, const char __user *buf,
+				 size_t count, loff_t *pos)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_request *req = 0;
+	int ret;
+
+	/* Are we online? */
+	if (!atomic_read(&ccidg->online))
+		return -ENODEV;
+
+	/* Avoid Zero Length Packets (ZLP) */
+	if (!count)
+		return 0;
+
+	/* Make sure we have enough space for a whole package */
+	if (count > sizeof(struct ccidg_bulk_out_header)) {
+		DBG(ccidg->function.config->cdev,
+				"ccid: too much data. %i provided, but we can only handle %i\n",
+				count, sizeof(struct ccidg_bulk_out_header));
+		return -ENOMEM;
+	}
+
+	if (list_empty(&bulk_dev->tx_idle) &&
+			file->f_flags & (O_NONBLOCK | O_NDELAY))
+		return -EAGAIN;
+
+	/* get an idle tx request to use */
+	ret = wait_event_interruptible(bulk_dev->write_wq,
+		((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle))));
+
+	if (ret < 0)
+		return -ERESTARTSYS;
+
+	if (copy_from_user(req->buf, buf, count)) {
+		if (!atomic_read(&ccidg->online)) {
+			ccidg_request_free(req, ccidg->in);
+			return -ENODEV;
+		} else {
+			ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+			return -EFAULT;
+		}
+	}
+
+	req->length = count;
+	ret = usb_ep_queue(ccidg->in, req, GFP_KERNEL);
+	if (ret < 0) {
+		ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+
+		if (!atomic_read(&ccidg->online)) {
+			/* Free up all requests if we are not online */
+			while ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle)))
+				ccidg_request_free(req, ccidg->in);
+
+			return -ENODEV;
+		}
+		return -EIO;
+	}
+
+	return count;
+}
+
+static __poll_t ccidg_bulk_poll(struct file *file, poll_table * wait)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	__poll_t	ret = 0;
+
+	poll_wait(file, &bulk_dev->read_wq, wait);
+	poll_wait(file, &bulk_dev->write_wq, wait);
+
+	if (list_empty(&bulk_dev->tx_idle))
+		ret |= EPOLLOUT | EPOLLWRNORM;
+
+	if (atomic_read(&bulk_dev->rx_done))
+		ret |= EPOLLIN | EPOLLRDNORM;
+
+	return ret;
+}
+
+static const struct file_operations f_ccidg_fops = {
+	.owner = THIS_MODULE,
+	.read = ccidg_bulk_read,
+	.write = ccidg_bulk_write,
+	.open = ccidg_bulk_open,
+	.poll = ccidg_bulk_poll,
+	.release = ccidg_bulk_release,
+};
+
+static int ccidg_bulk_device_init(struct f_ccidg *dev)
+{
+	struct ccidg_bulk_dev *bulk_dev = &dev->bulk_dev;
+
+	init_waitqueue_head(&bulk_dev->read_wq);
+	init_waitqueue_head(&bulk_dev->write_wq);
+	INIT_LIST_HEAD(&bulk_dev->tx_idle);
+
+	return 0;
+}
+
+static void ccidg_function_free(struct usb_function *f)
+{
+	struct f_ccidg *ccidg;
+	struct f_ccidg_opts *opts;
+
+	ccidg = func_to_ccidg(f);
+	opts = container_of(f->fi, struct f_ccidg_opts, func_inst);
+
+	kfree(ccidg);
+	mutex_lock(&opts->lock);
+	--opts->refcnt;
+	mutex_unlock(&opts->lock);
+}
+
+static void ccidg_function_unbind(struct usb_configuration *c,
+					struct usb_function *f)
+{
+	struct f_ccidg *ccidg = func_to_ccidg(f);
+
+	device_destroy(ccidg_class, MKDEV(major, ccidg->minor));
+	cdev_del(&ccidg->cdev);
+
+	/* disable/free request and end point */
+	usb_free_all_descriptors(f);
+}
+
+static int ccidg_function_bind(struct usb_configuration *c,
+					struct usb_function *f)
+{
+	struct f_ccidg *ccidg = func_to_ccidg(f);
+	struct usb_ep *ep;
+	struct usb_composite_dev *cdev = c->cdev;
+	struct device *device;
+	dev_t dev;
+	int ifc_id;
+	int ret;
+
+	/* allocate instance-specific interface IDs, and patch descriptors */
+	ifc_id = usb_interface_id(c, f);
+	if (ifc_id < 0) {
+		ERROR(cdev, "ccid: unable to allocate ifc id, err:%d\n",
+				ifc_id);
+		return ifc_id;
+	}
+	ccid_interface_desc.bInterfaceNumber = ifc_id;
+
+	/* allocate instance-specific endpoints */
+	ep = usb_ep_autoconfig(cdev->gadget, &ccid_fs_in_desc);
+	if (!ep) {
+		ERROR(cdev, "ccid: usb epin autoconfig failed\n");
+		ret = -ENODEV;
+		goto ep_auto_in_fail;
+	}
+	ccidg->in = ep;
+	ep->driver_data = ccidg;
+
+	ep = usb_ep_autoconfig(cdev->gadget, &ccid_fs_out_desc);
+	if (!ep) {
+		ERROR(cdev, "ccid: usb epout autoconfig failed\n");
+		ret = -ENODEV;
+		goto ep_auto_out_fail;
+	}
+	ccidg->out = ep;
+	ep->driver_data = ccidg;
+
+	/* set descriptor dynamic values */
+	ccid_class_desc.dwFeatures	= cpu_to_le32(ccidg->features);
+	ccid_class_desc.bPINSupport	= ccidg->pinsupport;
+	ccid_class_desc.wLcdLayout	= cpu_to_le16(ccidg->lcdlayout);
+	ccid_class_desc.bMaxSlotIndex	= ccidg->nslots;
+	ccid_class_desc.dwProtocols	= cpu_to_le32(ccidg->protocols);
+
+	if (ccidg->protocols == CCID_PROTOCOL_NOT_SEL) {
+		ccidg->protocols = CCID_PROTOCOL_T0 | CCID_PROTOCOL_T1;
+		INFO(ccidg->function.config->cdev,
+			"ccid: No protocol selected. Support both T0 and T1.\n");
+	}
+
+
+	ccid_hs_in_desc.bEndpointAddress =
+			ccid_fs_in_desc.bEndpointAddress;
+	ccid_hs_out_desc.bEndpointAddress =
+			ccid_fs_out_desc.bEndpointAddress;
+
+	ret  = usb_assign_descriptors(f, ccid_fs_descs,
+			ccid_hs_descs, NULL, NULL);
+	if (ret)
+		goto ep_auto_out_fail;
+
+	/* create char device */
+	cdev_init(&ccidg->cdev, &f_ccidg_fops);
+	dev = MKDEV(major, ccidg->minor);
+	ret = cdev_add(&ccidg->cdev, dev, 1);
+	if (ret)
+		goto fail_free_descs;
+
+	device = device_create(ccidg_class, NULL, dev, NULL,
+			       "%s%d", "ccidg", ccidg->minor);
+	if (IS_ERR(device)) {
+		ret = PTR_ERR(device);
+		goto del;
+	}
+
+	return 0;
+
+del:
+	cdev_del(&ccidg->cdev);
+fail_free_descs:
+	usb_free_all_descriptors(f);
+ep_auto_out_fail:
+	ccidg->out->driver_data = NULL;
+	ccidg->out = NULL;
+ep_auto_in_fail:
+	ccidg->in->driver_data = NULL;
+	ccidg->in = NULL;
+	ERROR(f->config->cdev, "ccidg_bind FAILED\n");
+
+	return ret;
+}
+
+static struct usb_function *ccidg_alloc(struct usb_function_instance *fi)
+{
+	struct f_ccidg *ccidg;
+	struct f_ccidg_opts *opts;
+	int ret;
+
+	ccidg = kzalloc(sizeof(*ccidg), GFP_KERNEL);
+	if (!ccidg)
+		return ERR_PTR(-ENOMEM);
+
+	spin_lock_init(&ccidg->lock);
+
+	ret = ccidg_bulk_device_init(ccidg);
+	if (ret) {
+		kfree(ccidg);
+		return ERR_PTR(ret);
+	}
+
+	opts = container_of(fi, struct f_ccidg_opts, func_inst);
+
+	mutex_lock(&opts->lock);
+	++opts->refcnt;
+
+	ccidg->minor = opts->minor;
+	ccidg->features = opts->features;
+	ccidg->protocols = opts->protocols;
+	ccidg->pinsupport = opts->pinsupport;
+	ccidg->nslots = opts->nslots;
+	mutex_unlock(&opts->lock);
+
+	ccidg->function.name	= "ccid";
+	ccidg->function.bind	= ccidg_function_bind;
+	ccidg->function.unbind	= ccidg_function_unbind;
+	ccidg->function.set_alt	= ccidg_function_set_alt;
+	ccidg->function.disable	= ccidg_function_disable;
+	ccidg->function.setup	= ccidg_function_setup;
+	ccidg->function.free_func = ccidg_function_free;
+
+	return &ccidg->function;
+}
+
+static void ccidg_free_inst(struct usb_function_instance *f)
+{
+	struct f_ccidg_opts *opts;
+
+	opts = container_of(f, struct f_ccidg_opts, func_inst);
+	mutex_lock(&ccidg_ida_lock);
+
+	ccidg_put_minor(opts->minor);
+	if (ida_is_empty(&ccidg_ida))
+		ccidg_cleanup();
+
+	mutex_unlock(&ccidg_ida_lock);
+
+	kfree(opts);
+}
+
+static struct usb_function_instance *ccidg_alloc_inst(void)
+{
+	struct f_ccidg_opts *opts;
+	struct usb_function_instance *ret;
+	int status = 0;
+
+	opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+	if (!opts)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_init(&opts->lock);
+	opts->func_inst.free_func_inst = ccidg_free_inst;
+	ret = &opts->func_inst;
+
+	mutex_lock(&ccidg_ida_lock);
+
+	if (ida_is_empty(&ccidg_ida)) {
+		status = ccidg_setup();
+		if (status)  {
+			ret = ERR_PTR(status);
+			kfree(opts);
+			goto unlock;
+		}
+	}
+
+	opts->minor = ccidg_get_minor();
+	if (opts->minor < 0) {
+		ret = ERR_PTR(opts->minor);
+		kfree(opts);
+		if (ida_is_empty(&ccidg_ida))
+			ccidg_cleanup();
+		goto unlock;
+	}
+
+	config_group_init_type_name(&opts->func_inst.group,
+			"", &ccidg_func_type);
+
+unlock:
+	mutex_unlock(&ccidg_ida_lock);
+	return ret;
+}
+
+DECLARE_USB_FUNCTION_INIT(ccid, ccidg_alloc_inst, ccidg_alloc);
+
+MODULE_DESCRIPTION("USB CCID Gadget driver");
+MODULE_AUTHOR("Marcus Folkesson <marcus.folkesson@gmail.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/usb/gadget/function/f_ccid.h b/drivers/usb/gadget/function/f_ccid.h
new file mode 100644
index 000000000000..9dc182d1f819
--- /dev/null
+++ b/drivers/usb/gadget/function/f_ccid.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
+ */
+
+#ifndef F_CCID_H
+#define F_CCID_H
+
+#define CCID1_10                0x0110
+#define CCID_DECRIPTOR_TYPE     0x21
+#define ABDATA_SIZE		512
+#define SMART_CARD_DEVICE_CLASS	0x0B
+
+/* CCID Class Specific Request */
+#define CCIDGENERICREQ_ABORT                    0x01
+#define CCIDGENERICREQ_GET_CLOCK_FREQUENCIES    0x02
+#define CCIDGENERICREQ_GET_DATA_RATES           0x03
+
+/* Supported voltages */
+#define CCID_VOLTS_AUTO                             0x00
+#define CCID_VOLTS_5_0                              0x01
+#define CCID_VOLTS_3_0                              0x02
+#define CCID_VOLTS_1_8                              0x03
+
+struct f_ccidg_opts {
+	struct usb_function_instance func_inst;
+	int	minor;
+	__u32	features;
+	__u32	protocols;
+	__u8	pinsupport;
+	__u8	nslots;
+	__u8	lcdlayout;
+
+	/*
+	 * Protect the data form concurrent access by read/write
+	 * and create symlink/remove symlink.
+	 */
+	struct mutex	lock;
+	int		refcnt;
+};
+
+struct ccidg_bulk_in_header {
+	__u8	bMessageType;
+	__u32	wLength;
+	__u8	bSlot;
+	__u8	bSeq;
+	__u8	bStatus;
+	__u8	bError;
+	__u8	bSpecific;
+	__u8	abData[ABDATA_SIZE];
+	__u8	bSizeToSend;
+} __packed;
+
+struct ccidg_bulk_out_header {
+	__u8	 bMessageType;
+	__u32	 wLength;
+	__u8	 bSlot;
+	__u8	 bSeq;
+	__u8	 bSpecific_0;
+	__u8	 bSpecific_1;
+	__u8	 bSpecific_2;
+	__u8	 APDU[ABDATA_SIZE];
+} __packed;
+
+struct ccid_class_descriptor {
+	__u8	bLength;
+	__u8	bDescriptorType;
+	__u16	bcdCCID;
+	__u8	bMaxSlotIndex;
+	__u8	bVoltageSupport;
+	__u32	dwProtocols;
+	__u32	dwDefaultClock;
+	__u32	dwMaximumClock;
+	__u8	bNumClockSupported;
+	__u32	dwDataRate;
+	__u32	dwMaxDataRate;
+	__u8	bNumDataRatesSupported;
+	__u32	dwMaxIFSD;
+	__u32	dwSynchProtocols;
+	__u32	dwMechanical;
+	__u32	dwFeatures;
+	__u32	dwMaxCCIDMessageLength;
+	__u8	bClassGetResponse;
+	__u8	bClassEnvelope;
+	__u16	wLcdLayout;
+	__u8	bPINSupport;
+	__u8	bMaxCCIDBusySlots;
+} __packed;
+
+
+#endif
diff --git a/include/uapi/linux/usb/ccid.h b/include/uapi/linux/usb/ccid.h
new file mode 100644
index 000000000000..517897201563
--- /dev/null
+++ b/include/uapi/linux/usb/ccid.h
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
+ *
+ * This file holds USB constants defined by the CCID Specification.
+ */
+
+#ifndef CCID_H
+#define CCID_H
+
+/* Slot error register when bmCommandStatus = 1 */
+#define CCID_CMD_ABORTED                            0xFF
+#define CCID_ICC_MUTE                               0xFE
+#define CCID_XFR_PARITY_ERROR                       0xFD
+#define CCID_XFR_OVERRUN                            0xFC
+#define CCID_HW_ERROR                               0xFB
+#define CCID_BAD_ATR_TS                             0xF8
+#define CCID_BAD_ATR_TCK                            0xF7
+#define CCID_ICC_PROTOCOL_NOT_SUPPORTED             0xF6
+#define CCID_ICC_CLASS_NOT_SUPPORTED                0xF5
+#define CCID_PROCEDURE_BYTE_CONFLICT                0xF4
+#define CCID_DEACTIVATED_PROTOCOL                   0xF3
+#define CCID_BUSY_WITH_AUTO_SEQUENCE                0xF2
+#define CCID_PIN_TIMEOUT                            0xF0
+#define CCID_PIN_CANCELLED                          0xEF
+#define CCID_CMD_SLOT_BUSY                          0xE0
+
+/* PC to RDR messages (bulk out) */
+#define CCID_PC_TO_RDR_ICCPOWERON                   0x62
+#define CCID_PC_TO_RDR_ICCPOWEROFF                  0x63
+#define CCID_PC_TO_RDR_GETSLOTSTATUS                0x65
+#define CCID_PC_TO_RDR_XFRBLOCK                     0x6F
+#define CCID_PC_TO_RDR_GETPARAMETERS                0x6C
+#define CCID_PC_TO_RDR_RESETPARAMETERS              0x6D
+#define CCID_PC_TO_RDR_SETPARAMETERS                0x61
+#define CCID_PC_TO_RDR_ESCAPE                       0x6B
+#define CCID_PC_TO_RDR_ICCCLOCK                     0x6E
+#define CCID_PC_TO_RDR_T0APDU                       0x6A
+#define CCID_PC_TO_RDR_SECURE                       0x69
+#define CCID_PC_TO_RDR_MECHANICAL                   0x71
+#define CCID_PC_TO_RDR_ABORT                        0x72
+#define CCID_PC_TO_RDR_SETDATARATEANDCLOCKFREQUENCY 0x73
+
+/* RDR to PC messages (bulk in) */
+#define CCID_RDR_TO_PC_DATABLOCK                    0x80
+#define CCID_RDR_TO_PC_SLOTSTATUS                   0x81
+#define CCID_RDR_TO_PC_PARAMETERS                   0x82
+#define CCID_RDR_TO_PC_ESCAPE                       0x83
+#define CCID_RDR_TO_PC_DATARATEANDCLOCKFREQUENCY    0x84
+
+/* Class Features */
+
+/* No special characteristics */
+#define CCID_FEATURES_NADA       0x00000000
+/* Automatic parameter configuration based on ATR data */
+#define CCID_FEATURES_AUTO_PCONF 0x00000002
+/* Automatic activation of ICC on inserting */
+#define CCID_FEATURES_AUTO_ACTIV 0x00000004
+/* Automatic ICC voltage selection */
+#define CCID_FEATURES_AUTO_VOLT  0x00000008
+/* Automatic ICC clock frequency change */
+#define CCID_FEATURES_AUTO_CLOCK 0x00000010
+/* Automatic baud rate change */
+#define CCID_FEATURES_AUTO_BAUD  0x00000020
+/*Automatic parameters negotiation made by the CCID */
+#define CCID_FEATURES_AUTO_PNEGO 0x00000040
+/* Automatic PPS made by the CCID according to the active parameters */
+#define CCID_FEATURES_AUTO_PPS   0x00000080
+/* CCID can set ICC in clock stop mode */
+#define CCID_FEATURES_ICCSTOP    0x00000100
+/* NAD value other than 00 accepted (T=1 protocol in use) */
+#define CCID_FEATURES_NAD        0x00000200
+/* Automatic IFSD exchange as first exchange (T=1 protocol in use) */
+#define CCID_FEATURES_AUTO_IFSD  0x00000400
+/* TPDU level exchanges with CCID */
+#define CCID_FEATURES_EXC_TPDU   0x00010000
+/* Short APDU level exchange with CCID */
+#define CCID_FEATURES_EXC_SAPDU  0x00020000
+/* Short and Extended APDU level exchange with CCID */
+#define CCID_FEATURES_EXC_APDU   0x00040000
+/* USB Wake up signaling supported on card insertion and removal */
+#define CCID_FEATURES_WAKEUP	0x00100000
+
+/* Supported protocols */
+#define CCID_PROTOCOL_NOT_SEL	0x00
+#define CCID_PROTOCOL_T0	0x01
+#define CCID_PROTOCOL_T1	0x02
+
+#define CCID_PINSUPOORT_NONE		0x00
+#define CCID_PINSUPOORT_VERIFICATION	(1 << 1)
+#define CCID_PINSUPOORT_MODIFICATION	(1 << 2)
+
+#endif
-- 
2.16.2

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH RESEND] Documentation: filesystems: update filesystem locking documentation
From: Al Viro @ 2018-05-26  3:41 UTC (permalink / raw)
  To: Sean Anderson
  Cc: corbet, linux-doc, linux-fsdevel, Matthew Wilcox, Jeff Layton
In-Reply-To: <e4262947-a2c3-c932-caef-67d96fa59cb1@gmail.com>

On Wed, May 23, 2018 at 10:29:10PM -0400, Sean Anderson wrote:
> Documentation/filesystems/Locking no longer reflects current locking
> semantics. i_mutex is no longer used for locking, and has been superseded
> by i_rwsem. Additionally, ->iterate_shared() was not documented.

Your mission, should you choose to accept it, shall be to locate the
old sig regarding the usual reaction to use of Quoted-Printable...

IOW, fix your mail setup.  Applied, but...
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/3] bpf: add boot parameters for sysctl knobs
From: Alexei Starovoitov @ 2018-05-25 19:44 UTC (permalink / raw)
  To: Eugene Syromiatnikov
  Cc: Jesper Dangaard Brouer, netdev, linux-kernel, linux-doc,
	Kees Cook, Kai-Heng Feng, Daniel Borkmann, Alexei Starovoitov,
	Jonathan Corbet, Jiri Olsa
In-Reply-To: <20180525165009.GA20952@asgard.redhat.com>

On Fri, May 25, 2018 at 06:50:09PM +0200, Eugene Syromiatnikov wrote:
> On Thu, May 24, 2018 at 04:34:51PM -0700, Alexei Starovoitov wrote:
> > On Thu, May 24, 2018 at 09:41:08AM +0200, Jesper Dangaard Brouer wrote:
> > > On Wed, 23 May 2018 15:02:45 -0700
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > 
> > > > On Wed, May 23, 2018 at 02:18:19PM +0200, Eugene Syromiatnikov wrote:
> > > > > Some BPF sysctl knobs affect the loading of BPF programs, and during
> > > > > system boot/init stages these sysctls are not yet configured.
> > > > > A concrete example is systemd, that has implemented loading of BPF
> > > > > programs.
> > > > > 
> > > > > Thus, to allow controlling these setting at early boot, this patch set
> > > > > adds the ability to change the default setting of these sysctl knobs
> > > > > as well as option to override them via a boot-time kernel parameter
> > > > > (in order to avoid rebuilding kernel each time a need of changing these
> > > > > defaults arises).
> > > > > 
> > > > > The sysctl knobs in question are kernel.unprivileged_bpf_disable,
> > > > > net.core.bpf_jit_harden, and net.core.bpf_jit_kallsyms.  
> > > > 
> > > > - systemd is root. today it only uses cgroup-bpf progs which require root,
> > > >   so disabling unpriv during boot time makes no difference to systemd.
> > > >   what is the actual reason to present time?
> systemd also runs a lot of code, some of which is unprivileged.

systemd processes sysctl configs first. It's essential for system
security to do so. If you have concerns in how systemd does that
please bring it up with systemd folks.

> > > > - say in the future systemd wants to use so_reuseport+bpf for faster
> > > >   networking. With unpriv disable during boot, it will force systemd
> > > >   to do such networking from root, which will lower its security barrier.
> No, it will force systemd not to use SO_REUSEPORT BPF.

sorry this argument makes no sense to me.

> > > > - bpf_jit_kallsyms sysctl has immediate effect on loaded programs.
> > > >   Flipping it during the boot or right after or any time after
> > > >   is the same thing. Why add such boot flag then?
> Well, that one was for completeness.

Should we convert all sysctls to bootparams for 'completeness' ?

> > > > - jit_harden can be turned on by systemd. so turning it during the boot
> > > >   will make systemd progs to be constant blinded.
> > > >   Constant blinding protects kernel from unprivileged JIT spraying.
> > > >   Are you worried that systemd will attack the kernel with JIT spraying?
> I'm worried that systemd can be exploited for a JIT spraying attack.

I'm afraid we're not on the same page with definition of 'JIT spraying attack'.

> Another thing I'm concerned with is that the generated code is different,
> which introduces additional complication during debugging.

specifically what kind of complication?

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V3 2/6] hwmon: Add support for RPi voltage sensor
From: Guenter Roeck @ 2018-05-25 19:37 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Jean Delvare, Jonathan Corbet, Eric Anholt, Florian Fainelli,
	Ray Jui, Scott Branden, Phil Elwell, bcm-kernel-feedback-list,
	linux-arm-kernel, linux-rpi-kernel, linux-hwmon, linux-doc,
	Noralf Trønnes
In-Reply-To: <1527276279-23876-3-git-send-email-stefan.wahren@i2se.com>

On Fri, May 25, 2018 at 09:24:35PM +0200, Stefan Wahren wrote:
> Currently there is no easy way to detect undervoltage conditions on a
> remote Raspberry Pi. This hwmon driver retrieves the state of the
> undervoltage sensor via mailbox interface. The handling based on
> Noralf's modifications to the downstream firmware driver. In case of
> an undervoltage condition only an entry is written to the kernel log.
> 
> CC: "Noralf Trønnes" <noralf@tronnes.org>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

... assuming this will go through some arm tree.

> ---
>  Documentation/hwmon/raspberrypi-hwmon |  22 +++++
>  drivers/hwmon/Kconfig                 |  10 ++
>  drivers/hwmon/Makefile                |   1 +
>  drivers/hwmon/raspberrypi-hwmon.c     | 166 ++++++++++++++++++++++++++++++++++
>  4 files changed, 199 insertions(+)
>  create mode 100644 Documentation/hwmon/raspberrypi-hwmon
>  create mode 100644 drivers/hwmon/raspberrypi-hwmon.c
> 
> diff --git a/Documentation/hwmon/raspberrypi-hwmon b/Documentation/hwmon/raspberrypi-hwmon
> new file mode 100644
> index 0000000..3c92e2c
> --- /dev/null
> +++ b/Documentation/hwmon/raspberrypi-hwmon
> @@ -0,0 +1,22 @@
> +Kernel driver raspberrypi-hwmon
> +===============================
> +
> +Supported boards:
> +  * Raspberry Pi A+ (via GPIO on SoC)
> +  * Raspberry Pi B+ (via GPIO on SoC)
> +  * Raspberry Pi 2 B (via GPIO on SoC)
> +  * Raspberry Pi 3 B (via GPIO on port expander)
> +  * Raspberry Pi 3 B+ (via PMIC)
> +
> +Author: Stefan Wahren <stefan.wahren@i2se.com>
> +
> +Description
> +-----------
> +
> +This driver periodically polls a mailbox property of the VC4 firmware to detect
> +undervoltage conditions.
> +
> +Sysfs entries
> +-------------
> +
> +in0_lcrit_alarm		Undervoltage alarm
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index f10840a..fdaab82 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1298,6 +1298,16 @@ config SENSORS_PWM_FAN
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called pwm-fan.
>  
> +config SENSORS_RASPBERRYPI_HWMON
> +	tristate "Raspberry Pi voltage monitor"
> +	depends on RASPBERRYPI_FIRMWARE || COMPILE_TEST
> +	help
> +	  If you say yes here you get support for voltage sensor on the
> +	  Raspberry Pi.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called raspberrypi-hwmon.
> +
>  config SENSORS_SHT15
>  	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
>  	depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a3..a929770 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -141,6 +141,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
>  obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
>  obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
> +obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
>  obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>  obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
>  obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> new file mode 100644
> index 0000000..fb4e4a6
> --- /dev/null
> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Raspberry Pi voltage sensor driver
> + *
> + * Based on firmware/raspberrypi.c by Noralf Trønnes
> + *
> + * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> + */
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
> +#define UNDERVOLTAGE_STICKY_BIT	BIT(16)
> +
> +struct rpi_hwmon_data {
> +	struct device *hwmon_dev;
> +	struct rpi_firmware *fw;
> +	u32 last_throttled;
> +	struct delayed_work get_values_poll_work;
> +};
> +
> +static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
> +{
> +	u32 new_uv, old_uv, value;
> +	int ret;
> +
> +	/* Request firmware to clear sticky bits */
> +	value = 0xffff;
> +
> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> +				    &value, sizeof(value));
> +	if (ret) {
> +		dev_err_once(data->hwmon_dev, "Failed to get throttled (%d)\n",
> +			     ret);
> +		return;
> +	}
> +
> +	new_uv = value & UNDERVOLTAGE_STICKY_BIT;
> +	old_uv = data->last_throttled & UNDERVOLTAGE_STICKY_BIT;
> +	data->last_throttled = value;
> +
> +	if (new_uv == old_uv)
> +		return;
> +
> +	if (new_uv)
> +		dev_crit(data->hwmon_dev, "Undervoltage detected!\n");
> +	else
> +		dev_info(data->hwmon_dev, "Voltage normalised\n");
> +
> +	sysfs_notify(&data->hwmon_dev->kobj, NULL, "in0_lcrit_alarm");
> +}
> +
> +static void get_values_poll(struct work_struct *work)
> +{
> +	struct rpi_hwmon_data *data;
> +
> +	data = container_of(work, struct rpi_hwmon_data,
> +			    get_values_poll_work.work);
> +
> +	rpi_firmware_get_throttled(data);
> +
> +	/*
> +	 * We can't run faster than the sticky shift (100ms) since we get
> +	 * flipping in the sticky bits that are cleared.
> +	 */
> +	schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> +}
> +
> +static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
> +		    u32 attr, int channel, long *val)
> +{
> +	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
> +
> +	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> +	return 0;
> +}
> +
> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> +			      u32 attr, int channel)
> +{
> +	return 0444;
> +}
> +
> +static const u32 rpi_in_config[] = {
> +	HWMON_I_LCRIT_ALARM,
> +	0
> +};
> +
> +static const struct hwmon_channel_info rpi_in = {
> +	.type = hwmon_in,
> +	.config = rpi_in_config,
> +};
> +
> +static const struct hwmon_channel_info *rpi_info[] = {
> +	&rpi_in,
> +	NULL
> +};
> +
> +static const struct hwmon_ops rpi_hwmon_ops = {
> +	.is_visible = rpi_is_visible,
> +	.read = rpi_read,
> +};
> +
> +static const struct hwmon_chip_info rpi_chip_info = {
> +	.ops = &rpi_hwmon_ops,
> +	.info = rpi_info,
> +};
> +
> +static int rpi_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rpi_hwmon_data *data;
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	/* Parent driver assure that firmware is correct */
> +	data->fw = dev_get_drvdata(dev->parent);
> +
> +	/* Init throttled */
> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> +				    &data->last_throttled,
> +				    sizeof(data->last_throttled));
> +
> +	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
> +							       data,
> +							       &rpi_chip_info,
> +							       NULL);
> +
> +	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
> +	platform_set_drvdata(pdev, data);
> +
> +	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
> +		schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> +
> +	return PTR_ERR_OR_ZERO(data->hwmon_dev);
> +}
> +
> +static int rpi_hwmon_remove(struct platform_device *pdev)
> +{
> +	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
> +
> +	cancel_delayed_work_sync(&data->get_values_poll_work);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rpi_hwmon_driver = {
> +	.probe = rpi_hwmon_probe,
> +	.remove = rpi_hwmon_remove,
> +	.driver = {
> +		.name = "raspberrypi-hwmon",
> +	},
> +};
> +module_platform_driver(rpi_hwmon_driver);
> +
> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> +MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V3 3/6] firmware: raspberrypi: Register hwmon driver
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren
In-Reply-To: <1527276279-23876-1-git-send-email-stefan.wahren@i2se.com>

Since the raspberrypi-hwmon driver is tied to the VC4 firmware instead of
particular hardware its registration should be in the firmware driver.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/firmware/raspberrypi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 6692888f..0602626 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -21,6 +21,8 @@
 #define MBOX_DATA28(msg)		((msg) & ~0xf)
 #define MBOX_CHAN_PROPERTY		8
 
+static struct platform_device *rpi_hwmon;
+
 struct rpi_firmware {
 	struct mbox_client cl;
 	struct mbox_chan *chan; /* The property channel. */
@@ -183,6 +185,20 @@ rpi_firmware_print_firmware_revision(struct rpi_firmware *fw)
 	}
 }
 
+static void
+rpi_register_hwmon_driver(struct device *dev, struct rpi_firmware *fw)
+{
+	u32 packet;
+	int ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_THROTTLED,
+					&packet, sizeof(packet));
+
+	if (ret)
+		return;
+
+	rpi_hwmon = platform_device_register_data(dev, "raspberrypi-hwmon",
+						  -1, NULL, 0);
+}
+
 static int rpi_firmware_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -209,6 +225,7 @@ static int rpi_firmware_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, fw);
 
 	rpi_firmware_print_firmware_revision(fw);
+	rpi_register_hwmon_driver(dev, fw);
 
 	return 0;
 }
@@ -217,6 +234,8 @@ static int rpi_firmware_remove(struct platform_device *pdev)
 {
 	struct rpi_firmware *fw = platform_get_drvdata(pdev);
 
+	platform_device_unregister(rpi_hwmon);
+	rpi_hwmon = NULL;
 	mbox_free_channel(fw->chan);
 
 	return 0;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V3 2/6] hwmon: Add support for RPi voltage sensor
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren, Noralf Trønnes
In-Reply-To: <1527276279-23876-1-git-send-email-stefan.wahren@i2se.com>

Currently there is no easy way to detect undervoltage conditions on a
remote Raspberry Pi. This hwmon driver retrieves the state of the
undervoltage sensor via mailbox interface. The handling based on
Noralf's modifications to the downstream firmware driver. In case of
an undervoltage condition only an entry is written to the kernel log.

CC: "Noralf Trønnes" <noralf@tronnes.org>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 Documentation/hwmon/raspberrypi-hwmon |  22 +++++
 drivers/hwmon/Kconfig                 |  10 ++
 drivers/hwmon/Makefile                |   1 +
 drivers/hwmon/raspberrypi-hwmon.c     | 166 ++++++++++++++++++++++++++++++++++
 4 files changed, 199 insertions(+)
 create mode 100644 Documentation/hwmon/raspberrypi-hwmon
 create mode 100644 drivers/hwmon/raspberrypi-hwmon.c

diff --git a/Documentation/hwmon/raspberrypi-hwmon b/Documentation/hwmon/raspberrypi-hwmon
new file mode 100644
index 0000000..3c92e2c
--- /dev/null
+++ b/Documentation/hwmon/raspberrypi-hwmon
@@ -0,0 +1,22 @@
+Kernel driver raspberrypi-hwmon
+===============================
+
+Supported boards:
+  * Raspberry Pi A+ (via GPIO on SoC)
+  * Raspberry Pi B+ (via GPIO on SoC)
+  * Raspberry Pi 2 B (via GPIO on SoC)
+  * Raspberry Pi 3 B (via GPIO on port expander)
+  * Raspberry Pi 3 B+ (via PMIC)
+
+Author: Stefan Wahren <stefan.wahren@i2se.com>
+
+Description
+-----------
+
+This driver periodically polls a mailbox property of the VC4 firmware to detect
+undervoltage conditions.
+
+Sysfs entries
+-------------
+
+in0_lcrit_alarm		Undervoltage alarm
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f10840a..fdaab82 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1298,6 +1298,16 @@ config SENSORS_PWM_FAN
 	  This driver can also be built as a module.  If so, the module
 	  will be called pwm-fan.
 
+config SENSORS_RASPBERRYPI_HWMON
+	tristate "Raspberry Pi voltage monitor"
+	depends on RASPBERRYPI_FIRMWARE || COMPILE_TEST
+	help
+	  If you say yes here you get support for voltage sensor on the
+	  Raspberry Pi.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called raspberrypi-hwmon.
+
 config SENSORS_SHT15
 	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
 	depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a3..a929770 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -141,6 +141,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
 obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
+obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
 obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
new file mode 100644
index 0000000..fb4e4a6
--- /dev/null
+++ b/drivers/hwmon/raspberrypi-hwmon.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Raspberry Pi voltage sensor driver
+ *
+ * Based on firmware/raspberrypi.c by Noralf Trønnes
+ *
+ * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define UNDERVOLTAGE_STICKY_BIT	BIT(16)
+
+struct rpi_hwmon_data {
+	struct device *hwmon_dev;
+	struct rpi_firmware *fw;
+	u32 last_throttled;
+	struct delayed_work get_values_poll_work;
+};
+
+static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
+{
+	u32 new_uv, old_uv, value;
+	int ret;
+
+	/* Request firmware to clear sticky bits */
+	value = 0xffff;
+
+	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
+				    &value, sizeof(value));
+	if (ret) {
+		dev_err_once(data->hwmon_dev, "Failed to get throttled (%d)\n",
+			     ret);
+		return;
+	}
+
+	new_uv = value & UNDERVOLTAGE_STICKY_BIT;
+	old_uv = data->last_throttled & UNDERVOLTAGE_STICKY_BIT;
+	data->last_throttled = value;
+
+	if (new_uv == old_uv)
+		return;
+
+	if (new_uv)
+		dev_crit(data->hwmon_dev, "Undervoltage detected!\n");
+	else
+		dev_info(data->hwmon_dev, "Voltage normalised\n");
+
+	sysfs_notify(&data->hwmon_dev->kobj, NULL, "in0_lcrit_alarm");
+}
+
+static void get_values_poll(struct work_struct *work)
+{
+	struct rpi_hwmon_data *data;
+
+	data = container_of(work, struct rpi_hwmon_data,
+			    get_values_poll_work.work);
+
+	rpi_firmware_get_throttled(data);
+
+	/*
+	 * We can't run faster than the sticky shift (100ms) since we get
+	 * flipping in the sticky bits that are cleared.
+	 */
+	schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
+}
+
+static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
+		    u32 attr, int channel, long *val)
+{
+	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
+
+	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
+	return 0;
+}
+
+static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
+			      u32 attr, int channel)
+{
+	return 0444;
+}
+
+static const u32 rpi_in_config[] = {
+	HWMON_I_LCRIT_ALARM,
+	0
+};
+
+static const struct hwmon_channel_info rpi_in = {
+	.type = hwmon_in,
+	.config = rpi_in_config,
+};
+
+static const struct hwmon_channel_info *rpi_info[] = {
+	&rpi_in,
+	NULL
+};
+
+static const struct hwmon_ops rpi_hwmon_ops = {
+	.is_visible = rpi_is_visible,
+	.read = rpi_read,
+};
+
+static const struct hwmon_chip_info rpi_chip_info = {
+	.ops = &rpi_hwmon_ops,
+	.info = rpi_info,
+};
+
+static int rpi_hwmon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rpi_hwmon_data *data;
+	int ret;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	/* Parent driver assure that firmware is correct */
+	data->fw = dev_get_drvdata(dev->parent);
+
+	/* Init throttled */
+	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
+				    &data->last_throttled,
+				    sizeof(data->last_throttled));
+
+	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
+							       data,
+							       &rpi_chip_info,
+							       NULL);
+
+	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
+	platform_set_drvdata(pdev, data);
+
+	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
+		schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
+
+	return PTR_ERR_OR_ZERO(data->hwmon_dev);
+}
+
+static int rpi_hwmon_remove(struct platform_device *pdev)
+{
+	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&data->get_values_poll_work);
+
+	return 0;
+}
+
+static struct platform_driver rpi_hwmon_driver = {
+	.probe = rpi_hwmon_probe,
+	.remove = rpi_hwmon_remove,
+	.driver = {
+		.name = "raspberrypi-hwmon",
+	},
+};
+module_platform_driver(rpi_hwmon_driver);
+
+MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
+MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V3 5/6] ARM: multi_v7_defconfig: Enable RPi voltage sensor
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren
In-Reply-To: <1527276279-23876-1-git-send-email-stefan.wahren@i2se.com>

The patch enables the hwmon driver for the Raspberry Pi.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 7b2283a..2af46fd 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -449,6 +449,7 @@ CONFIG_SENSORS_LM90=y
 CONFIG_SENSORS_LM95245=y
 CONFIG_SENSORS_NTC_THERMISTOR=m
 CONFIG_SENSORS_PWM_FAN=m
+CONFIG_SENSORS_RASPBERRYPI_HWMON=m
 CONFIG_SENSORS_INA2XX=m
 CONFIG_CPU_THERMAL=y
 CONFIG_IMX_THERMAL=y
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V3 6/6] arm64: defconfig: Enable RPi voltage sensor
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren
In-Reply-To: <1527276279-23876-1-git-send-email-stefan.wahren@i2se.com>

The patch enables the hwmon driver for the Raspberry Pi.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 3299505..e5c7198 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -353,6 +353,7 @@ CONFIG_BATTERY_BQ27XXX=y
 CONFIG_SENSORS_ARM_SCPI=y
 CONFIG_SENSORS_LM90=m
 CONFIG_SENSORS_INA2XX=m
+CONFIG_SENSORS_RASPBERRYPI_HWMON=m
 CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
 CONFIG_CPU_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V3 4/6] ARM: bcm2835_defconfig: Enable RPi voltage sensor
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren
In-Reply-To: <1527276279-23876-1-git-send-email-stefan.wahren@i2se.com>

The patch enables the hwmon driver for the Raspberry Pi.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/configs/bcm2835_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig
index e4d188f..e9bc889 100644
--- a/arch/arm/configs/bcm2835_defconfig
+++ b/arch/arm/configs/bcm2835_defconfig
@@ -86,7 +86,7 @@ CONFIG_SPI=y
 CONFIG_SPI_BCM2835=y
 CONFIG_SPI_BCM2835AUX=y
 CONFIG_GPIO_SYSFS=y
-# CONFIG_HWMON is not set
+CONFIG_SENSORS_RASPBERRYPI_HWMON=m
 CONFIG_THERMAL=y
 CONFIG_BCM2835_THERMAL=y
 CONFIG_WATCHDOG=y
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V3 0/6] hwmon: Add support for Raspberry Pi voltage sensor
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren

A common issue for the Raspberry Pi is an inadequate power supply. 
Noralf Trønnes started a discussion [1] about writing such undervoltage
conditions into the kernel log.

Changes in V3:
- rebase
- simplify probing

Changes in V2:
- simplified Kconfig dependency suggested by Robin Murphy
- replace dt-binding by probing from firmware driver
- add hwmon documentation
- minor improvements suggested by Guenter Roeck

[1] - https://github.com/raspberrypi/linux/issues/2367

Stefan Wahren (6):
  ARM: bcm2835: Add GET_THROTTLED firmware property
  hwmon: Add support for RPi voltage sensor
  firmware: raspberrypi: Register hwmon driver
  ARM: bcm2835_defconfig: Enable RPi voltage sensor
  ARM: multi_v7_defconfig: Enable RPi voltage sensor
  arm64: defconfig: Enable RPi voltage sensor

 Documentation/hwmon/raspberrypi-hwmon      |  22 ++++
 arch/arm/configs/bcm2835_defconfig         |   2 +-
 arch/arm/configs/multi_v7_defconfig        |   1 +
 arch/arm64/configs/defconfig               |   1 +
 drivers/firmware/raspberrypi.c             |  19 ++++
 drivers/hwmon/Kconfig                      |  10 ++
 drivers/hwmon/Makefile                     |   1 +
 drivers/hwmon/raspberrypi-hwmon.c          | 166 +++++++++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h |   1 +
 9 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/hwmon/raspberrypi-hwmon
 create mode 100644 drivers/hwmon/raspberrypi-hwmon.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V3 1/6] ARM: bcm2835: Add GET_THROTTLED firmware property
From: Stefan Wahren @ 2018-05-25 19:24 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Eric Anholt
  Cc: Florian Fainelli, Ray Jui, Scott Branden, Phil Elwell,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-hwmon, linux-doc, Stefan Wahren
In-Reply-To: <1527276279-23876-1-git-send-email-stefan.wahren@i2se.com>

Recent Raspberry Pi firmware provides a mailbox property to detect
under-voltage conditions. Here is the current definition.

The u32 value returned by the firmware is divided into 2 parts:
  - lower 16-bits are the live value
  - upper 16-bits are the history or sticky value

  Bits:
  0: undervoltage
  1: arm frequency capped
  2: currently throttled
  16: undervoltage has occurred
  17: arm frequency capped has occurred
  18: throttling has occurred

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 8ee8991..c4a5c9e 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -75,6 +75,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_GET_EDID_BLOCK =                         0x00030020,
 	RPI_FIRMWARE_GET_CUSTOMER_OTP =                       0x00030021,
 	RPI_FIRMWARE_GET_DOMAIN_STATE =                       0x00030030,
+	RPI_FIRMWARE_GET_THROTTLED =                          0x00030046,
 	RPI_FIRMWARE_SET_CLOCK_STATE =                        0x00038001,
 	RPI_FIRMWARE_SET_CLOCK_RATE =                         0x00038002,
 	RPI_FIRMWARE_SET_VOLTAGE =                            0x00038003,
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 1/6] arm64: add type casts to untagged_addr macro
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

This patch makes the untagged_addr macro accept all kinds of address types
(void *, unsigned long, etc.) and allows not to specify type casts in each
place where it is used. This is done by using __typeof__.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index e66b0fca99c2..2d6451cbaa86 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -102,7 +102,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
  * up with a tagged userland pointer. Clear the tag to get a sane pointer to
  * pass on to access_ok(), for instance.
  */
-#define untagged_addr(addr)		sign_extend64(addr, 55)
+#define untagged_addr(addr)		\
+	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
 #define access_ok(type, addr, size)	__range_ok(addr, size)
 #define user_addr_max			get_fs
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 3/6] arm64: untag user addresses in access_ok and __uaccess_mask_ptr
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

copy_from_user (and a few other similar functions) are used to copy data
from user memory into the kernel memory or vice versa. Since a user can
provided a tagged pointer to one of the syscalls that use copy_from_user,
we need to correctly handle such pointers.

Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr,
before performing access validity checks.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/uaccess.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 2d6451cbaa86..fa7318d3d7d5 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -105,7 +105,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
 #define untagged_addr(addr)		\
 	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
-#define access_ok(type, addr, size)	__range_ok(addr, size)
+#define access_ok(type, addr, size)	\
+	__range_ok(untagged_addr(addr), size)
 #define user_addr_max			get_fs
 
 #define _ASM_EXTABLE(from, to)						\
@@ -237,7 +238,8 @@ static inline void uaccess_enable_not_uao(void)
 
 /*
  * Sanitise a uaccess pointer such that it becomes NULL if above the
- * current addr_limit.
+ * current addr_limit. In case the pointer is tagged (has the top byte set),
+ * untag the pointer before checking.
  */
 #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr)
 static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
@@ -245,10 +247,11 @@ static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
 	void __user *safe_ptr;
 
 	asm volatile(
-	"	bics	xzr, %1, %2\n"
+	"	bics	xzr, %3, %2\n"
 	"	csel	%0, %1, xzr, eq\n"
 	: "=&r" (safe_ptr)
-	: "r" (ptr), "r" (current_thread_info()->addr_limit)
+	: "r" (ptr), "r" (current_thread_info()->addr_limit),
+	  "r" (untagged_addr(ptr))
 	: "cc");
 
 	csdb();
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 4/6] mm, arm64: untag user addresses in mm/gup.c
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

mm/gup.c provides a kernel interface that accepts user addresses and
manipulates user pages directly (for example get_user_pages, that is used
by the futex syscall). Here we also need to handle the case of tagged user
pointers.

Add untagging to gup.c functions that use user pointers for vma lookup.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/gup.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index 541904a7c60f..5d0e9715bab7 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -650,6 +650,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
 	if (!nr_pages)
 		return 0;
 
+	start = untagged_addr(start);
+
 	VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
 
 	/*
@@ -804,6 +806,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
 	struct vm_area_struct *vma;
 	int ret, major = 0;
 
+	address = untagged_addr(address);
+
 	if (unlocked)
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY;
 
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 0/6] arm64: untag user pointers passed to the kernel
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya

arm64 has a feature called Top Byte Ignore, which allows to embed pointer
tags into the top byte of each pointer. Userspace programs (such as
HWASan, a memory debugging tool [1]) might use this feature and pass
tagged user pointers to the kernel through syscalls or other interfaces.

This patch makes a few of the kernel interfaces accept tagged user
pointers. The kernel is already able to handle user faults with tagged
pointers and has the untagged_addr macro, which this patchset reuses.

We're not trying to cover all possible ways the kernel accepts user
pointers in one patchset, so this one should be considered as a start.

Thanks!

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Changes in v3:
- Rebased onto e5c51f30 (4.17-rc6+).
- Added linux-arch@ to the list of recipients.

Changes in v2:
- Rebased onto 2d618bdf (4.17-rc3+).
- Removed excessive untagging in gup.c.
- Removed untagging pointers returned from __uaccess_mask_ptr.

Changes in v1:
- Rebased onto 4.17-rc1.

Changes in RFC v2:
- Added "#ifndef untagged_addr..." fallback in linux/uaccess.h instead of
  defining it for each arch individually.
- Updated Documentation/arm64/tagged-pointers.txt.
- Dropped “mm, arm64: untag user addresses in memory syscalls”.
- Rebased onto 3eb2ce82 (4.16-rc7).

Andrey Konovalov (6):
  arm64: add type casts to untagged_addr macro
  uaccess: add untagged_addr definition for other arches
  arm64: untag user addresses in access_ok and __uaccess_mask_ptr
  mm, arm64: untag user addresses in mm/gup.c
  lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
  arm64: update Documentation/arm64/tagged-pointers.txt

 Documentation/arm64/tagged-pointers.txt |  5 +++--
 arch/arm64/include/asm/uaccess.h        | 14 +++++++++-----
 include/linux/uaccess.h                 |  4 ++++
 lib/strncpy_from_user.c                 |  2 ++
 lib/strnlen_user.c                      |  2 ++
 mm/gup.c                                |  4 ++++
 6 files changed, 24 insertions(+), 7 deletions(-)

-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 5/6] lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

strncpy_from_user and strnlen_user accept user addresses as arguments, and
do not go through the same path as copy_from_user and others, so here we
need to handle the case of tagged user addresses separately.

Untag user pointers passed to these functions.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 lib/strncpy_from_user.c | 2 ++
 lib/strnlen_user.c      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index b53e1b5d80f4..97467cd2bc59 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -106,6 +106,8 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	src = untagged_addr(src);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)src;
 	if (likely(src_addr < max_addr)) {
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 60d0bbda8f5e..8b5f56466e00 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -108,6 +108,8 @@ long strnlen_user(const char __user *str, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	str = untagged_addr(str);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)str;
 	if (likely(src_addr < max_addr)) {
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 6/6] arm64: update Documentation/arm64/tagged-pointers.txt
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

Add a note that work on passing tagged user pointers to the kernel via
syscalls has started, but might not be complete yet.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 Documentation/arm64/tagged-pointers.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.txt b/Documentation/arm64/tagged-pointers.txt
index a25a99e82bb1..361481283f00 100644
--- a/Documentation/arm64/tagged-pointers.txt
+++ b/Documentation/arm64/tagged-pointers.txt
@@ -35,8 +35,9 @@ Using non-zero address tags in any of these locations may result in an
 error code being returned, a (fatal) signal being raised, or other modes
 of failure.
 
-For these reasons, passing non-zero address tags to the kernel via
-system calls is forbidden, and using a non-zero address tag for sp is
+Some initial work for supporting non-zero address tags passed to the
+kernel via system calls has been done, but the kernel doesn't provide
+any guarantees at this point. Using a non-zero address tag for sp is
 strongly discouraged.
 
 Programs maintaining a frame pointer and frame records that use non-zero
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 2/6] uaccess: add untagged_addr definition for other arches
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Robin Murphy, Al Viro,
	Andrey Konovalov, Kees Cook, Kate Stewart, Greg Kroah-Hartman,
	Andrew Morton, Ingo Molnar, Kirill A . Shutemov, linux-arm-kernel,
	linux-doc, linux-mm, linux-arch, linux-kernel
  Cc: Dmitry Vyukov, Kostya Serebryany, Evgeniy Stepanov, Lee Smith,
	Ramana Radhakrishnan, Jacob Bramley, Ruben Ayrapetyan,
	Chintan Pandya
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Define it as a noop for other architectures besides arm64.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 include/linux/uaccess.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index efe79c1cdd47..c045b4eff95e 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -13,6 +13,10 @@
 
 #include <asm/uaccess.h>
 
+#ifndef untagged_addr
+#define untagged_addr(addr) addr
+#endif
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and
-- 
2.17.0.921.gf22659ad46-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH bpf-next v2 0/3] bpf: add boot parameters for sysctl knobs
From: Eugene Syromiatnikov @ 2018-05-25 16:50 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jesper Dangaard Brouer, netdev, linux-kernel, linux-doc,
	Kees Cook, Kai-Heng Feng, Daniel Borkmann, Alexei Starovoitov,
	Jonathan Corbet, Jiri Olsa
In-Reply-To: <20180524233449.ga664pzexrkzepfv@ast-mbp>

On Thu, May 24, 2018 at 04:34:51PM -0700, Alexei Starovoitov wrote:
> On Thu, May 24, 2018 at 09:41:08AM +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 23 May 2018 15:02:45 -0700
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > 
> > > On Wed, May 23, 2018 at 02:18:19PM +0200, Eugene Syromiatnikov wrote:
> > > > Some BPF sysctl knobs affect the loading of BPF programs, and during
> > > > system boot/init stages these sysctls are not yet configured.
> > > > A concrete example is systemd, that has implemented loading of BPF
> > > > programs.
> > > > 
> > > > Thus, to allow controlling these setting at early boot, this patch set
> > > > adds the ability to change the default setting of these sysctl knobs
> > > > as well as option to override them via a boot-time kernel parameter
> > > > (in order to avoid rebuilding kernel each time a need of changing these
> > > > defaults arises).
> > > > 
> > > > The sysctl knobs in question are kernel.unprivileged_bpf_disable,
> > > > net.core.bpf_jit_harden, and net.core.bpf_jit_kallsyms.  
> > > 
> > > - systemd is root. today it only uses cgroup-bpf progs which require root,
> > >   so disabling unpriv during boot time makes no difference to systemd.
> > >   what is the actual reason to present time?
systemd also runs a lot of code, some of which is unprivileged.

> > > - say in the future systemd wants to use so_reuseport+bpf for faster
> > >   networking. With unpriv disable during boot, it will force systemd
> > >   to do such networking from root, which will lower its security barrier.
No, it will force systemd not to use SO_REUSEPORT BPF.

> > > - bpf_jit_kallsyms sysctl has immediate effect on loaded programs.
> > >   Flipping it during the boot or right after or any time after
> > >   is the same thing. Why add such boot flag then?
Well, that one was for completeness.

> > > - jit_harden can be turned on by systemd. so turning it during the boot
> > >   will make systemd progs to be constant blinded.
> > >   Constant blinding protects kernel from unprivileged JIT spraying.
> > >   Are you worried that systemd will attack the kernel with JIT spraying?
I'm worried that systemd can be exploited for a JIT spraying attack.

Another thing I'm concerned with is that the generated code is different,
which introduces additional complication during debugging.

> > I think you are missing that, we want the ability to change these
> > defaults in-order to avoid depending on /etc/sysctl.conf settings, and
> > that the these sysctl.conf setting happen too late.
> 
> What does it mean 'happens too late' ?
> Too late for what?
> sysctl.conf has plenty of system critical knobs like
> kernel.perf_event_paranoid, kernel.core_pattern, etc
> The behavior of the host is drastically different after sysctl config
> is applied.
> 
> > For example with jit_harden, there will be a difference between the
> > loaded BPF program that got loaded at boot-time with systemd (no
> > constant blinding) and when someone reloads that systemd service after
> > /etc/sysctl.conf have been evaluated and setting bpf_jit_harden (now
> > slower due to constant blinding).   This is inconsistent behavior.
> 
> net.core.bpf_jit_harden can be flipped back and forth at run-time,
> so bpf progs before and after will be either blinded or not.
> I don't see any inconsistency.

That can't be the reason to maintain that inconsistency.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFT v2 1/4] perf cs-etm: Generate sample for missed packets
From: Leo Yan @ 2018-05-25 15:54 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Robert Walker, Mathieu Poirier, Jonathan Corbet, Peter Zijlstra,
	Ingo Molnar, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-arm-kernel, linux-doc, linux-kernel, Tor Jeremiassen,
	mike.leach, kim.phillips, coresight, Mike Leach
In-Reply-To: <20180525152713.GD4091@kernel.org>

Hi Arnaldo, Rob,

On Fri, May 25, 2018 at 12:27:13PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 25, 2018 at 03:03:47PM +0100, Robert Walker escreveu:
> > Hi Leo,
> > 
> > Following the discussions from your reply to this with a simplified patch,
> > this version of the patch works better as you also need to emit a branch
> > sample when handling a CS_ETM_TRACE_ON packet to indicate the end of a block
> > of trace.

I also will follow the suggestion as Rob mentioned in another email:
"The deadbeefdeadbeef addresses are a bit ugly - these are just dummy
values emitted in the decoder layer - maybe these should be changed
to 0."

> > This patch does not break the output from perf inject to generate
> > instruction samples for AutoFDO, so I am happy with that.

Thanks for confirmation.

> > Regards
> > 
> > Rob
> > 
> > Reviewed-by: Robert Walker <robert.walker@arm.com>
> 
> So, Leo, can you please resubmit, bumping the v2 to v3 (or the latest
> one, I haven't fully reread this thread) add this "Reviewed-by: Robert"
> tag and any other that people may have provided, so that I can merge it?

Sure!  I will respin the v3 patch series by following up Rob's
suggestion and add Rob's review tag.

BTW, I'd like to get ack from Mathieu as well.  Mathieu is working on
CPU wide tracing, so I talked with Mathieu he will review the patch
series if has conflict with CPU wide tracing.

[...]

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFT v2 1/4] perf cs-etm: Generate sample for missed packets
From: Arnaldo Carvalho de Melo @ 2018-05-25 15:27 UTC (permalink / raw)
  To: Leo Yan
  Cc: Robert Walker, Mathieu Poirier, Jonathan Corbet, Peter Zijlstra,
	Ingo Molnar, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-arm-kernel, linux-doc, linux-kernel, Tor Jeremiassen,
	mike.leach, kim.phillips, coresight, Mike Leach
In-Reply-To: <03a53a72-8af5-02de-21a8-fd02a0974ed6@arm.com>

Em Fri, May 25, 2018 at 03:03:47PM +0100, Robert Walker escreveu:
> Hi Leo,
> 
> Following the discussions from your reply to this with a simplified patch,
> this version of the patch works better as you also need to emit a branch
> sample when handling a CS_ETM_TRACE_ON packet to indicate the end of a block
> of trace.
> 
> This patch does not break the output from perf inject to generate
> instruction samples for AutoFDO, so I am happy with that.
> 
> Regards
> 
> Rob
> 
> Reviewed-by: Robert Walker <robert.walker@arm.com>

So, Leo, can you please resubmit, bumping the v2 to v3 (or the latest
one, I haven't fully reread this thread) add this "Reviewed-by: Robert"
tag and any other that people may have provided, so that I can merge it?

Thanks,

- Arnaldo
 
> 
> On 22/05/18 09:39, Leo Yan wrote:
> > Hi Rob,
> > 
> > On Mon, May 21, 2018 at 12:27:42PM +0100, Robert Walker wrote:
> > > Hi Leo,
> > > 
> > > On 21/05/18 09:52, Leo Yan wrote:
> > > > Commit e573e978fb12 ("perf cs-etm: Inject capabilitity for CoreSight
> > > > traces") reworks the samples generation flow from CoreSight trace to
> > > > match the correct format so Perf report tool can display the samples
> > > > properly.  But the change has side effect for packet handling, it only
> > > > generate samples when 'prev_packet->last_instr_taken_branch' is true,
> > > > this results in the start tracing packet and exception packets are
> > > > dropped.
> > > > 
> > > > This patch checks extra two conditions for complete samples:
> > > > 
> > > > - If 'prev_packet->sample_type' is zero we can use this condition to
> > > >    get to know this is the start tracing packet; for this case, the start
> > > >    packet's end_addr is zero as well so we need to handle it in the
> > > >    function cs_etm__last_executed_instr();
> > > > 
> > > I think you also need to add something in to handle discontinuities in
> > > trace - for example it is possible to configure the ETM to only trace
> > > execution in specific code regions or to trace a few cycles every so
> > > often. In these cases, prev_packet->sample_type will not be zero, but
> > > whatever the previous packet was.  You will get a CS_ETM_TRACE_ON packet in
> > > such cases, generated by an I_TRACE_ON element in the trace stream.
> > > You also get this on exception return.
> > > 
> > > However, you should also keep the test for prev_packet->sample_type == 0
> > > as you may not see a CS_ETM_TRACE_ON when decoding a buffer that has
> > > wrapped.
> > Thanks for reviewing.  Let's dig more detailed into this issue,
> > especially for handling packet CS_ETM_TRACE_ON, I'd like divide into two
> > sub cases.
> > 
> > - The first case is for using python script:
> > 
> >    I use python script to analyze packets with below command:
> >    ./perf script --itrace=ril128 -s arm-cs-trace-disasm.py -F cpu,event,ip,addr,sym -- -v -d objdump -k ./vmlinux
> > 
> >    What I observe is after we pass python script with parameter '-s
> >    arm-cs-trace-disasm.py', then instruction tracing options
> >    '--itrace=ril128' isn't really used;  the perf tool creates another
> >    new process for launch python script and re-enter cmd_script()
> >    function, but at the second time when invoke cmd_script() for python
> >    script execution the option '--itrace=ril128' is dropped and all
> >    parameters are only valid defined by the python script.
> > 
> >    As result, I can the variable 'etmq->etm->synth_opts.last_branch' is
> >    always FALSE for running python script.  So all CS_ETM_TRACE_ON
> >    packets will be ignored in the function cs_etm__flush().
> > 
> >    Even the CS_ETM_TRACE_ON packets are missed to handle, the program
> >    flow still can work well.  The reason is without the interference by
> >    CS_ETM_TRACE_ON, the CS_ETM_RANGE packets can smoothly create
> >    instruction range by ignore the middle CS_ETM_TRACE_ON packet.
> > 
> >    Please see below example, in this example there have 3 packets, the
> >    first one packet is CS_ETM_RANGE packet which is labelled with
> >    'PACKET_1', the first one packet can properly generate branch sample
> >    data with previous packet as expected;  the second packet is
> >    PACKET_2 which is CS_ETM_TRACE_ON, but
> >    'etmq->etm->synth_opts.last_branch' is false so function
> >    cs_etm__flush() doesn't handle it and skip the swap operation
> >    "etmq->prev_packet = tmp"; the third packet is PACKET_3, which is
> >    CS_ETM_RANGE packet and we can see it's smoontly to create
> >    continous instruction range between PACKET_1 and PACKET_3.
> > 
> >    cs_etm__sample: prev_packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f79c end_addr=0xffff000008a5f7bc last_instr_taken_branch=1
> >    PACKET_1: cs_etm__sample: packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f858 end_addr=0xffff000008a5f864 last_instr_taken_branch=1
> >    cs_etm__synth_branch_sample: ip=0xffff000008a5f7b8 addr=0xffff000008a5f858 pid=2290 tid=2290 id=1000000021 stream_id=1000000021 period=1 cpu=1 flags=0 cpumode=2
> > 
> >    cs_etm__flush: prev_packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f858 end_addr=0xffff000008a5f864 last_instr_taken_branch=1
> >    PACKET_2: cs_etm__flush: packet: sample_type=2 exc=0 exc_ret=0 cpu=2 start_addr=0xdeadbeefdeadbeef end_addr=0xdeadbeefdeadbeef last_instr_taken_branch=1
> > 
> >    cs_etm__sample: prev_packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f858 end_addr=0xffff000008a5f864 last_instr_taken_branch=1
> >    PACKET_3: cs_etm__sample: packet: sample_type=1 exc=0 exc_ret=0 cpu=2 start_addr=0xffff000008be7528 end_addr=0xffff000008be7538 last_instr_taken_branch=1
> >    cs_etm__synth_branch_sample: ip=0xffff000008a5f860 addr=0xffff000008be7528 pid=2290 tid=2290 id=1000000021 stream_id=1000000021 period=1 cpu=2 flags=0 cpumode=2
> > 
> >    So seems to me, the CS_ETM_TRACE_ON packet doesn't introduce trouble
> >    for the program flow analysis if we can handle all CS_ETM_RANGE
> >    packets and without handling CS_ETM_TRACE_ON packet for branch
> >    samples.
> > 
> > - The second case is for --itrace option without python script:
> >    ./perf script --itrace=ril -F cpu,event,ip,addr,sym -k ./vmlinux
> > 
> >    In this case, the flag 'etmq->etm->synth_opts.last_branch' is true
> >    so CS_ETM_TRACE_ON packet will be handled; but I can observe the
> >    CS_ETM_RANGE packet in etmq->prev_packet isn't handled in the
> >    function cs_etm__flush() for branch sample, so actually we miss some
> >    branch sample for this case.
> > 
> >    So I think we also need handle CS_ETM_RANGE packet in function
> >    cs_etm__flush() to generate branch samples.  But this has side
> >    effect, we introduce the extra track for CS_ETM_TRACE_ON packet for
> >    branch samples, so we will see one branch range like:
> >    [ 0xdeadbeefdeadbeef .. 0xdeadbeefdeadbeef ].
> > 
> > Please reivew below change is okay for you?  Thanks a lot for
> > suggestions.
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 822ba91..37d3722 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -495,6 +495,13 @@ static inline void cs_etm__reset_last_branch_rb(struct cs_etm_queue *etmq)
> >   static inline u64 cs_etm__last_executed_instr(struct cs_etm_packet *packet)
> >   {
> >   	/*
> > +	 * The packet is the start tracing packet if the end_addr is zero,
> > +	 * returns 0 for this case.
> > +	 */
> > +	if (!packet->end_addr)
> > +		return 0;
> > +
> > +	/*
> >   	 * The packet records the execution range with an exclusive end address
> >   	 *
> >   	 * A64 instructions are constant size, so the last executed
> > @@ -897,13 +904,28 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
> >   		etmq->period_instructions = instrs_over;
> >   	}
> > -	if (etm->sample_branches &&
> > -	    etmq->prev_packet &&
> > -	    etmq->prev_packet->sample_type == CS_ETM_RANGE &&
> > -	    etmq->prev_packet->last_instr_taken_branch) {
> > -		ret = cs_etm__synth_branch_sample(etmq);
> > -		if (ret)
> > -			return ret;
> > +	if (etm->sample_branches && etmq->prev_packet) {
> > +		bool generate_sample = false;
> > +
> > +		/* Generate sample for start tracing packet */
> > +		if (etmq->prev_packet->sample_type == 0 ||
> > +		    etmq->prev_packet->sample_type == CS_ETM_TRACE_ON)
> > +			generate_sample = true;
> > +
> > +		/* Generate sample for exception packet */
> > +		if (etmq->prev_packet->exc == true)
> > +			generate_sample = true;
> > +
> > +		/* Generate sample for normal branch packet */
> > +		if (etmq->prev_packet->sample_type == CS_ETM_RANGE &&
> > +		    etmq->prev_packet->last_instr_taken_branch)
> > +			generate_sample = true;
> > +
> > +		if (generate_sample) {
> > +			ret = cs_etm__synth_branch_sample(etmq);
> > +			if (ret)
> > +				return ret;
> > +		}
> >   	}
> >   	if (etm->sample_branches || etm->synth_opts.last_branch) {
> > @@ -921,12 +943,17 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
> >   static int cs_etm__flush(struct cs_etm_queue *etmq)
> >   {
> > +	struct cs_etm_auxtrace *etm = etmq->etm;
> >   	int err = 0;
> >   	struct cs_etm_packet *tmp;
> > -	if (etmq->etm->synth_opts.last_branch &&
> > -	    etmq->prev_packet &&
> > -	    etmq->prev_packet->sample_type == CS_ETM_RANGE) {
> > +	if (!etmq->prev_packet)
> > +		return 0;
> > +
> > +	if (etmq->prev_packet->sample_type != CS_ETM_RANGE)
> > +		return 0;
> > +
> > +	if (etmq->etm->synth_opts.last_branch) {
> >   		/*
> >   		 * Generate a last branch event for the branches left in the
> >   		 * circular buffer at the end of the trace.
> > @@ -939,18 +966,25 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
> >   		err = cs_etm__synth_instruction_sample(
> >   			etmq, addr,
> >   			etmq->period_instructions);
> > +		if (err)
> > +			return err;
> >   		etmq->period_instructions = 0;
> > +	}
> > -		/*
> > -		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
> > -		 * the next incoming packet.
> > -		 */
> > -		tmp = etmq->packet;
> > -		etmq->packet = etmq->prev_packet;
> > -		etmq->prev_packet = tmp;
> > +	if (etm->sample_branches) {
> > +		err = cs_etm__synth_branch_sample(etmq);
> > +		if (err)
> > +			return err;
> >   	}
> > -	return err;
> > +	/*
> > +	 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
> > +	 * the next incoming packet.
> > +	 */
> > +	tmp = etmq->packet;
> > +	etmq->packet = etmq->prev_packet;
> > +	etmq->prev_packet = tmp;
> > +	return 0;
> >   }
> >   static int cs_etm__run_decoder(struct cs_etm_queue *etmq)
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v8 3/6] cpuset: Add cpuset.sched.load_balance flag to v2
From: Waiman Long @ 2018-05-25 14:45 UTC (permalink / raw)
  To: Patrick Bellasi
  Cc: Juri Lelli, Tejun Heo, Li Zefan, Johannes Weiner, Peter Zijlstra,
	Ingo Molnar, cgroups, linux-kernel, linux-doc, kernel-team, pjt,
	luto, Mike Galbraith, torvalds, Roman Gushchin
In-Reply-To: <20180525094050.GB30654@e110439-lin>

On 05/25/2018 05:40 AM, Patrick Bellasi wrote:
> On 24-May 11:22, Waiman Long wrote:
>> On 05/24/2018 11:16 AM, Juri Lelli wrote:
>>> On 24/05/18 11:09, Waiman Long wrote:
>>>> On 05/24/2018 10:36 AM, Juri Lelli wrote:
>>>>> On 17/05/18 16:55, Waiman Long wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>> +	A parent cgroup cannot distribute all its CPUs to child
>>>>>> +	scheduling domain cgroups unless its load balancing flag is
>>>>>> +	turned off.
>>>>>> +
>>>>>> +  cpuset.sched.load_balance
>>>>>> +	A read-write single value file which exists on non-root
>>>>>> +	cpuset-enabled cgroups.  It is a binary value flag that accepts
>>>>>> +	either "0" (off) or a non-zero value (on).  This flag is set
>>>>>> +	by the parent and is not delegatable.
>>>>>> +
>>>>>> +	When it is on, tasks within this cpuset will be load-balanced
>>>>>> +	by the kernel scheduler.  Tasks will be moved from CPUs with
>>>>>> +	high load to other CPUs within the same cpuset with less load
>>>>>> +	periodically.
>>>>>> +
>>>>>> +	When it is off, there will be no load balancing among CPUs on
>>>>>> +	this cgroup.  Tasks will stay in the CPUs they are running on
>>>>>> +	and will not be moved to other CPUs.
>>>>>> +
>>>>>> +	The initial value of this flag is "1".	This flag is then
>>>>>> +	inherited by child cgroups with cpuset enabled.  Its state
>>>>>> +	can only be changed on a scheduling domain cgroup with no
>>>>>> +	cpuset-enabled children.
>>>>> [...]
>>>>>
>>>>>> +	/*
>>>>>> +	 * On default hierachy, a load balance flag change is only allowed
>>>>>> +	 * in a scheduling domain with no child cpuset.
>>>>>> +	 */
>>>>>> +	if (cgroup_subsys_on_dfl(cpuset_cgrp_subsys) && balance_flag_changed &&
>>>>>> +	   (!is_sched_domain(cs) || css_has_online_children(&cs->css))) {
>>>>>> +		err = -EINVAL;
>>>>>> +		goto out;
>>>>>> +	}
>>>>> The rule is actually
>>>>>
>>>>>  - no child cpuset
>>>>>  - and it must be a scheduling domain
> I always a bit confused by the usage of "scheduling domain", which
> overlaps with the SD concept from the scheduler standpoint.

It is supposed to mimic SD concept of scheduler.

>
> AFAIU a cpuset sched domain is not granted to be turned into an
> actual scheduler SD, am I wrong?
>
> If that's the case, why not better disambiguate these two concept by
> calling the cpuset one a "cpus partition" or eventually "cpuset domain"?

Good point. Peter has similar comment. I will probably change the name
and clarifying it better in the documentation.

Cheers,
Longman

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFT v2 1/4] perf cs-etm: Generate sample for missed packets
From: Robert Walker @ 2018-05-25 14:03 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Mathieu Poirier, Jonathan Corbet,
	Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, linux-arm-kernel, linux-doc, linux-kernel,
	Tor Jeremiassen, mike.leach, kim.phillips, coresight, Mike Leach
In-Reply-To: <20180522083920.GD31075@leoy-ThinkPad-X240s>

Hi Leo,

Following the discussions from your reply to this with a simplified 
patch, this version of the patch works better as you also need to emit a 
branch sample when handling a CS_ETM_TRACE_ON packet to indicate the end 
of a block of trace.

This patch does not break the output from perf inject to generate 
instruction samples for AutoFDO, so I am happy with that.

Regards

Rob

Reviewed-by: Robert Walker <robert.walker@arm.com>


On 22/05/18 09:39, Leo Yan wrote:
> Hi Rob,
>
> On Mon, May 21, 2018 at 12:27:42PM +0100, Robert Walker wrote:
>> Hi Leo,
>>
>> On 21/05/18 09:52, Leo Yan wrote:
>>> Commit e573e978fb12 ("perf cs-etm: Inject capabilitity for CoreSight
>>> traces") reworks the samples generation flow from CoreSight trace to
>>> match the correct format so Perf report tool can display the samples
>>> properly.  But the change has side effect for packet handling, it only
>>> generate samples when 'prev_packet->last_instr_taken_branch' is true,
>>> this results in the start tracing packet and exception packets are
>>> dropped.
>>>
>>> This patch checks extra two conditions for complete samples:
>>>
>>> - If 'prev_packet->sample_type' is zero we can use this condition to
>>>    get to know this is the start tracing packet; for this case, the start
>>>    packet's end_addr is zero as well so we need to handle it in the
>>>    function cs_etm__last_executed_instr();
>>>
>> I think you also need to add something in to handle discontinuities in
>> trace - for example it is possible to configure the ETM to only trace
>> execution in specific code regions or to trace a few cycles every so
>> often. In these cases, prev_packet->sample_type will not be zero, but
>> whatever the previous packet was.  You will get a CS_ETM_TRACE_ON packet in
>> such cases, generated by an I_TRACE_ON element in the trace stream.
>> You also get this on exception return.
>>
>> However, you should also keep the test for prev_packet->sample_type == 0
>> as you may not see a CS_ETM_TRACE_ON when decoding a buffer that has
>> wrapped.
> Thanks for reviewing.  Let's dig more detailed into this issue,
> especially for handling packet CS_ETM_TRACE_ON, I'd like divide into two
> sub cases.
>
> - The first case is for using python script:
>
>    I use python script to analyze packets with below command:
>    ./perf script --itrace=ril128 -s arm-cs-trace-disasm.py -F cpu,event,ip,addr,sym -- -v -d objdump -k ./vmlinux
>
>    What I observe is after we pass python script with parameter '-s
>    arm-cs-trace-disasm.py', then instruction tracing options
>    '--itrace=ril128' isn't really used;  the perf tool creates another
>    new process for launch python script and re-enter cmd_script()
>    function, but at the second time when invoke cmd_script() for python
>    script execution the option '--itrace=ril128' is dropped and all
>    parameters are only valid defined by the python script.
>
>    As result, I can the variable 'etmq->etm->synth_opts.last_branch' is
>    always FALSE for running python script.  So all CS_ETM_TRACE_ON
>    packets will be ignored in the function cs_etm__flush().
>
>    Even the CS_ETM_TRACE_ON packets are missed to handle, the program
>    flow still can work well.  The reason is without the interference by
>    CS_ETM_TRACE_ON, the CS_ETM_RANGE packets can smoothly create
>    instruction range by ignore the middle CS_ETM_TRACE_ON packet.
>
>    Please see below example, in this example there have 3 packets, the
>    first one packet is CS_ETM_RANGE packet which is labelled with
>    'PACKET_1', the first one packet can properly generate branch sample
>    data with previous packet as expected;  the second packet is
>    PACKET_2 which is CS_ETM_TRACE_ON, but
>    'etmq->etm->synth_opts.last_branch' is false so function
>    cs_etm__flush() doesn't handle it and skip the swap operation
>    "etmq->prev_packet = tmp"; the third packet is PACKET_3, which is
>    CS_ETM_RANGE packet and we can see it's smoontly to create
>    continous instruction range between PACKET_1 and PACKET_3.
>
>    cs_etm__sample: prev_packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f79c end_addr=0xffff000008a5f7bc last_instr_taken_branch=1
>    PACKET_1: cs_etm__sample: packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f858 end_addr=0xffff000008a5f864 last_instr_taken_branch=1
>    cs_etm__synth_branch_sample: ip=0xffff000008a5f7b8 addr=0xffff000008a5f858 pid=2290 tid=2290 id=1000000021 stream_id=1000000021 period=1 cpu=1 flags=0 cpumode=2
>
>    cs_etm__flush: prev_packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f858 end_addr=0xffff000008a5f864 last_instr_taken_branch=1
>    PACKET_2: cs_etm__flush: packet: sample_type=2 exc=0 exc_ret=0 cpu=2 start_addr=0xdeadbeefdeadbeef end_addr=0xdeadbeefdeadbeef last_instr_taken_branch=1
>
>    cs_etm__sample: prev_packet: sample_type=1 exc=0 exc_ret=0 cpu=1 start_addr=0xffff000008a5f858 end_addr=0xffff000008a5f864 last_instr_taken_branch=1
>    PACKET_3: cs_etm__sample: packet: sample_type=1 exc=0 exc_ret=0 cpu=2 start_addr=0xffff000008be7528 end_addr=0xffff000008be7538 last_instr_taken_branch=1
>    cs_etm__synth_branch_sample: ip=0xffff000008a5f860 addr=0xffff000008be7528 pid=2290 tid=2290 id=1000000021 stream_id=1000000021 period=1 cpu=2 flags=0 cpumode=2
>
>    So seems to me, the CS_ETM_TRACE_ON packet doesn't introduce trouble
>    for the program flow analysis if we can handle all CS_ETM_RANGE
>    packets and without handling CS_ETM_TRACE_ON packet for branch
>    samples.
>
> - The second case is for --itrace option without python script:
>    ./perf script --itrace=ril -F cpu,event,ip,addr,sym -k ./vmlinux
>
>    In this case, the flag 'etmq->etm->synth_opts.last_branch' is true
>    so CS_ETM_TRACE_ON packet will be handled; but I can observe the
>    CS_ETM_RANGE packet in etmq->prev_packet isn't handled in the
>    function cs_etm__flush() for branch sample, so actually we miss some
>    branch sample for this case.
>
>    So I think we also need handle CS_ETM_RANGE packet in function
>    cs_etm__flush() to generate branch samples.  But this has side
>    effect, we introduce the extra track for CS_ETM_TRACE_ON packet for
>    branch samples, so we will see one branch range like:
>    [ 0xdeadbeefdeadbeef .. 0xdeadbeefdeadbeef ].
>
> Please reivew below change is okay for you?  Thanks a lot for
> suggestions.
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 822ba91..37d3722 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -495,6 +495,13 @@ static inline void cs_etm__reset_last_branch_rb(struct cs_etm_queue *etmq)
>   static inline u64 cs_etm__last_executed_instr(struct cs_etm_packet *packet)
>   {
>   	/*
> +	 * The packet is the start tracing packet if the end_addr is zero,
> +	 * returns 0 for this case.
> +	 */
> +	if (!packet->end_addr)
> +		return 0;
> +
> +	/*
>   	 * The packet records the execution range with an exclusive end address
>   	 *
>   	 * A64 instructions are constant size, so the last executed
> @@ -897,13 +904,28 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
>   		etmq->period_instructions = instrs_over;
>   	}
>   
> -	if (etm->sample_branches &&
> -	    etmq->prev_packet &&
> -	    etmq->prev_packet->sample_type == CS_ETM_RANGE &&
> -	    etmq->prev_packet->last_instr_taken_branch) {
> -		ret = cs_etm__synth_branch_sample(etmq);
> -		if (ret)
> -			return ret;
> +	if (etm->sample_branches && etmq->prev_packet) {
> +		bool generate_sample = false;
> +
> +		/* Generate sample for start tracing packet */
> +		if (etmq->prev_packet->sample_type == 0 ||
> +		    etmq->prev_packet->sample_type == CS_ETM_TRACE_ON)
> +			generate_sample = true;
> +
> +		/* Generate sample for exception packet */
> +		if (etmq->prev_packet->exc == true)
> +			generate_sample = true;
> +
> +		/* Generate sample for normal branch packet */
> +		if (etmq->prev_packet->sample_type == CS_ETM_RANGE &&
> +		    etmq->prev_packet->last_instr_taken_branch)
> +			generate_sample = true;
> +
> +		if (generate_sample) {
> +			ret = cs_etm__synth_branch_sample(etmq);
> +			if (ret)
> +				return ret;
> +		}
>   	}
>   
>   	if (etm->sample_branches || etm->synth_opts.last_branch) {
> @@ -921,12 +943,17 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
>   
>   static int cs_etm__flush(struct cs_etm_queue *etmq)
>   {
> +	struct cs_etm_auxtrace *etm = etmq->etm;
>   	int err = 0;
>   	struct cs_etm_packet *tmp;
>   
> -	if (etmq->etm->synth_opts.last_branch &&
> -	    etmq->prev_packet &&
> -	    etmq->prev_packet->sample_type == CS_ETM_RANGE) {
> +	if (!etmq->prev_packet)
> +		return 0;
> +
> +	if (etmq->prev_packet->sample_type != CS_ETM_RANGE)
> +		return 0;
> +
> +	if (etmq->etm->synth_opts.last_branch) {
>   		/*
>   		 * Generate a last branch event for the branches left in the
>   		 * circular buffer at the end of the trace.
> @@ -939,18 +966,25 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
>   		err = cs_etm__synth_instruction_sample(
>   			etmq, addr,
>   			etmq->period_instructions);
> +		if (err)
> +			return err;
>   		etmq->period_instructions = 0;
> +	}
>   
> -		/*
> -		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
> -		 * the next incoming packet.
> -		 */
> -		tmp = etmq->packet;
> -		etmq->packet = etmq->prev_packet;
> -		etmq->prev_packet = tmp;
> +	if (etm->sample_branches) {
> +		err = cs_etm__synth_branch_sample(etmq);
> +		if (err)
> +			return err;
>   	}
>   
> -	return err;
> +	/*
> +	 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
> +	 * the next incoming packet.
> +	 */
> +	tmp = etmq->packet;
> +	etmq->packet = etmq->prev_packet;
> +	etmq->prev_packet = tmp;
> +	return 0;
>   }
>   
>   static int cs_etm__run_decoder(struct cs_etm_queue *etmq)

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFT v2 1/4] perf cs-etm: Generate sample for missed packets
From: Robert Walker @ 2018-05-25 13:56 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Mathieu Poirier, Jonathan Corbet,
	Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, linux-arm-kernel, linux-doc, linux-kernel,
	Tor Jeremiassen, mike.leach, kim.phillips, coresight, Mike Leach
In-Reply-To: <20180523132203.GA30299@leoy-ThinkPad-X240s>

Hi Leo,


On 23/05/18 14:22, Leo Yan wrote:
> Hi Rob,
>
> On Wed, May 23, 2018 at 12:21:18PM +0100, Robert Walker wrote:
>> Hi Leo,
>>
>> On 22/05/18 10:52, Leo Yan wrote:
>>> On Tue, May 22, 2018 at 04:39:20PM +0800, Leo Yan wrote:
>>>
>>> [...]
>>>
>>> Rather than the patch I posted in my previous email, I think below new
>>> patch is more reasonable for me.
>>>
>>> In the below change, 'etmq->prev_packet' is only used to store the
>>> previous CS_ETM_RANGE packet, we don't need to save CS_ETM_TRACE_ON
>>> packet into 'etmq->prev_packet'.
>>>
>>> On the other hand, cs_etm__flush() can use 'etmq->period_instructions'
>>> to indicate if need to generate instruction sample or not.  If it's
>>> non-zero, then generate instruction sample and
>>> 'etmq->period_instructions' will be cleared; so next time if there
>>> have more tracing CS_ETM_TRACE_ON packet, it can skip to generate
>>> instruction sample due 'etmq->period_instructions' is zero.
>>>
>>> How about you think for this?
>>>
>>> Thanks,
>>> Leo Yan
>>>
>> I don't think this covers the cases where CS_ETM_TRACE_ON is used to
>> indicate a discontinuity in the trace.  For example, there is work in
>> progress to configure the ETM so that it only traces a few thousand cycles
>> with a gap of many thousands of cycles between each chunk of trace - this
>> can be used to sample program execution in the form of instruction events
>> with branch stacks for feedback directed optimization (AutoFDO).
>>
>> In this case, the raw trace is something like:
>>
>>      ...
>>      I_ADDR_L_64IS0 : Address, Long, 64 bit, IS0.; Addr=0x0000007E7B886908;
>>      I_ATOM_F3 : Atom format 3.; EEN
>>      I_ATOM_F1 : Atom format 1.; E
>> # Trace stops here
>>
>> # Some time passes, and then trace is turned on again
>>      I_TRACE_ON : Trace On.
>>      I_ADDR_CTXT_L_64IS0 : Address & Context, Long, 64 bit, IS0.;
>> Addr=0x00000057224322F4; Ctxt: AArch64,EL0, NS;
>>      I_ATOM_F3 : Atom format 3.; ENN
>>      I_ATOM_F5 : Atom format 5.; ENENE
>>      ...
>>
>> This results in the following packets from the decoder:
>>
>> CS_ETM_RANGE: [0x7e7b886908-0x7e7b886930] br
>> CS_ETM_RANGE: [0x7e7b88699c-0x7e7b8869a4] br
>> CS_ETM_RANGE: [0x7e7b8869d8-0x7e7b8869f0]
>> CS_ETM_RANGE: [0x7e7b8869f0-0x7e7b8869fc] br
>> CS_ETM_TRACE_ON
>> CS_ETM_RANGE: [0x57224322f4-0x5722432304] br
>> CS_ETM_RANGE: [0x57224320e8-0x57224320ec]
>> CS_ETM_RANGE: [0x57224320ec-0x57224320f8]
>> CS_ETM_RANGE: [0x57224320f8-0x572243212c] br
>> CS_ETM_RANGE: [0x5722439b80-0x5722439bec]
>> CS_ETM_RANGE: [0x5722439bec-0x5722439c14] br
>> CS_ETM_RANGE: [0x5722437c30-0x5722437c6c]
>> CS_ETM_RANGE: [0x5722437c6c-0x5722437c7c] br
>>
>> Without handling the CS_ETM_TRACE_ON, this would be interpreted as a branch
>> from 0x7e7b8869f8 to 0x57224322f4, when there is actually a gap of many
>> thousand instructions between these.
>>
>> I think this patch will break the branch stacks - by removing the
>> prev_packet swap from cs_etm__flush(), the next time a CS_ETM_RANGE packet
>> is handled, cs_etm__sample() will see prev_packet contains the last
>> CS_ETM_RANGE from the previous block of trace, causing an erroneous call to
>> cs_etm__update_last_branch_rb().  In the example above, the branch stack
>> will contain an erroneous branch from 0x7e7b8869f8 to 0x57224322f4.
>>
>> I think what you need to do is add a check for the previous packet being a
>> CS_ETM_TRACE_ON when determining the generate_sample value.
> I still can see there have hole for packets handling with your
> suggestion, let's focus on below three packets:
>
> CS_ETM_RANGE:    [0x7e7b8869f0-0x7e7b8869fc] br
> CS_ETM_TRACE_ON: [0xdeadbeefdeadbeef-0xdeadbeefdeadbeef]
> CS_ETM_RANGE:    [0x57224322f4-0x5722432304] br
>
> When the CS_ETM_TRACE_ON packet is coming, cs_etm__flush() doesn't
> handle for 'etmq->prev_packet' to generate branch sample, this results
> in we miss the info for 0x7e7b8869fc, and with packet swapping
> 'etmq->prev_packet' is assigned to CS_ETM_TRACE_ON packet.
>
> When the last CS_ETM_RANGE packet is coming, cs_etm__sample() will
> combine the values from CS_ETM_TRACE_ON packet and the last
> CS_ETM_RANGE packet to generate branch sample packet; at the end
> we get below sample packets:
>
>    packet(n):   sample::addr=0x7e7b8869f0
>    packet(n+1): sample::ip=0xdeadbeefdeadbeeb sample::addr=0x57224322f4
>
> So I think we also need to generate branch sample, and we can get
> below results:
>
>    packet(n):   sample::addr=0x7e7b8869f0
>    packet(n+1): sample::ip=0x7e7b8869f8 sample::addr=0xdeadbeefdeadbeef
>    packet(n+2): sample::ip=0xdeadbeefdeadbeeb sample::addr=0x57224322f4
>
> So we also can rely on this to get to know there have one address
> range is [0xdeadbeefdeadbeef..0xdeadbeefdeadbeeb] to indicate there
> have a discontinuity in the trace.
Yes, I agree you need the extra branch sample from cs_etm__flush().

With a discontinuity in trace, I get output from perf script like this:

branches:u:        59ee6e2e08 sqlite3VdbeExec (speedtest1) =>       
59ee6e2e64 sqlite3VdbeExec (spe
branches:u:        59ee6e2e7c sqlite3VdbeExec (speedtest1) =>       
59ee6e2eec sqlite3VdbeExec (spe
branches:u:        59ee6e2efc sqlite3VdbeExec (speedtest1) =>       
59ee6e2f14 sqlite3VdbeExec (spe
branches:u:        59ee6e2f3c sqlite3VdbeExec (speedtest1) => 
deadbeefdeadbeef [unknown] ([unknown])
branches:u:  deadbeefdeadbeeb [unknown] ([unknown]) => 769949daa0 memcpy 
(/system/lib64/libc.so)
branches:u:        769949dacc memcpy (/system/lib64/libc.so) =>       
59ee6f0664 insertCell (speedtest1)
branches:u:        59ee6f0664 insertCell (speedtest1) => 59ee6f0684 
insertCell (speedtest1)
branches:u:        59ee6f06a4 insertCell (speedtest1) => 59ee6a4d50 
memmove@plt (speedtest1)
branches:u:        59ee6a4d5c memmove@plt (speedtest1) => 769949ebf8 
memmove (/system/lib64/libc.so)

Showing there is a break in trace between 59ee6e2f3c and 769949daa0.  
The deadbeefdeadbeef addresses are a bit ugly - these are just dummy 
values emitted in the decoder layer - maybe these should be changed to 
0.  Or you could add a new sample type (i.e. not branch) to indicate the 
start / end of trace, with only the valid address.

With this change, it becomes the same as the patch from your previous mail.

Regards

Rob

>> Regards
>>
>> Rob
>>
>>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>>> index 822ba91..dd354ad 100644
>>> --- a/tools/perf/util/cs-etm.c
>>> +++ b/tools/perf/util/cs-etm.c
>>> @@ -495,6 +495,13 @@ static inline void cs_etm__reset_last_branch_rb(struct cs_etm_queue *etmq)
>>>   static inline u64 cs_etm__last_executed_instr(struct cs_etm_packet *packet)
>>>   {
>>>   	/*
>>> +	 * The packet is the start tracing packet if the end_addr is zero,
>>> +	 * returns 0 for this case.
>>> +	 */
>>> +	if (!packet->end_addr)
>>> +		return 0;
>>> +
>>> +	/*
>>>   	 * The packet records the execution range with an exclusive end address
>>>   	 *
>>>   	 * A64 instructions are constant size, so the last executed
>>> @@ -897,13 +904,27 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
>>>   		etmq->period_instructions = instrs_over;
>>>   	}
>>> -	if (etm->sample_branches &&
>>> -	    etmq->prev_packet &&
>>> -	    etmq->prev_packet->sample_type == CS_ETM_RANGE &&
>>> -	    etmq->prev_packet->last_instr_taken_branch) {
>>> -		ret = cs_etm__synth_branch_sample(etmq);
>>> -		if (ret)
>>> -			return ret;
>>> +	if (etm->sample_branches && etmq->prev_packet) {
>>> +		bool generate_sample = false;
>>> +
>>> +		/* Generate sample for start tracing packet */
>>> +		if (etmq->prev_packet->sample_type == 0)
>>> +			generate_sample = true;
>> Also check for etmq->prev_packet->sample_type == CS_ETM_TRACE_ON here and
>> set generate_sample = true.
> Agree, will add this.
>
>>> +
>>> +		/* Generate sample for exception packet */
>>> +		if (etmq->prev_packet->exc == true)
>>> +			generate_sample = true;
>>> +
>>> +		/* Generate sample for normal branch packet */
>>> +		if (etmq->prev_packet->sample_type == CS_ETM_RANGE &&
>>> +		    etmq->prev_packet->last_instr_taken_branch)
>>> +			generate_sample = true;
>>> +
>>> +		if (generate_sample) {
>>> +			ret = cs_etm__synth_branch_sample(etmq);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>>   	}
>>>   	if (etm->sample_branches || etm->synth_opts.last_branch) {
>>> @@ -922,11 +943,12 @@ static int cs_etm__sample(struct cs_etm_queue *etmq)
>>>   static int cs_etm__flush(struct cs_etm_queue *etmq)
>>>   {
>>>   	int err = 0;
>>> -	struct cs_etm_packet *tmp;
>>>   	if (etmq->etm->synth_opts.last_branch &&
>>>   	    etmq->prev_packet &&
>>> -	    etmq->prev_packet->sample_type == CS_ETM_RANGE) {
>>> +	    etmq->prev_packet->sample_type == CS_ETM_RANGE &&
>>> +	    etmq->period_instructions) {
>>> +
>> I don't think this is needed.
> Okay, I will keep this.
>
>>>   		/*
>>>   		 * Generate a last branch event for the branches left in the
>>>   		 * circular buffer at the end of the trace.
>>> @@ -940,14 +962,6 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
>>>   			etmq, addr,
>>>   			etmq->period_instructions);
>>>   		etmq->period_instructions = 0;
>>> -
>>> -		/*
>>> -		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
>>> -		 * the next incoming packet.
>>> -		 */
>>> -		tmp = etmq->packet;
>>> -		etmq->packet = etmq->prev_packet;
>>> -		etmq->prev_packet = tmp;
>> This should not be changed as discussed above.
> Okay, will keep this.  But I suggest we add some change like below:
>
> +    if (etm->sample_branches) {
> +        err = cs_etm__synth_branch_sample(etmq);
> +        if (err)
> +            return err;
> +    }
>
> If so, could you review my posted another patch for this?
> http://archive.armlinux.org.uk/lurker/message/20180522.083920.184f1f78.en.html
WIll do - with these changes, it is the same as your original patch.
> Thanks,
> Leo Yan
>
>>>   	}
>>>   	return err;
>>>

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox