Re: [PATCH] soc: qcom: Add HWKM v1 support for wrapped keys

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 10/26/25 5:47 PM, Neeraj Soni wrote:
> HWKM v1 and v2 differ slightly in wrapped key size and the bit fields for
> certain status registers and operating mode (legacy or standard).
> 
> Add support to select HWKM version based on the major and minor revisions.
> Use this HWKM version to select wrapped key size and to configure the bit
> fields in registers for operating modes and hardware status.
> 
> Support for SCM calls for wrapped keys is being added in the TrustZone for
> few SoCs with HWKM v1. Existing check of qcom_scm_has_wrapped_key_support()
> API ensures that HWKM is used only if these SCM calls are supported in
> TrustZone for that SoC.
> 
> Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
> ---

The subject must say "ice:"

[...]

>  static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -114,9 +126,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
>  		return false;
>  	}
>  
> +	/* HWKM version v2 is present from ICE 3.2.1 onwards while version v1
> +	 * is present only in ICE 3.2.0.

What about before v3.2.0?

> +	 */
> +	if (major == 4 ||
> +	   (major == 3 && (minor >= 3 || (minor == 2 && step >= 1))))
> +		ice->hwkm_version = QCOM_ICE_HWKM_V2;
> +	else if ((major == 3) && (minor == 2))
> +		ice->hwkm_version = QCOM_ICE_HWKM_V1;
> +	else
> +		ice->hwkm_version = 0;

"if major > 3" is more future-proof than "== 4", unless you know for
a fact major == 5 etc. will have an incompatible software interface

>  	dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
>  		 major, minor, step);
>  
> +	if (!ice->hwkm_version)
> +		dev_info(dev, "QC Hard Ware Key Manager (HWKM) not supported\n");

"Hard Ware" looks *really* poor

[...]

> -	BUILD_BUG_ON(QCOM_ICE_HWKM_WRAPPED_KEY_SIZE >
> +	BUILD_BUG_ON(QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version) >
>  		     BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE);

This is not going to work how you want it to..

>  	/*
>  	 * When ICE is in HWKM mode, it only supports wrapped keys.
> @@ -238,9 +266,18 @@ static void qcom_ice_hwkm_init(struct qcom_ice *ice)
>  	 *
>  	 * Put ICE in HWKM mode.  ICE defaults to legacy mode.
>  	 */
> -	regval = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> -	regval &= ~QCOM_ICE_LEGACY_MODE_ENABLED;
> -	qcom_ice_writel(ice, regval, QCOM_ICE_REG_CONTROL);
> +	if (ice->hwkm_version == QCOM_ICE_HWKM_V2) {
> +		regval = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> +		regval &= ~QCOM_ICE_LEGACY_MODE_ENABLED;
> +		qcom_ice_writel(ice, regval, QCOM_ICE_REG_CONTROL);
> +	} else if (ice->hwkm_version == QCOM_ICE_HWKM_V1) {
> +		regval = qcom_ice_readl(ice, QCOM_ICE_REG_HWKM_TZ_KM_CTL);
> +		regval &= ~QCOM_ICE_HWKM_ICE_LEGACY_MODE_ENABLED;
> +		qcom_ice_writel(ice, regval, QCOM_ICE_REG_HWKM_TZ_KM_CTL);
> +	} else {
> +		dev_err(ice->dev, "Invalid HWKM version\n");
> +		return;
> +	}

The else path is impossible to reach

>  
>  	/* Disable CRC checks.  This HWKM feature is not used. */
>  	qcom_ice_writel(ice, QCOM_ICE_HWKM_DISABLE_CRC_CHECKS_VAL,
> @@ -298,7 +335,8 @@ EXPORT_SYMBOL_GPL(qcom_ice_suspend);
>  
>  static unsigned int translate_hwkm_slot(struct qcom_ice *ice, unsigned int slot)
>  {
> -	return slot * 2;
> +	/* The slot offset depends upon HWKM version */
> +	return (ice->hwkm_version == QCOM_ICE_HWKM_V1) ? (slot) : (slot * 2);

The parentheses are unnecessary

Konrad