driver: add PPI support in tpm driver

PATCH Superseded Zhang, Xiaoyan 2 participants 5 messages Jun 21, 2012 – Aug 15, 2012

Subsystems (2)

ABI/API TPM device driver

lore

Share & tools

lore

Zhang, Xiaoyan

author

2012-06-21 06:54 UTC #0 lore raw

PATCH v3 1/2 driver: add PPI support in tpm driver

From: Xiaoyan Zhang <[email protected]>

The Physical Presence Interface enables the OS and the BIOS to cooperate and provides a simple and straightforward platform user experience for administering the TPM without sacrificing security.

V2: separate the patch out in a separate source file, add #ifdef CONFIG_ACPI so it compiles out on ppc, and use standard error instead of ACPI error as return code of show/store fns.

V3: move #ifdef CONFIG_ACPI from .c file to .h file

Signed-off-by Xiaoyan Zhang <[email protected]>

---
 drivers/char/tpm/Makefile  |    4 +-
 drivers/char/tpm/tpm.c     |    5 +
 drivers/char/tpm/tpm.h     |    8 +
 drivers/char/tpm/tpm_ppi.c |  461 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 477 insertions(+), 1 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_ppi.c

drivers/char/tpm/Makefile +3 -1

index ea3a1e0..132ad95 100644
@@ -1,8 +1,10 @@
 #
 # Makefile for the kernel tpm device drivers.
 #
-obj-$(CONFIG_TCG_TPM) += tpm.o
+obj-$(CONFIG_TCG_TPM) += tpm_main.o
+tpm_main-y += tpm.o
 ifdef CONFIG_ACPI
+	tpm_main-y += tpm_ppi.o
 	obj-$(CONFIG_TCG_TPM) += tpm_bios.o
 endif
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o

drivers/char/tpm/tpm.c +5 -0

index ad7c732..ef2ff79 100644
@@ -1416,6 +1416,11 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 		return NULL;
 	}
 
+	if (sys_add_ppi(&dev->kobj, &ppi_attr_grp)) {
+		misc_deregister(&chip->vendor.miscdev);
+		put_device(chip->dev);
+		return NULL;
+	}
 	chip->bios_dir = tpm_bios_log_setup(devname);
 
 	/* Make chip available */

drivers/char/tpm/tpm.h +8 -0

index b1c5280..c5d121b 100644
@@ -303,9 +303,12 @@ extern int tpm_pm_suspend(struct device *, pm_message_t);
 extern int tpm_pm_resume(struct device *);
 extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
 			     wait_queue_head_t *);
+extern struct attribute_group ppi_attr_grp;
 #ifdef CONFIG_ACPI
 extern struct dentry ** tpm_bios_log_setup(char *);
 extern void tpm_bios_log_teardown(struct dentry **);
+extern ssize_t sys_add_ppi(struct kobject *parent,
+			   struct attribute_group *grp);
 #else
 static inline struct dentry ** tpm_bios_log_setup(char *name)
 {

@@ -314,4 +317,9 @@ static inline struct dentry ** tpm_bios_log_setup(char *name)
 static inline void tpm_bios_log_teardown(struct dentry **dir)
 {
 }
+static inline ssize_t sys_add_ppi(struct kobject *parent,
+				  struct attribute_group *grp)
+{
+	return 0;
+}
 #endif

drivers/char/tpm/tpm_ppi.c new file +461 -0

461 lines not on this page · raw message

Signature · 1 line

1.7.6.5

Kent Yoder

2012-07-11 15:24 UTC #1 lore raw

Hi Xiaoyan,

Quoted 26 lines · Xiaoyan

From: Xiaoyan Zhang <[email protected]>
The Physical Presence Interface enables the OS and the BIOS to cooperate and provides a simple and straightforward platform user experience for administering the TPM without sacrificing security.
V2: separate the patch out in a separate source file, add #ifdef CONFIG_ACPI so it compiles out on ppc, and use standard error instead of ACPI error as return code of show/store fns.
V3: move #ifdef CONFIG_ACPI from .c file to .h file
Signed-off-by Xiaoyan Zhang <[email protected]>
---
 drivers/char/tpm/Makefile  |    4 +-
 drivers/char/tpm/tpm.c     |    5 +
 drivers/char/tpm/tpm.h     |    8 +
 drivers/char/tpm/tpm_ppi.c |  461 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 477 insertions(+), 1 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_ppi.c
drivers/char/tpm/Makefile +0 -0
index ea3a1e0..132ad95 100644

drivers/char/tpm/Makefile line 8

Quoted 9 lines

drivers/char/tpm/Makefile +3 -1

@@ -1,8 +1,10 @@
 #
 # Makefile for the kernel tpm device drivers.
 #
-obj-$(CONFIG_TCG_TPM) += tpm.o
+obj-$(CONFIG_TCG_TPM) += tpm_main.o
+tpm_main-y += tpm.o
 ifdef CONFIG_ACPI
+	tpm_main-y += tpm_ppi.o
 	obj-$(CONFIG_TCG_TPM) += tpm_bios.o

This will change the name of the tpm driver, which I really don't want
to do. Can you apply on top of [1] and resubmit? I've added the following patch which modularizes the event logging code and has a couple Makefile changes too. This is needed for a future driver on PPC, but it should make adding your file obvious. Note that the patch below applies on top of Infineon's I2C driver patches.

[1] git://github.com/shpedoikal/linux.git tpmdd-next

Thanks,
Kent

tpm: modularize event log collection

Break ACPI-specific pieces of the event log handling into their own file and create tpm_eventlog.[ch] to store common event log handling code. This will be required to integrate future event log sources on platforms without ACPI tables.

Signed-off-by Kent Yoder <[email protected]>

---
 drivers/char/tpm/Makefile       |    1 +
 drivers/char/tpm/tpm.c          |    1 +
 drivers/char/tpm/tpm_acpi.c     |  104 ++++++++
 drivers/char/tpm/tpm_bios.c     |  556 ---------------------------------------
 drivers/char/tpm/tpm_eventlog.c |  419 +++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h |   71 +++++
 6 files changed, 596 insertions(+), 556 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_acpi.c
 delete mode 100644 drivers/char/tpm/tpm_bios.c
 create mode 100644 drivers/char/tpm/tpm_eventlog.c
 create mode 100644 drivers/char/tpm/tpm_eventlog.h

drivers/char/tpm/Makefile +1 -0

index a9c3afc..beac52f6 100644
@@ -4,6 +4,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 ifdef CONFIG_ACPI
 	obj-$(CONFIG_TCG_TPM) += tpm_bios.o
+	tpm_bios-objs += tpm_eventlog.o tpm_acpi.o
 endif
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o

drivers/char/tpm/tpm.c +1 -0

index d39b1f6..beb98c3 100644
@@ -30,6 +30,7 @@
 #include <linux/freezer.h>
 
 #include "tpm.h"
+#include "tpm_eventlog.h"
 
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */

drivers/char/tpm/tpm_acpi.c new file +104 -0

104 lines not on this page · raw message

drivers/char/tpm/tpm_bios.c deleted +0 ~~-556~~

556 lines not on this page · raw message

drivers/char/tpm/tpm_eventlog.c new file +419 -0

419 lines not on this page · raw message

drivers/char/tpm/tpm_eventlog.h new file +71 -0

71 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Zhang, Xiaoyan

author

2012-07-12 02:38 UTC #2 lore raw

Hi Kent,

Thanks for your comment on the patch. But there's some confusion on my side.
You mentioned not to change the tpm driver name. But the driver is linked from tpm.c and tpm_ppi.c, so I should change the original tpm.c file name, right? Is it acceptable to change tpm.c to tpm_common.c or tpm_utils.c or else?

Thanks,
Xiaoyan

-----Original Message-----
From: Kent Yoder [mailto:[email protected]]
Sent: Wednesday, July 11, 2012 11:25 PM
To: Zhang, Xiaoyan
Cc: [email protected]; Cihula, Joseph; Wei, Gang; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V3 1/2] driver: add PPI support in tpm driver

Hi Xiaoyan,

Quoted 27 lines · Xiaoyan

From: Xiaoyan Zhang <[email protected]>
The Physical Presence Interface enables the OS and the BIOS to cooperate and provides a simple and straightforward platform user experience for administering the TPM without sacrificing security.
V2: separate the patch out in a separate source file, add #ifdef CONFIG_ACPI so it compiles out on ppc, and use standard error instead of ACPI error as return code of show/store fns.
V3: move #ifdef CONFIG_ACPI from .c file to .h file
Signed-off-by Xiaoyan Zhang <[email protected]>
---
 drivers/char/tpm/Makefile  |    4 +-
 drivers/char/tpm/tpm.c     |    5 +
 drivers/char/tpm/tpm.h     |    8 +
 drivers/char/tpm/tpm_ppi.c |  461 
++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 477 insertions(+), 1 deletions(-)  create mode 
100644 drivers/char/tpm/tpm_ppi.c
drivers/char/tpm/Makefile +0 -0
index ea3a1e0..132ad95 100644

drivers/char/tpm/Makefile line 8

Quoted 9 lines

drivers/char/tpm/Makefile +3 -1

@@ -1,8 +1,10 @@
 #
 # Makefile for the kernel tpm device drivers.
 #
-obj-$(CONFIG_TCG_TPM) += tpm.o
+obj-$(CONFIG_TCG_TPM) += tpm_main.o
+tpm_main-y += tpm.o
 ifdef CONFIG_ACPI
+	tpm_main-y += tpm_ppi.o
 	obj-$(CONFIG_TCG_TPM) += tpm_bios.o

This will change the name of the tpm driver, which I really don't want to do. Can you apply on top of [1] and resubmit? I've added the following patch which modularizes the event logging code and has a couple Makefile changes too. This is needed for a future driver on PPC, but it should make adding your file obvious. Note that the patch below applies on top of Infineon's I2C driver patches.

[1] git://github.com/shpedoikal/linux.git tpmdd-next

Thanks,
Kent

tpm: modularize event log collection

Signed-off-by Kent Yoder <[email protected]>

---
 drivers/char/tpm/Makefile       |    1 +
 drivers/char/tpm/tpm.c          |    1 +
 drivers/char/tpm/tpm_acpi.c     |  104 ++++++++
 drivers/char/tpm/tpm_bios.c     |  556 ---------------------------------------
 drivers/char/tpm/tpm_eventlog.c |  419 +++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h |   71 +++++
 6 files changed, 596 insertions(+), 556 deletions(-)  create mode 100644 drivers/char/tpm/tpm_acpi.c  delete mode 100644 drivers/char/tpm/tpm_bios.c  create mode 100644 drivers/char/tpm/tpm_eventlog.c  create mode 100644 drivers/char/tpm/tpm_eventlog.h

drivers/char/tpm/Makefile +1 -0

@@ -4,6 +4,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 ifdef CONFIG_ACPI
 	obj-$(CONFIG_TCG_TPM) += tpm_bios.o
+	tpm_bios-objs += tpm_eventlog.o tpm_acpi.o
 endif
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c index d39b1f6..beb98c3 100644

drivers/char/tpm/tpm.c +1 -0

@@ -30,6 +30,7 @@
 #include <linux/freezer.h>
 
 #include "tpm.h"
+#include "tpm_eventlog.h"
 
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */

drivers/char/tpm/tpm_acpi.c new file +104 -0

104 lines not on this page · raw message

drivers/char/tpm/tpm_bios.c deleted +0 ~~-547~~

547 lines not on this page · raw message

drivers/char/tpm/tpm_eventlog.c new file +411 -0

411 lines not on this page · raw message

drivers/char/tpm/tpm_eventlog.h new file +70 -0

70 lines not on this page · raw message

--
1.7.5.4

Kent Yoder

2012-07-12 14:45 UTC #3 lore raw

Quoted 4 lines · Xiaoyan

Hi Kent,
Thanks for your comment on the patch. But there's some confusion on my side.
You mentioned not to change the tpm driver name. But the driver is linked from tpm.c and tpm_ppi.c, so I should change the original tpm.c file name, right? Is it acceptable to change tpm.c to tpm_common.c or tpm_utils.c or else?

I think you can just add tpm_ppi.c like this:

drivers/char/tpm/Makefile +1 -1

index beac52f6..96b9222 100644
@@ -4,7 +4,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 ifdef CONFIG_ACPI
        obj-$(CONFIG_TCG_TPM) += tpm_bios.o
-       tpm_bios-objs += tpm_eventlog.o tpm_acpi.o
+       tpm_bios-objs += tpm_eventlog.o tpm_acpi.o tpm_ppi.o
 endif
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o

Kent

Quoted 3 lines

Thanks,
Xiaoyan

Kent Yoder

2012-08-15 20:04 UTC #4 lore raw

Quoted 13 lines · Kent Yoder

Quoted 4 lines · Xiaoyan
Hi Kent,
Thanks for your comment on the patch. But there's some confusion on my side.
You mentioned not to change the tpm driver name. But the driver is linked from tpm.c and tpm_ppi.c, so I should change the original tpm.c file name, right? Is it acceptable to change tpm.c to tpm_common.c or tpm_utils.c or else?
I think you can just add tpm_ppi.c like this:
drivers/char/tpm/Makefile +0 -0
index beac52f6..96b9222 100644

drivers/char/tpm/Makefile line 10

Quoted 8 lines

drivers/char/tpm/Makefile +1 -1

@@ -4,7 +4,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 ifdef CONFIG_ACPI
        obj-$(CONFIG_TCG_TPM) += tpm_bios.o
-       tpm_bios-objs += tpm_eventlog.o tpm_acpi.o
+       tpm_bios-objs += tpm_eventlog.o tpm_acpi.o tpm_ppi.o
 endif
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o

Hi Xiaoyan,

Can you please resubmit the PPI patches applying on top of this tree?

git://github.com:shpedoikal/linux.git v3.6-rc1-tpmdd-staging

Thanks,
Kent

Quoted 13 lines

Kent
Quoted 3 lines
Thanks,
Xiaoyan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/