linux-kernel - Re: [PATCH v7 0/6] tpm: Add driver for cr50

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191006223831.GA10397@linux.intel.com>
Date:   Mon, 7 Oct 2019 01:39:00 +0300
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Peter Huewe <peterhuewe@....de>, linux-kernel@...r.kernel.org,
        linux-integrity@...r.kernel.org,
        Andrey Pronin <apronin@...omium.org>,
        Duncan Laurie <dlaurie@...omium.org>,
        Jason Gunthorpe <jgg@...pe.ca>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Guenter Roeck <groeck@...omium.org>,
        Alexander Steffen <Alexander.Steffen@...ineon.com>,
        Heiko Stuebner <heiko@...ech.de>
Subject: Re: [PATCH v7 0/6] tpm: Add driver for cr50

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/1469757314-116169-1-git-send-email-apronin@chromium.org
> 
> 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/20190829224110.91103-1-swboyd@chromium.org):
>  * 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/20190828082150.42194-1-swboyd@chromium.org):
>  * 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/20190812223622.73297-1-swboyd@chromium.org):
>  * 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 <apronin@...omium.org>
> Cc: Duncan Laurie <dlaurie@...omium.org>
> Cc: Jason Gunthorpe <jgg@...pe.ca>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Guenter Roeck <groeck@...omium.org>
> Cc: Alexander Steffen <Alexander.Steffen@...ineon.com>
> Cc: Heiko Stuebner <heiko@...ech.de>

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