Add Inline Crypto Engine (ICE) driver
Subsystems (5)
cover Add Inline Crypto Engine (ICE) driver
This patch series adds support for QualComm ICE driver which is embedded in storage device such as UFS/eMMC.
ICE HW provides high throughputs, which meets the line speed of storage devices. ICE works in bypass or non-bypass mode, during bypass mode ICE does not perform any crypto operations but in non-bypass mode ICE will perform encryption/decryption based on the configuration.
ICE HW supports AES128/256 bit ECB & XTS mode algorithms, which is used on many SoCs like msm8916, msm8996, msm8953, sdm845 and many more MTPs from QualComm.
Also adds new functions to firmware driver which are needed to request secure OS to restore ICE key config when device reset.
These patches have been tested on sdm845 MTP using some additional patches (a) interconnect changes (b) ufs-ice interface driver
(c) dtsi changes for device init (d) kernel config changes. Once these patches are accepted rest will be posted.
AnilKumar Chimata (3):
firmware: qcom: scm: Update qcom_scm_call signature
dt-bindings: Add ICE device specific parameters
crypto: qce: ice: Add support for Inline Crypto Engine
Documentation/crypto/msm/ice.txt | 235 +++ .../devicetree/bindings/crypto/msm/ice.txt | 34 + drivers/crypto/Kconfig | 10 + drivers/crypto/qce/Makefile | 1 + drivers/crypto/qce/ice.c | 1613 ++++++++++++++++++++ drivers/crypto/qce/iceregs.h | 159 ++ drivers/firmware/qcom_scm-32.c | 30 +- drivers/firmware/qcom_scm-64.c | 77 +- drivers/firmware/qcom_scm.c | 8 +- drivers/firmware/qcom_scm.h | 5 +- include/crypto/ice.h | 80 + include/linux/qcom_scm.h | 5 + 12 files changed, 2213 insertions(+), 44 deletions(-) create mode 100644 Documentation/crypto/msm/ice.txt create mode 100644 Documentation/devicetree/bindings/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
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 1/3 firmware: qcom: scm: Update qcom_scm_call signature
Add new syscall for ICE key config restore during
device reset, which needs to update the existingqcom_scm_call() signature.
--- drivers/firmware/qcom_scm-32.c | 30 +++++++++------- drivers/firmware/qcom_scm-64.c | 77 ++++++++++++++++++++++++++---------------- drivers/firmware/qcom_scm.c | 8 ++++- drivers/firmware/qcom_scm.h | 5 ++- include/linux/qcom_scm.h | 5 +++ 5 files changed, 81 insertions(+), 44 deletions(-)
drivers/firmware/qcom_scm-32.c +18 -12
@@ -1,4 +1,4 @@ -/* Copyright (c) 2010,2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2010,2015,2018 The Linux Foundation. All rights reserved. * Copyright (C) 2015 Linaro Ltd. * * This program is free software; you can redistribute it and/or modify
@@ -172,7 +172,7 @@ static u32 smc(u32 cmd_addr)* and response buffers is taken care of by qcom_scm_call; however, callers are * responsible for any other cached buffers passed over to the secure world. */ -static int qcom_scm_call (struct device *dev,u32 svc_id,u32 cmd_id, +static int qcom_scm_call (struct device *dev,u32 own_id,u32 svc_id,u32 cmd_id,const void *cmd_buf,size_t cmd_len,void *resp_buf,size_t resp_len) {
@@ -406,7 +406,7 @@ int __qcom_scm_set_warm_boot_addr(struct device *dev, void *entry, cmd.addr =cpu_to_le32 (virt_to_phys (entry)); cmd.flags =cpu_to_le32 (flags); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_BOOT ,QCOM_SCM_BOOT_ADDR , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_BOOT ,QCOM_SCM_BOOT_ADDR , &cmd,sizeof (cmd),NULL ,0 );if (!ret) {for_each_cpu (cpu, cpus)
@@ -436,7 +436,7 @@ int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id)__le32 svc_cmd =cpu_to_le32 ((svc_id <<10 ) | cmd_id);__le32 ret_val =0 ; - ret =qcom_scm_call (dev,QCOM_SCM_SVC_INFO ,QCOM_IS_CALL_AVAIL_CMD , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_INFO ,QCOM_IS_CALL_AVAIL_CMD , &svc_cmd,sizeof (svc_cmd), &ret_val,sizeof (ret_val));if (ret)
@@ -451,7 +451,7 @@ int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,if (req_cnt >QCOM_SCM_HDCP_MAX_REQ_CNT )return -ERANGE ; -return qcom_scm_call (dev,QCOM_SCM_SVC_HDCP ,QCOM_SCM_CMD_HDCP , +return qcom_scm_call (dev,0 ,QCOM_SCM_SVC_HDCP ,QCOM_SCM_CMD_HDCP , req, req_cnt *sizeof (*req), resp,sizeof (*resp)); }
@@ -466,7 +466,7 @@ bool __qcom_scm_pas_supported(struct device *dev, u32 peripheral)int ret; in =cpu_to_le32 (peripheral); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_PIL , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_IS_SUPPORTED_CMD , &in,sizeof (in), &out,sizeof (out));
@@ -487,7 +487,7 @@ int __qcom_scm_pas_init_image(struct device *dev, u32 peripheral, request.proc =cpu_to_le32 (peripheral); request.image_addr =cpu_to_le32 (metadata_phys); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_PIL , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_INIT_IMAGE_CMD , &request,sizeof (request), &scm_ret,sizeof (scm_ret));
@@ -510,7 +510,7 @@ int __qcom_scm_pas_mem_setup(struct device *dev, u32 peripheral, request.addr =cpu_to_le32 (addr); request.len =cpu_to_le32 (size); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_PIL , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_MEM_SETUP_CMD , &request,sizeof (request), &scm_ret,sizeof (scm_ret));
@@ -525,7 +525,7 @@ int __qcom_scm_pas_auth_and_reset(struct device *dev, u32 peripheral)int ret; in =cpu_to_le32 (peripheral); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_PIL , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_AUTH_AND_RESET_CMD , &in,sizeof (in), &out,sizeof (out));
@@ -540,7 +540,7 @@ int __qcom_scm_pas_shutdown(struct device *dev, u32 peripheral)int ret; in =cpu_to_le32 (peripheral); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_PIL , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_SHUTDOWN_CMD , &in,sizeof (in), &out,sizeof (out));
@@ -554,7 +554,7 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)__le32 in =cpu_to_le32 (reset);int ret; - ret =qcom_scm_call (dev,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_MSS_RESET , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_PIL ,QCOM_SCM_PAS_MSS_RESET , &in,sizeof (in), &out,sizeof (out));
@@ -579,7 +579,8 @@ int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 id) req.state =cpu_to_le32 (state); req.id =cpu_to_le32 (id); - ret =qcom_scm_call (dev,QCOM_SCM_SVC_BOOT ,QCOM_SCM_SET_REMOTE_STATE , + ret =qcom_scm_call (dev,0 ,QCOM_SCM_SVC_BOOT , +QCOM_SCM_SET_REMOTE_STATE , &req,sizeof (req), &scm_ret,sizeof (scm_ret));return ret ? :le32_to_cpu (scm_ret);
@@ -592,6 +593,11 @@ int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region,return -ENODEV ; } +int __qcom_scm_restore_cfg (struct device *dev,u32 arginfo,u32 type) +{ +return -ENODEV ; +} +int __qcom_scm_restore_sec_cfg (struct device *dev,u32 device_id,u32 spare) {
drivers/firmware/qcom_scm-64.c +47 -30
192 lines not on this page · raw message
drivers/firmware/qcom_scm.c +7 -1
@@ -1,7 +1,7 @@/* * Qualcomm SCM driver * -* Copyright (c) 2010,2015, The Linux Foundation. All rights reserved. +* Copyright (c) 2010,2015,2018 The Linux Foundation. All rights reserved. * Copyright (C) 2015 Linaro Ltd. * * This program is free software; you can redistribute it and/or modify
@@ -335,6 +335,12 @@ static int qcom_scm_pas_reset_deassert(struct reset_controller_dev *rcdev, .deassert = qcom_scm_pas_reset_deassert, }; +int qcom_scm_ice_restore_cfg (u32 arginfo,u32 storage_type) +{ +return __qcom_scm_restore_cfg (__scm->dev, arginfo, storage_type); +} +EXPORT_SYMBOL (qcom_scm_ice_restore_cfg); +int qcom_scm_restore_sec_cfg (u32 device_id,u32 spare) {return __qcom_scm_restore_sec_cfg (__scm->dev, device_id, spare);
drivers/firmware/qcom_scm.h +4 -1
@@ -1,4 +1,4 @@ -/* Copyright (c) 2010-2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2010-2015,2018 The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and
@@ -93,6 +93,9 @@ static inline int qcom_scm_remap_error(int err)return -EINVAL ; } +#define QCOM_SCM_SVC_KEYSTORE 0x5 +#define QCOM_SCM_RESTORE_CFG 6 +extern int __qcom_scm_restore_cfg (struct device *dev,u32 arginfo,u32 type);#define QCOM_SCM_SVC_MP 0xc#define QCOM_SCM_RESTORE_SEC_CFG 2 extern int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id,
include/linux/qcom_scm.h +5 -0
@@ -60,6 +60,7 @@ extern int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,extern u32 qcom_scm_get_version (void );extern int qcom_scm_set_remote_state (u32 state,u32 id);extern int qcom_scm_restore_sec_cfg (u32 device_id,u32 spare); +extern int qcom_scm_ice_restore_cfg (u32 arginfo,u32 storage_type);extern int qcom_scm_iommu_secure_ptbl_size (u32 spare,size_t *size);extern int qcom_scm_iommu_secure_ptbl_init (u64 addr,u32 size,u32 spare);extern int qcom_scm_io_readl (phys_addr_t addr,unsigned int *val);
@@ -96,6 +97,10 @@ static inline void qcom_scm_cpu_power_down(u32 flags) {}static inline u32 qcom_scm_set_remote_state (u32 state,u32 id) {return -ENODEV ; }static inline int qcom_scm_restore_sec_cfg (u32 device_id,u32 spare) {return -ENODEV ; } +static inline int qcom_scm_ice_restore_cfg (u32 arginfo,u32 storage_type) +{ +return -ENODEV ; +}static inline int qcom_scm_iommu_secure_ptbl_size (u32 spare,size_t *size) {return -ENODEV ; }static inline int qcom_scm_iommu_secure_ptbl_init (u64 addr,u32 size,u32 spare) {return -ENODEV ; }static inline int qcom_scm_io_readl (phys_addr_t addr,unsigned int *val) {return -ENODEV ; }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 2/3 dt-bindings: Add ICE device specific parameters
In reply to AnilKumar ChimataAdd dt parameters information specific to the Inline Crypto Engine (ICE) device.
--- .../devicetree/bindings/crypto/msm/ice.txt | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/msm/ice.txt
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +34 -0
@@ -0,0 +1,34 @@ +* Inline Crypto Engine (ICE) + +Required properties: + - compatible : should be "qcom,ice" + - reg : <register mapping> + +Optional properties: + - interrupt-names : name describing the interrupts for ICE IRQ + - interrupts : <interrupt mapping for ICE IRQ> + - qcom,enable-ice-clk : should enable clocks for ICE HW + - clocks : List of phandle and clock specifier pairs + - clock-names : List of clock input name strings sorted in the same + order as the clocks property. + - qcom,op-freq-hz : max clock speed sorted in the same order as the clocks + property. + - qcom,instance-type : describe the storage type for which ICE node is defined + currently, only "ufs" and "sdcc" are supported storage type + - power-domains : regulator supply to be used by ICE HW + +Example: + ufs_ice: ufsice@1d90000 { + compatible = "qcom,ice"; + reg = <0x1d90000 0x8000>; + qcom,enable-ice-clk; + clock-names = "ufs_core_clk", "bus_clk", + "iface_clk", "ice_core_clk"; + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, + <&gcc GCC_UFS_MEM_CLKREF_CLK>, + <&gcc GCC_UFS_PHY_AHB_CLK>, + <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; + qcom,op-freq-hz = <0>, <0>, <0>, <300000000>; + power-domains = <&gcc UFS_PHY_GDSC>; + qcom,instance-type = "ufs"; + };
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Rob Herring
maintainerRe: [PATCH 2/3] dt-bindings: Add ICE device specific parameters
Quoted 15 lines · AnilKumar Chimata
Add dt parameters information specific to the Inline Crypto Engine (ICE) device.
Signed-off-by AnilKumar Chimata <[email protected]>--- .../devicetree/bindings/crypto/msm/ice.txt | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/msm/ice.txtDocumentation/devicetree/bindings/crypto/msm/ice.txt new file +0
-0
Documentation/devicetree/bindings/crypto/msm/ice.txt line 4
Quoted 4 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +4
-0@@ -0,0 +1,34 @@ +* Inline Crypto Engine (ICE) + +Required properties: + - compatible : should be "qcom,ice"
Only 1 version ever? Probably not and this needs an SoC specific compatible string. Does this follow any standard or ICE is a QCom thing?
Documentation/devicetree/bindings/crypto/msm/ice.txt line 5
Quoted 1 line
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +1
-0+ - reg : <register mapping>
No need to define standard properties. You need to say how many register ranges.
Documentation/devicetree/bindings/crypto/msm/ice.txt line 8
Quoted 3 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +3
-0+ +Optional properties: + - interrupt-names : name describing the interrupts for ICE IRQ
No point to this if there is only 1 IRQ.
Documentation/devicetree/bindings/crypto/msm/ice.txt line 10
Quoted 2 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +2
-0+ - interrupts : <interrupt mapping for ICE IRQ> + - qcom,enable-ice-clk : should enable clocks for ICE HW
This shouldn't be needed.
Documentation/devicetree/bindings/crypto/msm/ice.txt line 13
Quoted 3 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +3
-0+ - clocks : List of phandle and clock specifier pairs + - clock-names : List of clock input name strings sorted in the same + order as the clocks property.
How many? You need to give the
Documentation/devicetree/bindings/crypto/msm/ice.txt line 15
Quoted 2 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +2
-0+ - qcom,op-freq-hz : max clock speed sorted in the same order as the clocks + property.
Use the assigned-clocks properties for this.
Documentation/devicetree/bindings/crypto/msm/ice.txt line 17
Quoted 2 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +2
-0+ - qcom,instance-type : describe the storage type for which ICE node is defined + currently, only "ufs" and "sdcc" are supported storage type
What if there is more than one instance of ufs or SD? Do you need to know which ICE goes with which controller?
Documentation/devicetree/bindings/crypto/msm/ice.txt line 21
Quoted 4 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +4
-0+ - power-domains : regulator supply to be used by ICE HW + +Example: + ufs_ice: ufsice@1d90000 {
crytpo@...
Quoted 17 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +13
-0+ compatible = "qcom,ice"; + reg = <0x1d90000 0x8000>; + qcom,enable-ice-clk; + clock-names = "ufs_core_clk", "bus_clk", + "iface_clk", "ice_core_clk"; + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, + <&gcc GCC_UFS_MEM_CLKREF_CLK>, + <&gcc GCC_UFS_PHY_AHB_CLK>, + <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; + qcom,op-freq-hz = <0>, <0>, <0>, <300000000>; + power-domains = <&gcc UFS_PHY_GDSC>; + qcom,instance-type = "ufs"; + };Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/3] dt-bindings: Add ICE device specific parameters
Hi Rob,
Thanks for the comments,
Quoted 12 lines · Rob Herring
Quoted 11 lines · AnilKumar Chimata
Add dt parameters information specific to the Inline Crypto Engine (ICE) device.
Signed-off-by AnilKumar Chimata <[email protected]>--- .../devicetree/bindings/crypto/msm/ice.txt | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644
Documentation/devicetree/bindings/crypto/msm/ice.txt
Quoted 13 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +4
-0@@ -0,0 +1,34 @@ +* Inline Crypto Engine (ICE) + +Required properties: + - compatible : should be "qcom,ice"Only 1 version ever? Probably not and this needs an SoC specific compatible string. Does this follow any standard or ICE is a QCom thing?
Yes, currently one version and in future we might have to add if there are any hardware specific changes and this is Qualcomm specific hardware.
Quoted 5 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 5
Quoted 2 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +1
-0+ - reg : <register mapping>No need to define standard properties. You need to say how many register ranges.
Changed.
Quoted 6 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 8
Quoted 4 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +3
-0+ +Optional properties: + - interrupt-names : name describing the interrupts for ICE IRQNo point to this if there is only 1 IRQ.
There are two IRQ lines which hardware can support one for non-secure operating system and another for secure operating system.
Quoted 5 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 10
Quoted 3 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +2
-0+ - interrupts : <interrupt mapping for ICE IRQ> + - qcom,enable-ice-clk : should enable clocks for ICE HWThis shouldn't be needed.
Changed.
Quoted 7 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 13
Quoted 5 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +2
-0+ - clocks : List of phandle and clock specifier pairs + - clock-names : List of clock input name strings sorted inthe same
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +1
-0+ order as the clocks property.How many? You need to give the
Currently four clocks are needed, details updated accordingly.
Quoted 5 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 15
Quoted 3 lines
+ - qcom,op-freq-hz : max clock speed sorted in the same order
as the clocksDocumentation/devicetree/bindings/crypto/msm/ice.txt new file +1
-0+ property.Use the assigned-clocks properties for this.
Actually that is not a max clock speed, its any array of operating frequencies which ICE can support.
Quoted 7 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 17
Quoted 4 lines
+ - qcom,instance-type : describe the storage type for which ICE
node is definedDocumentation/devicetree/bindings/crypto/msm/ice.txt new file +1
-0+ currently, only "ufs" and "sdcc" are supported storage typeWhat if there is more than one instance of ufs or SD? Do you need to know which ICE goes with which controller?
That is right, needs to have multiple device node entries which differentiate between storage instances.
Quoted 25 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt line 21
Quoted 5 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +4
-0+ - power-domains : regulator supply to be used by ICE HW + +Example: + ufs_ice: ufsice@1d90000 {crytpo@...
Quoted 17 lines
Documentation/devicetree/bindings/crypto/msm/ice.txt new file +13
-1+ compatible = "qcom,ice"; + reg = <0x1d90000 0x8000>; + qcom,enable-ice-clk; + clock-names = "ufs_core_clk", "bus_clk", + "iface_clk", "ice_core_clk"; + clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, + <&gcc GCC_UFS_MEM_CLKREF_CLK>, + <&gcc GCC_UFS_PHY_AHB_CLK>, + <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; + qcom,op-freq-hz = <0>, <0>, <0>, <300000000>; + power-domains = <&gcc UFS_PHY_GDSC>; + qcom,instance-type = "ufs"; + }; --The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 3/3 crypto: qce: ice: Add support for Inline Crypto Engine
In reply to AnilKumar ChimataThis 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.
--- 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
Documentation/crypto/msm/ice.txt new file +235 -0
@@ -0,0 +1,235 @@ +Introduction: +============= +Storage encryption has been one of the most required feature from security +point of view. QTI based storage encryption solution uses general purpose +crypto engine. While this kind of solution provide a decent amount of +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 +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). + +When a switch between the operation modes(plain to crypto or crypto to plain) +is desired for a particular partition, SW must complete all transactions for +that particular partition before switching the crypto mode i.e. no crypto, one +direction crypto or both direction crypto operation. Requests for other +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. + +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 +as HLOS as well. This requires that ICE driver to be split in 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- +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 +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 : +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 +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 + 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 +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. +This mechanism though not optimal but prevents file-system corruption. +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 +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 +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 +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.
drivers/crypto/Kconfig +10 -0
@@ -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 + 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
drivers/crypto/qce/Makefile +1 -0
@@ -5,3 +5,4 @@ qcrypto-objs := core.o \ dma.o \ sha.o \ ablkcipher.o +obj-$(CONFIG_CRYPTO_DEV_QTI_ICE) += ice.o
drivers/crypto/qce/ice.c new file +1613 -0
1,613 lines not on this page · raw message
drivers/crypto/qce/iceregs.h new file +159 -0
159 lines not on this page · raw message
include/crypto/ice.h new file +80 -0
80 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
First, thanks for the effort for working on getting the core ICE driver support into upstreamable patches.
Documentation/crypto/msm/ice.txt line 235
Quoted 13 lines · AnilKumar Chimata
+2) Per File Encryption (PFE)
Documentation/crypto/msm/ice.txt new file +11
-0+Per File Manager(PFM) calls QSEECom api to create the key. PFM has a peer comp- +onent(PFT) at kernel layer which gets the corresponding key index from PFM. +Further Improvements +==================== +Currently, Due to PFE use case, ICE driver is dependent upon dm-req-crypt to +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 +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.
However, this documentation is referencing components which are not in the mainline kernel.
In the long term, what I'd like to see for per-file key support is a clean solution which interfaces with the in-kernel fscrypt-enabled file systems (e.g., f2fs and ext4). What I think we need to do is to add a field in the bio structure which references a key id, and then define a bdi-specific interface which allows the file system to register a struct key and get a key id. Use of the key id will be refcounted, so the device driver knows how many I/O operations are in flight using a particular key --- since each ICE hardware will have a different number of active keys that it can support.
Until that's there, at least for the upstream documentation, I think it would be better to drop mention of code that is not yet upstream --- and which may have problems ever going upstream in their current form.
(I haven't checked in a while, but last time I looked the code in question blindly dereferenced private pointers assuming that the only two file systems that could ever use ICE were ext4 and f2fs.... not that the code used by Google handsets were _that_ much cleaner, but at least we dropped in a crypto key into the struct bio, instead of playing unnatural games with private pointers from the wrong abstraction layer. :-)
- Ted
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Hi Theodore,
Thanks for the comments,
Quoted 20 lines · Theodore Y. Ts'o
First, thanks for the effort for working on getting the core ICE driver support into upstreamable patches.
Quoted 16 lines · AnilKumar Chimata
+2) Per File Encryption (PFE)
+Per File Manager(PFM) calls QSEECom api to create the key. PFM has a peer comp-
+onent(PFT) at kernel layer which gets the corresponding key index from PFM.
...Documentation/crypto/msm/ice.txt new file +3
-0+Further Improvements +==================== +Currently, Due to PFE use case, ICE driver is dependent upondm-req-crypt to
+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
+and registration based interface APIs from ICE driver. ICE driver would use
Quoted 8 lines
Quoted 5 lines
+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.However, this documentation is referencing components which are not in the mainline kernel.
Sure, will clean the documentation and try to minimize the unknown components. Provided these details for better understanding of the flow.
Quoted 4 lines
In the long term, what I'd like to see for per-file key support is a clean solution which interfaces with the in-kernel fscrypt-enabled file systems (e.g., f2fs and ext4). What I think we need to do is to
Agree, we are working on making per-file key (PFK) driver generic to support any file system.
Quoted 6 lines
add a field in the bio structure which references a key id, and then define a bdi-specific interface which allows the file system to register a struct key and get a key id. Use of the key id will be refcounted, so the device driver knows how many I/O operations are in flight using a particular key --- since each ICE hardware will have a different number of active keys that it can support.
Understand the point here, we will consider this refcount while working on upstreamable PFK driver.
Quoted 5 lines
Until that's there, at least for the upstream documentation, I think it would be better to drop mention of code that is not yet upstream --- and which may have problems ever going upstream in their current form.
Will clean the documentation.
Quoted 8 lines
(I haven't checked in a while, but last time I looked the code in question blindly dereferenced private pointers assuming that the only two file systems that could ever use ICE were ext4 and f2fs.... not that the code used by Google handsets were _that_ much cleaner, but at least we dropped in a crypto key into the struct bio, instead of playing unnatural games with private pointers from the wrong abstraction layer. :-)
before upstreaming the PFK drivers, we will try to avoid private pointer dereferences to achieve better abstraction to file system.
Quoted 2 lines
- Ted
Randy Dunlap
reviewerRe: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
In reply to AnilKumar ChimataQuoted 25 lines · AnilKumar Chimata
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 <[email protected]>--- 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.hDocumentation/crypto/msm/ice.txt new file +0
-0
Documentation/crypto/msm/ice.txt line 3
Quoted 3 lines
Documentation/crypto/msm/ice.txt new file +3
-0@@ -0,0 +1,235 @@ +Introduction: +============= +Storage encryption has been one of the most required feature from security
features
Documentation/crypto/msm/ice.txt line 5
Quoted 2 lines
Documentation/crypto/msm/ice.txt new file +2
-0+point of view. QTI based storage encryption solution uses general purpose +crypto engine. While this kind of solution provide a decent amount of
provides
Documentation/crypto/msm/ice.txt line 13
Quoted 8 lines
Documentation/crypto/msm/ice.txt new file +8
-0+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
Documentation/crypto/msm/ice.txt line 17
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+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 ...
Documentation/crypto/msm/ice.txt line 19
Quoted 2 lines
Documentation/crypto/msm/ice.txt new file +2
-0+ +When a switch between the operation modes(plain to crypto or crypto to plain)
modes (plain ...
Documentation/crypto/msm/ice.txt line 20
Quoted 1 line
Documentation/crypto/msm/ice.txt new file +1
-0+is desired for a particular partition, SW must complete all transactions for
s/SW/software/
Documentation/crypto/msm/ice.txt line 21
Quoted 1 line
Documentation/crypto/msm/ice.txt new file +1
-0+that particular partition before switching the crypto mode i.e. no crypto, one
crypto mode;
Documentation/crypto/msm/ice.txt line 22
Quoted 1 line
Documentation/crypto/msm/ice.txt new file +1
-0+direction crypto or both direction crypto operation. Requests for other
directions
Documentation/crypto/msm/ice.txt line 29
Quoted 7 lines
Documentation/crypto/msm/ice.txt new file +7
-0+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
Documentation/crypto/msm/ice.txt line 34
Quoted 5 lines
Documentation/crypto/msm/ice.txt new file +5
-0+ +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...
Documentation/crypto/msm/ice.txt line 35
Quoted 1 line
Documentation/crypto/msm/ice.txt new file +1
-0+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
Quoted 49 lines
Documentation/crypto/msm/ice.txt new file +49
-0+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.
Documentation/crypto/msm/ice.txt line 94
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+ +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.
Quoted 29 lines
Documentation/crypto/msm/ice.txt new file +29
-0+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 + + + +++++++++++++++++++++++++++++++++++++++++
Documentation/crypto/msm/ice.txt line 133
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+ +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
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+finally dispatch request to storage device. + +d) Error Handling +Due to issue # 1 mentioned in "Known Issues", ICE driver does not register for
Documentation/crypto/msm/ice.txt line 147
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+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:
Quoted 11 lines
Documentation/crypto/msm/ice.txt new file +11
-0+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,
Documentation/crypto/msm/ice.txt line 168
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+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
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+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
Documentation/crypto/msm/ice.txt line 188
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+ 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
Quoted 16 lines
Documentation/crypto/msm/ice.txt new file +16
-0+ 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 +==============
Documentation/crypto/msm/ice.txt line 214
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +10
-0+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?
Documentation/crypto/msm/ice.txt line 218
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+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/
Documentation/crypto/msm/ice.txt line 219
Quoted 1 line
Documentation/crypto/msm/ice.txt new file +1
-0+This mechanism though not optimal but prevents file-system corruption.
mangled sentence above.
Documentation/crypto/msm/ice.txt line 223
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+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
Documentation/crypto/msm/ice.txt line 229
Quoted 6 lines
Documentation/crypto/msm/ice.txt new file +6
-0+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
Documentation/crypto/msm/ice.txt line 231
Quoted 2 lines
Documentation/crypto/msm/ice.txt new file +2
-0+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
Documentation/crypto/msm/ice.txt line 235
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+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
drivers/crypto/Kconfig config CRYPTO_DEV_QCOM_RNG · line 601
Quoted 11 lines
drivers/crypto/Kconfig +3
-0@@ -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.
Quoted 10 lines
drivers/crypto/Kconfig +7
-0+ 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
Signature · 1 line
~Randy
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Hi Randy,
Thanks for your comments,
Quoted 21 lines · Randy Dunlap
Quoted 20 lines · AnilKumar Chimata
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 <[email protected]>--- 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
Quoted 12 lines
Quoted 10 lines
Documentation/crypto/msm/ice.txt new file +3
-0@@ -0,0 +1,235 @@ +Introduction: +============= +Storage encryption has been one of the most required feature fromsecurity
features
changed.
Quoted 6 lines
Quoted 4 lines
+point of view. QTI based storage encryption solution uses general purpose
+crypto engine. While this kind of solution provide a decent amount ofprovides
changed.
Quoted 15 lines
Quoted 12 lines
+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 supposedDocumentation/crypto/msm/ice.txt new file +5
-0+to meet the line speed of storage devices. + +Hardware Description +==================== +ICE is a HW block that is embedded into storage device such asQuoted 1 line
UFS/eMMC. By
s/HW/hardware/ devices
changed.
Quoted 10 lines
Quoted 8 lines
+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 bothdirection(both encryption & decryption).directions (both ...
changed.
Quoted 6 lines
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +2
-0+ +When a switch between the operation modes(plain to crypto or cryptoto plain)
modes (plain ...
changed.
Quoted 5 lines
Quoted 3 lines
+is desired for a particular partition, SW must complete all transactions for
s/SW/software/
changed.
Quoted 5 lines
Quoted 3 lines
+that particular partition before switching the crypto mode i.e. no crypto, one
crypto mode;
changed.
Quoted 5 lines
Quoted 3 lines
+direction crypto or both direction crypto operation. Requests for other
directions
changed.
Quoted 13 lines
Quoted 11 lines
Documentation/crypto/msm/ice.txt new file +3
-0+partitions are not impacted due to crypto mode switch. + +ICE HW currently supports AES128/256 bit ECB & XTS mode encryptionalgorithms.
Documentation/crypto/msm/ice.txt new file +2
-0+ +Keys for crypto operations are loaded from SW. Keys are stored in alookup
+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
changed.
Quoted 10 lines
Quoted 8 lines
Documentation/crypto/msm/ice.txt new file +4
-0+ +SW Description +============== +ICE HW has categorized ICE registers in 2 groups: those which can beaccessed by
+only secure side i.e. TZ and those which can be accessed by non-secure side suchat least tell the reader what TZ and HLOS mean...
changed.
Quoted 6 lines
Quoted 3 lines
+as HLOS as well. This requires that ICE driver to be split in two
pieces: onethat the ICE driver be split into
two pieces: one
changed.
Quoted 66 lines
Quoted 66 lines
Documentation/crypto/msm/ice.txt new file +9
-0+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 forcontrol path:
Documentation/crypto/msm/ice.txt new file +33
-0+ ++++++++++++++++++++++++++++++++++++++++++ ++ 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 probefunction 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 HWDocumentation/crypto/msm/ice.txt new file +7
-0+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 libqseecomto 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.
Quoted 8 lines
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +3
-0+ +2) Per File Encryption (PFE) +Per File Manager(PFM) calls QSEECom api to create the key. PFM has apeer comp-
Preferably don't split (hyphenate) "component." But if you must, it should be com-
ponent.
Taken into account, will follow on future patches. For this patch changed.
Quoted 44 lines
Quoted 44 lines
+onent(PFT) at kernel layer which gets the corresponding key index from PFM.
Documentation/crypto/msm/ice.txt new file +2
-0+ +Following are all the components involved in the ICE driver for datapath:
Documentation/crypto/msm/ice.txt new file +29
-0+ + +++++++++++++++++++++++++++++++++++++++++ + + 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 devicemapping 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
Quoted 8 lines
Quoted 6 lines
+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 wouldprovides
changed.
Quoted 14 lines
Quoted 14 lines
Documentation/crypto/msm/ice.txt new file +4
-0+finally dispatch request to storage device. + +d) Error Handling +Due to issue # 1 mentioned in "Known Issues", ICE driver does notregister 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.
Quoted 8 lines
Documentation/crypto/msm/ice.txt line 147
Quoted 6 lines
Documentation/crypto/msm/ice.txt new file +6
-0+ +Error handling would be changed in future chips. + +Interfaces +========== +ICE driver exposes interfaces for storage driver to :to:
changed.
Quoted 21 lines
Quoted 21 lines
+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 interruptDocumentation/crypto/msm/ice.txt new file +4
-0+ +Driver Parameters +================= +This driver is built and statically linked into the kernel;therefore,
Documentation/crypto/msm/ice.txt new file +7
-0+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 ofstorage
Quoted 8 lines
Quoted 6 lines
+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
changed.
Quoted 24 lines
Quoted 24 lines
+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 powerDocumentation/crypto/msm/ice.txt new file +7
-0+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);Documentation/crypto/msm/ice.txt new file +1
-0+ -- This function is invoked by storage controller duringinitialization of
Documentation/crypto/msm/ice.txt new file +1
-0+ storage controller. Storage controller would provide success anderror call
Documentation/crypto/msm/ice.txt new file +1
-0+ backs which would be invoked asynchronously once ICE HW init isdone.
Documentation/crypto/msm/ice.txt new file +3
-0+ +int (*reset)(struct platform_device *); + -- ICE HW reset as part of storage controller reset. When storagecontroller
Documentation/crypto/msm/ice.txt new file +1
-0+ received reset command, it would call reset on ICE HW. As of now,
Quoted 8 lines
Quoted 6 lines
ICE HW
Documentation/crypto/msm/ice.txt new file +4
-0+ 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 datafrom ICE
^no comma
changed.
Quoted 30 lines
Quoted 30 lines
Documentation/crypto/msm/ice.txt new file +1
-0+ HW. ICE driver would reconfigure those data as part of resumeoperation.
Documentation/crypto/msm/ice.txt new file +3
-0+ +int (*suspend)(struct platform_device *); + -- This API would be called by storage driver when storage device isgoing to
Documentation/crypto/msm/ice.txt new file +1
-0+ suspend mode. As of today, ICE driver does not do anything to handlesuspend.
Documentation/crypto/msm/ice.txt new file +2
-0+ +int (*config)(struct platform_device *, struct request* , structice_data_setting*);
Documentation/crypto/msm/ice.txt new file +1
-0+ -- Storage driver would call this interface to get all crypto datarequired to
Documentation/crypto/msm/ice.txt new file +4
-0+ perform crypto operation. + +int (*status)(struct platform_device *); + -- Storage driver would call this interface to check if previousdata transfer
Documentation/crypto/msm/ice.txt new file +5
-0+ generated any error. + +Config options +============== +This driver is enabled by the kernel config option
CONFIG_CRYPTO_DEV_MSM_ICE.Documentation/crypto/msm/ice.txt new file +4
-0+ +Dependencies +============ +ICE driver depends upon corresponding ICE driver on TZ side tofunction
Quoted 8 lines
Documentation/crypto/msm/ice.txt line 214
Quoted 5 lines
Documentation/crypto/msm/ice.txt new file +5
-0+appropriately. + +Known Issues +============ +1. ICE HW emits 0s even if it has generated an interruptThat statement is unclear. Emits where? in a data stream? in interrupt status?
Its interrupt status line unchanged, currently its handled on storage driver
Quoted 11 lines
Quoted 9 lines
+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/
changed.
Quoted 5 lines
Quoted 3 lines
+This mechanism though not optimal but prevents file-system corruption.
mangled sentence above.
changed.
Quoted 8 lines
Quoted 6 lines
Documentation/crypto/msm/ice.txt new file +4
-0+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 cryptomaterial
necessitates
changed.
Quoted 10 lines
Quoted 8 lines
Documentation/crypto/msm/ice.txt new file +6
-0+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 upondm-req-crypt to
due
changed.
Quoted 8 lines
Quoted 5 lines
+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 basedto expose IOCTL
based
changed.
Quoted 10 lines
Quoted 8 lines
+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
changed.
Quoted 9 lines
Quoted 9 lines
drivers/crypto/Kconfig +0
-0@@ -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.
Quoted 5 lines
drivers/crypto/Kconfig config CRYPTO_DEV_QCOM_RNG · line 601
Quoted 3 lines
drivers/crypto/Kconfig +3
-0+config CRYPTO_DEV_QTI_ICE + tristate "Qualcomm inline crypto engine accelerator" + default nPlease drop the redundant "default n" since n is already the default.
Agree, will drop this change.
Quoted 12 lines
Quoted 12 lines
drivers/crypto/Kconfig +3
-0+ depends on (ARCH_QCOM || COMPILE_TEST) && BLK_DEV_DM + help + This driver supports Inline Crypto Engine for QTI chipsets,MSM8994
drivers/crypto/Kconfig +4
-0+ 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
kbuild test robot
build botRe: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
In reply to AnilKumar ChimataThank you for the patch! Yet something to improve:
Hi AnilKumar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.19-rc8 next-20181018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/AnilKumar-Chimata/firmware-qcom-scm-Update-qcom_scm_call-signature/20181018-182318 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All errors (new ones prefixed by >>):
>> drivers/crypto/qce/ice.c:1372:5: error: redefinition of 'qcom_ice_setup_ice_hw'
int qcom_ice_setup_ice_hw(const char *storage_type, int enable)
^~~~~~~~~~~~~~~~~~~~~
In file included from drivers/crypto/qce/ice.c:25:0:
include/crypto/ice.h:60:19: note: previous definition of 'qcom_ice_setup_ice_hw' was here
static inline int qcom_ice_setup_ice_hw(const char *storage_type, int enable)
^~~~~~~~~~~~~~~~~~~~~
vim +/qcom_ice_setup_ice_hw +1372 drivers/crypto/qce/ice.c
1371
> 1372 int qcom_ice_setup_ice_hw(const char *storage_type, int enable)
1373 {
1374 struct ice_device *ice_dev = NULL;
1375 int ret = -1;
1376
1377 ice_dev = get_ice_device_from_storage_type(storage_type);
1378 if (ice_dev == ERR_PTR(-EPROBE_DEFER))
1379 return -EPROBE_DEFER;
1380
1381 if (!ice_dev)
1382 return ret;
1383
1384 if (enable)
1385 return enable_ice_setup(ice_dev);
1386
1387 return disable_ice_setup(ice_dev);
1388 }
1389
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Hi,
Thanks for the comments, response inline.
Thanks,
AnilKumar
Quoted 49 lines · kbuild test robot
Hi AnilKumar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.19-rc8 next-20181018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]url:
https://github.com/0day-ci/linux/commits/AnilKumar-Chimata/firmware-qcom-scm-Update-qcom_scm_call-signature/20181018-182318
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=shAll errors (new ones prefixed by >>):
Quoted 1 line
Quoted 1 line
drivers/crypto/qce/ice.c:1372:5: error: redefinition of 'qcom_ice_setup_ice_hw'int
qcom_ice_setup_ice_hw(const char *storage_type, int enable)
^~~~~~~~~~~~~~~~~~~~~
In file included fromdrivers/crypto/qce/ice.c:25:0:
include/crypto/ice.h:60:19: note: previous definition of
'qcom_ice_setup_ice_hw' was here
static inline int qcom_ice_setup_ice_hw(const char*storage_type,
int enable)^~~~~~~~~~~~~~~~~~~~~vim +/qcom_ice_setup_ice_hw +1372
drivers/crypto/qce/ice.c1371
Quoted 1 line
1372 int
qcom_ice_setup_ice_hw(const char *storage_type, int enable)1373 {
1374 structice_device*ice_dev= NULL;
1375 int ret = -1;
1376
1377ice_dev=get_ice_device_from_storage_type(storage_type);
1378 if (ice_dev==ERR_PTR(-EPROBE_DEFER))
1379 return -EPROBE_DEFER;
1380
1381 if (!ice_dev)
Quoted 8 lines
1382 return ret;
1383
1384 if (enable)
1385 returnenable_ice_setup(ice_dev);
1386
1387 returndisable_ice_setup(ice_dev);
1388 }
1389
We will check and get back on the compilation. What is the idea behind for this effort, is this for testing the ICE driver? If so, this is not possible as ICE module is inline with storage controller and hence testing has to trigger from Storage controller/driver. So testing crypto functionality (AES-XTS and AES-CBC) with cryptodev is not possible with this driver.
Quoted 4 lines
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Rob Herring
maintainerRe: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Quoted 3 lines · On Wed, Oct 24, 2018 at 04:44:37PM +0530, [email protected] wrote:
Hi,
Thanks for the comments, response inline.
FYI, this was from a bot.
Quoted 65 lines
Thanks,
AnilKumarQuoted 50 lines · kbuild test robot
Hi AnilKumar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.19-rc8 next-20181018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]url:
https://github.com/0day-ci/linux/commits/AnilKumar-Chimata/firmware-qcom-scm-Update-qcom_scm_call-signature/20181018-182318
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=shAll errors (new ones prefixed by >>):
Quoted 2 lines
Quoted 2 lines
drivers/crypto/qce/ice.c:1372:5: error: redefinition of 'qcom_ice_setup_ice_hw'int
qcom_ice_setup_ice_hw(const char *storage_type, int enable)
^~~~~~~~~~~~~~~~~~~~~
In file included fromdrivers/crypto/qce/ice.c:25:0:
include/crypto/ice.h:60:19: note: previous definition of
'qcom_ice_setup_ice_hw' was here
static inline int qcom_ice_setup_ice_hw(const char*storage_type,
int enable)^~~~~~~~~~~~~~~~~~~~~vim +/qcom_ice_setup_ice_hw +1372
drivers/crypto/qce/ice.c1371
Quoted 1 line
1372 int
qcom_ice_setup_ice_hw(const char *storage_type, int enable)1373 {
1374 structice_device*ice_dev= NULL;
1375 int ret = -1;
1376
1377ice_dev=get_ice_device_from_storage_type(storage_type);
1378 if (ice_dev==ERR_PTR(-EPROBE_DEFER))
1379 return -EPROBE_DEFER;
1380
1381 if (!ice_dev)Quoted 8 lines
1382 return ret;
1383
1384 if (enable)
1385 returnenable_ice_setup(ice_dev);
1386
1387 returndisable_ice_setup(ice_dev);
1388 }
1389
Quoted 3 lines
We will check and get back on the compilation. What is the idea behind for this
effort, is this for testing the ICE driver?
The purpose is to make sure changes don't break builds that you likely haven't tested. Either different architectures or configurations.
Rob
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Hi Rob,
Thanks for the review,
Quoted 6 lines · Rob Herring
Quoted 3 lines · On Wed, Oct 24, 2018 at 04:44:37PM +0530, [email protected] wrote:
Hi,
Thanks for the comments, response inline.
FYI, this was from a bot.
I have realized after I sent the mail.
Quoted 66 lines
Quoted 66 lines
Thanks,
AnilKumarQuoted 58 lines · kbuild test robot
Hi AnilKumar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.19-rc8 next-20181018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]url:
https://github.com/0day-ci/linux/commits/AnilKumar-Chimata/firmware-qcom-scm-Update-qcom_scm_call-signature/20181018-182318
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=shAll errors (new ones prefixed by >>):
Quoted 2 lines
Quoted 2 lines
drivers/crypto/qce/ice.c:1372:5: error: redefinition of 'qcom_ice_setup_ice_hw'int
qcom_ice_setup_ice_hw(const char *storage_type, int enable)
^~~~~~~~~~~~~~~~~~~~~
In file included fromdrivers/crypto/qce/ice.c:25:0:
include/crypto/ice.h:60:19: note: previous definition of
'qcom_ice_setup_ice_hw' was here
static inline int qcom_ice_setup_ice_hw(const char*storage_type,
int enable)^~~~~~~~~~~~~~~~~~~~~vim +/qcom_ice_setup_ice_hw +1372
drivers/crypto/qce/ice.c1371
Quoted 1 line
1372 int
qcom_ice_setup_ice_hw(const char *storage_type, int enable)1373 {
1374 structice_device*ice_dev= NULL;
1375 int ret = -1;
1376
1377ice_dev=get_ice_device_from_storage_type(storage_type);
1378 if (ice_dev==ERR_PTR(-EPROBE_DEFER))
1379 return -EPROBE_DEFER;
1380
1381 if (!ice_dev)
1382 return ret;
1383
1384 if (enable)
1385 returnenable_ice_setup(ice_dev);
1386
1387 returndisable_ice_setup(ice_dev);
1388 }
1389
Quoted 6 lines
Quoted 3 lines
We will check and get back on the compilation. What is the idea behind for this
effort, is this for testing the ICE driver?The purpose is to make sure changes don't break builds that you likely haven't tested. Either different architectures or configurations.
Got the point, will try to avoid this in future.
Quoted 2 lines
Rob
Rob Herring
maintainerRe: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
In reply to AnilKumar ChimataQuoted 25 lines · AnilKumar Chimata
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 <[email protected]>--- 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.hDocumentation/crypto/msm/ice.txt new file +0
-0
Documentation/crypto/msm/ice.txt line 9
Quoted 9 lines
Documentation/crypto/msm/ice.txt new file +9
-0@@ -0,0 +1,235 @@ +Introduction: +============= +Storage encryption has been one of the most required feature from security +point of view. QTI based storage encryption solution uses general purpose +crypto engine. While this kind of solution provide a decent amount of +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.
Is ICE part of the storage device or part of the host as the binding suggests?
Rob
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Quoted 4 lines · Rob Herring
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+Introduction: +============= +Storage encryption has been one of the most required feature from security +point of view. QTI based storage encryption solution uses general purpose
Quoted 8 lines
Documentation/crypto/msm/ice.txt line 9
Quoted 5 lines
Documentation/crypto/msm/ice.txt new file +5
-0+crypto engine. While this kind of solution provide a decent amount of +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.Is ICE part of the storage device or part of the host as the binding suggests?
My understanding is that for this particular instantiation, the Inline Crypto Engine is located on the SOC.
However, from the perspective of generic kernel support, the inline crypto support could be implemented on the SOC, or in the host bus adaptor, or as a "bump in the wire", or on the storage device. And whatever abstract interface in the block layer should be able to support all of these cases.
I do not believe it would be wise to assume that inline crypto will forever be a mobile-only thing. I could easily see use cases in the data center; for example, if you believe that Nation State Actors might be trying to create "implants" that attack hard drive firmware, per some of the Snowden leaks, creating an open design ICE engine with auditable firmware and a trusted secure key store, and which sits between the host CPU and the storage device might be one way to mitigate against this threat.
- Ted
Rob Herring
maintainerRe: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
Quoted 15 lines · Theodore Y. Ts'o
Quoted 4 lines · Rob Herring
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+Introduction: +============= +Storage encryption has been one of the most required feature from security +point of view. QTI based storage encryption solution uses general purposeQuoted 8 lines
Documentation/crypto/msm/ice.txt line 9
Quoted 5 lines
Documentation/crypto/msm/ice.txt new file +5
-0+crypto engine. While this kind of solution provide a decent amount of +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.Is ICE part of the storage device or part of the host as the binding suggests?
Quoted 2 lines
My understanding is that for this particular instantiation, the Inline Crypto Engine is located on the SOC.
Mine too, but that is not what this doc says.
Quoted 5 lines
However, from the perspective of generic kernel support, the inline crypto support could be implemented on the SOC, or in the host bus adaptor, or as a "bump in the wire", or on the storage device. And whatever abstract interface in the block layer should be able to support all of these cases.
Yes, certainly.
Rob
Re: [PATCH 3/3] crypto: qce: ice: Add support for Inline Crypto Engine
In reply to Theodore Y. Ts'oHi Rob,
Thanks for the comments,
Quoted 14 lines · Theodore Y. Ts'o
Quoted 4 lines · Rob Herring
Quoted 4 lines
Documentation/crypto/msm/ice.txt new file +4
-0+Introduction: +============= +Storage encryption has been one of the most required feature from security +point of view. QTI based storage encryption solution uses general purposeQuoted 8 lines
Documentation/crypto/msm/ice.txt line 9
Quoted 5 lines
Documentation/crypto/msm/ice.txt new file +5
-0+crypto engine. While this kind of solution provide a decent amount of +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.Is ICE part of the storage device or part of the host as the binding suggests?
Quoted 2 lines
My understanding is that for this particular instantiation, the Inline Crypto Engine is located on the SOC.
This is part of the Storage controller, illustration below
--------------------
| !_ICE_! | | | UFS/SDCC | | Controller | | | | |
--------------------
Quoted 6 lines
However, from the perspective of generic kernel support, the inline crypto support could be implemented on the SOC, or in the host bus adaptor, or as a "bump in the wire", or on the storage device. And whatever abstract interface in the block layer should be able to support all of these cases.
As name suggests ICE hardware is inline with the data lines of storage controller that is the reason why throughput is inline with storage speed. Having it out side of the controller kills the purpose of introducing ICE on storage controller. If this ICE hardware is placed outside then its similar to other crypto engines which are used for cryptographic operations. The main reason to keep ICE hardware inline with storage is to avoid extra latency (buffer copy) during read/writes which involves decryption/encryption.
Quoted 9 lines
I do not believe it would be wise to assume that inline crypto will forever be a mobile-only thing. I could easily see use cases in the data center; for example, if you believe that Nation State Actors might be trying to create "implants" that attack hard drive firmware, per some of the Snowden leaks, creating an open design ICE engine with auditable firmware and a trusted secure key store, and which sits between the host CPU and the storage device might be one way to mitigate against this threat.
Above comments valid here as well.
Quoted 2 lines
- Ted