On 06/19/2024 12:12 AM PDT, Neil Armstrong wrote: > Le 19/06/2024 à 00:08, Gaurav Kashyap (QUIC) a écrit : > > Hello Neil, > > > > On 06/18/2024 12:14 AM PDT, Neil Armstrong wrote: > >> On 17/06/2024 02:50, Gaurav Kashyap wrote: > >>> Qualcomm's ICE (Inline Crypto Engine) contains a proprietary key > >>> management hardware called Hardware Key Manager (HWKM). > >>> This patch integrates HWKM support in ICE when it is available. HWKM > >>> primarily provides hardware wrapped key support where the ICE > >>> (storage) keys are not available in software and protected in > >>> hardware. > >>> > >>> When HWKM software support is not fully available (from Trustzone), > >>> there can be a scenario where the ICE hardware supports HWKM, but it > >>> cannot be used for wrapped keys. In this case, standard keys have to > >>> be used without using HWKM. Hence, providing a toggle controlled by > >>> a devicetree entry to use HWKM or not. > >>> > >>> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > >>> Signed-off-by: Gaurav Kashyap <quic_gaurkash@xxxxxxxxxxx> > >>> --- > >>> drivers/soc/qcom/ice.c | 153 > >> +++++++++++++++++++++++++++++++++++++++-- > >>> include/soc/qcom/ice.h | 1 + > >>> 2 files changed, 150 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c index > >>> 6f941d32fffb..d5e74cf2946b 100644 > >>> --- a/drivers/soc/qcom/ice.c > >>> +++ b/drivers/soc/qcom/ice.c > >>> @@ -26,6 +26,40 @@ > >> > >> <snip> > >> > >>> + > >>> static struct qcom_ice *qcom_ice_create(struct device *dev, > >>> void __iomem *base) > >>> { > >>> @@ -239,6 +382,8 @@ static struct qcom_ice *qcom_ice_create(struct > >> device *dev, > >>> engine->core_clk = devm_clk_get_enabled(dev, NULL); > >>> if (IS_ERR(engine->core_clk)) > >>> return ERR_CAST(engine->core_clk); > >>> + engine->use_hwkm = of_property_read_bool(dev->of_node, > >>> + "qcom,ice-use-hwkm"); > >> > >> Please drop this property and instead add an scm function calling: > >> > >> __qcom_scm_is_call_available(QCOM_SCM_SVC_ES, > >> QCOM_SCM_ES_DERIVE_SW_SECRET) > >> > >> like > >> > >> bool qcom_scm_derive_sw_secret_available(void) > >> { > >> if (!__qcom_scm_is_call_available(__scm->dev, > QCOM_SCM_SVC_ES, > >> QCOM_SCM_ES_DERIVE_SW_SECRET)) > >> return false; > >> > >> return true; > >> } > >> > >> You may perhaps only call qcom_scm_derive_sw_secret_available() for > >> some ICE versions. > >> > >> Neil > > > > The issue here is that for the same ICE version, based on the chipset, > > there might be different configurations. > > So use a combination of a list of compatible strings + > qcom_scm_derive_sw_secret_available() > to enable hwkm. > > Neil > Okay, that makes sense to me, I will try that. In fact, looking for only QCOM_SCM_ES_GENERATE_ICE_KEY instead of SW_SECRET works better. And would work even without compatible strings. As availability of QCOM_SCM_ES_GENERATE_ICE_KEY will correlate with TZ/firmware having the necessary support. > > > > Is it acceptable to use the addressable size from DTSI instead? > > Meaning, if it 0x8000, it would take the legacy route, and only when > > it has been updated to 0x10000, we would use HWKM and wrapped keys. > > > >> > >>> > >>> if (!qcom_ice_check_supported(engine)) > >>> return ERR_PTR(-EOPNOTSUPP); diff --git > >>> a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h index > >>> 9dd835dba2a7..1f52e82e3e1c 100644 > >>> --- a/include/soc/qcom/ice.h > >>> +++ b/include/soc/qcom/ice.h > >>> @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice, > >>> const struct blk_crypto_key *bkey, > >>> u8 data_unit_size, int slot); > >>> int qcom_ice_evict_key(struct qcom_ice *ice, int slot); > >>> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice); > >>> struct qcom_ice *of_qcom_ice_get(struct device *dev); > >>> #endif /* __QCOM_ICE_H__ */ > > > > Regards, > > Gaurav Regards, Gaurav