| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4b6a8b75-8fc1-4888-a76a-f14a67521359@quicinc.com>
Date: Mon, 5 Feb 2024 23:52:42 +0530
From: Om Prakash Singh <quic_omprsing@...cinc.com>
To: Gaurav Kashyap <quic_gaurkash@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
<andersson@...nel.org>, <ebiggers@...gle.com>,
<neil.armstrong@...aro.org>, <srinivas.kandagatla@...aro.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<robh+dt@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <linux-mmc@...r.kernel.org>,
<kernel@...cinc.com>, <linux-crypto@...r.kernel.org>,
<devicetree@...r.kernel.org>, <quic_nguyenb@...cinc.com>,
<bartosz.golaszewski@...aro.org>, <konrad.dybcio@...aro.org>,
<ulf.hansson@...aro.org>, <jejb@...ux.ibm.com>,
<martin.petersen@...cle.com>, <mani@...nel.org>, <davem@...emloft.net>,
<herbert@...dor.apana.org.au>
Subject: Re: [PATCH v4 04/15] soc: qcom: ice: add hwkm support in ice
On 1/28/2024 4:44 AM, 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.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@...cinc.com>
> Tested-by: Neil Armstrong <neil.armstrong@...aro.org>
> ---
> drivers/soc/qcom/ice.c | 126 ++++++++++++++++++++++++++++++++++++++++-
> include/soc/qcom/ice.h | 1 +
> 2 files changed, 124 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 6f941d32fffb..c718e8153b23 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -26,6 +26,20 @@
> #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> #define QCOM_ICE_REG_BIST_STATUS 0x0070
> #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +#define QCOM_ICE_REG_CONTROL 0x0
> +/* QCOM ICE HWKM registers */
> +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
> +
> +/* QCOM ICE HWKM BIST vals */
> +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x14007
> +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
>
> /* BIST ("built-in self-test") status flags */
> #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> @@ -34,6 +48,9 @@
> #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
>
> +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> +#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
> +
> #define qcom_ice_writel(engine, val, reg) \
> writel((val), (engine)->base + (reg))
>
> @@ -46,6 +63,9 @@ struct qcom_ice {
> struct device_link *link;
>
> struct clk *core_clk;
> + u8 hwkm_version;
> + bool use_hwkm;
> + bool hwkm_init_complete;
> };
>
> static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -63,8 +83,21 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> return false;
> }
>
> - dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> - major, minor, step);
> + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> + ice->hwkm_version = 2;
> + else if (major == 3 && minor == 2)
> + ice->hwkm_version = 1;
> + else
> + ice->hwkm_version = 0;
> +
> + if (ice->hwkm_version == 0)
> + ice->use_hwkm = false;
> +
> + dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d, HWKM v%d\n",
> + major, minor, step, ice->hwkm_version);
> +
> + if (!ice->use_hwkm)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not used/supported");
>
> /* If fuses are blown, ICE might not work in the standard way. */
> regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
> @@ -113,10 +146,14 @@ static void qcom_ice_optimization_enable(struct qcom_ice *ice)
> * fails, so we needn't do it in software too, and (c) properly testing
> * storage encryption requires testing the full storage stack anyway,
> * and not relying on hardware-level self-tests.
> + *
> + * However, we still care about if HWKM BIST failed (when supported) as
> + * important functionality would fail later, so disable hwkm on failure.
> */
> static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> {
> u32 regval;
> + u32 bist_done_reg;
> int err;
>
> err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> @@ -125,15 +162,85 @@ static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> if (err)
> dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
>
> + if (ice->use_hwkm) {
> + bist_done_reg = (ice->hwkm_version == 1) ?
> + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> + if (qcom_ice_readl(ice,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> + bist_done_reg) {
> + dev_err(ice->dev, "HWKM BIST error\n");
err is not upsated to capture this failure.
> + ice->use_hwkm = false;
> + }
> + }
> return err;
> }
>
> +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
> +{
> + u32 val = 0;
> +
> + /*
> + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> + * keys, and when it is in legacy mode, it only supports standard
> + * (non HW wrapped) keys.
> + *
> + * Put ICE in standard mode, ICE defaults to legacy mode.
> + * Legacy mode - ICE HWKM slave not supported.
> + * Standard mode - ICE HWKM slave supported.
> + *
> + * Depending on the version of HWKM, it is controlled by different
> + * registers in ICE.
> + */
> + if (ice->hwkm_version >= 2) {
> + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> + val = val & 0xFFFFFFFE;
do not use constant "0xFFFFFFFE". Better to define bits that are being set.
> + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> + } else {
> + qcom_ice_writel(ice, 0x7, HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
do not use constant "0x7". Better to define bits that are being set.
> + }
> +}
> +
> +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> +{
> + /* Disable CRC checks. This HWKM feature is not used. */
> + qcom_ice_writel(ice, 0x6,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> +
> + /*
> + * Give register bank of the HWKM slave access to read and modify
> + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> + * be able to program keys into ICE.
> + */
> + qcom_ice_writel(ice, 0xFFFFFFFF, HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
> + qcom_ice_writel(ice, 0xFFFFFFFF, HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> + qcom_ice_writel(ice, 0xFFFFFFFF, HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> + qcom_ice_writel(ice, 0xFFFFFFFF, HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> + qcom_ice_writel(ice, 0xFFFFFFFF, HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> +
> + /* Clear HWKM response FIFO before doing anything */
> + qcom_ice_writel(ice, 0x8, HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
Do not use constant "0x8". Please define bits that are being set.
> + ice->hwkm_init_complete = true;
> +}
> +
> int qcom_ice_enable(struct qcom_ice *ice)
> {
> + int err;
> +
> qcom_ice_low_power_mode_enable(ice);
> qcom_ice_optimization_enable(ice);
>
> - return qcom_ice_wait_bist_status(ice);
> + if (ice->use_hwkm)
> + qcom_ice_enable_standard_mode(ice);
> +
> + err = qcom_ice_wait_bist_status(ice);
> + if (err)
> + return err;
> +
> + if (ice->use_hwkm)
> + qcom_ice_hwkm_init(ice);
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(qcom_ice_enable);
>
> @@ -149,6 +256,10 @@ int qcom_ice_resume(struct qcom_ice *ice)
> return err;
> }
>
> + if (ice->use_hwkm) {
> + qcom_ice_enable_standard_mode(ice);
> + qcom_ice_hwkm_init(ice);
> + }
> return qcom_ice_wait_bist_status(ice);
> }
> EXPORT_SYMBOL_GPL(qcom_ice_resume);
> @@ -156,6 +267,7 @@ EXPORT_SYMBOL_GPL(qcom_ice_resume);
> int qcom_ice_suspend(struct qcom_ice *ice)
> {
> clk_disable_unprepare(ice->core_clk);
> + ice->hwkm_init_complete = false;
>
> return 0;
> }
> @@ -205,6 +317,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> + return ice->use_hwkm;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> @@ -239,6 +357,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");
>
> 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__ */
Powered by blists - more mailing lists