diff mbox series

[v2,2/2] drivers: misc: Add driver to access ZynqMP efuses

Message ID 20240515124125.732487-3-lukas.funke-oss@weidmueller.com
State Superseded
Delegated to: Michal Simek
Headers show
Series Add eFuse access for ZynqMP | expand

Commit Message

Lukas Funke May 15, 2024, 12:41 p.m. UTC
From: Lukas Funke <lukas.funke@weidmueller.com>

Add driver to access ZynqMP efuses. This is a u-boot port of [1].

[1] https://lore.kernel.org/all/20240224114516.86365-8-srinivas.kandagatla@linaro.org/

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

Changes in v2:
- Drop vendor specific fuse cmd, use existing fuse cmd
- Minor code refactoring (reverse x-mas tree)

 drivers/misc/Kconfig        |   8 +
 drivers/misc/Makefile       |   1 +
 drivers/misc/zynqmp_efuse.c | 324 ++++++++++++++++++++++++++++++++++++
 3 files changed, 333 insertions(+)
 create mode 100644 drivers/misc/zynqmp_efuse.c

Comments

Michal Simek May 29, 2024, 2:40 p.m. UTC | #1
On 5/15/24 14:41, lukas.funke-oss@weidmueller.com wrote:
> From: Lukas Funke <lukas.funke@weidmueller.com>
> 
> Add driver to access ZynqMP efuses. This is a u-boot port of [1].
> 
> [1] https://lore.kernel.org/all/20240224114516.86365-8-srinivas.kandagatla@linaro.org/
> 
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> ---
> 
> Changes in v2:
> - Drop vendor specific fuse cmd, use existing fuse cmd
> - Minor code refactoring (reverse x-mas tree)
> 
>   drivers/misc/Kconfig        |   8 +
>   drivers/misc/Makefile       |   1 +
>   drivers/misc/zynqmp_efuse.c | 324 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 333 insertions(+)
>   create mode 100644 drivers/misc/zynqmp_efuse.c

would be good to get also 3/3 which enables
CONFIG_CMD_FUSE=y
CONFIG_ZYNQMP_EFUSE=y

in zynqmp defconfigs (virt and kria) to have it enabled.

> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 6009d55f400..c07f50c9a76 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -298,6 +298,14 @@ config FSL_SEC_MON
>   	  Security Monitor can be transitioned on any security failures,
>   	  like software violations or hardware security violations.
>   
> +config ZYNQMP_EFUSE
> +	bool "Enable ZynqMP eFUSE Driver"
> +	depends on ZYNQMP_FIRMWARE
> +	help
> +	  Enable access to Zynq UltraScale (ZynqMP) eFUSEs thought PMU firmware
> +	  interface. ZnyqMP has 256 eFUSEs where some of them are security related
> +	  and cannot be read back (i.e. AES key).
> +
>   choice
>   	prompt "Security monitor interaction endianess"
>   	depends on FSL_SEC_MON
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e53d52c47b3..68ba5648eab 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -92,3 +92,4 @@ obj-$(CONFIG_ESM_K3) += k3_esm.o
>   obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
>   obj-$(CONFIG_SL28CPLD) += sl28cpld.o
>   obj-$(CONFIG_SPL_SOCFPGA_DT_REG) += socfpga_dtreg.o
> +obj-$(CONFIG_ZYNQMP_EFUSE) += zynqmp_efuse.o
> diff --git a/drivers/misc/zynqmp_efuse.c b/drivers/misc/zynqmp_efuse.c
> new file mode 100644
> index 00000000000..98a39c1ebdd
> --- /dev/null
> +++ b/drivers/misc/zynqmp_efuse.c
> @@ -0,0 +1,324 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2014 - 2015 Xilinx, Inc.
> + * Michal Simek <michal.simek@amd.com>
> + *
> + * (C) Copyright 2024 Weidmueller Interface GmbH
> + * Lukas Funke <lukas.funke@weidmueller.com>
> + */
> +
> +#include <compiler.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <zynqmp_firmware.h>
> +#include <asm/dma-mapping.h>
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <misc.h>
> +
> +#define SILICON_REVISION_MASK 0xF
> +#define P_USER_0_64_UPPER_MASK	0x5FFF0000
> +#define P_USER_127_LOWER_4_BIT_MASK 0xF
> +#define WORD_INBYTES		(4)
> +#define SOC_VER_SIZE		(0x4)
> +#define SOC_VERSION_OFFSET	(0x0)
> +#define EFUSE_START_OFFSET	(0xC)
> +#define EFUSE_END_OFFSET	(0xFC)
> +#define EFUSE_PUF_START_OFFSET	(0x100)
> +#define EFUSE_PUF_MID_OFFSET	(0x140)
> +#define EFUSE_PUF_END_OFFSET	(0x17F)
> +#define EFUSE_NOT_ENABLED	(29)
> +#define EFUSE_READ		(0)
> +#define EFUSE_WRITE		(1)

I did compare it with the latest upstream version and there are some differences 
which don't need to be there. Above macros for example

> +
> +/**
> + * struct efuse_map_entry - offset and length of zynqmp fuses
> + * @offset:	offset of efuse to be read/write
> + * @length:	length of efuse
> + */
> +struct efuse_map_entry {
> +	u32 offset;
> +	u32 length;
> +};
> +
> +struct efuse_map_entry zynqmp_efuse_table[] = {
> +	{0x000, 0x04}, /* soc revision */
> +	{0x00c, 0x0c}, /* SoC DNA */
> +	{0x020, 0x04}, /* efuse-usr0 */
> +	{0x024, 0x04}, /* efuse-usr1 */
> +	{0x028, 0x04}, /* efuse-usr2 */
> +	{0x02c, 0x04}, /* efuse-usr3 */
> +	{0x030, 0x04}, /* efuse-usr4 */
> +	{0x034, 0x04}, /* efuse-usr5 */
> +	{0x038, 0x04}, /* efuse-usr6 */
> +	{0x03c, 0x04}, /* efuse-usr7 */
> +	{0x040, 0x04}, /* efuse-miscusr */
> +	{0x050, 0x04}, /* efuse-chash */
> +	{0x054, 0x04}, /* efuse-pufmisc */
> +	{0x058, 0x04}, /* efuse-sec */
> +	{0x05c, 0x04}, /* efuse-spkid */
> +	{0x060, 0x30}, /* efuse-aeskey */
> +	{0x0a0, 0x30}, /* ppk0-hash */
> +	{0x0d0, 0x30}, /* ppk1-hash */
> +	{0x100, 0x7f}, /* pufuser */
> +};
> +
> +/**
> + * struct xilinx_efuse - the basic structure
> + * @src:	address of the buffer to store the data to be write/read
> + * @size:	no of words to be read/write
> + * @offset:	offset to be read/write`

a little bit different description compare to upsream linux

> + * @flag:	0 - represents efuse read and 1- represents efuse write
> + * @pufuserfuse:0 - represents non-puf efuses, offset is used for read/write
> + *		1 - represents puf user fuse row number.
> + *
> + * this structure stores all the required details to
> + * read/write efuse memory.
> + */
> +struct xilinx_efuse {
> +	u64 src;
> +	u32 size;
> +	u32 offset;
> +	u32 flag;
> +	u32 pufuserfuse;
> +};
> +
> +static int zynqmp_efuse_get_length(u32 offset, u32 *base_offset, u32 *len)
> +{
> +	struct efuse_map_entry *fuse;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(zynqmp_efuse_table); ++i) {
> +		fuse = &zynqmp_efuse_table[i];
> +		if (offset >= fuse->offset &&
> +		    offset < fuse->offset + fuse->length) {
> +			*base_offset = fuse->offset;
> +			*len = fuse->length;
> +			return 0;
> +			}
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +static int zynqmp_efuse_access(struct udevice *dev, unsigned int offset,
> +			       void *val, size_t bytes, unsigned int flag,
> +			       unsigned int pufflag)
> +{
> +	size_t words = bytes / WORD_INBYTES;
> +	struct xilinx_efuse *efuse;
> +	ulong dma_addr, dma_buf;
> +	int ret, value;
> +	char *data;
> +
> +	if (bytes % WORD_INBYTES != 0) {
> +		dev_err(dev, "Bytes requested should be word aligned\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (pufflag == 0 && offset % WORD_INBYTES) {
> +		dev_err(dev, "Offset requested should be word aligned\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (pufflag == 1 && flag == EFUSE_WRITE) {
> +		memcpy(&value, val, bytes);
> +		if ((offset == EFUSE_PUF_START_OFFSET ||
> +		     offset == EFUSE_PUF_MID_OFFSET) &&
> +		    value & P_USER_0_64_UPPER_MASK) {
> +			dev_err(dev, "Only lower 4 bytes are allowed to be programmed in P_USER_0 & P_USER_64\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (offset == EFUSE_PUF_END_OFFSET &&
> +		    (value & P_USER_127_LOWER_4_BIT_MASK)) {
> +			dev_err(dev, "Only MSB 28 bits are allowed to be programmed for P_USER_127\n");
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +	efuse = dma_alloc_coherent(sizeof(struct xilinx_efuse), &dma_addr);
> +	if (!efuse)
> +		return -ENOMEM;
> +
> +	data = dma_alloc_coherent(bytes, &dma_buf);
> +	if (!data) {
> +		dma_free_coherent(efuse);
> +		return -ENOMEM;
> +	}
> +
> +	if (flag == EFUSE_WRITE) {
> +		memcpy(data, val, bytes);
> +		efuse->flag = EFUSE_WRITE;
> +	} else {
> +		efuse->flag = EFUSE_READ;
> +	}
> +
> +	efuse->src = dma_buf;
> +	efuse->size = words;
> +	efuse->offset = offset;
> +	efuse->pufuserfuse = pufflag;
> +
> +	flush_dcache_range((ulong)efuse, (ulong)efuse +
> +				   roundup(sizeof(struct xilinx_efuse), ARCH_DMA_MINALIGN));
> +	flush_dcache_range((ulong)data, (ulong)data +
> +				   roundup(sizeof(struct xilinx_efuse), ARCH_DMA_MINALIGN));
> +
> +	zynqmp_pm_efuse_access(dma_addr, (u32 *)&ret);
> +	if (ret != 0) {
> +		if (ret == EFUSE_NOT_ENABLED) {
> +			dev_err(dev, "efuse access is not enabled\n");
> +			ret = -EOPNOTSUPP;
> +			goto END;
> +		}
> +		dev_err(dev, "Error in efuse read %x\n", ret);
> +		ret = -EPERM;
> +		goto END;
> +	}
> +
> +	if (flag == EFUSE_READ)
> +		memcpy(val, data, bytes);
> +END:

this has also changed and was commented by Stefan before.

efuse_access_err:
	dma_free_coherent(dev, sizeof(bytes),
			  data, dma_buf);
efuse_data_fail:
	dma_free_coherent(dev, sizeof(struct xilinx_efuse),
			  efuse, dma_addr);



> +
> +	dma_free_coherent(efuse);
> +	dma_free_coherent(data);
> +
> +	return ret;
> +}
> +
> +static int zynqmp_nvmem_read(struct udevice *dev, int offset,
> +			     void *val, int bytes)

please check variables types.

> +{
> +	int ret, pufflag = 0;
> +	int idcode, version;
> +
> +	if (offset >= EFUSE_PUF_START_OFFSET && offset <= EFUSE_PUF_END_OFFSET)
> +		pufflag = 1;
> +
> +	dev_dbg(dev, "reading from offset=0x%x, bytes=%d\n", offset, bytes);
> +
> +	switch (offset) {
> +	/* Soc version offset is zero */
> +	case SOC_VERSION_OFFSET:
> +		if (bytes != SOC_VER_SIZE)
> +			return -EOPNOTSUPP;
> +
> +		ret = zynqmp_pm_get_chipid((u32 *)&idcode, (u32 *)&version);
> +		if (ret < 0)
> +			return ret;
> +
> +		*(int *)val = version & SILICON_REVISION_MASK;
> +		break;
> +	/* Efuse offset starts from 0xc */
> +	case EFUSE_START_OFFSET ... EFUSE_END_OFFSET:
> +	case EFUSE_PUF_START_OFFSET ... EFUSE_PUF_END_OFFSET:
> +		ret = zynqmp_efuse_access(dev, offset, val,
> +					  bytes, EFUSE_READ, pufflag);
> +		break;
> +	default:
> +		*(u32 *)val = 0xDEADBEEF;
> +		ret = 0;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int zynqmp_nvmem_write(struct udevice *dev, int offset, const void *val,
> +			      int bytes)

here too. offset for example

> +{
> +	int pufflag = 0;
> +
> +	dev_dbg(dev, "writing to offset=0x%x, bytes=%d", offset, bytes);
> +
> +	if (offset < EFUSE_START_OFFSET || offset > EFUSE_PUF_END_OFFSET)
> +		return -EOPNOTSUPP;
> +
> +	if (offset >= EFUSE_PUF_START_OFFSET && offset <= EFUSE_PUF_END_OFFSET)
> +		pufflag = 1;
> +
> +	return zynqmp_efuse_access(dev, offset,
> +				   (void *)val, bytes, EFUSE_WRITE, pufflag);
> +}
> +
> +int fuse_read(u32 bank, u32 word, u32 *val)
> +{
> +	u32 base_offset, offset, len;
> +	struct udevice *dev;
> +	u8 buf[128];

128 is correct number but would be better to have connection where this number 
is coming from. MAX size is 0x7f which is 127 but that connection is not visible 
from it. I am fine with the comment to say it.  Please look below.

> +	int ret;
> +
> +	memset(buf, 0, sizeof(buf));

I would call it before misc_read not to waste time in error case.

> +
> +	ret = uclass_get_device_by_driver(UCLASS_MISC,
> +					  DM_DRIVER_GET(zynqmp_efuse), &dev);
> +	if (ret)
> +		return log_msg_ret("uclass_drv", ret);

Above c&p from Linux is using dev_dbg structure and here you are using log 
infrastructure. Would be much better if you can use the same.


> +
> +	ret = zynqmp_efuse_get_length(word, &base_offset, &len);
> +	if (ret)
> +		return log_msg_ret("efuse_offset", ret);
> +

And here please check that len is below 128 to make sure that we never overwrite 
stack. Other entries can be added and it should be spotted.

> +	ret = misc_read(dev, base_offset, (void *)buf, len);
> +	if (ret)
> +		return log_msg_ret("misc_read", ret);
> +
> +	offset = word - base_offset;
> +	*val = ((u32 *)buf)[offset];
> +
> +	return 0;
> +}
> +
> +int fuse_prog(u32 bank, u32 word, u32 val)
> +{
> +	u32 base_offset, len;
> +	struct udevice *dev;
> +	int ret;
> +
> +	ret = zynqmp_efuse_get_length(word, &base_offset, &len);
> +	if (ret)
> +		return log_msg_ret("efuse_offset", ret);
> +
> +	ret = uclass_get_device_by_driver(UCLASS_MISC,
> +					  DM_DRIVER_GET(zynqmp_efuse), &dev);
> +	if (ret)
> +		return log_msg_ret("uclass_drv", ret);
> +
> +	if (word != base_offset || len != sizeof(val))
> +		return -EINVAL;
> +
> +	ret = misc_write(dev, word, (void *)&val, sizeof(val));
> +	if (ret)
> +		return log_msg_ret("misc_write", ret);
> +
> +	return 0;
> +}
> +
> +int fuse_sense(u32 bank, u32 word, u32 *val)
> +{
> +	/* We do not support sense */

avoid we - imperative mood please.

> +	return -ENOSYS;
> +}
> +
> +int fuse_override(u32 bank, u32 word, u32 val)
> +{
> +	/* We do not support overriding */

avoid we

> +	return -ENOSYS;
> +}
> +
> +static const struct udevice_id zynqmp_efuse_match[] = {
> +	{ .compatible = "xlnx,zynqmp-nvmem-fw", },
> +	{ /* sentinel */ },
> +};
> +
> +static const struct misc_ops zynqmp_efuse_ops = {
> +	.read = zynqmp_nvmem_read,
> +	.write = zynqmp_nvmem_write,
> +};
> +
> +U_BOOT_DRIVER(zynqmp_efuse) = {
> +	.name = "zynqmp_efuse",
> +	.id = UCLASS_MISC,
> +	.of_match = zynqmp_efuse_match,
> +	.ops = &zynqmp_efuse_ops,
> +};

Thanks,
Michal
Lukas Funke June 3, 2024, 6:48 a.m. UTC | #2
On 29.05.2024 16:40, Michal Simek wrote:
> 
> 
> On 5/15/24 14:41, lukas.funke-oss@weidmueller.com wrote:
>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>
>> Add driver to access ZynqMP efuses. This is a u-boot port of [1].
>>
>> [1] 
>> https://lore.kernel.org/all/20240224114516.86365-8-srinivas.kandagatla@linaro.org/
>>
>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>> ---
>>
>> Changes in v2:
>> - Drop vendor specific fuse cmd, use existing fuse cmd
>> - Minor code refactoring (reverse x-mas tree)
>>
>>   drivers/misc/Kconfig        |   8 +
>>   drivers/misc/Makefile       |   1 +
>>   drivers/misc/zynqmp_efuse.c | 324 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 333 insertions(+)
>>   create mode 100644 drivers/misc/zynqmp_efuse.c
> 
> would be good to get also 3/3 which enables
> CONFIG_CMD_FUSE=y
> CONFIG_ZYNQMP_EFUSE=y
> 
> in zynqmp defconfigs (virt and kria) to have it enabled.
> 
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 6009d55f400..c07f50c9a76 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -298,6 +298,14 @@ config FSL_SEC_MON
>>         Security Monitor can be transitioned on any security failures,
>>         like software violations or hardware security violations.
>> +config ZYNQMP_EFUSE
>> +    bool "Enable ZynqMP eFUSE Driver"
>> +    depends on ZYNQMP_FIRMWARE
>> +    help
>> +      Enable access to Zynq UltraScale (ZynqMP) eFUSEs thought PMU 
>> firmware
>> +      interface. ZnyqMP has 256 eFUSEs where some of them are 
>> security related
>> +      and cannot be read back (i.e. AES key).
>> +
>>   choice
>>       prompt "Security monitor interaction endianess"
>>       depends on FSL_SEC_MON
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index e53d52c47b3..68ba5648eab 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -92,3 +92,4 @@ obj-$(CONFIG_ESM_K3) += k3_esm.o
>>   obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
>>   obj-$(CONFIG_SL28CPLD) += sl28cpld.o
>>   obj-$(CONFIG_SPL_SOCFPGA_DT_REG) += socfpga_dtreg.o
>> +obj-$(CONFIG_ZYNQMP_EFUSE) += zynqmp_efuse.o
>> diff --git a/drivers/misc/zynqmp_efuse.c b/drivers/misc/zynqmp_efuse.c
>> new file mode 100644
>> index 00000000000..98a39c1ebdd
>> --- /dev/null
>> +++ b/drivers/misc/zynqmp_efuse.c
>> @@ -0,0 +1,324 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2014 - 2015 Xilinx, Inc.
>> + * Michal Simek <michal.simek@amd.com>
>> + *
>> + * (C) Copyright 2024 Weidmueller Interface GmbH
>> + * Lukas Funke <lukas.funke@weidmueller.com>
>> + */
>> +
>> +#include <compiler.h>
>> +#include <linux/types.h>
>> +#include <linux/errno.h>
>> +#include <zynqmp_firmware.h>
>> +#include <asm/dma-mapping.h>
>> +#include <dm.h>
>> +#include <dm/device_compat.h>
>> +#include <misc.h>
>> +
>> +#define SILICON_REVISION_MASK 0xF
>> +#define P_USER_0_64_UPPER_MASK    0x5FFF0000
>> +#define P_USER_127_LOWER_4_BIT_MASK 0xF
>> +#define WORD_INBYTES        (4)
>> +#define SOC_VER_SIZE        (0x4)
>> +#define SOC_VERSION_OFFSET    (0x0)
>> +#define EFUSE_START_OFFSET    (0xC)
>> +#define EFUSE_END_OFFSET    (0xFC)
>> +#define EFUSE_PUF_START_OFFSET    (0x100)
>> +#define EFUSE_PUF_MID_OFFSET    (0x140)
>> +#define EFUSE_PUF_END_OFFSET    (0x17F)
>> +#define EFUSE_NOT_ENABLED    (29)
>> +#define EFUSE_READ        (0)
>> +#define EFUSE_WRITE        (1)
> 
> I did compare it with the latest upstream version and there are some 
> differences which don't need to be there. Above macros for example
> 
>> +
>> +/**
>> + * struct efuse_map_entry - offset and length of zynqmp fuses
>> + * @offset:    offset of efuse to be read/write
>> + * @length:    length of efuse
>> + */
>> +struct efuse_map_entry {
>> +    u32 offset;
>> +    u32 length;
>> +};
>> +
>> +struct efuse_map_entry zynqmp_efuse_table[] = {
>> +    {0x000, 0x04}, /* soc revision */
>> +    {0x00c, 0x0c}, /* SoC DNA */
>> +    {0x020, 0x04}, /* efuse-usr0 */
>> +    {0x024, 0x04}, /* efuse-usr1 */
>> +    {0x028, 0x04}, /* efuse-usr2 */
>> +    {0x02c, 0x04}, /* efuse-usr3 */
>> +    {0x030, 0x04}, /* efuse-usr4 */
>> +    {0x034, 0x04}, /* efuse-usr5 */
>> +    {0x038, 0x04}, /* efuse-usr6 */
>> +    {0x03c, 0x04}, /* efuse-usr7 */
>> +    {0x040, 0x04}, /* efuse-miscusr */
>> +    {0x050, 0x04}, /* efuse-chash */
>> +    {0x054, 0x04}, /* efuse-pufmisc */
>> +    {0x058, 0x04}, /* efuse-sec */
>> +    {0x05c, 0x04}, /* efuse-spkid */
>> +    {0x060, 0x30}, /* efuse-aeskey */
>> +    {0x0a0, 0x30}, /* ppk0-hash */
>> +    {0x0d0, 0x30}, /* ppk1-hash */
>> +    {0x100, 0x7f}, /* pufuser */
>> +};
>> +
>> +/**
>> + * struct xilinx_efuse - the basic structure
>> + * @src:    address of the buffer to store the data to be write/read
>> + * @size:    no of words to be read/write
>> + * @offset:    offset to be read/write`
> 
> a little bit different description compare to upsream linux
> 
>> + * @flag:    0 - represents efuse read and 1- represents efuse write
>> + * @pufuserfuse:0 - represents non-puf efuses, offset is used for 
>> read/write
>> + *        1 - represents puf user fuse row number.
>> + *
>> + * this structure stores all the required details to
>> + * read/write efuse memory.
>> + */
>> +struct xilinx_efuse {
>> +    u64 src;
>> +    u32 size;
>> +    u32 offset;
>> +    u32 flag;
>> +    u32 pufuserfuse;
>> +};
>> +
>> +static int zynqmp_efuse_get_length(u32 offset, u32 *base_offset, u32 
>> *len)
>> +{
>> +    struct efuse_map_entry *fuse;
>> +    int i;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(zynqmp_efuse_table); ++i) {
>> +        fuse = &zynqmp_efuse_table[i];
>> +        if (offset >= fuse->offset &&
>> +            offset < fuse->offset + fuse->length) {
>> +            *base_offset = fuse->offset;
>> +            *len = fuse->length;
>> +            return 0;
>> +            }
>> +    }
>> +
>> +    return -ENOENT;
>> +}
>> +
>> +static int zynqmp_efuse_access(struct udevice *dev, unsigned int offset,
>> +                   void *val, size_t bytes, unsigned int flag,
>> +                   unsigned int pufflag)
>> +{
>> +    size_t words = bytes / WORD_INBYTES;
>> +    struct xilinx_efuse *efuse;
>> +    ulong dma_addr, dma_buf;
>> +    int ret, value;
>> +    char *data;
>> +
>> +    if (bytes % WORD_INBYTES != 0) {
>> +        dev_err(dev, "Bytes requested should be word aligned\n");
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    if (pufflag == 0 && offset % WORD_INBYTES) {
>> +        dev_err(dev, "Offset requested should be word aligned\n");
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    if (pufflag == 1 && flag == EFUSE_WRITE) {
>> +        memcpy(&value, val, bytes);
>> +        if ((offset == EFUSE_PUF_START_OFFSET ||
>> +             offset == EFUSE_PUF_MID_OFFSET) &&
>> +            value & P_USER_0_64_UPPER_MASK) {
>> +            dev_err(dev, "Only lower 4 bytes are allowed to be 
>> programmed in P_USER_0 & P_USER_64\n");
>> +            return -EOPNOTSUPP;
>> +        }
>> +
>> +        if (offset == EFUSE_PUF_END_OFFSET &&
>> +            (value & P_USER_127_LOWER_4_BIT_MASK)) {
>> +            dev_err(dev, "Only MSB 28 bits are allowed to be 
>> programmed for P_USER_127\n");
>> +            return -EOPNOTSUPP;
>> +        }
>> +    }
>> +
>> +    efuse = dma_alloc_coherent(sizeof(struct xilinx_efuse), &dma_addr);
>> +    if (!efuse)
>> +        return -ENOMEM;
>> +
>> +    data = dma_alloc_coherent(bytes, &dma_buf);
>> +    if (!data) {
>> +        dma_free_coherent(efuse);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    if (flag == EFUSE_WRITE) {
>> +        memcpy(data, val, bytes);
>> +        efuse->flag = EFUSE_WRITE;
>> +    } else {
>> +        efuse->flag = EFUSE_READ;
>> +    }
>> +
>> +    efuse->src = dma_buf;
>> +    efuse->size = words;
>> +    efuse->offset = offset;
>> +    efuse->pufuserfuse = pufflag;
>> +
>> +    flush_dcache_range((ulong)efuse, (ulong)efuse +
>> +                   roundup(sizeof(struct xilinx_efuse), 
>> ARCH_DMA_MINALIGN));
>> +    flush_dcache_range((ulong)data, (ulong)data +
>> +                   roundup(sizeof(struct xilinx_efuse), 
>> ARCH_DMA_MINALIGN));
>> +
>> +    zynqmp_pm_efuse_access(dma_addr, (u32 *)&ret);
>> +    if (ret != 0) {
>> +        if (ret == EFUSE_NOT_ENABLED) {
>> +            dev_err(dev, "efuse access is not enabled\n");
>> +            ret = -EOPNOTSUPP;
>> +            goto END;
>> +        }
>> +        dev_err(dev, "Error in efuse read %x\n", ret);
>> +        ret = -EPERM;
>> +        goto END;
>> +    }
>> +
>> +    if (flag == EFUSE_READ)
>> +        memcpy(val, data, bytes);
>> +END:
> 
> this has also changed and was commented by Stefan before.
> 
> efuse_access_err:
>      dma_free_coherent(dev, sizeof(bytes),
>                data, dma_buf);
> efuse_data_fail:
>      dma_free_coherent(dev, sizeof(struct xilinx_efuse),
>                efuse, dma_addr);
> 
> 

I see.

BTW: Is 'sizeof(bytes)' correct here? Some efuse access will be greater 
than sizeof(bytes) I guess. That's why I changed the dma-alloc to 
'dma_alloc_coherent(bytes, &dma_buf)' in the u-boot port in contrast to 
the original code.

> 
>> +
>> +    dma_free_coherent(efuse);
>> +    dma_free_coherent(data);
>> +
>> +    return ret;
>> +}
>> +
>> +static int zynqmp_nvmem_read(struct udevice *dev, int offset,
>> +                 void *val, int bytes)
> 
> please check variables types.
> 
>> +{
>> +    int ret, pufflag = 0;
>> +    int idcode, version;
>> +
>> +    if (offset >= EFUSE_PUF_START_OFFSET && offset <= 
>> EFUSE_PUF_END_OFFSET)
>> +        pufflag = 1;
>> +
>> +    dev_dbg(dev, "reading from offset=0x%x, bytes=%d\n", offset, bytes);
>> +
>> +    switch (offset) {
>> +    /* Soc version offset is zero */
>> +    case SOC_VERSION_OFFSET:
>> +        if (bytes != SOC_VER_SIZE)
>> +            return -EOPNOTSUPP;
>> +
>> +        ret = zynqmp_pm_get_chipid((u32 *)&idcode, (u32 *)&version);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        *(int *)val = version & SILICON_REVISION_MASK;
>> +        break;
>> +    /* Efuse offset starts from 0xc */
>> +    case EFUSE_START_OFFSET ... EFUSE_END_OFFSET:
>> +    case EFUSE_PUF_START_OFFSET ... EFUSE_PUF_END_OFFSET:
>> +        ret = zynqmp_efuse_access(dev, offset, val,
>> +                      bytes, EFUSE_READ, pufflag);
>> +        break;
>> +    default:
>> +        *(u32 *)val = 0xDEADBEEF;
>> +        ret = 0;
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static int zynqmp_nvmem_write(struct udevice *dev, int offset, const 
>> void *val,
>> +                  int bytes)
> 
> here too. offset for example
> 
>> +{
>> +    int pufflag = 0;
>> +
>> +    dev_dbg(dev, "writing to offset=0x%x, bytes=%d", offset, bytes);
>> +
>> +    if (offset < EFUSE_START_OFFSET || offset > EFUSE_PUF_END_OFFSET)
>> +        return -EOPNOTSUPP;
>> +
>> +    if (offset >= EFUSE_PUF_START_OFFSET && offset <= 
>> EFUSE_PUF_END_OFFSET)
>> +        pufflag = 1;
>> +
>> +    return zynqmp_efuse_access(dev, offset,
>> +                   (void *)val, bytes, EFUSE_WRITE, pufflag);
>> +}
>> +
>> +int fuse_read(u32 bank, u32 word, u32 *val)
>> +{
>> +    u32 base_offset, offset, len;
>> +    struct udevice *dev;
>> +    u8 buf[128];
> 
> 128 is correct number but would be better to have connection where this 
> number is coming from. MAX size is 0x7f which is 127 but that connection 
> is not visible from it. I am fine with the comment to say it.  Please 
> look below.
> 
>> +    int ret;
>> +
>> +    memset(buf, 0, sizeof(buf));
> 
> I would call it before misc_read not to waste time in error case.
> 
>> +
>> +    ret = uclass_get_device_by_driver(UCLASS_MISC,
>> +                      DM_DRIVER_GET(zynqmp_efuse), &dev);
>> +    if (ret)
>> +        return log_msg_ret("uclass_drv", ret);
> 
> Above c&p from Linux is using dev_dbg structure and here you are using 
> log infrastructure. Would be much better if you can use the same.

Can we use dev_dbg here? Since the return indicates that 'dev' is 
invalid here. My idea was to use the dev_dbg for the Linux port and 
log-infrastructure for u-boot related functions.

> 
> 
>> +
>> +    ret = zynqmp_efuse_get_length(word, &base_offset, &len);
>> +    if (ret)
>> +        return log_msg_ret("efuse_offset", ret);
>> +
> 
> And here please check that len is below 128 to make sure that we never 
> overwrite stack. Other entries can be added and it should be spotted.
> 
>> +    ret = misc_read(dev, base_offset, (void *)buf, len);
>> +    if (ret)
>> +        return log_msg_ret("misc_read", ret);
>> +
>> +    offset = word - base_offset;
>> +    *val = ((u32 *)buf)[offset];
>> +
>> +    return 0;
>> +}
>> +
>> +int fuse_prog(u32 bank, u32 word, u32 val)
>> +{
>> +    u32 base_offset, len;
>> +    struct udevice *dev;
>> +    int ret;
>> +
>> +    ret = zynqmp_efuse_get_length(word, &base_offset, &len);
>> +    if (ret)
>> +        return log_msg_ret("efuse_offset", ret);
>> +
>> +    ret = uclass_get_device_by_driver(UCLASS_MISC,
>> +                      DM_DRIVER_GET(zynqmp_efuse), &dev);
>> +    if (ret)
>> +        return log_msg_ret("uclass_drv", ret);
>> +
>> +    if (word != base_offset || len != sizeof(val))
>> +        return -EINVAL;
>> +
>> +    ret = misc_write(dev, word, (void *)&val, sizeof(val));
>> +    if (ret)
>> +        return log_msg_ret("misc_write", ret);
>> +
>> +    return 0;
>> +}
>> +
>> +int fuse_sense(u32 bank, u32 word, u32 *val)
>> +{
>> +    /* We do not support sense */
> 
> avoid we - imperative mood please.
> 
>> +    return -ENOSYS;
>> +}
>> +
>> +int fuse_override(u32 bank, u32 word, u32 val)
>> +{
>> +    /* We do not support overriding */
> 
> avoid we
> 
>> +    return -ENOSYS;
>> +}
>> +
>> +static const struct udevice_id zynqmp_efuse_match[] = {
>> +    { .compatible = "xlnx,zynqmp-nvmem-fw", },
>> +    { /* sentinel */ },
>> +};
>> +
>> +static const struct misc_ops zynqmp_efuse_ops = {
>> +    .read = zynqmp_nvmem_read,
>> +    .write = zynqmp_nvmem_write,
>> +};
>> +
>> +U_BOOT_DRIVER(zynqmp_efuse) = {
>> +    .name = "zynqmp_efuse",
>> +    .id = UCLASS_MISC,
>> +    .of_match = zynqmp_efuse_match,
>> +    .ops = &zynqmp_efuse_ops,
>> +};
> 
> Thanks,
> Michal
diff mbox series

Patch

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 6009d55f400..c07f50c9a76 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -298,6 +298,14 @@  config FSL_SEC_MON
 	  Security Monitor can be transitioned on any security failures,
 	  like software violations or hardware security violations.
 
+config ZYNQMP_EFUSE
+	bool "Enable ZynqMP eFUSE Driver"
+	depends on ZYNQMP_FIRMWARE
+	help
+	  Enable access to Zynq UltraScale (ZynqMP) eFUSEs thought PMU firmware
+	  interface. ZnyqMP has 256 eFUSEs where some of them are security related
+	  and cannot be read back (i.e. AES key).
+
 choice
 	prompt "Security monitor interaction endianess"
 	depends on FSL_SEC_MON
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index e53d52c47b3..68ba5648eab 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -92,3 +92,4 @@  obj-$(CONFIG_ESM_K3) += k3_esm.o
 obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
 obj-$(CONFIG_SL28CPLD) += sl28cpld.o
 obj-$(CONFIG_SPL_SOCFPGA_DT_REG) += socfpga_dtreg.o
+obj-$(CONFIG_ZYNQMP_EFUSE) += zynqmp_efuse.o
diff --git a/drivers/misc/zynqmp_efuse.c b/drivers/misc/zynqmp_efuse.c
new file mode 100644
index 00000000000..98a39c1ebdd
--- /dev/null
+++ b/drivers/misc/zynqmp_efuse.c
@@ -0,0 +1,324 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2014 - 2015 Xilinx, Inc.
+ * Michal Simek <michal.simek@amd.com>
+ *
+ * (C) Copyright 2024 Weidmueller Interface GmbH
+ * Lukas Funke <lukas.funke@weidmueller.com>
+ */
+
+#include <compiler.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <zynqmp_firmware.h>
+#include <asm/dma-mapping.h>
+#include <dm.h>
+#include <dm/device_compat.h>
+#include <misc.h>
+
+#define SILICON_REVISION_MASK 0xF
+#define P_USER_0_64_UPPER_MASK	0x5FFF0000
+#define P_USER_127_LOWER_4_BIT_MASK 0xF
+#define WORD_INBYTES		(4)
+#define SOC_VER_SIZE		(0x4)
+#define SOC_VERSION_OFFSET	(0x0)
+#define EFUSE_START_OFFSET	(0xC)
+#define EFUSE_END_OFFSET	(0xFC)
+#define EFUSE_PUF_START_OFFSET	(0x100)
+#define EFUSE_PUF_MID_OFFSET	(0x140)
+#define EFUSE_PUF_END_OFFSET	(0x17F)
+#define EFUSE_NOT_ENABLED	(29)
+#define EFUSE_READ		(0)
+#define EFUSE_WRITE		(1)
+
+/**
+ * struct efuse_map_entry - offset and length of zynqmp fuses
+ * @offset:	offset of efuse to be read/write
+ * @length:	length of efuse
+ */
+struct efuse_map_entry {
+	u32 offset;
+	u32 length;
+};
+
+struct efuse_map_entry zynqmp_efuse_table[] = {
+	{0x000, 0x04}, /* soc revision */
+	{0x00c, 0x0c}, /* SoC DNA */
+	{0x020, 0x04}, /* efuse-usr0 */
+	{0x024, 0x04}, /* efuse-usr1 */
+	{0x028, 0x04}, /* efuse-usr2 */
+	{0x02c, 0x04}, /* efuse-usr3 */
+	{0x030, 0x04}, /* efuse-usr4 */
+	{0x034, 0x04}, /* efuse-usr5 */
+	{0x038, 0x04}, /* efuse-usr6 */
+	{0x03c, 0x04}, /* efuse-usr7 */
+	{0x040, 0x04}, /* efuse-miscusr */
+	{0x050, 0x04}, /* efuse-chash */
+	{0x054, 0x04}, /* efuse-pufmisc */
+	{0x058, 0x04}, /* efuse-sec */
+	{0x05c, 0x04}, /* efuse-spkid */
+	{0x060, 0x30}, /* efuse-aeskey */
+	{0x0a0, 0x30}, /* ppk0-hash */
+	{0x0d0, 0x30}, /* ppk1-hash */
+	{0x100, 0x7f}, /* pufuser */
+};
+
+/**
+ * struct xilinx_efuse - the basic structure
+ * @src:	address of the buffer to store the data to be write/read
+ * @size:	no of words to be read/write
+ * @offset:	offset to be read/write`
+ * @flag:	0 - represents efuse read and 1- represents efuse write
+ * @pufuserfuse:0 - represents non-puf efuses, offset is used for read/write
+ *		1 - represents puf user fuse row number.
+ *
+ * this structure stores all the required details to
+ * read/write efuse memory.
+ */
+struct xilinx_efuse {
+	u64 src;
+	u32 size;
+	u32 offset;
+	u32 flag;
+	u32 pufuserfuse;
+};
+
+static int zynqmp_efuse_get_length(u32 offset, u32 *base_offset, u32 *len)
+{
+	struct efuse_map_entry *fuse;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(zynqmp_efuse_table); ++i) {
+		fuse = &zynqmp_efuse_table[i];
+		if (offset >= fuse->offset &&
+		    offset < fuse->offset + fuse->length) {
+			*base_offset = fuse->offset;
+			*len = fuse->length;
+			return 0;
+			}
+	}
+
+	return -ENOENT;
+}
+
+static int zynqmp_efuse_access(struct udevice *dev, unsigned int offset,
+			       void *val, size_t bytes, unsigned int flag,
+			       unsigned int pufflag)
+{
+	size_t words = bytes / WORD_INBYTES;
+	struct xilinx_efuse *efuse;
+	ulong dma_addr, dma_buf;
+	int ret, value;
+	char *data;
+
+	if (bytes % WORD_INBYTES != 0) {
+		dev_err(dev, "Bytes requested should be word aligned\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (pufflag == 0 && offset % WORD_INBYTES) {
+		dev_err(dev, "Offset requested should be word aligned\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (pufflag == 1 && flag == EFUSE_WRITE) {
+		memcpy(&value, val, bytes);
+		if ((offset == EFUSE_PUF_START_OFFSET ||
+		     offset == EFUSE_PUF_MID_OFFSET) &&
+		    value & P_USER_0_64_UPPER_MASK) {
+			dev_err(dev, "Only lower 4 bytes are allowed to be programmed in P_USER_0 & P_USER_64\n");
+			return -EOPNOTSUPP;
+		}
+
+		if (offset == EFUSE_PUF_END_OFFSET &&
+		    (value & P_USER_127_LOWER_4_BIT_MASK)) {
+			dev_err(dev, "Only MSB 28 bits are allowed to be programmed for P_USER_127\n");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	efuse = dma_alloc_coherent(sizeof(struct xilinx_efuse), &dma_addr);
+	if (!efuse)
+		return -ENOMEM;
+
+	data = dma_alloc_coherent(bytes, &dma_buf);
+	if (!data) {
+		dma_free_coherent(efuse);
+		return -ENOMEM;
+	}
+
+	if (flag == EFUSE_WRITE) {
+		memcpy(data, val, bytes);
+		efuse->flag = EFUSE_WRITE;
+	} else {
+		efuse->flag = EFUSE_READ;
+	}
+
+	efuse->src = dma_buf;
+	efuse->size = words;
+	efuse->offset = offset;
+	efuse->pufuserfuse = pufflag;
+
+	flush_dcache_range((ulong)efuse, (ulong)efuse +
+				   roundup(sizeof(struct xilinx_efuse), ARCH_DMA_MINALIGN));
+	flush_dcache_range((ulong)data, (ulong)data +
+				   roundup(sizeof(struct xilinx_efuse), ARCH_DMA_MINALIGN));
+
+	zynqmp_pm_efuse_access(dma_addr, (u32 *)&ret);
+	if (ret != 0) {
+		if (ret == EFUSE_NOT_ENABLED) {
+			dev_err(dev, "efuse access is not enabled\n");
+			ret = -EOPNOTSUPP;
+			goto END;
+		}
+		dev_err(dev, "Error in efuse read %x\n", ret);
+		ret = -EPERM;
+		goto END;
+	}
+
+	if (flag == EFUSE_READ)
+		memcpy(val, data, bytes);
+END:
+
+	dma_free_coherent(efuse);
+	dma_free_coherent(data);
+
+	return ret;
+}
+
+static int zynqmp_nvmem_read(struct udevice *dev, int offset,
+			     void *val, int bytes)
+{
+	int ret, pufflag = 0;
+	int idcode, version;
+
+	if (offset >= EFUSE_PUF_START_OFFSET && offset <= EFUSE_PUF_END_OFFSET)
+		pufflag = 1;
+
+	dev_dbg(dev, "reading from offset=0x%x, bytes=%d\n", offset, bytes);
+
+	switch (offset) {
+	/* Soc version offset is zero */
+	case SOC_VERSION_OFFSET:
+		if (bytes != SOC_VER_SIZE)
+			return -EOPNOTSUPP;
+
+		ret = zynqmp_pm_get_chipid((u32 *)&idcode, (u32 *)&version);
+		if (ret < 0)
+			return ret;
+
+		*(int *)val = version & SILICON_REVISION_MASK;
+		break;
+	/* Efuse offset starts from 0xc */
+	case EFUSE_START_OFFSET ... EFUSE_END_OFFSET:
+	case EFUSE_PUF_START_OFFSET ... EFUSE_PUF_END_OFFSET:
+		ret = zynqmp_efuse_access(dev, offset, val,
+					  bytes, EFUSE_READ, pufflag);
+		break;
+	default:
+		*(u32 *)val = 0xDEADBEEF;
+		ret = 0;
+		break;
+	}
+
+	return ret;
+}
+
+static int zynqmp_nvmem_write(struct udevice *dev, int offset, const void *val,
+			      int bytes)
+{
+	int pufflag = 0;
+
+	dev_dbg(dev, "writing to offset=0x%x, bytes=%d", offset, bytes);
+
+	if (offset < EFUSE_START_OFFSET || offset > EFUSE_PUF_END_OFFSET)
+		return -EOPNOTSUPP;
+
+	if (offset >= EFUSE_PUF_START_OFFSET && offset <= EFUSE_PUF_END_OFFSET)
+		pufflag = 1;
+
+	return zynqmp_efuse_access(dev, offset,
+				   (void *)val, bytes, EFUSE_WRITE, pufflag);
+}
+
+int fuse_read(u32 bank, u32 word, u32 *val)
+{
+	u32 base_offset, offset, len;
+	struct udevice *dev;
+	u8 buf[128];
+	int ret;
+
+	memset(buf, 0, sizeof(buf));
+
+	ret = uclass_get_device_by_driver(UCLASS_MISC,
+					  DM_DRIVER_GET(zynqmp_efuse), &dev);
+	if (ret)
+		return log_msg_ret("uclass_drv", ret);
+
+	ret = zynqmp_efuse_get_length(word, &base_offset, &len);
+	if (ret)
+		return log_msg_ret("efuse_offset", ret);
+
+	ret = misc_read(dev, base_offset, (void *)buf, len);
+	if (ret)
+		return log_msg_ret("misc_read", ret);
+
+	offset = word - base_offset;
+	*val = ((u32 *)buf)[offset];
+
+	return 0;
+}
+
+int fuse_prog(u32 bank, u32 word, u32 val)
+{
+	u32 base_offset, len;
+	struct udevice *dev;
+	int ret;
+
+	ret = zynqmp_efuse_get_length(word, &base_offset, &len);
+	if (ret)
+		return log_msg_ret("efuse_offset", ret);
+
+	ret = uclass_get_device_by_driver(UCLASS_MISC,
+					  DM_DRIVER_GET(zynqmp_efuse), &dev);
+	if (ret)
+		return log_msg_ret("uclass_drv", ret);
+
+	if (word != base_offset || len != sizeof(val))
+		return -EINVAL;
+
+	ret = misc_write(dev, word, (void *)&val, sizeof(val));
+	if (ret)
+		return log_msg_ret("misc_write", ret);
+
+	return 0;
+}
+
+int fuse_sense(u32 bank, u32 word, u32 *val)
+{
+	/* We do not support sense */
+	return -ENOSYS;
+}
+
+int fuse_override(u32 bank, u32 word, u32 val)
+{
+	/* We do not support overriding */
+	return -ENOSYS;
+}
+
+static const struct udevice_id zynqmp_efuse_match[] = {
+	{ .compatible = "xlnx,zynqmp-nvmem-fw", },
+	{ /* sentinel */ },
+};
+
+static const struct misc_ops zynqmp_efuse_ops = {
+	.read = zynqmp_nvmem_read,
+	.write = zynqmp_nvmem_write,
+};
+
+U_BOOT_DRIVER(zynqmp_efuse) = {
+	.name = "zynqmp_efuse",
+	.id = UCLASS_MISC,
+	.of_match = zynqmp_efuse_match,
+	.ops = &zynqmp_efuse_ops,
+};