linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Randy Dunlap <rdunlap@infradead.org>
To: AnilKumar Chimata <anilc@codeaurora.org>,
	andy.gross@linaro.org, david.brown@linaro.org,
	robh+dt@kernel.org, mark.rutland@arm.com,
	herbert@gondor.apana.org.au, davem@davemloft.net
Cc: linux-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Date: Wed, 17 Oct 2018 10:39:07 -0700	[thread overview]
Message-ID: <48c7da0d-4e57-bc36-a683-94088b3b8319@infradead.org> (raw)
In-Reply-To: <1539789476-6098-4-git-send-email-anilc@codeaurora.org>

On 10/17/18 8:17 AM, AnilKumar Chimata wrote:
> This patch adds support for Inline Crypto Engine (ICE), which
> is embedded into storage device/controller such as UFS/eMMC.
> ICE is intended for high throughput cryptographic encryption
> or decryption of storage data.
> 
> Signed-off-by: AnilKumar Chimata <anilc@codeaurora.org>
> ---
>  Documentation/crypto/msm/ice.txt |  235 ++++++
>  drivers/crypto/Kconfig           |   10 +
>  drivers/crypto/qce/Makefile      |    1 +
>  drivers/crypto/qce/ice.c         | 1613 ++++++++++++++++++++++++++++++++++++++
>  drivers/crypto/qce/iceregs.h     |  159 ++++
>  include/crypto/ice.h             |   80 ++
>  6 files changed, 2098 insertions(+)
>  create mode 100644 Documentation/crypto/msm/ice.txt
>  create mode 100644 drivers/crypto/qce/ice.c
>  create mode 100644 drivers/crypto/qce/iceregs.h
>  create mode 100644 include/crypto/ice.h
> 
> diff --git a/Documentation/crypto/msm/ice.txt b/Documentation/crypto/msm/ice.txt
> new file mode 100644
> index 0000000..58f7081
> --- /dev/null
> +++ b/Documentation/crypto/msm/ice.txt
> @@ -0,0 +1,235 @@
> +Introduction:
> +=============
> +Storage encryption has been one of the most required feature from security

                                                        features

> +point of view. QTI based storage encryption solution uses general purpose
> +crypto engine. While this kind of solution provide a decent amount of

                                              provides

> +performance, it falls short as storage speed is improving significantly
> +continuously. To overcome performance degradation, newer chips are going to
> +have Inline Crypto Engine (ICE) embedded into storage device. ICE is supposed
> +to meet the line speed of storage devices.
> +
> +Hardware Description
> +====================
> +ICE is a HW block that is embedded into storage device such as UFS/eMMC. By

s/HW/hardware/                                     devices

> +default, ICE works in bypass mode i.e. ICE HW does not perform any crypto
> +operation on data to be processed by storage device. If required, ICE can be
> +configured to perform crypto operation in one direction (i.e. either encryption
> +or decryption) or in both direction(both encryption & decryption).

                             directions (both ...

> +
> +When a switch between the operation modes(plain to crypto or crypto to plain)

                                       modes (plain ...

> +is desired for a particular partition, SW must complete all transactions for

s/SW/software/

> +that particular partition before switching the crypto mode i.e. no crypto, one

                                                  crypto mode;

> +direction crypto or both direction crypto operation. Requests for other

                            directions

> +partitions are not impacted due to crypto mode switch.
> +
> +ICE HW currently supports AES128/256 bit ECB & XTS mode encryption algorithms.
> +
> +Keys for crypto operations are loaded from SW. Keys are stored in a lookup
> +table(LUT) located inside ICE HW. Maximum of 32 keys can be loaded in ICE key
> +LUT. A Key inside the LUT can be referred using a key index.

                                    referred to using

> +
> +SW Description
> +==============
> +ICE HW has categorized ICE registers in 2 groups: those which can be accessed by
> +only secure side i.e. TZ and those which can be accessed by non-secure side such

at least tell the reader what TZ and HLOS mean...

> +as HLOS as well. This requires that ICE driver to be split in two pieces: one

                                  that the ICE driver be split into two pieces: one

> +running from TZ space and another from HLOS space.
> +
> +ICE driver from TZ would configure keys as requested by HLOS side.
> +
> +ICE driver on HLOS side is responsible for initialization of ICE HW.
> +
> +SW Architecture Diagram
> +=======================
> +Following are all the components involved in the ICE driver for control path:
> +
> ++++++++++++++++++++++++++++++++++++++++++
> ++               App layer               +
> ++++++++++++++++++++++++++++++++++++++++++
> ++             System layer              +
> ++   ++++++++         +++++++            +
> ++   + VOLD +         + PFM +            +
> ++   ++++++++         +++++++            +
> ++         ||         ||                 +
> ++         ||         ||                 +
> ++         \/         \/                 +
> ++     +++++++++++++++++++++++++++       +
> ++     + LibQSEECom / cryptfs_hw +       +
> ++     +++++++++++++++++++++++++++       +
> ++++++++++++++++++++++++++++++++++++++++++
> ++             Kernel                    +       +++++++++++++++++
> ++                                       +       +     KMS       +
> ++  +++++++  +++++++++++  +++++++++++    +       +++++++++++++++++
> ++  + ICE +  + Storage +  + QSEECom +    +       +   ICE Driver  +
> ++++++++++++++++++++++++++++++++++++++++++ <===> +++++++++++++++++
> +               ||                                    ||
> +               ||                                    ||
> +               \/                                    \/
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++                      Storage Device                           +
> ++                      ++++++++++++++                           +
> ++                      +   ICE HW   +                           +
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +
> +Use Cases:
> +----------
> +a) Device bootup
> +ICE HW is detected during bootup time and corresponding probe function is
> +called. ICE driver parses its data from device tree node. ICE HW and storage
> +HW are tightly coupled. Storage device probing is dependent upon ICE device
> +probing. ICE driver configures all the required registers to put the ICE HW
> +in bypass mode.
> +
> +b) Configuring keys
> +Currently, there are couple of use cases to configure the keys.
> +
> +1) Full Disk Encryption(FDE)
> +System layer(VOLD) at invocation of apps layer would call libqseecom to create
> +the encryption key. Libqseecom calls qseecom driver to communicate with KMS
> +module on the secure side i.e. TZ. KMS would call ICE driver on the TZ side to
> +create and set the keys in ICE HW. At the end of transaction, VOLD would have
> +key index of key LUT where encryption key is present.
> +
> +2) Per File Encryption (PFE)
> +Per File Manager(PFM) calls QSEECom api to create the key. PFM has a peer comp-

Preferably don't split (hyphenate) "component."  But if you must, it should be com-
ponent.

> +onent(PFT) at kernel layer which gets the corresponding key index from PFM.
> +
> +Following are all the components involved in the ICE driver for data path:
> +
> +       +++++++++++++++++++++++++++++++++++++++++
> +       +               App layer               +
> +       +++++++++++++++++++++++++++++++++++++++++
> +       +              VFS                      +
> +       +---------------------------------------+
> +       +         File System (EXT4)            +
> +       +---------------------------------------+
> +       +             Block Layer               +
> +       + --------------------------------------+
> +       +                              +++++++  +
> +       +              dm-req-crypt => + PFT +  +
> +       +                              +++++++  +
> +       +                                       +
> +       +---------------------------------------+
> +       +    +++++++++++           +++++++      +
> +       +    + Storage +           + ICE +      +
> +       +++++++++++++++++++++++++++++++++++++++++
> +       +                  ||                   +
> +       +                  || (Storage Req with +
> +       +                  \/  ICE parameters ) +
> +       +++++++++++++++++++++++++++++++++++++++++
> +       +          Storage Device               +
> +       +          ++++++++++++++               +
> +       +          +   ICE HW   +               +
> +       +++++++++++++++++++++++++++++++++++++++++
> +
> +c) Data transaction
> +Once the crypto key has been configured, VOLD/PFM creates device mapping for
> +data partition. As part of device mapping VOLD passes key index, crypto
> +algorithm, mode and key length to dm layer. In case of PFE, keys are provided
> +by PFT as and when request is processed by dm-req-crypt. When any application
> +needs to read/write data, it would go through DM layer which would add crypto
> +information, provided by VOLD/PFT, to Request. For each Request, Storage driver
> +would ask ICE driver to configure crypto part of request. ICE driver extracts
> +crypto data from Request structure and provide it to storage driver which would

                                          provides

> +finally dispatch request to storage device.
> +
> +d) Error Handling
> +Due to issue # 1 mentioned in "Known Issues", ICE driver does not register for
> +any interrupt. However, it enables sources of interrupt for ICE HW. After each
> +data transaction, Storage driver receives transaction completion event. As part
> +of event handling, storage driver calls  ICE driver to check if any of ICE
> +interrupt status is set. If yes, storage driver returns error to upper layer.
> +
> +Error handling would be changed in future chips.
> +
> +Interfaces
> +==========
> +ICE driver exposes interfaces for storage driver to :

                                                    to:

> +1. Get the global instance of ICE driver
> +2. Get the implemented interfaces of the particular ice instance
> +3. Initialize the ICE HW
> +4. Reset the ICE HW
> +5. Resume/Suspend the ICE HW
> +6. Get the Crypto configuration for the data request for storage
> +7. Check if current data transaction has generated any interrupt
> +
> +Driver Parameters
> +=================
> +This driver is built and statically linked into the kernel; therefore,
> +there are no module parameters supported by this driver.
> +
> +There are no kernel command line parameters supported by this driver.
> +
> +Power Management
> +================
> +ICE driver does not do power management on its own as it is part of storage
> +hardware. Whenever storage driver receives request for power collapse/suspend
> +resume, it would call ICE driver which exposes APIs for Storage HW. ICE HW
> +during power collapse or reset, wipes crypto configuration data. When ICE

                                 ^no comma

> +driver receives request to resume, it would ask ICE driver on TZ side to
> +restore the configuration. ICE driver does not do anything as part of power
> +collapse or suspend event.
> +
> +Interface:
> +==========
> +ICE driver exposes following APIs for storage driver to use:
> +
> +int (*init)(struct platform_device *, void *, ice_success_cb, ice_error_cb);
> +	-- This function is invoked by storage controller during initialization of
> +	storage controller. Storage controller would provide success and error call
> +	backs which would be invoked asynchronously once ICE HW init is done.
> +
> +int (*reset)(struct platform_device *);
> +	-- ICE HW reset as part of storage controller reset. When storage controller
> +	received reset command, it would call reset on ICE HW. As of now, ICE HW
> +	does not need to do anything as part of reset.
> +
> +int (*resume)(struct platform_device *);
> +	-- ICE HW while going to reset, wipes all crypto keys and other data from ICE

                                      ^no comma

> +	HW. ICE driver would reconfigure those data as part of resume operation.
> +
> +int (*suspend)(struct platform_device *);
> +	-- This API would be called by storage driver when storage device is going to
> +	suspend mode. As of today, ICE driver does not do anything to handle suspend.
> +
> +int (*config)(struct platform_device *, struct request* , struct ice_data_setting*);
> +	-- Storage driver would call this interface to get all crypto data required to
> +	perform crypto operation.
> +
> +int (*status)(struct platform_device *);
> +	-- Storage driver would call this interface to check if previous data transfer
> +	generated any error.
> +
> +Config options
> +==============
> +This driver is enabled by the kernel config option CONFIG_CRYPTO_DEV_MSM_ICE.
> +
> +Dependencies
> +============
> +ICE driver depends upon corresponding ICE driver on TZ side to function
> +appropriately.
> +
> +Known Issues
> +============
> +1. ICE HW emits 0s even if it has generated an interrupt

That statement is unclear.  Emits where?  in a data stream?  in interrupt status?

> +This issue has significant impact on how ICE interrupts are handled. Currently,
> +ICE driver does not register for any of the ICE interrupts but enables the
> +sources of interrupt. Once storage driver asks to check the status of interrupt,
> +it reads and clears the clear status and provide read status to storage driver.

               "clears the clear status"???         s/provide/provides/

> +This mechanism though not optimal but prevents file-system corruption.

mangled sentence above.

> +This issue has been fixed in newer chips.
> +
> +2. ICE HW wipes all crypto data during power collapse
> +This issue necessitate that ICE driver on TZ side store the crypto material

              necessitates

> +which is not required in the case of general purpose crypto engine.
> +This issue has been fixed in newer chips.
> +
> +Further Improvements
> +====================
> +Currently, Due to PFE use case, ICE driver is dependent upon dm-req-crypt to

              due

> +provide the keys as part of request structure. This couples ICE driver with
> +dm-req-crypt based solution. It is under discussion to expose an IOCTL based

                                                       to expose IOCTL based

> +and registration based interface APIs from ICE driver. ICE driver would use
> +these two interfaces to find out if any key exists for current request. If
> +yes, choose the right key index received from IOCTL or registration based
> +APIs. If not, dont set any crypto parameter in the request.

                 don't

> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index a8c4ce0..e40750e 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -596,6 +596,16 @@ config CRYPTO_DEV_QCOM_RNG
>  	  To compile this driver as a module, choose M here. The
>            module will be called qcom-rng. If unsure, say N.
>  
> +config CRYPTO_DEV_QTI_ICE
> +	tristate "Qualcomm inline crypto engine accelerator"
> +	default n

Please drop the redundant "default n" since n is already the default.

> +	depends on (ARCH_QCOM || COMPILE_TEST) && BLK_DEV_DM
> +	help
> +	  This driver supports Inline Crypto Engine for QTI chipsets, MSM8994
> +	  and later, to accelerate crypto operations for storage needs.
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ice.
> +
>  config CRYPTO_DEV_VMX
>  	bool "Support for VMX cryptographic acceleration instructions"
>  	depends on PPC64 && VSX

-- 
~Randy

  parent reply	other threads:[~2018-10-17 17:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-17 15:17 [PATCH 0/3] Add Inline Crypto Engine (ICE) driver AnilKumar Chimata
2018-10-17 15:17 ` [PATCH 1/3] firmware: qcom: scm: Update qcom_scm_call signature AnilKumar Chimata
2018-10-17 15:17 ` [PATCH 2/3] dt-bindings: Add ICE device specific parameters AnilKumar Chimata
2018-10-25 18:15   ` Rob Herring
2018-10-29 13:30     ` AnilKumar Chimata
2018-10-17 15:17 ` [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine AnilKumar Chimata
2018-10-17 17:04   ` Theodore Y. Ts'o
2018-10-24 12:04     ` AnilKumar Chimata
2018-10-17 17:39   ` Randy Dunlap [this message]
2018-10-24 14:43     ` AnilKumar Chimata
2018-10-18 11:43   ` kbuild test robot
2018-10-24 11:14     ` anilc
2018-10-25 14:58       ` Rob Herring
2018-10-29 13:31         ` AnilKumar Chimata
2018-10-25 14:55   ` Rob Herring
2018-10-25 15:28     ` Theodore Y. Ts'o
2018-10-25 15:45       ` Rob Herring
2018-10-29 13:47       ` AnilKumar Chimata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48c7da0d-4e57-bc36-a683-94088b3b8319@infradead.org \
    --to=rdunlap@infradead.org \
    --cc=andy.gross@linaro.org \
    --cc=anilc@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-soc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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