[v7,0/6] tpm: Add driver for cr50

Message

Stephen Boyd Sept. 20, 2019, 6:32 p.m. UTC

This patch series adds support for the H1 secure microcontroller
running cr50 firmware found on various recent Chromebooks. This driver
is necessary to boot into a ChromeOS userspace environment. It
implements support for several functions, including TPM-like
functionality over a SPI interface.

The last time this was series sent looks to be [1]. I've looked over the
patches and review comments and tried to address any feedback that
Andrey didn't address (really minor things like newlines). I've reworked
the patches from the last version to layer on top of the existing TPM
TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
palatable than combining the two drivers together into one file.

Please review so we can get the approach to supporting this device
sorted out.

[1] https://lkml.kernel.org/r/[e-mail]

TODO:
 * Add a patch to spit out WARN_ON() when TPM is suspended and some
   kernel code attempts to use it
 * Rework the i2c driver per Alexander's comments on v2

Changes from v6 (https://lkml.kernel.org/r/[e-mail]):
 * Two new patches to cleanup includes and module usage
 * Moved cr50 C file to tpm_tis_spi_cr50.c
 * Used the tpm_tis_spi_mod target approach to make the module work
 * Brought back Kconfig option to allow user to disable cr50 code
 * Rebased to v5.3

Changes from v5 (https://lkml.kernel.org/r/[e-mail]):
 * Picked up Jarkko's ack/review tags
 * Fixed bug with irqs happening before completion is initialized
 * Dropped is_cr50 bool
 * Moved wake_after to tpm_tis_spi struct
 * Changed authorship of main cr50 patch to Andrey as I'm just shuffling
   code here

Changes from v4 (https://lkml.kernel.org/r/[e-mail]):
 * Dropped the 'pre-transfer' hook patch and added a 'ready' member instead
 * Combined cr50_spi and tpm_tis_spi into one kernel module
 * Introduced a swizzle in tpm_tis_spi probe routine to jump to cr50
   probe path
 * Moved binding to start of the thread
 * Picked up Jarkko reviewed-by tag on new flag for suspend/resume
 * Added a comment to flow control patch indicating what it's all about

Changes from v3:
 * Split out hooks into separate patches
 * Update commit text to not say "libify"
 * Collapse if statement into one for first patch
 * Update commit text on first patch to mention flag
 * Drop TIS_IS_CR50 as it's unused

Changes from v2:
 * Sent khwrng thread patch separately
 * New patch to expose TPM SPI functionality from tpm_tis_spi.c
 * Usage of that new patch in cr50 SPI driver
 * Drop i2c version of cr50 SPI driver for now (will resend later)
 * New patch to add a TPM chip flag indicating TPM shouldn't be reset
   over suspend. Allows us to get rid of the cr50 suspend/resume functions
   that are mostly generic

Changes from v1:
 * Dropped symlink and sysfs patches
 * Removed 'is_suspended' bits
 * Added new patch to freeze khwrng thread
 * Moved binding to google,cr50.txt and added Reviewed-by tag from Rob

Cc: Andrey Pronin <[e-mail]>
Cc: Duncan Laurie <[e-mail]>
Cc: Jason Gunthorpe <[e-mail]>
Cc: Arnd Bergmann <[e-mail]>
Cc: Greg Kroah-Hartman <[e-mail]>
Cc: Guenter Roeck <[e-mail]>
Cc: Alexander Steffen <[e-mail]>
Cc: Heiko Stuebner <[e-mail]>

Andrey Pronin (2):
  dt-bindings: tpm: document properties for cr50
  tpm: tpm_tis_spi: Support cr50 devices

Stephen Boyd (4):
  tpm: Add a flag to indicate TPM power is managed by firmware
  tpm: tpm_tis_spi: Introduce a flow control callback
  tpm: tpm_tis_spi: Cleanup includes
  tpm: tpm_tis_spi: Drop THIS_MODULE usage from driver struct

 .../bindings/security/tpm/google,cr50.txt     |  19 ++
 drivers/char/tpm/Kconfig                      |   7 +
 drivers/char/tpm/Makefile                     |   4 +-
 drivers/char/tpm/tpm-interface.c              |   8 +-
 drivers/char/tpm/tpm.h                        |   1 +
 drivers/char/tpm/tpm_tis_spi.c                | 143 +++++---
 drivers/char/tpm/tpm_tis_spi.h                |  53 +++
 drivers/char/tpm/tpm_tis_spi_cr50.c           | 321 ++++++++++++++++++
 8 files changed, 498 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.txt
 create mode 100644 drivers/char/tpm/tpm_tis_spi.h
 create mode 100644 drivers/char/tpm/tpm_tis_spi_cr50.c


base-commit: 4d856f72c10ecb060868ed10ff1b1453943fc6c8

Comments

Jarkko Sakkinen Oct. 6, 2019, 10:39 p.m. UTC | #1

On Fri, Sep 20, 2019 at 11:32:34AM -0700, Stephen Boyd wrote:
> This patch series adds support for the H1 secure microcontroller
> running cr50 firmware found on various recent Chromebooks. This driver
> is necessary to boot into a ChromeOS userspace environment. It
> implements support for several functions, including TPM-like
> functionality over a SPI interface.
> 
> The last time this was series sent looks to be [1]. I've looked over the
> patches and review comments and tried to address any feedback that
> Andrey didn't address (really minor things like newlines). I've reworked
> the patches from the last version to layer on top of the existing TPM
> TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
> palatable than combining the two drivers together into one file.
> 
> Please review so we can get the approach to supporting this device
> sorted out.
> 
> [1] https://lkml.kernel.org/r/[e-mail]
> 
> TODO:
>  * Add a patch to spit out WARN_ON() when TPM is suspended and some
>    kernel code attempts to use it
>  * Rework the i2c driver per Alexander's comments on v2
> 
> Changes from v6 (https://lkml.kernel.org/r/[e-mail]):
>  * Two new patches to cleanup includes and module usage
>  * Moved cr50 C file to tpm_tis_spi_cr50.c
>  * Used the tpm_tis_spi_mod target approach to make the module work
>  * Brought back Kconfig option to allow user to disable cr50 code
>  * Rebased to v5.3
> 
> Changes from v5 (https://lkml.kernel.org/r/[e-mail]):
>  * Picked up Jarkko's ack/review tags
>  * Fixed bug with irqs happening before completion is initialized
>  * Dropped is_cr50 bool
>  * Moved wake_after to tpm_tis_spi struct
>  * Changed authorship of main cr50 patch to Andrey as I'm just shuffling
>    code here
> 
> Changes from v4 (https://lkml.kernel.org/r/[e-mail]):
>  * Dropped the 'pre-transfer' hook patch and added a 'ready' member instead
>  * Combined cr50_spi and tpm_tis_spi into one kernel module
>  * Introduced a swizzle in tpm_tis_spi probe routine to jump to cr50
>    probe path
>  * Moved binding to start of the thread
>  * Picked up Jarkko reviewed-by tag on new flag for suspend/resume
>  * Added a comment to flow control patch indicating what it's all about
> 
> Changes from v3:
>  * Split out hooks into separate patches
>  * Update commit text to not say "libify"
>  * Collapse if statement into one for first patch
>  * Update commit text on first patch to mention flag
>  * Drop TIS_IS_CR50 as it's unused
> 
> Changes from v2:
>  * Sent khwrng thread patch separately
>  * New patch to expose TPM SPI functionality from tpm_tis_spi.c
>  * Usage of that new patch in cr50 SPI driver
>  * Drop i2c version of cr50 SPI driver for now (will resend later)
>  * New patch to add a TPM chip flag indicating TPM shouldn't be reset
>    over suspend. Allows us to get rid of the cr50 suspend/resume functions
>    that are mostly generic
> 
> Changes from v1:
>  * Dropped symlink and sysfs patches
>  * Removed 'is_suspended' bits
>  * Added new patch to freeze khwrng thread
>  * Moved binding to google,cr50.txt and added Reviewed-by tag from Rob
> 
> Cc: Andrey Pronin <[e-mail]>
> Cc: Duncan Laurie <[e-mail]>
> Cc: Jason Gunthorpe <[e-mail]>
> Cc: Arnd Bergmann <[e-mail]>
> Cc: Greg Kroah-Hartman <[e-mail]>
> Cc: Guenter Roeck <[e-mail]>
> Cc: Alexander Steffen <[e-mail]>
> Cc: Heiko Stuebner <[e-mail]>

OK, so, I put these to my master in hopes to get testing exposure.
I think the changes are in great shape now. Thank you.

/Jarkko

Heiko Stübner Oct. 11, 2019, 7:50 a.m. UTC | #2

Am Montag, 7. Oktober 2019, 00:39:00 CEST schrieb Jarkko Sakkinen:
> On Fri, Sep 20, 2019 at 11:32:34AM -0700, Stephen Boyd wrote:
> > This patch series adds support for the H1 secure microcontroller
> > running cr50 firmware found on various recent Chromebooks. This driver
> > is necessary to boot into a ChromeOS userspace environment. It
> > implements support for several functions, including TPM-like
> > functionality over a SPI interface.
> > 
> > The last time this was series sent looks to be [1]. I've looked over the
> > patches and review comments and tried to address any feedback that
> > Andrey didn't address (really minor things like newlines). I've reworked
> > the patches from the last version to layer on top of the existing TPM
> > TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
> > palatable than combining the two drivers together into one file.
> > 
> > Please review so we can get the approach to supporting this device
> > sorted out.
> > 
> > [1] https://lkml.kernel.org/r/[e-mail]

[...]

> OK, so, I put these to my master in hopes to get testing exposure.
> I think the changes are in great shape now. Thank you.

on a rk3399-gru-bob it works nicely for me, so
Tested-by: Heiko Stuebner <[e-mail]>

Thanks
Heiko

Jarkko Sakkinen Oct. 14, 2019, 7:56 p.m. UTC | #3

On Fri, Oct 11, 2019 at 09:50:27AM +0200, Heiko Stübner wrote:
> Am Montag, 7. Oktober 2019, 00:39:00 CEST schrieb Jarkko Sakkinen:
> > On Fri, Sep 20, 2019 at 11:32:34AM -0700, Stephen Boyd wrote:
> > > This patch series adds support for the H1 secure microcontroller
> > > running cr50 firmware found on various recent Chromebooks. This driver
> > > is necessary to boot into a ChromeOS userspace environment. It
> > > implements support for several functions, including TPM-like
> > > functionality over a SPI interface.
> > > 
> > > The last time this was series sent looks to be [1]. I've looked over the
> > > patches and review comments and tried to address any feedback that
> > > Andrey didn't address (really minor things like newlines). I've reworked
> > > the patches from the last version to layer on top of the existing TPM
> > > TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
> > > palatable than combining the two drivers together into one file.
> > > 
> > > Please review so we can get the approach to supporting this device
> > > sorted out.
> > > 
> > > [1] https://lkml.kernel.org/r/[e-mail]
> 
> [...]
> 
> > OK, so, I put these to my master in hopes to get testing exposure.
> > I think the changes are in great shape now. Thank you.
> 
> on a rk3399-gru-bob it works nicely for me, so
> Tested-by: Heiko Stuebner <[e-mail]>

Thank you! I updated my tree with your tag. Mind if I also add
reviewed-by's?

/Jarkko

Heiko Stübner Oct. 15, 2019, 8:23 p.m. UTC | #4

Hi,

Am Montag, 14. Oktober 2019, 21:56:30 CEST schrieb Jarkko Sakkinen:
> On Fri, Oct 11, 2019 at 09:50:27AM +0200, Heiko Stübner wrote:
> > Am Montag, 7. Oktober 2019, 00:39:00 CEST schrieb Jarkko Sakkinen:
> > > On Fri, Sep 20, 2019 at 11:32:34AM -0700, Stephen Boyd wrote:
> > > > This patch series adds support for the H1 secure microcontroller
> > > > running cr50 firmware found on various recent Chromebooks. This driver
> > > > is necessary to boot into a ChromeOS userspace environment. It
> > > > implements support for several functions, including TPM-like
> > > > functionality over a SPI interface.
> > > > 
> > > > The last time this was series sent looks to be [1]. I've looked over the
> > > > patches and review comments and tried to address any feedback that
> > > > Andrey didn't address (really minor things like newlines). I've reworked
> > > > the patches from the last version to layer on top of the existing TPM
> > > > TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
> > > > palatable than combining the two drivers together into one file.
> > > > 
> > > > Please review so we can get the approach to supporting this device
> > > > sorted out.
> > > > 
> > > > [1] https://lkml.kernel.org/r/[e-mail]
> > 
> > [...]
> > 
> > > OK, so, I put these to my master in hopes to get testing exposure.
> > > I think the changes are in great shape now. Thank you.
> > 
> > on a rk3399-gru-bob it works nicely for me, so
> > Tested-by: Heiko Stuebner <[e-mail]>
> 
> Thank you! I updated my tree with your tag. Mind if I also add
> reviewed-by's?

I think I did spent enough time with the patches to warrant that, so
Reviewed-by: Heiko Stuebner <[e-mail]>

Heiko

Jarkko Sakkinen Oct. 16, 2019, 3:27 p.m. UTC | #5

On Tue, Oct 15, 2019 at 10:23:15PM +0200, Heiko Stuebner wrote:
> Hi,
> 
> Am Montag, 14. Oktober 2019, 21:56:30 CEST schrieb Jarkko Sakkinen:
> > On Fri, Oct 11, 2019 at 09:50:27AM +0200, Heiko Stübner wrote:
> > > Am Montag, 7. Oktober 2019, 00:39:00 CEST schrieb Jarkko Sakkinen:
> > > > On Fri, Sep 20, 2019 at 11:32:34AM -0700, Stephen Boyd wrote:
> > > > > This patch series adds support for the H1 secure microcontroller
> > > > > running cr50 firmware found on various recent Chromebooks. This driver
> > > > > is necessary to boot into a ChromeOS userspace environment. It
> > > > > implements support for several functions, including TPM-like
> > > > > functionality over a SPI interface.
> > > > > 
> > > > > The last time this was series sent looks to be [1]. I've looked over the
> > > > > patches and review comments and tried to address any feedback that
> > > > > Andrey didn't address (really minor things like newlines). I've reworked
> > > > > the patches from the last version to layer on top of the existing TPM
> > > > > TIS SPI implementation in tpm_tis_spi.c. Hopefully this is more
> > > > > palatable than combining the two drivers together into one file.
> > > > > 
> > > > > Please review so we can get the approach to supporting this device
> > > > > sorted out.
> > > > > 
> > > > > [1] https://lkml.kernel.org/r/[e-mail]
> > > 
> > > [...]
> > > 
> > > > OK, so, I put these to my master in hopes to get testing exposure.
> > > > I think the changes are in great shape now. Thank you.
> > > 
> > > on a rk3399-gru-bob it works nicely for me, so
> > > Tested-by: Heiko Stuebner <[e-mail]>
> > 
> > Thank you! I updated my tree with your tag. Mind if I also add
> > reviewed-by's?
> 
> I think I did spent enough time with the patches to warrant that, so
> Reviewed-by: Heiko Stuebner <[e-mail]>

Thank you!

/Jarkko