mbox series

[U-Boot,00/18] Introduce SPI TPM v2.0 support

Message ID 20180308154021.25255-1-miquel.raynal@bootlin.com
Headers show
Series Introduce SPI TPM v2.0 support | expand

Message

Miquel Raynal March 8, 2018, 3:40 p.m. UTC
Current U-Boot supports TPM v1.2 specification. The new specification
(v2.0) is not backward compatible and renames/introduces several
functions.

This series introduces a new SPI driver following the TPM v2.0
specification. It has been tested on a ST TPM but should be usable with
others v2.0 compliant chips.

Then, basic functionalities are introduced one by one for the v2.0
specification. The INIT command now can receive a parameter to
distinguish further TPMv1/TPMv2 commands. After that, the library itself
will know which one is pertinent and will return a special error if the
desired command is not supported for the selected specification.

Available commands for v2.0 TPMs are:
* STARTUP
* SELF TEST
* CLEAR
* PCR EXTEND
* PCR READ
* GET CAPABILITY
* DICTIONARY ATTACK LOCK RESET
* DICTIONARY ATTACK CHANGE PARAMETERS
* HIERARCHY CHANGE AUTH

Two commands have been written but could not be tested (unsupported by
the TPM chosen):
* PCR CHANGE AUTH POLICY
* PCR CHANGE AUTH VALUE

With this set of function, minimal TPMv2.0 handling is possible with the
following sequence.

* First, initialize the TPM stack in U-Boot: "TPM2" is a new parameter
  to discern the format of the commands:

> tpm init TPM2

* Then send the STARTUP command to the TPM. The flag is slightly
  different between the revisions.

> tpm startup TPM2_SU_CLEAR

* To enable full TPM capabilities, continue the tests (or do them all
  again). It seems like self_test_full always waits for the operation to
  finish, while continue_self_test returns a busy state if called to
  early.

> tpm continue_self_test
> tpm self_test_full

* Manage passwords (force_clear also resets a lot of internal stuff).
  Olderly, TAKE OWNERSHIP == CLEAR + CHANGE AUTH. LOCKOUT is an example,
  ENDORSEMENT and PLATFORM hierarchies are available too:

> tpm force_clear TPM2_RH_LOCKOUT [<pw>]
> tpm change_auth TPM2_RH_LOCKOUT <new_pw> [<old_pw>]

* Dictionary Attack Mitigation (DAM) parameters can be changed. It is
  possible to reset the failure counter and disable the lockout (values
  erased after a CLEAR). It is then possible to check the parameters
  have been correctly applied.

> tpm dam_reset_counter [<pw>]
> tpm dam_set_parameters 0xffff 1 0 [<pw>]
> tpm get_capability 0x0006 0x020e 0x4000000 4

* PCR policy may be changed (untested).
  PCR can be extended (no protection against packet replay yet).
  PCR can be read (the counter with the number of "extensions" is also
  given).

> tpm pcr_setauthpolicy 0 12345678901234567890123456789012 [<pw>]
> tpm pcr_read 0 0x4000000
> tpm pcr_extend 0 0x4000000


Miquel Raynal (18):
  tpm: add Revision ID field in the chip structure
  tpm: rename tpm_tis_infineon in tpm_tis_infineon_i2c
  tpm: add support for TPMv2 SPI modules
  tpm: fix indentation in command list before adding more
  tpm: prepare support for TPMv2 commands
  tpm: add macros for TPMv2 commands
  tpm: add possible traces to analyze buffers returned by the TPM
  tpm: handle different buffer sizes
  tpm: add TPM2_Startup command support
  tpm: add TPM2_SelfTest command support
  tpm: add TPM2_Clear command support
  tpm: rename the _extend() function to be _pcr_event()
  tpm: add TPM2_PCR_Extend command support
  tpm: add TPM2_PCR_Read command support
  tpm: add TPM2_GetCapability command support
  tpm: add dictionary attack mitigation commands support
  tpm: add TPM2_HierarchyChangeAuth command support
  tpm: add PCR authentication commands support

 cmd/tpm.c                                          | 360 +++++++++--
 cmd/tpm_test.c                                     |  10 +-
 drivers/tpm/Kconfig                                |  13 +-
 drivers/tpm/Makefile                               |   3 +-
 drivers/tpm/tpm_tis.h                              |   4 +
 .../{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} |   2 +-
 drivers/tpm/tpm_tis_spi.c                          | 656 +++++++++++++++++++++
 include/tpm.h                                      | 183 +++++-
 lib/tpm.c                                          | 654 ++++++++++++++++++--
 9 files changed, 1739 insertions(+), 146 deletions(-)
 rename drivers/tpm/{tpm_tis_infineon.c => tpm_tis_infineon_i2c.c} (99%)
 create mode 100644 drivers/tpm/tpm_tis_spi.c

Comments

Tom Rini March 8, 2018, 5:20 p.m. UTC | #1
On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:

> Current U-Boot supports TPM v1.2 specification. The new specification
> (v2.0) is not backward compatible and renames/introduces several
> functions.
> 
> This series introduces a new SPI driver following the TPM v2.0
> specification. It has been tested on a ST TPM but should be usable with
> others v2.0 compliant chips.
> 
> Then, basic functionalities are introduced one by one for the v2.0
> specification. The INIT command now can receive a parameter to
> distinguish further TPMv1/TPMv2 commands. After that, the library itself
> will know which one is pertinent and will return a special error if the
> desired command is not supported for the selected specification.

Thanks for doing all of this.  Can you please enable this feature on
sandbox and/or an x86 QEMU variant where I assume we could also then
setup automated testing?
Miquel Raynal March 9, 2018, 7:53 a.m. UTC | #2
Hi Tom,

On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:

> On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> 
> > Current U-Boot supports TPM v1.2 specification. The new specification
> > (v2.0) is not backward compatible and renames/introduces several
> > functions.
> > 
> > This series introduces a new SPI driver following the TPM v2.0
> > specification. It has been tested on a ST TPM but should be usable with
> > others v2.0 compliant chips.
> > 
> > Then, basic functionalities are introduced one by one for the v2.0
> > specification. The INIT command now can receive a parameter to
> > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > will know which one is pertinent and will return a special error if the
> > desired command is not supported for the selected specification.  
> 
> Thanks for doing all of this.  Can you please enable this feature on
> sandbox and/or an x86 QEMU variant where I assume we could also then
> setup automated testing?
> 

Not sure I understand your request correctly: the TPM commands are
already available in the sandbox (I don't see what I could add), I just
extended the current set of commands.

However, even with these commands, we won't be able to test them in a
sandbox unless with an actual device.

I probably miss something, can you explain a bit more what you would
like?

Thank you,
Miquèl
Tom Rini March 9, 2018, 12:18 p.m. UTC | #3
On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:
> Hi Tom,
> 
> On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> > 
> > > Current U-Boot supports TPM v1.2 specification. The new specification
> > > (v2.0) is not backward compatible and renames/introduces several
> > > functions.
> > > 
> > > This series introduces a new SPI driver following the TPM v2.0
> > > specification. It has been tested on a ST TPM but should be usable with
> > > others v2.0 compliant chips.
> > > 
> > > Then, basic functionalities are introduced one by one for the v2.0
> > > specification. The INIT command now can receive a parameter to
> > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > > will know which one is pertinent and will return a special error if the
> > > desired command is not supported for the selected specification.  
> > 
> > Thanks for doing all of this.  Can you please enable this feature on
> > sandbox and/or an x86 QEMU variant where I assume we could also then
> > setup automated testing?
> > 
> 
> Not sure I understand your request correctly: the TPM commands are
> already available in the sandbox (I don't see what I could add), I just
> extended the current set of commands.
> 
> However, even with these commands, we won't be able to test them in a
> sandbox unless with an actual device.
> 
> I probably miss something, can you explain a bit more what you would
> like?

Can we add a valid TPM via QEMU and then test it that way?  If so, we
should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
on other arches, other QEMU targets) and write some test/py/tests/ code
that exercises the TPM commands.  Does that make sense?
Miquel Raynal March 20, 2018, 1:36 p.m. UTC | #4
Hi Tom,

Sorry for the delay.

On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:

> On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:
> > Hi Tom,
> > 
> > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
> >   
> > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> > >   
> > > > Current U-Boot supports TPM v1.2 specification. The new specification
> > > > (v2.0) is not backward compatible and renames/introduces several
> > > > functions.
> > > > 
> > > > This series introduces a new SPI driver following the TPM v2.0
> > > > specification. It has been tested on a ST TPM but should be usable with
> > > > others v2.0 compliant chips.
> > > > 
> > > > Then, basic functionalities are introduced one by one for the v2.0
> > > > specification. The INIT command now can receive a parameter to
> > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > > > will know which one is pertinent and will return a special error if the
> > > > desired command is not supported for the selected specification.    
> > > 
> > > Thanks for doing all of this.  Can you please enable this feature on
> > > sandbox and/or an x86 QEMU variant where I assume we could also then
> > > setup automated testing?
> > >   
> > 
> > Not sure I understand your request correctly: the TPM commands are
> > already available in the sandbox (I don't see what I could add), I just
> > extended the current set of commands.
> > 
> > However, even with these commands, we won't be able to test them in a
> > sandbox unless with an actual device.
> > 
> > I probably miss something, can you explain a bit more what you would
> > like?  
> 
> Can we add a valid TPM via QEMU and then test it that way?  If so, we
> should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
> on other arches, other QEMU targets) and write some test/py/tests/ code
> that exercises the TPM commands.  Does that make sense?
> 

I suppose this is doable, but for what I know, the effort is
consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
packets exchanged at TPM level are completely different. Hence, I
think there is almost nothing that we can take from the TPM 1.x
implementation already existing in QEMU.

I am certain we all would benefit such a contribution, however I'm
not sure I could handle that anytime soon.

About the series, I think it would be better that I change a macro name
("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
tell me its status otherwise?

Thank you,
Miquèl
Tom Rini March 20, 2018, 2:04 p.m. UTC | #5
On Tue, Mar 20, 2018 at 02:36:56PM +0100, Miquel Raynal wrote:
> Hi Tom,
> 
> Sorry for the delay.
> 
> On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:
> > > Hi Tom,
> > > 
> > > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
> > >   
> > > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> > > >   
> > > > > Current U-Boot supports TPM v1.2 specification. The new specification
> > > > > (v2.0) is not backward compatible and renames/introduces several
> > > > > functions.
> > > > > 
> > > > > This series introduces a new SPI driver following the TPM v2.0
> > > > > specification. It has been tested on a ST TPM but should be usable with
> > > > > others v2.0 compliant chips.
> > > > > 
> > > > > Then, basic functionalities are introduced one by one for the v2.0
> > > > > specification. The INIT command now can receive a parameter to
> > > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > > > > will know which one is pertinent and will return a special error if the
> > > > > desired command is not supported for the selected specification.    
> > > > 
> > > > Thanks for doing all of this.  Can you please enable this feature on
> > > > sandbox and/or an x86 QEMU variant where I assume we could also then
> > > > setup automated testing?
> > > >   
> > > 
> > > Not sure I understand your request correctly: the TPM commands are
> > > already available in the sandbox (I don't see what I could add), I just
> > > extended the current set of commands.
> > > 
> > > However, even with these commands, we won't be able to test them in a
> > > sandbox unless with an actual device.
> > > 
> > > I probably miss something, can you explain a bit more what you would
> > > like?  
> > 
> > Can we add a valid TPM via QEMU and then test it that way?  If so, we
> > should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
> > on other arches, other QEMU targets) and write some test/py/tests/ code
> > that exercises the TPM commands.  Does that make sense?
> > 
> 
> I suppose this is doable, but for what I know, the effort is
> consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
> packets exchanged at TPM level are completely different. Hence, I
> think there is almost nothing that we can take from the TPM 1.x
> implementation already existing in QEMU.

Ah, OK.  I thought QEMU had a TPM 2.0 implementation now too, but I see
I'm mistaken.

> I am certain we all would benefit such a contribution, however I'm
> not sure I could handle that anytime soon.
> 
> About the series, I think it would be better that I change a macro name
> ("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
> tell me its status otherwise?

We have the usual linux/stringify.h header available, so yes, you should
make use of that.  And I still would like to see tests written, even if
they can only be executed on $board with $TPM attached via $interface,
with those 3 variables documented so that others can try it out too.
Does that make sense?  Thanks!
Miquel Raynal March 20, 2018, 2:51 p.m. UTC | #6
Hi Tom,

On Tue, 20 Mar 2018 10:04:55 -0400, Tom Rini <trini@konsulko.com> wrote:

> On Tue, Mar 20, 2018 at 02:36:56PM +0100, Miquel Raynal wrote:
> > Hi Tom,
> > 
> > Sorry for the delay.
> > 
> > On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:
> >   
> > > On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:  
> > > > Hi Tom,
> > > > 
> > > > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
> > > >     
> > > > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> > > > >     
> > > > > > Current U-Boot supports TPM v1.2 specification. The new specification
> > > > > > (v2.0) is not backward compatible and renames/introduces several
> > > > > > functions.
> > > > > > 
> > > > > > This series introduces a new SPI driver following the TPM v2.0
> > > > > > specification. It has been tested on a ST TPM but should be usable with
> > > > > > others v2.0 compliant chips.
> > > > > > 
> > > > > > Then, basic functionalities are introduced one by one for the v2.0
> > > > > > specification. The INIT command now can receive a parameter to
> > > > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > > > > > will know which one is pertinent and will return a special error if the
> > > > > > desired command is not supported for the selected specification.      
> > > > > 
> > > > > Thanks for doing all of this.  Can you please enable this feature on
> > > > > sandbox and/or an x86 QEMU variant where I assume we could also then
> > > > > setup automated testing?
> > > > >     
> > > > 
> > > > Not sure I understand your request correctly: the TPM commands are
> > > > already available in the sandbox (I don't see what I could add), I just
> > > > extended the current set of commands.
> > > > 
> > > > However, even with these commands, we won't be able to test them in a
> > > > sandbox unless with an actual device.
> > > > 
> > > > I probably miss something, can you explain a bit more what you would
> > > > like?    
> > > 
> > > Can we add a valid TPM via QEMU and then test it that way?  If so, we
> > > should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
> > > on other arches, other QEMU targets) and write some test/py/tests/ code
> > > that exercises the TPM commands.  Does that make sense?
> > >   
> > 
> > I suppose this is doable, but for what I know, the effort is
> > consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
> > packets exchanged at TPM level are completely different. Hence, I
> > think there is almost nothing that we can take from the TPM 1.x
> > implementation already existing in QEMU.  
> 
> Ah, OK.  I thought QEMU had a TPM 2.0 implementation now too, but I see
> I'm mistaken.
> 
> > I am certain we all would benefit such a contribution, however I'm
> > not sure I could handle that anytime soon.
> > 
> > About the series, I think it would be better that I change a macro name
> > ("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
> > tell me its status otherwise?  
> 
> We have the usual linux/stringify.h header available, so yes, you should
> make use of that.

Actually the name is misleading as I don't want to "stringify".

I am looking for a way to easily fill a buffer of bytes from integer
values, ie:

u32 value = 0x12345678;
u8 buf[x] = { MACRO(value), ...} to be {0x12, 0x34, 0x56, 0x78, ...}


>  And I still would like to see tests written, even if
> they can only be executed on $board with $TPM attached via $interface,
> with those 3 variables documented so that others can try it out too.
> Does that make sense?  Thanks!

I see some TPM tests for v1.x, I can probably add some there. This will
test the library functions but not the "user" commands.

To test the commands, I suggest following the lines I inserted in my
cover letter, but maybe I can put it also in some documentation?

Would this fit your expectations?

[1] https://lists.denx.de/pipermail/u-boot/2018-March/322286.html

Thanks,
Miquèl
Tom Rini March 21, 2018, 1:49 p.m. UTC | #7
On Tue, Mar 20, 2018 at 03:51:22PM +0100, Miquel Raynal wrote:
> Hi Tom,
> 
> On Tue, 20 Mar 2018 10:04:55 -0400, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Tue, Mar 20, 2018 at 02:36:56PM +0100, Miquel Raynal wrote:
> > > Hi Tom,
> > > 
> > > Sorry for the delay.
> > > 
> > > On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:
> > >   
> > > > On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:  
> > > > > Hi Tom,
> > > > > 
> > > > > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
> > > > >     
> > > > > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> > > > > >     
> > > > > > > Current U-Boot supports TPM v1.2 specification. The new specification
> > > > > > > (v2.0) is not backward compatible and renames/introduces several
> > > > > > > functions.
> > > > > > > 
> > > > > > > This series introduces a new SPI driver following the TPM v2.0
> > > > > > > specification. It has been tested on a ST TPM but should be usable with
> > > > > > > others v2.0 compliant chips.
> > > > > > > 
> > > > > > > Then, basic functionalities are introduced one by one for the v2.0
> > > > > > > specification. The INIT command now can receive a parameter to
> > > > > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> > > > > > > will know which one is pertinent and will return a special error if the
> > > > > > > desired command is not supported for the selected specification.      
> > > > > > 
> > > > > > Thanks for doing all of this.  Can you please enable this feature on
> > > > > > sandbox and/or an x86 QEMU variant where I assume we could also then
> > > > > > setup automated testing?
> > > > > >     
> > > > > 
> > > > > Not sure I understand your request correctly: the TPM commands are
> > > > > already available in the sandbox (I don't see what I could add), I just
> > > > > extended the current set of commands.
> > > > > 
> > > > > However, even with these commands, we won't be able to test them in a
> > > > > sandbox unless with an actual device.
> > > > > 
> > > > > I probably miss something, can you explain a bit more what you would
> > > > > like?    
> > > > 
> > > > Can we add a valid TPM via QEMU and then test it that way?  If so, we
> > > > should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
> > > > on other arches, other QEMU targets) and write some test/py/tests/ code
> > > > that exercises the TPM commands.  Does that make sense?
> > > >   
> > > 
> > > I suppose this is doable, but for what I know, the effort is
> > > consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
> > > packets exchanged at TPM level are completely different. Hence, I
> > > think there is almost nothing that we can take from the TPM 1.x
> > > implementation already existing in QEMU.  
> > 
> > Ah, OK.  I thought QEMU had a TPM 2.0 implementation now too, but I see
> > I'm mistaken.
> > 
> > > I am certain we all would benefit such a contribution, however I'm
> > > not sure I could handle that anytime soon.
> > > 
> > > About the series, I think it would be better that I change a macro name
> > > ("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
> > > tell me its status otherwise?  
> > 
> > We have the usual linux/stringify.h header available, so yes, you should
> > make use of that.
> 
> Actually the name is misleading as I don't want to "stringify".
> 
> I am looking for a way to easily fill a buffer of bytes from integer
> values, ie:
> 
> u32 value = 0x12345678;
> u8 buf[x] = { MACRO(value), ...} to be {0x12, 0x34, 0x56, 0x78, ...}
> 
> 
> >  And I still would like to see tests written, even if
> > they can only be executed on $board with $TPM attached via $interface,
> > with those 3 variables documented so that others can try it out too.
> > Does that make sense?  Thanks!
> 
> I see some TPM tests for v1.x, I can probably add some there. This will
> test the library functions but not the "user" commands.
> 
> To test the commands, I suggest following the lines I inserted in my
> cover letter, but maybe I can put it also in some documentation?
> 
> Would this fit your expectations?

We have a framework to run those commands on the target and confirm that
they behave as expected.  Please write the tests to run those commands
and confirm that they work as expected.  Thanks!
Simon Glass March 23, 2018, 2:42 p.m. UTC | #8
Hi Miquel,

On 21 March 2018 at 07:49, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Mar 20, 2018 at 03:51:22PM +0100, Miquel Raynal wrote:
>> Hi Tom,
>>
>> On Tue, 20 Mar 2018 10:04:55 -0400, Tom Rini <trini@konsulko.com> wrote:
>>
>> > On Tue, Mar 20, 2018 at 02:36:56PM +0100, Miquel Raynal wrote:
>> > > Hi Tom,
>> > >
>> > > Sorry for the delay.
>> > >
>> > > On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:
>> > >
>> > > > On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:
>> > > > > Hi Tom,
>> > > > >
>> > > > > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
>> > > > >
>> > > > > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
>> > > > > >
>> > > > > > > Current U-Boot supports TPM v1.2 specification. The new specification
>> > > > > > > (v2.0) is not backward compatible and renames/introduces several
>> > > > > > > functions.
>> > > > > > >
>> > > > > > > This series introduces a new SPI driver following the TPM v2.0
>> > > > > > > specification. It has been tested on a ST TPM but should be usable with
>> > > > > > > others v2.0 compliant chips.
>> > > > > > >
>> > > > > > > Then, basic functionalities are introduced one by one for the v2.0
>> > > > > > > specification. The INIT command now can receive a parameter to
>> > > > > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
>> > > > > > > will know which one is pertinent and will return a special error if the
>> > > > > > > desired command is not supported for the selected specification.
>> > > > > >
>> > > > > > Thanks for doing all of this.  Can you please enable this feature on
>> > > > > > sandbox and/or an x86 QEMU variant where I assume we could also then
>> > > > > > setup automated testing?
>> > > > > >
>> > > > >
>> > > > > Not sure I understand your request correctly: the TPM commands are
>> > > > > already available in the sandbox (I don't see what I could add), I just
>> > > > > extended the current set of commands.
>> > > > >
>> > > > > However, even with these commands, we won't be able to test them in a
>> > > > > sandbox unless with an actual device.
>> > > > >
>> > > > > I probably miss something, can you explain a bit more what you would
>> > > > > like?
>> > > >
>> > > > Can we add a valid TPM via QEMU and then test it that way?  If so, we
>> > > > should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
>> > > > on other arches, other QEMU targets) and write some test/py/tests/ code
>> > > > that exercises the TPM commands.  Does that make sense?
>> > > >
>> > >
>> > > I suppose this is doable, but for what I know, the effort is
>> > > consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
>> > > packets exchanged at TPM level are completely different. Hence, I
>> > > think there is almost nothing that we can take from the TPM 1.x
>> > > implementation already existing in QEMU.
>> >
>> > Ah, OK.  I thought QEMU had a TPM 2.0 implementation now too, but I see
>> > I'm mistaken.
>> >
>> > > I am certain we all would benefit such a contribution, however I'm
>> > > not sure I could handle that anytime soon.
>> > >
>> > > About the series, I think it would be better that I change a macro name
>> > > ("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
>> > > tell me its status otherwise?
>> >
>> > We have the usual linux/stringify.h header available, so yes, you should
>> > make use of that.
>>
>> Actually the name is misleading as I don't want to "stringify".
>>
>> I am looking for a way to easily fill a buffer of bytes from integer
>> values, ie:
>>
>> u32 value = 0x12345678;
>> u8 buf[x] = { MACRO(value), ...} to be {0x12, 0x34, 0x56, 0x78, ...}
>>
>>
>> >  And I still would like to see tests written, even if
>> > they can only be executed on $board with $TPM attached via $interface,
>> > with those 3 variables documented so that others can try it out too.
>> > Does that make sense?  Thanks!
>>
>> I see some TPM tests for v1.x, I can probably add some there. This will
>> test the library functions but not the "user" commands.
>>
>> To test the commands, I suggest following the lines I inserted in my
>> cover letter, but maybe I can put it also in some documentation?
>>
>> Would this fit your expectations?
>
> We have a framework to run those commands on the target and confirm that
> they behave as expected.  Please write the tests to run those commands
> and confirm that they work as expected.  Thanks!

Re sandbox, it has a TPM emulator used for testing in
tpm_tis_sandbox.c - you should be able to add something similar for
v2.

Regards,
Simon
Miquel Raynal March 29, 2018, 7:39 a.m. UTC | #9
Hi Simon and Tom,

On Fri, 23 Mar 2018 08:42:02 -0600, Simon Glass <sjg@chromium.org>
wrote:

> Hi Miquel,
> 
> On 21 March 2018 at 07:49, Tom Rini <trini@konsulko.com> wrote:
> > On Tue, Mar 20, 2018 at 03:51:22PM +0100, Miquel Raynal wrote:  
> >> Hi Tom,
> >>
> >> On Tue, 20 Mar 2018 10:04:55 -0400, Tom Rini <trini@konsulko.com> wrote:
> >>  
> >> > On Tue, Mar 20, 2018 at 02:36:56PM +0100, Miquel Raynal wrote:  
> >> > > Hi Tom,
> >> > >
> >> > > Sorry for the delay.
> >> > >
> >> > > On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:
> >> > >  
> >> > > > On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:  
> >> > > > > Hi Tom,
> >> > > > >
> >> > > > > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
> >> > > > >  
> >> > > > > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
> >> > > > > >  
> >> > > > > > > Current U-Boot supports TPM v1.2 specification. The new specification
> >> > > > > > > (v2.0) is not backward compatible and renames/introduces several
> >> > > > > > > functions.
> >> > > > > > >
> >> > > > > > > This series introduces a new SPI driver following the TPM v2.0
> >> > > > > > > specification. It has been tested on a ST TPM but should be usable with
> >> > > > > > > others v2.0 compliant chips.
> >> > > > > > >
> >> > > > > > > Then, basic functionalities are introduced one by one for the v2.0
> >> > > > > > > specification. The INIT command now can receive a parameter to
> >> > > > > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
> >> > > > > > > will know which one is pertinent and will return a special error if the
> >> > > > > > > desired command is not supported for the selected specification.  
> >> > > > > >
> >> > > > > > Thanks for doing all of this.  Can you please enable this feature on
> >> > > > > > sandbox and/or an x86 QEMU variant where I assume we could also then
> >> > > > > > setup automated testing?
> >> > > > > >  
> >> > > > >
> >> > > > > Not sure I understand your request correctly: the TPM commands are
> >> > > > > already available in the sandbox (I don't see what I could add), I just
> >> > > > > extended the current set of commands.
> >> > > > >
> >> > > > > However, even with these commands, we won't be able to test them in a
> >> > > > > sandbox unless with an actual device.
> >> > > > >
> >> > > > > I probably miss something, can you explain a bit more what you would
> >> > > > > like?  
> >> > > >
> >> > > > Can we add a valid TPM via QEMU and then test it that way?  If so, we
> >> > > > should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
> >> > > > on other arches, other QEMU targets) and write some test/py/tests/ code
> >> > > > that exercises the TPM commands.  Does that make sense?
> >> > > >  
> >> > >
> >> > > I suppose this is doable, but for what I know, the effort is
> >> > > consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
> >> > > packets exchanged at TPM level are completely different. Hence, I
> >> > > think there is almost nothing that we can take from the TPM 1.x
> >> > > implementation already existing in QEMU.  
> >> >
> >> > Ah, OK.  I thought QEMU had a TPM 2.0 implementation now too, but I see
> >> > I'm mistaken.
> >> >  
> >> > > I am certain we all would benefit such a contribution, however I'm
> >> > > not sure I could handle that anytime soon.
> >> > >
> >> > > About the series, I think it would be better that I change a macro name
> >> > > ("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
> >> > > tell me its status otherwise?  
> >> >
> >> > We have the usual linux/stringify.h header available, so yes, you should
> >> > make use of that.  
> >>
> >> Actually the name is misleading as I don't want to "stringify".
> >>
> >> I am looking for a way to easily fill a buffer of bytes from integer
> >> values, ie:
> >>
> >> u32 value = 0x12345678;
> >> u8 buf[x] = { MACRO(value), ...} to be {0x12, 0x34, 0x56, 0x78, ...}
> >>
> >>  
> >> >  And I still would like to see tests written, even if
> >> > they can only be executed on $board with $TPM attached via $interface,
> >> > with those 3 variables documented so that others can try it out too.
> >> > Does that make sense?  Thanks!  
> >>
> >> I see some TPM tests for v1.x, I can probably add some there. This will
> >> test the library functions but not the "user" commands.
> >>
> >> To test the commands, I suggest following the lines I inserted in my
> >> cover letter, but maybe I can put it also in some documentation?
> >>
> >> Would this fit your expectations?  
> >
> > We have a framework to run those commands on the target and confirm that
> > they behave as expected.  Please write the tests to run those commands
> > and confirm that they work as expected.  Thanks!  
> 
> Re sandbox, it has a TPM emulator used for testing in
> tpm_tis_sandbox.c - you should be able to add something similar for
> v2.

I wrote a complete test suite for all the TPMv2.0 commands. It runs
completely fine with real hardware, see the last patch of the v2 series.
I had a look to sandbox but I'm sorry, I really don't have enough time
for now to achieve this additional work, I really hope the full Python
test suite will be enough.

Regards,
Miquèl
Simon Glass March 29, 2018, 10:41 p.m. UTC | #10
Hi Miquel,

On 29 March 2018 at 15:39, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Simon and Tom,
>
> On Fri, 23 Mar 2018 08:42:02 -0600, Simon Glass <sjg@chromium.org>
> wrote:
>
>> Hi Miquel,
>>
>> On 21 March 2018 at 07:49, Tom Rini <trini@konsulko.com> wrote:
>> > On Tue, Mar 20, 2018 at 03:51:22PM +0100, Miquel Raynal wrote:
>> >> Hi Tom,
>> >>
>> >> On Tue, 20 Mar 2018 10:04:55 -0400, Tom Rini <trini@konsulko.com> wrote:
>> >>
>> >> > On Tue, Mar 20, 2018 at 02:36:56PM +0100, Miquel Raynal wrote:
>> >> > > Hi Tom,
>> >> > >
>> >> > > Sorry for the delay.
>> >> > >
>> >> > > On Fri, 9 Mar 2018 07:18:40 -0500, Tom Rini <trini@konsulko.com> wrote:
>> >> > >
>> >> > > > On Fri, Mar 09, 2018 at 08:53:40AM +0100, Miquel Raynal wrote:
>> >> > > > > Hi Tom,
>> >> > > > >
>> >> > > > > On Thu, 8 Mar 2018 12:20:30 -0500, Tom Rini <trini@konsulko.com> wrote:
>> >> > > > >
>> >> > > > > > On Thu, Mar 08, 2018 at 04:40:03PM +0100, Miquel Raynal wrote:
>> >> > > > > >
>> >> > > > > > > Current U-Boot supports TPM v1.2 specification. The new specification
>> >> > > > > > > (v2.0) is not backward compatible and renames/introduces several
>> >> > > > > > > functions.
>> >> > > > > > >
>> >> > > > > > > This series introduces a new SPI driver following the TPM v2.0
>> >> > > > > > > specification. It has been tested on a ST TPM but should be usable with
>> >> > > > > > > others v2.0 compliant chips.
>> >> > > > > > >
>> >> > > > > > > Then, basic functionalities are introduced one by one for the v2.0
>> >> > > > > > > specification. The INIT command now can receive a parameter to
>> >> > > > > > > distinguish further TPMv1/TPMv2 commands. After that, the library itself
>> >> > > > > > > will know which one is pertinent and will return a special error if the
>> >> > > > > > > desired command is not supported for the selected specification.
>> >> > > > > >
>> >> > > > > > Thanks for doing all of this.  Can you please enable this feature on
>> >> > > > > > sandbox and/or an x86 QEMU variant where I assume we could also then
>> >> > > > > > setup automated testing?
>> >> > > > > >
>> >> > > > >
>> >> > > > > Not sure I understand your request correctly: the TPM commands are
>> >> > > > > already available in the sandbox (I don't see what I could add), I just
>> >> > > > > extended the current set of commands.
>> >> > > > >
>> >> > > > > However, even with these commands, we won't be able to test them in a
>> >> > > > > sandbox unless with an actual device.
>> >> > > > >
>> >> > > > > I probably miss something, can you explain a bit more what you would
>> >> > > > > like?
>> >> > > >
>> >> > > > Can we add a valid TPM via QEMU and then test it that way?  If so, we
>> >> > > > should enable the TPM code on qemu-x86_64 (and, well, if we can pass it
>> >> > > > on other arches, other QEMU targets) and write some test/py/tests/ code
>> >> > > > that exercises the TPM commands.  Does that make sense?
>> >> > > >
>> >> > >
>> >> > > I suppose this is doable, but for what I know, the effort is
>> >> > > consequent. TPM 2.0 are not compatible at all with TPM 1.x , the
>> >> > > packets exchanged at TPM level are completely different. Hence, I
>> >> > > think there is almost nothing that we can take from the TPM 1.x
>> >> > > implementation already existing in QEMU.
>> >> >
>> >> > Ah, OK.  I thought QEMU had a TPM 2.0 implementation now too, but I see
>> >> > I'm mistaken.
>> >> >
>> >> > > I am certain we all would benefit such a contribution, however I'm
>> >> > > not sure I could handle that anytime soon.
>> >> > >
>> >> > > About the series, I think it would be better that I change a macro name
>> >> > > ("STRINGIFY", which is wrongly named), I will send a v2 soon, can you
>> >> > > tell me its status otherwise?
>> >> >
>> >> > We have the usual linux/stringify.h header available, so yes, you should
>> >> > make use of that.
>> >>
>> >> Actually the name is misleading as I don't want to "stringify".
>> >>
>> >> I am looking for a way to easily fill a buffer of bytes from integer
>> >> values, ie:
>> >>
>> >> u32 value = 0x12345678;
>> >> u8 buf[x] = { MACRO(value), ...} to be {0x12, 0x34, 0x56, 0x78, ...}
>> >>
>> >>
>> >> >  And I still would like to see tests written, even if
>> >> > they can only be executed on $board with $TPM attached via $interface,
>> >> > with those 3 variables documented so that others can try it out too.
>> >> > Does that make sense?  Thanks!
>> >>
>> >> I see some TPM tests for v1.x, I can probably add some there. This will
>> >> test the library functions but not the "user" commands.
>> >>
>> >> To test the commands, I suggest following the lines I inserted in my
>> >> cover letter, but maybe I can put it also in some documentation?
>> >>
>> >> Would this fit your expectations?
>> >
>> > We have a framework to run those commands on the target and confirm that
>> > they behave as expected.  Please write the tests to run those commands
>> > and confirm that they work as expected.  Thanks!
>>
>> Re sandbox, it has a TPM emulator used for testing in
>> tpm_tis_sandbox.c - you should be able to add something similar for
>> v2.
>
> I wrote a complete test suite for all the TPMv2.0 commands. It runs
> completely fine with real hardware, see the last patch of the v2 series.
> I had a look to sandbox but I'm sorry, I really don't have enough time
> for now to achieve this additional work, I really hope the full Python
> test suite will be enough.

This should not take very long to do. I am not keen to add this new
support without sandbox tests. The Python test suite is a great idea,
but running it on real hardware is not an option for most people.

Lots of people have made big efforts to improve the sandbox testing in
U-Boot. We need to keep pushing it forward, not regressing.

Regards,
Simon