Re: [PATCH v5 04/15] soc: qcom: ice: add hwkm support in ice

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 16, 2024 at 05:50:59PM GMT, 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 @@
>  #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 reg vals */
> +#define QCOM_ICE_HWKM_BIST_DONE_V1		BIT(16)
> +#define QCOM_ICE_HWKM_BIST_DONE_V2		BIT(9)
> +#define QCOM_ICE_HWKM_BIST_DONE(ver)		QCOM_ICE_HWKM_BIST_DONE_V##ver
> +
> +#define QCOM_ICE_HWKM_CRYPTO_BIST_DONE_V1		BIT(14)
> +#define QCOM_ICE_HWKM_CRYPTO_BIST_DONE_V2		BIT(7)
> +#define QCOM_ICE_HWKM_CRYPTO_BIST_DONE(v)		QCOM_ICE_HWKM_CRYPTO_BIST_DONE_V##v
> +
> +#define QCOM_ICE_HWKM_BOOT_CMD_LIST1_DONE		BIT(2)
> +#define QCOM_ICE_HWKM_BOOT_CMD_LIST0_DONE		BIT(1)
> +#define QCOM_ICE_HWKM_KT_CLEAR_DONE			BIT(0)
> +
> +#define QCOM_ICE_HWKM_BIST_VAL(v)	(QCOM_ICE_HWKM_BIST_DONE(v) |		\
> +					QCOM_ICE_HWKM_CRYPTO_BIST_DONE(v) |	\
> +					QCOM_ICE_HWKM_BOOT_CMD_LIST1_DONE |	\
> +					QCOM_ICE_HWKM_BOOT_CMD_LIST0_DONE |	\
> +					QCOM_ICE_HWKM_KT_CLEAR_DONE)
> +
> +#define QCOM_ICE_HWKM_V1_STANDARD_MODE_VAL	(BIT(0) | BIT(1) | BIT(2))
> +#define QCOM_ICE_HWKM_V2_STANDARD_MODE_MASK	GENMASK(31, 1)
> +#define QCOM_ICE_HWKM_DISABLE_CRC_CHECKS_VAL	(BIT(1) | BIT(2))
> +#define QCOM_ICE_HWKM_RSP_FIFO_CLEAR_VAL	BIT(3)
>  
>  /* BIST ("built-in self-test") status flags */
>  #define QCOM_ICE_BIST_STATUS_MASK		GENMASK(31, 28)
> @@ -34,6 +68,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 +83,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 +103,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,27 +166,106 @@ 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_val;
>  	int err;
>  
>  	err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
>  				 regval, !(regval & QCOM_ICE_BIST_STATUS_MASK),
>  				 50, 5000);
> -	if (err)
> +	if (err) {
>  		dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
> +		return err;
> +	}
>  
> +	if (ice->use_hwkm) {
> +		bist_done_val = ice->hwkm_version == 1 ?
> +				QCOM_ICE_HWKM_BIST_VAL(1) :
> +				QCOM_ICE_HWKM_BIST_VAL(2);
> +		if (qcom_ice_readl(ice,
> +				   HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> +				   bist_done_val) {
> +			dev_err(ice->dev, "HWKM BIST error\n");
> +			ice->use_hwkm = false;
> +			err = -ENODEV;
> +		}
> +	}
>  	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.

I can't say this is very logical.

standard mode => HW wrapped keys
legacy mode => standard keys

Consider changing the terms.

> +	 *
> +	 * Put ICE in standard mode, ICE defaults to legacy mode.
> +	 * Legacy mode - ICE HWKM slave not supported.
> +	 * Standard mode - ICE HWKM slave supported.

s/slave/some other term/

Is it possible to use both kind of keys when working on standard mode?
If not, it should be the user who selects what type of keys to be used.
Enforcing this via DT is not a way to go.

> +	 *
> +	 * 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 & QCOM_ICE_HWKM_V2_STANDARD_MODE_MASK;
> +		qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> +	} else {
> +		qcom_ice_writel(ice, QCOM_ICE_HWKM_V1_STANDARD_MODE_VAL,
> +				HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> +	}
> +}
> +
> +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> +{
> +	/* Disable CRC checks. This HWKM feature is not used. */
> +	qcom_ice_writel(ice, QCOM_ICE_HWKM_DISABLE_CRC_CHECKS_VAL,
> +			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, GENMASK(31, 0), HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
> +	qcom_ice_writel(ice, GENMASK(31, 0), HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> +	qcom_ice_writel(ice, GENMASK(31, 0), HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> +	qcom_ice_writel(ice, GENMASK(31, 0), HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> +	qcom_ice_writel(ice, GENMASK(31, 0), HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> +
> +	/* Clear HWKM response FIFO before doing anything */
> +	qcom_ice_writel(ice, QCOM_ICE_HWKM_RSP_FIFO_CLEAR_VAL,
> +			HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> +	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 +281,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 +292,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 +342,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
>  }
>  EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>  

Documentation?

> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> +	return ice->use_hwkm;

I see that use_hwkm can change during runtime. Will it have an impact on
a driver that calls this first?

> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> +
>  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");

DT bindings should come before driver changes.

>  
>  	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__ */
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry




[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux