* [PATCH 1/3] firmware: zynqmp: Add support to access efuses
2024-05-14 14:04 [PATCH 0/3] Add eFuse access for ZynqMP lukas.funke-oss
@ 2024-05-14 14:04 ` lukas.funke-oss
2024-05-15 6:04 ` Stefan Roese
2024-05-14 14:04 ` [PATCH 2/3] amd64: zynqmp: Add command to program efuses lukas.funke-oss
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: lukas.funke-oss @ 2024-05-14 14:04 UTC (permalink / raw)
To: u-boot
Cc: Michal Simek, Lukas Funke, Algapally Santosh Sagar,
Ashok Reddy Soma, Ilias Apalodimas, Neil Armstrong, Qu Wenruo,
Stefan Herbrechtsmeier, Stefan Roese, Tanmay Shah, Tom Rini,
Venkatesh Yadav Abbarapu
From: Lukas Funke <lukas.funke@weidmueller.com>
Add functions to access efuses through PMU firmware
interface.
Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---
drivers/firmware/firmware-zynqmp.c | 31 ++++++++++++++++++++++++++++++
include/zynqmp_firmware.h | 2 ++
2 files changed, 33 insertions(+)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index f99507d86c6..7483f2a8709 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -210,6 +210,37 @@ int zynqmp_pm_feature(const u32 api_id)
return ret_payload[1] & FIRMWARE_VERSION_MASK;
}
+int zynqmp_pm_get_chipid(u32 *idcode, u32 *version)
+{
+ int ret;
+ u32 ret_payload[PAYLOAD_ARG_CNT];
+
+ if (!idcode || !version)
+ return -EINVAL;
+
+ ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0, ret_payload);
+ *idcode = ret_payload[1];
+ *version = ret_payload[2];
+
+ return ret;
+}
+
+int zynqmp_pm_efuse_access(const u64 address, u32 *out)
+{
+ int ret;
+ u32 ret_payload[PAYLOAD_ARG_CNT];
+
+ if (!out)
+ return -EINVAL;
+
+ ret = xilinx_pm_request(PM_EFUSE_ACCESS, upper_32_bits(address),
+ lower_32_bits(address), 0, 0, ret_payload);
+
+ *out = ret_payload[1];
+
+ return ret;
+}
+
int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id)
{
int ret;
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
index 73198a6a6ea..7f18b4d59bf 100644
--- a/include/zynqmp_firmware.h
+++ b/include/zynqmp_firmware.h
@@ -453,6 +453,8 @@ int xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
u32 value);
+int zynqmp_pm_get_chipid(u32 *idcode, u32 *version);
+int zynqmp_pm_efuse_access(const u64 address, u32 *out);
int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id);
int zynqmp_mmio_read(const u32 address, u32 *value);
int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value);
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] firmware: zynqmp: Add support to access efuses
2024-05-14 14:04 ` [PATCH 1/3] firmware: zynqmp: Add support to access efuses lukas.funke-oss
@ 2024-05-15 6:04 ` Stefan Roese
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2024-05-15 6:04 UTC (permalink / raw)
To: lukas.funke-oss, u-boot
Cc: Michal Simek, Lukas Funke, Algapally Santosh Sagar,
Ashok Reddy Soma, Ilias Apalodimas, Neil Armstrong, Qu Wenruo,
Stefan Herbrechtsmeier, Tanmay Shah, Tom Rini,
Venkatesh Yadav Abbarapu
Hi Lukas,
On 5/14/24 16:04, lukas.funke-oss@weidmueller.com wrote:
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
> Add functions to access efuses through PMU firmware
> interface.
>
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> ---
>
> drivers/firmware/firmware-zynqmp.c | 31 ++++++++++++++++++++++++++++++
> include/zynqmp_firmware.h | 2 ++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index f99507d86c6..7483f2a8709 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -210,6 +210,37 @@ int zynqmp_pm_feature(const u32 api_id)
> return ret_payload[1] & FIRMWARE_VERSION_MASK;
> }
>
> +int zynqmp_pm_get_chipid(u32 *idcode, u32 *version)
> +{
> + int ret;
> + u32 ret_payload[PAYLOAD_ARG_CNT];
Reverse x-mas tree ordering looks better IMHO.
> +
> + if (!idcode || !version)
> + return -EINVAL;
> +
> + ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0, ret_payload);
> + *idcode = ret_payload[1];
> + *version = ret_payload[2];
You don't check ret for an error above but still pass the return values
here. Perhaps it makes sense to return with error above instead?
> +
> + return ret;
> +}
> +
> +int zynqmp_pm_efuse_access(const u64 address, u32 *out)
> +{
> + int ret;
> + u32 ret_payload[PAYLOAD_ARG_CNT];
> +
> + if (!out)
> + return -EINVAL;
> +
> + ret = xilinx_pm_request(PM_EFUSE_ACCESS, upper_32_bits(address),
> + lower_32_bits(address), 0, 0, ret_payload);
Same here.
Thanks,
Stefan
> +
> + *out = ret_payload[1];
> +
> + return ret;
> +}
> +
> int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id)
> {
> int ret;
> diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
> index 73198a6a6ea..7f18b4d59bf 100644
> --- a/include/zynqmp_firmware.h
> +++ b/include/zynqmp_firmware.h
> @@ -453,6 +453,8 @@ int xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
> int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
> u32 value);
> +int zynqmp_pm_get_chipid(u32 *idcode, u32 *version);
> +int zynqmp_pm_efuse_access(const u64 address, u32 *out);
> int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id);
> int zynqmp_mmio_read(const u32 address, u32 *value);
> int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value);
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] amd64: zynqmp: Add command to program efuses
2024-05-14 14:04 [PATCH 0/3] Add eFuse access for ZynqMP lukas.funke-oss
2024-05-14 14:04 ` [PATCH 1/3] firmware: zynqmp: Add support to access efuses lukas.funke-oss
@ 2024-05-14 14:04 ` lukas.funke-oss
2024-05-14 14:04 ` [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses lukas.funke-oss
2024-05-15 7:08 ` [PATCH 0/3] Add eFuse access for ZynqMP Marek Behún
3 siblings, 0 replies; 9+ messages in thread
From: lukas.funke-oss @ 2024-05-14 14:04 UTC (permalink / raw)
To: u-boot
Cc: Michal Simek, Lukas Funke, Christian Taedcke, Ilias Apalodimas,
Simon Glass, Tom Rini, Venkatesh Yadav Abbarapu
From: Lukas Funke <lukas.funke@weidmueller.com>
Add subcommands to read/write eFuses using u-boot. The subcommands
through the 'zynqmp' command.
Example:
=> zynqmp efuse_read 0xc 0xc
00000000: 85 36 b1 3c 34 f2 3b 01 00 00 00 40 .f.<D.:....@
Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---
board/xilinx/zynqmp/cmds.c | 101 +++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
diff --git a/board/xilinx/zynqmp/cmds.c b/board/xilinx/zynqmp/cmds.c
index bf39c5472ea..e8d70ffecd6 100644
--- a/board/xilinx/zynqmp/cmds.c
+++ b/board/xilinx/zynqmp/cmds.c
@@ -16,6 +16,9 @@
#include <asm/arch/sys_proto.h>
#include <asm/io.h>
#include <mach/zynqmp_aes.h>
+#include <misc.h>
+#include <hexdump.h>
+#include <dm.h>
static int do_zynqmp_verify_secure(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
@@ -340,6 +343,99 @@ static int do_zynqmp_sha3(struct cmd_tbl *cmdtp, int flag,
return CMD_RET_SUCCESS;
}
+static int do_zynqmp_efuse_read(struct cmd_tbl *cmdtp, int flag,
+ int argc, char * const argv[])
+{
+ struct udevice *dev;
+ u32 offset, len;
+ u8 buf[32];
+ int ret;
+
+ if (!CONFIG_IS_ENABLED(ZYNQMP_EFUSE)) {
+ printf("Failed: not supported\n");
+ return CMD_RET_FAILURE;
+ }
+
+ if (argc > cmdtp->maxargs || argc < (cmdtp->maxargs - 1))
+ return CMD_RET_USAGE;
+
+ memset(buf, 0, sizeof(buf));
+
+ offset = hextoul(argv[2], NULL);
+ len = hextoul(argv[3], NULL);
+
+ if (len > sizeof(buf)) {
+ printf("Failed: length exceeds buffer size");
+ return CMD_RET_FAILURE;
+ }
+
+ ret = uclass_get_device_by_driver(UCLASS_MISC,
+ DM_DRIVER_GET(zynqmp_efuse), &dev);
+ if (ret) {
+ printf("Failed to initialize zynqmp_efuse: %d\n", ret);
+ return CMD_RET_FAILURE;
+ }
+
+ ret = misc_read(dev, offset, (void *)buf, len);
+ if (ret) {
+ printf("Failed: cannot read efuse at 0x%x, errocode %d\n",
+ offset, ret);
+ return CMD_RET_FAILURE;
+ }
+
+ if (CONFIG_IS_ENABLED(HEXDUMP))
+ print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
+
+ return CMD_RET_SUCCESS;
+}
+
+static int do_zynqmp_efuse_write(struct cmd_tbl *cmdtp, int flag,
+ int argc, char * const argv[])
+{
+ struct udevice *dev;
+ u64 value;
+ u32 offset, len;
+ u8 buf[sizeof(u64)];
+ int ret;
+
+ if (!CONFIG_IS_ENABLED(ZYNQMP_EFUSE)) {
+ printf("Failed: not supported\n");
+ return CMD_RET_FAILURE;
+ }
+
+ if (argc > cmdtp->maxargs || argc < (cmdtp->maxargs - 1))
+ return CMD_RET_USAGE;
+
+ memset(buf, 0, sizeof(buf));
+
+ offset = hextoul(argv[2], NULL);
+ len = hextoul(argv[3], NULL);
+
+ if (len <= sizeof(u64)) {
+ value = hextoul(argv[4], NULL);
+ memcpy(buf, &value, sizeof(value));
+ } else {
+ printf("Cannot write more than %zu byte to efuse\n", sizeof(u64));
+ return CMD_RET_FAILURE;
+ }
+
+ ret = uclass_get_device_by_driver(UCLASS_MISC,
+ DM_DRIVER_GET(zynqmp_efuse), &dev);
+ if (ret) {
+ printf("Failed to initialize zynqmp_efuse: %d\n", ret);
+ return CMD_RET_FAILURE;
+ }
+
+ ret = misc_write(dev, offset, (void *)buf, sizeof(buf));
+ if (ret) {
+ printf("Failed: cannot read efuse at 0x%x, errocode 0x%x\n",
+ offset, ret);
+ return CMD_RET_FAILURE;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
static struct cmd_tbl cmd_zynqmp_sub[] = {
U_BOOT_CMD_MKENT(secure, 5, 0, do_zynqmp_verify_secure, "", ""),
U_BOOT_CMD_MKENT(pmufw, 4, 0, do_zynqmp_pmufw, "", ""),
@@ -348,6 +444,8 @@ static struct cmd_tbl cmd_zynqmp_sub[] = {
U_BOOT_CMD_MKENT(aes, 9, 0, do_zynqmp_aes, "", ""),
U_BOOT_CMD_MKENT(rsa, 7, 0, do_zynqmp_rsa, "", ""),
U_BOOT_CMD_MKENT(sha3, 5, 0, do_zynqmp_sha3, "", ""),
+ U_BOOT_CMD_MKENT(efuse_read, 4, 0, do_zynqmp_efuse_read, "", ""),
+ U_BOOT_CMD_MKENT(efuse_write, 5, 0, do_zynqmp_efuse_write, "", ""),
#ifdef CONFIG_DEFINE_TCM_OCM_MMAP
U_BOOT_CMD_MKENT(tcminit, 3, 0, do_zynqmp_tcm_init, "", ""),
#endif
@@ -422,6 +520,9 @@ U_BOOT_LONGHELP(zynqmp,
" 48 bytes hash value into srcaddr\n"
" Optional key_addr can be specified for saving sha3 hash value\n"
" Note: srcaddr/srclen should not be 0\n"
+ "zynqmp efuse_read offset len - read efuse at given offset\n"
+ "zynqmp efuse_write offset len value - write value of length <len>\n"
+ " to efuse at given offset\n"
);
U_BOOT_CMD(
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses
2024-05-14 14:04 [PATCH 0/3] Add eFuse access for ZynqMP lukas.funke-oss
2024-05-14 14:04 ` [PATCH 1/3] firmware: zynqmp: Add support to access efuses lukas.funke-oss
2024-05-14 14:04 ` [PATCH 2/3] amd64: zynqmp: Add command to program efuses lukas.funke-oss
@ 2024-05-14 14:04 ` lukas.funke-oss
2024-05-15 6:12 ` Stefan Roese
2024-05-15 7:08 ` [PATCH 0/3] Add eFuse access for ZynqMP Marek Behún
3 siblings, 1 reply; 9+ messages in thread
From: lukas.funke-oss @ 2024-05-14 14:04 UTC (permalink / raw)
To: u-boot
Cc: Michal Simek, Lukas Funke, Caleb Connolly, Heinrich Schuchardt,
Ilias Apalodimas, Jonas Karlman, Kever Yang, Marek Behún,
Peng Fan, Simon Glass, Stefan Roese, Tom Rini, Wan Yee Lau
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>
---
drivers/misc/Kconfig | 8 ++
drivers/misc/Makefile | 1 +
drivers/misc/zynqmp_efuse.c | 213 ++++++++++++++++++++++++++++++++++++
3 files changed, 222 insertions(+)
create mode 100644 drivers/misc/zynqmp_efuse.c
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..0cfc42a4f39
--- /dev/null
+++ b/drivers/misc/zynqmp_efuse.c
@@ -0,0 +1,213 @@
+// 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 EFUSE_MEMORY_SIZE (0x177)
+#define UNUSED_SPACE (0x8)
+#define ZYNQMP_NVMEM_SIZE (SOC_VER_SIZE + UNUSED_SPACE + \
+ EFUSE_MEMORY_SIZE)
+#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 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_access(struct udevice *dev, unsigned int offset,
+ void *val, size_t bytes, unsigned int flag,
+ unsigned int pufflag)
+{
+ size_t words = bytes / WORD_INBYTES;
+ ulong dma_addr, dma_buf;
+ struct xilinx_efuse *efuse;
+ char *data;
+ int ret, value;
+
+ 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);
+}
+
+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,
+};
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses
2024-05-14 14:04 ` [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses lukas.funke-oss
@ 2024-05-15 6:12 ` Stefan Roese
2024-05-15 6:33 ` Lukas Funke
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Roese @ 2024-05-15 6:12 UTC (permalink / raw)
To: lukas.funke-oss, u-boot
Cc: Michal Simek, Lukas Funke, Caleb Connolly, Heinrich Schuchardt,
Ilias Apalodimas, Jonas Karlman, Kever Yang, Marek Behún,
Peng Fan, Simon Glass, Tom Rini, Wan Yee Lau
Hi Lukas,
On 5/14/24 16:04, 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>
> ---
>
> drivers/misc/Kconfig | 8 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/zynqmp_efuse.c | 213 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 222 insertions(+)
> create mode 100644 drivers/misc/zynqmp_efuse.c
>
> 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..0cfc42a4f39
> --- /dev/null
> +++ b/drivers/misc/zynqmp_efuse.c
> @@ -0,0 +1,213 @@
> +// 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 EFUSE_MEMORY_SIZE (0x177)
> +#define UNUSED_SPACE (0x8)
> +#define ZYNQMP_NVMEM_SIZE (SOC_VER_SIZE + UNUSED_SPACE + \
> + EFUSE_MEMORY_SIZE)
> +#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 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_access(struct udevice *dev, unsigned int offset,
> + void *val, size_t bytes, unsigned int flag,
> + unsigned int pufflag)
> +{
> + size_t words = bytes / WORD_INBYTES;
> + ulong dma_addr, dma_buf;
> + struct xilinx_efuse *efuse;
> + char *data;
> + int ret, value;
> +
> + 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));
efuse and data are allocated via dma_alloc_coherent(). It should not be
necessary to use flush the cache here IIUTC.
> +
> + 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:
Nitpicking: Upper case label ist pretty uncommon AFAIK.
> +
> + 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);
> +}
> +
> +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,
> +};
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses
2024-05-15 6:12 ` Stefan Roese
@ 2024-05-15 6:33 ` Lukas Funke
2024-05-15 9:19 ` Stefan Roese
0 siblings, 1 reply; 9+ messages in thread
From: Lukas Funke @ 2024-05-15 6:33 UTC (permalink / raw)
To: Stefan Roese, u-boot
Cc: Michal Simek, Lukas Funke, Caleb Connolly, Heinrich Schuchardt,
Ilias Apalodimas, Jonas Karlman, Kever Yang, Marek Behún,
Peng Fan, Simon Glass, Tom Rini, Wan Yee Lau
Hi Stefan,
On 15.05.2024 08:12, Stefan Roese wrote:
> Hi Lukas,
>
> On 5/14/24 16:04, 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>
>> ---
>>
>> drivers/misc/Kconfig | 8 ++
>> drivers/misc/Makefile | 1 +
>> drivers/misc/zynqmp_efuse.c | 213 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 222 insertions(+)
>> create mode 100644 drivers/misc/zynqmp_efuse.c
>>
>> 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..0cfc42a4f39
>> --- /dev/null
>> +++ b/drivers/misc/zynqmp_efuse.c
>> @@ -0,0 +1,213 @@
>> +// 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 EFUSE_MEMORY_SIZE (0x177)
>> +#define UNUSED_SPACE (0x8)
>> +#define ZYNQMP_NVMEM_SIZE (SOC_VER_SIZE + UNUSED_SPACE + \
>> + EFUSE_MEMORY_SIZE)
>> +#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 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_access(struct udevice *dev, unsigned int offset,
>> + void *val, size_t bytes, unsigned int flag,
>> + unsigned int pufflag)
>> +{
>> + size_t words = bytes / WORD_INBYTES;
>> + ulong dma_addr, dma_buf;
>> + struct xilinx_efuse *efuse;
>> + char *data;
>> + int ret, value;
>> +
>> + 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));
>
> efuse and data are allocated via dma_alloc_coherent(). It should not be
> necessary to use flush the cache here IIUTC.
If I understand correctly dma_alloc_coherent() maps to an aligned
malloc() which in turn just returns some physical memory without any
caching attributes (is this correct?). We have to ensure that the data
written here is *not* cached but written back to memory because the PMU
is running on a co-processor and data is exchanged via DRAM.
Also: this is the way it was implemented in the other PMU calls as well.
>
>> +
>> + 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:
>
> Nitpicking: Upper case label ist pretty uncommon AFAIK.
Since this is a port of the actual Linux driver I wanted to change as
little as possible. If this is absolutly not acceptable I'm open to
change this.
BTW: thanks for your review!
>
>> +
>> + 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);
>> +}
>> +
>> +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,
>> +};
>
> Viele Grüße,
> Stefan Roese
>
Best regards
- Lukas
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses
2024-05-15 6:33 ` Lukas Funke
@ 2024-05-15 9:19 ` Stefan Roese
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2024-05-15 9:19 UTC (permalink / raw)
To: Lukas Funke, u-boot
Cc: Michal Simek, Lukas Funke, Caleb Connolly, Heinrich Schuchardt,
Ilias Apalodimas, Jonas Karlman, Kever Yang, Marek Behún,
Peng Fan, Simon Glass, Tom Rini, Wan Yee Lau
Hi Lukas,
On 5/15/24 08:33, Lukas Funke wrote:
> Hi Stefan,
>
> On 15.05.2024 08:12, Stefan Roese wrote:
>> Hi Lukas,
>>
>> On 5/14/24 16:04, 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>
>>> ---
>>>
>>> drivers/misc/Kconfig | 8 ++
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/zynqmp_efuse.c | 213 ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 222 insertions(+)
>>> create mode 100644 drivers/misc/zynqmp_efuse.c
>>>
>>> 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..0cfc42a4f39
>>> --- /dev/null
>>> +++ b/drivers/misc/zynqmp_efuse.c
>>> @@ -0,0 +1,213 @@
>>> +// 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 EFUSE_MEMORY_SIZE (0x177)
>>> +#define UNUSED_SPACE (0x8)
>>> +#define ZYNQMP_NVMEM_SIZE (SOC_VER_SIZE + UNUSED_SPACE + \
>>> + EFUSE_MEMORY_SIZE)
>>> +#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 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_access(struct udevice *dev, unsigned int
>>> offset,
>>> + void *val, size_t bytes, unsigned int flag,
>>> + unsigned int pufflag)
>>> +{
>>> + size_t words = bytes / WORD_INBYTES;
>>> + ulong dma_addr, dma_buf;
>>> + struct xilinx_efuse *efuse;
>>> + char *data;
>>> + int ret, value;
>>> +
>>> + 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));
>>
>> efuse and data are allocated via dma_alloc_coherent(). It should not be
>> necessary to use flush the cache here IIUTC.
>
> If I understand correctly dma_alloc_coherent() maps to an aligned
> malloc() which in turn just returns some physical memory without any
> caching attributes (is this correct?). We have to ensure that the data
> written here is *not* cached but written back to memory because the PMU
> is running on a co-processor and data is exchanged via DRAM.
Frankly, I did not look into the U-Boot implementation of
dma_alloc_coherent() - I've rarely seen it here before. But the
original implementation in Linux guarantees that "DMA safe" (uncached)
memory is allocated AFAIU.
> Also: this is the way it was implemented in the other PMU calls as well.
Agreed. I also would copy such stuff from already existing code. Even
though this could be wrong from the beginning.
I just stumbled over this and wondered, if this really is needed
this way.
Thanks,
Stefan
>>
>>> +
>>> + 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:
>>
>> Nitpicking: Upper case label ist pretty uncommon AFAIK.
>
> Since this is a port of the actual Linux driver I wanted to change as
> little as possible. If this is absolutly not acceptable I'm open to
> change this.
>
> BTW: thanks for your review!
>
>>
>>> +
>>> + 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);
>>> +}
>>> +
>>> +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,
>>> +};
>>
>> Viele Grüße,
>> Stefan Roese
>>
>
> Best regards
> - Lukas
>
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Add eFuse access for ZynqMP
2024-05-14 14:04 [PATCH 0/3] Add eFuse access for ZynqMP lukas.funke-oss
` (2 preceding siblings ...)
2024-05-14 14:04 ` [PATCH 3/3] drivers: misc: Add driver to access ZynqMP efuses lukas.funke-oss
@ 2024-05-15 7:08 ` Marek Behún
3 siblings, 0 replies; 9+ messages in thread
From: Marek Behún @ 2024-05-15 7:08 UTC (permalink / raw)
To: lukas.funke-oss
Cc: u-boot, Michal Simek, Lukas Funke, Algapally Santosh Sagar,
Ashok Reddy Soma, Caleb Connolly, Christian Taedcke,
Heinrich Schuchardt, Ilias Apalodimas, Jonas Karlman, Kever Yang,
Neil Armstrong, Peng Fan, Qu Wenruo, Simon Glass,
Stefan Herbrechtsmeier, Stefan Roese, Tanmay Shah, Tom Rini,
Venkatesh Yadav Abbarapu, Wan Yee Lau
On Tue, 14 May 2024 16:04:13 +0200
lukas.funke-oss@weidmueller.com wrote:
> From: Lukas Funke <lukas.funke@weidmueller.com>
>
>
> This series adds a driver to read and write ZynqMP eFuses [1]. The
> driver can be accessed by the 'efuse_read' and 'efuse_write' subcommands
> of the 'zynqmp' command.
Vendor specific commands aren't great.
There is the 'fuse' command in u-boot. You need to implement the
fuse_read()
fuse_sense()
fuse_prog()
fuse_override()
functions.
See for example arch/arm/mach-mvebu/efuse.c, or other implementations.
Please don't invent new vendor specific commands in new code, it is an
antipattern.
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread