| Message ID | 20240617005825.1443206-15-quic_gaurkash@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Hardware wrapped key support for qcom ice and ufs | expand |
On 17/06/2024 02:51, Gaurav Kashyap wrote: > The Inline Crypto Engine (ICE) for UFS/EMMC supports the > Hardware Key Manager (HWKM) to securely manage storage > keys. Enable using this hardware on sm8650. > > This requires two changes: > 1. Register size increase: HWKM is an additional piece of hardware > sitting alongside ICE, and extends the old ICE's register space. > 2. Explicitly tell the ICE driver to use HWKM with ICE so that > wrapped keys are used in sm8650. > > Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sm8650.dtsi | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi > index bb0b3c48ee4b..a34c4b7ccbac 100644 > --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi > @@ -2593,9 +2593,11 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, > ice: crypto@1d88000 { > compatible = "qcom,sm8650-inline-crypto-engine", > "qcom,inline-crypto-engine"; > - reg = <0 0x01d88000 0 0x8000>; > + reg = <0 0x01d88000 0 0x10000>; > > clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; > + > + qcom,ice-use-hwkm; Nah... this proves this is compatible specific, so drop the property. You already ignored such feedback once, so let me be clear: respond that you acknowledge the comment and that you will implement it. Best regards, Krzysztof
Hi, On 17/06/2024 02:51, Gaurav Kashyap wrote: > The Inline Crypto Engine (ICE) for UFS/EMMC supports the > Hardware Key Manager (HWKM) to securely manage storage > keys. Enable using this hardware on sm8650. > > This requires two changes: > 1. Register size increase: HWKM is an additional piece of hardware > sitting alongside ICE, and extends the old ICE's register space. > 2. Explicitly tell the ICE driver to use HWKM with ICE so that > wrapped keys are used in sm8650. > > Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sm8650.dtsi | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi > index bb0b3c48ee4b..a34c4b7ccbac 100644 > --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi > @@ -2593,9 +2593,11 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, > ice: crypto@1d88000 { > compatible = "qcom,sm8650-inline-crypto-engine", > "qcom,inline-crypto-engine"; > - reg = <0 0x01d88000 0 0x8000>; > + reg = <0 0x01d88000 0 0x10000>; > > clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; > + > + qcom,ice-use-hwkm; > }; > > tcsr_mutex: hwlock@1f40000 { Please split this (and next) in two patches: - one extending the register size + Fixes tag so it can backported to stable kernels - one adding qcom,ice-use-hwkm (if bindings maintainers agrees with this property) And please send sm8550 before sm8650... Thanks, Neil
On 6/17/24 10:28, neil.armstrong@linaro.org wrote: > Hi, > > On 17/06/2024 02:51, Gaurav Kashyap wrote: >> The Inline Crypto Engine (ICE) for UFS/EMMC supports the >> Hardware Key Manager (HWKM) to securely manage storage >> keys. Enable using this hardware on sm8650. >> >> This requires two changes: >> 1. Register size increase: HWKM is an additional piece of hardware >> sitting alongside ICE, and extends the old ICE's register space. >> 2. Explicitly tell the ICE driver to use HWKM with ICE so that >> wrapped keys are used in sm8650. >> >> Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com> >> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> >> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sm8650.dtsi | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi >> index bb0b3c48ee4b..a34c4b7ccbac 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi >> @@ -2593,9 +2593,11 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, >> ice: crypto@1d88000 { >> compatible = "qcom,sm8650-inline-crypto-engine", >> "qcom,inline-crypto-engine"; >> - reg = <0 0x01d88000 0 0x8000>; >> + reg = <0 0x01d88000 0 0x10000>; >> clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; >> + >> + qcom,ice-use-hwkm; >> }; >> tcsr_mutex: hwlock@1f40000 { > > Please split this (and next) in two patches: > - one extending the register size + Fixes tag so it can backported to stable kernels Would also be helpful to know which chipsets require this, so we can fix it up. FWIW: rg qcom,ufshc arch/arm64/boot/dts/qcom -l | wc -l returns 17 Konrad
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi index bb0b3c48ee4b..a34c4b7ccbac 100644 --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi @@ -2593,9 +2593,11 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, ice: crypto@1d88000 { compatible = "qcom,sm8650-inline-crypto-engine", "qcom,inline-crypto-engine"; - reg = <0 0x01d88000 0 0x8000>; + reg = <0 0x01d88000 0 0x10000>; clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; + + qcom,ice-use-hwkm; }; tcsr_mutex: hwlock@1f40000 {