Re: [PATCH v2 6/7] arm64: dts: qcom: hamoa: Add inline crypto for UFS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The mapped register size for the `ice` (Inline Crypto Engine) node is too small, which will cause a kernel panic when Hardware Key Manager (HWKM) features are used.
--

commit d49e015ad1235b81d3dc9cb4256b44b206a03560
Author: Harrison Vanderbyl <harrison.vanderbyl@gmail.com>

arm64: dts: qcom: hamoa: Add inline crypto for UFS

This commit adds the Inline Crypto Engine node and wires it to ufs_mem to
enable UFS storage encryption on x1e80100 and derivative SOCs.

> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi

[ ... ]

> @@ -3997,6 +3999,14 @@ opp-300000000 {
>  			};
>  		};
>  
> +		ice: crypto@1d88000 {
> +			compatible = "qcom,x1e80100-inline-crypto-engine",
> +				     "qcom,inline-crypto-engine";
> +			reg = <0x0 0x01d88000 0x0 0x8000>;

Will this mapped size be large enough when hardware-wrapped keys are enabled
via the Hardware Key Manager (HWKM)?

When the qcom_ice driver initializes HWKM, it accesses registers at offsets
that appear to exceed the 0x8000 length provided here. For example, it accesses
QCOM_ICE_REG_HWKM_TZ_KM_STATUS at 0x9004, and QCOM_ICE_REG_HWKM_BANK0_BBAC_4
at 0xD010.

Could reading or writing to these offsets cause a synchronous external abort
and crash the kernel since they fall outside the mapped memory region?

Should the size be expanded to cover the full HWKM address space, such as
0x18000, similar to the configuration on other HWKM-capable SoCs?

> +
> +			clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778822464.git.harrison.vanderbyl@gmail.com?part=6