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

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bef7c78c-2257-47ff-bccc-a9422afb1c42@oss.qualcomm.com>
Date: Mon, 27 Oct 2025 09:54:38 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Neeraj Soni <neeraj.soni@....qualcomm.com>, andersson@...nel.org,
        konradybcio@...nel.org
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc: qcom: Add HWKM v1 support for wrapped keys

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@....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