linux-kernel - Re: [PATCH] soc: imx: Dump higher 64bits UID

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250317094518.yf2hbq5sjl7lgfwb@pengutronix.de>
Date: Mon, 17 Mar 2025 10:45:18 +0100
From: Marco Felsch <m.felsch@...gutronix.de>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
Cc: shawnguo@...nel.org, s.hauer@...gutronix.de, marex@...x.de,
	imx@...ts.linux.dev, Peng Fan <peng.fan@....com>,
	linux-kernel@...r.kernel.org, kernel@...gutronix.de,
	festevam@...il.com, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] soc: imx: Dump higher 64bits UID

Hi Peng,

On 25-03-14, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@....com>
> 
> i.MX8MP UID is actually 128bits and partitioned into two parts.
> The 1st 64bits are at 0x410 and 0x420, and 2nd 64bits are at 0xE00
> and 0xE10.
> 
> Dump the whole 128bits for i.MX8MP, by introducing soc_uid_h.

Thanks for taking care of this :) We noticed that this is crucial for
burning the field-return fuse to keep the LOCK fuse unlocked.

> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
>  drivers/soc/imx/soc-imx8m.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
> index 3ed8161d7d28..02d8c85e9a28 100644
> --- a/drivers/soc/imx/soc-imx8m.c
> +++ b/drivers/soc/imx/soc-imx8m.c
> @@ -24,13 +24,15 @@
>  #define OCOTP_UID_HIGH			0x420
>  
>  #define IMX8MP_OCOTP_UID_OFFSET		0x10
> +#define IMX8MP_OCOTP_UID_HIGH		0xE00
>  
>  /* Same as ANADIG_DIGPROG_IMX7D */
>  #define ANADIG_DIGPROG_IMX8MM	0x800
>  
>  struct imx8_soc_data {
>  	char *name;
> -	int (*soc_revision)(u32 *socrev, u64 *socuid);
> +	int (*soc_revision)(u32 *socrev, u64 *socuid, u64 *socuid_h);
> +	bool uid_len_128;

Albeit your patch is not wrong, it could be far simpler if we redefine
the soc_revision to serial_number. This way each SoC can specify 
the dedicated mechanism of setting the serial number. Going this way we
need two patches, the first one is converting the logic and the 2nd one
is adding a dedicated hook for the i.MX8MP.

Regards,
  Marco

>  };
>  
>  #ifdef CONFIG_HAVE_ARM_SMCCC
> @@ -49,7 +51,7 @@ static u32 imx8mq_soc_revision_from_atf(void)
>  static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
>  #endif
>  
> -static int imx8mq_soc_revision(u32 *socrev, u64 *socuid)
> +static int imx8mq_soc_revision(u32 *socrev, u64 *socuid, u64 *socuid_h)
>  {
>  	struct device_node *np __free(device_node) =
>  		of_find_compatible_node(NULL, NULL, "fsl,imx8mq-ocotp");
> @@ -102,7 +104,7 @@ static int imx8mq_soc_revision(u32 *socrev, u64 *socuid)
>  	return ret;
>  }
>  
> -static int imx8mm_soc_uid(u64 *socuid)
> +static int imx8mm_soc_uid(u64 *socuid, u64 *socuid_h)
>  {
>  	struct device_node *np __free(device_node) =
>  		of_find_compatible_node(NULL, NULL, "fsl,imx8mm-ocotp");
> @@ -131,6 +133,12 @@ static int imx8mm_soc_uid(u64 *socuid)
>  	*socuid <<= 32;
>  	*socuid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW + offset);
>  
> +	if (offset) {
> +		*socuid_h = readl_relaxed(ocotp_base + IMX8MP_OCOTP_UID_HIGH + 0x10);
> +		*socuid_h <<= 32;
> +		*socuid_h |= readl_relaxed(ocotp_base + IMX8MP_OCOTP_UID_HIGH);
> +	}
> +
>  	clk_disable_unprepare(clk);
>  	clk_put(clk);
>  
> @@ -139,7 +147,7 @@ static int imx8mm_soc_uid(u64 *socuid)
>  	return ret;
>  }
>  
> -static int imx8mm_soc_revision(u32 *socrev, u64 *socuid)
> +static int imx8mm_soc_revision(u32 *socrev, u64 *socuid, u64 *socuid_h)
>  {
>  	struct device_node *np __free(device_node) =
>  		of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
> @@ -156,7 +164,7 @@ static int imx8mm_soc_revision(u32 *socrev, u64 *socuid)
>  
>  	iounmap(anatop_base);
>  
> -	return imx8mm_soc_uid(socuid);
> +	return imx8mm_soc_uid(socuid, socuid_h);
>  }
>  
>  static const struct imx8_soc_data imx8mq_soc_data = {
> @@ -177,6 +185,7 @@ static const struct imx8_soc_data imx8mn_soc_data = {
>  static const struct imx8_soc_data imx8mp_soc_data = {
>  	.name = "i.MX8MP",
>  	.soc_revision = imx8mm_soc_revision,
> +	.uid_len_128 = true,
>  };
>  
>  static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> @@ -211,7 +220,7 @@ static int imx8m_soc_probe(struct platform_device *pdev)
>  	const struct of_device_id *id;
>  	struct soc_device *soc_dev;
>  	u32 soc_rev = 0;
> -	u64 soc_uid = 0;
> +	u64 soc_uid = 0, soc_uid_h = 0;
>  	int ret;
>  
>  	soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
> @@ -232,7 +241,7 @@ static int imx8m_soc_probe(struct platform_device *pdev)
>  	if (data) {
>  		soc_dev_attr->soc_id = data->name;
>  		if (data->soc_revision) {
> -			ret = data->soc_revision(&soc_rev, &soc_uid);
> +			ret = data->soc_revision(&soc_rev, &soc_uid, &soc_uid_h);
>  			if (ret)
>  				return ret;
>  		}
> @@ -242,7 +251,11 @@ static int imx8m_soc_probe(struct platform_device *pdev)
>  	if (!soc_dev_attr->revision)
>  		return -ENOMEM;
>  
> -	soc_dev_attr->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%016llX", soc_uid);
> +	if (data && data->uid_len_128)
> +		soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%016llX%016llX",
> +							soc_uid_h, soc_uid);
> +	else
> +		soc_dev_attr->serial_number = devm_kasprintf(dev, GFP_KERNEL, "%016llX", soc_uid);
>  	if (!soc_dev_attr->serial_number)
>  		return -ENOMEM;
>  
> -- 
> 2.37.1
> 
> 
>