* [PATCH v11 0/4] Multi-instance vTPM proxy driver
@ 2016-04-18 17:26 Stefan Berger
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
` (4 more replies)
0 siblings, 5 replies; 25+ messages in thread
From: Stefan Berger @ 2016-04-18 17:26 UTC (permalink / raw)
To: tpmdd-devel
Cc: jarkko.sakkinen, jgunthorpe, linux-security-module, linux-kernel,
Stefan Berger
The following series of patches implements a multi-instance vTPM
proxy driver that can dynamically create TPM 'server' and client device
pairs.
Using an ioctl on the provided /dev/vtpmx, a client-side vTPM device
and a server side file descriptor is created. The file descriptor must
be passed to a TPM emulator. The device driver will initialize the
emulated TPM using TPM 1.2 or TPM 2 startup commands and it will read
the command durations from the device in case of a TPM 1.2. The choice
of emulated TPM device (1.2 or 2) must be provided with a flag in
the ioctl.
The patches are based on the tip of a recent checkout of Jarkko's tree
(master branch).
Stefan
v10->v11:
- patch 3: merge of v10 patches 3 and 4
- patch 3: removed unnecessary NULL pointer checks
- Added Jason's Reviewed-by's
v9->v10:
- add new patch 1 with sysfs related rework; reworked patch 2 as a consequence
- patch 3: fixed ioctl from using _IOW to using _IOWR (reinstall headers
and rebuild test tools)
- patch 3: do not use priv field anymore but uses dev_set_drvdata /
dev_get_drvdata instead now
v8->v9:
- move constant from public header into tpm_vtpm_proxy.c
- Replaced VTPM_PROXY_MAGIC in ioctl definition with its value (0xa1)
- Check for the STATE_OPEN_FLAG in wait_event_interruptable and after
returning from it
v7->v8:
- minor tweaks on the documentation
- Reordered function calls in the VTPM proxy driver's server side release
function so that a client holding the 'ops' lock releases it before the
driver tries to grab the lock when unregistering the device.
v6->v7:
- Adjusted name of driver to tpm_vtpm_proxy from tpm_vtpm. Adjust function
names, names of structures, and names of constants.
- Adjusted IOCTL to use magic 0xa1 rather than the completely used 0xa0.
- Extended driver documentation and added documentation of ioctl.
- Moved test program to own project (dropped patch 11).
v5->v6:
- Adapted errno's for unsupported flags and ioctls following Jason's
comments
v4->v5:
- Introduced different error codes for unsupported flags and ioctls
- Added documentation patch
Jason Gunthorpe (1):
tpm: Remove all uses of drvdata from the TPM Core
Stefan Berger (3):
tpm: Introduce TPM_CHIP_FLAG_VIRTUAL
tpm: Proxy driver for supporting multiple emulated TPMs
tpm: Add documentation for the tpm_vtpm_proxy device driver
Documentation/ioctl/ioctl-number.txt | 1 +
Documentation/tpm/tpm_vtpm_proxy.txt | 71 ++++
drivers/char/tpm/Kconfig | 10 +
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm-chip.c | 76 +++--
drivers/char/tpm/tpm-interface.c | 7 +-
drivers/char/tpm/tpm-sysfs.c | 61 ++--
drivers/char/tpm/tpm.h | 11 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 644 +++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/vtpm_proxy.h | 36 ++
11 files changed, 852 insertions(+), 67 deletions(-)
create mode 100644 Documentation/tpm/tpm_vtpm_proxy.txt
create mode 100644 drivers/char/tpm/tpm_vtpm_proxy.c
create mode 100644 include/uapi/linux/vtpm_proxy.h
--
2.4.3
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-18 17:26 [PATCH v11 0/4] Multi-instance vTPM proxy driver Stefan Berger
@ 2016-04-18 17:26 ` Stefan Berger
2016-04-19 10:12 ` Jarkko Sakkinen
` (2 more replies)
2016-04-18 17:26 ` [PATCH v11 2/4] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL Stefan Berger
` (3 subsequent siblings)
4 siblings, 3 replies; 25+ messages in thread
From: Stefan Berger @ 2016-04-18 17:26 UTC (permalink / raw)
To: tpmdd-devel
Cc: jarkko.sakkinen, jgunthorpe, linux-security-module, linux-kernel,
Stefan Berger
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
The final thing preventing this was the way the sysfs files were
attached to the pdev. Follow the approach developed for ppi and move
the sysfs files to the chip->dev with symlinks from the pdev
for compatibility. Everything in the core now sanely uses container_of
to get the chip.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
drivers/char/tpm/tpm-interface.c | 7 ++--
drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
drivers/char/tpm/tpm.h | 10 +++---
4 files changed, 84 insertions(+), 67 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 5bc530c..7e2c9cf 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
chip->dev.class = tpm_class;
chip->dev.release = tpm_dev_release;
chip->dev.parent = dev;
-#ifdef CONFIG_ACPI
chip->dev.groups = chip->groups;
-#endif
if (chip->dev_num == 0)
chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
@@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
static int tpm1_chip_register(struct tpm_chip *chip)
{
- int rc;
-
if (chip->flags & TPM_CHIP_FLAG_TPM2)
return 0;
- rc = tpm_sysfs_add_device(chip);
- if (rc)
- return rc;
+ tpm_sysfs_add_device(chip);
chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
@@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
if (chip->bios_dir)
tpm_bios_log_teardown(chip->bios_dir);
+}
+
+static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
+{
+ struct attribute **i;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ return;
- tpm_sysfs_del_device(chip);
+ sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
+
+ for (i = chip->groups[0]->attrs; *i != NULL; ++i)
+ sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
}
+/* For compatibility with legacy sysfs paths we provide symlinks from the
+ * parent dev directory to selected names within the tpm chip directory. Old
+ * kernel versions created these files directly under the parent.
+ */
+static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
+{
+ struct attribute **i;
+ int rc;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ return 0;
+
+ rc = __compat_only_sysfs_link_entry_to_kobj(
+ &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
+ if (rc && rc != -ENOENT)
+ return rc;
+
+ /* All the names from tpm-sysfs */
+ for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
+ rc = __compat_only_sysfs_link_entry_to_kobj(
+ &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
+ if (rc) {
+ tpm_del_legacy_sysfs(chip);
+ return rc;
+ }
+ }
+
+ return 0;
+}
/*
* tpm_chip_register() - create a character device for the TPM chip
* @chip: TPM chip to use.
@@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
tpm_add_ppi(chip);
rc = tpm_add_char_device(chip);
- if (rc)
- goto out_err;
+ if (rc) {
+ tpm1_chip_unregister(chip);
+ return rc;
+ }
chip->flags |= TPM_CHIP_FLAG_REGISTERED;
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
- rc = __compat_only_sysfs_link_entry_to_kobj(
- &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
- if (rc && rc != -ENOENT) {
- tpm_chip_unregister(chip);
- return rc;
- }
+ rc = tpm_add_legacy_sysfs(chip);
+ if (rc) {
+ tpm_chip_unregister(chip);
+ return rc;
}
return 0;
-out_err:
- tpm1_chip_unregister(chip);
- return rc;
}
EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
return;
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
- sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
+ tpm_del_legacy_sysfs(chip);
tpm1_chip_unregister(chip);
tpm_del_char_device(chip);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 7cba092..080dade 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
.ordinal = TPM_ORD_GET_CAP
};
-ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
+ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
const char *desc)
{
struct tpm_cmd_t tpm_cmd;
int rc;
- struct tpm_chip *chip = dev_get_drvdata(dev);
tpm_cmd.header.in = tpm_getcap_header;
if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
@@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
*/
int tpm_pm_suspend(struct device *dev)
{
- struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct tpm_chip *chip = to_tpm_chip(dev);
struct tpm_cmd_t cmd;
int rc, try;
@@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
*/
int tpm_pm_resume(struct device *dev)
{
- struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct tpm_chip *chip = to_tpm_chip(dev);
if (chip == NULL)
return -ENODEV;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index a7c3473..b46cf70 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
int i, rc;
char *str = buf;
- struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct tpm_chip *chip = to_tpm_chip(dev);
tpm_cmd.header.in = tpm_readpubek_header;
err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
@@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
ssize_t rc;
int i, j, num_pcrs;
char *str = buf;
- struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct tpm_chip *chip = to_tpm_chip(dev);
- rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
+ rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
"attempting to determine the number of PCRS");
if (rc)
return 0;
@@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
- "attempting to determine the permanent enabled state");
+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
+ "attempting to determine the permanent enabled state");
if (rc)
return 0;
@@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
- "attempting to determine the permanent active state");
+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
+ "attempting to determine the permanent active state");
if (rc)
return 0;
@@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
- "attempting to determine the owner state");
+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
+ "attempting to determine the owner state");
if (rc)
return 0;
@@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
cap_t cap;
ssize_t rc;
- rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
- "attempting to determine the temporary state");
+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
+ "attempting to determine the temporary state");
if (rc)
return 0;
@@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct tpm_chip *chip = to_tpm_chip(dev);
cap_t cap;
ssize_t rc;
char *str = buf;
- rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
+ rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
"attempting to determine the manufacturer");
if (rc)
return 0;
@@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
be32_to_cpu(cap.manufacturer_id));
/* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
- rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
- "attempting to determine the 1.2 version");
+ rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
+ "attempting to determine the 1.2 version");
if (!rc) {
str += sprintf(str,
"TCG version: %d.%d\nFirmware version: %d.%d\n",
@@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
cap.tpm_version_1_2.revMinor);
} else {
/* Otherwise just use TPM_STRUCT_VER */
- rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
+ rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
"attempting to determine the 1.1 version");
if (rc)
return 0;
@@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
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);
+ struct tpm_chip *chip = to_tpm_chip(dev);
if (chip == NULL)
return 0;
@@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct tpm_chip *chip = to_tpm_chip(dev);
if (chip->duration[TPM_LONG] == 0)
return 0;
@@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct tpm_chip *chip = to_tpm_chip(dev);
return sprintf(buf, "%d %d %d %d [%s]\n",
jiffies_to_usecs(chip->timeout_a),
@@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
.attrs = tpm_dev_attrs,
};
-int tpm_sysfs_add_device(struct tpm_chip *chip)
+void tpm_sysfs_add_device(struct tpm_chip *chip)
{
- int err;
- err = sysfs_create_group(&chip->dev.parent->kobj,
- &tpm_dev_group);
-
- if (err)
- dev_err(&chip->dev,
- "failed to create sysfs attributes, %d\n", err);
- return err;
-}
-
-void tpm_sysfs_del_device(struct tpm_chip *chip)
-{
- /* The sysfs routines rely on an implicit tpm_try_get_ops, this
- * function is called before ops is null'd and the sysfs core
- * synchronizes this removal so that no callbacks are running or can
- * run again
+ /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
+ * is called before ops is null'd and the sysfs core synchronizes this
+ * removal so that no callbacks are running or can run again
*/
- sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
+ WARN_ON(chip->groups_cnt != 0);
+ chip->groups[chip->groups_cnt++] = &tpm_dev_group;
}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8bc6fb8..508e8e0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -168,9 +168,9 @@ struct tpm_chip {
struct dentry **bios_dir;
-#ifdef CONFIG_ACPI
- const struct attribute_group *groups[2];
+ const struct attribute_group *groups[3];
unsigned int groups_cnt;
+#ifdef CONFIG_ACPI
acpi_handle acpi_dev_handle;
char ppi_version[TPM_PPI_VERSION_LEN + 1];
#endif /* CONFIG_ACPI */
@@ -471,7 +471,8 @@ extern dev_t tpm_devt;
extern const struct file_operations tpm_fops;
extern struct idr dev_nums_idr;
-ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
+ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
+ const char *desc);
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,
@@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
extern int tpm_chip_register(struct tpm_chip *chip);
extern void tpm_chip_unregister(struct tpm_chip *chip);
-int tpm_sysfs_add_device(struct tpm_chip *chip);
-void tpm_sysfs_del_device(struct tpm_chip *chip);
+void tpm_sysfs_add_device(struct tpm_chip *chip);
int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
--
2.4.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 2/4] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL
2016-04-18 17:26 [PATCH v11 0/4] Multi-instance vTPM proxy driver Stefan Berger
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
@ 2016-04-18 17:26 ` Stefan Berger
2016-04-22 16:46 ` Jarkko Sakkinen
2016-04-18 17:26 ` [PATCH v11 3/4] tpm: Proxy driver for supporting multiple emulated TPMs Stefan Berger
` (2 subsequent siblings)
4 siblings, 1 reply; 25+ messages in thread
From: Stefan Berger @ 2016-04-18 17:26 UTC (permalink / raw)
To: tpmdd-devel
Cc: jarkko.sakkinen, jgunthorpe, linux-security-module, linux-kernel,
Stefan Berger
Introduce TPM_CHIP_FLAG_VIRTUAL to be used when the chip device has no
parent device.
Prevent sysfs entries requiring a parent device from being created.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/char/tpm/tpm-chip.c | 7 +++++--
drivers/char/tpm/tpm.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 7e2c9cf..a12db8e 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -181,6 +181,9 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
if (rc)
goto out;
+ if (!dev)
+ chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
+
cdev_init(&chip->cdev, &tpm_fops);
chip->cdev.owner = THIS_MODULE;
chip->cdev.kobj.parent = &chip->dev.kobj;
@@ -297,7 +300,7 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
{
struct attribute **i;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
return;
sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
@@ -315,7 +318,7 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
struct attribute **i;
int rc;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
return 0;
rc = __compat_only_sysfs_link_entry_to_kobj(
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 508e8e0..ff4e53c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -138,6 +138,7 @@ enum tpm_chip_flags {
TPM_CHIP_FLAG_REGISTERED = BIT(0),
TPM_CHIP_FLAG_TPM2 = BIT(1),
TPM_CHIP_FLAG_IRQ = BIT(2),
+ TPM_CHIP_FLAG_VIRTUAL = BIT(3),
};
struct tpm_chip {
--
2.4.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 3/4] tpm: Proxy driver for supporting multiple emulated TPMs
2016-04-18 17:26 [PATCH v11 0/4] Multi-instance vTPM proxy driver Stefan Berger
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
2016-04-18 17:26 ` [PATCH v11 2/4] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL Stefan Berger
@ 2016-04-18 17:26 ` Stefan Berger
2016-04-22 16:47 ` Jarkko Sakkinen
2016-04-18 17:26 ` [PATCH v11 4/4] tpm: Add documentation for the tpm_vtpm_proxy device driver Stefan Berger
2016-04-22 16:54 ` [PATCH v11 0/4] Multi-instance vTPM proxy driver Jarkko Sakkinen
4 siblings, 1 reply; 25+ messages in thread
From: Stefan Berger @ 2016-04-18 17:26 UTC (permalink / raw)
To: tpmdd-devel
Cc: jarkko.sakkinen, jgunthorpe, linux-security-module, linux-kernel,
Stefan Berger, linux-doc, linux-api
This patch implements a proxy driver for supporting multiple emulated TPMs
in a system.
The driver implements a device /dev/vtpmx that is used to created
a client device pair /dev/tpmX (e.g., /dev/tpm10) and a server side that
is accessed using a file descriptor returned by an ioctl.
The device /dev/tpmX is the usual TPM device created by the core TPM
driver. Applications or kernel subsystems can send TPM commands to it
and the corresponding server-side file descriptor receives these
commands and delivers them to an emulated TPM.
The driver retrievs the TPM 1.2 durations and timeouts. Since this requires
the startup of the TPM, we send a startup for TPM 1.2 as well as TPM 2.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
CC: linux-kernel@vger.kernel.org
CC: linux-doc@vger.kernel.org
CC: linux-api@vger.kernel.org
---
drivers/char/tpm/Kconfig | 10 +
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm_vtpm_proxy.c | 644 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/vtpm_proxy.h | 36 +++
5 files changed, 692 insertions(+)
create mode 100644 drivers/char/tpm/tpm_vtpm_proxy.c
create mode 100644 include/uapi/linux/vtpm_proxy.h
diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 3b84a8b..0eac596 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -122,5 +122,15 @@ config TCG_CRB
from within Linux. To compile this driver as a module, choose
M here; the module will be called tpm_crb.
+config TCG_VTPM_PROXY
+ tristate "VTPM Proxy Interface"
+ depends on TCG_TPM
+ ---help---
+ This driver proxies for an emulated TPM (vTPM) running in userspace.
+ A device /dev/vtpmx is provided that creates a device pair
+ /dev/vtpmX and a server-side file descriptor on which the vTPM
+ can receive commands.
+
+
source "drivers/char/tpm/st33zp24/Kconfig"
endif # TCG_TPM
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 56e8f1f..98de5e6 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
obj-$(CONFIG_TCG_CRB) += tpm_crb.o
+obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
new file mode 100644
index 0000000..4384042
--- /dev/null
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -0,0 +1,644 @@
+/*
+ * Copyright (C) 2015, 2016 IBM Corporation
+ *
+ * Author: Stefan Berger <stefanb@us.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * Device driver for vTPM (vTPM proxy driver)
+ *
+ * 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/types.h>
+#include <linux/spinlock.h>
+#include <linux/uaccess.h>
+#include <linux/wait.h>
+#include <linux/miscdevice.h>
+#include <linux/vtpm_proxy.h>
+#include <linux/file.h>
+#include <linux/anon_inodes.h>
+#include <linux/poll.h>
+#include <linux/compat.h>
+
+#include "tpm.h"
+
+#define VTPM_PROXY_REQ_COMPLETE_FLAG BIT(0)
+
+struct proxy_dev {
+ struct tpm_chip *chip;
+
+ u32 flags; /* public API flags */
+
+ wait_queue_head_t wq;
+
+ struct mutex buf_lock; /* protect buffer and flags */
+
+ long state; /* internal state */
+#define STATE_OPENED_FLAG BIT(0)
+#define STATE_WAIT_RESPONSE_FLAG BIT(1) /* waiting for emulator response */
+
+ size_t req_len; /* length of queued TPM request */
+ size_t resp_len; /* length of queued TPM response */
+ u8 buffer[TPM_BUFSIZE]; /* request/response buffer */
+
+ struct work_struct work; /* task that retrieves TPM timeouts */
+};
+
+/* all supported flags */
+#define VTPM_PROXY_FLAGS_ALL (VTPM_PROXY_FLAG_TPM2)
+
+static struct workqueue_struct *workqueue;
+
+static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev);
+
+/*
+ * Functions related to 'server side'
+ */
+
+/**
+ * vtpm_proxy_fops_read - Read TPM commands on 'server side'
+ *
+ * Return value:
+ * Number of bytes read or negative error code
+ */
+static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
+ size_t count, loff_t *off)
+{
+ struct proxy_dev *proxy_dev = filp->private_data;
+ size_t len;
+ int sig, rc;
+
+ sig = wait_event_interruptible(proxy_dev->wq,
+ proxy_dev->req_len != 0 ||
+ !(proxy_dev->state & STATE_OPENED_FLAG));
+ if (sig)
+ return -EINTR;
+
+ mutex_lock(&proxy_dev->buf_lock);
+
+ if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ return -EPIPE;
+ }
+
+ len = proxy_dev->req_len;
+
+ if (count < len) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n",
+ count, len);
+ return -EIO;
+ }
+
+ rc = copy_to_user(buf, proxy_dev->buffer, len);
+ memset(proxy_dev->buffer, 0, len);
+ proxy_dev->req_len = 0;
+
+ if (!rc)
+ proxy_dev->state |= STATE_WAIT_RESPONSE_FLAG;
+
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ if (rc)
+ return -EFAULT;
+
+ return len;
+}
+
+/**
+ * vtpm_proxy_fops_write - Write TPM responses on 'server side'
+ *
+ * Return value:
+ * Number of bytes read or negative error value
+ */
+static ssize_t vtpm_proxy_fops_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *off)
+{
+ struct proxy_dev *proxy_dev = filp->private_data;
+
+ mutex_lock(&proxy_dev->buf_lock);
+
+ if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ return -EPIPE;
+ }
+
+ if (count > sizeof(proxy_dev->buffer) ||
+ !(proxy_dev->state & STATE_WAIT_RESPONSE_FLAG)) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ return -EIO;
+ }
+
+ proxy_dev->state &= ~STATE_WAIT_RESPONSE_FLAG;
+
+ proxy_dev->req_len = 0;
+
+ if (copy_from_user(proxy_dev->buffer, buf, count)) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ return -EFAULT;
+ }
+
+ proxy_dev->resp_len = count;
+
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ wake_up_interruptible(&proxy_dev->wq);
+
+ return count;
+}
+
+/*
+ * vtpm_proxy_fops_poll: Poll status on 'server side'
+ *
+ * Return value:
+ * Poll flags
+ */
+static unsigned int vtpm_proxy_fops_poll(struct file *filp, poll_table *wait)
+{
+ struct proxy_dev *proxy_dev = filp->private_data;
+ unsigned ret;
+
+ poll_wait(filp, &proxy_dev->wq, wait);
+
+ ret = POLLOUT;
+
+ mutex_lock(&proxy_dev->buf_lock);
+
+ if (proxy_dev->req_len)
+ ret |= POLLIN | POLLRDNORM;
+
+ if (!(proxy_dev->state & STATE_OPENED_FLAG))
+ ret |= POLLHUP;
+
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ return ret;
+}
+
+/*
+ * vtpm_proxy_fops_open - Open vTPM device on 'server side'
+ *
+ * Called when setting up the anonymous file descriptor
+ */
+static void vtpm_proxy_fops_open(struct file *filp)
+{
+ struct proxy_dev *proxy_dev = filp->private_data;
+
+ proxy_dev->state |= STATE_OPENED_FLAG;
+}
+
+/**
+ * vtpm_proxy_fops_undo_open - counter-part to vtpm_fops_open
+ *
+ * Call to undo vtpm_proxy_fops_open
+ */
+static void vtpm_proxy_fops_undo_open(struct proxy_dev *proxy_dev)
+{
+ mutex_lock(&proxy_dev->buf_lock);
+
+ proxy_dev->state &= ~STATE_OPENED_FLAG;
+
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ /* no more TPM responses -- wake up anyone waiting for them */
+ wake_up_interruptible(&proxy_dev->wq);
+}
+
+/*
+ * vtpm_proxy_fops_release: Close 'server side'
+ *
+ * Return value:
+ * Always returns 0.
+ */
+static int vtpm_proxy_fops_release(struct inode *inode, struct file *filp)
+{
+ struct proxy_dev *proxy_dev = filp->private_data;
+
+ filp->private_data = NULL;
+
+ vtpm_proxy_delete_device(proxy_dev);
+
+ return 0;
+}
+
+static const struct file_operations vtpm_proxy_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .read = vtpm_proxy_fops_read,
+ .write = vtpm_proxy_fops_write,
+ .poll = vtpm_proxy_fops_poll,
+ .release = vtpm_proxy_fops_release,
+};
+
+/*
+ * Functions invoked by the core TPM driver to send TPM commands to
+ * 'server side' and receive responses from there.
+ */
+
+/*
+ * Called when core TPM driver reads TPM responses from 'server side'
+ *
+ * Return value:
+ * Number of TPM response bytes read, negative error value otherwise
+ */
+static int vtpm_proxy_tpm_op_recv(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+ struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+ size_t len;
+
+ /* process gone ? */
+ mutex_lock(&proxy_dev->buf_lock);
+
+ if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ return -EPIPE;
+ }
+
+ len = proxy_dev->resp_len;
+ if (count < len) {
+ dev_err(&chip->dev,
+ "Invalid size in recv: count=%zd, resp_len=%zd\n",
+ count, len);
+ len = -EIO;
+ goto out;
+ }
+
+ memcpy(buf, proxy_dev->buffer, len);
+ proxy_dev->resp_len = 0;
+
+out:
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ return len;
+}
+
+/*
+ * Called when core TPM driver forwards TPM requests to 'server side'.
+ *
+ * Return value:
+ * 0 in case of success, negative error value otherwise.
+ */
+static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+ struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+ int rc = 0;
+
+ if (count > sizeof(proxy_dev->buffer)) {
+ dev_err(&chip->dev,
+ "Invalid size in send: count=%zd, buffer size=%zd\n",
+ count, sizeof(proxy_dev->buffer));
+ return -EIO;
+ }
+
+ mutex_lock(&proxy_dev->buf_lock);
+
+ if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
+ mutex_unlock(&proxy_dev->buf_lock);
+ return -EPIPE;
+ }
+
+ proxy_dev->resp_len = 0;
+
+ proxy_dev->req_len = count;
+ memcpy(proxy_dev->buffer, buf, count);
+
+ proxy_dev->state &= ~STATE_WAIT_RESPONSE_FLAG;
+
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ wake_up_interruptible(&proxy_dev->wq);
+
+ return rc;
+}
+
+static void vtpm_proxy_tpm_op_cancel(struct tpm_chip *chip)
+{
+ /* not supported */
+}
+
+static u8 vtpm_proxy_tpm_op_status(struct tpm_chip *chip)
+{
+ struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+
+ if (proxy_dev->resp_len)
+ return VTPM_PROXY_REQ_COMPLETE_FLAG;
+
+ return 0;
+}
+
+static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip *chip, u8 status)
+{
+ struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+ bool ret;
+
+ mutex_lock(&proxy_dev->buf_lock);
+
+ ret = !(proxy_dev->state & STATE_OPENED_FLAG);
+
+ mutex_unlock(&proxy_dev->buf_lock);
+
+ return ret;
+}
+
+static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
+ .recv = vtpm_proxy_tpm_op_recv,
+ .send = vtpm_proxy_tpm_op_send,
+ .cancel = vtpm_proxy_tpm_op_cancel,
+ .status = vtpm_proxy_tpm_op_status,
+ .req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
+ .req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
+ .req_canceled = vtpm_proxy_tpm_req_canceled,
+};
+
+/*
+ * Code related to the startup of the TPM 2 and startup of TPM 1.2 +
+ * retrieval of timeouts and durations.
+ */
+
+static void vtpm_proxy_work(struct work_struct *work)
+{
+ struct proxy_dev *proxy_dev = container_of(work, struct proxy_dev,
+ work);
+ int rc;
+
+ if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
+ rc = tpm2_startup(proxy_dev->chip, TPM2_SU_CLEAR);
+ else
+ rc = tpm_get_timeouts(proxy_dev->chip);
+
+ if (rc)
+ goto err;
+
+ rc = tpm_chip_register(proxy_dev->chip);
+ if (rc)
+ goto err;
+
+ return;
+
+err:
+ vtpm_proxy_fops_undo_open(proxy_dev);
+}
+
+/*
+ * vtpm_proxy_work_stop: make sure the work has finished
+ *
+ * This function is useful when user space closed the fd
+ * while the driver still determines timeouts.
+ */
+static void vtpm_proxy_work_stop(struct proxy_dev *proxy_dev)
+{
+ vtpm_proxy_fops_undo_open(proxy_dev);
+ flush_work(&proxy_dev->work);
+}
+
+/*
+ * vtpm_proxy_work_start: Schedule the work for TPM 1.2 & 2 initialization
+ */
+static inline void vtpm_proxy_work_start(struct proxy_dev *proxy_dev)
+{
+ queue_work(workqueue, &proxy_dev->work);
+}
+
+/*
+ * Code related to creation and deletion of device pairs
+ */
+static struct proxy_dev *vtpm_proxy_create_proxy_dev(void)
+{
+ struct proxy_dev *proxy_dev;
+ struct tpm_chip *chip;
+ int err;
+
+ proxy_dev = kzalloc(sizeof(*proxy_dev), GFP_KERNEL);
+ if (proxy_dev == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ init_waitqueue_head(&proxy_dev->wq);
+ mutex_init(&proxy_dev->buf_lock);
+ INIT_WORK(&proxy_dev->work, vtpm_proxy_work);
+
+ chip = tpm_chip_alloc(NULL, &vtpm_proxy_tpm_ops);
+ if (IS_ERR(chip)) {
+ err = PTR_ERR(chip);
+ goto err_proxy_dev_free;
+ }
+ dev_set_drvdata(&chip->dev, proxy_dev);
+
+ proxy_dev->chip = chip;
+
+ return proxy_dev;
+
+err_proxy_dev_free:
+ kfree(proxy_dev);
+
+ return ERR_PTR(err);
+}
+
+/*
+ * Undo what has been done in vtpm_create_proxy_dev
+ */
+static inline void vtpm_proxy_delete_proxy_dev(struct proxy_dev *proxy_dev)
+{
+ put_device(&proxy_dev->chip->dev); /* frees chip */
+ kfree(proxy_dev);
+}
+
+/*
+ * Create a /dev/tpm%d and 'server side' file descriptor pair
+ *
+ * Return value:
+ * Returns file pointer on success, an error value otherwise
+ */
+static struct file *vtpm_proxy_create_device(
+ struct vtpm_proxy_new_dev *vtpm_new_dev)
+{
+ struct proxy_dev *proxy_dev;
+ int rc, fd;
+ struct file *file;
+
+ if (vtpm_new_dev->flags & ~VTPM_PROXY_FLAGS_ALL)
+ return ERR_PTR(-EOPNOTSUPP);
+
+ proxy_dev = vtpm_proxy_create_proxy_dev();
+ if (IS_ERR(proxy_dev))
+ return ERR_CAST(proxy_dev);
+
+ proxy_dev->flags = vtpm_new_dev->flags;
+
+ /* setup an anonymous file for the server-side */
+ fd = get_unused_fd_flags(O_RDWR);
+ if (fd < 0) {
+ rc = fd;
+ goto err_delete_proxy_dev;
+ }
+
+ file = anon_inode_getfile("[vtpms]", &vtpm_proxy_fops, proxy_dev,
+ O_RDWR);
+ if (IS_ERR(file)) {
+ rc = PTR_ERR(file);
+ goto err_put_unused_fd;
+ }
+
+ /* from now on we can unwind with put_unused_fd() + fput() */
+ /* simulate an open() on the server side */
+ vtpm_proxy_fops_open(file);
+
+ if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
+ proxy_dev->chip->flags |= TPM_CHIP_FLAG_TPM2;
+
+ vtpm_proxy_work_start(proxy_dev);
+
+ vtpm_new_dev->fd = fd;
+ vtpm_new_dev->major = MAJOR(proxy_dev->chip->dev.devt);
+ vtpm_new_dev->minor = MINOR(proxy_dev->chip->dev.devt);
+ vtpm_new_dev->tpm_num = proxy_dev->chip->dev_num;
+
+ return file;
+
+err_put_unused_fd:
+ put_unused_fd(fd);
+
+err_delete_proxy_dev:
+ vtpm_proxy_delete_proxy_dev(proxy_dev);
+
+ return ERR_PTR(rc);
+}
+
+/*
+ * Counter part to vtpm_create_device.
+ */
+static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev)
+{
+ vtpm_proxy_work_stop(proxy_dev);
+
+ /*
+ * A client may hold the 'ops' lock, so let it know that the server
+ * side shuts down before we try to grab the 'ops' lock when
+ * unregistering the chip.
+ */
+ vtpm_proxy_fops_undo_open(proxy_dev);
+
+ tpm_chip_unregister(proxy_dev->chip);
+
+ vtpm_proxy_delete_proxy_dev(proxy_dev);
+}
+
+/*
+ * Code related to the control device /dev/vtpmx
+ */
+
+/*
+ * vtpmx_fops_ioctl: ioctl on /dev/vtpmx
+ *
+ * Return value:
+ * Returns 0 on success, a negative error code otherwise.
+ */
+static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
+ unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ struct vtpm_proxy_new_dev *vtpm_new_dev_p;
+ struct vtpm_proxy_new_dev vtpm_new_dev;
+ struct file *file;
+
+ switch (ioctl) {
+ case VTPM_PROXY_IOC_NEW_DEV:
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ vtpm_new_dev_p = argp;
+ if (copy_from_user(&vtpm_new_dev, vtpm_new_dev_p,
+ sizeof(vtpm_new_dev)))
+ return -EFAULT;
+ file = vtpm_proxy_create_device(&vtpm_new_dev);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+ if (copy_to_user(vtpm_new_dev_p, &vtpm_new_dev,
+ sizeof(vtpm_new_dev))) {
+ put_unused_fd(vtpm_new_dev.fd);
+ fput(file);
+ return -EFAULT;
+ }
+
+ fd_install(vtpm_new_dev.fd, file);
+ return 0;
+
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
+#ifdef CONFIG_COMPAT
+static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
+ unsigned long arg)
+{
+ return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
+}
+#endif
+
+static const struct file_operations vtpmx_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = vtpmx_fops_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = vtpmx_fops_compat_ioctl,
+#endif
+ .llseek = noop_llseek,
+};
+
+static struct miscdevice vtpmx_miscdev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "vtpmx",
+ .fops = &vtpmx_fops,
+};
+
+static int vtpmx_init(void)
+{
+ return misc_register(&vtpmx_miscdev);
+}
+
+static void vtpmx_cleanup(void)
+{
+ misc_deregister(&vtpmx_miscdev);
+}
+
+static int __init vtpm_module_init(void)
+{
+ int rc;
+
+ rc = vtpmx_init();
+ if (rc) {
+ pr_err("couldn't create vtpmx device\n");
+ return rc;
+ }
+
+ workqueue = create_workqueue("tpm-vtpm");
+ if (!workqueue) {
+ pr_err("couldn't create workqueue\n");
+ rc = -ENOMEM;
+ goto err_vtpmx_cleanup;
+ }
+
+ return 0;
+
+err_vtpmx_cleanup:
+ vtpmx_cleanup();
+
+ return rc;
+}
+
+static void __exit vtpm_module_exit(void)
+{
+ destroy_workqueue(workqueue);
+ vtpmx_cleanup();
+}
+
+module_init(vtpm_module_init);
+module_exit(vtpm_module_exit);
+
+MODULE_AUTHOR("Stefan Berger (stefanb@us.ibm.com)");
+MODULE_DESCRIPTION("vTPM Driver");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index ebd10e6..ac767ce 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -449,6 +449,7 @@ header-y += virtio_scsi.h
header-y += virtio_types.h
header-y += vm_sockets.h
header-y += vt.h
+header-y += vtpm_proxy.h
header-y += wait.h
header-y += wanrouter.h
header-y += watchdog.h
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
new file mode 100644
index 0000000..41e8e22
--- /dev/null
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -0,0 +1,36 @@
+/*
+ * Definitions for the VTPM proxy driver
+ * Copyright (c) 2015, 2016, IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef _UAPI_LINUX_VTPM_PROXY_H
+#define _UAPI_LINUX_VTPM_PROXY_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/* ioctls */
+
+struct vtpm_proxy_new_dev {
+ __u32 flags; /* input */
+ __u32 tpm_num; /* output */
+ __u32 fd; /* output */
+ __u32 major; /* output */
+ __u32 minor; /* output */
+};
+
+/* above flags */
+#define VTPM_PROXY_FLAG_TPM2 1 /* emulator is TPM 2 */
+
+#define VTPM_PROXY_IOC_NEW_DEV _IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
+
+#endif /* _UAPI_LINUX_VTPM_PROXY_H */
--
2.4.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 4/4] tpm: Add documentation for the tpm_vtpm_proxy device driver
2016-04-18 17:26 [PATCH v11 0/4] Multi-instance vTPM proxy driver Stefan Berger
` (2 preceding siblings ...)
2016-04-18 17:26 ` [PATCH v11 3/4] tpm: Proxy driver for supporting multiple emulated TPMs Stefan Berger
@ 2016-04-18 17:26 ` Stefan Berger
2016-04-22 16:47 ` Jarkko Sakkinen
2016-04-22 16:54 ` [PATCH v11 0/4] Multi-instance vTPM proxy driver Jarkko Sakkinen
4 siblings, 1 reply; 25+ messages in thread
From: Stefan Berger @ 2016-04-18 17:26 UTC (permalink / raw)
To: tpmdd-devel
Cc: jarkko.sakkinen, jgunthorpe, linux-security-module, linux-kernel,
Stefan Berger, linux-doc, linux-api
Add documentation for the tpm_vtpm device driver that implements
support for providing TPM functionality to Linux containers.
Parts of this documentation were recycled from the Xen vTPM
device driver documentation.
Update the documentation for the ioctl numbers.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
CC: linux-kernel@vger.kernel.org
CC: linux-doc@vger.kernel.org
CC: linux-api@vger.kernel.org
---
Documentation/ioctl/ioctl-number.txt | 1 +
Documentation/tpm/tpm_vtpm_proxy.txt | 71 ++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)
create mode 100644 Documentation/tpm/tpm_vtpm_proxy.txt
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 91261a3..7dbec90 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -303,6 +303,7 @@ Code Seq#(hex) Include File Comments
<mailto:buk@buks.ipn.de>
0xA0 all linux/sdp/sdp.h Industrial Device Project
<mailto:kenji@bitgate.com>
+0xA1 0 linux/vtpm_proxy.h TPM Emulator Proxy Driver
0xA2 00-0F arch/tile/include/asm/hardwall.h
0xA3 80-8F Port ACL in development:
<mailto:tlewis@mindspring.com>
diff --git a/Documentation/tpm/tpm_vtpm_proxy.txt b/Documentation/tpm/tpm_vtpm_proxy.txt
new file mode 100644
index 0000000..30d1902
--- /dev/null
+++ b/Documentation/tpm/tpm_vtpm_proxy.txt
@@ -0,0 +1,71 @@
+Virtual TPM Proxy Driver for Linux Containers
+
+Authors: Stefan Berger (IBM)
+
+This document describes the virtual Trusted Platform Module (vTPM)
+proxy device driver for Linux containers.
+
+INTRODUCTION
+------------
+
+The goal of this work is to provide TPM functionality to each Linux
+container. This allows programs to interact with a TPM in a container
+the same way they interact with a TPM on the physical system. Each
+container gets its own unique, emulated, software TPM.
+
+
+DESIGN
+------
+
+To make an emulated software TPM available to each container, the container
+management stack needs to create a device pair consisting of a client TPM
+character device /dev/tpmX (with X=0,1,2...) and a 'server side' file
+descriptor. The former is moved into the container by creating a character
+device with the appropriate major and minor numbers while the file descriptor
+is passed to the TPM emulator. Software inside the container can then send
+TPM commands using the character device and the emulator will receive the
+commands via the file descriptor and use it for sending back responses.
+
+To support this, the virtual TPM proxy driver provides a device /dev/vtpmx
+that is used to create device pairs using an ioctl. The ioctl takes as
+an input flags for configuring the device. The flags for example indicate
+whether TPM 1.2 or TPM 2 functionality is supported by the TPM emulator.
+The result of the ioctl are the file descriptor for the 'server side'
+as well as the major and minor numbers of the character device that was created.
+Besides that the number of the TPM character device is return. If for
+example /dev/tpm10 was created, the number (dev_num) 10 is returned.
+
+The following is the data structure of the TPM_PROXY_IOC_NEW_DEV ioctl:
+
+struct vtpm_proxy_new_dev {
+ __u32 flags; /* input */
+ __u32 tpm_num; /* output */
+ __u32 fd; /* output */
+ __u32 major; /* output */
+ __u32 minor; /* output */
+};
+
+Note that if unsupported flags are passed to the device driver, the ioctl will
+fail and errno will be set to EOPNOTSUPP. Similarly, if an unsupported ioctl is
+called on the device driver, the ioctl will fail and errno will be set to
+ENOTTY.
+
+See /usr/include/linux/vtpm_proxy.h for definitions related to the public interface
+of this vTPM device driver.
+
+Once the device has been created, the driver will immediately try to talk
+to the TPM. All commands from the driver can be read from the file descriptor
+returned by the ioctl. The commands should be responded to immediately.
+
+Depending on the version of TPM the following commands will be sent by the
+driver:
+
+- TPM 1.2:
+ - the driver will send a TPM_Startup command to the TPM emulator
+ - the driver will send commands to read the command durations and
+ interface timeouts from the TPM emulator
+- TPM 2:
+ - the driver will send a TPM2_Startup command to the TPM emulator
+
+The TPM device /dev/tpmX will only appear if all of the relevant commands
+were responded to properly.
--
2.4.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
@ 2016-04-19 10:12 ` Jarkko Sakkinen
2016-04-19 10:36 ` Stefan Berger
2016-04-22 15:06 ` Jarkko Sakkinen
2016-05-10 13:55 ` [BUG, bisect] " Jeremiah Mahler
2 siblings, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-19 10:12 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>
> The final thing preventing this was the way the sysfs files were
> attached to the pdev. Follow the approach developed for ppi and move
> the sysfs files to the chip->dev with symlinks from the pdev
> for compatibility. Everything in the core now sanely uses container_of
> to get the chip.
Can you give me a quick recap why this patch was mandatory to make the
patch set work? Which regression in the earlier versions of the patch
set this fixes?
/Jarkko
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
> drivers/char/tpm/tpm-interface.c | 7 ++--
> drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
> drivers/char/tpm/tpm.h | 10 +++---
> 4 files changed, 84 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5bc530c..7e2c9cf 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
> chip->dev.class = tpm_class;
> chip->dev.release = tpm_dev_release;
> chip->dev.parent = dev;
> -#ifdef CONFIG_ACPI
> chip->dev.groups = chip->groups;
> -#endif
>
> if (chip->dev_num == 0)
> chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>
> static int tpm1_chip_register(struct tpm_chip *chip)
> {
> - int rc;
> -
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> return 0;
>
> - rc = tpm_sysfs_add_device(chip);
> - if (rc)
> - return rc;
> + tpm_sysfs_add_device(chip);
>
> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>
> @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>
> if (chip->bios_dir)
> tpm_bios_log_teardown(chip->bios_dir);
> +}
> +
> +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return;
>
> - tpm_sysfs_del_device(chip);
> + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> +
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> }
>
> +/* For compatibility with legacy sysfs paths we provide symlinks from the
> + * parent dev directory to selected names within the tpm chip directory. Old
> + * kernel versions created these files directly under the parent.
> + */
> +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> + int rc;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return 0;
> +
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> + if (rc && rc != -ENOENT)
> + return rc;
> +
> + /* All the names from tpm-sysfs */
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> + if (rc) {
> + tpm_del_legacy_sysfs(chip);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> /*
> * tpm_chip_register() - create a character device for the TPM chip
> * @chip: TPM chip to use.
> @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> tpm_add_ppi(chip);
>
> rc = tpm_add_char_device(chip);
> - if (rc)
> - goto out_err;
> + if (rc) {
> + tpm1_chip_unregister(chip);
> + return rc;
> + }
>
> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> - rc = __compat_only_sysfs_link_entry_to_kobj(
> - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> - if (rc && rc != -ENOENT) {
> - tpm_chip_unregister(chip);
> - return rc;
> - }
> + rc = tpm_add_legacy_sysfs(chip);
> + if (rc) {
> + tpm_chip_unregister(chip);
> + return rc;
> }
>
> return 0;
> -out_err:
> - tpm1_chip_unregister(chip);
> - return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_chip_register);
>
> @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> return;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> - sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> + tpm_del_legacy_sysfs(chip);
>
> tpm1_chip_unregister(chip);
> tpm_del_char_device(chip);
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 7cba092..080dade 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
> .ordinal = TPM_ORD_GET_CAP
> };
>
> -ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> const char *desc)
> {
> struct tpm_cmd_t tpm_cmd;
> int rc;
> - struct tpm_chip *chip = dev_get_drvdata(dev);
>
> tpm_cmd.header.in = tpm_getcap_header;
> if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
> @@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
> */
> int tpm_pm_suspend(struct device *dev)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
> struct tpm_cmd_t cmd;
> int rc, try;
>
> @@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
> */
> int tpm_pm_resume(struct device *dev)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> if (chip == NULL)
> return -ENODEV;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index a7c3473..b46cf70 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> int i, rc;
> char *str = buf;
>
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> tpm_cmd.header.in = tpm_readpubek_header;
> err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> @@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> ssize_t rc;
> int i, j, num_pcrs;
> char *str = buf;
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
> + rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
> "attempting to determine the number of PCRS");
> if (rc)
> return 0;
> @@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> - "attempting to determine the permanent enabled state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> + "attempting to determine the permanent enabled state");
> if (rc)
> return 0;
>
> @@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> - "attempting to determine the permanent active state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> + "attempting to determine the permanent active state");
> if (rc)
> return 0;
>
> @@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
> - "attempting to determine the owner state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
> + "attempting to determine the owner state");
> if (rc)
> return 0;
>
> @@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
> - "attempting to determine the temporary state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
> + "attempting to determine the temporary state");
> if (rc)
> return 0;
>
> @@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
> static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> + struct tpm_chip *chip = to_tpm_chip(dev);
> cap_t cap;
> ssize_t rc;
> char *str = buf;
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
> + rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
> "attempting to determine the manufacturer");
> if (rc)
> return 0;
> @@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> be32_to_cpu(cap.manufacturer_id));
>
> /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
> - rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
> - "attempting to determine the 1.2 version");
> + rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
> + "attempting to determine the 1.2 version");
> if (!rc) {
> str += sprintf(str,
> "TCG version: %d.%d\nFirmware version: %d.%d\n",
> @@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> cap.tpm_version_1_2.revMinor);
> } else {
> /* Otherwise just use TPM_STRUCT_VER */
> - rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
> + rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
> "attempting to determine the 1.1 version");
> if (rc)
> return 0;
> @@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
> 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);
> + struct tpm_chip *chip = to_tpm_chip(dev);
> if (chip == NULL)
> return 0;
>
> @@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
> static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> if (chip->duration[TPM_LONG] == 0)
> return 0;
> @@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
> static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> return sprintf(buf, "%d %d %d %d [%s]\n",
> jiffies_to_usecs(chip->timeout_a),
> @@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
> .attrs = tpm_dev_attrs,
> };
>
> -int tpm_sysfs_add_device(struct tpm_chip *chip)
> +void tpm_sysfs_add_device(struct tpm_chip *chip)
> {
> - int err;
> - err = sysfs_create_group(&chip->dev.parent->kobj,
> - &tpm_dev_group);
> -
> - if (err)
> - dev_err(&chip->dev,
> - "failed to create sysfs attributes, %d\n", err);
> - return err;
> -}
> -
> -void tpm_sysfs_del_device(struct tpm_chip *chip)
> -{
> - /* The sysfs routines rely on an implicit tpm_try_get_ops, this
> - * function is called before ops is null'd and the sysfs core
> - * synchronizes this removal so that no callbacks are running or can
> - * run again
> + /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> + * is called before ops is null'd and the sysfs core synchronizes this
> + * removal so that no callbacks are running or can run again
> */
> - sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
> + WARN_ON(chip->groups_cnt != 0);
> + chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8bc6fb8..508e8e0 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -168,9 +168,9 @@ struct tpm_chip {
>
> struct dentry **bios_dir;
>
> -#ifdef CONFIG_ACPI
> - const struct attribute_group *groups[2];
> + const struct attribute_group *groups[3];
> unsigned int groups_cnt;
> +#ifdef CONFIG_ACPI
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> #endif /* CONFIG_ACPI */
> @@ -471,7 +471,8 @@ extern dev_t tpm_devt;
> extern const struct file_operations tpm_fops;
> extern struct idr dev_nums_idr;
>
> -ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> + const char *desc);
> 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,
> @@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
> extern int tpm_chip_register(struct tpm_chip *chip);
> extern void tpm_chip_unregister(struct tpm_chip *chip);
>
> -int tpm_sysfs_add_device(struct tpm_chip *chip);
> -void tpm_sysfs_del_device(struct tpm_chip *chip);
> +void tpm_sysfs_add_device(struct tpm_chip *chip);
>
> int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-19 10:12 ` Jarkko Sakkinen
@ 2016-04-19 10:36 ` Stefan Berger
2016-04-19 17:06 ` Jason Gunthorpe
2016-04-19 17:21 ` Jarkko Sakkinen
0 siblings, 2 replies; 25+ messages in thread
From: Stefan Berger @ 2016-04-19 10:36 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote:
> On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
>> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>
>> The final thing preventing this was the way the sysfs files were
>> attached to the pdev. Follow the approach developed for ppi and move
>> the sysfs files to the chip->dev with symlinks from the pdev
>> for compatibility. Everything in the core now sanely uses container_of
>> to get the chip.
> Can you give me a quick recap why this patch was mandatory to make the
> patch set work? Which regression in the earlier versions of the patch
> set this fixes?
The below patch removes usage of dev_get_drvdata() for retrieving the
chip. With Christophe's series dropping the priv field I now can use the
drvdata to store proxy_dev rather than re-introducing the priv field in
the chip structure. Besides that it doesn't seem necessary to use the
drvdata field to get from the chip to the device if a simple
container_of can do it.
Stefan
>
> /Jarkko
>
>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>> drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
>> drivers/char/tpm/tpm-interface.c | 7 ++--
>> drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
>> drivers/char/tpm/tpm.h | 10 +++---
>> 4 files changed, 84 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 5bc530c..7e2c9cf 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
>> chip->dev.class = tpm_class;
>> chip->dev.release = tpm_dev_release;
>> chip->dev.parent = dev;
>> -#ifdef CONFIG_ACPI
>> chip->dev.groups = chip->groups;
>> -#endif
>>
>> if (chip->dev_num == 0)
>> chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
>> @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>>
>> static int tpm1_chip_register(struct tpm_chip *chip)
>> {
>> - int rc;
>> -
>> if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> return 0;
>>
>> - rc = tpm_sysfs_add_device(chip);
>> - if (rc)
>> - return rc;
>> + tpm_sysfs_add_device(chip);
>>
>> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>>
>> @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>>
>> if (chip->bios_dir)
>> tpm_bios_log_teardown(chip->bios_dir);
>> +}
>> +
>> +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>> +{
>> + struct attribute **i;
>> +
>> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> + return;
>>
>> - tpm_sysfs_del_device(chip);
>> + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
>> +
>> + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
>> + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
>> }
>>
>> +/* For compatibility with legacy sysfs paths we provide symlinks from the
>> + * parent dev directory to selected names within the tpm chip directory. Old
>> + * kernel versions created these files directly under the parent.
>> + */
>> +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>> +{
>> + struct attribute **i;
>> + int rc;
>> +
>> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> + return 0;
>> +
>> + rc = __compat_only_sysfs_link_entry_to_kobj(
>> + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
>> + if (rc && rc != -ENOENT)
>> + return rc;
>> +
>> + /* All the names from tpm-sysfs */
>> + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
>> + rc = __compat_only_sysfs_link_entry_to_kobj(
>> + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
>> + if (rc) {
>> + tpm_del_legacy_sysfs(chip);
>> + return rc;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> /*
>> * tpm_chip_register() - create a character device for the TPM chip
>> * @chip: TPM chip to use.
>> @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
>> tpm_add_ppi(chip);
>>
>> rc = tpm_add_char_device(chip);
>> - if (rc)
>> - goto out_err;
>> + if (rc) {
>> + tpm1_chip_unregister(chip);
>> + return rc;
>> + }
>>
>> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>>
>> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
>> - rc = __compat_only_sysfs_link_entry_to_kobj(
>> - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
>> - if (rc && rc != -ENOENT) {
>> - tpm_chip_unregister(chip);
>> - return rc;
>> - }
>> + rc = tpm_add_legacy_sysfs(chip);
>> + if (rc) {
>> + tpm_chip_unregister(chip);
>> + return rc;
>> }
>>
>> return 0;
>> -out_err:
>> - tpm1_chip_unregister(chip);
>> - return rc;
>> }
>> EXPORT_SYMBOL_GPL(tpm_chip_register);
>>
>> @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
>> return;
>>
>> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
>> - sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
>> + tpm_del_legacy_sysfs(chip);
>>
>> tpm1_chip_unregister(chip);
>> tpm_del_char_device(chip);
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index 7cba092..080dade 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
>> .ordinal = TPM_ORD_GET_CAP
>> };
>>
>> -ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
>> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
>> const char *desc)
>> {
>> struct tpm_cmd_t tpm_cmd;
>> int rc;
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>>
>> tpm_cmd.header.in = tpm_getcap_header;
>> if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
>> @@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
>> */
>> int tpm_pm_suspend(struct device *dev)
>> {
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>> struct tpm_cmd_t cmd;
>> int rc, try;
>>
>> @@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
>> */
>> int tpm_pm_resume(struct device *dev)
>> {
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>>
>> if (chip == NULL)
>> return -ENODEV;
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index a7c3473..b46cf70 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
>> int i, rc;
>> char *str = buf;
>>
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>>
>> tpm_cmd.header.in = tpm_readpubek_header;
>> err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
>> @@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
>> ssize_t rc;
>> int i, j, num_pcrs;
>> char *str = buf;
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>>
>> - rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
>> + rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
>> "attempting to determine the number of PCRS");
>> if (rc)
>> return 0;
>> @@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
>> cap_t cap;
>> ssize_t rc;
>>
>> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
>> - "attempting to determine the permanent enabled state");
>> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
>> + "attempting to determine the permanent enabled state");
>> if (rc)
>> return 0;
>>
>> @@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
>> cap_t cap;
>> ssize_t rc;
>>
>> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
>> - "attempting to determine the permanent active state");
>> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
>> + "attempting to determine the permanent active state");
>> if (rc)
>> return 0;
>>
>> @@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
>> cap_t cap;
>> ssize_t rc;
>>
>> - rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
>> - "attempting to determine the owner state");
>> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
>> + "attempting to determine the owner state");
>> if (rc)
>> return 0;
>>
>> @@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
>> cap_t cap;
>> ssize_t rc;
>>
>> - rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
>> - "attempting to determine the temporary state");
>> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
>> + "attempting to determine the temporary state");
>> if (rc)
>> return 0;
>>
>> @@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
>> static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>> cap_t cap;
>> ssize_t rc;
>> char *str = buf;
>>
>> - rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
>> + rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
>> "attempting to determine the manufacturer");
>> if (rc)
>> return 0;
>> @@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
>> be32_to_cpu(cap.manufacturer_id));
>>
>> /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
>> - rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
>> - "attempting to determine the 1.2 version");
>> + rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
>> + "attempting to determine the 1.2 version");
>> if (!rc) {
>> str += sprintf(str,
>> "TCG version: %d.%d\nFirmware version: %d.%d\n",
>> @@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
>> cap.tpm_version_1_2.revMinor);
>> } else {
>> /* Otherwise just use TPM_STRUCT_VER */
>> - rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
>> + rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
>> "attempting to determine the 1.1 version");
>> if (rc)
>> return 0;
>> @@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
>> 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);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>> if (chip == NULL)
>> return 0;
>>
>> @@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
>> static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>>
>> if (chip->duration[TPM_LONG] == 0)
>> return 0;
>> @@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
>> static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> - struct tpm_chip *chip = dev_get_drvdata(dev);
>> + struct tpm_chip *chip = to_tpm_chip(dev);
>>
>> return sprintf(buf, "%d %d %d %d [%s]\n",
>> jiffies_to_usecs(chip->timeout_a),
>> @@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
>> .attrs = tpm_dev_attrs,
>> };
>>
>> -int tpm_sysfs_add_device(struct tpm_chip *chip)
>> +void tpm_sysfs_add_device(struct tpm_chip *chip)
>> {
>> - int err;
>> - err = sysfs_create_group(&chip->dev.parent->kobj,
>> - &tpm_dev_group);
>> -
>> - if (err)
>> - dev_err(&chip->dev,
>> - "failed to create sysfs attributes, %d\n", err);
>> - return err;
>> -}
>> -
>> -void tpm_sysfs_del_device(struct tpm_chip *chip)
>> -{
>> - /* The sysfs routines rely on an implicit tpm_try_get_ops, this
>> - * function is called before ops is null'd and the sysfs core
>> - * synchronizes this removal so that no callbacks are running or can
>> - * run again
>> + /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
>> + * is called before ops is null'd and the sysfs core synchronizes this
>> + * removal so that no callbacks are running or can run again
>> */
>> - sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
>> + WARN_ON(chip->groups_cnt != 0);
>> + chip->groups[chip->groups_cnt++] = &tpm_dev_group;
>> }
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 8bc6fb8..508e8e0 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -168,9 +168,9 @@ struct tpm_chip {
>>
>> struct dentry **bios_dir;
>>
>> -#ifdef CONFIG_ACPI
>> - const struct attribute_group *groups[2];
>> + const struct attribute_group *groups[3];
>> unsigned int groups_cnt;
>> +#ifdef CONFIG_ACPI
>> acpi_handle acpi_dev_handle;
>> char ppi_version[TPM_PPI_VERSION_LEN + 1];
>> #endif /* CONFIG_ACPI */
>> @@ -471,7 +471,8 @@ extern dev_t tpm_devt;
>> extern const struct file_operations tpm_fops;
>> extern struct idr dev_nums_idr;
>>
>> -ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
>> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
>> + const char *desc);
>> 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,
>> @@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
>> extern int tpm_chip_register(struct tpm_chip *chip);
>> extern void tpm_chip_unregister(struct tpm_chip *chip);
>>
>> -int tpm_sysfs_add_device(struct tpm_chip *chip);
>> -void tpm_sysfs_del_device(struct tpm_chip *chip);
>> +void tpm_sysfs_add_device(struct tpm_chip *chip);
>>
>> int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>>
>> --
>> 2.4.3
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-19 10:36 ` Stefan Berger
@ 2016-04-19 17:06 ` Jason Gunthorpe
2016-04-20 5:33 ` Jarkko Sakkinen
2016-04-19 17:21 ` Jarkko Sakkinen
1 sibling, 1 reply; 25+ messages in thread
From: Jason Gunthorpe @ 2016-04-19 17:06 UTC (permalink / raw)
To: Stefan Berger
Cc: Jarkko Sakkinen, tpmdd-devel, linux-security-module, linux-kernel
On Tue, Apr 19, 2016 at 06:36:22AM -0400, Stefan Berger wrote:
> On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote:
> >On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> >>From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >>
> >>The final thing preventing this was the way the sysfs files were
> >>attached to the pdev. Follow the approach developed for ppi and move
> >>the sysfs files to the chip->dev with symlinks from the pdev
> >>for compatibility. Everything in the core now sanely uses container_of
> >>to get the chip.
> >Can you give me a quick recap why this patch was mandatory to make the
> >patch set work? Which regression in the earlier versions of the patch
> >set this fixes?
>
> The below patch removes usage of dev_get_drvdata() for retrieving the chip.
> With Christophe's series dropping the priv field I now can use the drvdata
> to store proxy_dev rather than re-introducing the priv field in the chip
> structure. Besides that it doesn't seem necessary to use the drvdata field
> to get from the chip to the device if a simple container_of can do it.
More specifically, since the vtpm patches use a NULL parent, the
approach of putting the sysfs files on the parent is no longer
workable.
The early vtpm patches simply moved the sysfs files to the tpm_chip
when a parent is NULL, which is inconsistent for userspace. This also
created a problem where drvdata on the chip now had to point back to
the chip, meaning it became unusable for its new intended purpose.
The fix is to make everything uniform and put the sysfs files in the
correct place for all drivers (under the chip) and use symlinks for
compat.
Jason
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-19 10:36 ` Stefan Berger
2016-04-19 17:06 ` Jason Gunthorpe
@ 2016-04-19 17:21 ` Jarkko Sakkinen
1 sibling, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-19 17:21 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On Tue, Apr 19, 2016 at 06:36:22AM -0400, Stefan Berger wrote:
> On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote:
> >On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> >>From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >>
> >>The final thing preventing this was the way the sysfs files were
> >>attached to the pdev. Follow the approach developed for ppi and move
> >>the sysfs files to the chip->dev with symlinks from the pdev
> >>for compatibility. Everything in the core now sanely uses container_of
> >>to get the chip.
> >Can you give me a quick recap why this patch was mandatory to make the
> >patch set work? Which regression in the earlier versions of the patch
> >set this fixes?
>
> The below patch removes usage of dev_get_drvdata() for retrieving the chip.
> With Christophe's series dropping the priv field I now can use the drvdata
> to store proxy_dev rather than re-introducing the priv field in the chip
> structure. Besides that it doesn't seem necessary to use the drvdata field
> to get from the chip to the device if a simple container_of can do it.
Right, ok sounds justified.
> Stefan
>
> >
> >/Jarkko
> >
> >>Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >>Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> >>---
> >> drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
> >> drivers/char/tpm/tpm-interface.c | 7 ++--
> >> drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
> >> drivers/char/tpm/tpm.h | 10 +++---
> >> 4 files changed, 84 insertions(+), 67 deletions(-)
> >>
> >>diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >>index 5bc530c..7e2c9cf 100644
> >>--- a/drivers/char/tpm/tpm-chip.c
> >>+++ b/drivers/char/tpm/tpm-chip.c
> >>@@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
> >> chip->dev.class = tpm_class;
> >> chip->dev.release = tpm_dev_release;
> >> chip->dev.parent = dev;
> >>-#ifdef CONFIG_ACPI
> >> chip->dev.groups = chip->groups;
> >>-#endif
> >> if (chip->dev_num == 0)
> >> chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> >>@@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
> >> static int tpm1_chip_register(struct tpm_chip *chip)
> >> {
> >>- int rc;
> >>-
> >> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >> return 0;
> >>- rc = tpm_sysfs_add_device(chip);
> >>- if (rc)
> >>- return rc;
> >>+ tpm_sysfs_add_device(chip);
> >> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
> >>@@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
> >> if (chip->bios_dir)
> >> tpm_bios_log_teardown(chip->bios_dir);
> >>+}
> >>+
> >>+static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> >>+{
> >>+ struct attribute **i;
> >>+
> >>+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >>+ return;
> >>- tpm_sysfs_del_device(chip);
> >>+ sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> >>+
> >>+ for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> >>+ sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> >> }
> >>+/* For compatibility with legacy sysfs paths we provide symlinks from the
> >>+ * parent dev directory to selected names within the tpm chip directory. Old
> >>+ * kernel versions created these files directly under the parent.
> >>+ */
> >>+static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> >>+{
> >>+ struct attribute **i;
> >>+ int rc;
> >>+
> >>+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >>+ return 0;
> >>+
> >>+ rc = __compat_only_sysfs_link_entry_to_kobj(
> >>+ &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> >>+ if (rc && rc != -ENOENT)
> >>+ return rc;
> >>+
> >>+ /* All the names from tpm-sysfs */
> >>+ for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> >>+ rc = __compat_only_sysfs_link_entry_to_kobj(
> >>+ &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> >>+ if (rc) {
> >>+ tpm_del_legacy_sysfs(chip);
> >>+ return rc;
> >>+ }
> >>+ }
> >>+
> >>+ return 0;
> >>+}
> >> /*
> >> * tpm_chip_register() - create a character device for the TPM chip
> >> * @chip: TPM chip to use.
> >>@@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> >> tpm_add_ppi(chip);
> >> rc = tpm_add_char_device(chip);
> >>- if (rc)
> >>- goto out_err;
> >>+ if (rc) {
> >>+ tpm1_chip_unregister(chip);
> >>+ return rc;
> >>+ }
> >> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
> >>- if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> >>- rc = __compat_only_sysfs_link_entry_to_kobj(
> >>- &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> >>- if (rc && rc != -ENOENT) {
> >>- tpm_chip_unregister(chip);
> >>- return rc;
> >>- }
> >>+ rc = tpm_add_legacy_sysfs(chip);
> >>+ if (rc) {
> >>+ tpm_chip_unregister(chip);
> >>+ return rc;
> >> }
> >> return 0;
> >>-out_err:
> >>- tpm1_chip_unregister(chip);
> >>- return rc;
> >> }
> >> EXPORT_SYMBOL_GPL(tpm_chip_register);
> >>@@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> >> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> >> return;
> >>- if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> >>- sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> >>+ tpm_del_legacy_sysfs(chip);
> >> tpm1_chip_unregister(chip);
> >> tpm_del_char_device(chip);
> >>diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> >>index 7cba092..080dade 100644
> >>--- a/drivers/char/tpm/tpm-interface.c
> >>+++ b/drivers/char/tpm/tpm-interface.c
> >>@@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
> >> .ordinal = TPM_ORD_GET_CAP
> >> };
> >>-ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> >>+ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> >> const char *desc)
> >> {
> >> struct tpm_cmd_t tpm_cmd;
> >> int rc;
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >> tpm_cmd.header.in = tpm_getcap_header;
> >> if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
> >>@@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
> >> */
> >> int tpm_pm_suspend(struct device *dev)
> >> {
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> struct tpm_cmd_t cmd;
> >> int rc, try;
> >>@@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
> >> */
> >> int tpm_pm_resume(struct device *dev)
> >> {
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> if (chip == NULL)
> >> return -ENODEV;
> >>diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> >>index a7c3473..b46cf70 100644
> >>--- a/drivers/char/tpm/tpm-sysfs.c
> >>+++ b/drivers/char/tpm/tpm-sysfs.c
> >>@@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> >> int i, rc;
> >> char *str = buf;
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> tpm_cmd.header.in = tpm_readpubek_header;
> >> err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> >>@@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> >> ssize_t rc;
> >> int i, j, num_pcrs;
> >> char *str = buf;
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >>- rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
> >>+ rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
> >> "attempting to determine the number of PCRS");
> >> if (rc)
> >> return 0;
> >>@@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
> >> cap_t cap;
> >> ssize_t rc;
> >>- rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> >>- "attempting to determine the permanent enabled state");
> >>+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> >>+ "attempting to determine the permanent enabled state");
> >> if (rc)
> >> return 0;
> >>@@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
> >> cap_t cap;
> >> ssize_t rc;
> >>- rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> >>- "attempting to determine the permanent active state");
> >>+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> >>+ "attempting to determine the permanent active state");
> >> if (rc)
> >> return 0;
> >>@@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
> >> cap_t cap;
> >> ssize_t rc;
> >>- rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
> >>- "attempting to determine the owner state");
> >>+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
> >>+ "attempting to determine the owner state");
> >> if (rc)
> >> return 0;
> >>@@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
> >> cap_t cap;
> >> ssize_t rc;
> >>- rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
> >>- "attempting to determine the temporary state");
> >>+ rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
> >>+ "attempting to determine the temporary state");
> >> if (rc)
> >> return 0;
> >>@@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
> >> static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> >> char *buf)
> >> {
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> cap_t cap;
> >> ssize_t rc;
> >> char *str = buf;
> >>- rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
> >>+ rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
> >> "attempting to determine the manufacturer");
> >> if (rc)
> >> return 0;
> >>@@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> >> be32_to_cpu(cap.manufacturer_id));
> >> /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
> >>- rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
> >>- "attempting to determine the 1.2 version");
> >>+ rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
> >>+ "attempting to determine the 1.2 version");
> >> if (!rc) {
> >> str += sprintf(str,
> >> "TCG version: %d.%d\nFirmware version: %d.%d\n",
> >>@@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> >> cap.tpm_version_1_2.revMinor);
> >> } else {
> >> /* Otherwise just use TPM_STRUCT_VER */
> >>- rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
> >>+ rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
> >> "attempting to determine the 1.1 version");
> >> if (rc)
> >> return 0;
> >>@@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
> >> 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);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> if (chip == NULL)
> >> return 0;
> >>@@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
> >> static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> >> char *buf)
> >> {
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> if (chip->duration[TPM_LONG] == 0)
> >> return 0;
> >>@@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
> >> static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> >> char *buf)
> >> {
> >>- struct tpm_chip *chip = dev_get_drvdata(dev);
> >>+ struct tpm_chip *chip = to_tpm_chip(dev);
> >> return sprintf(buf, "%d %d %d %d [%s]\n",
> >> jiffies_to_usecs(chip->timeout_a),
> >>@@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
> >> .attrs = tpm_dev_attrs,
> >> };
> >>-int tpm_sysfs_add_device(struct tpm_chip *chip)
> >>+void tpm_sysfs_add_device(struct tpm_chip *chip)
> >> {
> >>- int err;
> >>- err = sysfs_create_group(&chip->dev.parent->kobj,
> >>- &tpm_dev_group);
> >>-
> >>- if (err)
> >>- dev_err(&chip->dev,
> >>- "failed to create sysfs attributes, %d\n", err);
> >>- return err;
> >>-}
> >>-
> >>-void tpm_sysfs_del_device(struct tpm_chip *chip)
> >>-{
> >>- /* The sysfs routines rely on an implicit tpm_try_get_ops, this
> >>- * function is called before ops is null'd and the sysfs core
> >>- * synchronizes this removal so that no callbacks are running or can
> >>- * run again
> >>+ /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> >>+ * is called before ops is null'd and the sysfs core synchronizes this
> >>+ * removal so that no callbacks are running or can run again
> >> */
> >>- sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
> >>+ WARN_ON(chip->groups_cnt != 0);
> >>+ chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> >> }
> >>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> >>index 8bc6fb8..508e8e0 100644
> >>--- a/drivers/char/tpm/tpm.h
> >>+++ b/drivers/char/tpm/tpm.h
> >>@@ -168,9 +168,9 @@ struct tpm_chip {
> >> struct dentry **bios_dir;
> >>-#ifdef CONFIG_ACPI
> >>- const struct attribute_group *groups[2];
> >>+ const struct attribute_group *groups[3];
> >> unsigned int groups_cnt;
> >>+#ifdef CONFIG_ACPI
> >> acpi_handle acpi_dev_handle;
> >> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> >> #endif /* CONFIG_ACPI */
> >>@@ -471,7 +471,8 @@ extern dev_t tpm_devt;
> >> extern const struct file_operations tpm_fops;
> >> extern struct idr dev_nums_idr;
> >>-ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> >>+ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> >>+ const char *desc);
> >> 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,
> >>@@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
> >> extern int tpm_chip_register(struct tpm_chip *chip);
> >> extern void tpm_chip_unregister(struct tpm_chip *chip);
> >>-int tpm_sysfs_add_device(struct tpm_chip *chip);
> >>-void tpm_sysfs_del_device(struct tpm_chip *chip);
> >>+void tpm_sysfs_add_device(struct tpm_chip *chip);
> >> int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> >>--
> >>2.4.3
> >>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-19 17:06 ` Jason Gunthorpe
@ 2016-04-20 5:33 ` Jarkko Sakkinen
0 siblings, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-20 5:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Stefan Berger, tpmdd-devel, linux-security-module, linux-kernel
On Tue, Apr 19, 2016 at 11:06:45AM -0600, Jason Gunthorpe wrote:
> On Tue, Apr 19, 2016 at 06:36:22AM -0400, Stefan Berger wrote:
> > On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote:
> > >On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> > >>From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > >>
> > >>The final thing preventing this was the way the sysfs files were
> > >>attached to the pdev. Follow the approach developed for ppi and move
> > >>the sysfs files to the chip->dev with symlinks from the pdev
> > >>for compatibility. Everything in the core now sanely uses container_of
> > >>to get the chip.
> > >Can you give me a quick recap why this patch was mandatory to make the
> > >patch set work? Which regression in the earlier versions of the patch
> > >set this fixes?
> >
> > The below patch removes usage of dev_get_drvdata() for retrieving the chip.
> > With Christophe's series dropping the priv field I now can use the drvdata
> > to store proxy_dev rather than re-introducing the priv field in the chip
> > structure. Besides that it doesn't seem necessary to use the drvdata field
> > to get from the chip to the device if a simple container_of can do it.
>
> More specifically, since the vtpm patches use a NULL parent, the
> approach of putting the sysfs files on the parent is no longer
> workable.
>
> The early vtpm patches simply moved the sysfs files to the tpm_chip
> when a parent is NULL, which is inconsistent for userspace. This also
> created a problem where drvdata on the chip now had to point back to
> the chip, meaning it became unusable for its new intended purpose.
>
> The fix is to make everything uniform and put the sysfs files in the
> correct place for all drivers (under the chip) and use symlinks for
> compat.
OK, thanks for explaining this, make perfect sense now. I'll do a more
detailed review (and testing) later on.
> Jason
/Jarkko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
2016-04-19 10:12 ` Jarkko Sakkinen
@ 2016-04-22 15:06 ` Jarkko Sakkinen
2016-04-25 19:13 ` Stefan Berger
2016-05-10 13:55 ` [BUG, bisect] " Jeremiah Mahler
2 siblings, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-22 15:06 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>
> The final thing preventing this was the way the sysfs files were
> attached to the pdev. Follow the approach developed for ppi and move
> the sysfs files to the chip->dev with symlinks from the pdev
> for compatibility. Everything in the core now sanely uses container_of
> to get the chip.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Three configurations:
* Haswell NUC with PTT (tpm_crb)
* Another NUC with dTPM 2.0 chip
* Dell E6420, which has TPM 1.2 chip
Things seem to be unbroken.
Stefan, have you verified that sysfs attributes work through routes:
1. From char device sysfs directory
2. Through link
An also tried insmod/rmmod couple of rounds?
If you have and things continue work could you give this Tested-by?
/Jarkko
> ---
> drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
> drivers/char/tpm/tpm-interface.c | 7 ++--
> drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
> drivers/char/tpm/tpm.h | 10 +++---
> 4 files changed, 84 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5bc530c..7e2c9cf 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
> chip->dev.class = tpm_class;
> chip->dev.release = tpm_dev_release;
> chip->dev.parent = dev;
> -#ifdef CONFIG_ACPI
> chip->dev.groups = chip->groups;
> -#endif
>
> if (chip->dev_num == 0)
> chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>
> static int tpm1_chip_register(struct tpm_chip *chip)
> {
> - int rc;
> -
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> return 0;
>
> - rc = tpm_sysfs_add_device(chip);
> - if (rc)
> - return rc;
> + tpm_sysfs_add_device(chip);
>
> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>
> @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>
> if (chip->bios_dir)
> tpm_bios_log_teardown(chip->bios_dir);
> +}
> +
> +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return;
>
> - tpm_sysfs_del_device(chip);
> + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> +
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> }
>
> +/* For compatibility with legacy sysfs paths we provide symlinks from the
> + * parent dev directory to selected names within the tpm chip directory. Old
> + * kernel versions created these files directly under the parent.
> + */
> +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> + int rc;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return 0;
> +
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> + if (rc && rc != -ENOENT)
> + return rc;
> +
> + /* All the names from tpm-sysfs */
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> + if (rc) {
> + tpm_del_legacy_sysfs(chip);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> /*
> * tpm_chip_register() - create a character device for the TPM chip
> * @chip: TPM chip to use.
> @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> tpm_add_ppi(chip);
>
> rc = tpm_add_char_device(chip);
> - if (rc)
> - goto out_err;
> + if (rc) {
> + tpm1_chip_unregister(chip);
> + return rc;
> + }
>
> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> - rc = __compat_only_sysfs_link_entry_to_kobj(
> - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> - if (rc && rc != -ENOENT) {
> - tpm_chip_unregister(chip);
> - return rc;
> - }
> + rc = tpm_add_legacy_sysfs(chip);
> + if (rc) {
> + tpm_chip_unregister(chip);
> + return rc;
> }
>
> return 0;
> -out_err:
> - tpm1_chip_unregister(chip);
> - return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_chip_register);
>
> @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> return;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> - sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> + tpm_del_legacy_sysfs(chip);
>
> tpm1_chip_unregister(chip);
> tpm_del_char_device(chip);
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 7cba092..080dade 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
> .ordinal = TPM_ORD_GET_CAP
> };
>
> -ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> const char *desc)
> {
> struct tpm_cmd_t tpm_cmd;
> int rc;
> - struct tpm_chip *chip = dev_get_drvdata(dev);
>
> tpm_cmd.header.in = tpm_getcap_header;
> if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
> @@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
> */
> int tpm_pm_suspend(struct device *dev)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
> struct tpm_cmd_t cmd;
> int rc, try;
>
> @@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
> */
> int tpm_pm_resume(struct device *dev)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> if (chip == NULL)
> return -ENODEV;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index a7c3473..b46cf70 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> int i, rc;
> char *str = buf;
>
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> tpm_cmd.header.in = tpm_readpubek_header;
> err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> @@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> ssize_t rc;
> int i, j, num_pcrs;
> char *str = buf;
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
> + rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
> "attempting to determine the number of PCRS");
> if (rc)
> return 0;
> @@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> - "attempting to determine the permanent enabled state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> + "attempting to determine the permanent enabled state");
> if (rc)
> return 0;
>
> @@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> - "attempting to determine the permanent active state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> + "attempting to determine the permanent active state");
> if (rc)
> return 0;
>
> @@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
> - "attempting to determine the owner state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
> + "attempting to determine the owner state");
> if (rc)
> return 0;
>
> @@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
> - "attempting to determine the temporary state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
> + "attempting to determine the temporary state");
> if (rc)
> return 0;
>
> @@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
> static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> + struct tpm_chip *chip = to_tpm_chip(dev);
> cap_t cap;
> ssize_t rc;
> char *str = buf;
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
> + rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
> "attempting to determine the manufacturer");
> if (rc)
> return 0;
> @@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> be32_to_cpu(cap.manufacturer_id));
>
> /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
> - rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
> - "attempting to determine the 1.2 version");
> + rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
> + "attempting to determine the 1.2 version");
> if (!rc) {
> str += sprintf(str,
> "TCG version: %d.%d\nFirmware version: %d.%d\n",
> @@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> cap.tpm_version_1_2.revMinor);
> } else {
> /* Otherwise just use TPM_STRUCT_VER */
> - rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
> + rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
> "attempting to determine the 1.1 version");
> if (rc)
> return 0;
> @@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
> 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);
> + struct tpm_chip *chip = to_tpm_chip(dev);
> if (chip == NULL)
> return 0;
>
> @@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
> static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> if (chip->duration[TPM_LONG] == 0)
> return 0;
> @@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
> static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> return sprintf(buf, "%d %d %d %d [%s]\n",
> jiffies_to_usecs(chip->timeout_a),
> @@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
> .attrs = tpm_dev_attrs,
> };
>
> -int tpm_sysfs_add_device(struct tpm_chip *chip)
> +void tpm_sysfs_add_device(struct tpm_chip *chip)
> {
> - int err;
> - err = sysfs_create_group(&chip->dev.parent->kobj,
> - &tpm_dev_group);
> -
> - if (err)
> - dev_err(&chip->dev,
> - "failed to create sysfs attributes, %d\n", err);
> - return err;
> -}
> -
> -void tpm_sysfs_del_device(struct tpm_chip *chip)
> -{
> - /* The sysfs routines rely on an implicit tpm_try_get_ops, this
> - * function is called before ops is null'd and the sysfs core
> - * synchronizes this removal so that no callbacks are running or can
> - * run again
> + /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> + * is called before ops is null'd and the sysfs core synchronizes this
> + * removal so that no callbacks are running or can run again
> */
> - sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
> + WARN_ON(chip->groups_cnt != 0);
> + chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8bc6fb8..508e8e0 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -168,9 +168,9 @@ struct tpm_chip {
>
> struct dentry **bios_dir;
>
> -#ifdef CONFIG_ACPI
> - const struct attribute_group *groups[2];
> + const struct attribute_group *groups[3];
> unsigned int groups_cnt;
> +#ifdef CONFIG_ACPI
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> #endif /* CONFIG_ACPI */
> @@ -471,7 +471,8 @@ extern dev_t tpm_devt;
> extern const struct file_operations tpm_fops;
> extern struct idr dev_nums_idr;
>
> -ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> + const char *desc);
> 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,
> @@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
> extern int tpm_chip_register(struct tpm_chip *chip);
> extern void tpm_chip_unregister(struct tpm_chip *chip);
>
> -int tpm_sysfs_add_device(struct tpm_chip *chip);
> -void tpm_sysfs_del_device(struct tpm_chip *chip);
> +void tpm_sysfs_add_device(struct tpm_chip *chip);
>
> int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 2/4] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL
2016-04-18 17:26 ` [PATCH v11 2/4] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL Stefan Berger
@ 2016-04-22 16:46 ` Jarkko Sakkinen
0 siblings, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-22 16:46 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On Mon, Apr 18, 2016 at 01:26:14PM -0400, Stefan Berger wrote:
> Introduce TPM_CHIP_FLAG_VIRTUAL to be used when the chip device has no
> parent device.
>
> Prevent sysfs entries requiring a parent device from being created.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
> ---
> drivers/char/tpm/tpm-chip.c | 7 +++++--
> drivers/char/tpm/tpm.h | 1 +
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 7e2c9cf..a12db8e 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -181,6 +181,9 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
> if (rc)
> goto out;
>
> + if (!dev)
> + chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
> +
> cdev_init(&chip->cdev, &tpm_fops);
> chip->cdev.owner = THIS_MODULE;
> chip->cdev.kobj.parent = &chip->dev.kobj;
> @@ -297,7 +300,7 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> {
> struct attribute **i;
>
> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> return;
>
> sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> @@ -315,7 +318,7 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> struct attribute **i;
> int rc;
>
> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
> return 0;
>
> rc = __compat_only_sysfs_link_entry_to_kobj(
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 508e8e0..ff4e53c 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -138,6 +138,7 @@ enum tpm_chip_flags {
> TPM_CHIP_FLAG_REGISTERED = BIT(0),
> TPM_CHIP_FLAG_TPM2 = BIT(1),
> TPM_CHIP_FLAG_IRQ = BIT(2),
> + TPM_CHIP_FLAG_VIRTUAL = BIT(3),
> };
>
> struct tpm_chip {
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" 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 [flat|nested] 25+ messages in thread
* Re: [PATCH v11 3/4] tpm: Proxy driver for supporting multiple emulated TPMs
2016-04-18 17:26 ` [PATCH v11 3/4] tpm: Proxy driver for supporting multiple emulated TPMs Stefan Berger
@ 2016-04-22 16:47 ` Jarkko Sakkinen
0 siblings, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-22 16:47 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel,
linux-doc, linux-api
On Mon, Apr 18, 2016 at 01:26:15PM -0400, Stefan Berger wrote:
> This patch implements a proxy driver for supporting multiple emulated TPMs
> in a system.
>
> The driver implements a device /dev/vtpmx that is used to created
> a client device pair /dev/tpmX (e.g., /dev/tpm10) and a server side that
> is accessed using a file descriptor returned by an ioctl.
> The device /dev/tpmX is the usual TPM device created by the core TPM
> driver. Applications or kernel subsystems can send TPM commands to it
> and the corresponding server-side file descriptor receives these
> commands and delivers them to an emulated TPM.
>
> The driver retrievs the TPM 1.2 durations and timeouts. Since this requires
> the startup of the TPM, we send a startup for TPM 1.2 as well as TPM 2.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
>
> CC: linux-kernel@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> CC: linux-api@vger.kernel.org
> ---
> drivers/char/tpm/Kconfig | 10 +
> drivers/char/tpm/Makefile | 1 +
> drivers/char/tpm/tpm_vtpm_proxy.c | 644 ++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/vtpm_proxy.h | 36 +++
> 5 files changed, 692 insertions(+)
> create mode 100644 drivers/char/tpm/tpm_vtpm_proxy.c
> create mode 100644 include/uapi/linux/vtpm_proxy.h
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 3b84a8b..0eac596 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -122,5 +122,15 @@ config TCG_CRB
> from within Linux. To compile this driver as a module, choose
> M here; the module will be called tpm_crb.
>
> +config TCG_VTPM_PROXY
> + tristate "VTPM Proxy Interface"
> + depends on TCG_TPM
> + ---help---
> + This driver proxies for an emulated TPM (vTPM) running in userspace.
> + A device /dev/vtpmx is provided that creates a device pair
> + /dev/vtpmX and a server-side file descriptor on which the vTPM
> + can receive commands.
> +
> +
> source "drivers/char/tpm/st33zp24/Kconfig"
> endif # TCG_TPM
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 56e8f1f..98de5e6 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
> obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
> obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
> obj-$(CONFIG_TCG_CRB) += tpm_crb.o
> +obj-$(CONFIG_TCG_VTPM_PROXY) += tpm_vtpm_proxy.o
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> new file mode 100644
> index 0000000..4384042
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -0,0 +1,644 @@
> +/*
> + * Copyright (C) 2015, 2016 IBM Corporation
> + *
> + * Author: Stefan Berger <stefanb@us.ibm.com>
> + *
> + * Maintained by: <tpmdd-devel@lists.sourceforge.net>
> + *
> + * Device driver for vTPM (vTPM proxy driver)
> + *
> + * 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/types.h>
> +#include <linux/spinlock.h>
> +#include <linux/uaccess.h>
> +#include <linux/wait.h>
> +#include <linux/miscdevice.h>
> +#include <linux/vtpm_proxy.h>
> +#include <linux/file.h>
> +#include <linux/anon_inodes.h>
> +#include <linux/poll.h>
> +#include <linux/compat.h>
> +
> +#include "tpm.h"
> +
> +#define VTPM_PROXY_REQ_COMPLETE_FLAG BIT(0)
> +
> +struct proxy_dev {
> + struct tpm_chip *chip;
> +
> + u32 flags; /* public API flags */
> +
> + wait_queue_head_t wq;
> +
> + struct mutex buf_lock; /* protect buffer and flags */
> +
> + long state; /* internal state */
> +#define STATE_OPENED_FLAG BIT(0)
> +#define STATE_WAIT_RESPONSE_FLAG BIT(1) /* waiting for emulator response */
> +
> + size_t req_len; /* length of queued TPM request */
> + size_t resp_len; /* length of queued TPM response */
> + u8 buffer[TPM_BUFSIZE]; /* request/response buffer */
> +
> + struct work_struct work; /* task that retrieves TPM timeouts */
> +};
> +
> +/* all supported flags */
> +#define VTPM_PROXY_FLAGS_ALL (VTPM_PROXY_FLAG_TPM2)
> +
> +static struct workqueue_struct *workqueue;
> +
> +static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev);
> +
> +/*
> + * Functions related to 'server side'
> + */
> +
> +/**
> + * vtpm_proxy_fops_read - Read TPM commands on 'server side'
> + *
> + * Return value:
> + * Number of bytes read or negative error code
> + */
> +static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
> + size_t count, loff_t *off)
> +{
> + struct proxy_dev *proxy_dev = filp->private_data;
> + size_t len;
> + int sig, rc;
> +
> + sig = wait_event_interruptible(proxy_dev->wq,
> + proxy_dev->req_len != 0 ||
> + !(proxy_dev->state & STATE_OPENED_FLAG));
> + if (sig)
> + return -EINTR;
> +
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + return -EPIPE;
> + }
> +
> + len = proxy_dev->req_len;
> +
> + if (count < len) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n",
> + count, len);
> + return -EIO;
> + }
> +
> + rc = copy_to_user(buf, proxy_dev->buffer, len);
> + memset(proxy_dev->buffer, 0, len);
> + proxy_dev->req_len = 0;
> +
> + if (!rc)
> + proxy_dev->state |= STATE_WAIT_RESPONSE_FLAG;
> +
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + if (rc)
> + return -EFAULT;
> +
> + return len;
> +}
> +
> +/**
> + * vtpm_proxy_fops_write - Write TPM responses on 'server side'
> + *
> + * Return value:
> + * Number of bytes read or negative error value
> + */
> +static ssize_t vtpm_proxy_fops_write(struct file *filp, const char __user *buf,
> + size_t count, loff_t *off)
> +{
> + struct proxy_dev *proxy_dev = filp->private_data;
> +
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + return -EPIPE;
> + }
> +
> + if (count > sizeof(proxy_dev->buffer) ||
> + !(proxy_dev->state & STATE_WAIT_RESPONSE_FLAG)) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + return -EIO;
> + }
> +
> + proxy_dev->state &= ~STATE_WAIT_RESPONSE_FLAG;
> +
> + proxy_dev->req_len = 0;
> +
> + if (copy_from_user(proxy_dev->buffer, buf, count)) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + return -EFAULT;
> + }
> +
> + proxy_dev->resp_len = count;
> +
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + wake_up_interruptible(&proxy_dev->wq);
> +
> + return count;
> +}
> +
> +/*
> + * vtpm_proxy_fops_poll: Poll status on 'server side'
> + *
> + * Return value:
> + * Poll flags
> + */
> +static unsigned int vtpm_proxy_fops_poll(struct file *filp, poll_table *wait)
> +{
> + struct proxy_dev *proxy_dev = filp->private_data;
> + unsigned ret;
> +
> + poll_wait(filp, &proxy_dev->wq, wait);
> +
> + ret = POLLOUT;
> +
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + if (proxy_dev->req_len)
> + ret |= POLLIN | POLLRDNORM;
> +
> + if (!(proxy_dev->state & STATE_OPENED_FLAG))
> + ret |= POLLHUP;
> +
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + return ret;
> +}
> +
> +/*
> + * vtpm_proxy_fops_open - Open vTPM device on 'server side'
> + *
> + * Called when setting up the anonymous file descriptor
> + */
> +static void vtpm_proxy_fops_open(struct file *filp)
> +{
> + struct proxy_dev *proxy_dev = filp->private_data;
> +
> + proxy_dev->state |= STATE_OPENED_FLAG;
> +}
> +
> +/**
> + * vtpm_proxy_fops_undo_open - counter-part to vtpm_fops_open
> + *
> + * Call to undo vtpm_proxy_fops_open
> + */
> +static void vtpm_proxy_fops_undo_open(struct proxy_dev *proxy_dev)
> +{
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + proxy_dev->state &= ~STATE_OPENED_FLAG;
> +
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + /* no more TPM responses -- wake up anyone waiting for them */
> + wake_up_interruptible(&proxy_dev->wq);
> +}
> +
> +/*
> + * vtpm_proxy_fops_release: Close 'server side'
> + *
> + * Return value:
> + * Always returns 0.
> + */
> +static int vtpm_proxy_fops_release(struct inode *inode, struct file *filp)
> +{
> + struct proxy_dev *proxy_dev = filp->private_data;
> +
> + filp->private_data = NULL;
> +
> + vtpm_proxy_delete_device(proxy_dev);
> +
> + return 0;
> +}
> +
> +static const struct file_operations vtpm_proxy_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_llseek,
> + .read = vtpm_proxy_fops_read,
> + .write = vtpm_proxy_fops_write,
> + .poll = vtpm_proxy_fops_poll,
> + .release = vtpm_proxy_fops_release,
> +};
> +
> +/*
> + * Functions invoked by the core TPM driver to send TPM commands to
> + * 'server side' and receive responses from there.
> + */
> +
> +/*
> + * Called when core TPM driver reads TPM responses from 'server side'
> + *
> + * Return value:
> + * Number of TPM response bytes read, negative error value otherwise
> + */
> +static int vtpm_proxy_tpm_op_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> + struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
> + size_t len;
> +
> + /* process gone ? */
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + return -EPIPE;
> + }
> +
> + len = proxy_dev->resp_len;
> + if (count < len) {
> + dev_err(&chip->dev,
> + "Invalid size in recv: count=%zd, resp_len=%zd\n",
> + count, len);
> + len = -EIO;
> + goto out;
> + }
> +
> + memcpy(buf, proxy_dev->buffer, len);
> + proxy_dev->resp_len = 0;
> +
> +out:
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + return len;
> +}
> +
> +/*
> + * Called when core TPM driver forwards TPM requests to 'server side'.
> + *
> + * Return value:
> + * 0 in case of success, negative error value otherwise.
> + */
> +static int vtpm_proxy_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> + struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
> + int rc = 0;
> +
> + if (count > sizeof(proxy_dev->buffer)) {
> + dev_err(&chip->dev,
> + "Invalid size in send: count=%zd, buffer size=%zd\n",
> + count, sizeof(proxy_dev->buffer));
> + return -EIO;
> + }
> +
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + if (!(proxy_dev->state & STATE_OPENED_FLAG)) {
> + mutex_unlock(&proxy_dev->buf_lock);
> + return -EPIPE;
> + }
> +
> + proxy_dev->resp_len = 0;
> +
> + proxy_dev->req_len = count;
> + memcpy(proxy_dev->buffer, buf, count);
> +
> + proxy_dev->state &= ~STATE_WAIT_RESPONSE_FLAG;
> +
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + wake_up_interruptible(&proxy_dev->wq);
> +
> + return rc;
> +}
> +
> +static void vtpm_proxy_tpm_op_cancel(struct tpm_chip *chip)
> +{
> + /* not supported */
> +}
> +
> +static u8 vtpm_proxy_tpm_op_status(struct tpm_chip *chip)
> +{
> + struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
> +
> + if (proxy_dev->resp_len)
> + return VTPM_PROXY_REQ_COMPLETE_FLAG;
> +
> + return 0;
> +}
> +
> +static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip *chip, u8 status)
> +{
> + struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
> + bool ret;
> +
> + mutex_lock(&proxy_dev->buf_lock);
> +
> + ret = !(proxy_dev->state & STATE_OPENED_FLAG);
> +
> + mutex_unlock(&proxy_dev->buf_lock);
> +
> + return ret;
> +}
> +
> +static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
> + .recv = vtpm_proxy_tpm_op_recv,
> + .send = vtpm_proxy_tpm_op_send,
> + .cancel = vtpm_proxy_tpm_op_cancel,
> + .status = vtpm_proxy_tpm_op_status,
> + .req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
> + .req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
> + .req_canceled = vtpm_proxy_tpm_req_canceled,
> +};
> +
> +/*
> + * Code related to the startup of the TPM 2 and startup of TPM 1.2 +
> + * retrieval of timeouts and durations.
> + */
> +
> +static void vtpm_proxy_work(struct work_struct *work)
> +{
> + struct proxy_dev *proxy_dev = container_of(work, struct proxy_dev,
> + work);
> + int rc;
> +
> + if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
> + rc = tpm2_startup(proxy_dev->chip, TPM2_SU_CLEAR);
> + else
> + rc = tpm_get_timeouts(proxy_dev->chip);
> +
> + if (rc)
> + goto err;
> +
> + rc = tpm_chip_register(proxy_dev->chip);
> + if (rc)
> + goto err;
> +
> + return;
> +
> +err:
> + vtpm_proxy_fops_undo_open(proxy_dev);
> +}
> +
> +/*
> + * vtpm_proxy_work_stop: make sure the work has finished
> + *
> + * This function is useful when user space closed the fd
> + * while the driver still determines timeouts.
> + */
> +static void vtpm_proxy_work_stop(struct proxy_dev *proxy_dev)
> +{
> + vtpm_proxy_fops_undo_open(proxy_dev);
> + flush_work(&proxy_dev->work);
> +}
> +
> +/*
> + * vtpm_proxy_work_start: Schedule the work for TPM 1.2 & 2 initialization
> + */
> +static inline void vtpm_proxy_work_start(struct proxy_dev *proxy_dev)
> +{
> + queue_work(workqueue, &proxy_dev->work);
> +}
> +
> +/*
> + * Code related to creation and deletion of device pairs
> + */
> +static struct proxy_dev *vtpm_proxy_create_proxy_dev(void)
> +{
> + struct proxy_dev *proxy_dev;
> + struct tpm_chip *chip;
> + int err;
> +
> + proxy_dev = kzalloc(sizeof(*proxy_dev), GFP_KERNEL);
> + if (proxy_dev == NULL)
> + return ERR_PTR(-ENOMEM);
> +
> + init_waitqueue_head(&proxy_dev->wq);
> + mutex_init(&proxy_dev->buf_lock);
> + INIT_WORK(&proxy_dev->work, vtpm_proxy_work);
> +
> + chip = tpm_chip_alloc(NULL, &vtpm_proxy_tpm_ops);
> + if (IS_ERR(chip)) {
> + err = PTR_ERR(chip);
> + goto err_proxy_dev_free;
> + }
> + dev_set_drvdata(&chip->dev, proxy_dev);
> +
> + proxy_dev->chip = chip;
> +
> + return proxy_dev;
> +
> +err_proxy_dev_free:
> + kfree(proxy_dev);
> +
> + return ERR_PTR(err);
> +}
> +
> +/*
> + * Undo what has been done in vtpm_create_proxy_dev
> + */
> +static inline void vtpm_proxy_delete_proxy_dev(struct proxy_dev *proxy_dev)
> +{
> + put_device(&proxy_dev->chip->dev); /* frees chip */
> + kfree(proxy_dev);
> +}
> +
> +/*
> + * Create a /dev/tpm%d and 'server side' file descriptor pair
> + *
> + * Return value:
> + * Returns file pointer on success, an error value otherwise
> + */
> +static struct file *vtpm_proxy_create_device(
> + struct vtpm_proxy_new_dev *vtpm_new_dev)
> +{
> + struct proxy_dev *proxy_dev;
> + int rc, fd;
> + struct file *file;
> +
> + if (vtpm_new_dev->flags & ~VTPM_PROXY_FLAGS_ALL)
> + return ERR_PTR(-EOPNOTSUPP);
> +
> + proxy_dev = vtpm_proxy_create_proxy_dev();
> + if (IS_ERR(proxy_dev))
> + return ERR_CAST(proxy_dev);
> +
> + proxy_dev->flags = vtpm_new_dev->flags;
> +
> + /* setup an anonymous file for the server-side */
> + fd = get_unused_fd_flags(O_RDWR);
> + if (fd < 0) {
> + rc = fd;
> + goto err_delete_proxy_dev;
> + }
> +
> + file = anon_inode_getfile("[vtpms]", &vtpm_proxy_fops, proxy_dev,
> + O_RDWR);
> + if (IS_ERR(file)) {
> + rc = PTR_ERR(file);
> + goto err_put_unused_fd;
> + }
> +
> + /* from now on we can unwind with put_unused_fd() + fput() */
> + /* simulate an open() on the server side */
> + vtpm_proxy_fops_open(file);
> +
> + if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
> + proxy_dev->chip->flags |= TPM_CHIP_FLAG_TPM2;
> +
> + vtpm_proxy_work_start(proxy_dev);
> +
> + vtpm_new_dev->fd = fd;
> + vtpm_new_dev->major = MAJOR(proxy_dev->chip->dev.devt);
> + vtpm_new_dev->minor = MINOR(proxy_dev->chip->dev.devt);
> + vtpm_new_dev->tpm_num = proxy_dev->chip->dev_num;
> +
> + return file;
> +
> +err_put_unused_fd:
> + put_unused_fd(fd);
> +
> +err_delete_proxy_dev:
> + vtpm_proxy_delete_proxy_dev(proxy_dev);
> +
> + return ERR_PTR(rc);
> +}
> +
> +/*
> + * Counter part to vtpm_create_device.
> + */
> +static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev)
> +{
> + vtpm_proxy_work_stop(proxy_dev);
> +
> + /*
> + * A client may hold the 'ops' lock, so let it know that the server
> + * side shuts down before we try to grab the 'ops' lock when
> + * unregistering the chip.
> + */
> + vtpm_proxy_fops_undo_open(proxy_dev);
> +
> + tpm_chip_unregister(proxy_dev->chip);
> +
> + vtpm_proxy_delete_proxy_dev(proxy_dev);
> +}
> +
> +/*
> + * Code related to the control device /dev/vtpmx
> + */
> +
> +/*
> + * vtpmx_fops_ioctl: ioctl on /dev/vtpmx
> + *
> + * Return value:
> + * Returns 0 on success, a negative error code otherwise.
> + */
> +static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
> + unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> + struct vtpm_proxy_new_dev *vtpm_new_dev_p;
> + struct vtpm_proxy_new_dev vtpm_new_dev;
> + struct file *file;
> +
> + switch (ioctl) {
> + case VTPM_PROXY_IOC_NEW_DEV:
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> + vtpm_new_dev_p = argp;
> + if (copy_from_user(&vtpm_new_dev, vtpm_new_dev_p,
> + sizeof(vtpm_new_dev)))
> + return -EFAULT;
> + file = vtpm_proxy_create_device(&vtpm_new_dev);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
> + if (copy_to_user(vtpm_new_dev_p, &vtpm_new_dev,
> + sizeof(vtpm_new_dev))) {
> + put_unused_fd(vtpm_new_dev.fd);
> + fput(file);
> + return -EFAULT;
> + }
> +
> + fd_install(vtpm_new_dev.fd, file);
> + return 0;
> +
> + default:
> + return -ENOIOCTLCMD;
> + }
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long vtpmx_fops_compat_ioctl(struct file *f, unsigned int ioctl,
> + unsigned long arg)
> +{
> + return vtpmx_fops_ioctl(f, ioctl, (unsigned long)compat_ptr(arg));
> +}
> +#endif
> +
> +static const struct file_operations vtpmx_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = vtpmx_fops_ioctl,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl = vtpmx_fops_compat_ioctl,
> +#endif
> + .llseek = noop_llseek,
> +};
> +
> +static struct miscdevice vtpmx_miscdev = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = "vtpmx",
> + .fops = &vtpmx_fops,
> +};
> +
> +static int vtpmx_init(void)
> +{
> + return misc_register(&vtpmx_miscdev);
> +}
> +
> +static void vtpmx_cleanup(void)
> +{
> + misc_deregister(&vtpmx_miscdev);
> +}
> +
> +static int __init vtpm_module_init(void)
> +{
> + int rc;
> +
> + rc = vtpmx_init();
> + if (rc) {
> + pr_err("couldn't create vtpmx device\n");
> + return rc;
> + }
> +
> + workqueue = create_workqueue("tpm-vtpm");
> + if (!workqueue) {
> + pr_err("couldn't create workqueue\n");
> + rc = -ENOMEM;
> + goto err_vtpmx_cleanup;
> + }
> +
> + return 0;
> +
> +err_vtpmx_cleanup:
> + vtpmx_cleanup();
> +
> + return rc;
> +}
> +
> +static void __exit vtpm_module_exit(void)
> +{
> + destroy_workqueue(workqueue);
> + vtpmx_cleanup();
> +}
> +
> +module_init(vtpm_module_init);
> +module_exit(vtpm_module_exit);
> +
> +MODULE_AUTHOR("Stefan Berger (stefanb@us.ibm.com)");
> +MODULE_DESCRIPTION("vTPM Driver");
> +MODULE_VERSION("0.1");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index ebd10e6..ac767ce 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -449,6 +449,7 @@ header-y += virtio_scsi.h
> header-y += virtio_types.h
> header-y += vm_sockets.h
> header-y += vt.h
> +header-y += vtpm_proxy.h
> header-y += wait.h
> header-y += wanrouter.h
> header-y += watchdog.h
> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> new file mode 100644
> index 0000000..41e8e22
> --- /dev/null
> +++ b/include/uapi/linux/vtpm_proxy.h
> @@ -0,0 +1,36 @@
> +/*
> + * Definitions for the VTPM proxy driver
> + * Copyright (c) 2015, 2016, IBM Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef _UAPI_LINUX_VTPM_PROXY_H
> +#define _UAPI_LINUX_VTPM_PROXY_H
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +
> +/* ioctls */
> +
> +struct vtpm_proxy_new_dev {
> + __u32 flags; /* input */
> + __u32 tpm_num; /* output */
> + __u32 fd; /* output */
> + __u32 major; /* output */
> + __u32 minor; /* output */
> +};
> +
> +/* above flags */
> +#define VTPM_PROXY_FLAG_TPM2 1 /* emulator is TPM 2 */
> +
> +#define VTPM_PROXY_IOC_NEW_DEV _IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
> +
> +#endif /* _UAPI_LINUX_VTPM_PROXY_H */
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 4/4] tpm: Add documentation for the tpm_vtpm_proxy device driver
2016-04-18 17:26 ` [PATCH v11 4/4] tpm: Add documentation for the tpm_vtpm_proxy device driver Stefan Berger
@ 2016-04-22 16:47 ` Jarkko Sakkinen
0 siblings, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-22 16:47 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel,
linux-doc, linux-api
On Mon, Apr 18, 2016 at 01:26:16PM -0400, Stefan Berger wrote:
> Add documentation for the tpm_vtpm device driver that implements
> support for providing TPM functionality to Linux containers.
>
> Parts of this documentation were recycled from the Xen vTPM
> device driver documentation.
>
> Update the documentation for the ioctl numbers.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> CC: linux-kernel@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> CC: linux-api@vger.kernel.org
> ---
> Documentation/ioctl/ioctl-number.txt | 1 +
> Documentation/tpm/tpm_vtpm_proxy.txt | 71 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 Documentation/tpm/tpm_vtpm_proxy.txt
>
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index 91261a3..7dbec90 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -303,6 +303,7 @@ Code Seq#(hex) Include File Comments
> <mailto:buk@buks.ipn.de>
> 0xA0 all linux/sdp/sdp.h Industrial Device Project
> <mailto:kenji@bitgate.com>
> +0xA1 0 linux/vtpm_proxy.h TPM Emulator Proxy Driver
> 0xA2 00-0F arch/tile/include/asm/hardwall.h
> 0xA3 80-8F Port ACL in development:
> <mailto:tlewis@mindspring.com>
> diff --git a/Documentation/tpm/tpm_vtpm_proxy.txt b/Documentation/tpm/tpm_vtpm_proxy.txt
> new file mode 100644
> index 0000000..30d1902
> --- /dev/null
> +++ b/Documentation/tpm/tpm_vtpm_proxy.txt
> @@ -0,0 +1,71 @@
> +Virtual TPM Proxy Driver for Linux Containers
> +
> +Authors: Stefan Berger (IBM)
> +
> +This document describes the virtual Trusted Platform Module (vTPM)
> +proxy device driver for Linux containers.
> +
> +INTRODUCTION
> +------------
> +
> +The goal of this work is to provide TPM functionality to each Linux
> +container. This allows programs to interact with a TPM in a container
> +the same way they interact with a TPM on the physical system. Each
> +container gets its own unique, emulated, software TPM.
> +
> +
> +DESIGN
> +------
> +
> +To make an emulated software TPM available to each container, the container
> +management stack needs to create a device pair consisting of a client TPM
> +character device /dev/tpmX (with X=0,1,2...) and a 'server side' file
> +descriptor. The former is moved into the container by creating a character
> +device with the appropriate major and minor numbers while the file descriptor
> +is passed to the TPM emulator. Software inside the container can then send
> +TPM commands using the character device and the emulator will receive the
> +commands via the file descriptor and use it for sending back responses.
> +
> +To support this, the virtual TPM proxy driver provides a device /dev/vtpmx
> +that is used to create device pairs using an ioctl. The ioctl takes as
> +an input flags for configuring the device. The flags for example indicate
> +whether TPM 1.2 or TPM 2 functionality is supported by the TPM emulator.
> +The result of the ioctl are the file descriptor for the 'server side'
> +as well as the major and minor numbers of the character device that was created.
> +Besides that the number of the TPM character device is return. If for
> +example /dev/tpm10 was created, the number (dev_num) 10 is returned.
> +
> +The following is the data structure of the TPM_PROXY_IOC_NEW_DEV ioctl:
> +
> +struct vtpm_proxy_new_dev {
> + __u32 flags; /* input */
> + __u32 tpm_num; /* output */
> + __u32 fd; /* output */
> + __u32 major; /* output */
> + __u32 minor; /* output */
> +};
> +
> +Note that if unsupported flags are passed to the device driver, the ioctl will
> +fail and errno will be set to EOPNOTSUPP. Similarly, if an unsupported ioctl is
> +called on the device driver, the ioctl will fail and errno will be set to
> +ENOTTY.
> +
> +See /usr/include/linux/vtpm_proxy.h for definitions related to the public interface
> +of this vTPM device driver.
> +
> +Once the device has been created, the driver will immediately try to talk
> +to the TPM. All commands from the driver can be read from the file descriptor
> +returned by the ioctl. The commands should be responded to immediately.
> +
> +Depending on the version of TPM the following commands will be sent by the
> +driver:
> +
> +- TPM 1.2:
> + - the driver will send a TPM_Startup command to the TPM emulator
> + - the driver will send commands to read the command durations and
> + interface timeouts from the TPM emulator
> +- TPM 2:
> + - the driver will send a TPM2_Startup command to the TPM emulator
> +
> +The TPM device /dev/tpmX will only appear if all of the relevant commands
> +were responded to properly.
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 0/4] Multi-instance vTPM proxy driver
2016-04-18 17:26 [PATCH v11 0/4] Multi-instance vTPM proxy driver Stefan Berger
` (3 preceding siblings ...)
2016-04-18 17:26 ` [PATCH v11 4/4] tpm: Add documentation for the tpm_vtpm_proxy device driver Stefan Berger
@ 2016-04-22 16:54 ` Jarkko Sakkinen
2016-04-26 9:28 ` [tpmdd-devel] " Jarkko Sakkinen
4 siblings, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-22 16:54 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On Mon, Apr 18, 2016 at 01:26:12PM -0400, Stefan Berger wrote:
> The following series of patches implements a multi-instance vTPM
> proxy driver that can dynamically create TPM 'server' and client device
> pairs.
>
> Using an ioctl on the provided /dev/vtpmx, a client-side vTPM device
> and a server side file descriptor is created. The file descriptor must
> be passed to a TPM emulator. The device driver will initialize the
> emulated TPM using TPM 1.2 or TPM 2 startup commands and it will read
> the command durations from the device in case of a TPM 1.2. The choice
> of emulated TPM device (1.2 or 2) must be provided with a flag in
> the ioctl.
>
> The patches are based on the tip of a recent checkout of Jarkko's tree
> (master branch).
I've applied this series to my master branch. These patches have been
baking long enough in my opinion that they could be merged to next and
therefore find their way into Linux 4.7
Stefan, I would still require your Tested-by for the first patch before
merging because it is not as mature as other patches are.
/Jarkko
> Stefan
>
> v10->v11:
> - patch 3: merge of v10 patches 3 and 4
> - patch 3: removed unnecessary NULL pointer checks
> - Added Jason's Reviewed-by's
>
> v9->v10:
> - add new patch 1 with sysfs related rework; reworked patch 2 as a consequence
> - patch 3: fixed ioctl from using _IOW to using _IOWR (reinstall headers
> and rebuild test tools)
> - patch 3: do not use priv field anymore but uses dev_set_drvdata /
> dev_get_drvdata instead now
>
> v8->v9:
> - move constant from public header into tpm_vtpm_proxy.c
> - Replaced VTPM_PROXY_MAGIC in ioctl definition with its value (0xa1)
> - Check for the STATE_OPEN_FLAG in wait_event_interruptable and after
> returning from it
>
> v7->v8:
> - minor tweaks on the documentation
> - Reordered function calls in the VTPM proxy driver's server side release
> function so that a client holding the 'ops' lock releases it before the
> driver tries to grab the lock when unregistering the device.
>
> v6->v7:
> - Adjusted name of driver to tpm_vtpm_proxy from tpm_vtpm. Adjust function
> names, names of structures, and names of constants.
> - Adjusted IOCTL to use magic 0xa1 rather than the completely used 0xa0.
> - Extended driver documentation and added documentation of ioctl.
> - Moved test program to own project (dropped patch 11).
>
> v5->v6:
> - Adapted errno's for unsupported flags and ioctls following Jason's
> comments
>
> v4->v5:
> - Introduced different error codes for unsupported flags and ioctls
> - Added documentation patch
>
>
> Jason Gunthorpe (1):
> tpm: Remove all uses of drvdata from the TPM Core
>
> Stefan Berger (3):
> tpm: Introduce TPM_CHIP_FLAG_VIRTUAL
> tpm: Proxy driver for supporting multiple emulated TPMs
> tpm: Add documentation for the tpm_vtpm_proxy device driver
>
> Documentation/ioctl/ioctl-number.txt | 1 +
> Documentation/tpm/tpm_vtpm_proxy.txt | 71 ++++
> drivers/char/tpm/Kconfig | 10 +
> drivers/char/tpm/Makefile | 1 +
> drivers/char/tpm/tpm-chip.c | 76 +++--
> drivers/char/tpm/tpm-interface.c | 7 +-
> drivers/char/tpm/tpm-sysfs.c | 61 ++--
> drivers/char/tpm/tpm.h | 11 +-
> drivers/char/tpm/tpm_vtpm_proxy.c | 644 +++++++++++++++++++++++++++++++++++
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/vtpm_proxy.h | 36 ++
> 11 files changed, 852 insertions(+), 67 deletions(-)
> create mode 100644 Documentation/tpm/tpm_vtpm_proxy.txt
> create mode 100644 drivers/char/tpm/tpm_vtpm_proxy.c
> create mode 100644 include/uapi/linux/vtpm_proxy.h
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
2016-04-22 15:06 ` Jarkko Sakkinen
@ 2016-04-25 19:13 ` Stefan Berger
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Berger @ 2016-04-25 19:13 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel, jgunthorpe, linux-security-module, linux-kernel
On 04/22/2016 11:06 AM, Jarkko Sakkinen wrote:
> On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
>> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>
>> The final thing preventing this was the way the sysfs files were
>> attached to the pdev. Follow the approach developed for ppi and move
>> the sysfs files to the chip->dev with symlinks from the pdev
>> for compatibility. Everything in the core now sanely uses container_of
>> to get the chip.
>>
>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> Three configurations:
>
> * Haswell NUC with PTT (tpm_crb)
> * Another NUC with dTPM 2.0 chip
> * Dell E6420, which has TPM 1.2 chip
>
> Things seem to be unbroken.
>
> Stefan, have you verified that sysfs attributes work through routes:
>
> 1. From char device sysfs directory
> 2. Through link
Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> An also tried insmod/rmmod couple of rounds?
Works as-is in polling mode but requires Jason's patch "tpm: Fix IRQ
unwind ordering in TIS" for interrupt mode to not throw errors on 'rmmod'.
Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [PATCH v11 0/4] Multi-instance vTPM proxy driver
2016-04-22 16:54 ` [PATCH v11 0/4] Multi-instance vTPM proxy driver Jarkko Sakkinen
@ 2016-04-26 9:28 ` Jarkko Sakkinen
2016-04-26 11:30 ` Stefan Berger
0 siblings, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-26 9:28 UTC (permalink / raw)
To: Stefan Berger; +Cc: linux-security-module, tpmdd-devel, linux-kernel
On Fri, Apr 22, 2016 at 07:54:27PM +0300, Jarkko Sakkinen wrote:
> On Mon, Apr 18, 2016 at 01:26:12PM -0400, Stefan Berger wrote:
> > The following series of patches implements a multi-instance vTPM
> > proxy driver that can dynamically create TPM 'server' and client device
> > pairs.
> >
> > Using an ioctl on the provided /dev/vtpmx, a client-side vTPM device
> > and a server side file descriptor is created. The file descriptor must
> > be passed to a TPM emulator. The device driver will initialize the
> > emulated TPM using TPM 1.2 or TPM 2 startup commands and it will read
> > the command durations from the device in case of a TPM 1.2. The choice
> > of emulated TPM device (1.2 or 2) must be provided with a flag in
> > the ioctl.
> >
> > The patches are based on the tip of a recent checkout of Jarkko's tree
> > (master branch).
>
> I've applied this series to my master branch. These patches have been
> baking long enough in my opinion that they could be merged to next and
> therefore find their way into Linux 4.7
>
> Stefan, I would still require your Tested-by for the first patch before
> merging because it is not as mature as other patches are.
Now I have augmented Tested-by's to relevant patches and added my
Signed-off-by's. I plan to merge this to the next branch tomorrow.
> /Jarkko
/Jarkko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [PATCH v11 0/4] Multi-instance vTPM proxy driver
2016-04-26 9:28 ` [tpmdd-devel] " Jarkko Sakkinen
@ 2016-04-26 11:30 ` Stefan Berger
2016-04-27 12:56 ` Jarkko Sakkinen
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Berger @ 2016-04-26 11:30 UTC (permalink / raw)
To: Jarkko Sakkinen; +Cc: linux-security-module, tpmdd-devel, linux-kernel
On 04/26/2016 05:28 AM, Jarkko Sakkinen wrote:
> On Fri, Apr 22, 2016 at 07:54:27PM +0300, Jarkko Sakkinen wrote:
>> On Mon, Apr 18, 2016 at 01:26:12PM -0400, Stefan Berger wrote:
>>> The following series of patches implements a multi-instance vTPM
>>> proxy driver that can dynamically create TPM 'server' and client device
>>> pairs.
>>>
>>> Using an ioctl on the provided /dev/vtpmx, a client-side vTPM device
>>> and a server side file descriptor is created. The file descriptor must
>>> be passed to a TPM emulator. The device driver will initialize the
>>> emulated TPM using TPM 1.2 or TPM 2 startup commands and it will read
>>> the command durations from the device in case of a TPM 1.2. The choice
>>> of emulated TPM device (1.2 or 2) must be provided with a flag in
>>> the ioctl.
>>>
>>> The patches are based on the tip of a recent checkout of Jarkko's tree
>>> (master branch).
>> I've applied this series to my master branch. These patches have been
>> baking long enough in my opinion that they could be merged to next and
>> therefore find their way into Linux 4.7
>>
>> Stefan, I would still require your Tested-by for the first patch before
>> merging because it is not as mature as other patches are.
> Now I have augmented Tested-by's to relevant patches and added my
> Signed-off-by's. I plan to merge this to the next branch tomorrow.
Please don't forget Jason's patch for tpm_tis.c that fixes the device
used in the devm_request_irq/devm_free_irq calls. The rmmod of tpm_tis
only work correctly in interrupt mode when that patch is applied.
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [PATCH v11 0/4] Multi-instance vTPM proxy driver
2016-04-26 11:30 ` Stefan Berger
@ 2016-04-27 12:56 ` Jarkko Sakkinen
2016-04-27 16:27 ` Stefan Berger
0 siblings, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-04-27 12:56 UTC (permalink / raw)
To: Stefan Berger; +Cc: linux-security-module, tpmdd-devel, linux-kernel
On Tue, Apr 26, 2016 at 07:30:26AM -0400, Stefan Berger wrote:
> On 04/26/2016 05:28 AM, Jarkko Sakkinen wrote:
> >On Fri, Apr 22, 2016 at 07:54:27PM +0300, Jarkko Sakkinen wrote:
> >>On Mon, Apr 18, 2016 at 01:26:12PM -0400, Stefan Berger wrote:
> >>>The following series of patches implements a multi-instance vTPM
> >>>proxy driver that can dynamically create TPM 'server' and client device
> >>>pairs.
> >>>
> >>>Using an ioctl on the provided /dev/vtpmx, a client-side vTPM device
> >>>and a server side file descriptor is created. The file descriptor must
> >>>be passed to a TPM emulator. The device driver will initialize the
> >>>emulated TPM using TPM 1.2 or TPM 2 startup commands and it will read
> >>>the command durations from the device in case of a TPM 1.2. The choice
> >>>of emulated TPM device (1.2 or 2) must be provided with a flag in
> >>>the ioctl.
> >>>
> >>>The patches are based on the tip of a recent checkout of Jarkko's tree
> >>>(master branch).
> >>I've applied this series to my master branch. These patches have been
> >>baking long enough in my opinion that they could be merged to next and
> >>therefore find their way into Linux 4.7
> >>
> >>Stefan, I would still require your Tested-by for the first patch before
> >>merging because it is not as mature as other patches are.
> >Now I have augmented Tested-by's to relevant patches and added my
> >Signed-off-by's. I plan to merge this to the next branch tomorrow.
>
> Please don't forget Jason's patch for tpm_tis.c that fixes the device used
> in the devm_request_irq/devm_free_irq calls. The rmmod of tpm_tis only work
> correctly in interrupt mode when that patch is applied.
It has been merged already earlier to the next branch. I've now merged
these patches to the next branch.
> Stefan
/Jarkko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [PATCH v11 0/4] Multi-instance vTPM proxy driver
2016-04-27 12:56 ` Jarkko Sakkinen
@ 2016-04-27 16:27 ` Stefan Berger
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Berger @ 2016-04-27 16:27 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-security-module, tpmdd-devel, linux-kernel, Jason Gunthorpe
On 04/27/2016 08:56 AM, Jarkko Sakkinen wrote:
> On Tue, Apr 26, 2016 at 07:30:26AM -0400, Stefan Berger wrote:
>> On 04/26/2016 05:28 AM, Jarkko Sakkinen wrote:
>>> On Fri, Apr 22, 2016 at 07:54:27PM +0300, Jarkko Sakkinen wrote:
>>>> On Mon, Apr 18, 2016 at 01:26:12PM -0400, Stefan Berger wrote:
>>>>> The following series of patches implements a multi-instance vTPM
>>>>> proxy driver that can dynamically create TPM 'server' and client device
>>>>> pairs.
>>>>>
>>>>> Using an ioctl on the provided /dev/vtpmx, a client-side vTPM device
>>>>> and a server side file descriptor is created. The file descriptor must
>>>>> be passed to a TPM emulator. The device driver will initialize the
>>>>> emulated TPM using TPM 1.2 or TPM 2 startup commands and it will read
>>>>> the command durations from the device in case of a TPM 1.2. The choice
>>>>> of emulated TPM device (1.2 or 2) must be provided with a flag in
>>>>> the ioctl.
>>>>>
>>>>> The patches are based on the tip of a recent checkout of Jarkko's tree
>>>>> (master branch).
>>>> I've applied this series to my master branch. These patches have been
>>>> baking long enough in my opinion that they could be merged to next and
>>>> therefore find their way into Linux 4.7
>>>>
>>>> Stefan, I would still require your Tested-by for the first patch before
>>>> merging because it is not as mature as other patches are.
>>> Now I have augmented Tested-by's to relevant patches and added my
>>> Signed-off-by's. I plan to merge this to the next branch tomorrow.
>> Please don't forget Jason's patch for tpm_tis.c that fixes the device used
>> in the devm_request_irq/devm_free_irq calls. The rmmod of tpm_tis only work
>> correctly in interrupt mode when that patch is applied.
> It has been merged already earlier to the next branch. I've now merged
> these patches to the next branch.
Please check the devm_request_irq() call whether it still uses the wrong
&chip->dev or rather chip->dev.parent.
Stefan
>
>> Stefan
> /Jarkko
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [BUG, bisect] tpm: Remove all uses of drvdata from the TPM Core
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
2016-04-19 10:12 ` Jarkko Sakkinen
2016-04-22 15:06 ` Jarkko Sakkinen
@ 2016-05-10 13:55 ` Jeremiah Mahler
2016-05-10 14:38 ` Jarkko Sakkinen
[not found] ` <201605101505.u4AF5nHd032349@d03av04.boulder.ibm.com>
2 siblings, 2 replies; 25+ messages in thread
From: Jeremiah Mahler @ 2016-05-10 13:55 UTC (permalink / raw)
To: Stefan Berger
Cc: tpmdd-devel, jarkko.sakkinen, jgunthorpe, linux-security-module,
linux-kernel
all,
My machine is locking up during suspend and I have bisected the
problem to this patch (e89f8b1ade9cc1a) in the latest linux-next
(20160506) kernel.
Let me know if I can test patches or do anything else to help.
On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>
> The final thing preventing this was the way the sysfs files were
> attached to the pdev. Follow the approach developed for ppi and move
> the sysfs files to the chip->dev with symlinks from the pdev
> for compatibility. Everything in the core now sanely uses container_of
> to get the chip.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
> drivers/char/tpm/tpm-interface.c | 7 ++--
> drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
> drivers/char/tpm/tpm.h | 10 +++---
> 4 files changed, 84 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5bc530c..7e2c9cf 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
> chip->dev.class = tpm_class;
> chip->dev.release = tpm_dev_release;
> chip->dev.parent = dev;
> -#ifdef CONFIG_ACPI
> chip->dev.groups = chip->groups;
> -#endif
>
> if (chip->dev_num == 0)
> chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>
> static int tpm1_chip_register(struct tpm_chip *chip)
> {
> - int rc;
> -
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> return 0;
>
> - rc = tpm_sysfs_add_device(chip);
> - if (rc)
> - return rc;
> + tpm_sysfs_add_device(chip);
>
> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>
> @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>
> if (chip->bios_dir)
> tpm_bios_log_teardown(chip->bios_dir);
> +}
> +
> +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return;
>
> - tpm_sysfs_del_device(chip);
> + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> +
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> }
>
> +/* For compatibility with legacy sysfs paths we provide symlinks from the
> + * parent dev directory to selected names within the tpm chip directory. Old
> + * kernel versions created these files directly under the parent.
> + */
> +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> + int rc;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return 0;
> +
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> + if (rc && rc != -ENOENT)
> + return rc;
> +
> + /* All the names from tpm-sysfs */
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> + if (rc) {
> + tpm_del_legacy_sysfs(chip);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> /*
> * tpm_chip_register() - create a character device for the TPM chip
> * @chip: TPM chip to use.
> @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> tpm_add_ppi(chip);
>
> rc = tpm_add_char_device(chip);
> - if (rc)
> - goto out_err;
> + if (rc) {
> + tpm1_chip_unregister(chip);
> + return rc;
> + }
>
> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> - rc = __compat_only_sysfs_link_entry_to_kobj(
> - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> - if (rc && rc != -ENOENT) {
> - tpm_chip_unregister(chip);
> - return rc;
> - }
> + rc = tpm_add_legacy_sysfs(chip);
> + if (rc) {
> + tpm_chip_unregister(chip);
> + return rc;
> }
>
> return 0;
> -out_err:
> - tpm1_chip_unregister(chip);
> - return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_chip_register);
>
> @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> return;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> - sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> + tpm_del_legacy_sysfs(chip);
>
> tpm1_chip_unregister(chip);
> tpm_del_char_device(chip);
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 7cba092..080dade 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
> .ordinal = TPM_ORD_GET_CAP
> };
>
> -ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> const char *desc)
> {
> struct tpm_cmd_t tpm_cmd;
> int rc;
> - struct tpm_chip *chip = dev_get_drvdata(dev);
>
> tpm_cmd.header.in = tpm_getcap_header;
> if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
> @@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
> */
> int tpm_pm_suspend(struct device *dev)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
> struct tpm_cmd_t cmd;
> int rc, try;
>
> @@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
> */
> int tpm_pm_resume(struct device *dev)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> if (chip == NULL)
> return -ENODEV;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index a7c3473..b46cf70 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> int i, rc;
> char *str = buf;
>
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> tpm_cmd.header.in = tpm_readpubek_header;
> err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> @@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> ssize_t rc;
> int i, j, num_pcrs;
> char *str = buf;
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
> + rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
> "attempting to determine the number of PCRS");
> if (rc)
> return 0;
> @@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> - "attempting to determine the permanent enabled state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> + "attempting to determine the permanent enabled state");
> if (rc)
> return 0;
>
> @@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> - "attempting to determine the permanent active state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> + "attempting to determine the permanent active state");
> if (rc)
> return 0;
>
> @@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
> - "attempting to determine the owner state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
> + "attempting to determine the owner state");
> if (rc)
> return 0;
>
> @@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
> cap_t cap;
> ssize_t rc;
>
> - rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
> - "attempting to determine the temporary state");
> + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
> + "attempting to determine the temporary state");
> if (rc)
> return 0;
>
> @@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
> static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> + struct tpm_chip *chip = to_tpm_chip(dev);
> cap_t cap;
> ssize_t rc;
> char *str = buf;
>
> - rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
> + rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
> "attempting to determine the manufacturer");
> if (rc)
> return 0;
> @@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> be32_to_cpu(cap.manufacturer_id));
>
> /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
> - rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
> - "attempting to determine the 1.2 version");
> + rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
> + "attempting to determine the 1.2 version");
> if (!rc) {
> str += sprintf(str,
> "TCG version: %d.%d\nFirmware version: %d.%d\n",
> @@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> cap.tpm_version_1_2.revMinor);
> } else {
> /* Otherwise just use TPM_STRUCT_VER */
> - rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
> + rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
> "attempting to determine the 1.1 version");
> if (rc)
> return 0;
> @@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
> 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);
> + struct tpm_chip *chip = to_tpm_chip(dev);
> if (chip == NULL)
> return 0;
>
> @@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
> static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> if (chip->duration[TPM_LONG] == 0)
> return 0;
> @@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
> static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> + struct tpm_chip *chip = to_tpm_chip(dev);
>
> return sprintf(buf, "%d %d %d %d [%s]\n",
> jiffies_to_usecs(chip->timeout_a),
> @@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
> .attrs = tpm_dev_attrs,
> };
>
> -int tpm_sysfs_add_device(struct tpm_chip *chip)
> +void tpm_sysfs_add_device(struct tpm_chip *chip)
> {
> - int err;
> - err = sysfs_create_group(&chip->dev.parent->kobj,
> - &tpm_dev_group);
> -
> - if (err)
> - dev_err(&chip->dev,
> - "failed to create sysfs attributes, %d\n", err);
> - return err;
> -}
> -
> -void tpm_sysfs_del_device(struct tpm_chip *chip)
> -{
> - /* The sysfs routines rely on an implicit tpm_try_get_ops, this
> - * function is called before ops is null'd and the sysfs core
> - * synchronizes this removal so that no callbacks are running or can
> - * run again
> + /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> + * is called before ops is null'd and the sysfs core synchronizes this
> + * removal so that no callbacks are running or can run again
> */
> - sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
> + WARN_ON(chip->groups_cnt != 0);
> + chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8bc6fb8..508e8e0 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -168,9 +168,9 @@ struct tpm_chip {
>
> struct dentry **bios_dir;
>
> -#ifdef CONFIG_ACPI
> - const struct attribute_group *groups[2];
> + const struct attribute_group *groups[3];
> unsigned int groups_cnt;
> +#ifdef CONFIG_ACPI
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> #endif /* CONFIG_ACPI */
> @@ -471,7 +471,8 @@ extern dev_t tpm_devt;
> extern const struct file_operations tpm_fops;
> extern struct idr dev_nums_idr;
>
> -ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> + const char *desc);
> 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,
> @@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
> extern int tpm_chip_register(struct tpm_chip *chip);
> extern void tpm_chip_unregister(struct tpm_chip *chip);
>
> -int tpm_sysfs_add_device(struct tpm_chip *chip);
> -void tpm_sysfs_del_device(struct tpm_chip *chip);
> +void tpm_sysfs_add_device(struct tpm_chip *chip);
>
> int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>
> --
> 2.4.3
>
--
- Jeremiah Mahler
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [BUG, bisect] tpm: Remove all uses of drvdata from the TPM Core
2016-05-10 13:55 ` [BUG, bisect] " Jeremiah Mahler
@ 2016-05-10 14:38 ` Jarkko Sakkinen
[not found] ` <201605101505.u4AF5nHd032349@d03av04.boulder.ibm.com>
1 sibling, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2016-05-10 14:38 UTC (permalink / raw)
To: Jeremiah Mahler, Stefan Berger, tpmdd-devel, jgunthorpe,
linux-security-module, linux-kernel
On Tue, May 10, 2016 at 06:55:23AM -0700, Jeremiah Mahler wrote:
> all,
>
> My machine is locking up during suspend and I have bisected the
> problem to this patch (e89f8b1ade9cc1a) in the latest linux-next
> (20160506) kernel.
>
> Let me know if I can test patches or do anything else to help.
Thank you. Could you email me the output of acpidump? If you want,
you can send it to me as private email. It's up to you.
/Jarkko
> On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >
> > The final thing preventing this was the way the sysfs files were
> > attached to the pdev. Follow the approach developed for ppi and move
> > the sysfs files to the chip->dev with symlinks from the pdev
> > for compatibility. Everything in the core now sanely uses container_of
> > to get the chip.
> >
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > ---
> > drivers/char/tpm/tpm-chip.c | 73 ++++++++++++++++++++++++++++------------
> > drivers/char/tpm/tpm-interface.c | 7 ++--
> > drivers/char/tpm/tpm-sysfs.c | 61 ++++++++++++++-------------------
> > drivers/char/tpm/tpm.h | 10 +++---
> > 4 files changed, 84 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 5bc530c..7e2c9cf 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -170,9 +170,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
> > chip->dev.class = tpm_class;
> > chip->dev.release = tpm_dev_release;
> > chip->dev.parent = dev;
> > -#ifdef CONFIG_ACPI
> > chip->dev.groups = chip->groups;
> > -#endif
> >
> > if (chip->dev_num == 0)
> > chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
kjjjjkk> >
> > static int tpm1_chip_register(struct tpm_chip *chip)
> > {
> > - int rc;
> > -
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > return 0;
> >
> > - rc = tpm_sysfs_add_device(chip);
> > - if (rc)
> > - return rc;
> > + tpm_sysfs_add_device(chip);
> >
> > chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
> >
> > @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
> >
> > if (chip->bios_dir)
> > tpm_bios_log_teardown(chip->bios_dir);
> > +}
> > +
> > +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> > +{
> > + struct attribute **i;
> > +
> > + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > + return;
> >
> > - tpm_sysfs_del_device(chip);
> > + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> > +
> > + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> > + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> > }
> >
> > +/* For compatibility with legacy sysfs paths we provide symlinks from the
> > + * parent dev directory to selected names within the tpm chip directory. Old
> > + * kernel versions created these files directly under the parent.
> > + */
> > +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> > +{
> > + struct attribute **i;
> > + int rc;
> > +
> > + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > + return 0;
> > +
> > + rc = __compat_only_sysfs_link_entry_to_kobj(
> > + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> > + if (rc && rc != -ENOENT)
> > + return rc;
> > +
> > + /* All the names from tpm-sysfs */
> > + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> > + rc = __compat_only_sysfs_link_entry_to_kobj(
> > + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> > + if (rc) {
> > + tpm_del_legacy_sysfs(chip);
> > + return rc;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > /*
> > * tpm_chip_register() - create a character device for the TPM chip
> > * @chip: TPM chip to use.
> > @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > tpm_add_ppi(chip);
> >
> > rc = tpm_add_char_device(chip);
> > - if (rc)
> > - goto out_err;
> > + if (rc) {
> > + tpm1_chip_unregister(chip);
> > + return rc;
> > + }
> >
> > chip->flags |= TPM_CHIP_FLAG_REGISTERED;
> >
> > - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > - rc = __compat_only_sysfs_link_entry_to_kobj(
> > - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> > - if (rc && rc != -ENOENT) {
> > - tpm_chip_unregister(chip);
> > - return rc;
> > - }
> > + rc = tpm_add_legacy_sysfs(chip);
> > + if (rc) {
> > + tpm_chip_unregister(chip);
> > + return rc;
> > }
> >
> > return 0;
> > -out_err:
> > - tpm1_chip_unregister(chip);
> > - return rc;
> > }
> > EXPORT_SYMBOL_GPL(tpm_chip_register);
> >
> > @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> > if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> > return;
> >
> > - if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > - sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> > + tpm_del_legacy_sysfs(chip);
> >
> > tpm1_chip_unregister(chip);
> > tpm_del_char_device(chip);
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index 7cba092..080dade 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
> > .ordinal = TPM_ORD_GET_CAP
> > };
> >
> > -ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> > +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> > const char *desc)
> > {
> > struct tpm_cmd_t tpm_cmd;
> > int rc;
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> >
> > tpm_cmd.header.in = tpm_getcap_header;
> > if (subcap_id == CAP_VERSION_1_1 || subcap_id == CAP_VERSION_1_2) {
> > @@ -935,7 +934,7 @@ static struct tpm_input_header savestate_header = {
> > */
> > int tpm_pm_suspend(struct device *dev)
> > {
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> > struct tpm_cmd_t cmd;
> > int rc, try;
> >
> > @@ -996,7 +995,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
> > */
> > int tpm_pm_resume(struct device *dev)
> > {
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> >
> > if (chip == NULL)
> > return -ENODEV;
> > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> > index a7c3473..b46cf70 100644
> > --- a/drivers/char/tpm/tpm-sysfs.c
> > +++ b/drivers/char/tpm/tpm-sysfs.c
> > @@ -36,7 +36,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> > int i, rc;
> > char *str = buf;
> >
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> >
> > tpm_cmd.header.in = tpm_readpubek_header;
> > err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
> > @@ -92,9 +92,9 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> > ssize_t rc;
> > int i, j, num_pcrs;
> > char *str = buf;
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> >
> > - rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
> > + rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
> > "attempting to determine the number of PCRS");
> > if (rc)
> > return 0;
> > @@ -119,8 +119,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
> > cap_t cap;
> > ssize_t rc;
> >
> > - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> > - "attempting to determine the permanent enabled state");
> > + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> > + "attempting to determine the permanent enabled state");
> > if (rc)
> > return 0;
> >
> > @@ -135,8 +135,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
> > cap_t cap;
> > ssize_t rc;
> >
> > - rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
> > - "attempting to determine the permanent active state");
> > + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
> > + "attempting to determine the permanent active state");
> > if (rc)
> > return 0;
> >
> > @@ -151,8 +151,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
> > cap_t cap;
> > ssize_t rc;
> >
> > - rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
> > - "attempting to determine the owner state");
> > + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
> > + "attempting to determine the owner state");
> > if (rc)
> > return 0;
> >
> > @@ -167,8 +167,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
> > cap_t cap;
> > ssize_t rc;
> >
> > - rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
> > - "attempting to determine the temporary state");
> > + rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
> > + "attempting to determine the temporary state");
> > if (rc)
> > return 0;
> >
> > @@ -180,11 +180,12 @@ static DEVICE_ATTR_RO(temp_deactivated);
> > static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> > char *buf)
> > {
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> > cap_t cap;
> > ssize_t rc;
> > char *str = buf;
> >
> > - rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
> > + rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
> > "attempting to determine the manufacturer");
> > if (rc)
> > return 0;
> > @@ -192,8 +193,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> > be32_to_cpu(cap.manufacturer_id));
> >
> > /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
> > - rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
> > - "attempting to determine the 1.2 version");
> > + rc = tpm_getcap(chip, CAP_VERSION_1_2, &cap,
> > + "attempting to determine the 1.2 version");
> > if (!rc) {
> > str += sprintf(str,
> > "TCG version: %d.%d\nFirmware version: %d.%d\n",
> > @@ -203,7 +204,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> > cap.tpm_version_1_2.revMinor);
> > } else {
> > /* Otherwise just use TPM_STRUCT_VER */
> > - rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
> > + rc = tpm_getcap(chip, CAP_VERSION_1_1, &cap,
> > "attempting to determine the 1.1 version");
> > if (rc)
> > return 0;
> > @@ -222,7 +223,7 @@ static DEVICE_ATTR_RO(caps);
> > 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);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> > if (chip == NULL)
> > return 0;
> >
> > @@ -234,7 +235,7 @@ static DEVICE_ATTR_WO(cancel);
> > static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
> > char *buf)
> > {
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> >
> > if (chip->duration[TPM_LONG] == 0)
> > return 0;
> > @@ -251,7 +252,7 @@ static DEVICE_ATTR_RO(durations);
> > static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> > char *buf)
> > {
> > - struct tpm_chip *chip = dev_get_drvdata(dev);
> > + struct tpm_chip *chip = to_tpm_chip(dev);
> >
> > return sprintf(buf, "%d %d %d %d [%s]\n",
> > jiffies_to_usecs(chip->timeout_a),
> > @@ -281,24 +282,12 @@ static const struct attribute_group tpm_dev_group = {
> > .attrs = tpm_dev_attrs,
> > };
> >
> > -int tpm_sysfs_add_device(struct tpm_chip *chip)
> > +void tpm_sysfs_add_device(struct tpm_chip *chip)
> > {
> > - int err;
> > - err = sysfs_create_group(&chip->dev.parent->kobj,
> > - &tpm_dev_group);
> > -
> > - if (err)
> > - dev_err(&chip->dev,
> > - "failed to create sysfs attributes, %d\n", err);
> > - return err;
> > -}
> > -
> > -void tpm_sysfs_del_device(struct tpm_chip *chip)
> > -{
> > - /* The sysfs routines rely on an implicit tpm_try_get_ops, this
> > - * function is called before ops is null'd and the sysfs core
> > - * synchronizes this removal so that no callbacks are running or can
> > - * run again
> > + /* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> > + * is called before ops is null'd and the sysfs core synchronizes this
> > + * removal so that no callbacks are running or can run again
> > */
> > - sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
> > + WARN_ON(chip->groups_cnt != 0);
> > + chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> > }
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 8bc6fb8..508e8e0 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -168,9 +168,9 @@ struct tpm_chip {
> >
> > struct dentry **bios_dir;
> >
> > -#ifdef CONFIG_ACPI
> > - const struct attribute_group *groups[2];
> > + const struct attribute_group *groups[3];
> > unsigned int groups_cnt;
> > +#ifdef CONFIG_ACPI
> > acpi_handle acpi_dev_handle;
> > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > #endif /* CONFIG_ACPI */
> > @@ -471,7 +471,8 @@ extern dev_t tpm_devt;
> > extern const struct file_operations tpm_fops;
> > extern struct idr dev_nums_idr;
> >
> > -ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> > +ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
> > + const char *desc);
> > 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,
> > @@ -496,8 +497,7 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
> > extern int tpm_chip_register(struct tpm_chip *chip);
> > extern void tpm_chip_unregister(struct tpm_chip *chip);
> >
> > -int tpm_sysfs_add_device(struct tpm_chip *chip);
> > -void tpm_sysfs_del_device(struct tpm_chip *chip);
> > +void tpm_sysfs_add_device(struct tpm_chip *chip);
> >
> > int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> >
> > --
> > 2.4.3
> >
>
> --
> - Jeremiah Mahler
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [BUG, bisect] tpm: Remove all uses of drvdata from the TPM Core
[not found] ` <201605101505.u4AF5nHd032349@d03av04.boulder.ibm.com>
@ 2016-05-11 1:26 ` Jeremiah Mahler
[not found] ` <201605111027.u4BARtQ4021061@d01av01.pok.ibm.com>
0 siblings, 1 reply; 25+ messages in thread
From: Jeremiah Mahler @ 2016-05-11 1:26 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-kernel, linux-security-module, Stefan Berger, tpmdd-devel
Stefan,
On Tue, May 10, 2016 at 11:05:43AM -0400, Stefan Berger wrote:
> Jeremiah Mahler <jmmahler@gmail.com> wrote on 05/10/2016 09:55:23 AM:
>
> >
> > all,
> >
> > My machine is locking up during suspend and I have bisected the
> > problem to this patch (e89f8b1ade9cc1a) in the latest linux-next
> > (20160506) kernel.
> >
> > Let me know if I can test patches or do anything else to help.
>
> The chip retrieval method in the pm functions is wrong. Sending patch.
>
> Stefan
>
>
Thanks for the quick reply Stefan. Let me know when you have a
patch for me to test.
--
- Jeremiah Mahler
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [BUG, bisect] tpm: Remove all uses of drvdata from the TPM Core
[not found] ` <201605111027.u4BARtQ4021061@d01av01.pok.ibm.com>
@ 2016-05-11 14:29 ` Jeremiah Mahler
2016-05-11 15:20 ` Stefan Berger
0 siblings, 1 reply; 25+ messages in thread
From: Jeremiah Mahler @ 2016-05-11 14:29 UTC (permalink / raw)
To: Stefan Berger
Cc: linux-kernel, linux-security-module, Stefan Berger, tpmdd-devel
Stefan,
On Wed, May 11, 2016 at 06:27:52AM -0400, Stefan Berger wrote:
> Jeremiah Mahler <jmmahler@gmail.com> wrote on 05/10/2016 09:26:17 PM:
>
[...]
>
> I forgot to cc you on the patch and obviously you weren't cc'ed through
> the Reported-by: line.
>
> Here's a pointer to the patch.
>
> http://thread.gmane.org/gmane.comp.encryption.tpmdd.devel/974
>
>
> Thanks for testing and bisecting.
>
> Regards,
> Stefan
>
[...]
I tested out the patch and it fixed the problem. Thanks Stefan.
I would add my Tested-by but I couldn't find the patch on the main
Linux mailing list. Perhaps I am not subscribed to the right list?
--
- Jeremiah Mahler
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tpmdd-devel] [BUG, bisect] tpm: Remove all uses of drvdata from the TPM Core
2016-05-11 14:29 ` Jeremiah Mahler
@ 2016-05-11 15:20 ` Stefan Berger
0 siblings, 0 replies; 25+ messages in thread
From: Stefan Berger @ 2016-05-11 15:20 UTC (permalink / raw)
To: Jeremiah Mahler, Stefan Berger, linux-kernel,
linux-security-module, tpmdd-devel
On 05/11/2016 10:29 AM, Jeremiah Mahler wrote:
> Stefan,
>
> On Wed, May 11, 2016 at 06:27:52AM -0400, Stefan Berger wrote:
>> Jeremiah Mahler <jmmahler@gmail.com> wrote on 05/10/2016 09:26:17 PM:
>>
> [...]
>> I forgot to cc you on the patch and obviously you weren't cc'ed through
>> the Reported-by: line.
>>
>> Here's a pointer to the patch.
>>
>> http://thread.gmane.org/gmane.comp.encryption.tpmdd.devel/974
>>
>>
>> Thanks for testing and bisecting.
>>
>> Regards,
>> Stefan
>>
> [...]
>
> I tested out the patch and it fixed the problem. Thanks Stefan.
>
> I would add my Tested-by but I couldn't find the patch on the main
> Linux mailing list. Perhaps I am not subscribed to the right list?
>
Thanks. I will add your Tested-by to it. I had posted the patch only to
the tpmdd-devel mailing list.
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2016-05-11 15:20 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-18 17:26 [PATCH v11 0/4] Multi-instance vTPM proxy driver Stefan Berger
2016-04-18 17:26 ` [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core Stefan Berger
2016-04-19 10:12 ` Jarkko Sakkinen
2016-04-19 10:36 ` Stefan Berger
2016-04-19 17:06 ` Jason Gunthorpe
2016-04-20 5:33 ` Jarkko Sakkinen
2016-04-19 17:21 ` Jarkko Sakkinen
2016-04-22 15:06 ` Jarkko Sakkinen
2016-04-25 19:13 ` Stefan Berger
2016-05-10 13:55 ` [BUG, bisect] " Jeremiah Mahler
2016-05-10 14:38 ` Jarkko Sakkinen
[not found] ` <201605101505.u4AF5nHd032349@d03av04.boulder.ibm.com>
2016-05-11 1:26 ` [tpmdd-devel] " Jeremiah Mahler
[not found] ` <201605111027.u4BARtQ4021061@d01av01.pok.ibm.com>
2016-05-11 14:29 ` Jeremiah Mahler
2016-05-11 15:20 ` Stefan Berger
2016-04-18 17:26 ` [PATCH v11 2/4] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL Stefan Berger
2016-04-22 16:46 ` Jarkko Sakkinen
2016-04-18 17:26 ` [PATCH v11 3/4] tpm: Proxy driver for supporting multiple emulated TPMs Stefan Berger
2016-04-22 16:47 ` Jarkko Sakkinen
2016-04-18 17:26 ` [PATCH v11 4/4] tpm: Add documentation for the tpm_vtpm_proxy device driver Stefan Berger
2016-04-22 16:47 ` Jarkko Sakkinen
2016-04-22 16:54 ` [PATCH v11 0/4] Multi-instance vTPM proxy driver Jarkko Sakkinen
2016-04-26 9:28 ` [tpmdd-devel] " Jarkko Sakkinen
2016-04-26 11:30 ` Stefan Berger
2016-04-27 12:56 ` Jarkko Sakkinen
2016-04-27 16:27 ` Stefan Berger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).