tpmdd-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [PATCH] tpm: consolidate the TPM startup code
@ 2017-06-20 18:13 Jarkko Sakkinen
       [not found] ` <20170620181334.28363-1-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2017-06-20 18:13 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: linux-security-module, Jarkko Sakkinen, Peter Huewe,
	Marcel Selhorst, Jason Gunthorpe, open list

Consolidated all the "manual" TPM startup code to a single function
in order to make code flows a bit cleaner and migrate to tpm_buf.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
 drivers/char/tpm/tpm.h           |  6 +---
 drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
 3 files changed, 44 insertions(+), 61 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d2b4df6d9894..fbef47d8bd06 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
 }
 EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
 
+#define TPM_ORD_STARTUP 153
+#define TPM_ST_CLEAR 1
+
+/**
+ * tpm_startup - turn on the TPM
+ * @chip: TPM chip to use
+ *
+ * Normally the firmware should start the TPM. This function is provided as a
+ * workaround if this does not happen. A legal case for this could be for
+ * example when a TPM emulator is used.
+ *
+ * Return: same as tpm_transmit_cmd()
+ */
+int tpm_startup(struct tpm_chip *chip)
+{
+	struct tpm_buf buf;
+	int rc;
+
+	dev_info(&chip->dev, "starting up the TPM manually\n");
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
+		if (rc < 0)
+			return rc;
+
+		tpm_buf_append_u32(&buf, TPM2_SU_CLEAR);
+	} else {
+		rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
+		if (rc < 0)
+			return rc;
+
+		tpm_buf_append_u32(&buf, TPM_ST_CLEAR);
+	}
+
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+			      "attempting to start the TPM");
+
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
 #define TPM_DIGEST_SIZE 20
 #define TPM_RET_CODE_IDX 6
 #define TPM_INTERNAL_RESULT_SIZE 200
@@ -586,27 +627,6 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 }
 EXPORT_SYMBOL_GPL(tpm_getcap);
 
-#define TPM_ORD_STARTUP 153
-#define TPM_ST_CLEAR cpu_to_be16(1)
-#define TPM_ST_STATE cpu_to_be16(2)
-#define TPM_ST_DEACTIVATED cpu_to_be16(3)
-static const struct tpm_input_header tpm_startup_header = {
-	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-	.length = cpu_to_be32(12),
-	.ordinal = cpu_to_be32(TPM_ORD_STARTUP)
-};
-
-static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
-{
-	struct tpm_cmd_t start_cmd;
-	start_cmd.header.in = tpm_startup_header;
-
-	start_cmd.params.startup_in.startup_type = startup_type;
-	return tpm_transmit_cmd(chip, NULL, &start_cmd,
-				TPM_INTERNAL_RESULT_SIZE, 0,
-				0, "attempting to start the TPM");
-}
-
 int tpm_get_timeouts(struct tpm_chip *chip)
 {
 	cap_t cap;
@@ -636,10 +656,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
 			sizeof(cap.timeout));
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
-		/* The TPM is not started, we are the first to talk to it.
-		   Execute a startup command. */
-		dev_info(&chip->dev, "Issuing TPM_STARTUP\n");
-		if (tpm_startup(chip, TPM_ST_CLEAR))
+		if (tpm_startup(chip))
 			return rc;
 
 		rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d9835b31f652..1f9094f03151 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -371,16 +371,11 @@ struct tpm_getrandom_in {
 	__be32 num_bytes;
 } __packed;
 
-struct tpm_startup_in {
-	__be16	startup_type;
-} __packed;
-
 typedef union {
 	struct	tpm_pcrread_in	pcrread_in;
 	struct	tpm_pcrread_out	pcrread_out;
 	struct	tpm_getrandom_in getrandom_in;
 	struct	tpm_getrandom_out getrandom_out;
-	struct tpm_startup_in startup_in;
 } tpm_cmd_params;
 
 struct tpm_cmd_t {
@@ -506,6 +501,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
 			 const void *buf, size_t bufsiz,
 			 size_t min_rsp_body_length, unsigned int flags,
 			 const char *desc);
+int tpm_startup(struct tpm_chip *chip);
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		   const char *desc, size_t min_cap_length);
 int tpm_get_timeouts(struct tpm_chip *);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 3a9964326279..1962c9b15cd5 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -779,36 +779,6 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 }
 EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
 
-#define TPM2_STARTUP_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_startup_in))
-
-static const struct tpm_input_header tpm2_startup_header = {
-	.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-	.length = cpu_to_be32(TPM2_STARTUP_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_STARTUP)
-};
-
-/**
- * tpm2_startup() - send startup command to the TPM chip
- *
- * @chip:		TPM chip to use.
- * @startup_type:	startup type. The value is either
- *			TPM_SU_CLEAR or TPM_SU_STATE.
- *
- * Return: Same as with tpm_transmit_cmd.
- */
-static int tpm2_startup(struct tpm_chip *chip, u16 startup_type)
-{
-	struct tpm2_cmd cmd;
-
-	cmd.header.in = tpm2_startup_header;
-
-	cmd.params.startup_in.startup_type = cpu_to_be16(startup_type);
-	return tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
-				"attempting to start the TPM");
-}
-
 #define TPM2_SHUTDOWN_IN_SIZE \
 	(sizeof(struct tpm_input_header) + \
 	 sizeof(struct tpm2_startup_in))
@@ -1150,7 +1120,7 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 	}
 
 	if (rc == TPM2_RC_INITIALIZE) {
-		rc = tpm2_startup(chip, TPM2_SU_CLEAR);
+		rc = tpm_startup(chip);
 		if (rc)
 			goto out;
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
       [not found] ` <20170620181334.28363-1-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-06-20 19:31   ` Jason Gunthorpe
       [not found]     ` <20170620193152.GA3368-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
  2017-06-20 20:58     ` [tpmdd-devel] " Ken Goldman
  0 siblings, 2 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2017-06-20 19:31 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, open list

On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
> Consolidated all the "manual" TPM startup code to a single function
> in order to make code flows a bit cleaner and migrate to tpm_buf.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>  drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
>  drivers/char/tpm/tpm.h           |  6 +---
>  drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
>  3 files changed, 44 insertions(+), 61 deletions(-)

Makes sense to me

> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index d2b4df6d9894..fbef47d8bd06 100644
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>  }
>  EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
>  
> +#define TPM_ORD_STARTUP 153
> +#define TPM_ST_CLEAR 1

We should really have a tpm1.h and tpm2.h that has all these various
constants and things instead of open coding them randomly all over..

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
       [not found]     ` <20170620193152.GA3368-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-06-20 20:55       ` Jarkko Sakkinen
  2017-06-20 21:25         ` Stefan Berger
       [not found]         ` <20170620205529.ku3nueglj3mqkd3s-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
  0 siblings, 2 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2017-06-20 20:55 UTC (permalink / raw)
  To: Jason Gunthorpe, stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, open list

On Tue, Jun 20, 2017 at 01:31:52PM -0600, Jason Gunthorpe wrote:
> On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
> > Consolidated all the "manual" TPM startup code to a single function
> > in order to make code flows a bit cleaner and migrate to tpm_buf.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >  drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
> >  drivers/char/tpm/tpm.h           |  6 +---
> >  drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
> >  3 files changed, 44 insertions(+), 61 deletions(-)
> 
> Makes sense to me
> 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index d2b4df6d9894..fbef47d8bd06 100644
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> >  }
> >  EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
> >  
> > +#define TPM_ORD_STARTUP 153
> > +#define TPM_ST_CLEAR 1
> 
> We should really have a tpm1.h and tpm2.h that has all these various
> constants and things instead of open coding them randomly all over..
> 
> Jason

I agree.

Is this patch acceptable to be applied?

Stefan, can you peer test this with a TPM emulator? For convenient testing
I created 'readpubek' branch that includes also my readpubek bug fixes.
Seeing that the module loads and you can output pubek sysfs attribute is
sufficient for seeing that all the three patches work.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [tpmdd-devel] [PATCH] tpm: consolidate the TPM startup code
  2017-06-20 19:31   ` Jason Gunthorpe
       [not found]     ` <20170620193152.GA3368-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-06-20 20:58     ` Ken Goldman
  1 sibling, 0 replies; 9+ messages in thread
From: Ken Goldman @ 2017-06-20 20:58 UTC (permalink / raw)
  Cc: linux-security-module, tpmdd-devel, open list

On 6/20/2017 3:31 PM, Jason Gunthorpe wrote:
>> 
>> +#define TPM_ORD_STARTUP 153 +#define TPM_ST_CLEAR 1
> 
> We should really have a tpm1.h and tpm2.h that has all these various 
> constants and things instead of open coding them randomly all over..

While you're doing that, perhaps put the ordinal numbers in hex.  The
TPM 1.2 spec uses both hex and decimal, but TPM 2.0 uses hex.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
  2017-06-20 20:55       ` Jarkko Sakkinen
@ 2017-06-20 21:25         ` Stefan Berger
  2017-06-20 21:32           ` Jarkko Sakkinen
       [not found]         ` <20170620205529.ku3nueglj3mqkd3s-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Stefan Berger @ 2017-06-20 21:25 UTC (permalink / raw)
  To: Jarkko Sakkinen, Jason Gunthorpe
  Cc: tpmdd-devel, linux-security-module, Peter Huewe, Marcel Selhorst,
	open list

On 06/20/2017 04:55 PM, Jarkko Sakkinen wrote:
> On Tue, Jun 20, 2017 at 01:31:52PM -0600, Jason Gunthorpe wrote:
>> On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
>>> Consolidated all the "manual" TPM startup code to a single function
>>> in order to make code flows a bit cleaner and migrate to tpm_buf.
>>>
>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>   drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
>>>   drivers/char/tpm/tpm.h           |  6 +---
>>>   drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
>>>   3 files changed, 44 insertions(+), 61 deletions(-)
>> Makes sense to me
>>
>>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>>> index d2b4df6d9894..fbef47d8bd06 100644
>>> +++ b/drivers/char/tpm/tpm-interface.c
>>> @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>>>   }
>>>   EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
>>>   
>>> +#define TPM_ORD_STARTUP 153
>>> +#define TPM_ST_CLEAR 1
>> We should really have a tpm1.h and tpm2.h that has all these various
>> constants and things instead of open coding them randomly all over..
>>
>> Jason
> I agree.
>
> Is this patch acceptable to be applied?
>
> Stefan, can you peer test this with a TPM emulator? For convenient testing
> I created 'readpubek' branch that includes also my readpubek bug fixes.
> Seeing that the module loads and you can output pubek sysfs attribute is
> sufficient for seeing that all the three patches work.

Doesn't work. The startup_type is be16, but you are appending a u32 now 
for both TPM1.2 and TPM2.

+               tpm_buf_append_u32(&buf, TPM_ST_CLEAR);


-struct tpm_startup_in {
-       __be16  startup_type;
-} __packed;
-

struct tpm2_startup_in can probably be removed as well?

   Stefan

>
> /Jarkko
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
  2017-06-20 21:25         ` Stefan Berger
@ 2017-06-20 21:32           ` Jarkko Sakkinen
  2017-06-20 21:38             ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2017-06-20 21:32 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Jason Gunthorpe, tpmdd-devel, linux-security-module, Peter Huewe,
	Marcel Selhorst, open list

On Tue, Jun 20, 2017 at 05:25:57PM -0400, Stefan Berger wrote:
> On 06/20/2017 04:55 PM, Jarkko Sakkinen wrote:
> > On Tue, Jun 20, 2017 at 01:31:52PM -0600, Jason Gunthorpe wrote:
> > > On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
> > > > Consolidated all the "manual" TPM startup code to a single function
> > > > in order to make code flows a bit cleaner and migrate to tpm_buf.
> > > > 
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > >   drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
> > > >   drivers/char/tpm/tpm.h           |  6 +---
> > > >   drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
> > > >   3 files changed, 44 insertions(+), 61 deletions(-)
> > > Makes sense to me
> > > 
> > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > > > index d2b4df6d9894..fbef47d8bd06 100644
> > > > +++ b/drivers/char/tpm/tpm-interface.c
> > > > @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > > >   }
> > > >   EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
> > > > +#define TPM_ORD_STARTUP 153
> > > > +#define TPM_ST_CLEAR 1
> > > We should really have a tpm1.h and tpm2.h that has all these various
> > > constants and things instead of open coding them randomly all over..
> > > 
> > > Jason
> > I agree.
> > 
> > Is this patch acceptable to be applied?
> > 
> > Stefan, can you peer test this with a TPM emulator? For convenient testing
> > I created 'readpubek' branch that includes also my readpubek bug fixes.
> > Seeing that the module loads and you can output pubek sysfs attribute is
> > sufficient for seeing that all the three patches work.
> 
> Doesn't work. The startup_type is be16, but you are appending a u32 now for
> both TPM1.2 and TPM2.
> 
> +               tpm_buf_append_u32(&buf, TPM_ST_CLEAR);

Ah. Thanks. I'll fix that.

> -struct tpm_startup_in {
> -       __be16  startup_type;
> -} __packed;
> -
> 
> struct tpm2_startup_in can probably be removed as well?
> 
>   Stefan

Not yet but soon. tpm2_shutdown() still uses it.

/Jarkko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
  2017-06-20 21:32           ` Jarkko Sakkinen
@ 2017-06-20 21:38             ` Jarkko Sakkinen
  2017-06-20 22:16               ` Stefan Berger
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2017-06-20 21:38 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Jason Gunthorpe, tpmdd-devel, linux-security-module, Peter Huewe,
	Marcel Selhorst, open list

On Tue, Jun 20, 2017 at 11:32:41PM +0200, Jarkko Sakkinen wrote:
> On Tue, Jun 20, 2017 at 05:25:57PM -0400, Stefan Berger wrote:
> > On 06/20/2017 04:55 PM, Jarkko Sakkinen wrote:
> > > On Tue, Jun 20, 2017 at 01:31:52PM -0600, Jason Gunthorpe wrote:
> > > > On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
> > > > > Consolidated all the "manual" TPM startup code to a single function
> > > > > in order to make code flows a bit cleaner and migrate to tpm_buf.
> > > > > 
> > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > >   drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
> > > > >   drivers/char/tpm/tpm.h           |  6 +---
> > > > >   drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
> > > > >   3 files changed, 44 insertions(+), 61 deletions(-)
> > > > Makes sense to me
> > > > 
> > > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > > > > index d2b4df6d9894..fbef47d8bd06 100644
> > > > > +++ b/drivers/char/tpm/tpm-interface.c
> > > > > @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > > > >   }
> > > > >   EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
> > > > > +#define TPM_ORD_STARTUP 153
> > > > > +#define TPM_ST_CLEAR 1
> > > > We should really have a tpm1.h and tpm2.h that has all these various
> > > > constants and things instead of open coding them randomly all over..
> > > > 
> > > > Jason
> > > I agree.
> > > 
> > > Is this patch acceptable to be applied?
> > > 
> > > Stefan, can you peer test this with a TPM emulator? For convenient testing
> > > I created 'readpubek' branch that includes also my readpubek bug fixes.
> > > Seeing that the module loads and you can output pubek sysfs attribute is
> > > sufficient for seeing that all the three patches work.
> > 
> > Doesn't work. The startup_type is be16, but you are appending a u32 now for
> > both TPM1.2 and TPM2.
> > 
> > +               tpm_buf_append_u32(&buf, TPM_ST_CLEAR);
> 
> Ah. Thanks. I'll fix that.

It's fixed in the 'readpubek' branch. If that is the only concern, can
you test that branch and see if it is OK so that we don't need a new
cycle for the patch set?

/Jarkko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
       [not found]         ` <20170620205529.ku3nueglj3mqkd3s-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-06-20 21:46           ` Jarkko Sakkinen
  0 siblings, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2017-06-20 21:46 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, open list

On Tue, Jun 20, 2017 at 10:55:29PM +0200, Jarkko Sakkinen wrote:
> On Tue, Jun 20, 2017 at 01:31:52PM -0600, Jason Gunthorpe wrote:
> > On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
> > > Consolidated all the "manual" TPM startup code to a single function
> > > in order to make code flows a bit cleaner and migrate to tpm_buf.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > >  drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
> > >  drivers/char/tpm/tpm.h           |  6 +---
> > >  drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
> > >  3 files changed, 44 insertions(+), 61 deletions(-)
> > 
> > Makes sense to me
> > 
> > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > > index d2b4df6d9894..fbef47d8bd06 100644
> > > +++ b/drivers/char/tpm/tpm-interface.c
> > > @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> > >  }
> > >  EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
> > >  
> > > +#define TPM_ORD_STARTUP 153
> > > +#define TPM_ST_CLEAR 1
> > 
> > We should really have a tpm1.h and tpm2.h that has all these various
> > constants and things instead of open coding them randomly all over..
> > 
> > Jason
> 
> I agree.

I agree means here that I agree if you mean only protocol specific
constants. I do not think it would make sense with things like struct
tpm_readpubek_out that is a throw away only used by that one function as
helper (would compare it to a helper function). It would only make
reading the code slower.

It would make sense, for example, to have enum tpm_ordinals for all
TPM 1.2 ordinals in tpm1_proto.h (yes would add _proto for clarity).

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] tpm: consolidate the TPM startup code
  2017-06-20 21:38             ` Jarkko Sakkinen
@ 2017-06-20 22:16               ` Stefan Berger
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Berger @ 2017-06-20 22:16 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Jason Gunthorpe, tpmdd-devel, linux-security-module, Peter Huewe,
	Marcel Selhorst, open list

On 06/20/2017 05:38 PM, Jarkko Sakkinen wrote:
> On Tue, Jun 20, 2017 at 11:32:41PM +0200, Jarkko Sakkinen wrote:
>> On Tue, Jun 20, 2017 at 05:25:57PM -0400, Stefan Berger wrote:
>>> On 06/20/2017 04:55 PM, Jarkko Sakkinen wrote:
>>>> On Tue, Jun 20, 2017 at 01:31:52PM -0600, Jason Gunthorpe wrote:
>>>>> On Tue, Jun 20, 2017 at 08:13:34PM +0200, Jarkko Sakkinen wrote:
>>>>>> Consolidated all the "manual" TPM startup code to a single function
>>>>>> in order to make code flows a bit cleaner and migrate to tpm_buf.
>>>>>>
>>>>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>>>>    drivers/char/tpm/tpm-interface.c | 67 +++++++++++++++++++++++++---------------
>>>>>>    drivers/char/tpm/tpm.h           |  6 +---
>>>>>>    drivers/char/tpm/tpm2-cmd.c      | 32 +------------------
>>>>>>    3 files changed, 44 insertions(+), 61 deletions(-)
>>>>> Makes sense to me
>>>>>
>>>>>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>>>>>> index d2b4df6d9894..fbef47d8bd06 100644
>>>>>> +++ b/drivers/char/tpm/tpm-interface.c
>>>>>> @@ -540,6 +540,47 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
>>>>>>    }
>>>>>>    EXPORT_SYMBOL_GPL(tpm_transmit_cmd);
>>>>>> +#define TPM_ORD_STARTUP 153
>>>>>> +#define TPM_ST_CLEAR 1
>>>>> We should really have a tpm1.h and tpm2.h that has all these various
>>>>> constants and things instead of open coding them randomly all over..
>>>>>
>>>>> Jason
>>>> I agree.
>>>>
>>>> Is this patch acceptable to be applied?
>>>>
>>>> Stefan, can you peer test this with a TPM emulator? For convenient testing
>>>> I created 'readpubek' branch that includes also my readpubek bug fixes.
>>>> Seeing that the module loads and you can output pubek sysfs attribute is
>>>> sufficient for seeing that all the three patches work.
>>> Doesn't work. The startup_type is be16, but you are appending a u32 now for
>>> both TPM1.2 and TPM2.
>>>
>>> +               tpm_buf_append_u32(&buf, TPM_ST_CLEAR);
>> Ah. Thanks. I'll fix that.
> It's fixed in the 'readpubek' branch. If that is the only concern, can
> you test that branch and see if it is OK so that we don't need a new
> cycle for the patch set?

TPM1.2 works now. TPM2 does not work and also needs u16 I think.

    Stefan


>
> /Jarkko
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-06-20 22:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 18:13 [PATCH] tpm: consolidate the TPM startup code Jarkko Sakkinen
     [not found] ` <20170620181334.28363-1-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-06-20 19:31   ` Jason Gunthorpe
     [not found]     ` <20170620193152.GA3368-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-06-20 20:55       ` Jarkko Sakkinen
2017-06-20 21:25         ` Stefan Berger
2017-06-20 21:32           ` Jarkko Sakkinen
2017-06-20 21:38             ` Jarkko Sakkinen
2017-06-20 22:16               ` Stefan Berger
     [not found]         ` <20170620205529.ku3nueglj3mqkd3s-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-06-20 21:46           ` Jarkko Sakkinen
2017-06-20 20:58     ` [tpmdd-devel] " Ken Goldman

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).