Linux userland API discussions
 help / color / mirror / Atom feed
* [PATCH v3 5/6] tpm: TPM 2.0 CRB Interface
From: Jarkko Sakkinen @ 2014-10-15 11:35 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
	christophe.ricard, will.c.arthur, monty.wiseman, Jarkko Sakkinen
In-Reply-To: <1413372916-12091-1-git-send-email-jarkko.sakkinen@linux.intel.com>

tpm_crb is a driver for TPM 2.0 Command Response Buffer (CRB) Interface
as defined in PC Client Platform TPM Profile (PTP) Specification.

Only polling and single locality is supported as these are the limitations
of the available hardware, Platform Trust Techonlogy (PTT) in Haswell
CPUs.

The driver always applies CRB with ACPI start because PTT reports using
only ACPI start as start method but as a result of my testing it requires
also CRB start.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Kconfig   |   9 ++
 drivers/char/tpm/Makefile  |   1 +
 drivers/char/tpm/tpm_crb.c | 329 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 339 insertions(+)
 create mode 100644 drivers/char/tpm/tpm_crb.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index c54cac3..10c9419 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -122,4 +122,13 @@ config TCG_XEN
 	  To compile this driver as a module, choose M here; the module
 	  will be called xen-tpmfront.
 
+config TCG_CRB
+	tristate "TPM 2.0 CRB Interface"
+	depends on X86 && ACPI
+	---help---
+	  If you have a TPM security chip that is compliant with the
+	  TCG CRB 2.0 TPM specification say Yes and it will be accessible
+	  from within Linux.  To compile this driver as a module, choose
+	  M here; the module will be called tpm_crb.
+
 endif # TCG_TPM
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index d3cf905..15e3b4c 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
 obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
 obj-$(CONFIG_TCG_ST33_I2C) += tpm_i2c_stm_st33.o
 obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
+obj-$(CONFIG_TCG_CRB) += tpm_crb.o
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
new file mode 100644
index 0000000..0509228
--- /dev/null
+++ b/drivers/char/tpm/tpm_crb.c
@@ -0,0 +1,329 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This device driver implements the TPM interface as defined in
+ * the TCG CRB 2.0 TPM specification.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/acpi.h>
+#include <linux/highmem.h>
+#include <linux/rculist.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include "tpm.h"
+
+#define ACPI_SIG_TPM2 "TPM2"
+
+static const u8 CRB_ACPI_START_UUID[] = {
+	/* 0000 */ 0xAB, 0x6C, 0xBF, 0x6B, 0x63, 0x54, 0x14, 0x47,
+	/* 0008 */ 0xB7, 0xCD, 0xF0, 0x20, 0x3C, 0x03, 0x68, 0xD4
+};
+
+enum crb_defaults {
+	CRB_ACPI_START_REVISION_ID = 1,
+	CRB_ACPI_START_INDEX = 1,
+};
+
+enum crb_start_method {
+	CRB_SM_ACPI_START = 2,
+	CRB_SM_CRB = 7,
+	CRB_SM_CRB_WITH_ACPI_START = 8,
+};
+
+struct acpi_tpm2 {
+	struct acpi_table_header hdr;
+	u16 platform_class;
+	u16 reserved;
+	u64 control_area_pa;
+	u32 start_method;
+};
+
+enum crb_ca_request {
+	CRB_CA_REQ_GO_IDLE	= BIT(0),
+	CRB_CA_REQ_CMD_READY	= BIT(1),
+};
+
+enum crb_ca_status {
+	CRB_CA_STS_ERROR	= BIT(0),
+	CRB_CA_STS_TPM_IDLE	= BIT(1),
+};
+
+struct crb_control_area {
+	u32 req;
+	u32 sts;
+	u32 cancel;
+	u32 start;
+	u32 int_enable;
+	u32 int_sts;
+	u32 cmd_size;
+	u64 cmd_pa;
+	u32 rsp_size;
+	u64 rsp_pa;
+} __packed;
+
+enum crb_status {
+	CRB_STS_COMPLETE	= BIT(0),
+};
+
+enum crb_flags {
+	CRB_FL_ACPI_START	= BIT(0),
+	CRB_FL_CRB_START	= BIT(1),
+};
+
+struct crb_priv {
+	unsigned int flags;
+	struct crb_control_area *cca;
+	unsigned long cca_pa;
+	acpi_handle dev_handle;
+};
+
+#ifdef CONFIG_PM_SLEEP
+int crb_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int crb_resume(struct device *dev)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	(void) tpm2_do_selftest(chip);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(crb_pm, crb_suspend, crb_resume);
+
+static u8 crb_status(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	u8 sts = 0;
+
+	if ((le32_to_cpu(priv->cca->start) & 1) != 1)
+		sts |= CRB_STS_COMPLETE;
+
+	return sts;
+}
+
+static int crb_recv(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	struct crb_control_area *cca;
+	unsigned int expected;
+	unsigned long offset;
+	u8 *resp;
+
+	cca = priv->cca;
+	if (le32_to_cpu(cca->sts) & CRB_CA_STS_ERROR)
+		return -EIO;
+
+	offset = le64_to_cpu(cca->rsp_pa) - priv->cca_pa;
+	resp = (u8 *) ((unsigned long) cca + offset);
+	memcpy(buf, resp, 6);
+	expected = be32_to_cpup((__be32 *) &buf[2]);
+
+	if (expected > count)
+		return -EIO;
+
+	memcpy(&buf[6], &resp[6], expected - 6);
+
+	return expected;
+}
+
+static int crb_do_acpi_start(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	union acpi_object *obj;
+	int rc;
+
+	obj = acpi_evaluate_dsm(priv->dev_handle,
+				CRB_ACPI_START_UUID,
+				CRB_ACPI_START_REVISION_ID,
+				CRB_ACPI_START_INDEX,
+				NULL);
+	if (!obj)
+		return -ENXIO;
+	rc = obj->integer.value == 0 ? 0 : -ENXIO;
+	ACPI_FREE(obj);
+	return rc;
+}
+
+static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	struct crb_control_area *cca;
+	u8 *cmd;
+	int rc = 0;
+
+	cca = priv->cca;
+
+	if (len > le32_to_cpu(cca->cmd_size)) {
+		dev_err(chip->dev,
+			"invalid command count value %x %zx\n",
+			(unsigned int) len,
+			(size_t) le32_to_cpu(cca->cmd_size));
+		return -E2BIG;
+	}
+
+	cmd = (u8 *) ((unsigned long) cca + le64_to_cpu(cca->cmd_pa) - priv->cca_pa);
+	memcpy(cmd, buf, len);
+	wmb();
+
+	cca->start = cpu_to_le32(1);
+	rc = crb_do_acpi_start(chip);
+	return rc;
+}
+
+static void crb_cancel(struct tpm_chip *chip)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+	struct crb_control_area *cca;
+
+	cca = priv->cca;
+	cca->cancel = cpu_to_le32(1);
+	wmb();
+
+	if (crb_do_acpi_start(chip))
+		dev_err(chip->dev, "ACPI Start failed\n");
+
+	cca->cancel = 0;
+}
+
+static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
+{
+	struct crb_priv *priv = chip->vendor.priv;
+
+	return (le32_to_cpu(priv->cca->cancel) & 1) == 1;
+}
+
+static const struct tpm_class_ops tpm_crb = {
+	.status = crb_status,
+	.recv = crb_recv,
+	.send = crb_send,
+	.cancel = crb_cancel,
+	.req_canceled = crb_req_canceled,
+	.req_complete_mask = CRB_STS_COMPLETE,
+	.req_complete_val = CRB_STS_COMPLETE,
+};
+
+static int crb_acpi_add(struct acpi_device *device)
+{
+	struct tpm_chip *chip;
+	struct acpi_tpm2 *buf;
+	struct crb_priv *priv;
+	struct device *dev = &device->dev;
+	acpi_status status;
+	u32 sm;
+	int rc;
+
+	chip = tpmm_chip_alloc(dev, &tpm_crb);
+	if (IS_ERR(chip)) {
+		dev_err(dev, "allocating chip failed\n");
+		return PTR_ERR(chip);
+	}
+
+	chip->flags = TPM_CHIP_FLAG_TPM2;
+
+	status = acpi_get_table(ACPI_SIG_TPM2, 1,
+				(struct acpi_table_header **) &buf);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "could not get TPM2 ACPI table\n");
+		return -ENODEV;
+	}
+
+	priv = (struct crb_priv *) devm_kzalloc(dev, sizeof(struct crb_priv),
+						GFP_KERNEL);
+	if (!priv) {
+		dev_err(dev, "allocating memory failed\n");
+		return -ENOMEM;
+	}
+
+	sm = le32_to_cpu(buf->start_method);
+
+	if (sm == CRB_SM_CRB || sm == CRB_SM_CRB_WITH_ACPI_START)
+		priv->flags |= CRB_FL_CRB_START;
+
+	if (sm == CRB_SM_ACPI_START || sm == CRB_SM_CRB_WITH_ACPI_START)
+		priv->flags |= CRB_FL_ACPI_START;
+
+	priv->dev_handle = device->handle;
+	priv->cca_pa = le32_to_cpu(buf->control_area_pa);
+	priv->cca = (struct crb_control_area *)
+		devm_ioremap_nocache(dev, buf->control_area_pa, 0x1000);
+	if (!priv->cca) {
+		dev_err(dev, "allocating memory failed\n");
+		return -ENOMEM;
+	}
+
+	chip->vendor.priv = priv;
+
+	/* Default timeouts and durations */
+	chip->vendor.timeout_a = usecs_to_jiffies(TPM2_TIMEOUT_A);
+	chip->vendor.timeout_b = usecs_to_jiffies(TPM2_TIMEOUT_B);
+	chip->vendor.timeout_c = usecs_to_jiffies(TPM2_TIMEOUT_C);
+	chip->vendor.timeout_d = usecs_to_jiffies(TPM2_TIMEOUT_D);
+	chip->vendor.duration[TPM_SHORT] =
+		usecs_to_jiffies(TPM2_DURATION_SHORT);
+	chip->vendor.duration[TPM_MEDIUM] =
+		usecs_to_jiffies(TPM2_DURATION_MEDIUM);
+	chip->vendor.duration[TPM_LONG] =
+		usecs_to_jiffies(TPM2_DURATION_LONG);
+
+	rc = tpm2_do_selftest(chip);
+	if (rc) {
+		dev_err(dev, "self-test failed\n");
+		return rc;
+	}
+
+	rc = tpm_chip_register(chip);
+	if (rc) {
+		dev_err(dev, "registering the chip failed\n");
+		return rc;
+	}
+
+	return 0;
+}
+
+int crb_acpi_remove(struct acpi_device *device)
+{
+	struct device *dev = &device->dev;
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	tpm_chip_unregister(chip);
+	return 0;
+}
+
+static struct acpi_device_id crb_device_ids[] = {
+	{"MSFT0101", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, crb_device_ids);
+
+static struct acpi_driver crb_acpi_driver = {
+	.name = "tpm_crb",
+	.ids = crb_device_ids,
+	.ops = {
+		.add = crb_acpi_add,
+		.remove = crb_acpi_remove,
+	},
+	.drv = {
+		.pm = &crb_pm,
+	},
+};
+
+module_acpi_driver(crb_acpi_driver);
+MODULE_AUTHOR("Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>");
+MODULE_DESCRIPTION("TPM2 Driver");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
-- 
2.1.0

^ permalink raw reply related

* [PATCH v3 4/6] tpm: TPM 2.0 sysfs attributes
From: Jarkko Sakkinen @ 2014-10-15 11:35 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
	christophe.ricard, will.c.arthur, monty.wiseman, Jarkko Sakkinen
In-Reply-To: <1413372916-12091-1-git-send-email-jarkko.sakkinen@linux.intel.com>

Implemented sysfs attributes for TPM2 devices. TPM2 sysfs attributes
are mounted in the actual device associated with the chip instead of
platform device like with TPM1 devices.

Documentation/ABI/stable/sysfs-class/tpm2 contains descriptions
of these attributes.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 Documentation/ABI/stable/sysfs-class-tpm2 |  72 +++++++
 drivers/char/tpm/Makefile                 |   2 +-
 drivers/char/tpm/tpm-chip.c               |  12 +-
 drivers/char/tpm/tpm.h                    |  24 +++
 drivers/char/tpm/tpm2-sysfs.c             | 314 ++++++++++++++++++++++++++++++
 5 files changed, 421 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-class-tpm2
 create mode 100644 drivers/char/tpm/tpm2-sysfs.c

diff --git a/Documentation/ABI/stable/sysfs-class-tpm2 b/Documentation/ABI/stable/sysfs-class-tpm2
new file mode 100644
index 0000000..b3d20a4
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-class-tpm2
@@ -0,0 +1,72 @@
+What:		/sys/class/misc/tpmX/device/
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The device/ directory under a specific TPM instance exposes
+		the properties of that TPM chip.
+
+What:		/sys/class/misc/tpmX/device/version
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "version" property prints the protocol version number
+		in the major.minor format.
+
+What:		/sys/class/misc/tpmX/device/sh_enabled
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "sh_enabled" property prints a '1' if the Storage Hierarchy
+		is enabled, i.e. if PM_PT_STARTUP_CLEAR.shEnable is set.
+
+What:		/sys/class/misc/tpmX/device/sh_owned
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "sh_owned" property prints a '1' if the ownership of the
+		Storage Hierarchy has been taken, i.e. if
+		TPM_PT_PERMANENT.ownerAuthSet is set.
+
+What:		/sys/class/misc/tpmX/device/eh_enabled
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "eh_enabled" property prints a '1' if the Endorsement
+		Hierarchy is enabled, i.e if PM_PT_STARTUP_CLEAR.ehEnable is
+		set.
+
+What:		/sys/class/misc/tpmX/device/eh_owned
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "eh_owned" property prints a '1' if the ownership of the
+		Endrosoment Hierarchy has been taken, i.e if
+		TPM_PT_PERMANENT.endorsementAuthSet is set.
+
+What:		/sys/class/misc/tpmX/device/manufacturer
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "manufacturer" property prints the vendor ID of the TPM
+		manufacturer.
+
+What:		/sys/class/misc/tpmX/device/firmware
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The property prints the vendor-specific value indicating the
+		version of the firmware.
+
+What:		/sys/class/misc/tpmX/device/pcr/sha1/X
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	These files print PCR values for the SHA-1 bank.
+
+What:		/sys/class/misc/tpmX/device/cancel
+Date:		October 2014
+KernelVersion:	3.19
+Contact:	tpmdd-devel@lists.sf.net
+Description:	The "cancel" property allows you to cancel the currently
+		pending TPM command. Writing any value to cancel will call the
+		TPM chip specific cancel operation.
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index ae56af9..d3cf905 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o tpm2-sysfs.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 3bb0d9f..5fac0a8 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -140,9 +140,13 @@ int tpm_chip_register(struct tpm_chip *chip)
 		return rc;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		rc = tpm_add_ppi(&chip->vendor.miscdev.this_device->kobj);
+		rc = tpm2_sysfs_add_device(chip);
 		if (rc)
 			goto del_misc;
+
+		rc = tpm_add_ppi(&chip->vendor.miscdev.this_device->kobj);
+		if (rc)
+			goto del_sysfs;
 	} else {
 		rc = tpm_sysfs_add_device(chip);
 		if (rc)
@@ -162,7 +166,10 @@ int tpm_chip_register(struct tpm_chip *chip)
 
 	return 0;
 del_sysfs:
-	tpm_sysfs_del_device(chip);
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		tpm2_sysfs_del_device(chip);
+	else
+		tpm_sysfs_del_device(chip);
 del_misc:
 	tpm_dev_del_device(chip);
 	return rc;
@@ -190,6 +197,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		tpm_remove_ppi(&chip->vendor.miscdev.this_device->kobj);
+		tpm2_sysfs_del_device(chip);
 	} else {
 		tpm_bios_log_teardown(chip->bios_dir);
 		tpm_remove_ppi(&chip->dev->kobj);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index b1de902..9f1f146 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -107,6 +107,24 @@ enum tpm2_capabilities {
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_tpm_properties {
+	TPM2_PT_MANUFACTURER		= 0x00000105,
+	TPM2_PT_FIRMWARE_VERSION_1	= 0x00000111,
+	TPM2_PT_FIRMWARE_VERSION_2	= 0x00000111,
+	TPM2_PT_PERMANENT		= 0x00000200,
+	TPM2_PT_STARTUP_CLEAR		= 0x00000201,
+};
+
+enum tpm2_pt_startup_clear {
+	TPM2_PT_SC_SH_ENABLE	= BIT(1),
+	TPM2_PT_SC_EH_ENABLE	= BIT(2),
+};
+
+enum tpm2_pt_permanent {
+	TPM2_PT_PM_OWNER_AUTH_SET	= BIT(0),
+	TPM2_PT_PM_ENDORSEMENT_AUTH_SET	= BIT(1),
+};
+
 enum tpm2_startup_types {
 	TPM2_SU_CLEAR	= 0x0000,
 	TPM2_SU_STATE	= 0x0001,
@@ -165,6 +183,9 @@ struct tpm_chip {
 
 	struct dentry **bios_dir;
 
+	struct kobject *pcrs_kobj;
+	void *sha1_bank;
+
 	struct list_head list;
 };
 
@@ -416,3 +437,6 @@ extern ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			       u32* value, const char *desc);
 extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
 extern int tpm2_do_selftest(struct tpm_chip *chip);
+
+int tpm2_sysfs_add_device(struct tpm_chip *chip);
+void tpm2_sysfs_del_device(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-sysfs.c b/drivers/char/tpm/tpm2-sysfs.c
new file mode 100644
index 0000000..e502032
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sysfs.c
@@ -0,0 +1,314 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2013 Obsidian Research Corp
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * sysfs filesystem inspection interface to the TPM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+#include <linux/device.h>
+#include <linux/slab.h>
+#include "tpm.h"
+
+static ssize_t sh_enabled_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_STARTUP_CLEAR, &value,
+			     "could not retrieve STARTUP_CLEAR property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", (value & TPM2_PT_SC_SH_ENABLE) > 0);
+	return rc;
+}
+static DEVICE_ATTR_RO(sh_enabled);
+
+static ssize_t sh_owned_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_PERMANENT, &value,
+			     "could not retrieve PERMANENT property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", (value & TPM2_PT_PM_OWNER_AUTH_SET) > 0);
+	return rc;
+}
+static DEVICE_ATTR_RO(sh_owned);
+
+static ssize_t eh_enabled_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_STARTUP_CLEAR, &value,
+			     "could not retrieve STARTUP_CLEAR property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", (value & TPM2_PT_SC_EH_ENABLE) > 0);
+	return rc;
+}
+static DEVICE_ATTR_RO(eh_enabled);
+
+static ssize_t eh_owned_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	u32 value;
+	ssize_t rc;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_PERMANENT, &value,
+			     "could not retrieve PERMANENT property");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", (value & TPM2_PT_PM_ENDORSEMENT_AUTH_SET) > 0);
+	return rc;
+}
+static DEVICE_ATTR_RO(eh_owned);
+
+static ssize_t manufacturer_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	u32 manufacturer;
+	ssize_t rc;
+	char *str = buf;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, (u32 *) &manufacturer,
+			     "could not retrieve MANUFACTURER property");
+	if (rc)
+		return 0;
+
+	str += sprintf(str, "0x%08x\n", be32_to_cpu(manufacturer));
+
+	return str - buf;
+}
+static DEVICE_ATTR_RO(manufacturer);
+
+static ssize_t firmware_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	u32 firmware1;
+	u32 firmware2;
+	ssize_t rc;
+	char *str = buf;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, (u32 *) &firmware1,
+			     "could not retrieve FIRMWARE_VERSION_1 property");
+	if (rc)
+		return 0;
+
+	rc = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, (u32 *) &firmware2,
+			     "could not retrieve FIRMWARE_VERSION_2 property");
+	if (rc)
+		return 0;
+
+	str += sprintf(str, "0x%08x.0x%08x\n", firmware1, firmware2);
+
+	return str - buf;
+}
+static DEVICE_ATTR_RO(firmware);
+
+static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	if (chip == NULL)
+		return 0;
+
+	chip->ops->cancel(chip);
+	return count;
+}
+static DEVICE_ATTR_WO(cancel);
+
+static ssize_t version_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	char *str = buf;
+
+	str += sprintf(str, "2.0\n");
+
+	return str - buf;
+}
+static DEVICE_ATTR_RO(version);
+
+static struct attribute *tpm_dev_attrs[] = {
+	&dev_attr_sh_enabled.attr,
+	&dev_attr_sh_owned.attr,
+	&dev_attr_eh_enabled.attr,
+	&dev_attr_eh_owned.attr,
+	&dev_attr_manufacturer.attr,
+	&dev_attr_firmware.attr,
+	&dev_attr_cancel.attr,
+	&dev_attr_version.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm_dev_group = {
+	.attrs	= tpm_dev_attrs,
+};
+
+struct pcr_attr {
+	struct attribute attr;
+	unsigned int index;
+	char name[3];
+};
+
+struct pcr_bank {
+	struct kobject kobj;
+	struct kobj_type ktype;
+	struct device *dev;
+	struct pcr_attr pcr_attrs[TPM2_PLATFORM_PCR];
+	struct attribute *attrs[TPM2_PLATFORM_PCR + 1];
+};
+
+static ssize_t pcr_bank_attr_show(struct kobject *kobj,
+				  struct attribute *attr,
+				  char *buf)
+{
+	u8 digest[TPM_DIGEST_SIZE];
+	ssize_t rc;
+	int i;
+	char *str = buf;
+	struct tpm_chip *chip;
+	struct pcr_attr *pcr_attr;
+	struct pcr_bank *pcr_bank;
+
+	pcr_attr = container_of(attr, struct pcr_attr, attr);
+	pcr_bank = container_of(kobj, struct pcr_bank, kobj);
+	chip = dev_get_drvdata(pcr_bank->dev);
+
+	rc = tpm2_pcr_read(chip, pcr_attr->index, digest);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < TPM_DIGEST_SIZE; i++)
+		str += sprintf(str, "%02X", digest[i]);
+
+	str += sprintf(str, "\n");
+
+	return str - buf;
+}
+
+static void pcr_bank_release(struct kobject *kobj)
+{
+	struct pcr_bank *pcr_bank;
+	pcr_bank = container_of(kobj, struct pcr_bank, kobj);
+	kfree(pcr_bank);
+}
+
+static const struct sysfs_ops pcr_bank_sysfs_ops = {
+	.show		= pcr_bank_attr_show,
+};
+
+static struct pcr_bank *pcr_bank_create(struct device *dev,
+					struct kobject *parent_kobj)
+{
+	struct pcr_bank *pcr_bank;
+	struct pcr_attr *pcr_attr;
+	struct attribute *attr;
+	int i;
+	int rc;
+
+	pcr_bank = kzalloc(sizeof(*pcr_bank), GFP_KERNEL);
+	if (!pcr_bank)
+		return NULL;
+
+	pcr_bank->dev			= dev;
+	pcr_bank->ktype.sysfs_ops	= &pcr_bank_sysfs_ops;
+	pcr_bank->ktype.default_attrs	= pcr_bank->attrs;
+	pcr_bank->ktype.release		= pcr_bank_release;
+
+	for (i = 0; i < TPM2_PLATFORM_PCR; i++) {
+		pcr_attr = &pcr_bank->pcr_attrs[i];
+		pcr_attr->index = i;
+		sprintf(pcr_attr->name, "%d", i);
+
+		attr = &pcr_attr->attr;
+		attr->name = pcr_attr->name;
+		attr->mode = S_IRUGO;
+
+		pcr_bank->attrs[i] = attr;
+	}
+
+	pcr_bank->attrs[i] = NULL;
+
+	rc = kobject_init_and_add(&pcr_bank->kobj, &pcr_bank->ktype,
+				  parent_kobj, "sha1");
+	if (rc) {
+		kfree(pcr_bank);
+		return NULL;
+	}
+
+	return pcr_bank;
+}
+
+int tpm2_sysfs_add_device(struct tpm_chip *chip)
+{
+	struct pcr_bank *pcr_bank;
+	struct kobject *pcrs_kobj;
+	struct device *dev = chip->dev;
+	int rc;
+
+	rc = sysfs_create_group(&chip->vendor.miscdev.this_device->kobj,
+				&tpm_dev_group);
+	if (rc) {
+		dev_err(dev, "failed to create sysfs attributes, %d\n", rc);
+		return rc;
+	}
+
+	pcrs_kobj = kobject_create_and_add("pcrs",
+					   &chip->vendor.miscdev.this_device->kobj);
+	if (!pcrs_kobj) {
+		sysfs_remove_group(&dev->kobj, &tpm_dev_group);
+		dev_err(dev, "failed to create sysfs attributes, %d\n", rc);
+		return -ENOMEM;
+	}
+
+	pcr_bank = pcr_bank_create(chip->vendor.miscdev.this_device,
+				   pcrs_kobj);
+	if (!pcr_bank) {
+		kobject_put(pcrs_kobj);
+		sysfs_remove_group(&dev->kobj, &tpm_dev_group);
+		dev_err(dev, "failed to create sysfs attributes, %d\n", rc);
+		return -ENOMEM;
+	}
+
+	kobject_uevent(pcrs_kobj, KOBJ_ADD);
+
+	chip = dev_get_drvdata(dev);
+	chip->pcrs_kobj = pcrs_kobj;
+	chip->sha1_bank = &pcr_bank->kobj;
+
+	return 0;
+}
+
+void tpm2_sysfs_del_device(struct tpm_chip *chip)
+{
+	kobject_put(chip->sha1_bank);
+	kobject_put(chip->pcrs_kobj);
+	sysfs_remove_group(&chip->vendor.miscdev.this_device->kobj,
+			   &tpm_dev_group);
+}
-- 
2.1.0

^ permalink raw reply related

* [PATCH v3 3/6] tpm: TPM 2.0 commands
From: Jarkko Sakkinen @ 2014-10-15 11:35 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
	christophe.ricard, will.c.arthur, monty.wiseman, Jarkko Sakkinen
In-Reply-To: <1413372916-12091-1-git-send-email-jarkko.sakkinen@linux.intel.com>

This adds the following internal functions:

- tpm2_get_random()
- tpm2_get_tpm_pt()
- tpm2_pcr_extend()
- tpm2_pcr_read()
- tpm2_startup()

and the following exported functions for implementing new device
drivers:

- tpm2_do_selftest()
- tpm2_calc_ordinal_durations()
- tpm2_gen_interrupt()

Functions that are defined in include/linux/tpm.h and tpm_transmit()
use TPM2 commands for TPM2 chips transparently in order to make the
transition easier for external subsystems. This is achieved with a
new field "flags" in struct tpm_chip and enum tpm_chip_flags.
TPM2 chips must set TPM_CHIP_FLAG_TPM2 after tpm_chip_alloc() in
order to enable TPM2 commands.

PPI attributes are attached to miscdev for TPM2 devices instead
of attaching them to platform device as miscdev represents the
actual device accessed from the user space.

The code for tpm2_calc_ordinal_duration() and tpm2_startup() is
derived from the code originally written by Will Arthur.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Will Arthur <will.c.arthur@intel.com>
---
 drivers/char/tpm/Makefile        |   2 +-
 drivers/char/tpm/tpm-chip.c      |  35 ++-
 drivers/char/tpm/tpm-dev.c       |   3 +
 drivers/char/tpm/tpm-interface.c |  24 +-
 drivers/char/tpm/tpm.h           |  66 +++++
 drivers/char/tpm/tpm2-cmd.c      | 543 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 657 insertions(+), 16 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-cmd.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 837da04..ae56af9 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 4a29421..3bb0d9f 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -139,15 +139,21 @@ int tpm_chip_register(struct tpm_chip *chip)
 	if (rc)
 		return rc;
 
-	rc = tpm_sysfs_add_device(chip);
-	if (rc)
-		goto del_misc;
-
-	rc = tpm_add_ppi(&chip->dev->kobj);
-	if (rc)
-		goto del_sysfs;
-
-	chip->bios_dir = tpm_bios_log_setup(chip->devname);
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		rc = tpm_add_ppi(&chip->vendor.miscdev.this_device->kobj);
+		if (rc)
+			goto del_misc;
+	} else {
+		rc = tpm_sysfs_add_device(chip);
+		if (rc)
+			goto del_misc;
+
+		rc = tpm_add_ppi(&chip->dev->kobj);
+		if (rc)
+			goto del_sysfs;
+
+		chip->bios_dir = tpm_bios_log_setup(chip->devname);
+	}
 
 	/* Make the chip available. */
 	spin_lock(&driver_lock);
@@ -182,9 +188,14 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 	spin_unlock(&driver_lock);
 	synchronize_rcu();
 
-	tpm_sysfs_del_device(chip);
-	tpm_remove_ppi(&chip->dev->kobj);
-	tpm_bios_log_teardown(chip->bios_dir);
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm_remove_ppi(&chip->vendor.miscdev.this_device->kobj);
+	} else {
+		tpm_bios_log_teardown(chip->bios_dir);
+		tpm_remove_ppi(&chip->dev->kobj);
+		tpm_sysfs_del_device(chip);
+	}
+
 	tpm_dev_del_device(chip);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index bd79d33..5dc2d88 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -202,7 +202,10 @@ int tpm_dev_add_device(struct tpm_chip *chip)
 			"unable to misc_register %s, minor %d err=%d\n",
 			chip->vendor.miscdev.name,
 			chip->vendor.miscdev.minor, rc);
+	} else {
+		dev_set_drvdata(chip->vendor.miscdev.this_device, chip);
 	}
+
 	return rc;
 }
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 59d6654..caf712c 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -360,7 +360,10 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 	if (chip->vendor.irq)
 		goto out_recv;
 
-	stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		stop = jiffies + tpm2_calc_ordinal_duration(chip, ordinal);
+	else
+		stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
 	do {
 		u8 status = chip->ops->status(chip);
 		if ((status & chip->ops->req_complete_mask) ==
@@ -482,7 +485,7 @@ static const struct tpm_input_header tpm_startup_header = {
 static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
 {
 	struct tpm_cmd_t start_cmd;
-	start_cmd.header.in = tpm_startup_header;
+
 	start_cmd.params.startup_in.startup_type = startup_type;
 	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
 				"attempting to start the TPM");
@@ -679,7 +682,10 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
 	chip = tpm_chip_find_get(chip_num);
 	if (chip == NULL)
 		return -ENODEV;
-	rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
+	else
+		rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
 	tpm_chip_put(chip);
 	return rc;
 }
@@ -713,6 +719,12 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 	if (chip == NULL)
 		return -ENODEV;
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
+		tpm_chip_put(chip);
+		return rc;
+	}
+
 	cmd.header.in = pcrextend_header;
 	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
 	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
@@ -973,6 +985,12 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 	if (chip == NULL)
 		return -ENODEV;
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		err = tpm2_get_random(chip, out, max);
+		tpm_chip_put(chip);
+		return err;
+	}
+
 	do {
 		tpm_cmd.header.in = tpm_getrandom_header;
 		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index fa0974c..b1de902 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -61,6 +61,57 @@ enum tpm_duration {
 #define TPM_ERR_INVALID_POSTINIT 38
 
 #define TPM_HEADER_SIZE		10
+
+enum tpm2_const {
+	TPM2_PLATFORM_PCR	= 24,
+	TPM2_PCR_SELECT_MIN	= ((TPM2_PLATFORM_PCR + 7) / 8),
+	TPM2_TIMEOUT_A		= 750 * 1000,
+	TPM2_TIMEOUT_B		= 2000 * 1000,
+	TPM2_TIMEOUT_C		= 200 * 1000,
+	TPM2_TIMEOUT_D		= 30 * 1000,
+	TPM2_DURATION_SHORT	= 20 * 1000,
+	TPM2_DURATION_MEDIUM	= 750 * 1000,
+	TPM2_DURATION_LONG	= 2000 * 1000,
+};
+
+enum tpm2_structures {
+	TPM2_ST_NO_SESSIONS	= 0x8001,
+	TPM2_ST_SESSIONS	= 0x8002,
+};
+
+enum tpm2_return_codes {
+	TPM2_RC_TESTING		= 0x090A,
+	TPM2_RC_DISABLED	= 0x0120,
+};
+
+enum tpm2_algorithms {
+	TPM2_ALG_SHA1		= 0x0004,
+};
+
+enum tpm2_command_codes {
+	TPM2_CC_FIRST		= 0x011F,
+	TPM2_CC_SELF_TEST	= 0x0143,
+	TPM2_CC_STARTUP		= 0x0144,
+	TPM2_CC_GET_CAPABILITY	= 0x017A,
+	TPM2_CC_GET_RANDOM	= 0x017B,
+	TPM2_CC_PCR_READ	= 0x017E,
+	TPM2_CC_PCR_EXTEND	= 0x0182,
+	TPM2_CC_LAST		= 0x018F,
+};
+
+enum tpm2_permanent_handles {
+	TPM2_RS_PW		= 0x40000009,
+};
+
+enum tpm2_capabilities {
+	TPM2_CAP_TPM_PROPERTIES = 6,
+};
+
+enum tpm2_startup_types {
+	TPM2_SU_CLEAR	= 0x0000,
+	TPM2_SU_STATE	= 0x0001,
+};
+
 struct tpm_chip;
 
 struct tpm_vendor_specific {
@@ -94,9 +145,14 @@ struct tpm_vendor_specific {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 
+enum tpm_chip_flags {
+	TPM_CHIP_FLAG_TPM2	= BIT(0),
+};
+
 struct tpm_chip {
 	struct device *dev;	/* Device stuff */
 	const struct tpm_class_ops *ops;
+	unsigned int flags;
 
 	int dev_num;		/* /dev/tpm# */
 	char devname[7];
@@ -350,3 +406,13 @@ static inline void tpm_remove_ppi(struct kobject *parent)
 {
 }
 #endif
+
+int tpm2_startup(struct tpm_chip *chip, __be16 startup_type);
+int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
+int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+
+extern ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
+			       u32* value, const char *desc);
+extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
+extern int tpm2_do_selftest(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
new file mode 100644
index 0000000..e0c455d
--- /dev/null
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -0,0 +1,543 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This file contains TPM2 protocol implementations of the commands
+ * used by the kernel internally.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include "tpm.h"
+
+struct tpm2_startup_in {
+	__be16	startup_type;
+} __packed;
+
+struct tpm2_self_test_in {
+	u8	full_test;
+} __packed;
+
+struct tpm2_pcr_read_in {
+	__be32	pcr_selects_cnt;
+	__be16	hash_alg;
+	u8	pcr_select_size;
+	u8	pcr_select[TPM2_PCR_SELECT_MIN];
+} __packed;
+
+struct tpm2_pcr_read_out {
+	__be32	update_cnt;
+	__be32	pcr_selects_cnt;
+	__be16	hash_alg;
+	u8	pcr_select_size;
+	u8	pcr_select[TPM2_PCR_SELECT_MIN];
+	__be32	digests_cnt;
+	__be16	digest_size;
+	u8	digest[TPM_DIGEST_SIZE];
+} __packed;
+
+struct tpm2_null_auth_area {
+	__be32			handle;
+	__be16			nonce_size;
+	u8			attributes;
+	__be16			auth_size;
+} __packed;
+
+struct tpm2_pcr_extend_in {
+	__be32				pcr_idx;
+	__be32				auth_area_size;
+	struct tpm2_null_auth_area	auth_area;
+	__be32				digest_cnt;
+	__be16				hash_alg;
+	u8				digest[TPM_DIGEST_SIZE];
+} __packed;
+
+struct tpm2_get_tpm_pt_in {
+	__be32	cap_id;
+	__be32	property_id;
+	__be32	property_cnt;
+} __packed;
+
+struct tpm2_get_tpm_pt_out {
+	u8	more_data;
+	__be32	subcap_id;
+	__be32	property_cnt;
+	__be32	property_id;
+	__be32	value;
+} __packed;
+
+struct tpm2_get_random_in {
+	__be16	size;
+} __packed;
+
+struct tpm2_get_random_out {
+	__be16	size;
+	u8	buffer[TPM_MAX_RNG_DATA];
+} __packed;
+
+union tpm2_cmd_params {
+	struct	tpm2_startup_in		startup_in;
+	struct	tpm2_self_test_in	selftest_in;
+	struct	tpm2_pcr_read_in	pcrread_in;
+	struct	tpm2_pcr_read_out	pcrread_out;
+	struct	tpm2_pcr_extend_in	pcrextend_in;
+	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
+	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
+	struct	tpm2_get_random_in	getrandom_in;
+	struct	tpm2_get_random_out	getrandom_out;
+};
+
+struct tpm2_cmd {
+	tpm_cmd_header		header;
+	union tpm2_cmd_params	params;
+} __packed;
+
+/*
+ * Array with one entry per ordinal defining the maximum amount
+ * of time the chip could take to return the result. The values
+ * of the SHORT, MEDIUM, and LONG durations are taken from the
+ * PC Client Profile (PTP) specification.
+ */
+static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
+	TPM_UNDEFINED,		/* 11F */
+	TPM_UNDEFINED,		/* 120 */
+	TPM_LONG,		/* 121 */
+	TPM_UNDEFINED,		/* 122 */
+	TPM_UNDEFINED,		/* 123 */
+	TPM_UNDEFINED,		/* 124 */
+	TPM_UNDEFINED,		/* 125 */
+	TPM_UNDEFINED,		/* 126 */
+	TPM_UNDEFINED,		/* 127 */
+	TPM_UNDEFINED,		/* 128 */
+	TPM_LONG,		/* 129 */
+	TPM_UNDEFINED,		/* 12a */
+	TPM_UNDEFINED,		/* 12b */
+	TPM_UNDEFINED,		/* 12c */
+	TPM_UNDEFINED,		/* 12d */
+	TPM_UNDEFINED,		/* 12e */
+	TPM_UNDEFINED,		/* 12f */
+	TPM_UNDEFINED,		/* 130 */
+	TPM_UNDEFINED,		/* 131 */
+	TPM_UNDEFINED,		/* 132 */
+	TPM_UNDEFINED,		/* 133 */
+	TPM_UNDEFINED,		/* 134 */
+	TPM_UNDEFINED,		/* 135 */
+	TPM_UNDEFINED,		/* 136 */
+	TPM_UNDEFINED,		/* 137 */
+	TPM_UNDEFINED,		/* 138 */
+	TPM_UNDEFINED,		/* 139 */
+	TPM_UNDEFINED,		/* 13a */
+	TPM_UNDEFINED,		/* 13b */
+	TPM_UNDEFINED,		/* 13c */
+	TPM_UNDEFINED,		/* 13d */
+	TPM_MEDIUM,		/* 13e */
+	TPM_UNDEFINED,		/* 13f */
+	TPM_UNDEFINED,		/* 140 */
+	TPM_UNDEFINED,		/* 141 */
+	TPM_UNDEFINED,		/* 142 */
+	TPM_LONG,		/* 143 */
+	TPM_MEDIUM,		/* 144 */
+	TPM_UNDEFINED,		/* 145 */
+	TPM_UNDEFINED,		/* 146 */
+	TPM_UNDEFINED,		/* 147 */
+	TPM_UNDEFINED,		/* 148 */
+	TPM_UNDEFINED,		/* 149 */
+	TPM_UNDEFINED,		/* 14a */
+	TPM_UNDEFINED,		/* 14b */
+	TPM_UNDEFINED,		/* 14c */
+	TPM_UNDEFINED,		/* 14d */
+	TPM_LONG,		/* 14e */
+	TPM_UNDEFINED,		/* 14f */
+	TPM_UNDEFINED,		/* 150 */
+	TPM_UNDEFINED,		/* 151 */
+	TPM_UNDEFINED,		/* 152 */
+	TPM_UNDEFINED,		/* 153 */
+	TPM_UNDEFINED,		/* 154 */
+	TPM_UNDEFINED,		/* 155 */
+	TPM_UNDEFINED,		/* 156 */
+	TPM_UNDEFINED,		/* 157 */
+	TPM_UNDEFINED,		/* 158 */
+	TPM_UNDEFINED,		/* 159 */
+	TPM_UNDEFINED,		/* 15a */
+	TPM_UNDEFINED,		/* 15b */
+	TPM_MEDIUM,		/* 15c */
+	TPM_UNDEFINED,		/* 15d */
+	TPM_UNDEFINED,		/* 15e */
+	TPM_UNDEFINED,		/* 15f */
+	TPM_UNDEFINED,		/* 160 */
+	TPM_UNDEFINED,		/* 161 */
+	TPM_UNDEFINED,		/* 162 */
+	TPM_UNDEFINED,		/* 163 */
+	TPM_UNDEFINED,		/* 164 */
+	TPM_UNDEFINED,		/* 165 */
+	TPM_UNDEFINED,		/* 166 */
+	TPM_UNDEFINED,		/* 167 */
+	TPM_UNDEFINED,		/* 168 */
+	TPM_UNDEFINED,		/* 169 */
+	TPM_UNDEFINED,		/* 16a */
+	TPM_UNDEFINED,		/* 16b */
+	TPM_UNDEFINED,		/* 16c */
+	TPM_UNDEFINED,		/* 16d */
+	TPM_UNDEFINED,		/* 16e */
+	TPM_UNDEFINED,		/* 16f */
+	TPM_UNDEFINED,		/* 170 */
+	TPM_UNDEFINED,		/* 171 */
+	TPM_UNDEFINED,		/* 172 */
+	TPM_UNDEFINED,		/* 173 */
+	TPM_UNDEFINED,		/* 174 */
+	TPM_UNDEFINED,		/* 175 */
+	TPM_UNDEFINED,		/* 176 */
+	TPM_LONG,		/* 177 */
+	TPM_UNDEFINED,		/* 178 */
+	TPM_UNDEFINED,		/* 179 */
+	TPM_MEDIUM,		/* 17a */
+	TPM_LONG,		/* 17b */
+	TPM_UNDEFINED,		/* 17c */
+	TPM_UNDEFINED,		/* 17d */
+	TPM_UNDEFINED,		/* 17e */
+	TPM_UNDEFINED,		/* 17f */
+	TPM_UNDEFINED,		/* 180 */
+	TPM_UNDEFINED,		/* 181 */
+	TPM_MEDIUM,		/* 182 */
+	TPM_UNDEFINED,		/* 183 */
+	TPM_UNDEFINED,		/* 184 */
+	TPM_MEDIUM,		/* 185 */
+	TPM_MEDIUM,		/* 186 */
+	TPM_UNDEFINED,		/* 187 */
+	TPM_UNDEFINED,		/* 188 */
+	TPM_UNDEFINED,		/* 189 */
+	TPM_UNDEFINED,		/* 18a */
+	TPM_UNDEFINED,		/* 18b */
+	TPM_UNDEFINED,		/* 18c */
+	TPM_UNDEFINED,		/* 18d */
+	TPM_UNDEFINED,		/* 18e */
+	TPM_UNDEFINED		/* 18f */
+};
+
+static const struct tpm_input_header tpm2_startup_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(12),
+	.ordinal = cpu_to_be32(TPM2_CC_STARTUP)
+};
+
+/**
+ * tpm2_startup() - send startup command to the TPM chip
+ * @chip:		TPM chip to use.
+ * @startup_type	startup type. The value is either
+ * 			TPM_SU_CLEAR or TPM_SU_STATE.
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+int tpm2_startup(struct tpm_chip *chip, __be16 startup_type)
+{
+	struct tpm2_cmd cmd;
+
+	cmd.header.in = tpm2_startup_header;
+
+	cmd.params.startup_in.startup_type = startup_type;
+	return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+				"attempting to start the TPM");
+}
+
+#define TPM2_PCR_READ_IN_SIZE \
+	(sizeof(struct tpm_input_header) + \
+	 sizeof(struct tpm2_pcr_read_in))
+
+static const struct tpm_input_header tpm2_pcrread_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
+	.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
+};
+
+/**
+ * tpm2_pcr_read() - read a PCR value
+ * @chip:	TPM chip to use.
+ * @pcr_idx:	index of the PCR to read.
+ * @ref_buf:	buffer to store the resulting hash,
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+{
+	int rc;
+	struct tpm2_cmd cmd;
+	u8 *buf;
+	int i, j;
+
+	if (pcr_idx >= TPM2_PLATFORM_PCR)
+		return -EINVAL;
+
+	cmd.header.in = tpm2_pcrread_header;
+	cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
+	cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+	cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
+
+	for (i = 0; i < TPM2_PCR_SELECT_MIN; i++) {
+		j = pcr_idx - i * 8;
+
+		cmd.params.pcrread_in.pcr_select[i] =
+			(j >= 0 && j < 8) ? 1 << j : 0;
+	}
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+			      "attempting to read a pcr value");
+
+	if (rc == 0) {
+		buf = cmd.params.pcrread_out.digest;
+		memcpy(res_buf, buf, TPM_DIGEST_SIZE);
+	}
+
+	return rc;
+}
+
+/**
+ * tpm2_pcr_extend() - extend a PCR value
+ * @chip:	TPM chip to use.
+ * @pcr_idx:	index of the PCR.
+ * @hash:	hash value to use for the extend operation.
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+static const struct tpm_input_header tpm2_pcrextend_header = {
+	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
+	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
+			      sizeof(struct tpm2_pcr_extend_in)),
+	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
+};
+
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
+{
+	struct tpm2_cmd cmd;
+	int rc;
+
+	cmd.header.in = tpm2_pcrextend_header;
+	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
+	cmd.params.pcrextend_in.auth_area_size =
+		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
+	cmd.params.pcrextend_in.auth_area.handle =
+		cpu_to_be32(TPM2_RS_PW);
+	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
+	cmd.params.pcrextend_in.auth_area.attributes = 0;
+	cmd.params.pcrextend_in.auth_area.auth_size = 0;
+	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
+	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+			      "attempting extend a PCR value");
+
+	return rc;
+}
+
+static const struct tpm_input_header tpm2_getrandom_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
+			      sizeof(struct tpm2_get_random_in)),
+	.ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
+};
+
+/**
+ * tpm2_get_random() - get random bytes from the TPM RNG
+ * @chip: TPM chip to use
+ * @out: destination buffer for the random bytes
+ * @max: the max number of bytes to write to @out
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+{
+	struct tpm2_cmd cmd;
+	u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
+	int err, total = 0, retries = 5;
+	u8 *dest = out;
+
+	if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
+		return -EINVAL;
+
+	do {
+		cmd.header.in = tpm2_getrandom_header;
+		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
+
+		err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+				       "attempting get random");
+		if (err)
+			break;
+
+		recd = be16_to_cpu(cmd.params.getrandom_out.size);
+		memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+
+		dest += recd;
+		total += recd;
+		num_bytes -= recd;
+	} while (retries-- && total < max);
+
+	return total ? total : -EIO;
+}
+
+#define TPM2_GET_TPM_PT_IN_SIZE \
+	(sizeof(struct tpm_input_header) + \
+	 sizeof(struct tpm2_get_tpm_pt_in))
+
+static const struct tpm_input_header tpm2_get_tpm_pt_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
+	.ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
+};
+
+/**
+ * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
+ * @chip:		TPM chip to use.
+ * @property_id:	property ID.
+ * @value:		output variable.
+ * @desc:		passed to tpm_transmit_cmd()
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32* value,
+			const char *desc)
+{
+	struct tpm2_cmd cmd;
+	int rc;
+
+	cmd.header.in = tpm2_get_tpm_pt_header;
+	cmd.params.get_tpm_pt_in.cap_id = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES);
+	cmd.params.get_tpm_pt_in.property_id = property_id;
+	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
+
+	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
+	if (!rc)
+		*value = cmd.params.get_tpm_pt_out.value;
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
+
+/*
+ * tpm2_calc_ordinal_duration() - maximum duration for a command
+ * @chip: 	TPM chip to use.
+ * @ordinal:	command code number.
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
+{
+	int index = TPM_UNDEFINED;
+	int duration = 0;
+
+	if (ordinal >= TPM2_CC_FIRST && ordinal <= TPM2_CC_LAST)
+		index = tpm2_ordinal_duration[ordinal - TPM2_CC_FIRST];
+
+	if (index != TPM_UNDEFINED)
+		duration = chip->vendor.duration[index];
+	if (duration <= 0)
+		return 2 * 60 * HZ;
+	else
+		return duration;
+}
+EXPORT_SYMBOL_GPL(tpm2_calc_ordinal_duration);
+
+static const struct tpm_input_header tpm2_selftest_header = {
+	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
+	.length = cpu_to_be32(sizeof(struct tpm_input_header) +
+			      sizeof(struct tpm2_self_test_in)),
+	.ordinal = cpu_to_be32(TPM2_CC_SELF_TEST)
+};
+
+#define TPM2_SELF_TEST_IN_SIZE \
+	(sizeof(struct tpm_input_header) + sizeof(struct tpm2_self_test_in))
+
+/**
+ * tpm2_continue_selftest() - start a self test
+ * @chip: TPM chip to use
+ * @full: test all commands instead of testing only those that were not
+ *        previously tested.
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
+{
+	int rc;
+	struct tpm2_cmd cmd;
+
+	cmd.header.in = tpm2_selftest_header;
+	cmd.params.selftest_in.full_test = full;
+
+	rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
+			      "continue selftest");
+
+	return rc;
+}
+
+/**
+ * tpm2_do_selftest() - run a full self test
+ * @chip: TPM chip to use
+ *
+ * During the self test TPM2 commands return with the error code RC_TESTING.
+ * Waiting is done by issuing PCR read until it executes succesfully.
+ *
+ * 0 is returned when the operation is succesful. When a negative number is
+ * returned it remarks a POSIX error code. When a positive number is returned
+ * it remarks a TPM error.
+ */
+int tpm2_do_selftest(struct tpm_chip *chip)
+{
+	int rc;
+	unsigned int loops;
+	unsigned int delay_msec = 100;
+	unsigned long duration;
+	struct tpm2_cmd cmd;
+	int i;
+
+	duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
+
+	loops = jiffies_to_msecs(duration) / delay_msec;
+
+	rc = tpm2_start_selftest(chip, true);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < loops; i++) {
+		/* Attempt to read a PCR value */
+		cmd.header.in = tpm2_pcrread_header;
+		cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
+		cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
+		cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
+		cmd.params.pcrread_in.pcr_select[0] = 0x01;
+		cmd.params.pcrread_in.pcr_select[1] = 0x00;
+		cmd.params.pcrread_in.pcr_select[2] = 0x00;
+
+		rc = tpm_transmit_cmd(chip, (u8 *) &cmd, sizeof(cmd), NULL);
+		if (rc < 0)
+			break;
+
+		rc = be32_to_cpu(cmd.header.out.return_code);
+		if (rc != TPM2_RC_TESTING)
+			break;
+
+		msleep(delay_msec);
+	}
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm2_do_selftest);
-- 
2.1.0

^ permalink raw reply related

* [PATCH v3 2/6] tpm: two-phase chip management functions
From: Jarkko Sakkinen @ 2014-10-15 11:35 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
	christophe.ricard, will.c.arthur, monty.wiseman, Jarkko Sakkinen
In-Reply-To: <1413372916-12091-1-git-send-email-jarkko.sakkinen@linux.intel.com>

tpm_register_hardware() and tpm_remove_hardware() are called often
before initializing the device. This is wrong order since it could
be that main TPM driver needs a fully initialized chip to be able to
do its job. For example, now it is impossible to move common startup
functions such as tpm_do_selftest() to tpm_register_hardware().

Added tpm_chip_alloc() and tpm_chip_register() where tpm_chip_alloc()
reserves memory resources and tpm_chip_register() initializes the
device driver. This way it is possible to alter struct tpm_chip
attributes and initialize the device driver before passing it to
tpm_chip_register().

The framework takes care of freeing struct tpm_chip by using devres
API. The broken release callback has been wiped. For example, ACPI
drivers do not ever get this callback.

This is a interm step to get proper life-cycle for TPM device drivers.
The next steps are adding proper ref counting and locking to tpm_chip
that is used in every place in the TPM driver.

Big thank you to Jason Gunthorpe for carefully reviewing this part
of the code. Without his contribution reaching the best quality would
not have been possible.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Makefile           |   2 +-
 drivers/char/tpm/tpm-chip.c         | 190 ++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-interface.c    | 148 +---------------------------
 drivers/char/tpm/tpm.h              |  11 ++-
 drivers/char/tpm/tpm_atmel.c        |  11 ++-
 drivers/char/tpm/tpm_i2c_atmel.c    |  33 +++----
 drivers/char/tpm/tpm_i2c_infineon.c |  37 ++-----
 drivers/char/tpm/tpm_i2c_nuvoton.c  |  44 +++------
 drivers/char/tpm/tpm_i2c_stm_st33.c |  22 ++---
 drivers/char/tpm/tpm_ibmvtpm.c      |  17 ++--
 drivers/char/tpm/tpm_infineon.c     |  13 ++-
 drivers/char/tpm/tpm_nsc.c          |  11 ++-
 drivers/char/tpm/tpm_tis.c          |  79 ++++++---------
 drivers/char/tpm/xen-tpmfront.c     |  14 +--
 14 files changed, 316 insertions(+), 316 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-chip.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 4d85dd6..837da04 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
new file mode 100644
index 0000000..4a29421
--- /dev/null
+++ b/drivers/char/tpm/tpm-chip.c
@@ -0,0 +1,190 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * TPM chip management routines.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/freezer.h>
+#include "tpm.h"
+#include "tpm_eventlog.h"
+
+static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
+static LIST_HEAD(tpm_chip_list);
+static DEFINE_SPINLOCK(driver_lock);
+
+/*
+ * tpm_chip_find_get - return tpm_chip for a given chip number
+ * @chip_num the device number for the chip
+ */
+struct tpm_chip *tpm_chip_find_get(int chip_num)
+{
+	struct tpm_chip *pos, *chip = NULL;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
+		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
+			continue;
+
+		if (try_module_get(pos->dev->driver->owner)) {
+			chip = pos;
+			break;
+		}
+	}
+	rcu_read_unlock();
+	return chip;
+}
+
+/**
+ * tpmm_chip_remove() - free chip memory and device number
+ * @data: points to struct tpm_chip instance
+ *
+ * This is used internally by tpmm_chip_alloc() and called by devres
+ * when the device is released. This funcion does the opposite of
+ * tpmm_chip_alloc() freeing memory and the device number.
+ */
+static void tpmm_chip_remove(void *data)
+{
+	struct tpm_chip *chip = (struct tpm_chip *) data;
+	dev_dbg(chip->dev, "%s\n", __func__);
+	clear_bit(chip->dev_num, dev_mask);
+	kfree(chip);
+}
+
+/**
+ * tpmm_chip_alloc() - allocate a new struct tpm_chip instance
+ * @dev: device to which the chip is associated
+ * @ops: struct tpm_class_ops instance
+ *
+ * Allocates a new struct tpm_chip instance and assigns a free
+ * device number for it. Caller does not have to worry about
+ * freeing the allocated resources. When the devices is removed
+ * devres calls tpmm_chip_remove() to do the job.
+ */
+struct tpm_chip *tpmm_chip_alloc(struct device *dev,
+				const struct tpm_class_ops *ops)
+{
+	struct tpm_chip *chip;
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	if (chip == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_init(&chip->tpm_mutex);
+	INIT_LIST_HEAD(&chip->list);
+
+	chip->ops = ops;
+	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
+
+	if (chip->dev_num >= TPM_NUM_DEVICES) {
+		dev_err(dev, "No available tpm device numbers\n");
+		kfree(chip);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	set_bit(chip->dev_num, dev_mask);
+
+	scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
+		  chip->dev_num);
+
+	chip->dev = dev;
+	devm_add_action(dev, tpmm_chip_remove, chip);
+	dev_set_drvdata(dev, chip);
+
+	return chip;
+}
+EXPORT_SYMBOL_GPL(tpmm_chip_alloc);
+
+/*
+ * tpm_chip_register() - create a misc driver for the TPM chip
+ * @chip: TPM chip to use.
+ *
+ * Creates a misc driver for the TPM chip and adds sysfs interfaces for
+ * the device, PPI and TCPA. As the last step this function adds the
+ * chip to the list of TPM chips available for use.
+ *
+ * NOTE: This function should be only called after the chip initialization
+ * is complete.
+ *
+ * Called from tpm_<specific>.c probe function only for devices
+ * the driver has determined it should claim.  Prior to calling
+ * this function the specific probe function has called pci_enable_device
+ * upon errant exit from this function specific probe function should call
+ * pci_disable_device
+ */
+int tpm_chip_register(struct tpm_chip *chip)
+{
+	int rc;
+
+	rc = tpm_dev_add_device(chip);
+	if (rc)
+		return rc;
+
+	rc = tpm_sysfs_add_device(chip);
+	if (rc)
+		goto del_misc;
+
+	rc = tpm_add_ppi(&chip->dev->kobj);
+	if (rc)
+		goto del_sysfs;
+
+	chip->bios_dir = tpm_bios_log_setup(chip->devname);
+
+	/* Make the chip available. */
+	spin_lock(&driver_lock);
+	list_add_rcu(&chip->list, &tpm_chip_list);
+	spin_unlock(&driver_lock);
+
+	return 0;
+del_sysfs:
+	tpm_sysfs_del_device(chip);
+del_misc:
+	tpm_dev_del_device(chip);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_register);
+
+/*
+ * tpm_chip_unregister() - release the TPM driver
+ * @chip: TPM chip to use.
+ *
+ * Takes the chip first away from the list of available TPM chips and then
+ * cleans up all the resources reserved by tpm_chip_register().
+ *
+ * NOTE: This function should be only called before deinitializing chip
+ * resources.
+ */
+void tpm_chip_unregister(struct tpm_chip *chip)
+{
+	dev_dbg(chip->dev, "%s\n", __func__);
+
+	spin_lock(&driver_lock);
+	list_del_rcu(&chip->list);
+	spin_unlock(&driver_lock);
+	synchronize_rcu();
+
+	tpm_sysfs_del_device(chip);
+	tpm_remove_ppi(&chip->dev->kobj);
+	tpm_bios_log_teardown(chip->bios_dir);
+	tpm_dev_del_device(chip);
+}
+EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fedb4d5..59d6654 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
  *
  * Authors:
  * Leendert van Doorn <leendert@watson.ibm.com>
@@ -47,10 +48,6 @@ module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 0644);
 MODULE_PARM_DESC(suspend_pcr,
 		 "PCR to use for dummy writes to faciltate flush on suspend.");
 
-static LIST_HEAD(tpm_chip_list);
-static DEFINE_SPINLOCK(driver_lock);
-static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
-
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result.  The ordinal
@@ -639,27 +636,6 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 	return rc;
 }
 
-/*
- * tpm_chip_find_get - return tpm_chip for given chip number
- */
-static struct tpm_chip *tpm_chip_find_get(int chip_num)
-{
-	struct tpm_chip *pos, *chip = NULL;
-
-	rcu_read_lock();
-	list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
-		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
-			continue;
-
-		if (try_module_get(pos->dev->driver->owner)) {
-			chip = pos;
-			break;
-		}
-	}
-	rcu_read_unlock();
-	return chip;
-}
-
 #define TPM_ORDINAL_PCRREAD cpu_to_be32(21)
 #define READ_PCR_RESULT_SIZE 30
 static struct tpm_input_header pcrread_header = {
@@ -887,30 +863,6 @@ again:
 }
 EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
 
-void tpm_remove_hardware(struct device *dev)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	if (chip == NULL) {
-		dev_err(dev, "No device data found\n");
-		return;
-	}
-
-	spin_lock(&driver_lock);
-	list_del_rcu(&chip->list);
-	spin_unlock(&driver_lock);
-	synchronize_rcu();
-
-	tpm_dev_del_device(chip);
-	tpm_sysfs_del_device(chip);
-	tpm_remove_ppi(&dev->kobj);
-	tpm_bios_log_teardown(chip->bios_dir);
-
-	/* write it this way to be explicit (chip->dev == dev) */
-	put_device(chip->dev);
-}
-EXPORT_SYMBOL_GPL(tpm_remove_hardware);
-
 #define TPM_ORD_SAVESTATE cpu_to_be32(152)
 #define SAVESTATE_RESULT_SIZE 10
 
@@ -1044,104 +996,6 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 }
 EXPORT_SYMBOL_GPL(tpm_get_random);
 
-/* In case vendor provided release function, call it too.*/
-
-void tpm_dev_vendor_release(struct tpm_chip *chip)
-{
-	if (!chip)
-		return;
-
-	clear_bit(chip->dev_num, dev_mask);
-}
-EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
-
-
-/*
- * Once all references to platform device are down to 0,
- * release all allocated structures.
- */
-static void tpm_dev_release(struct device *dev)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	if (!chip)
-		return;
-
-	tpm_dev_vendor_release(chip);
-
-	chip->release(dev);
-	kfree(chip);
-}
-
-/*
- * Called from tpm_<specific>.c probe function only for devices
- * the driver has determined it should claim.  Prior to calling
- * this function the specific probe function has called pci_enable_device
- * upon errant exit from this function specific probe function should call
- * pci_disable_device
- */
-struct tpm_chip *tpm_register_hardware(struct device *dev,
-				       const struct tpm_class_ops *ops)
-{
-	struct tpm_chip *chip;
-
-	/* Driver specific per-device data */
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-
-	if (chip == NULL)
-		return NULL;
-
-	mutex_init(&chip->tpm_mutex);
-	INIT_LIST_HEAD(&chip->list);
-
-	chip->ops = ops;
-	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
-
-	if (chip->dev_num >= TPM_NUM_DEVICES) {
-		dev_err(dev, "No available tpm device numbers\n");
-		goto out_free;
-	}
-
-	set_bit(chip->dev_num, dev_mask);
-
-	scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
-		  chip->dev_num);
-
-	chip->dev = get_device(dev);
-	chip->release = dev->release;
-	dev->release = tpm_dev_release;
-	dev_set_drvdata(dev, chip);
-
-	if (tpm_dev_add_device(chip))
-		goto put_device;
-
-	if (tpm_sysfs_add_device(chip))
-		goto del_misc;
-
-	if (tpm_add_ppi(&dev->kobj))
-		goto del_sysfs;
-
-	chip->bios_dir = tpm_bios_log_setup(chip->devname);
-
-	/* Make chip available */
-	spin_lock(&driver_lock);
-	list_add_rcu(&chip->list, &tpm_chip_list);
-	spin_unlock(&driver_lock);
-
-	return chip;
-
-del_sysfs:
-	tpm_sysfs_del_device(chip);
-del_misc:
-	tpm_dev_del_device(chip);
-put_device:
-	put_device(chip->dev);
-out_free:
-	kfree(chip);
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(tpm_register_hardware);
-
 MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
 MODULE_DESCRIPTION("TPM Driver");
 MODULE_VERSION("2.0");
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 12326e1..fa0974c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -110,7 +110,6 @@ struct tpm_chip {
 	struct dentry **bios_dir;
 
 	struct list_head list;
-	void (*release) (struct device *);
 };
 
 #define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
@@ -320,15 +319,17 @@ extern int tpm_get_timeouts(struct tpm_chip *);
 extern void tpm_gen_interrupt(struct tpm_chip *);
 extern int tpm_do_selftest(struct tpm_chip *);
 extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
-extern struct tpm_chip* tpm_register_hardware(struct device *,
-					      const struct tpm_class_ops *ops);
-extern void tpm_dev_vendor_release(struct tpm_chip *);
-extern void tpm_remove_hardware(struct device *);
 extern int tpm_pm_suspend(struct device *);
 extern int tpm_pm_resume(struct device *);
 extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
 			     wait_queue_head_t *, bool);
 
+struct tpm_chip *tpm_chip_find_get(int chip_num);
+extern struct tpm_chip *tpmm_chip_alloc(struct device *dev,
+				       const struct tpm_class_ops *ops);
+extern int tpm_chip_register(struct tpm_chip *chip);
+extern void tpm_chip_unregister(struct tpm_chip *chip);
+
 int tpm_dev_add_device(struct tpm_chip *chip);
 void tpm_dev_del_device(struct tpm_chip *chip);
 int tpm_sysfs_add_device(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index 6069d13..8e2576a 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -138,11 +138,11 @@ static void atml_plat_remove(void)
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
 
 	if (chip) {
+		tpm_chip_unregister(chip);
 		if (chip->vendor.have_region)
 			atmel_release_region(chip->vendor.base,
 					     chip->vendor.region_size);
 		atmel_put_base_addr(chip->vendor.iobase);
-		tpm_remove_hardware(chip->dev);
 		platform_device_unregister(pdev);
 	}
 }
@@ -184,8 +184,9 @@ static int __init init_atmel(void)
 		goto err_rel_reg;
 	}
 
-	if (!(chip = tpm_register_hardware(&pdev->dev, &tpm_atmel))) {
-		rc = -ENODEV;
+	chip = tpmm_chip_alloc(&pdev->dev, &tpm_atmel);
+	if (IS_ERR(chip)) {
+		rc = PTR_ERR(chip);
 		goto err_unreg_dev;
 	}
 
@@ -194,6 +195,10 @@ static int __init init_atmel(void)
 	chip->vendor.have_region = have_region;
 	chip->vendor.region_size = region_size;
 
+	rc = tpm_chip_register(chip);
+	if (rc)
+		goto err_unreg_dev;
+
 	return 0;
 
 err_unreg_dev:
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 7727292..8af3b4a 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -160,11 +160,9 @@ static int i2c_atmel_probe(struct i2c_client *client,
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
 		return -ENODEV;
 
-	chip = tpm_register_hardware(dev, &i2c_atmel);
-	if (!chip) {
-		dev_err(dev, "%s() error in tpm_register_hardware\n", __func__);
-		return -ENODEV;
-	}
+	chip = tpmm_chip_alloc(dev, &i2c_atmel);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
 
 	chip->vendor.priv = devm_kzalloc(dev, sizeof(struct priv_data),
 					 GFP_KERNEL);
@@ -179,21 +177,16 @@ static int i2c_atmel_probe(struct i2c_client *client,
 	/* There is no known way to probe for this device, and all version
 	 * information seems to be read via TPM commands. Thus we rely on the
 	 * TPM startup process in the common code to detect the device. */
-	if (tpm_get_timeouts(chip)) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	if (tpm_get_timeouts(chip))
+		return -ENODEV;
 
-	if (tpm_do_selftest(chip)) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	if (tpm_do_selftest(chip))
+		return -ENODEV;
 
-	return 0;
+	rc = tpm_chip_register(chip);
+	if (rc)
+		return rc;
 
-out_err:
-	tpm_dev_vendor_release(chip);
-	tpm_remove_hardware(chip->dev);
 	return rc;
 }
 
@@ -201,11 +194,7 @@ static int i2c_atmel_remove(struct i2c_client *client)
 {
 	struct device *dev = &(client->dev);
 	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	if (chip)
-		tpm_dev_vendor_release(chip);
-	tpm_remove_hardware(dev);
-	kfree(chip);
+	tpm_chip_unregister(chip);
 	return 0;
 }
 
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 472af4b..6f00bc3 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -581,10 +581,9 @@ static int tpm_tis_i2c_init(struct device *dev)
 	int rc = 0;
 	struct tpm_chip *chip;
 
-	chip = tpm_register_hardware(dev, &tpm_tis_i2c);
-	if (!chip) {
-		dev_err(dev, "could not register hardware\n");
-		rc = -ENODEV;
+	chip = tpmm_chip_alloc(dev, &tpm_tis_i2c);
+	if (IS_ERR(chip)) {
+		rc = PTR_ERR(chip);
 		goto out_err;
 	}
 
@@ -600,7 +599,7 @@ static int tpm_tis_i2c_init(struct device *dev)
 	if (request_locality(chip, 0) != 0) {
 		dev_err(dev, "could not request locality\n");
 		rc = -ENODEV;
-		goto out_vendor;
+		goto out_err;
 	}
 
 	/* read four bytes from DID_VID register */
@@ -628,21 +627,13 @@ static int tpm_tis_i2c_init(struct device *dev)
 	tpm_get_timeouts(chip);
 	tpm_do_selftest(chip);
 
-	return 0;
+	rc = tpm_chip_register(chip);
+	if (rc)
+		goto out_release;
 
+	return 0;
 out_release:
 	release_locality(chip, chip->vendor.locality, 1);
-
-out_vendor:
-	/* close file handles */
-	tpm_dev_vendor_release(chip);
-
-	/* remove hardware */
-	tpm_remove_hardware(chip->dev);
-
-	/* reset these pointers, otherwise we oops */
-	chip->dev->release = NULL;
-	chip->release = NULL;
 	tpm_dev.client = NULL;
 out_err:
 	return rc;
@@ -712,17 +703,9 @@ static int tpm_tis_i2c_probe(struct i2c_client *client,
 static int tpm_tis_i2c_remove(struct i2c_client *client)
 {
 	struct tpm_chip *chip = tpm_dev.chip;
-	release_locality(chip, chip->vendor.locality, 1);
 
-	/* close file handles */
-	tpm_dev_vendor_release(chip);
-
-	/* remove hardware */
-	tpm_remove_hardware(chip->dev);
-
-	/* reset these pointers, otherwise we oops */
-	chip->dev->release = NULL;
-	chip->release = NULL;
+	tpm_chip_unregister(chip);
+	release_locality(chip, chip->vendor.locality, 1);
 	tpm_dev.client = NULL;
 
 	return 0;
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 7b158ef..09f0c46 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -530,11 +530,9 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
 	dev_info(dev, "VID: %04X DID: %02X RID: %02X\n", (u16) vid,
 		 (u8) (vid >> 16), (u8) (vid >> 24));
 
-	chip = tpm_register_hardware(dev, &tpm_i2c);
-	if (!chip) {
-		dev_err(dev, "%s() error in tpm_register_hardware\n", __func__);
-		return -ENODEV;
-	}
+	chip = tpmm_chip_alloc(dev, &tpm_i2c);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
 
 	chip->vendor.priv = devm_kzalloc(dev, sizeof(struct priv_data),
 					 GFP_KERNEL);
@@ -584,7 +582,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
 							   TPM_DATA_FIFO_W,
 							   1, (u8 *) (&rc));
 				if (rc < 0)
-					goto out_err;
+					return rc;
 				/* TPM_STS <- 0x40 (commandReady) */
 				i2c_nuvoton_ready(chip);
 			} else {
@@ -594,45 +592,33 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
 				 * only TPM_STS_VALID should be set
 				 */
 				if (i2c_nuvoton_read_status(chip) !=
-				    TPM_STS_VALID) {
-					rc = -EIO;
-					goto out_err;
-				}
+				    TPM_STS_VALID)
+					return -EIO;
 			}
 		}
 	}
 
-	if (tpm_get_timeouts(chip)) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	if (tpm_get_timeouts(chip))
+		return -ENODEV;
 
-	if (tpm_do_selftest(chip)) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	if (tpm_do_selftest(chip))
+		return -ENODEV;
 
-	return 0;
+	rc = tpm_chip_register(chip);
+	if (rc)
+		return rc;
 
-out_err:
-	tpm_dev_vendor_release(chip);
-	tpm_remove_hardware(chip->dev);
-	return rc;
+	return 0;
 }
 
 static int i2c_nuvoton_remove(struct i2c_client *client)
 {
 	struct device *dev = &(client->dev);
 	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	if (chip)
-		tpm_dev_vendor_release(chip);
-	tpm_remove_hardware(dev);
-	kfree(chip);
+	tpm_chip_unregister(chip);
 	return 0;
 }
 
-
 static const struct i2c_device_id i2c_nuvoton_id[] = {
 	{I2C_DRIVER_NAME, 0},
 	{}
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index 4669e37..dd32429 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -619,10 +619,9 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		goto end;
 	}
 
-	chip = tpm_register_hardware(&client->dev, &st_i2c_tpm);
-	if (!chip) {
-		dev_info(&client->dev, "fail chip\n");
-		err = -ENODEV;
+	chip = tpmm_chip_alloc(&client->dev, &st_i2c_tpm);
+	if (IS_ERR(chip)) {
+		err = PTR_ERR(chip);
 		goto end;
 	}
 
@@ -631,14 +630,14 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (!platform_data) {
 		dev_info(&client->dev, "chip not available\n");
 		err = -ENODEV;
-		goto _tpm_clean_answer;
+		goto end;
 	}
 
 	platform_data->tpm_i2c_buffer[0] =
 	    kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
 	if (platform_data->tpm_i2c_buffer[0] == NULL) {
 		err = -ENOMEM;
-		goto _tpm_clean_answer;
+		goto end;
 	}
 	platform_data->tpm_i2c_buffer[1] =
 	    kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
@@ -716,7 +715,10 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	tpm_get_timeouts(chip);
 	tpm_do_selftest(chip);
 
-	dev_info(chip->dev, "TPM I2C Initialized\n");
+	err = tpm_chip_register(chip);
+	if (err)
+		goto _irq_set;
+
 	return 0;
 _irq_set:
 	free_irq(gpio_to_irq(platform_data->io_serirq), (void *)chip);
@@ -732,8 +734,6 @@ _tpm_clean_response2:
 _tpm_clean_response1:
 	kzfree(platform_data->tpm_i2c_buffer[0]);
 	platform_data->tpm_i2c_buffer[0] = NULL;
-_tpm_clean_answer:
-	tpm_remove_hardware(chip->dev);
 end:
 	pr_info("TPM I2C initialisation fail\n");
 	return err;
@@ -752,13 +752,13 @@ static int tpm_st33_i2c_remove(struct i2c_client *client)
 		((struct i2c_client *)TPM_VPRIV(chip))->dev.platform_data;
 
 	if (pin_infos != NULL) {
+		tpm_chip_unregister(chip);
+
 		free_irq(pin_infos->io_serirq, chip);
 
 		gpio_free(pin_infos->io_serirq);
 		gpio_free(pin_infos->io_lpcpd);
 
-		tpm_remove_hardware(chip->dev);
-
 		if (pin_infos->tpm_i2c_buffer[1] != NULL) {
 			kzfree(pin_infos->tpm_i2c_buffer[1]);
 			pin_infos->tpm_i2c_buffer[1] = NULL;
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index af74c57..eb95796 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -270,8 +270,11 @@ static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
 static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
 {
 	struct ibmvtpm_dev *ibmvtpm = ibmvtpm_get_data(&vdev->dev);
+	struct tpm_chip *chip = dev_get_drvdata(ibmvtpm->dev);
 	int rc = 0;
 
+	tpm_chip_unregister(chip);
+
 	free_irq(vdev->irq, ibmvtpm);
 
 	do {
@@ -290,8 +293,6 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
 		kfree(ibmvtpm->rtce_buf);
 	}
 
-	tpm_remove_hardware(ibmvtpm->dev);
-
 	kfree(ibmvtpm);
 
 	return 0;
@@ -555,11 +556,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
 	struct tpm_chip *chip;
 	int rc = -ENOMEM, rc1;
 
-	chip = tpm_register_hardware(dev, &tpm_ibmvtpm);
-	if (!chip) {
-		dev_err(dev, "tpm_register_hardware failed\n");
-		return -ENODEV;
-	}
+	chip = tpmm_chip_alloc(dev, &tpm_ibmvtpm);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
 
 	ibmvtpm = kzalloc(sizeof(struct ibmvtpm_dev), GFP_KERNEL);
 	if (!ibmvtpm) {
@@ -629,7 +628,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
 	if (rc)
 		goto init_irq_cleanup;
 
-	return rc;
+	return tpm_chip_register(chip);
 init_irq_cleanup:
 	do {
 		rc1 = plpar_hcall_norets(H_FREE_CRQ, vio_dev->unit_address);
@@ -644,8 +643,6 @@ cleanup:
 		kfree(ibmvtpm);
 	}
 
-	tpm_remove_hardware(dev);
-
 	return rc;
 }
 
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index dc0a255..d1559f6 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -546,7 +546,14 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
 			 vendorid[0], vendorid[1],
 			 productid[0], productid[1], chipname);
 
-		if (!(chip = tpm_register_hardware(&dev->dev, &tpm_inf)))
+		chip = tpmm_chip_alloc(&dev->dev, &tpm_inf);
+		if (IS_ERR(chip)) {
+			rc = PTR_ERR(chip);
+			goto err_release_region;
+		}
+
+		rc = tpm_chip_register(chip);
+		if (rc)
 			goto err_release_region;
 
 		return 0;
@@ -573,6 +580,8 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
 	struct tpm_chip *chip = pnp_get_drvdata(dev);
 
 	if (chip) {
+		tpm_chip_unregister(chip);
+
 		if (tpm_dev.iotype == TPM_INF_IO_PORT) {
 			release_region(tpm_dev.data_regs, tpm_dev.data_size);
 			release_region(tpm_dev.config_port,
@@ -581,8 +590,6 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
 			iounmap(tpm_dev.mem_base);
 			release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
 		}
-		tpm_dev_vendor_release(chip);
-		tpm_remove_hardware(chip->dev);
 	}
 }
 
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 3179ec9..151f96a 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -247,9 +247,9 @@ static struct platform_device *pdev = NULL;
 static void tpm_nsc_remove(struct device *dev)
 {
 	struct tpm_chip *chip = dev_get_drvdata(dev);
-	if ( chip ) {
+	if (chip) {
+		tpm_chip_unregister(chip);
 		release_region(chip->vendor.base, 2);
-		tpm_remove_hardware(chip->dev);
 	}
 }
 
@@ -308,11 +308,16 @@ static int __init init_nsc(void)
 		goto err_del_dev;
 	}
 
-	if (!(chip = tpm_register_hardware(&pdev->dev, &tpm_nsc))) {
+	chip = tpmm_chip_alloc(&pdev->dev, &tpm_nsc);
+	if (IS_ERR(chip)) {
 		rc = -ENODEV;
 		goto err_rel_reg;
 	}
 
+	rc = tpm_chip_register(chip);
+	if (rc)
+		goto err_rel_reg;
+
 	dev_dbg(&pdev->dev, "NSC TPM detected\n");
 	dev_dbg(&pdev->dev,
 		"NSC LDN 0x%x, SID 0x%x, SRID 0x%x\n",
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..e2cb04d 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -75,9 +75,6 @@ enum tis_defaults {
 #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
 #define	TPM_RID(l)			(0x0F04 | ((l) << 12))
 
-static LIST_HEAD(tis_chips);
-static DEFINE_MUTEX(tis_lock);
-
 #if defined(CONFIG_PNP) && defined(CONFIG_ACPI)
 static int is_itpm(struct pnp_dev *dev)
 {
@@ -528,6 +525,17 @@ static bool interrupts = true;
 module_param(interrupts, bool, 0444);
 MODULE_PARM_DESC(interrupts, "Enable interrupts");
 
+static void tpm_tis_remove(struct tpm_chip *chip)
+{
+	iowrite32(~TPM_GLOBAL_INT_ENABLE &
+		  ioread32(chip->vendor.iobase +
+			   TPM_INT_ENABLE(chip->vendor.
+					  locality)),
+		  chip->vendor.iobase +
+		  TPM_INT_ENABLE(chip->vendor.locality));
+	release_locality(chip, chip->vendor.locality, 1);
+}
+
 static int tpm_tis_init(struct device *dev, resource_size_t start,
 			resource_size_t len, unsigned int irq)
 {
@@ -535,14 +543,13 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 	int rc, i, irq_s, irq_e, probe;
 	struct tpm_chip *chip;
 
-	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
-		return -ENODEV;
+	chip = tpmm_chip_alloc(dev, &tpm_tis);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
 
-	chip->vendor.iobase = ioremap(start, len);
-	if (!chip->vendor.iobase) {
-		rc = -EIO;
-		goto out_err;
-	}
+	chip->vendor.iobase = devm_ioremap(dev, start, len);
+	if (!chip->vendor.iobase)
+		return -EIO;
 
 	/* Default timeouts */
 	chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
@@ -649,8 +656,8 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 		for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) {
 			iowrite8(i, chip->vendor.iobase +
 				 TPM_INT_VECTOR(chip->vendor.locality));
-			if (request_irq
-			    (i, tis_int_probe, IRQF_SHARED,
+			if (devm_request_irq
+			    (dev, i, tis_int_probe, IRQF_SHARED,
 			     chip->vendor.miscdev.name, chip) != 0) {
 				dev_info(chip->dev,
 					 "Unable to request irq: %d for probe\n",
@@ -690,15 +697,14 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 			iowrite32(intmask,
 				  chip->vendor.iobase +
 				  TPM_INT_ENABLE(chip->vendor.locality));
-			free_irq(i, chip);
 		}
 	}
 	if (chip->vendor.irq) {
 		iowrite8(chip->vendor.irq,
 			 chip->vendor.iobase +
 			 TPM_INT_VECTOR(chip->vendor.locality));
-		if (request_irq
-		    (chip->vendor.irq, tis_int_handler, IRQF_SHARED,
+		if (devm_request_irq
+		    (dev, chip->vendor.irq, tis_int_handler, IRQF_SHARED,
 		     chip->vendor.miscdev.name, chip) != 0) {
 			dev_info(chip->dev,
 				 "Unable to request irq: %d for use\n",
@@ -719,17 +725,9 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 		}
 	}
 
-	INIT_LIST_HEAD(&chip->vendor.list);
-	mutex_lock(&tis_lock);
-	list_add(&chip->vendor.list, &tis_chips);
-	mutex_unlock(&tis_lock);
-
-
-	return 0;
+	return tpm_chip_register(chip);
 out_err:
-	if (chip->vendor.iobase)
-		iounmap(chip->vendor.iobase);
-	tpm_remove_hardware(chip->dev);
+	tpm_tis_remove(chip);
 	return rc;
 }
 
@@ -811,13 +809,10 @@ MODULE_DEVICE_TABLE(pnp, tpm_pnp_tbl);
 static void tpm_tis_pnp_remove(struct pnp_dev *dev)
 {
 	struct tpm_chip *chip = pnp_get_drvdata(dev);
-
-	tpm_dev_vendor_release(chip);
-
-	kfree(chip);
+	tpm_chip_unregister(chip);
+	tpm_tis_remove(chip);
 }
 
-
 static struct pnp_driver tis_pnp_driver = {
 	.name = "tpm_tis",
 	.id_table = tpm_pnp_tbl,
@@ -836,7 +831,7 @@ MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
 
 static struct platform_driver tis_drv = {
 	.driver = {
-		.name = "tpm_tis",
+		.name		= "tpm_tis",
 		.owner		= THIS_MODULE,
 		.pm		= &tpm_tis_pm,
 	},
@@ -876,31 +871,17 @@ err_dev:
 
 static void __exit cleanup_tis(void)
 {
-	struct tpm_vendor_specific *i, *j;
 	struct tpm_chip *chip;
-	mutex_lock(&tis_lock);
-	list_for_each_entry_safe(i, j, &tis_chips, list) {
-		chip = to_tpm_chip(i);
-		tpm_remove_hardware(chip->dev);
-		iowrite32(~TPM_GLOBAL_INT_ENABLE &
-			  ioread32(chip->vendor.iobase +
-				   TPM_INT_ENABLE(chip->vendor.
-						  locality)),
-			  chip->vendor.iobase +
-			  TPM_INT_ENABLE(chip->vendor.locality));
-		release_locality(chip, chip->vendor.locality, 1);
-		if (chip->vendor.irq)
-			free_irq(chip->vendor.irq, chip);
-		iounmap(i->iobase);
-		list_del(&i->list);
-	}
-	mutex_unlock(&tis_lock);
 #ifdef CONFIG_PNP
 	if (!force) {
 		pnp_unregister_driver(&tis_pnp_driver);
 		return;
 	}
 #endif
+	chip = dev_get_drvdata(&pdev->dev);
+	/* FIXME might be broken when using force */
+	tpm_chip_unregister(chip);
+	tpm_tis_remove(chip);
 	platform_device_unregister(pdev);
 	platform_driver_unregister(&tis_drv);
 }
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 441b44e..c3b4f5a 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -175,9 +175,9 @@ static int setup_chip(struct device *dev, struct tpm_private *priv)
 {
 	struct tpm_chip *chip;
 
-	chip = tpm_register_hardware(dev, &tpm_vtpm);
-	if (!chip)
-		return -ENODEV;
+	chip = tpmm_chip_alloc(dev, &tpm_vtpm);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
 
 	init_waitqueue_head(&chip->vendor.read_queue);
 
@@ -286,6 +286,7 @@ static int tpmfront_probe(struct xenbus_device *dev,
 		const struct xenbus_device_id *id)
 {
 	struct tpm_private *priv;
+	struct tpm_chip *chip;
 	int rv;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
@@ -302,21 +303,22 @@ static int tpmfront_probe(struct xenbus_device *dev,
 
 	rv = setup_ring(dev, priv);
 	if (rv) {
-		tpm_remove_hardware(&dev->dev);
+		chip = dev_get_drvdata(&dev->dev);
+		tpm_chip_unregister(chip);
 		ring_free(priv);
 		return rv;
 	}
 
 	tpm_get_timeouts(priv->chip);
 
-	return rv;
+	return tpm_chip_register(priv->chip);
 }
 
 static int tpmfront_remove(struct xenbus_device *dev)
 {
 	struct tpm_chip *chip = dev_get_drvdata(&dev->dev);
 	struct tpm_private *priv = TPM_VPRIV(chip);
-	tpm_remove_hardware(&dev->dev);
+	tpm_chip_unregister(chip);
 	ring_free(priv);
 	TPM_VPRIV(chip) = NULL;
 	return 0;
-- 
2.1.0

^ permalink raw reply related

* [PATCH v3 1/6] tpm: merge duplicate transmit_cmd() functions
From: Jarkko Sakkinen @ 2014-10-15 11:35 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
	christophe.ricard, will.c.arthur, monty.wiseman, Jarkko Sakkinen
In-Reply-To: <1413372916-12091-1-git-send-email-jarkko.sakkinen@linux.intel.com>

Merged transmit_cmd() functions in tpm-interface.c and tpm-sysfs.c.
Added "tpm_" prefix for consistency sake. Changed cmd parameter as
opaque. This enables to use separate command structures for TPM1
and TPM2 commands instead of putting everything to struct tpm_cmd_t.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-dev.c       |  4 +--
 drivers/char/tpm/tpm-interface.c | 53 +++++++++++++++++++++-------------------
 drivers/char/tpm/tpm-sysfs.c     | 23 ++---------------
 drivers/char/tpm/tpm.h           |  5 ++--
 4 files changed, 34 insertions(+), 51 deletions(-)

diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index d9b774e..bd79d33 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -140,8 +140,8 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 	}
 
 	/* atomic tpm command send and result receive */
-	out_size = tpm_transmit(priv->chip, priv->data_buffer,
-				sizeof(priv->data_buffer));
+	out_size = tpm_transmit_cmd(priv->chip, priv->data_buffer,
+				    sizeof(priv->data_buffer), NULL);
 	if (out_size < 0) {
 		mutex_unlock(&priv->buffer_mutex);
 		return out_size;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 6af1700..fedb4d5 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -331,8 +331,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 /*
  * Internal kernel interface to transmit TPM commands
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-		     size_t bufsiz)
+static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+			    size_t bufsiz)
 {
 	ssize_t rc;
 	u32 count, ordinal;
@@ -398,9 +398,10 @@ out:
 #define TPM_DIGEST_SIZE 20
 #define TPM_RET_CODE_IDX 6
 
-static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
-			    int len, const char *desc)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
+			 int len, const char *desc)
 {
+	struct tpm_output_header *header;
 	int err;
 
 	len = tpm_transmit(chip, (u8 *) cmd, len);
@@ -409,7 +410,9 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
 	else if (len < TPM_HEADER_SIZE)
 		return -EFAULT;
 
-	err = be32_to_cpu(cmd->header.out.return_code);
+	header = (struct tpm_output_header *) cmd;
+
+	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
 		dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
 
@@ -448,7 +451,7 @@ ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
 		tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 		tpm_cmd.params.getcap_in.subcap = subcap_id;
 	}
-	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
+	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
 	if (!rc)
 		*cap = tpm_cmd.params.getcap_out.cap;
 	return rc;
@@ -464,8 +467,8 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
 	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
 
-	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
-			"attempting to determine the timeouts");
+	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+			      "attempting to determine the timeouts");
 }
 EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
 
@@ -484,8 +487,8 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
 	struct tpm_cmd_t start_cmd;
 	start_cmd.header.in = tpm_startup_header;
 	start_cmd.params.startup_in.startup_type = startup_type;
-	return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
-			    "attempting to start the TPM");
+	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+				"attempting to start the TPM");
 }
 
 int tpm_get_timeouts(struct tpm_chip *chip)
@@ -500,7 +503,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 	tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
 	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
-	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
+	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
 
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
 		/* The TPM is not started, we are the first to talk to it.
@@ -513,7 +516,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
 		tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 		tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
-		rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+		rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
 				  NULL);
 	}
 	if (rc) {
@@ -575,8 +578,8 @@ duration:
 	tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 	tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_DURATION;
 
-	rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
-			"attempting to determine the durations");
+	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+			      "attempting to determine the durations");
 	if (rc)
 		return rc;
 
@@ -631,8 +634,8 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 	struct tpm_cmd_t cmd;
 
 	cmd.header.in = continue_selftest_header;
-	rc = transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
-			  "continue selftest");
+	rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+			      "continue selftest");
 	return rc;
 }
 
@@ -672,8 +675,8 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 
 	cmd.header.in = pcrread_header;
 	cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
-	rc = transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
-			  "attempting to read a pcr value");
+	rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
+			      "attempting to read a pcr value");
 
 	if (rc == 0)
 		memcpy(res_buf, cmd.params.pcrread_out.pcr_result,
@@ -737,8 +740,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 	cmd.header.in = pcrextend_header;
 	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
 	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
-	rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
-			  "attempting extend a PCR value");
+	rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+			      "attempting extend a PCR value");
 
 	tpm_chip_put(chip);
 	return rc;
@@ -817,7 +820,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 	if (chip == NULL)
 		return -ENODEV;
 
-	rc = transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
+	rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
 
 	tpm_chip_put(chip);
 	return rc;
@@ -938,14 +941,14 @@ int tpm_pm_suspend(struct device *dev)
 		cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
 		memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
 		       TPM_DIGEST_SIZE);
-		rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
-				  "extending dummy pcr before suspend");
+		rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+				      "extending dummy pcr before suspend");
 	}
 
 	/* now do the actual savestate */
 	for (try = 0; try < TPM_RETRY; try++) {
 		cmd.header.in = savestate_header;
-		rc = transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
+		rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
 
 		/*
 		 * If the TPM indicates that it is too busy to respond to
@@ -1022,7 +1025,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 		tpm_cmd.header.in = tpm_getrandom_header;
 		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
 
-		err = transmit_cmd(chip, &tpm_cmd,
+		err = tpm_transmit_cmd(chip, &tpm_cmd,
 				   TPM_GETRANDOM_RESULT_SIZE + num_bytes,
 				   "attempting get random");
 		if (err)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 01730a2..8ecb052 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -20,25 +20,6 @@
 #include <linux/device.h>
 #include "tpm.h"
 
-/* XXX for now this helper is duplicated in tpm-interface.c */
-static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
-			    int len, const char *desc)
-{
-	int err;
-
-	len = tpm_transmit(chip, (u8 *) cmd, len);
-	if (len <  0)
-		return len;
-	else if (len < TPM_HEADER_SIZE)
-		return -EFAULT;
-
-	err = be32_to_cpu(cmd->header.out.return_code);
-	if (err != 0 && desc)
-		dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
-
-	return err;
-}
-
 #define READ_PUBEK_RESULT_SIZE 314
 #define TPM_ORD_READPUBEK cpu_to_be32(124)
 static struct tpm_input_header tpm_readpubek_header = {
@@ -58,8 +39,8 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 	struct tpm_chip *chip = dev_get_drvdata(dev);
 
 	tpm_cmd.header.in = tpm_readpubek_header;
-	err = transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
-			   "attempting to read the PUBEK");
+	err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
+			       "attempting to read the PUBEK");
 	if (err)
 		goto out;
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index e4d0888..12326e1 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -314,9 +314,8 @@ struct tpm_cmd_t {
 } __packed;
 
 ssize_t	tpm_getcap(struct device *, __be32, cap_t *, const char *);
-
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-		     size_t bufsiz);
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
+			 const char *desc);
 extern int tpm_get_timeouts(struct tpm_chip *);
 extern void tpm_gen_interrupt(struct tpm_chip *);
 extern int tpm_do_selftest(struct tpm_chip *);
-- 
2.1.0

^ permalink raw reply related

* [PATCH v3 0/6] TPM 2.0 support
From: Jarkko Sakkinen @ 2014-10-15 11:35 UTC (permalink / raw)
  To: Peter Huewe, Ashley Lai, Marcel Selhorst
  Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
	christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
	will.c.arthur-ral2JQCrhuEAvxtiuMwx3w,
	monty.wiseman-ral2JQCrhuEAvxtiuMwx3w, Jarkko Sakkinen

This patch set enables TPM2 protocol and provides drivers for FIFO and
CRB interfaces. In addition life-cycle model is somewhat improved (but
not yet fully fixed) for TPM device drivers. There's still work to do
in ref counting and locking but this patch set takes (I believe) the
correct interm steps towards right direction.

TPM2 sysfs attributes are placed to misc dev instead of binding them
to the platform device as misc dev represents the device to the user
space. TPM1 uses invalid place for the sysfs attributes.

Major changes since v1:

- Improved struct tpm_chip life-cycle by taking advantage of devres
  API.
- Refined sysfs attributes as simple key-values thereby not repeating
  mistakes in TPM1 sysfs attributes.
- Documented functions in tpm-chip.c and tpm2-cmd.c.
- Documented sysfs attributes.

Major changes since v2:

- Lots of fixes in calling order in device drivers (thanks to Jason
  Gunthorpe for pointing these out!).
- Attach sysfs attributes to the misc device because it represents
  TPM device to the user space.

Known opens:

- I'm still pending to test the FIFO (tpm_tis) driver. Now I should
  have system to actually get this done but I'm right now at the
  LinuxCon.
- struct tpm_chip would need proper ref counting and locking applied
  everywhere but this has been existing problem before this patch set.
  It is a large change that I'm willing to work on after getting this
  merged first.
- It might be that in some cases PPI interface might be attached to
  a wrong PPI interface if ACPI tree shows up having two PPI interface. 
  I think the way PPI interface is grabbed is invalid. Instead it should
  be passed an ACPI handle to the device and it should take PPI interface
  under that ACPi handle. I can add a fix to do this in v4 if everyone
  else agrees.

Jarkko Sakkinen (5):
  tpm: merge duplicate transmit_cmd() functions
  tpm: two-phase chip management functions
  tpm: TPM 2.0 commands
  tpm: TPM 2.0 sysfs attributes
  tpm: TPM 2.0 CRB Interface

Will Arthur (1):
  tpm: TPM 2.0 FIFO Interface

 Documentation/ABI/stable/sysfs-class-tpm2 |  72 ++++
 drivers/char/tpm/Kconfig                  |   9 +
 drivers/char/tpm/Makefile                 |   3 +-
 drivers/char/tpm/tpm-chip.c               | 209 ++++++++++++
 drivers/char/tpm/tpm-dev.c                |   7 +-
 drivers/char/tpm/tpm-interface.c          | 225 +++----------
 drivers/char/tpm/tpm-sysfs.c              |  23 +-
 drivers/char/tpm/tpm.h                    | 106 +++++-
 drivers/char/tpm/tpm2-cmd.c               | 543 ++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-sysfs.c             | 314 +++++++++++++++++
 drivers/char/tpm/tpm_atmel.c              |  11 +-
 drivers/char/tpm/tpm_crb.c                | 329 ++++++++++++++++++
 drivers/char/tpm/tpm_i2c_atmel.c          |  33 +-
 drivers/char/tpm/tpm_i2c_infineon.c       |  37 +-
 drivers/char/tpm/tpm_i2c_nuvoton.c        |  44 +--
 drivers/char/tpm/tpm_i2c_stm_st33.c       |  22 +-
 drivers/char/tpm/tpm_ibmvtpm.c            |  17 +-
 drivers/char/tpm/tpm_infineon.c           |  13 +-
 drivers/char/tpm/tpm_nsc.c                |  11 +-
 drivers/char/tpm/tpm_tis.c                | 160 +++++----
 drivers/char/tpm/xen-tpmfront.c           |  14 +-
 21 files changed, 1813 insertions(+), 389 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-class-tpm2
 create mode 100644 drivers/char/tpm/tpm-chip.c
 create mode 100644 drivers/char/tpm/tpm2-cmd.c
 create mode 100644 drivers/char/tpm/tpm2-sysfs.c
 create mode 100644 drivers/char/tpm/tpm_crb.c

-- 
2.1.0

^ permalink raw reply

* RE: [PATCH 3/8] iio: Add support for DA9150 GPADC
From: Opensource [Adam Thomson] @ 2014-10-15 10:34 UTC (permalink / raw)
  To: Hartmut Knaack, Opensource [Adam Thomson], Lee Jones,
	Samuel Ortiz, Jonathan Cameron,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton,
	Joe Perches, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Support Opensource
In-Reply-To: <5439B397.8060207-Mmb7MZpHnFY@public.gmane.org>

On October 11, 2014 23:48, Hartmut Knaack wrote:

> Hi,
> I have put a few comments inline.
> 
> Adam Thomson schrieb am 23.09.2014 12:53:
> > This patch adds support for DA9150 Charger & Fuel-Gauge IC GPADC.
> >
> > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> > ---
> >  drivers/iio/adc/Kconfig        |   9 +
> >  drivers/iio/adc/Makefile       |   1 +
> >  drivers/iio/adc/da9150-gpadc.c | 406
> +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 416 insertions(+)
> >  create mode 100644 drivers/iio/adc/da9150-gpadc.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 11b048a..8041347 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -127,6 +127,15 @@ config AT91_ADC
> >  	help
> >  	  Say yes here to build support for Atmel AT91 ADC.
> >
> > +config DA9150_GPADC
> > +	tristate "Dialog DA9150 GPADC driver support"
> > +	depends on MFD_DA9150
> > +	help
> > +	  Say yes here to build support for Dialog DA9150 GPADC.
> > +
> > +	  This driver can also be built as a module. If chosen, the module name
> > +	  will be da9150-gpadc.
> > +
> >  config EXYNOS_ADC
> >  	tristate "Exynos ADC driver support"
> >  	depends on ARCH_EXYNOS || (OF && COMPILE_TEST)
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index ad81b51..48413d2 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -14,6 +14,7 @@ obj-$(CONFIG_AD7793) += ad7793.o
> >  obj-$(CONFIG_AD7887) += ad7887.o
> >  obj-$(CONFIG_AD799X) += ad799x.o
> >  obj-$(CONFIG_AT91_ADC) += at91_adc.o
> > +obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
> >  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
> >  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> >  obj-$(CONFIG_MAX1027) += max1027.o
> > diff --git a/drivers/iio/adc/da9150-gpadc.c b/drivers/iio/adc/da9150-gpadc.c
> > new file mode 100644
> > index 0000000..2b83ee0
> > --- /dev/null
> > +++ b/drivers/iio/adc/da9150-gpadc.c
> > @@ -0,0 +1,406 @@
> > +/*
> > + * DA9150 GPADC Driver
> > + *
> > + * Copyright (c) 2014 Dialog Semiconductor
> > + *
> > + * Author: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mutex.h>
> > +#include <linux/completion.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/machine.h>
> > +#include <linux/iio/driver.h>
> > +#include <linux/mfd/da9150/core.h>
> > +#include <linux/mfd/da9150/registers.h>
> > +
> > +/* Channels */
> > +enum da9150_gpadc_hw_channel {
> > +	DA9150_GPADC_HW_CHAN_GPIOA_2V = 0,
> > +	DA9150_GPADC_HW_CHAN_GPIOA_2V_,
> > +	DA9150_GPADC_HW_CHAN_GPIOB_2V,
> > +	DA9150_GPADC_HW_CHAN_GPIOB_2V_,
> > +	DA9150_GPADC_HW_CHAN_GPIOC_2V,
> > +	DA9150_GPADC_HW_CHAN_GPIOC_2V_,
> > +	DA9150_GPADC_HW_CHAN_GPIOD_2V,
> > +	DA9150_GPADC_HW_CHAN_GPIOD_2V_,
> > +	DA9150_GPADC_HW_CHAN_IBUS_SENSE,
> > +	DA9150_GPADC_HW_CHAN_IBUS_SENSE_,
> > +	DA9150_GPADC_HW_CHAN_VBUS_DIV,
> > +	DA9150_GPADC_HW_CHAN_VBUS_DIV_,
> > +	DA9150_GPADC_HW_CHAN_ID,
> > +	DA9150_GPADC_HW_CHAN_ID_,
> > +	DA9150_GPADC_HW_CHAN_VSYS,
> > +	DA9150_GPADC_HW_CHAN_VSYS_,
> > +	DA9150_GPADC_HW_CHAN_GPIOA_6V,
> > +	DA9150_GPADC_HW_CHAN_GPIOA_6V_,
> > +	DA9150_GPADC_HW_CHAN_GPIOB_6V,
> > +	DA9150_GPADC_HW_CHAN_GPIOB_6V_,
> > +	DA9150_GPADC_HW_CHAN_GPIOC_6V,
> > +	DA9150_GPADC_HW_CHAN_GPIOC_6V_,
> > +	DA9150_GPADC_HW_CHAN_GPIOD_6V,
> > +	DA9150_GPADC_HW_CHAN_GPIOD_6V_,
> > +	DA9150_GPADC_HW_CHAN_VBAT,
> > +	DA9150_GPADC_HW_CHAN_VBAT_,
> > +	DA9150_GPADC_HW_CHAN_TBAT,
> > +	DA9150_GPADC_HW_CHAN_TBAT_,
> > +	DA9150_GPADC_HW_CHAN_TJUNC_CORE,
> > +	DA9150_GPADC_HW_CHAN_TJUNC_CORE_,
> > +	DA9150_GPADC_HW_CHAN_TJUNC_OVP,
> > +	DA9150_GPADC_HW_CHAN_TJUNC_OVP_,
> > +};
> > +
> > +enum da9150_gpadc_channel {
> > +	DA9150_GPADC_CHAN_GPIOA = 0,
> > +	DA9150_GPADC_CHAN_GPIOB,
> > +	DA9150_GPADC_CHAN_GPIOC,
> > +	DA9150_GPADC_CHAN_GPIOD,
> > +	DA9150_GPADC_CHAN_IBUS,
> > +	DA9150_GPADC_CHAN_VBUS,
> > +	DA9150_GPADC_CHAN_ID,
> > +	DA9150_GPADC_CHAN_VSYS,
> > +	DA9150_GPADC_CHAN_VBAT,
> > +	DA9150_GPADC_CHAN_TBAT,
> > +	DA9150_GPADC_CHAN_TJUNC_CORE,
> > +	DA9150_GPADC_CHAN_TJUNC_OVP,
> > +};
> > +
> > +/* Private data */
> > +struct da9150_gpadc {
> > +	struct da9150 *da9150;
> > +	struct device *dev;
> > +
> > +	struct mutex lock;
> > +	struct completion complete;
> > +};
> > +
> > +
> > +static irqreturn_t da9150_gpadc_irq(int irq, void *data)
> > +{
> > +
> > +	struct da9150_gpadc *gpadc = data;
> > +
> > +	complete(&gpadc->complete);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int da9150_gpadc_read_adc(struct da9150_gpadc *gpadc, int hw_chan)
> > +{
> > +	u8 result_regs[2];
> > +	int result;
> > +
> > +	mutex_lock(&gpadc->lock);
> > +
> > +	/* Set channel & enable measurement */
> > +	da9150_reg_write(gpadc->da9150, DA9150_GPADC_MAN,
> > +			 (DA9150_GPADC_EN_MASK |
> > +			  hw_chan << DA9150_GPADC_MUX_SHIFT));
> > +
> > +	/* Consume left-over completion from a previous timeout */
> > +	try_wait_for_completion(&gpadc->complete);
> > +
> > +	/* Check for actual completion */
> > +	wait_for_completion_timeout(&gpadc->complete, msecs_to_jiffies(5));
> > +
> > +	/* Read result and status from device */
> > +	da9150_bulk_read(gpadc->da9150, DA9150_GPADC_RES_A, 2, result_regs);
> > +
> > +	mutex_unlock(&gpadc->lock);
> > +
> > +	/* Check to make sure device really has completed reading */
> > +	if (result_regs[1] & DA9150_GPADC_RUN_MASK) {
> > +		dev_err(gpadc->dev, "Timeout on channel %d of GPADC\n",
> > +			hw_chan);
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	/* LSBs - 2 bits */
> > +	result = (result_regs[1] & DA9150_GPADC_RES_L_MASK) >>
> > +		 DA9150_GPADC_RES_L_SHIFT;
> > +	/* MSBs - 8 bits */
> > +	result |= result_regs[0] << DA9150_GPADC_RES_L_BITS;
> > +
> > +	return result;
> > +}
> > +
> > +static inline int da9150_gpadc_gpio_6v_voltage_now(int raw_val)
> > +{
> > +	/* Convert to mV */
> > +	return (6 * ((raw_val * 1000) + 500)) / 1024;
> > +}
> > +
> > +static inline int da9150_gpadc_ibus_current_avg(int raw_val)
> > +{
> > +	/* Convert to mA */
> > +	return (4 * ((raw_val * 1000) + 500)) / 2048;
> > +}
> > +
> > +static inline int da9150_gpadc_vbus_21v_voltage_now(int raw_val)
> > +{
> > +	/* Convert to mV */
> > +	return (21 * ((raw_val * 1000) + 500)) / 1024;
> > +}
> > +
> > +static inline int da9150_gpadc_vsys_6v_voltage_now(int raw_val)
> > +{
> > +	/* Convert to mV */
> > +	return (3 * ((raw_val * 1000) + 500)) / 512;
> > +}
> > +
> > +static int da9150_gpadc_read_processed(struct da9150_gpadc *gpadc, int
> channel,
> > +				       int hw_chan, int *val)
> > +{
> > +	int raw_val;
> > +
> > +	raw_val = da9150_gpadc_read_adc(gpadc, hw_chan);
> > +	if (raw_val < 0)
> > +		return raw_val;
> > +
> > +	switch (channel) {
> > +	case DA9150_GPADC_CHAN_GPIOA:
> > +	case DA9150_GPADC_CHAN_GPIOB:
> > +	case DA9150_GPADC_CHAN_GPIOC:
> > +	case DA9150_GPADC_CHAN_GPIOD:
> > +		*val = da9150_gpadc_gpio_6v_voltage_now(raw_val);
> > +		break;
> > +	case DA9150_GPADC_CHAN_IBUS:
> > +		*val = da9150_gpadc_ibus_current_avg(raw_val);
> > +		break;
> > +	case DA9150_GPADC_CHAN_VBUS:
> > +		*val = da9150_gpadc_vbus_21v_voltage_now(raw_val);
> > +		break;
> > +	case DA9150_GPADC_CHAN_VSYS:
> > +		*val = da9150_gpadc_vsys_6v_voltage_now(raw_val);
> > +		break;
> > +	default:
> > +		/* No processing for other channels so return raw value */
> > +		*val = raw_val;
> > +		break;
> > +	}
> > +
> > +	return IIO_VAL_INT;
> > +}
> > +
> > +static int da9150_gpadc_read_scale(int channel, int *val, int *val2)
> > +{
> > +	switch (channel) {
> > +	case DA9150_GPADC_CHAN_VBAT:
> > +		*val = 2932;
> > +		*val2 = 1000;
> > +		return IIO_VAL_FRACTIONAL;
> > +	case DA9150_GPADC_CHAN_TJUNC_CORE:
> > +	case DA9150_GPADC_CHAN_TJUNC_OVP:
> > +		*val = 1000000;
> > +		*val2 = 4420;
> > +		return IIO_VAL_FRACTIONAL;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int da9150_gpadc_read_offset(int channel, int *val)
> > +{
> > +	switch (channel) {
> > +	case DA9150_GPADC_CHAN_VBAT:
> > +		*val = 1500000 / 2932;
> > +		return IIO_VAL_INT;
> > +	case DA9150_GPADC_CHAN_TJUNC_CORE:
> > +	case DA9150_GPADC_CHAN_TJUNC_OVP:
> > +		*val = -144;
> > +		return IIO_VAL_INT;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int da9150_gpadc_read_raw(struct iio_dev *indio_dev,
> > +				 struct iio_chan_spec const *chan,
> > +				 int *val, int *val2, long mask)
> > +{
> > +	struct da9150_gpadc *gpadc = iio_priv(indio_dev);
> > +
> > +	if ((chan->channel < DA9150_GPADC_CHAN_GPIOA) ||
> > +	    (chan->channel > DA9150_GPADC_CHAN_TJUNC_OVP))
> > +		return -EINVAL;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +	case IIO_CHAN_INFO_PROCESSED:
> > +		return da9150_gpadc_read_processed(gpadc, chan->channel,
> > +						   chan->address, val);
> > +	case IIO_CHAN_INFO_SCALE:
> > +		return da9150_gpadc_read_scale(chan->channel, val, val2);
> > +	case IIO_CHAN_INFO_OFFSET:
> > +		return da9150_gpadc_read_offset(chan->channel, val);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static const struct iio_info da9150_gpadc_info = {
> > +	.read_raw = &da9150_gpadc_read_raw,
> > +	.driver_module = THIS_MODULE,
> > +};
> > +
> > +#define DA9150_GPADC_CHANNEL(_id, _hw_id, _type, chan_info,	\
> > +			     _ext_name) {			\
> > +	.type = _type,						\
> > +	.indexed = 1,						\
> > +	.channel = DA9150_GPADC_CHAN_##_id,			\
> > +	.address = DA9150_GPADC_HW_CHAN_##_hw_id,		\
> > +	.info_mask_separate = chan_info,			\
> > +	.extend_name = _ext_name,				\
> > +	.datasheet_name = #_id,					\
> > +}
> > +
> > +#define DA9150_GPADC_CHANNEL_RAW(_id, _hw_id, _type, _ext_name)	\
> > +	DA9150_GPADC_CHANNEL(_id, _hw_id, _type,		\
> > +			     BIT(IIO_CHAN_INFO_RAW), _ext_name)
> > +
> > +#define DA9150_GPADC_CHANNEL_SCALED(_id, _hw_id, _type, _ext_name)	\
> > +	DA9150_GPADC_CHANNEL(_id, _hw_id, _type,			\
> > +			     BIT(IIO_CHAN_INFO_RAW) |			\
> > +			     BIT(IIO_CHAN_INFO_SCALE) |			\
> > +			     BIT(IIO_CHAN_INFO_OFFSET),			\
> > +			     _ext_name)
> > +
> > +#define DA9150_GPADC_CHANNEL_PROCESSED(_id, _hw_id, _type, _ext_name)
> 	\
> > +	DA9150_GPADC_CHANNEL(_id, _hw_id, _type,			\
> > +			     BIT(IIO_CHAN_INFO_PROCESSED), _ext_name)
> > +
> > +/* Supported channels */
> > +static const struct iio_chan_spec da9150_gpadc_channels[] = {
> > +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOA, GPIOA_6V, IIO_VOLTAGE,
> "GPIOA"),
> > +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOB, GPIOB_6V, IIO_VOLTAGE,
> "GPIOB"),
> > +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOC, GPIOC_6V, IIO_VOLTAGE,
> "GPIOC"),
> > +	DA9150_GPADC_CHANNEL_PROCESSED(GPIOD, GPIOD_6V, IIO_VOLTAGE,
> "GPIOD"),
> > +	DA9150_GPADC_CHANNEL_PROCESSED(IBUS, IBUS_SENSE, IIO_CURRENT,
> "IBUS"),
> > +	DA9150_GPADC_CHANNEL_PROCESSED(VBUS, VBUS_DIV_, IIO_VOLTAGE,
> "VBUS"),
> > +	DA9150_GPADC_CHANNEL_RAW(ID, ID, IIO_VOLTAGE, "ID"),
> > +	DA9150_GPADC_CHANNEL_PROCESSED(VSYS, VSYS, IIO_VOLTAGE, "VSYS"),
> > +	DA9150_GPADC_CHANNEL_SCALED(VBAT, VBAT, IIO_VOLTAGE, "VBAT"),
> > +	DA9150_GPADC_CHANNEL_RAW(TBAT, TBAT, IIO_VOLTAGE, "TBAT"),
> > +	DA9150_GPADC_CHANNEL_SCALED(TJUNC_CORE, TJUNC_CORE,
> IIO_TEMP,
> > +				    "TJUNC_CORE"),
> > +	DA9150_GPADC_CHANNEL_SCALED(TJUNC_OVP, TJUNC_OVP, IIO_TEMP,
> > +				    "TJUNC_OVP"),
> > +};
> > +
> > +/* Default maps used by da9150-charger */
> > +static struct iio_map da9150_gpadc_default_maps[] = {
> > +	{
> > +		.consumer_dev_name = "da9150-charger",
> > +		.consumer_channel = "CHAN_IBUS",
> > +		.adc_channel_label = "IBUS",
> > +	},
> > +	{
> > +		.consumer_dev_name = "da9150-charger",
> > +		.consumer_channel = "CHAN_VBUS",
> > +		.adc_channel_label = "VBUS",
> > +	},
> > +	{
> > +		.consumer_dev_name = "da9150-charger",
> > +		.consumer_channel = "CHAN_TJUNC",
> > +		.adc_channel_label = "TJUNC_CORE",
> > +	},
> > +	{
> > +		.consumer_dev_name = "da9150-charger",
> > +		.consumer_channel = "CHAN_VBAT",
> > +		.adc_channel_label = "VBAT",
> > +	},
> > +	{},
> > +};
> > +
> > +static int da9150_gpadc_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct da9150 *da9150 = dev_get_drvdata(dev->parent);
> Maybe you could find a slightly different name for the instance of this da9150 than
> its type name.

This is quite common through other drivers, and I think the usage is obvious,
so would rather leave as is.

> > +	struct da9150_gpadc *gpadc;
> > +	struct iio_dev *indio_dev;
> > +	int irq, ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&pdev->dev,
> > +					  sizeof(struct da9150_gpadc));
> > +	if (!indio_dev) {
> > +		dev_err(&pdev->dev, "Failed to allocate IIO device\n");
> > +		return -ENOMEM;
> > +	}
> > +	gpadc = iio_priv(indio_dev);
> > +
> > +	platform_set_drvdata(pdev, indio_dev);
> > +	gpadc->da9150 = da9150;
> > +	gpadc->dev = dev;
> > +
> > +	ret = iio_map_array_register(indio_dev, da9150_gpadc_default_maps);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to register IIO maps: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	mutex_init(&gpadc->lock);
> > +	init_completion(&gpadc->complete);
> > +
> > +	irq = platform_get_irq_byname(pdev, "GPADC");
> Shouldn't you check irq for possible error codes? devm_request_threaded_irq()
> expects irq to be unsigned.

The intention was to cut down on code as devm_request_threaded_irq should fail
if the IRQ number is invalid. However, you're probably right here and there is
the opportunity for PROBE_DEFER to be returned, if using DT.

> > +	ret = devm_request_threaded_irq(dev, irq, NULL, da9150_gpadc_irq,
> > +					IRQF_ONESHOT, "GPADC", gpadc);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to request IRQ %d: %d\n", irq, ret);
> > +		goto iio_map_unreg;
> > +	}
> > +
> > +	indio_dev->name = dev_name(dev);
> > +	indio_dev->dev.parent = dev;
> > +	indio_dev->dev.of_node = pdev->dev.of_node;
> > +	indio_dev->info = &da9150_gpadc_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->channels = da9150_gpadc_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(da9150_gpadc_channels);
> > +
> > +	ret = iio_device_register(indio_dev);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to register IIO device: %d\n", ret);
> > +		goto iio_map_unreg;
> > +	}
> > +
> > +	return 0;
> > +
> > +iio_map_unreg:
> > +	iio_map_array_unregister(indio_dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static int da9150_gpadc_remove(struct platform_device *pdev)
> > +{
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +
> > +	iio_map_array_unregister(indio_dev);
> > +	iio_device_unregister(indio_dev);
> Not sure if that was the intention of Jonathans comment, but these unregister calls
> should be switched to be in proper reverse order.

Already planned to do that, but thanks.

> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver da9150_gpadc_driver = {
> > +	.driver = {
> > +		.name = "da9150-gpadc",
> > +	},
> > +	.probe = da9150_gpadc_probe,
> > +	.remove = da9150_gpadc_remove,
> > +};
> > +
> > +module_platform_driver(da9150_gpadc_driver);
> > +
> > +MODULE_DESCRIPTION("GPADC Driver for DA9150");
> > +MODULE_AUTHOR("Adam Thomson
> <Adam.Thomson.Opensource@diasemi.com");
> You are missing a > at the end of your email-address.

Good spot. :) Will update.

> > +MODULE_LICENSE("GPL");
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" 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 net-next RFC 0/3] virtio-net: Conditionally enable tx interrupt
From: Jason Wang @ 2014-10-15  7:28 UTC (permalink / raw)
  To: Michael S. Tsirkin, David Miller
  Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <20141014230627.GA23715@redhat.com>

On 10/15/2014 07:06 AM, Michael S. Tsirkin wrote:
> On Tue, Oct 14, 2014 at 02:53:27PM -0400, David Miller wrote:
>> > From: Jason Wang <jasowang@redhat.com>
>> > Date: Sat, 11 Oct 2014 15:16:43 +0800
>> > 
>>> > > We free old transmitted packets in ndo_start_xmit() currently, so any
>>> > > packet must be orphaned also there. This was used to reduce the overhead of
>>> > > tx interrupt to achieve better performance. But this may not work for some
>>> > > protocols such as TCP stream. TCP depends on the value of sk_wmem_alloc to
>>> > > implement various optimization for small packets stream such as TCP small
>>> > > queue and auto corking. But orphaning packets early in ndo_start_xmit()
>>> > > disable such things more or less since sk_wmem_alloc was not accurate. This
>>> > > lead extra low throughput for TCP stream of small writes.
>>> > > 
>>> > > This series tries to solve this issue by enable tx interrupts for all TCP
>>> > > packets other than the ones with push bit or pure ACK. This is done through
>>> > > the support of urgent descriptor which can force an interrupt for a
>>> > > specified packet. If tx interrupt was enabled for a packet, there's no need
>>> > > to orphan it in ndo_start_xmit(), we can free it tx napi which is scheduled
>>> > > by tx interrupt. Then sk_wmem_alloc was more accurate than before and TCP
>>> > > can batch more for small write. More larger skb was produced by TCP in this
>>> > > case to improve both throughput and cpu utilization.
>>> > > 
>>> > > Test shows great improvements on small write tcp streams. For most of the
>>> > > other cases, the throughput and cpu utilization are the same in the
>>> > > past. Only few cases, more cpu utilization was noticed which needs more
>>> > > investigation.
>>> > > 
>>> > > Review and comments are welcomed.
>> > 
>> > I think proper accounting and queueing (at all levels, not just TCP
>> > sockets) is more important than trying to skim a bunch of cycles by
>> > avoiding TX interrupts.
>> > 
>> > Having an event to free the SKB is absolutely essential for the stack
>> > to operate correctly.
>> > 
>> > And with virtio-net you don't even have the excuse of "the HW
>> > unfortunately doesn't have an appropriate TX event."
>> > 
>> > So please don't play games, and instead use TX interrupts all the
>> > time.  You can mitigate them in various ways, but don't turn them on
>> > selectively based upon traffic type, that's terrible.
>> > 
>> > You can even use ->xmit_more to defer the TX interrupt indication to
>> > the final TX packet in the chain.
> I guess we can just defer the kick, interrupt will naturally be
> deferred as well.
> This should solve the problem for old hosts as well.

Interrupt were delayed but not reduced, to support this we need publish
avail idx as used event. This should reduce the tx interrupt in the case
of bulk dequeuing.

I will draft a new rfc series contain this.
>
> We'll also need to implement bql for this.
> Something like the below?
> Completely untested - posting here to see if I figured the
> API out correctly. Has to be applied on top of the previous patch.

Looks so. I believe better to have but not a must.

^ permalink raw reply

* Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
From: Rusty Russell @ 2014-10-15  5:40 UTC (permalink / raw)
  To: Jason Wang, mst, virtualization, netdev, linux-kernel; +Cc: linux-api, kvm
In-Reply-To: <1413011806-3813-2-git-send-email-jasowang@redhat.com>

Jason Wang <jasowang@redhat.com> writes:
> Below should be useful for some experiments Jason is doing.
> I thought I'd send it out for early review/feedback.
>
> event idx feature allows us to defer interrupts until
> a specific # of descriptors were used.
> Sometimes it might be useful to get an interrupt after
> a specific descriptor, regardless.
> This adds a descriptor flag for this, and an API
> to create an urgent output descriptor.
> This is still an RFC:
> we'll need a feature bit for drivers to detect this,
> but we've run out of feature bits for virtio 0.X.
> For experimentation purposes, drivers can assume
> this is set, or add a driver-specific feature bit.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

The new VRING_DESC_F_URGENT bit is theoretically nicer, but for
networking (which tends to take packets in order) couldn't we just set
the event counter to give us a tx interrupt at the packet we want?

Cheers,
Rusty.

^ permalink raw reply

* Re: [PATCH net-next RFC 3/3] virtio-net: conditionally enable tx interrupt
From: Jason Wang @ 2014-10-15  3:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <20141014215146.GB23344@redhat.com>

On 10/15/2014 05:51 AM, Michael S. Tsirkin wrote:
> On Sat, Oct 11, 2014 at 03:16:46PM +0800, Jason Wang wrote:
>> > We free transmitted packets in ndo_start_xmit() in the past to get better
>> > performance in the past. One side effect is that skb_orphan() needs to be
>> > called in ndo_start_xmit() which makes sk_wmem_alloc not accurate in
>> > fact. For TCP protocol, this means several optimization could not work well
>> > such as TCP small queue and auto corking. This can lead extra low
>> > throughput of small packets stream.
>> > 
>> > Thanks to the urgent descriptor support. This patch tries to solve this
>> > issue by enable the tx interrupt selectively for stream packets. This means
>> > we don't need to orphan TCP stream packets in ndo_start_xmit() but enable
>> > tx interrupt for those packets. After we get tx interrupt, a tx napi was
>> > scheduled to free those packets.
>> > 
>> > With this method, sk_wmem_alloc of TCP socket were more accurate than in
>> > the past which let TCP can batch more through TSQ and auto corking.
>> > 
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > ---
>> >  drivers/net/virtio_net.c | 164 ++++++++++++++++++++++++++++++++++++-----------
>> >  1 file changed, 128 insertions(+), 36 deletions(-)
>> > 
>> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> > index 5810841..b450fc4 100644
>> > --- a/drivers/net/virtio_net.c
>> > +++ b/drivers/net/virtio_net.c
>> > @@ -72,6 +72,8 @@ struct send_queue {
>> >  
>> >  	/* Name of the send queue: output.$index */
>> >  	char name[40];
>> > +
>> > +	struct napi_struct napi;
>> >  };
>> >  
>> >  /* Internal representation of a receive virtqueue */
>> > @@ -217,15 +219,40 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
>> >  	return p;
>> >  }
>> >  
>> > +static int free_old_xmit_skbs(struct send_queue *sq, int budget)
>> > +{
>> > +	struct sk_buff *skb;
>> > +	unsigned int len;
>> > +	struct virtnet_info *vi = sq->vq->vdev->priv;
>> > +	struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>> > +	int sent = 0;
>> > +
>> > +	while (sent < budget &&
>> > +	       (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
>> > +		pr_debug("Sent skb %p\n", skb);
>> > +
>> > +		u64_stats_update_begin(&stats->tx_syncp);
>> > +		stats->tx_bytes += skb->len;
>> > +		stats->tx_packets++;
>> > +		u64_stats_update_end(&stats->tx_syncp);
>> > +
>> > +		dev_kfree_skb_any(skb);
>> > +		sent++;
>> > +	}
>> > +
>> > +	return sent;
>> > +}
>> > +
>> >  static void skb_xmit_done(struct virtqueue *vq)
>> >  {
>> >  	struct virtnet_info *vi = vq->vdev->priv;
>> > +	struct send_queue *sq = &vi->sq[vq2txq(vq)];
>> >  
>> > -	/* Suppress further interrupts. */
>> > -	virtqueue_disable_cb(vq);
>> > -
>> > -	/* We were probably waiting for more output buffers. */
>> > -	netif_wake_subqueue(vi->dev, vq2txq(vq));
>> > +	if (napi_schedule_prep(&sq->napi)) {
>> > +		virtqueue_disable_cb(vq);
>> > +		virtqueue_disable_cb_urgent(vq);
> This disable_cb is no longer safe in xmit_done callback,
> since queue can be running at the same time.
>
> You must do it under tx lock. And yes, this likely will not work
> work well without event_idx. We'll probably need extra
> synchronization for such old hosts.
>
>
>

Yes, and the virtqueue_enable_cb_prepare() in virtnet_poll_tx() needs to
be synced with virtqueue_enabled_cb_dealyed(). Otherwise an old idx will
be published and we may still see tx interrupt storm.

^ permalink raw reply

* Re: [PATCH net-next RFC 0/3] virtio-net: Conditionally enable tx interrupt
From: Jason Wang @ 2014-10-15  3:24 UTC (permalink / raw)
  To: Michael S. Tsirkin, David Miller
  Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <20141014215133.GA23344@redhat.com>

On 10/15/2014 05:51 AM, Michael S. Tsirkin wrote:
> On Tue, Oct 14, 2014 at 02:53:27PM -0400, David Miller wrote:
>> From: Jason Wang <jasowang@redhat.com>
>> Date: Sat, 11 Oct 2014 15:16:43 +0800
>>
>>> We free old transmitted packets in ndo_start_xmit() currently, so any
>>> packet must be orphaned also there. This was used to reduce the overhead of
>>> tx interrupt to achieve better performance. But this may not work for some
>>> protocols such as TCP stream. TCP depends on the value of sk_wmem_alloc to
>>> implement various optimization for small packets stream such as TCP small
>>> queue and auto corking. But orphaning packets early in ndo_start_xmit()
>>> disable such things more or less since sk_wmem_alloc was not accurate. This
>>> lead extra low throughput for TCP stream of small writes.
>>>
>>> This series tries to solve this issue by enable tx interrupts for all TCP
>>> packets other than the ones with push bit or pure ACK. This is done through
>>> the support of urgent descriptor which can force an interrupt for a
>>> specified packet. If tx interrupt was enabled for a packet, there's no need
>>> to orphan it in ndo_start_xmit(), we can free it tx napi which is scheduled
>>> by tx interrupt. Then sk_wmem_alloc was more accurate than before and TCP
>>> can batch more for small write. More larger skb was produced by TCP in this
>>> case to improve both throughput and cpu utilization.
>>>
>>> Test shows great improvements on small write tcp streams. For most of the
>>> other cases, the throughput and cpu utilization are the same in the
>>> past. Only few cases, more cpu utilization was noticed which needs more
>>> investigation.
>>>
>>> Review and comments are welcomed.
>> I think proper accounting and queueing (at all levels, not just TCP
>> sockets) is more important than trying to skim a bunch of cycles by
>> avoiding TX interrupts.
>>
>> Having an event to free the SKB is absolutely essential for the stack
>> to operate correctly.
>>
>> And with virtio-net you don't even have the excuse of "the HW
>> unfortunately doesn't have an appropriate TX event."
>>
>> So please don't play games, and instead use TX interrupts all the
>> time.  You can mitigate them in various ways, but don't turn them on
>> selectively based upon traffic type, that's terrible.
> This got me thinking: how about using virtqueue_enable_cb_delayed
> for this mitigation?

Should work, another possible solution is interrupt coalescing, which
can speed up also the case without event index.
> It's pretty easy to implement - I'll send a proof of concept patch
> separately.

^ permalink raw reply

* Re: [PATCHv1 0/8] CGroup Namespaces
From: Aditya Kali @ 2014-10-14 23:33 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Linux API, Linux Containers, Serge Hallyn,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar
In-Reply-To: <CALCETrVnjrBt3odufhAirf45_REq-S9T=HpoEWqmFef2M6PucA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Oct 14, 2014 at 3:42 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Oct 13, 2014 at 2:23 PM, Aditya Kali <adityakali@google.com> wrote:
>> Second take at the Cgroup Namespace patch-set.
>>
>> Major changes form RFC (V0):
>> 1. setns support for cgroupns
>> 2. 'mount -t cgroup cgroup <mntpt>' from inside a cgroupns now
>>    mounts the cgroup hierarcy with cgroupns-root as the filesystem root.
>> 3. writes to cgroup files outside of cgroupns-root are not allowed
>> 4. visibility of /proc/<pid>/cgroup is further restricted by not showing
>>    anything if the <pid> is in a sibling cgroupns and its cgroup falls outside
>>    your cgroupns-root.
>>
>> More details in the writeup below.
>>
>> Background
>>   Cgroups and Namespaces are used together to create “virtual”
>>   containers that isolates the host environment from the processes
>>   running in container. But since cgroups themselves are not
>>   “virtualized”, the task is always able to see global cgroups view
>>   through cgroupfs mount and via /proc/self/cgroup file.
>>
>>   $ cat /proc/self/cgroup
>>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>>
>>   This exposure of cgroup names to the processes running inside a
>>   container results in some problems:
>>   (1) The container names are typically host-container-management-agent
>>       (systemd, docker/libcontainer, etc.) data and leaking its name (or
>>       leaking the hierarchy) reveals too much information about the host
>>       system.
>>   (2) It makes the container migration across machines (CRIU) more
>>       difficult as the container names need to be unique across the
>>       machines in the migration domain.
>>   (3) It makes it difficult to run container management tools (like
>>       docker/libcontainer, lmctfy, etc.) within virtual containers
>>       without adding dependency on some state/agent present outside the
>>       container.
>>
>>   Note that the feature proposed here is completely different than the
>>   “ns cgroup” feature which existed in the linux kernel until recently.
>>   The ns cgroup also attempted to connect cgroups and namespaces by
>>   creating a new cgroup every time a new namespace was created. It did
>>   not solve any of the above mentioned problems and was later dropped
>>   from the kernel. Incidentally though, it used the same config option
>>   name CONFIG_CGROUP_NS as used in my prototype!
>>
>> Introducing CGroup Namespaces
>>   With unified cgroup hierarchy
>>   (Documentation/cgroups/unified-hierarchy.txt), the containers can now
>>   have a much more coherent cgroup view and its easy to associate a
>>   container with a single cgroup. This also allows us to virtualize the
>>   cgroup view for tasks inside the container.
>>
>>   The new CGroup Namespace allows a process to “unshare” its cgroup
>>   hierarchy starting from the cgroup its currently in.
>>   For Ex:
>>   $ cat /proc/self/cgroup
>>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>>   $ ls -l /proc/self/ns/cgroup
>>   lrwxrwxrwx 1 root root 0 2014-07-15 10:37 /proc/self/ns/cgroup -> cgroup:[4026531835]
>>   $ ~/unshare -c  # calls unshare(CLONE_NEWCGROUP) and exec’s /bin/bash
>>   [ns]$ ls -l /proc/self/ns/cgroup
>>   lrwxrwxrwx 1 root root 0 2014-07-15 10:35 /proc/self/ns/cgroup ->
>>   cgroup:[4026532183]
>>   # From within new cgroupns, process sees that its in the root cgroup
>>   [ns]$ cat /proc/self/cgroup
>>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
>>
>>   # From global cgroupns:
>>   $ cat /proc/<pid>/cgroup
>>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>>
>>   # Unshare cgroupns along with userns and mountns
>>   # Following calls unshare(CLONE_NEWCGROUP|CLONE_NEWUSER|CLONE_NEWNS), then
>>   # sets up uid/gid map and exec’s /bin/bash
>>   $ ~/unshare -c -u -m
>>
>>   # Originally, we were in /batchjobs/c_job_id1 cgroup. Mount our own cgroup
>>   # hierarchy.
>>   [ns]$ mount -t cgroup cgroup /tmp/cgroup
>>   [ns]$ ls -l /tmp/cgroup
>>   total 0
>>   -r--r--r-- 1 root root 0 2014-10-13 09:32 cgroup.controllers
>>   -r--r--r-- 1 root root 0 2014-10-13 09:32 cgroup.populated
>>   -rw-r--r-- 1 root root 0 2014-10-13 09:25 cgroup.procs
>>   -rw-r--r-- 1 root root 0 2014-10-13 09:32 cgroup.subtree_control
>>
>>   The cgroupns-root (/batchjobs/c_job_id1 in above example) becomes the
>>   filesystem root for the namespace specific cgroupfs mount.
>>
>>   The virtualization of /proc/self/cgroup file combined with restricting
>>   the view of cgroup hierarchy by namespace-private cgroupfs mount
>>   should provide a completely isolated cgroup view inside the container.
>>
>>   In its current form, the cgroup namespaces patcheset provides following
>>   behavior:
>>
>>   (1) The “root” cgroup for a cgroup namespace is the cgroup in which
>>       the process calling unshare is running.
>>       For ex. if a process in /batchjobs/c_job_id1 cgroup calls unshare,
>>       cgroup /batchjobs/c_job_id1 becomes the cgroupns-root.
>>       For the init_cgroup_ns, this is the real root (“/”) cgroup
>>       (identified in code as cgrp_dfl_root.cgrp).
>>
>>   (2) The cgroupns-root cgroup does not change even if the namespace
>>       creator process later moves to a different cgroup.
>>       $ ~/unshare -c # unshare cgroupns in some cgroup
>>       [ns]$ cat /proc/self/cgroup
>>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
>>       [ns]$ mkdir sub_cgrp_1
>>       [ns]$ echo 0 > sub_cgrp_1/cgroup.procs
>>       [ns]$ cat /proc/self/cgroup
>>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
>>
>>   (3) Each process gets its CGROUPNS specific view of
>>       /proc/<pid>/cgroup.
>>   (a) Processes running inside the cgroup namespace will be able to see
>>       cgroup paths (in /proc/self/cgroup) only inside their root cgroup
>>       [ns]$ sleep 100000 &  # From within unshared cgroupns
>>       [1] 7353
>>       [ns]$ echo 7353 > sub_cgrp_1/cgroup.procs
>>       [ns]$ cat /proc/7353/cgroup
>>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
>>
>>   (b) From global cgroupns, the real cgroup path will be visible:
>>       $ cat /proc/7353/cgroup
>>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
>
> This is a little weird.  Not sure it's a problem.
>
>>
>>   (c) From a sibling cgroupns (cgroupns root-ed at a sibling cgroup), no cgroup
>>       path will be visible:
>>       # ns2's cgroupns-root is at '/batchjobs/c_job_id2'
>>       [ns2]$ cat /proc/7353/cgroup
>>       [ns2]$
>>       This is same as when cgroup hierarchy is not mounted at all.
>>       (In correct container setup though, it should not be possible to
>>        access PIDs in another container in the first place.)
>>
>>   (4) Processes inside a cgroupns are not allowed to move out of the
>>       cgroupns-root. This is true even if a privileged process in global
>>       cgroupns tries to move the process out of its cgroupns-root.
>>
>>       # From global cgroupns
>>       $ cat /proc/7353/cgroup
>>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
>>       # cgroupns-root for 7353 is /batchjobs/c_job_id1
>>       $ echo 7353 > batchjobs/c_job_id2/cgroup.procs
>>       -bash: echo: write error: Operation not permitted
>>
>
>>
>>   (6) When some thread from a multi-threaded process unshares its
>>       cgroup-namespace, the new cgroupns gets applied to the entire
>>       process (all the threads). This should be OK since
>>       unified-hierarchy only allows process-level containerization. So
>>       all the threads in the process will have the same cgroup. And both
>>       - changing cgroups and unsharing namespaces - are protected under
>>       threadgroup_lock(task).
>
> This seems odd to me.  Does unsharing the cgroupns unshare for all
> tasks in the process?  If not, then I think that it shouldn't change
> the cgroup either.
>

Unsharing cgorupns unshares for all tasks in the process, yes.

The cgroup changes are protected by threadgroup_lock. So it made sense
to protect cgroupns changes (unshare or setns) by the same lock as we
don't want task's cgroup to change underneath while we are changing
its cgroup-namespace. No cgroup change happens during the
unshare/setns call.

> What did you end up doing to grant permission to unshare the cgroup ns?
>

Currently the only requirement is ns_capable(cgroupns->user_ns,
CAP_SYS_ADMIN). Its possible to refine this further, but for now I
just kept it simpler. I am looking into the explicit permission check
discussed previously (https://lkml.org/lkml/2014/7/29/402), but wanted
to get this out sooner.

> --Andy

Thanks,
-- 
Aditya
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCH net-next RFC 0/3] virtio-net: Conditionally enable tx interrupt
From: Michael S. Tsirkin @ 2014-10-14 23:06 UTC (permalink / raw)
  To: David Miller; +Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <20141014.145327.365091739350390288.davem@davemloft.net>

On Tue, Oct 14, 2014 at 02:53:27PM -0400, David Miller wrote:
> From: Jason Wang <jasowang@redhat.com>
> Date: Sat, 11 Oct 2014 15:16:43 +0800
> 
> > We free old transmitted packets in ndo_start_xmit() currently, so any
> > packet must be orphaned also there. This was used to reduce the overhead of
> > tx interrupt to achieve better performance. But this may not work for some
> > protocols such as TCP stream. TCP depends on the value of sk_wmem_alloc to
> > implement various optimization for small packets stream such as TCP small
> > queue and auto corking. But orphaning packets early in ndo_start_xmit()
> > disable such things more or less since sk_wmem_alloc was not accurate. This
> > lead extra low throughput for TCP stream of small writes.
> > 
> > This series tries to solve this issue by enable tx interrupts for all TCP
> > packets other than the ones with push bit or pure ACK. This is done through
> > the support of urgent descriptor which can force an interrupt for a
> > specified packet. If tx interrupt was enabled for a packet, there's no need
> > to orphan it in ndo_start_xmit(), we can free it tx napi which is scheduled
> > by tx interrupt. Then sk_wmem_alloc was more accurate than before and TCP
> > can batch more for small write. More larger skb was produced by TCP in this
> > case to improve both throughput and cpu utilization.
> > 
> > Test shows great improvements on small write tcp streams. For most of the
> > other cases, the throughput and cpu utilization are the same in the
> > past. Only few cases, more cpu utilization was noticed which needs more
> > investigation.
> > 
> > Review and comments are welcomed.
> 
> I think proper accounting and queueing (at all levels, not just TCP
> sockets) is more important than trying to skim a bunch of cycles by
> avoiding TX interrupts.
> 
> Having an event to free the SKB is absolutely essential for the stack
> to operate correctly.
> 
> And with virtio-net you don't even have the excuse of "the HW
> unfortunately doesn't have an appropriate TX event."
> 
> So please don't play games, and instead use TX interrupts all the
> time.  You can mitigate them in various ways, but don't turn them on
> selectively based upon traffic type, that's terrible.
> 
> You can even use ->xmit_more to defer the TX interrupt indication to
> the final TX packet in the chain.

I guess we can just defer the kick, interrupt will naturally be
deferred as well.
This should solve the problem for old hosts as well.

We'll also need to implement bql for this.
Something like the below?
Completely untested - posting here to see if I figured the
API out correctly. Has to be applied on top of the previous patch.

---

virtio_net: bql + xmit_more

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 62c059d..c245047 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -213,13 +213,15 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
 	return p;
 }
 
-static int free_old_xmit_skbs(struct send_queue *sq, int budget)
+static int free_old_xmit_skbs(struct netdev_queue *txq,
+			      struct send_queue *sq, int budget)
 {
 	struct sk_buff *skb;
 	unsigned int len;
 	struct virtnet_info *vi = sq->vq->vdev->priv;
 	struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
 	int sent = 0;
+	unsigned int bytes = 0;
 
 	while (sent < budget &&
 	       (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
@@ -227,6 +229,7 @@ static int free_old_xmit_skbs(struct send_queue *sq, int budget)
 
 		u64_stats_update_begin(&stats->tx_syncp);
 		stats->tx_bytes += skb->len;
+		bytes += skb->len;
 		stats->tx_packets++;
 		u64_stats_update_end(&stats->tx_syncp);
 
@@ -234,6 +237,8 @@ static int free_old_xmit_skbs(struct send_queue *sq, int budget)
 		sent++;
 	}
 
+	netdev_tx_completed_queue(txq, sent, bytes);
+
 	return sent;
 }
 
@@ -802,7 +807,7 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
 again:
 	__netif_tx_lock(txq, smp_processor_id());
 	virtqueue_disable_cb(sq->vq);
-	sent += free_old_xmit_skbs(sq, budget - sent);
+	sent += free_old_xmit_skbs(txq, sq, budget - sent);
 
 	if (sent < budget) {
 		r = virtqueue_enable_cb_prepare(sq->vq);
@@ -951,6 +956,9 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int qnum = skb_get_queue_mapping(skb);
 	struct send_queue *sq = &vi->sq[qnum];
 	int err, qsize = virtqueue_get_vring_size(sq->vq);
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
+	bool kick = !skb->xmit_more || netif_xmit_stopped(txq);
+	unsigned int bytes = skb->len;
 
 	virtqueue_disable_cb(sq->vq);
 
@@ -967,7 +975,11 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		dev_kfree_skb_any(skb);
 		return NETDEV_TX_OK;
 	}
-	virtqueue_kick(sq->vq);
+
+	netdev_tx_sent_queue(txq, bytes);
+
+	if (kick)
+		virtqueue_kick(sq->vq);
 
 	/* Apparently nice girls don't return TX_BUSY; stop the queue
 	 * before it gets out of hand.  Naturally, this wastes entries. */
@@ -975,14 +987,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_subqueue(dev, qnum);
 		if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
 			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(sq, qsize);
+			free_old_xmit_skbs(txq, sq, qsize);
 			if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
 				netif_start_subqueue(dev, qnum);
 				virtqueue_disable_cb(sq->vq);
 			}
 		}
 	} else if (virtqueue_enable_cb_delayed(sq->vq)) {
-		free_old_xmit_skbs(sq, qsize);
+		free_old_xmit_skbs(txq, sq, qsize);
 	}
 
 	return NETDEV_TX_OK;

^ permalink raw reply related

* Re: [PATCHv1 0/8] CGroup Namespaces
From: Andy Lutomirski @ 2014-10-14 22:42 UTC (permalink / raw)
  To: Aditya Kali
  Cc: Linux API, Linux Containers, Serge Hallyn,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Tejun Heo,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar
In-Reply-To: <1413235430-22944-1-git-send-email-adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Mon, Oct 13, 2014 at 2:23 PM, Aditya Kali <adityakali@google.com> wrote:
> Second take at the Cgroup Namespace patch-set.
>
> Major changes form RFC (V0):
> 1. setns support for cgroupns
> 2. 'mount -t cgroup cgroup <mntpt>' from inside a cgroupns now
>    mounts the cgroup hierarcy with cgroupns-root as the filesystem root.
> 3. writes to cgroup files outside of cgroupns-root are not allowed
> 4. visibility of /proc/<pid>/cgroup is further restricted by not showing
>    anything if the <pid> is in a sibling cgroupns and its cgroup falls outside
>    your cgroupns-root.
>
> More details in the writeup below.
>
> Background
>   Cgroups and Namespaces are used together to create “virtual”
>   containers that isolates the host environment from the processes
>   running in container. But since cgroups themselves are not
>   “virtualized”, the task is always able to see global cgroups view
>   through cgroupfs mount and via /proc/self/cgroup file.
>
>   $ cat /proc/self/cgroup
>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>
>   This exposure of cgroup names to the processes running inside a
>   container results in some problems:
>   (1) The container names are typically host-container-management-agent
>       (systemd, docker/libcontainer, etc.) data and leaking its name (or
>       leaking the hierarchy) reveals too much information about the host
>       system.
>   (2) It makes the container migration across machines (CRIU) more
>       difficult as the container names need to be unique across the
>       machines in the migration domain.
>   (3) It makes it difficult to run container management tools (like
>       docker/libcontainer, lmctfy, etc.) within virtual containers
>       without adding dependency on some state/agent present outside the
>       container.
>
>   Note that the feature proposed here is completely different than the
>   “ns cgroup” feature which existed in the linux kernel until recently.
>   The ns cgroup also attempted to connect cgroups and namespaces by
>   creating a new cgroup every time a new namespace was created. It did
>   not solve any of the above mentioned problems and was later dropped
>   from the kernel. Incidentally though, it used the same config option
>   name CONFIG_CGROUP_NS as used in my prototype!
>
> Introducing CGroup Namespaces
>   With unified cgroup hierarchy
>   (Documentation/cgroups/unified-hierarchy.txt), the containers can now
>   have a much more coherent cgroup view and its easy to associate a
>   container with a single cgroup. This also allows us to virtualize the
>   cgroup view for tasks inside the container.
>
>   The new CGroup Namespace allows a process to “unshare” its cgroup
>   hierarchy starting from the cgroup its currently in.
>   For Ex:
>   $ cat /proc/self/cgroup
>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>   $ ls -l /proc/self/ns/cgroup
>   lrwxrwxrwx 1 root root 0 2014-07-15 10:37 /proc/self/ns/cgroup -> cgroup:[4026531835]
>   $ ~/unshare -c  # calls unshare(CLONE_NEWCGROUP) and exec’s /bin/bash
>   [ns]$ ls -l /proc/self/ns/cgroup
>   lrwxrwxrwx 1 root root 0 2014-07-15 10:35 /proc/self/ns/cgroup ->
>   cgroup:[4026532183]
>   # From within new cgroupns, process sees that its in the root cgroup
>   [ns]$ cat /proc/self/cgroup
>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
>
>   # From global cgroupns:
>   $ cat /proc/<pid>/cgroup
>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>
>   # Unshare cgroupns along with userns and mountns
>   # Following calls unshare(CLONE_NEWCGROUP|CLONE_NEWUSER|CLONE_NEWNS), then
>   # sets up uid/gid map and exec’s /bin/bash
>   $ ~/unshare -c -u -m
>
>   # Originally, we were in /batchjobs/c_job_id1 cgroup. Mount our own cgroup
>   # hierarchy.
>   [ns]$ mount -t cgroup cgroup /tmp/cgroup
>   [ns]$ ls -l /tmp/cgroup
>   total 0
>   -r--r--r-- 1 root root 0 2014-10-13 09:32 cgroup.controllers
>   -r--r--r-- 1 root root 0 2014-10-13 09:32 cgroup.populated
>   -rw-r--r-- 1 root root 0 2014-10-13 09:25 cgroup.procs
>   -rw-r--r-- 1 root root 0 2014-10-13 09:32 cgroup.subtree_control
>
>   The cgroupns-root (/batchjobs/c_job_id1 in above example) becomes the
>   filesystem root for the namespace specific cgroupfs mount.
>
>   The virtualization of /proc/self/cgroup file combined with restricting
>   the view of cgroup hierarchy by namespace-private cgroupfs mount
>   should provide a completely isolated cgroup view inside the container.
>
>   In its current form, the cgroup namespaces patcheset provides following
>   behavior:
>
>   (1) The “root” cgroup for a cgroup namespace is the cgroup in which
>       the process calling unshare is running.
>       For ex. if a process in /batchjobs/c_job_id1 cgroup calls unshare,
>       cgroup /batchjobs/c_job_id1 becomes the cgroupns-root.
>       For the init_cgroup_ns, this is the real root (“/”) cgroup
>       (identified in code as cgrp_dfl_root.cgrp).
>
>   (2) The cgroupns-root cgroup does not change even if the namespace
>       creator process later moves to a different cgroup.
>       $ ~/unshare -c # unshare cgroupns in some cgroup
>       [ns]$ cat /proc/self/cgroup
>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
>       [ns]$ mkdir sub_cgrp_1
>       [ns]$ echo 0 > sub_cgrp_1/cgroup.procs
>       [ns]$ cat /proc/self/cgroup
>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
>
>   (3) Each process gets its CGROUPNS specific view of
>       /proc/<pid>/cgroup.
>   (a) Processes running inside the cgroup namespace will be able to see
>       cgroup paths (in /proc/self/cgroup) only inside their root cgroup
>       [ns]$ sleep 100000 &  # From within unshared cgroupns
>       [1] 7353
>       [ns]$ echo 7353 > sub_cgrp_1/cgroup.procs
>       [ns]$ cat /proc/7353/cgroup
>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
>
>   (b) From global cgroupns, the real cgroup path will be visible:
>       $ cat /proc/7353/cgroup
>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1

This is a little weird.  Not sure it's a problem.

>
>   (c) From a sibling cgroupns (cgroupns root-ed at a sibling cgroup), no cgroup
>       path will be visible:
>       # ns2's cgroupns-root is at '/batchjobs/c_job_id2'
>       [ns2]$ cat /proc/7353/cgroup
>       [ns2]$
>       This is same as when cgroup hierarchy is not mounted at all.
>       (In correct container setup though, it should not be possible to
>        access PIDs in another container in the first place.)
>
>   (4) Processes inside a cgroupns are not allowed to move out of the
>       cgroupns-root. This is true even if a privileged process in global
>       cgroupns tries to move the process out of its cgroupns-root.
>
>       # From global cgroupns
>       $ cat /proc/7353/cgroup
>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
>       # cgroupns-root for 7353 is /batchjobs/c_job_id1
>       $ echo 7353 > batchjobs/c_job_id2/cgroup.procs
>       -bash: echo: write error: Operation not permitted
>

>
>   (6) When some thread from a multi-threaded process unshares its
>       cgroup-namespace, the new cgroupns gets applied to the entire
>       process (all the threads). This should be OK since
>       unified-hierarchy only allows process-level containerization. So
>       all the threads in the process will have the same cgroup. And both
>       - changing cgroups and unsharing namespaces - are protected under
>       threadgroup_lock(task).

This seems odd to me.  Does unsharing the cgroupns unshare for all
tasks in the process?  If not, then I think that it shouldn't change
the cgroup either.

What did you end up doing to grant permission to unshare the cgroup ns?

--Andy
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCH net-next RFC 3/3] virtio-net: conditionally enable tx interrupt
From: Michael S. Tsirkin @ 2014-10-14 21:51 UTC (permalink / raw)
  To: Jason Wang; +Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <1413011806-3813-4-git-send-email-jasowang@redhat.com>

On Sat, Oct 11, 2014 at 03:16:46PM +0800, Jason Wang wrote:
> We free transmitted packets in ndo_start_xmit() in the past to get better
> performance in the past. One side effect is that skb_orphan() needs to be
> called in ndo_start_xmit() which makes sk_wmem_alloc not accurate in
> fact. For TCP protocol, this means several optimization could not work well
> such as TCP small queue and auto corking. This can lead extra low
> throughput of small packets stream.
> 
> Thanks to the urgent descriptor support. This patch tries to solve this
> issue by enable the tx interrupt selectively for stream packets. This means
> we don't need to orphan TCP stream packets in ndo_start_xmit() but enable
> tx interrupt for those packets. After we get tx interrupt, a tx napi was
> scheduled to free those packets.
> 
> With this method, sk_wmem_alloc of TCP socket were more accurate than in
> the past which let TCP can batch more through TSQ and auto corking.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/net/virtio_net.c | 164 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 128 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 5810841..b450fc4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -72,6 +72,8 @@ struct send_queue {
>  
>  	/* Name of the send queue: output.$index */
>  	char name[40];
> +
> +	struct napi_struct napi;
>  };
>  
>  /* Internal representation of a receive virtqueue */
> @@ -217,15 +219,40 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask)
>  	return p;
>  }
>  
> +static int free_old_xmit_skbs(struct send_queue *sq, int budget)
> +{
> +	struct sk_buff *skb;
> +	unsigned int len;
> +	struct virtnet_info *vi = sq->vq->vdev->priv;
> +	struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> +	int sent = 0;
> +
> +	while (sent < budget &&
> +	       (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		pr_debug("Sent skb %p\n", skb);
> +
> +		u64_stats_update_begin(&stats->tx_syncp);
> +		stats->tx_bytes += skb->len;
> +		stats->tx_packets++;
> +		u64_stats_update_end(&stats->tx_syncp);
> +
> +		dev_kfree_skb_any(skb);
> +		sent++;
> +	}
> +
> +	return sent;
> +}
> +
>  static void skb_xmit_done(struct virtqueue *vq)
>  {
>  	struct virtnet_info *vi = vq->vdev->priv;
> +	struct send_queue *sq = &vi->sq[vq2txq(vq)];
>  
> -	/* Suppress further interrupts. */
> -	virtqueue_disable_cb(vq);
> -
> -	/* We were probably waiting for more output buffers. */
> -	netif_wake_subqueue(vi->dev, vq2txq(vq));
> +	if (napi_schedule_prep(&sq->napi)) {
> +		virtqueue_disable_cb(vq);
> +		virtqueue_disable_cb_urgent(vq);

This disable_cb is no longer safe in xmit_done callback,
since queue can be running at the same time.

You must do it under tx lock. And yes, this likely will not work
work well without event_idx. We'll probably need extra
synchronization for such old hosts.



> +		__napi_schedule(&sq->napi);
> +	}
>  }
>  
>  static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx)
> @@ -772,7 +799,38 @@ again:
>  	return received;
>  }
>  
> +static int virtnet_poll_tx(struct napi_struct *napi, int budget)
> +{
> +	struct send_queue *sq =
> +		container_of(napi, struct send_queue, napi);
> +	struct virtnet_info *vi = sq->vq->vdev->priv;
> +	struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq));
> +	unsigned int r, sent = 0;
> +
> +again:
> +	__netif_tx_lock(txq, smp_processor_id());
> +	sent += free_old_xmit_skbs(sq, budget - sent);
> +
> +	if (sent < budget) {
> +		r = virtqueue_enable_cb_prepare_urgent(sq->vq);
> +		napi_complete(napi);
> +		__netif_tx_unlock(txq);
> +		if (unlikely(virtqueue_poll(sq->vq, r)) &&
> +		    napi_schedule_prep(napi)) {
> +			virtqueue_disable_cb_urgent(sq->vq);
> +			__napi_schedule(napi);
> +			goto again;
> +		}
> +	} else {
> +		__netif_tx_unlock(txq);
> +	}
> +
> +	netif_wake_subqueue(vi->dev, vq2txq(sq->vq));
> +	return sent;
> +}
> +
>  #ifdef CONFIG_NET_RX_BUSY_POLL
> +
>  /* must be called with local_bh_disable()d */
>  static int virtnet_busy_poll(struct napi_struct *napi)
>  {
> @@ -820,31 +878,13 @@ static int virtnet_open(struct net_device *dev)
>  			if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
>  				schedule_delayed_work(&vi->refill, 0);
>  		virtnet_napi_enable(&vi->rq[i]);
> +		napi_enable(&vi->sq[i].napi);
>  	}
>  
>  	return 0;
>  }
>  
> -static void free_old_xmit_skbs(struct send_queue *sq)
> -{
> -	struct sk_buff *skb;
> -	unsigned int len;
> -	struct virtnet_info *vi = sq->vq->vdev->priv;
> -	struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> -
> -	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> -		pr_debug("Sent skb %p\n", skb);
> -
> -		u64_stats_update_begin(&stats->tx_syncp);
> -		stats->tx_bytes += skb->len;
> -		stats->tx_packets++;
> -		u64_stats_update_end(&stats->tx_syncp);
> -
> -		dev_kfree_skb_any(skb);
> -	}
> -}
> -
> -static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> +static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool urgent)
>  {
>  	struct skb_vnet_hdr *hdr;
>  	const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> @@ -908,7 +948,43 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
>  		sg_set_buf(sq->sg, hdr, hdr_len);
>  		num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
>  	}
> -	return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
> +	if (urgent)
> +		return virtqueue_add_outbuf_urgent(sq->vq, sq->sg, num_sg,
> +						   skb, GFP_ATOMIC);
> +	else
> +		return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
> +					    GFP_ATOMIC);
> +}
> +
> +static bool virtnet_skb_needs_intr(struct sk_buff *skb)
> +{
> +	union {
> +		unsigned char *network;
> +		struct iphdr *ipv4;
> +		struct ipv6hdr *ipv6;
> +	} hdr;
> +	struct tcphdr *th = tcp_hdr(skb);
> +	u16 payload_len;
> +
> +	hdr.network = skb_network_header(skb);
> +
> +	/* Only IPv4/IPv6 with TCP is supported */
> +	if ((skb->protocol == htons(ETH_P_IP)) &&
> +	    hdr.ipv4->protocol == IPPROTO_TCP) {
> +		payload_len = ntohs(hdr.ipv4->tot_len) - hdr.ipv4->ihl * 4 -
> +			      th->doff * 4;
> +	} else if ((skb->protocol == htons(ETH_P_IPV6) ||
> +		   hdr.ipv6->nexthdr == IPPROTO_TCP)) {
> +		payload_len = ntohs(hdr.ipv6->payload_len) - th->doff * 4;
> +	} else {
> +		return false;
> +	}
> +
> +	/* We don't want to dealy packet with PUSH bit and pure ACK packet */
> +	if (!th->psh && payload_len)
> +		return true;
> +
> +	return false;
>  }
>  
>  static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> @@ -916,13 +992,15 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	struct virtnet_info *vi = netdev_priv(dev);
>  	int qnum = skb_get_queue_mapping(skb);
>  	struct send_queue *sq = &vi->sq[qnum];
> -	int err;
> +	bool urgent = virtnet_skb_needs_intr(skb);
> +	int err, qsize = virtqueue_get_vring_size(sq->vq);
>  
> +	virtqueue_disable_cb_urgent(sq->vq);
>  	/* Free up any pending old buffers before queueing new ones. */
> -	free_old_xmit_skbs(sq);
> +	free_old_xmit_skbs(sq, qsize);
>  
>  	/* Try to transmit */
> -	err = xmit_skb(sq, skb);
> +	err = xmit_skb(sq, skb, urgent);
>  
>  	/* This should not happen! */
>  	if (unlikely(err)) {
> @@ -935,22 +1013,26 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>  		return NETDEV_TX_OK;
>  	}
>  
> -	/* Don't wait up for transmitted skbs to be freed. */
> -	skb_orphan(skb);
> -	nf_reset(skb);
> +	if (!urgent) {
> +		skb_orphan(skb);
> +		nf_reset(skb);
> +	}
>  
>  	/* Apparently nice girls don't return TX_BUSY; stop the queue
>  	 * before it gets out of hand.  Naturally, this wastes entries. */
>  	if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
>  		netif_stop_subqueue(dev, qnum);
> +		virtqueue_disable_cb_urgent(sq->vq);
>  		if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
>  			/* More just got used, free them then recheck. */
> -			free_old_xmit_skbs(sq);
> +			free_old_xmit_skbs(sq, qsize);
>  			if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
>  				netif_start_subqueue(dev, qnum);
>  				virtqueue_disable_cb(sq->vq);
>  			}
>  		}
> +	} else if (virtqueue_enable_cb_urgent(sq->vq)) {
> +		free_old_xmit_skbs(sq, qsize);
>  	}
>  
>  	if (__netif_subqueue_stopped(dev, qnum) || !skb->xmit_more)
> @@ -1132,8 +1214,10 @@ static int virtnet_close(struct net_device *dev)
>  	/* Make sure refill_work doesn't re-enable napi! */
>  	cancel_delayed_work_sync(&vi->refill);
>  
> -	for (i = 0; i < vi->max_queue_pairs; i++)
> +	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		napi_disable(&vi->rq[i].napi);
> +		napi_disable(&vi->sq[i].napi);
> +	}
>  
>  	return 0;
>  }
> @@ -1452,8 +1536,10 @@ static void virtnet_free_queues(struct virtnet_info *vi)
>  {
>  	int i;
>  
> -	for (i = 0; i < vi->max_queue_pairs; i++)
> +	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		netif_napi_del(&vi->rq[i].napi);
> +		netif_napi_del(&vi->sq[i].napi);
> +	}
>  
>  	kfree(vi->rq);
>  	kfree(vi->sq);
> @@ -1607,6 +1693,8 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
>  		netif_napi_add(vi->dev, &vi->rq[i].napi, virtnet_poll,
>  			       napi_weight);
>  		napi_hash_add(&vi->rq[i].napi);
> +		netif_napi_add(vi->dev, &vi->sq[i].napi, virtnet_poll_tx,
> +			       napi_weight);
>  
>  		sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
>  		ewma_init(&vi->rq[i].mrg_avg_pkt_len, 1, RECEIVE_AVG_WEIGHT);
> @@ -1912,8 +2000,10 @@ static int virtnet_freeze(struct virtio_device *vdev)
>  	if (netif_running(vi->dev)) {
>  		for (i = 0; i < vi->max_queue_pairs; i++) {
>  			napi_disable(&vi->rq[i].napi);
> +			napi_disable(&vi->sq[i].napi);
>  			napi_hash_del(&vi->rq[i].napi);
>  			netif_napi_del(&vi->rq[i].napi);
> +			netif_napi_del(&vi->sq[i].napi);
>  		}
>  	}
>  
> @@ -1938,8 +2028,10 @@ static int virtnet_restore(struct virtio_device *vdev)
>  			if (!try_fill_recv(&vi->rq[i], GFP_KERNEL))
>  				schedule_delayed_work(&vi->refill, 0);
>  
> -		for (i = 0; i < vi->max_queue_pairs; i++)
> +		for (i = 0; i < vi->max_queue_pairs; i++) {
>  			virtnet_napi_enable(&vi->rq[i]);
> +			napi_enable(&vi->sq[i].napi);
> +		}
>  	}
>  
>  	netif_device_attach(vi->dev);
> -- 
> 1.8.3.1

^ permalink raw reply

* Re: [PATCH net-next RFC 0/3] virtio-net: Conditionally enable tx interrupt
From: Michael S. Tsirkin @ 2014-10-14 21:51 UTC (permalink / raw)
  To: David Miller; +Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <20141014.145327.365091739350390288.davem@davemloft.net>

On Tue, Oct 14, 2014 at 02:53:27PM -0400, David Miller wrote:
> From: Jason Wang <jasowang@redhat.com>
> Date: Sat, 11 Oct 2014 15:16:43 +0800
> 
> > We free old transmitted packets in ndo_start_xmit() currently, so any
> > packet must be orphaned also there. This was used to reduce the overhead of
> > tx interrupt to achieve better performance. But this may not work for some
> > protocols such as TCP stream. TCP depends on the value of sk_wmem_alloc to
> > implement various optimization for small packets stream such as TCP small
> > queue and auto corking. But orphaning packets early in ndo_start_xmit()
> > disable such things more or less since sk_wmem_alloc was not accurate. This
> > lead extra low throughput for TCP stream of small writes.
> > 
> > This series tries to solve this issue by enable tx interrupts for all TCP
> > packets other than the ones with push bit or pure ACK. This is done through
> > the support of urgent descriptor which can force an interrupt for a
> > specified packet. If tx interrupt was enabled for a packet, there's no need
> > to orphan it in ndo_start_xmit(), we can free it tx napi which is scheduled
> > by tx interrupt. Then sk_wmem_alloc was more accurate than before and TCP
> > can batch more for small write. More larger skb was produced by TCP in this
> > case to improve both throughput and cpu utilization.
> > 
> > Test shows great improvements on small write tcp streams. For most of the
> > other cases, the throughput and cpu utilization are the same in the
> > past. Only few cases, more cpu utilization was noticed which needs more
> > investigation.
> > 
> > Review and comments are welcomed.
> 
> I think proper accounting and queueing (at all levels, not just TCP
> sockets) is more important than trying to skim a bunch of cycles by
> avoiding TX interrupts.
> 
> Having an event to free the SKB is absolutely essential for the stack
> to operate correctly.
> 
> And with virtio-net you don't even have the excuse of "the HW
> unfortunately doesn't have an appropriate TX event."
> 
> So please don't play games, and instead use TX interrupts all the
> time.  You can mitigate them in various ways, but don't turn them on
> selectively based upon traffic type, that's terrible.

This got me thinking: how about using virtqueue_enable_cb_delayed
for this mitigation?

It's pretty easy to implement - I'll send a proof of concept patch
separately.

^ permalink raw reply

* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Daniel Borkmann @ 2014-10-14 20:27 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
	Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
	Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
	Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuws+AtOdwud8_YjXhs=yomt8nY+49f_UuhofcmhV58c1Q@mail.gmail.com>

On 10/14/2014 10:43 AM, Alexei Starovoitov wrote:
> On Tue, Oct 14, 2014 at 12:34 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 10/13/2014 11:49 PM, Alexei Starovoitov wrote:
>>>
>>> On Mon, Oct 13, 2014 at 10:21 AM, Daniel Borkmann <dborkman@redhat.com>
>>> wrote:
>>>>
>>>> On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
>>>> ...
>>>>>
>>>>> Ok, given you post the remaining two RFCs later on this window as
>>>>> you indicate, I have no objections:
>>>>>
>>>>> Acked-by: Daniel Borkmann <dborkman@redhat.com>
>>>>
>>>> Ping, Alexei, are you still sending the patch for bpf_common.h or
>>>> do you want me to take care of this?
>>>
>>> It's not forgotten.
>>> I'm not sending it only because net-next is closed
>>> and it seems to be -next material.
>>
>> Well, the point was since it's UAPI you're modifying, that it needs
>> to be shipped before it first gets exposed to user land ...
>>
>> I think that should be reason enough ... there's no point in doing
>> this at a later point in time.
>
> Moving common #defines from filter.h into bpf_common.h can
> be done at any point in time. For the sake of argument if
> there is an app that includes both filter.h and bpf.h, it will
> continue to work just fine.

Correct, but the argument was that we can _avoid_ this from the
very beginning. Thus, user space applications making use of eBPF
only need to include <linux/bpf.h>, nothing more.

Doing this at any later point in time will just lead to the need
to include both headers.

^ permalink raw reply

* Re: [PATCH net] net: filter: move common defines into bpf_common.h
From: David Miller @ 2014-10-14 20:07 UTC (permalink / raw)
  To: ast-uqk4Ao+rVK5Wk0Htik3J/w
  Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
	torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA,
	hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
	chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
	a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1413277734-13053-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Tue, 14 Oct 2014 02:08:54 -0700

> userspace programs that use eBPF instruction macros need to include two files:
> uapi/linux/filter.h and uapi/linux/bpf.h
> Move common macro definitions that are shared between classic BPF and eBPF
> into uapi/linux/bpf_common.h, so that user app can include only one bpf.h file
> 
> Cc: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> ---
> 
> Daniel believes that this patch has to be done for this merge window.
> I think it can wait till next, but it won't hurt now either.

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH net-next RFC 0/3] virtio-net: Conditionally enable tx interrupt
From: David Miller @ 2014-10-14 18:53 UTC (permalink / raw)
  To: jasowang-H+wXaHxf7aLQT0dZR+AlfA
  Cc: rusty-8n+1lVoiYb80n/F98K4Iww, mst-H+wXaHxf7aLQT0dZR+AlfA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, kvm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1413011806-3813-1-git-send-email-jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

From: Jason Wang <jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Sat, 11 Oct 2014 15:16:43 +0800

> We free old transmitted packets in ndo_start_xmit() currently, so any
> packet must be orphaned also there. This was used to reduce the overhead of
> tx interrupt to achieve better performance. But this may not work for some
> protocols such as TCP stream. TCP depends on the value of sk_wmem_alloc to
> implement various optimization for small packets stream such as TCP small
> queue and auto corking. But orphaning packets early in ndo_start_xmit()
> disable such things more or less since sk_wmem_alloc was not accurate. This
> lead extra low throughput for TCP stream of small writes.
> 
> This series tries to solve this issue by enable tx interrupts for all TCP
> packets other than the ones with push bit or pure ACK. This is done through
> the support of urgent descriptor which can force an interrupt for a
> specified packet. If tx interrupt was enabled for a packet, there's no need
> to orphan it in ndo_start_xmit(), we can free it tx napi which is scheduled
> by tx interrupt. Then sk_wmem_alloc was more accurate than before and TCP
> can batch more for small write. More larger skb was produced by TCP in this
> case to improve both throughput and cpu utilization.
> 
> Test shows great improvements on small write tcp streams. For most of the
> other cases, the throughput and cpu utilization are the same in the
> past. Only few cases, more cpu utilization was noticed which needs more
> investigation.
> 
> Review and comments are welcomed.

I think proper accounting and queueing (at all levels, not just TCP
sockets) is more important than trying to skim a bunch of cycles by
avoiding TX interrupts.

Having an event to free the SKB is absolutely essential for the stack
to operate correctly.

And with virtio-net you don't even have the excuse of "the HW
unfortunately doesn't have an appropriate TX event."

So please don't play games, and instead use TX interrupts all the
time.  You can mitigate them in various ways, but don't turn them on
selectively based upon traffic type, that's terrible.

You can even use ->xmit_more to defer the TX interrupt indication to
the final TX packet in the chain.

^ permalink raw reply

* [GIT PULL] kselftest 3.18-updates-2
From: Shuah Khan @ 2014-10-14 17:01 UTC (permalink / raw)
  To: linus Torvalds; +Cc: gregkh, akpm, linux-api, linux-kernel, Shuah Khan

Hi Linus,

Here are the second round updates for kselftest for 3.18. Please
see details in the signed tag.


The following changes since commit 69e273c0b0a3c337a521d083374c918dc52c666f:

  Linux 3.17-rc3 (2014-08-31 18:23:04 -0700)

are available in the git repository at:


git@gitolite.kernel.org:/pub/scm/linux/kernel/git/shuah/linux-kselftest
tags/kselftest-3.18-updates-2

for you to fetch changes up to 49296470830aa12027f4aa43ec54f19375be095b:

  selftests/timers: change test to use ksft framework (2014-10-06
08:29:20 -0600)

----------------------------------------------------------------
3.18 Kselftest updates-2:

This update adds a light weight kselftest framework that
provides a set of interfaces for tests to use to report
results. In addition, several tests are updated to use the
framework.

----------------------------------------------------------------
Shuah Khan (5):
      selftests: add kselftest framework for uniform test reporting
      selftests/breakpoints: change test to use ksft framework
      selftests/ipc: change test to use ksft framework
      selftests/kcmp: change test to use ksft framework
      selftests/timers: change test to use ksft framework

 .../selftests/breakpoints/breakpoint_test.c        | 10 ++--
 tools/testing/selftests/ipc/msgque.c               | 26 ++++-----
 tools/testing/selftests/kcmp/kcmp_test.c           | 27 +++++++---
 tools/testing/selftests/kselftest.h                | 62
++++++++++++++++++++++
 tools/testing/selftests/timers/posix_timers.c      | 14 ++---
 5 files changed, 110 insertions(+), 29 deletions(-)
 create mode 100644 tools/testing/selftests/kselftest.h

thanks,
-- Shuah
-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply

* Re: [RFC PATCH v2 3/3] i2c: show and change bus frequency via sysfs
From: Guenter Roeck @ 2014-10-14 15:41 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: wsa-z923LK4zBo2bacvFa/9K2g, johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1413298094-9276-4-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On Tue, Oct 14, 2014 at 05:48:14PM +0300, Octavian Purdila wrote:
> This patch adds three new sysfs files: bus_frequency,
> bus_min_frequency and bus_max_frequency which allows the user to view
> or change the bus frequency on a per bus level.
> 
> This is required for the case where multiple busses have the same
> adapter driver and where a module parameter does not allow controlling
> the bus speed individually (e.g. USB I2C adapters).
> 
> Signed-off-by: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/ABI/testing/sysfs-bus-i2c | 30 +++++++++++
>  drivers/i2c/i2c-core.c                  | 94 +++++++++++++++++++++++++++++++++
>  include/linux/i2c.h                     | 16 ++++++
>  3 files changed, 140 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-i2c b/Documentation/ABI/testing/sysfs-bus-i2c
> index 8075585..4a7f8e7 100644
> --- a/Documentation/ABI/testing/sysfs-bus-i2c
> +++ b/Documentation/ABI/testing/sysfs-bus-i2c
> @@ -43,3 +43,33 @@ Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>  Description:
>  		An i2c device attached to bus X that is enumerated via
>  		ACPI. Y is the ACPI device name and its format is "%s".
> +
> +What:		/sys/bus/i2c/devices/i2c-X/bus_frequency
> +KernelVersion:	3.19
> +Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> +		The current bus frequency for bus X. Can be updated if
> +		the bus supports it. The unit is HZ and format is
> +		"%d\n".
> +		If the bus does not support changing the frequency
> +		then this entry will be read-only.
> +		If the bus does not support showing the frequency than
> +		this entry will not be visible.
> +		When updating the bus frequency that value must be in
> +		the range defined by bus_frequency_min and
> +		bus_frequency_max otherwise writing to this entry will
> +		fail with -EINVAL.
> +		The bus may not support all of the frequencies in the
> +		min-max range and may round the frequency to the
> +		closest supported one. The user must read the entry
> +		after writing it to retrieve the actual set frequency.
> +
> +What:		/sys/bus/i2c/devices/i2c-X/bus_min_frequency
> +What:		/sys/bus/i2c/devices/i2c-X/bus_max_frequency
> +KernelVersion:	3.19
> +Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> +		Minimum and maximum frequencies for bus X. The unit is
> +		HZ and format is "%d\n".
> +		If the bus does not support changing the frequency
> +		these entries will not be visible.
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 632057a..ab77f7f 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -940,15 +940,101 @@ static DEVICE_ATTR(new_device, S_IWUSR, NULL, i2c_sysfs_new_device);
>  static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, S_IWUSR, NULL,
>  				   i2c_sysfs_delete_device);
>  
> +static ssize_t
> +i2c_sysfs_freq_show(struct device *dev, struct device_attribute *attr,
> +		    char *buf)
> +{
> +	struct i2c_adapter *adap = to_i2c_adapter(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", adap->freq);
> +}
> +
> +static ssize_t
> +i2c_sysfs_freq_store(struct device *dev, struct device_attribute *attr,
> +		     const char *buf, size_t count)
> +{
> +	struct i2c_adapter *adap = to_i2c_adapter(dev);
> +	unsigned int freq;
> +	int ret;
> +
> +	if (kstrtouint(buf, 0, &freq) || freq < adap->min_freq ||
> +	    freq > adap->max_freq)
> +		return -EINVAL;
> +
> +	i2c_lock_adapter(adap);
> +	ret = adap->set_freq(adap, &freq);
> +	i2c_unlock_adapter(adap);
> +
> +	if (ret)
> +		return ret;
> +
> +	adap->freq = freq;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(bus_frequency, S_IRUGO, i2c_sysfs_freq_show,
> +		   i2c_sysfs_freq_store);

Consider using DEVICE_ATTR_RO here. Also, extra empty line.

> +
> +
> +static ssize_t
> +i2c_sysfs_min_freq_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	struct i2c_adapter *adap = to_i2c_adapter(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", adap->min_freq);
> +}
> +
> +static DEVICE_ATTR(bus_min_frequency, S_IRUGO, i2c_sysfs_min_freq_show, NULL);
> +

Consider using DEVICE_ATTR_RO.

> +static ssize_t
> +i2c_sysfs_max_freq_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	struct i2c_adapter *adap = to_i2c_adapter(dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", adap->max_freq);
> +}
> +
> +static DEVICE_ATTR(bus_max_frequency, S_IRUGO, i2c_sysfs_max_freq_show, NULL);
> +
Consider using DEVICE_ATTR_RO.

Overall, it seems to me that the bus_ in front of the attrribute names
is really not necessary. The attributes are attached to the adapter, so it
should be obvious that the attributes describe the adapter (=bus) frequency and
not some other frequency.

> +umode_t i2c_adapter_visible_attr(struct kobject *kobj,
> +				 struct attribute *attr, int idx)

static umode_t

> +{
> +	struct device *dev = container_of(kobj, struct device, kobj);
> +	struct i2c_adapter *adap = to_i2c_adapter(dev);
> +	umode_t mode = attr->mode;
> +
> +	if (attr == &dev_attr_bus_min_frequency.attr)
> +		return adap->min_freq ? mode : 0;
> +
> +	if (attr == &dev_attr_bus_max_frequency.attr)
> +		return adap->max_freq ? mode : 0;
> +
> +	if (attr == &dev_attr_bus_frequency.attr) {
> +		if (adap->set_freq)
> +			mode |= S_IWUSR;
> +		return adap->freq ? mode : 0;

Personally, I would make all those attributes only visible if the adapter
supports setting the frquency, and not bother with other conditions,
to keep things simple. Not a strong call, though.

> +	}
> +
> +	return mode;
> +}
> +
> +
extra empty line

>  static struct attribute *i2c_adapter_attrs[] = {
>  	&dev_attr_name.attr,
>  	&dev_attr_new_device.attr,
>  	&dev_attr_delete_device.attr,
> +	&dev_attr_bus_frequency.attr,
> +	&dev_attr_bus_min_frequency.attr,
> +	&dev_attr_bus_max_frequency.attr,
>  	NULL
>  };
>  
>  static struct attribute_group i2c_adapter_attr_group = {
>  	.attrs		= i2c_adapter_attrs,
> +	.is_visible	= i2c_adapter_visible_attr,
>  };
>  
>  static const struct attribute_group *i2c_adapter_attr_groups[] = {
> @@ -1262,6 +1348,14 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
>  	struct device *dev = &adapter->dev;
>  	int id;
>  
> +	if (adapter->set_freq) {
> +		if (!adapter->freq || !adapter->min_freq || !adapter->max_freq)
> +			return -EINVAL;
> +	} else {
> +		if (adapter->min_freq || adapter->max_freq)
> +			return -EINVAL;
> +	}
> +
>  	if (dev->of_node) {
>  		id = of_alias_get_id(dev->of_node, "i2c");
>  		if (id >= 0) {
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 36041e2..3482b09 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -431,6 +431,17 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap);
>   *	usb->interface->dev, platform_device->dev etc.)
>   * @name: name of this i2c bus
>   * @bus_recovery_info: see struct i2c_bus_recovery_info. Optional.
> + * @set_freq: set the bus frequency (in HZ) and returns the actual set
> + * 	value. Since not all the freqeuncies in the min - max interval
> + * 	may be valid the driver may round the frequency to the closest
> + * 	supported one. Optional but must be set if min_freq or
> + * 	max_freq is set.
> + * @min_freq: minimum bus frequency. Optional but must be set if
> + *	set_freq is set.
> + * @max_freq: maximum bus frequency. Optional but must be set if
> + *	set_freq is set.
> + * @freq: initial bus frequency. Optional but must be set if set_freq
> + *	is set.
>   */
>  struct i2c_adapter {
>  	struct module *owner;
> @@ -438,6 +449,11 @@ struct i2c_adapter {
>  	const struct i2c_algorithm *algo; /* the algorithm to access the bus */
>  	void *algo_data;
>  
> +	unsigned int freq;
> +	unsigned int min_freq;
> +	unsigned int max_freq;
> +	int (*set_freq)(struct i2c_adapter *a, unsigned int *freq);
> +
>  	/* data fields that are valid for all devices	*/
>  	struct rt_mutex bus_lock;
>  
> -- 
> 1.9.1
> 

^ permalink raw reply

* [RFC PATCH v2 3/3] i2c: show and change bus frequency via sysfs
From: Octavian Purdila @ 2014-10-14 14:48 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel,
	Octavian Purdila
In-Reply-To: <1413298094-9276-1-git-send-email-octavian.purdila@intel.com>

This patch adds three new sysfs files: bus_frequency,
bus_min_frequency and bus_max_frequency which allows the user to view
or change the bus frequency on a per bus level.

This is required for the case where multiple busses have the same
adapter driver and where a module parameter does not allow controlling
the bus speed individually (e.g. USB I2C adapters).

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-i2c | 30 +++++++++++
 drivers/i2c/i2c-core.c                  | 94 +++++++++++++++++++++++++++++++++
 include/linux/i2c.h                     | 16 ++++++
 3 files changed, 140 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c b/Documentation/ABI/testing/sysfs-bus-i2c
index 8075585..4a7f8e7 100644
--- a/Documentation/ABI/testing/sysfs-bus-i2c
+++ b/Documentation/ABI/testing/sysfs-bus-i2c
@@ -43,3 +43,33 @@ Contact:	linux-i2c@vger.kernel.org
 Description:
 		An i2c device attached to bus X that is enumerated via
 		ACPI. Y is the ACPI device name and its format is "%s".
+
+What:		/sys/bus/i2c/devices/i2c-X/bus_frequency
+KernelVersion:	3.19
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		The current bus frequency for bus X. Can be updated if
+		the bus supports it. The unit is HZ and format is
+		"%d\n".
+		If the bus does not support changing the frequency
+		then this entry will be read-only.
+		If the bus does not support showing the frequency than
+		this entry will not be visible.
+		When updating the bus frequency that value must be in
+		the range defined by bus_frequency_min and
+		bus_frequency_max otherwise writing to this entry will
+		fail with -EINVAL.
+		The bus may not support all of the frequencies in the
+		min-max range and may round the frequency to the
+		closest supported one. The user must read the entry
+		after writing it to retrieve the actual set frequency.
+
+What:		/sys/bus/i2c/devices/i2c-X/bus_min_frequency
+What:		/sys/bus/i2c/devices/i2c-X/bus_max_frequency
+KernelVersion:	3.19
+Contact:	linux-i2c@vger.kernel.org
+Description:
+		Minimum and maximum frequencies for bus X. The unit is
+		HZ and format is "%d\n".
+		If the bus does not support changing the frequency
+		these entries will not be visible.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 632057a..ab77f7f 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -940,15 +940,101 @@ static DEVICE_ATTR(new_device, S_IWUSR, NULL, i2c_sysfs_new_device);
 static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, S_IWUSR, NULL,
 				   i2c_sysfs_delete_device);
 
+static ssize_t
+i2c_sysfs_freq_show(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", adap->freq);
+}
+
+static ssize_t
+i2c_sysfs_freq_store(struct device *dev, struct device_attribute *attr,
+		     const char *buf, size_t count)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+	unsigned int freq;
+	int ret;
+
+	if (kstrtouint(buf, 0, &freq) || freq < adap->min_freq ||
+	    freq > adap->max_freq)
+		return -EINVAL;
+
+	i2c_lock_adapter(adap);
+	ret = adap->set_freq(adap, &freq);
+	i2c_unlock_adapter(adap);
+
+	if (ret)
+		return ret;
+
+	adap->freq = freq;
+
+	return count;
+}
+
+static DEVICE_ATTR(bus_frequency, S_IRUGO, i2c_sysfs_freq_show,
+		   i2c_sysfs_freq_store);
+
+
+static ssize_t
+i2c_sysfs_min_freq_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", adap->min_freq);
+}
+
+static DEVICE_ATTR(bus_min_frequency, S_IRUGO, i2c_sysfs_min_freq_show, NULL);
+
+static ssize_t
+i2c_sysfs_max_freq_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", adap->max_freq);
+}
+
+static DEVICE_ATTR(bus_max_frequency, S_IRUGO, i2c_sysfs_max_freq_show, NULL);
+
+umode_t i2c_adapter_visible_attr(struct kobject *kobj,
+				 struct attribute *attr, int idx)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct i2c_adapter *adap = to_i2c_adapter(dev);
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_bus_min_frequency.attr)
+		return adap->min_freq ? mode : 0;
+
+	if (attr == &dev_attr_bus_max_frequency.attr)
+		return adap->max_freq ? mode : 0;
+
+	if (attr == &dev_attr_bus_frequency.attr) {
+		if (adap->set_freq)
+			mode |= S_IWUSR;
+		return adap->freq ? mode : 0;
+	}
+
+	return mode;
+}
+
+
 static struct attribute *i2c_adapter_attrs[] = {
 	&dev_attr_name.attr,
 	&dev_attr_new_device.attr,
 	&dev_attr_delete_device.attr,
+	&dev_attr_bus_frequency.attr,
+	&dev_attr_bus_min_frequency.attr,
+	&dev_attr_bus_max_frequency.attr,
 	NULL
 };
 
 static struct attribute_group i2c_adapter_attr_group = {
 	.attrs		= i2c_adapter_attrs,
+	.is_visible	= i2c_adapter_visible_attr,
 };
 
 static const struct attribute_group *i2c_adapter_attr_groups[] = {
@@ -1262,6 +1348,14 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 	struct device *dev = &adapter->dev;
 	int id;
 
+	if (adapter->set_freq) {
+		if (!adapter->freq || !adapter->min_freq || !adapter->max_freq)
+			return -EINVAL;
+	} else {
+		if (adapter->min_freq || adapter->max_freq)
+			return -EINVAL;
+	}
+
 	if (dev->of_node) {
 		id = of_alias_get_id(dev->of_node, "i2c");
 		if (id >= 0) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 36041e2..3482b09 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -431,6 +431,17 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap);
  *	usb->interface->dev, platform_device->dev etc.)
  * @name: name of this i2c bus
  * @bus_recovery_info: see struct i2c_bus_recovery_info. Optional.
+ * @set_freq: set the bus frequency (in HZ) and returns the actual set
+ * 	value. Since not all the freqeuncies in the min - max interval
+ * 	may be valid the driver may round the frequency to the closest
+ * 	supported one. Optional but must be set if min_freq or
+ * 	max_freq is set.
+ * @min_freq: minimum bus frequency. Optional but must be set if
+ *	set_freq is set.
+ * @max_freq: maximum bus frequency. Optional but must be set if
+ *	set_freq is set.
+ * @freq: initial bus frequency. Optional but must be set if set_freq
+ *	is set.
  */
 struct i2c_adapter {
 	struct module *owner;
@@ -438,6 +449,11 @@ struct i2c_adapter {
 	const struct i2c_algorithm *algo; /* the algorithm to access the bus */
 	void *algo_data;
 
+	unsigned int freq;
+	unsigned int min_freq;
+	unsigned int max_freq;
+	int (*set_freq)(struct i2c_adapter *a, unsigned int *freq);
+
 	/* data fields that are valid for all devices	*/
 	struct rt_mutex bus_lock;
 
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 2/3] i2c: document struct i2c_adapter
From: Octavian Purdila @ 2014-10-14 14:48 UTC (permalink / raw)
  To: wsa; +Cc: linux, johan, linux-i2c, linux-api, linux-kernel,
	Octavian Purdila
In-Reply-To: <1413298094-9276-1-git-send-email-octavian.purdila@intel.com>

Document the i2c_adapter fields that must be initialized before
calling i2c_add_adapter().

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 include/linux/i2c.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a95efeb..36041e2 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -418,9 +418,19 @@ int i2c_recover_bus(struct i2c_adapter *adap);
 int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
 int i2c_generic_scl_recovery(struct i2c_adapter *adap);
 
-/*
- * i2c_adapter is the structure used to identify a physical i2c bus along
- * with the access algorithms necessary to access it.
+/**
+ * struct i2c_adapter - represents an I2C physical bus
+ *
+ * The following members must be set by the adapter driver before
+ * calling i2c_add_adapter():
+ *
+ * @owner: the module owner, usually THIS_MODULE
+ * @class: bitmask of I2C_CLASS_*
+ * @algo: see struct i2c_algorithm
+ * @dev.parent: set this to the struct device of the driver (e.g. pci_dev->dev,
+ *	usb->interface->dev, platform_device->dev etc.)
+ * @name: name of this i2c bus
+ * @bus_recovery_info: see struct i2c_bus_recovery_info. Optional.
  */
 struct i2c_adapter {
 	struct module *owner;
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 1/3] i2c: document the existing i2c sysfs ABI
From: Octavian Purdila @ 2014-10-14 14:48 UTC (permalink / raw)
  To: wsa-z923LK4zBo2bacvFa/9K2g
  Cc: linux-0h96xk9xTtrk1uMJSBkQmQ, johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Octavian Purdila
In-Reply-To: <1413298094-9276-1-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This patch adds Documentation/ABI/testing/sysfs-bus-i2c which
documents the existing i2c sysfs ABI.

Signed-off-by: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-bus-i2c | 45 +++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c b/Documentation/ABI/testing/sysfs-bus-i2c
new file mode 100644
index 0000000..8075585
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c
@@ -0,0 +1,45 @@
+What:		/sys/bus/i2c/devices/i2c-X
+KernelVersion:	since at least 2.6.12
+Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+		This entry represents a registered i2c bus. X is the
+		bus number and its format is "%d".
+
+What:		/sys/bus/i2c/devices/i2c-X/Y
+What:		/sys/bus/i2c/devices/Y
+KernelVersion:	since at least 2.6.12
+Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+		An i2c device attached to bus X. Format of Y is
+		"%d-%04x" where the first number is the bus number (X)
+		and the second number is the device i2c address.
+
+What:		/sys/bus/i2c/devices/i2c-X/new_device
+KernelVersion:	2.6.31
+Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+		Write only entry that allows instantiating a
+		new i2c device on bus X. This is to be used when
+		enumeration mechanism such as ACPI or DT are not
+		present or not used for this device.
+		Format: "%s %hi\n" where the first argument is the
+		device name (no spaces allowed) and the second is the
+		i2c address of the device.
+
+What:		/sys/bus/i2c/devices/i2c-X/delete_device
+KernelVersion:	2.6.31
+Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+		Write only entry that allows the removal of an i2c
+		device from bus X.
+		Format: "%s %hi\n" where the first argument is the
+		device name (no spaces allowed) and the second is the
+		i2c address of the device.
+
+What:		/sys/bus/i2c/devices/i2c-X/i2c-Y
+What:		/sys/bus/i2c/devices/i2c-Y
+KernelVersion:	3.13
+Contact:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+		An i2c device attached to bus X that is enumerated via
+		ACPI. Y is the ACPI device name and its format is "%s".
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 0/3] i2c: show and change bus frequency via sysfs
From: Octavian Purdila @ 2014-10-14 14:48 UTC (permalink / raw)
  To: wsa-z923LK4zBo2bacvFa/9K2g
  Cc: linux-0h96xk9xTtrk1uMJSBkQmQ, johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Octavian Purdila

This second version gets rid of the frequency table and introduces a
min and max range. It also changes the ABI to allow the driver to
round the given frequency to one that is supported, since many drivers
seems to work this way (i2c-stu300, i2c-s3c2410, i2c-kempld).

Octavian Purdila (3):
  i2c: document the existing i2c sysfs ABI
  i2c: document struct i2c_adapter
  i2c: show and change bus frequency via sysfs

 Documentation/ABI/testing/sysfs-bus-i2c | 75 ++++++++++++++++++++++++++
 drivers/i2c/i2c-core.c                  | 94 +++++++++++++++++++++++++++++++++
 include/linux/i2c.h                     | 32 +++++++++--
 3 files changed, 198 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c

-- 
1.9.1

^ 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