The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/2] Add Versal PUF support
@ 2026-07-07  9:56 Harsh Jain
  2026-07-07  9:56 ` [PATCH 1/2] firmware: xilinx: Add Versal PUF PM API support Harsh Jain
  2026-07-07  9:56 ` [PATCH 2/2] misc: Add Xilinx PUF driver Harsh Jain
  0 siblings, 2 replies; 7+ messages in thread
From: Harsh Jain @ 2026-07-07  9:56 UTC (permalink / raw)
  To: arnd, gregkh, michal.simek, linux-kernel, sarat.chand.savitala,
	nkowlaku
  Cc: Harsh Jain

Versal devices contain a physically unclonable function (PUF) block
that derives device-unique secrets from intrinsic silicon characteristics.

Add a misc character driver to support PUF registration and regeneration.
Registration generates a unique secret and outputs binary helper data
that can later be used during regeneration to reproduce the same
secrets.

Harsh Jain (2):
  firmware: xilinx: Add Versal PUF PM API support
  misc: Add Xilinx PUF driver

 MAINTAINERS                                 |   6 +
 drivers/firmware/xilinx/zynqmp-crypto.c     |  51 +++
 drivers/misc/Kconfig                        |  11 +
 drivers/misc/Makefile                       |   1 +
 drivers/misc/xilinx_puf.c                   | 377 ++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp-crypto.h |  30 ++
 include/uapi/misc/xilinx_puf.h              |  82 +++++
 7 files changed, 558 insertions(+)
 create mode 100644 drivers/misc/xilinx_puf.c
 create mode 100644 include/uapi/misc/xilinx_puf.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] firmware: xilinx: Add Versal PUF PM API support
  2026-07-07  9:56 [PATCH 0/2] Add Versal PUF support Harsh Jain
@ 2026-07-07  9:56 ` Harsh Jain
  2026-07-07  9:56 ` [PATCH 2/2] misc: Add Xilinx PUF driver Harsh Jain
  1 sibling, 0 replies; 7+ messages in thread
From: Harsh Jain @ 2026-07-07  9:56 UTC (permalink / raw)
  To: arnd, gregkh, michal.simek, linux-kernel, sarat.chand.savitala,
	nkowlaku
  Cc: Harsh Jain

Add XPUF command IDs and versal_pm_puf_registration(),
versal_pm_puf_regeneration(), and versal_pm_puf_clear_id() wrappers
for invoking PUF operations via platform management firmware.

Also add versal_pm_puf_key_zero() so the misc driver can clear the
PUF-derived AES key via the existing XilSecure PM API.

Signed-off-by: Harsh Jain <h.jain@amd.com>
---
 drivers/firmware/xilinx/zynqmp-crypto.c     | 51 +++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp-crypto.h | 30 ++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp-crypto.c b/drivers/firmware/xilinx/zynqmp-crypto.c
index f06f1e2f67b8..4a4e0b79d126 100644
--- a/drivers/firmware/xilinx/zynqmp-crypto.c
+++ b/drivers/firmware/xilinx/zynqmp-crypto.c
@@ -236,3 +236,54 @@ int versal_pm_aes_init(void)
 	return zynqmp_pm_invoke_fn(XSECURE_API_AES_INIT, NULL, 0);
 }
 EXPORT_SYMBOL_GPL(versal_pm_aes_init);
+
+/**
+ * versal_pm_puf_registration - Invoke PUF registration through platform firmware
+ * @in_addr:	Physical address of the PUF parameter block
+ *
+ * Return:	Returns status from the firmware, or an error code.
+ */
+int versal_pm_puf_registration(const u64 in_addr)
+{
+	return zynqmp_pm_invoke_fn(XPUF_API_PUF_REGISTRATION, NULL, 2,
+				   lower_32_bits(in_addr),
+				   upper_32_bits(in_addr));
+}
+EXPORT_SYMBOL_GPL(versal_pm_puf_registration);
+
+/**
+ * versal_pm_puf_regeneration - Invoke PUF regeneration through platform firmware
+ * @in_addr:	Physical address of the PUF parameter block
+ *
+ * Return:	Returns status from the firmware, or an error code.
+ */
+int versal_pm_puf_regeneration(const u64 in_addr)
+{
+	return zynqmp_pm_invoke_fn(XPUF_API_PUF_REGENERATION, NULL, 2,
+				   lower_32_bits(in_addr),
+				   upper_32_bits(in_addr));
+}
+EXPORT_SYMBOL_GPL(versal_pm_puf_regeneration);
+
+/**
+ * versal_pm_puf_clear_id - Clear PUF ID via platform firmware
+ *
+ * Return:	Returns status from the firmware, or an error code.
+ */
+int versal_pm_puf_clear_id(void)
+{
+	return zynqmp_pm_invoke_fn(XPUF_API_PUF_CLEAR_PUF_ID, NULL, 0);
+}
+EXPORT_SYMBOL_GPL(versal_pm_puf_clear_id);
+
+/**
+ * versal_pm_puf_key_zero - Zero the PUF-derived KEK in the AES engine
+ *
+ * Return:	Returns status from the firmware, or an error code.
+ */
+int versal_pm_puf_key_zero(void)
+{
+	return zynqmp_pm_invoke_fn(XSECURE_API_AES_KEY_ZERO, NULL, 1,
+				   XSECURE_AES_PUF_KEY_SRC);
+}
+EXPORT_SYMBOL_GPL(versal_pm_puf_key_zero);
diff --git a/include/linux/firmware/xlnx-zynqmp-crypto.h b/include/linux/firmware/xlnx-zynqmp-crypto.h
index 56595ab37c43..710e40403663 100644
--- a/include/linux/firmware/xlnx-zynqmp-crypto.h
+++ b/include/linux/firmware/xlnx-zynqmp-crypto.h
@@ -32,6 +32,12 @@ struct xlnx_feature {
 #define XSECURE_API_AES_DECRYPT_FINAL	0x50f
 #define XSECURE_API_AES_KEY_ZERO	0x510
 #define XSECURE_API_AES_WRITE_KEY	0x511
+#define XSECURE_AES_PUF_KEY_SRC		0xb
+
+/* XilPuf API commands module id + api id */
+#define XPUF_API_PUF_REGISTRATION	0xc01
+#define XPUF_API_PUF_REGENERATION	0xc02
+#define XPUF_API_PUF_CLEAR_PUF_ID	0xc03
 
 #if IS_REACHABLE(CONFIG_ZYNQMP_FIRMWARE)
 int zynqmp_pm_aes_engine(const u64 address, u32 *out);
@@ -47,6 +53,10 @@ int versal_pm_aes_dec_update(const u64 in_params, const u64 in_addr);
 int versal_pm_aes_dec_final(const u64 gcm_addr);
 int versal_pm_aes_enc_final(const u64 gcm_addr);
 int versal_pm_aes_init(void);
+int versal_pm_puf_registration(const u64 in_addr);
+int versal_pm_puf_regeneration(const u64 in_addr);
+int versal_pm_puf_clear_id(void);
+int versal_pm_puf_key_zero(void);
 
 #else
 static inline int zynqmp_pm_aes_engine(const u64 address, u32 *out)
@@ -114,6 +124,26 @@ static inline int versal_pm_aes_init(void)
 	return -ENODEV;
 }
 
+static inline int versal_pm_puf_registration(const u64 in_addr)
+{
+	return -ENODEV;
+}
+
+static inline int versal_pm_puf_regeneration(const u64 in_addr)
+{
+	return -ENODEV;
+}
+
+static inline int versal_pm_puf_clear_id(void)
+{
+	return -ENODEV;
+}
+
+static inline int versal_pm_puf_key_zero(void)
+{
+	return -ENODEV;
+}
+
 #endif
 
 #endif /* __FIRMWARE_XLNX_ZYNQMP_CRYPTO_H__ */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] misc: Add Xilinx PUF driver
  2026-07-07  9:56 [PATCH 0/2] Add Versal PUF support Harsh Jain
  2026-07-07  9:56 ` [PATCH 1/2] firmware: xilinx: Add Versal PUF PM API support Harsh Jain
@ 2026-07-07  9:56 ` Harsh Jain
  2026-07-07 10:04   ` Arnd Bergmann
  2026-07-07 10:12   ` Greg KH
  1 sibling, 2 replies; 7+ messages in thread
From: Harsh Jain @ 2026-07-07  9:56 UTC (permalink / raw)
  To: arnd, gregkh, michal.simek, linux-kernel, sarat.chand.savitala,
	nkowlaku
  Cc: Harsh Jain

Versal devices contain a physically unclonable function (PUF) block
that derives device-unique secrets from intrinsic silicon
characteristics. The PUF generates a Key Encryption Key (KEK) used
internally by the AES engine for secure key storage, which is not
externally accessible. In addition, the PUF provides a readable
device-unique identifier derived from the same entropy source.

Add a misc character driver to support PUF registration and regeneration.
Registration generates a unique secret and outputs binary helper data
that can later be used during regeneration to reproduce the same
secrets.

The driver also implements PUF_CLEAR_ID and PUF_CLEAR_KEY ioctls to clear
the PUF ID and PUF-derived AES key via firmware. They are disabled by default
unless puf_clear is set.

Signed-off-by: Harsh Jain <h.jain@amd.com>
---
 MAINTAINERS                    |   6 +
 drivers/misc/Kconfig           |  11 +
 drivers/misc/Makefile          |   1 +
 drivers/misc/xilinx_puf.c      | 377 +++++++++++++++++++++++++++++++++
 include/uapi/misc/xilinx_puf.h |  82 +++++++
 5 files changed, 477 insertions(+)
 create mode 100644 drivers/misc/xilinx_puf.c
 create mode 100644 include/uapi/misc/xilinx_puf.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 15011f5752a9..a46a34f163b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29546,6 +29546,12 @@ L:	netdev@vger.kernel.org
 S:	Orphan
 F:	drivers/net/ethernet/xilinx/ll_temac*
 
+XILINX PUF DRIVER
+M:	Harsh Jain <h.jain@amd.com>
+S:	Maintained
+F:	drivers/misc/xilinx_puf.c
+F:	include/uapi/misc/xilinx_puf.h
+
 XILINX PWM DRIVER
 M:	Sean Anderson <sean.anderson@linux.dev>
 S:	Maintained
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 390256ed91f4..98ec00765647 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -490,6 +490,17 @@ config XILINX_SDFEC
 
 	  If unsure, say N.
 
+config XILINX_PUF
+	tristate "Xilinx PUF driver"
+	depends on ZYNQMP_FIRMWARE
+	help
+	  This option enables support for the Xilinx Physically Unclonable Function
+	  (PUF) driver on AMD Versal devices.
+	  It is a configurable driver to generate PUF KEK source either by using
+	  PUF registration or regeneration command.
+
+	  If unsure, say N.
+
 config MISC_RTSX
 	tristate
 	default MISC_RTSX_PCI || MISC_RTSX_USB
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index fed47c7672b9..66d1ea5d1f13 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_BCM_VK)		+= bcm-vk/
 obj-y				+= cardreader/
 obj-$(CONFIG_PVPANIC)   	+= pvpanic/
 obj-$(CONFIG_UACCE)		+= uacce/
+obj-$(CONFIG_XILINX_PUF)	+= xilinx_puf.o
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
 obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
 obj-$(CONFIG_NTSYNC)		+= ntsync.o
diff --git a/drivers/misc/xilinx_puf.c b/drivers/misc/xilinx_puf.c
new file mode 100644
index 000000000000..ad8ada58b105
--- /dev/null
+++ b/drivers/misc/xilinx_puf.c
@@ -0,0 +1,377 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Xilinx PUF device.
+ *
+ * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
+ *
+ * Description:
+ * This driver is developed for PUF registration and regeneration support.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <uapi/misc/xilinx_puf.h>
+
+static bool puf_clear;
+module_param(puf_clear, bool, 0600);
+MODULE_PARM_DESC(puf_clear, "Flag to enable clearing of PUF ID and key.");
+
+/**
+ * struct puf_params - parameters for PUF
+ * @pufoperation: PUF registration or regeneration operation
+ * @globalvarfilter: global variation filter
+ * @readoption: option to read PUF data from efuse cache or ram address
+ * @reserved: explicit padding to match the Versal PLM PUF parameter layout.
+ * @shuttervalue: shutter value for PUF registration/regeneration
+ * @readsyndromeaddr: address to store the syndrome data during registration
+ * @chashaddr: CHASH address
+ * @auxaddr: AUX address
+ * @pufidaddr: PUF ID address
+ * @writesyndromeaddr: address where syndrome data is present and it is passed to the user
+ * @trimsyndataaddr: trimmed syndrome data will be stored
+ */
+struct puf_params {
+	u8 pufoperation;
+	u8 globalvarfilter;
+	u8 readoption;
+	u8 reserved;
+	u32 shuttervalue;
+	u64 readsyndromeaddr;
+	u64 chashaddr;
+	u64 auxaddr;
+	u64 pufidaddr;
+	u64 writesyndromeaddr;
+	u64 trimsyndataaddr;
+};
+
+/**
+ * struct xpuf_dev - Driver data for PUF
+ * @dev: pointer to device struct
+ * @miscdev: misc device handle
+ */
+struct xpuf_dev {
+	struct device *dev;
+	struct miscdevice miscdev;
+};
+
+static int xlnx_puf_regis(struct xpuf_dev *puf, struct puf_usrparams *pufreq)
+{
+	struct puf_params *pufin;
+	struct pufdata *pufdat;
+	dma_addr_t dma_addr_data;
+	dma_addr_t dma_addr_in;
+	u32 buflen;
+	void *buf;
+	int ret;
+
+	if (pufreq->pufoperation != PUF_REGIS)
+		return -EINVAL;
+
+	if (pufreq->readoption != PUF_READ_FROM_RAM &&
+	    pufreq->readoption != PUF_READ_FROM_EFUSE_CACHE)
+		return -EINVAL;
+
+	buflen = sizeof(struct puf_params) + sizeof(struct pufdata);
+	buf = kzalloc(buflen, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	pufin = buf;
+	pufin->pufoperation = pufreq->pufoperation;
+	pufin->globalvarfilter = pufreq->globalvarfilter;
+	pufin->readoption = pufreq->readoption;
+	pufin->shuttervalue = pufreq->shuttervalue;
+
+	pufdat = buf + sizeof(struct puf_params);
+	dma_addr_in = dma_map_single(puf->dev, buf, buflen, DMA_BIDIRECTIONAL);
+	if (dma_mapping_error(puf->dev, dma_addr_in)) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	dma_addr_data = dma_addr_in + sizeof(struct puf_params);
+	pufin->readsyndromeaddr = (u64)dma_addr_data;
+	pufin->chashaddr = (u64)(pufin->readsyndromeaddr + sizeof(pufdat->pufhd.syndata));
+	pufin->auxaddr = (u64)(pufin->chashaddr + sizeof(pufdat->pufhd.chash));
+	pufin->pufidaddr = (u64)(pufin->auxaddr + sizeof(pufdat->pufhd.aux));
+	pufin->trimsyndataaddr = (u64)(pufin->pufidaddr + sizeof(pufdat->pufid));
+
+	dma_sync_single_for_device(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
+	ret = versal_pm_puf_registration(dma_addr_in);
+	dma_unmap_single(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
+	if (ret)
+		goto cleanup;
+
+	if (copy_to_user(u64_to_user_ptr(pufreq->pufdataaddr), pufdat, sizeof(struct pufdata))) {
+		ret = -EFAULT;
+		goto cleanup;
+	}
+
+cleanup:
+	kfree(buf);
+
+	return ret;
+}
+
+static int xlnx_puf_regen_id(struct xpuf_dev *puf, struct puf_usrparams *pufreq)
+{
+	struct puf_helperdata *pufhd;
+	struct puf_params *pufin;
+	dma_addr_t dma_addr_data;
+	dma_addr_t dma_addr_in;
+	u32 buflen;
+	void *buf;
+	int ret;
+
+	if (pufreq->pufoperation != PUF_REGEN &&
+	    pufreq->pufoperation != PUF_REGEN_ID)
+		return -EINVAL;
+
+	if (pufreq->readoption != PUF_READ_FROM_RAM &&
+	    pufreq->readoption != PUF_READ_FROM_EFUSE_CACHE)
+		return -EINVAL;
+
+	buflen = sizeof(struct puf_params) + sizeof(struct puf_helperdata) +
+		 PUF_ID_LEN_IN_BYTES;
+
+	buf = kzalloc(buflen, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	pufin = buf;
+	pufin->pufoperation = pufreq->pufoperation;
+	pufin->globalvarfilter = pufreq->globalvarfilter;
+	pufin->shuttervalue = pufreq->shuttervalue;
+	pufin->readoption = pufreq->readoption;
+
+	pufhd = buf + sizeof(struct puf_params);
+	if (copy_from_user(pufhd, u64_to_user_ptr(pufreq->pufdataaddr),
+			   sizeof(struct puf_helperdata))) {
+		ret = -EFAULT;
+		goto cleanup;
+	}
+
+	dma_addr_in = dma_map_single(puf->dev, buf, buflen, DMA_BIDIRECTIONAL);
+	if (dma_mapping_error(puf->dev, dma_addr_in)) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	dma_addr_data = dma_addr_in + sizeof(struct puf_params);
+	pufin->writesyndromeaddr = (u64)dma_addr_data;
+	pufin->chashaddr = (u64)(pufin->writesyndromeaddr + sizeof(pufhd->syndata));
+	pufin->auxaddr = (u64)(pufin->chashaddr + sizeof(pufhd->chash));
+	pufin->pufidaddr = (u64)(pufin->auxaddr + sizeof(pufhd->aux));
+
+	dma_sync_single_for_device(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
+	ret = versal_pm_puf_regeneration(dma_addr_in);
+	dma_unmap_single(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
+	if (ret)
+		goto cleanup;
+
+	if (copy_to_user(u64_to_user_ptr(pufreq->pufidaddr), ((char *)pufhd +
+			 sizeof(struct puf_helperdata)),
+			 PUF_ID_LEN_IN_BYTES)) {
+		ret = -EFAULT;
+		goto cleanup;
+	}
+
+cleanup:
+	kfree(buf);
+
+	return ret;
+}
+
+static long xlnx_puf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct xpuf_dev *puf = file->private_data;
+	struct puf_usrparams pufreq;
+	void __user *data = NULL;
+	int ret;
+
+	if (_IOC_TYPE(cmd) != PUF_IOC_MAGIC)
+		return -ENOTTY;
+
+	/* check if ioctl argument is present and valid */
+	if (_IOC_DIR(cmd) != _IOC_NONE) {
+		data = (void __user *)arg;
+		if (!data)
+			return -EINVAL;
+
+		if (copy_from_user(&pufreq, data, sizeof(struct puf_usrparams)))
+			return -EFAULT;
+	}
+
+	switch (cmd) {
+	case PUF_REGISTRATION:
+		ret = xlnx_puf_regis(puf, &pufreq);
+		break;
+	case PUF_REGENERATION:
+	case PUF_REGEN_ID_ONLY:
+		ret = xlnx_puf_regen_id(puf, &pufreq);
+		break;
+	case PUF_CLEAR_ID:
+		if (!puf_clear) {
+			ret = -EOPNOTSUPP;
+			break;
+		}
+		ret = versal_pm_puf_clear_id();
+		break;
+	case PUF_CLEAR_KEY:
+		if (!puf_clear) {
+			ret = -EOPNOTSUPP;
+			break;
+		}
+		ret = versal_pm_aes_init();
+		if (!ret)
+			ret = versal_pm_puf_key_zero();
+		break;
+	default:
+		return -ENOTTY;
+	}
+
+	return ret;
+}
+
+/**
+ * xlnx_puf_open - open puf device
+ * @inode:	inode object
+ * @file:	file object
+ *
+ * Return:	0 if successful; otherwise -errno
+ */
+static int xlnx_puf_open(struct inode *inode, struct file *file)
+{
+	struct xpuf_dev *xpuf;
+
+	xpuf = container_of(file->private_data, struct xpuf_dev, miscdev);
+	file->private_data = xpuf;
+
+	dev_dbg(xpuf->dev, "device /dev/xpuf opened\n");
+
+	return 0;
+}
+
+/**
+ * xlnx_puf_release - release puf resources
+ * @inode:	inode object
+ * @file:	file object
+ *
+ * Return:	0 if successful; otherwise -errno
+ */
+static int xlnx_puf_release(struct inode *inode, struct file *file)
+{
+	struct xpuf_dev *xpuf = file->private_data;
+
+	dev_dbg(xpuf->dev, "device /dev/xpuf closed\n");
+
+	return 0;
+}
+
+static const struct file_operations xlnx_puf_fops = {
+	.owner		= THIS_MODULE,
+	.open		= xlnx_puf_open,
+	.release	= xlnx_puf_release,
+	.unlocked_ioctl	= xlnx_puf_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+};
+
+/**
+ * xlnx_puf_probe - probe puf device
+ * @pdev: Pointer to puf platform device structure
+ *
+ * Return: 0 if successful; otherwise -errno
+ */
+static int xlnx_puf_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct xpuf_dev *xpuf;
+	int ret;
+
+	xpuf = devm_kzalloc(dev, sizeof(*xpuf), GFP_KERNEL);
+	if (!xpuf)
+		return -ENOMEM;
+
+	xpuf->dev = dev;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret < 0) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret < 0) {
+			dev_err(dev, "no usable DMA configuration\n");
+			return ret;
+		}
+	}
+
+	xpuf->miscdev.minor = MISC_DYNAMIC_MINOR;
+	xpuf->miscdev.name = "xpuf";
+	xpuf->miscdev.fops = &xlnx_puf_fops;
+	xpuf->miscdev.parent = dev;
+
+	ret = misc_register(&xpuf->miscdev);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, xpuf);
+
+	dev_dbg(dev, "puf registered as /dev/xpuf successfully\n");
+
+	return 0;
+}
+
+/**
+ * xlnx_puf_remove - clean up structures
+ * @pdev:	The structure containing the device's details
+ */
+static void xlnx_puf_remove(struct platform_device *pdev)
+{
+	struct xpuf_dev *xpuf = platform_get_drvdata(pdev);
+
+	misc_deregister(&xpuf->miscdev);
+
+	dev_dbg(xpuf->dev, "device /dev/xpuf removed\n");
+}
+
+static struct platform_driver xlnx_puf_drv = {
+	.probe = xlnx_puf_probe,
+	.remove = xlnx_puf_remove,
+	.driver = {
+		.name = "xlnx-puf",
+	},
+};
+
+static struct platform_device *xlnx_puf_pdev;
+
+static int __init xlnx_puf_driver_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&xlnx_puf_drv);
+	if (ret)
+		return ret;
+
+	xlnx_puf_pdev = platform_device_register_simple(xlnx_puf_drv.driver.name,
+							0, NULL, 0);
+	if (IS_ERR(xlnx_puf_pdev)) {
+		ret = PTR_ERR(xlnx_puf_pdev);
+		platform_driver_unregister(&xlnx_puf_drv);
+	}
+
+	return ret;
+}
+
+static void __exit xlnx_puf_driver_exit(void)
+{
+	platform_device_unregister(xlnx_puf_pdev);
+	platform_driver_unregister(&xlnx_puf_drv);
+}
+
+module_init(xlnx_puf_driver_init);
+module_exit(xlnx_puf_driver_exit);
+
+MODULE_AUTHOR("AMD");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Xilinx Versal PUF driver");
diff --git a/include/uapi/misc/xilinx_puf.h b/include/uapi/misc/xilinx_puf.h
new file mode 100644
index 000000000000..32565a748df3
--- /dev/null
+++ b/include/uapi/misc/xilinx_puf.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Driver for Xilinx PUF device.
+ *
+ * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
+ *
+ * Description:
+ * This driver is developed for PUF registration and regeneration support.
+ */
+
+#ifndef _UAPI_MISC_XILINX_PUF_H_
+#define _UAPI_MISC_XILINX_PUF_H_
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+#define PUF_MAX_SYNDROME_DATA_LEN_IN_WORDS	140
+#define PUF_EFUSE_TRIM_SYN_DATA_IN_WORDS	127
+#define PUF_ID_LEN_IN_WORDS			8
+#define PUF_ID_LEN_IN_BYTES			32
+#define PUF_REGIS				0
+#define PUF_REGEN				1
+#define PUF_REGEN_ID				2
+
+/**
+ * struct puf_usrparams - user parameters for PUF from user space.
+ * @pufoperation: PUF registration or regeneration operation
+ * @globalvarfilter: global variation filter
+ * @readoption: option to read PUF data from efuse cache or ram address
+ * @reserved: explicit padding to match the Versal PLM PUF parameter layout
+ * @shuttervalue: shutter value for PUF registration/regeneration
+ * @pufdataaddr: address to store/get the puf data during registration/regeneration
+ * @pufidaddr: puf id will be stored either during registration/regeneration
+ */
+struct puf_usrparams {
+	__u8 pufoperation;
+	__u8 globalvarfilter;
+	__u8 readoption;
+	__u8 reserved;
+	__u32 shuttervalue;
+	__u64 pufdataaddr;
+	__u64 pufidaddr;
+};
+
+/**
+ * struct puf_helperdata - parameters for puf helper data.
+ * @syndata: PUF syndrome data
+ * @chash: PUF chash
+ * @aux: PUF aux
+ */
+struct puf_helperdata {
+	__u32 syndata[PUF_MAX_SYNDROME_DATA_LEN_IN_WORDS];
+	__u32 chash;
+	__u32 aux;
+};
+
+/**
+ * struct pufdata - parameters for puf data.
+ * @pufhd: puf helper data of type struct puf_helperdata
+ * @pufid: PUF id
+ * @efusesyndata: PUF efuse syndrome data
+ */
+struct pufdata {
+	struct puf_helperdata pufhd;
+	__u32 pufid[PUF_ID_LEN_IN_WORDS];
+	__u32 efusesyndata[PUF_EFUSE_TRIM_SYN_DATA_IN_WORDS];
+};
+
+enum pufreadoption {
+	PUF_READ_FROM_RAM = 0,
+	PUF_READ_FROM_EFUSE_CACHE = 1
+};
+
+#define PUF_IOC_MAGIC 'P'
+
+#define PUF_REGISTRATION	_IOWR(PUF_IOC_MAGIC, 1, struct puf_usrparams)
+#define PUF_REGENERATION	_IOWR(PUF_IOC_MAGIC, 2, struct puf_usrparams)
+#define PUF_REGEN_ID_ONLY	_IOWR(PUF_IOC_MAGIC, 3, struct puf_usrparams)
+#define PUF_CLEAR_ID		_IO(PUF_IOC_MAGIC, 4)
+#define PUF_CLEAR_KEY		_IO(PUF_IOC_MAGIC, 5)
+
+#endif /* _UAPI_MISC_XILINX_PUF_H_ */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] misc: Add Xilinx PUF driver
  2026-07-07  9:56 ` [PATCH 2/2] misc: Add Xilinx PUF driver Harsh Jain
@ 2026-07-07 10:04   ` Arnd Bergmann
  2026-07-07 10:41     ` Jain, Harsh (AECG-SSW)
  2026-07-07 10:12   ` Greg KH
  1 sibling, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2026-07-07 10:04 UTC (permalink / raw)
  To: Harsh Jain, Greg Kroah-Hartman, Michal Simek, linux-kernel,
	sarat.chand.savitala, nkowlaku

On Tue, Jul 7, 2026, at 11:56, Harsh Jain wrote:
> Versal devices contain a physically unclonable function (PUF) block
> that derives device-unique secrets from intrinsic silicon
> characteristics. The PUF generates a Key Encryption Key (KEK) used
> internally by the AES engine for secure key storage, which is not
> externally accessible. In addition, the PUF provides a readable
> device-unique identifier derived from the same entropy source.
>
> Add a misc character driver to support PUF registration and regeneration.
> Registration generates a unique secret and outputs binary helper data
> that can later be used during regeneration to reproduce the same
> secrets.
>
> The driver also implements PUF_CLEAR_ID and PUF_CLEAR_KEY ioctls to clear
> the PUF ID and PUF-derived AES key via firmware. They are disabled by default
> unless puf_clear is set.
>
> Signed-off-by: Harsh Jain <h.jain@amd.com>

Hi Harsh,

This description sounds like it should be integrated into the
existing crypto infrastructure of the kernel. I'm not sure
what the exact feature set in your hardware is like, but
please work with the crypto and keyctl maintainers to either
use that infrastructure, or extend it to your requirements
instead of adding a custom ioctl interface.

My guess would be that you can have a driver similar
to drivers/platform/cznic/turris-omnia-mcu-keyctl.c
for managing private keys in the hardware and managing
it through the keyctl(2) infrastructure. Have you looked
at this already?

      Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] misc: Add Xilinx PUF driver
  2026-07-07  9:56 ` [PATCH 2/2] misc: Add Xilinx PUF driver Harsh Jain
  2026-07-07 10:04   ` Arnd Bergmann
@ 2026-07-07 10:12   ` Greg KH
  2026-07-07 10:52     ` Jain, Harsh (AECG-SSW)
  1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2026-07-07 10:12 UTC (permalink / raw)
  To: Harsh Jain
  Cc: arnd, michal.simek, linux-kernel, sarat.chand.savitala, nkowlaku

On Tue, Jul 07, 2026 at 03:26:20PM +0530, Harsh Jain wrote:
> Versal devices contain a physically unclonable function (PUF) block
> that derives device-unique secrets from intrinsic silicon
> characteristics. The PUF generates a Key Encryption Key (KEK) used
> internally by the AES engine for secure key storage, which is not
> externally accessible. In addition, the PUF provides a readable
> device-unique identifier derived from the same entropy source.

So why isn't this tied into the normal crypto/keys/tee/whatever
subsystem for this type of hardware?  Why is a custom user/kernel api ok
for this one specific piece of hardware?

And where is the userspace code that interacts with this?

> Add a misc character driver to support PUF registration and regeneration.
> Registration generates a unique secret and outputs binary helper data
> that can later be used during regeneration to reproduce the same
> secrets.
> 
> The driver also implements PUF_CLEAR_ID and PUF_CLEAR_KEY ioctls to clear
> the PUF ID and PUF-derived AES key via firmware. They are disabled by default
> unless puf_clear is set.

Please wrap the changelog at 72 columns, checkpatch should have caught
this, right?

> 
> Signed-off-by: Harsh Jain <h.jain@amd.com>
> ---
>  MAINTAINERS                    |   6 +
>  drivers/misc/Kconfig           |  11 +
>  drivers/misc/Makefile          |   1 +
>  drivers/misc/xilinx_puf.c      | 377 +++++++++++++++++++++++++++++++++
>  include/uapi/misc/xilinx_puf.h |  82 +++++++
>  5 files changed, 477 insertions(+)
>  create mode 100644 drivers/misc/xilinx_puf.c
>  create mode 100644 include/uapi/misc/xilinx_puf.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 15011f5752a9..a46a34f163b4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -29546,6 +29546,12 @@ L:	netdev@vger.kernel.org
>  S:	Orphan
>  F:	drivers/net/ethernet/xilinx/ll_temac*
>  
> +XILINX PUF DRIVER
> +M:	Harsh Jain <h.jain@amd.com>
> +S:	Maintained

You aren't paid to support this?  AMD doesn't care?  That's sad.

> +F:	drivers/misc/xilinx_puf.c
> +F:	include/uapi/misc/xilinx_puf.h
> +
>  XILINX PWM DRIVER
>  M:	Sean Anderson <sean.anderson@linux.dev>
>  S:	Maintained
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 390256ed91f4..98ec00765647 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -490,6 +490,17 @@ config XILINX_SDFEC
>  
>  	  If unsure, say N.
>  
> +config XILINX_PUF
> +	tristate "Xilinx PUF driver"
> +	depends on ZYNQMP_FIRMWARE
> +	help
> +	  This option enables support for the Xilinx Physically Unclonable Function
> +	  (PUF) driver on AMD Versal devices.
> +	  It is a configurable driver to generate PUF KEK source either by using
> +	  PUF registration or regeneration command.
> +
> +	  If unsure, say N.

Module name?

> +
>  config MISC_RTSX



>  	tristate
>  	default MISC_RTSX_PCI || MISC_RTSX_USB
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index fed47c7672b9..66d1ea5d1f13 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_BCM_VK)		+= bcm-vk/
>  obj-y				+= cardreader/
>  obj-$(CONFIG_PVPANIC)   	+= pvpanic/
>  obj-$(CONFIG_UACCE)		+= uacce/
> +obj-$(CONFIG_XILINX_PUF)	+= xilinx_puf.o
>  obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
>  obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
>  obj-$(CONFIG_NTSYNC)		+= ntsync.o
> diff --git a/drivers/misc/xilinx_puf.c b/drivers/misc/xilinx_puf.c
> new file mode 100644
> index 000000000000..ad8ada58b105
> --- /dev/null
> +++ b/drivers/misc/xilinx_puf.c
> @@ -0,0 +1,377 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Xilinx PUF device.
> + *
> + * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
> + *
> + * Description:
> + * This driver is developed for PUF registration and regeneration support.
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/firmware/xlnx-zynqmp.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <uapi/misc/xilinx_puf.h>
> +
> +static bool puf_clear;
> +module_param(puf_clear, bool, 0600);
> +MODULE_PARM_DESC(puf_clear, "Flag to enable clearing of PUF ID and key.");

This is not the 1990's, please do not add module parameters for things
that should be done other ways.  Especially for a device-specific thing.

> +
> +/**
> + * struct puf_params - parameters for PUF
> + * @pufoperation: PUF registration or regeneration operation
> + * @globalvarfilter: global variation filter
> + * @readoption: option to read PUF data from efuse cache or ram address
> + * @reserved: explicit padding to match the Versal PLM PUF parameter layout.
> + * @shuttervalue: shutter value for PUF registration/regeneration
> + * @readsyndromeaddr: address to store the syndrome data during registration
> + * @chashaddr: CHASH address
> + * @auxaddr: AUX address
> + * @pufidaddr: PUF ID address
> + * @writesyndromeaddr: address where syndrome data is present and it is passed to the user
> + * @trimsyndataaddr: trimmed syndrome data will be stored
> + */
> +struct puf_params {
> +	u8 pufoperation;
> +	u8 globalvarfilter;
> +	u8 readoption;
> +	u8 reserved;
> +	u32 shuttervalue;
> +	u64 readsyndromeaddr;
> +	u64 chashaddr;
> +	u64 auxaddr;
> +	u64 pufidaddr;
> +	u64 writesyndromeaddr;
> +	u64 trimsyndataaddr;
> +};
> +
> +/**
> + * struct xpuf_dev - Driver data for PUF
> + * @dev: pointer to device struct
> + * @miscdev: misc device handle
> + */
> +struct xpuf_dev {
> +	struct device *dev;
> +	struct miscdevice miscdev;
> +};
> +
> +static int xlnx_puf_regis(struct xpuf_dev *puf, struct puf_usrparams *pufreq)
> +{
> +	struct puf_params *pufin;
> +	struct pufdata *pufdat;
> +	dma_addr_t dma_addr_data;
> +	dma_addr_t dma_addr_in;
> +	u32 buflen;
> +	void *buf;
> +	int ret;
> +
> +	if (pufreq->pufoperation != PUF_REGIS)
> +		return -EINVAL;
> +
> +	if (pufreq->readoption != PUF_READ_FROM_RAM &&
> +	    pufreq->readoption != PUF_READ_FROM_EFUSE_CACHE)
> +		return -EINVAL;
> +
> +	buflen = sizeof(struct puf_params) + sizeof(struct pufdata);
> +	buf = kzalloc(buflen, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	pufin = buf;
> +	pufin->pufoperation = pufreq->pufoperation;
> +	pufin->globalvarfilter = pufreq->globalvarfilter;
> +	pufin->readoption = pufreq->readoption;
> +	pufin->shuttervalue = pufreq->shuttervalue;
> +
> +	pufdat = buf + sizeof(struct puf_params);
> +	dma_addr_in = dma_map_single(puf->dev, buf, buflen, DMA_BIDIRECTIONAL);
> +	if (dma_mapping_error(puf->dev, dma_addr_in)) {
> +		ret = -ENOMEM;
> +		goto cleanup;
> +	}
> +
> +	dma_addr_data = dma_addr_in + sizeof(struct puf_params);
> +	pufin->readsyndromeaddr = (u64)dma_addr_data;
> +	pufin->chashaddr = (u64)(pufin->readsyndromeaddr + sizeof(pufdat->pufhd.syndata));
> +	pufin->auxaddr = (u64)(pufin->chashaddr + sizeof(pufdat->pufhd.chash));
> +	pufin->pufidaddr = (u64)(pufin->auxaddr + sizeof(pufdat->pufhd.aux));
> +	pufin->trimsyndataaddr = (u64)(pufin->pufidaddr + sizeof(pufdat->pufid));
> +
> +	dma_sync_single_for_device(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
> +	ret = versal_pm_puf_registration(dma_addr_in);
> +	dma_unmap_single(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
> +	if (ret)
> +		goto cleanup;
> +
> +	if (copy_to_user(u64_to_user_ptr(pufreq->pufdataaddr), pufdat, sizeof(struct pufdata))) {
> +		ret = -EFAULT;
> +		goto cleanup;
> +	}
> +
> +cleanup:
> +	kfree(buf);
> +
> +	return ret;
> +}
> +
> +static int xlnx_puf_regen_id(struct xpuf_dev *puf, struct puf_usrparams *pufreq)
> +{
> +	struct puf_helperdata *pufhd;
> +	struct puf_params *pufin;
> +	dma_addr_t dma_addr_data;
> +	dma_addr_t dma_addr_in;
> +	u32 buflen;
> +	void *buf;
> +	int ret;
> +
> +	if (pufreq->pufoperation != PUF_REGEN &&
> +	    pufreq->pufoperation != PUF_REGEN_ID)
> +		return -EINVAL;
> +
> +	if (pufreq->readoption != PUF_READ_FROM_RAM &&
> +	    pufreq->readoption != PUF_READ_FROM_EFUSE_CACHE)
> +		return -EINVAL;
> +
> +	buflen = sizeof(struct puf_params) + sizeof(struct puf_helperdata) +
> +		 PUF_ID_LEN_IN_BYTES;
> +
> +	buf = kzalloc(buflen, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	pufin = buf;
> +	pufin->pufoperation = pufreq->pufoperation;
> +	pufin->globalvarfilter = pufreq->globalvarfilter;
> +	pufin->shuttervalue = pufreq->shuttervalue;
> +	pufin->readoption = pufreq->readoption;
> +
> +	pufhd = buf + sizeof(struct puf_params);
> +	if (copy_from_user(pufhd, u64_to_user_ptr(pufreq->pufdataaddr),
> +			   sizeof(struct puf_helperdata))) {
> +		ret = -EFAULT;
> +		goto cleanup;
> +	}
> +
> +	dma_addr_in = dma_map_single(puf->dev, buf, buflen, DMA_BIDIRECTIONAL);
> +	if (dma_mapping_error(puf->dev, dma_addr_in)) {
> +		ret = -ENOMEM;
> +		goto cleanup;
> +	}
> +
> +	dma_addr_data = dma_addr_in + sizeof(struct puf_params);
> +	pufin->writesyndromeaddr = (u64)dma_addr_data;
> +	pufin->chashaddr = (u64)(pufin->writesyndromeaddr + sizeof(pufhd->syndata));
> +	pufin->auxaddr = (u64)(pufin->chashaddr + sizeof(pufhd->chash));
> +	pufin->pufidaddr = (u64)(pufin->auxaddr + sizeof(pufhd->aux));
> +
> +	dma_sync_single_for_device(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
> +	ret = versal_pm_puf_regeneration(dma_addr_in);
> +	dma_unmap_single(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
> +	if (ret)
> +		goto cleanup;
> +
> +	if (copy_to_user(u64_to_user_ptr(pufreq->pufidaddr), ((char *)pufhd +
> +			 sizeof(struct puf_helperdata)),
> +			 PUF_ID_LEN_IN_BYTES)) {
> +		ret = -EFAULT;
> +		goto cleanup;
> +	}
> +
> +cleanup:
> +	kfree(buf);
> +
> +	return ret;
> +}
> +
> +static long xlnx_puf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct xpuf_dev *puf = file->private_data;
> +	struct puf_usrparams pufreq;
> +	void __user *data = NULL;
> +	int ret;
> +
> +	if (_IOC_TYPE(cmd) != PUF_IOC_MAGIC)
> +		return -ENOTTY;
> +
> +	/* check if ioctl argument is present and valid */
> +	if (_IOC_DIR(cmd) != _IOC_NONE) {
> +		data = (void __user *)arg;
> +		if (!data)
> +			return -EINVAL;
> +
> +		if (copy_from_user(&pufreq, data, sizeof(struct puf_usrparams)))
> +			return -EFAULT;
> +	}
> +
> +	switch (cmd) {
> +	case PUF_REGISTRATION:
> +		ret = xlnx_puf_regis(puf, &pufreq);
> +		break;
> +	case PUF_REGENERATION:
> +	case PUF_REGEN_ID_ONLY:
> +		ret = xlnx_puf_regen_id(puf, &pufreq);
> +		break;
> +	case PUF_CLEAR_ID:
> +		if (!puf_clear) {
> +			ret = -EOPNOTSUPP;
> +			break;
> +		}
> +		ret = versal_pm_puf_clear_id();
> +		break;
> +	case PUF_CLEAR_KEY:
> +		if (!puf_clear) {
> +			ret = -EOPNOTSUPP;
> +			break;
> +		}
> +		ret = versal_pm_aes_init();
> +		if (!ret)
> +			ret = versal_pm_puf_key_zero();
> +		break;
> +	default:
> +		return -ENOTTY;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * xlnx_puf_open - open puf device
> + * @inode:	inode object
> + * @file:	file object
> + *
> + * Return:	0 if successful; otherwise -errno
> + */
> +static int xlnx_puf_open(struct inode *inode, struct file *file)
> +{
> +	struct xpuf_dev *xpuf;
> +
> +	xpuf = container_of(file->private_data, struct xpuf_dev, miscdev);
> +	file->private_data = xpuf;
> +
> +	dev_dbg(xpuf->dev, "device /dev/xpuf opened\n");
> +
> +	return 0;
> +}
> +
> +/**
> + * xlnx_puf_release - release puf resources
> + * @inode:	inode object
> + * @file:	file object
> + *
> + * Return:	0 if successful; otherwise -errno
> + */
> +static int xlnx_puf_release(struct inode *inode, struct file *file)
> +{
> +	struct xpuf_dev *xpuf = file->private_data;
> +
> +	dev_dbg(xpuf->dev, "device /dev/xpuf closed\n");
> +
> +	return 0;
> +}

This does nothing, why is it needed at all?

> +
> +static const struct file_operations xlnx_puf_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= xlnx_puf_open,
> +	.release	= xlnx_puf_release,
> +	.unlocked_ioctl	= xlnx_puf_ioctl,
> +	.compat_ioctl	= compat_ptr_ioctl,
> +};
> +
> +/**
> + * xlnx_puf_probe - probe puf device
> + * @pdev: Pointer to puf platform device structure
> + *
> + * Return: 0 if successful; otherwise -errno
> + */
> +static int xlnx_puf_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct xpuf_dev *xpuf;
> +	int ret;
> +
> +	xpuf = devm_kzalloc(dev, sizeof(*xpuf), GFP_KERNEL);
> +	if (!xpuf)
> +		return -ENOMEM;
> +
> +	xpuf->dev = dev;
> +
> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	if (ret < 0) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret < 0) {
> +			dev_err(dev, "no usable DMA configuration\n");
> +			return ret;
> +		}
> +	}
> +
> +	xpuf->miscdev.minor = MISC_DYNAMIC_MINOR;
> +	xpuf->miscdev.name = "xpuf";
> +	xpuf->miscdev.fops = &xlnx_puf_fops;
> +	xpuf->miscdev.parent = dev;
> +
> +	ret = misc_register(&xpuf->miscdev);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, xpuf);
> +
> +	dev_dbg(dev, "puf registered as /dev/xpuf successfully\n");
> +
> +	return 0;
> +}
> +
> +/**
> + * xlnx_puf_remove - clean up structures
> + * @pdev:	The structure containing the device's details
> + */
> +static void xlnx_puf_remove(struct platform_device *pdev)
> +{
> +	struct xpuf_dev *xpuf = platform_get_drvdata(pdev);
> +
> +	misc_deregister(&xpuf->miscdev);
> +
> +	dev_dbg(xpuf->dev, "device /dev/xpuf removed\n");

ftrace is your friend :)

> +}
> +
> +static struct platform_driver xlnx_puf_drv = {
> +	.probe = xlnx_puf_probe,
> +	.remove = xlnx_puf_remove,
> +	.driver = {
> +		.name = "xlnx-puf",
> +	},
> +};
> +
> +static struct platform_device *xlnx_puf_pdev;
> +
> +static int __init xlnx_puf_driver_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&xlnx_puf_drv);
> +	if (ret)
> +		return ret;
> +
> +	xlnx_puf_pdev = platform_device_register_simple(xlnx_puf_drv.driver.name,
> +							0, NULL, 0);

Why isn't this using the faux bus instead?  This isn't a real platform
device at all as you have no resources for it that describe it.




> +	if (IS_ERR(xlnx_puf_pdev)) {
> +		ret = PTR_ERR(xlnx_puf_pdev);
> +		platform_driver_unregister(&xlnx_puf_drv);
> +	}
> +
> +	return ret;
> +}
> +
> +static void __exit xlnx_puf_driver_exit(void)
> +{
> +	platform_device_unregister(xlnx_puf_pdev);
> +	platform_driver_unregister(&xlnx_puf_drv);
> +}
> +
> +module_init(xlnx_puf_driver_init);
> +module_exit(xlnx_puf_driver_exit);
> +
> +MODULE_AUTHOR("AMD");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Xilinx Versal PUF driver");
> diff --git a/include/uapi/misc/xilinx_puf.h b/include/uapi/misc/xilinx_puf.h
> new file mode 100644
> index 000000000000..32565a748df3
> --- /dev/null
> +++ b/include/uapi/misc/xilinx_puf.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Driver for Xilinx PUF device.
> + *
> + * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
> + *
> + * Description:
> + * This driver is developed for PUF registration and regeneration support.
> + */
> +
> +#ifndef _UAPI_MISC_XILINX_PUF_H_
> +#define _UAPI_MISC_XILINX_PUF_H_
> +
> +#include <linux/ioctl.h>
> +#include <linux/types.h>
> +
> +#define PUF_MAX_SYNDROME_DATA_LEN_IN_WORDS	140
> +#define PUF_EFUSE_TRIM_SYN_DATA_IN_WORDS	127
> +#define PUF_ID_LEN_IN_WORDS			8
> +#define PUF_ID_LEN_IN_BYTES			32
> +#define PUF_REGIS				0
> +#define PUF_REGEN				1
> +#define PUF_REGEN_ID				2
> +
> +/**
> + * struct puf_usrparams - user parameters for PUF from user space.
> + * @pufoperation: PUF registration or regeneration operation
> + * @globalvarfilter: global variation filter
> + * @readoption: option to read PUF data from efuse cache or ram address
> + * @reserved: explicit padding to match the Versal PLM PUF parameter layout
> + * @shuttervalue: shutter value for PUF registration/regeneration
> + * @pufdataaddr: address to store/get the puf data during registration/regeneration
> + * @pufidaddr: puf id will be stored either during registration/regeneration
> + */
> +struct puf_usrparams {
> +	__u8 pufoperation;
> +	__u8 globalvarfilter;
> +	__u8 readoption;
> +	__u8 reserved;
> +	__u32 shuttervalue;
> +	__u64 pufdataaddr;
> +	__u64 pufidaddr;
> +};
> +
> +/**
> + * struct puf_helperdata - parameters for puf helper data.
> + * @syndata: PUF syndrome data
> + * @chash: PUF chash
> + * @aux: PUF aux
> + */
> +struct puf_helperdata {
> +	__u32 syndata[PUF_MAX_SYNDROME_DATA_LEN_IN_WORDS];
> +	__u32 chash;
> +	__u32 aux;
> +};
> +
> +/**
> + * struct pufdata - parameters for puf data.
> + * @pufhd: puf helper data of type struct puf_helperdata
> + * @pufid: PUF id
> + * @efusesyndata: PUF efuse syndrome data
> + */
> +struct pufdata {
> +	struct puf_helperdata pufhd;
> +	__u32 pufid[PUF_ID_LEN_IN_WORDS];
> +	__u32 efusesyndata[PUF_EFUSE_TRIM_SYN_DATA_IN_WORDS];
> +};
> +
> +enum pufreadoption {
> +	PUF_READ_FROM_RAM = 0,
> +	PUF_READ_FROM_EFUSE_CACHE = 1
> +};
> +
> +#define PUF_IOC_MAGIC 'P'

Why are you allowed to use "P" for your ioctl?  Isn't that already used
by other drivers?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] misc: Add Xilinx PUF driver
  2026-07-07 10:04   ` Arnd Bergmann
@ 2026-07-07 10:41     ` Jain, Harsh (AECG-SSW)
  0 siblings, 0 replies; 7+ messages in thread
From: Jain, Harsh (AECG-SSW) @ 2026-07-07 10:41 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Michal Simek, linux-kernel,
	sarat.chand.savitala, nkowlaku


On 7/7/2026 3:34 PM, Arnd Bergmann wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Tue, Jul 7, 2026, at 11:56, Harsh Jain wrote:
>> Versal devices contain a physically unclonable function (PUF) block
>> that derives device-unique secrets from intrinsic silicon
>> characteristics. The PUF generates a Key Encryption Key (KEK) used
>> internally by the AES engine for secure key storage, which is not
>> externally accessible. In addition, the PUF provides a readable
>> device-unique identifier derived from the same entropy source.
>>
>> Add a misc character driver to support PUF registration and regeneration.
>> Registration generates a unique secret and outputs binary helper data
>> that can later be used during regeneration to reproduce the same
>> secrets.
>>
>> The driver also implements PUF_CLEAR_ID and PUF_CLEAR_KEY ioctls to clear
>> the PUF ID and PUF-derived AES key via firmware. They are disabled by default
>> unless puf_clear is set.
>>
>> Signed-off-by: Harsh Jain <h.jain@amd.com>
> Hi Harsh,
>
> This description sounds like it should be integrated into the
> existing crypto infrastructure of the kernel. I'm not sure
> what the exact feature set in your hardware is like, but
> please work with the crypto and keyctl maintainers to either
> use that infrastructure, or extend it to your requirements
> instead of adding a custom ioctl interface.
>
> My guess would be that you can have a driver similar
> to drivers/platform/cznic/turris-omnia-mcu-keyctl.c
> for managing private keys in the hardware and managing
> it through the keyctl(2) infrastructure. Have you looked
> at this already?

Thanks Arnd for quick feedback. I will check the mentioned subsystem.


>
>        Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] misc: Add Xilinx PUF driver
  2026-07-07 10:12   ` Greg KH
@ 2026-07-07 10:52     ` Jain, Harsh (AECG-SSW)
  0 siblings, 0 replies; 7+ messages in thread
From: Jain, Harsh (AECG-SSW) @ 2026-07-07 10:52 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, michal.simek, linux-kernel, sarat.chand.savitala, nkowlaku


On 7/7/2026 3:42 PM, Greg KH wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Tue, Jul 07, 2026 at 03:26:20PM +0530, Harsh Jain wrote:
>> Versal devices contain a physically unclonable function (PUF) block
>> that derives device-unique secrets from intrinsic silicon
>> characteristics. The PUF generates a Key Encryption Key (KEK) used
>> internally by the AES engine for secure key storage, which is not
>> externally accessible. In addition, the PUF provides a readable
>> device-unique identifier derived from the same entropy source.
> So why isn't this tied into the normal crypto/keys/tee/whatever
> subsystem for this type of hardware?  Why is a custom user/kernel api ok
> for this one specific piece of hardware?
>
> And where is the userspace code that interacts with this?

We will check crypto and keyctl subsystem

>
>> Add a misc character driver to support PUF registration and regeneration.
>> Registration generates a unique secret and outputs binary helper data
>> that can later be used during regeneration to reproduce the same
>> secrets.
>>
>> The driver also implements PUF_CLEAR_ID and PUF_CLEAR_KEY ioctls to clear
>> the PUF ID and PUF-derived AES key via firmware. They are disabled by default
>> unless puf_clear is set.
> Please wrap the changelog at 72 columns, checkpatch should have caught
> this, right?

I ran it on files instead of patches. Will take care next time.

Thanks for quick reply,

>
>> Signed-off-by: Harsh Jain <h.jain@amd.com>
>> ---
>>   MAINTAINERS                    |   6 +
>>   drivers/misc/Kconfig           |  11 +
>>   drivers/misc/Makefile          |   1 +
>>   drivers/misc/xilinx_puf.c      | 377 +++++++++++++++++++++++++++++++++
>>   include/uapi/misc/xilinx_puf.h |  82 +++++++
>>   5 files changed, 477 insertions(+)
>>   create mode 100644 drivers/misc/xilinx_puf.c
>>   create mode 100644 include/uapi/misc/xilinx_puf.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 15011f5752a9..a46a34f163b4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -29546,6 +29546,12 @@ L:   netdev@vger.kernel.org
>>   S:   Orphan
>>   F:   drivers/net/ethernet/xilinx/ll_temac*
>>
>> +XILINX PUF DRIVER
>> +M:   Harsh Jain <h.jain@amd.com>
>> +S:   Maintained
> You aren't paid to support this?  AMD doesn't care?  That's sad.
>
>> +F:   drivers/misc/xilinx_puf.c
>> +F:   include/uapi/misc/xilinx_puf.h
>> +
>>   XILINX PWM DRIVER
>>   M:   Sean Anderson <sean.anderson@linux.dev>
>>   S:   Maintained
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 390256ed91f4..98ec00765647 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -490,6 +490,17 @@ config XILINX_SDFEC
>>
>>          If unsure, say N.
>>
>> +config XILINX_PUF
>> +     tristate "Xilinx PUF driver"
>> +     depends on ZYNQMP_FIRMWARE
>> +     help
>> +       This option enables support for the Xilinx Physically Unclonable Function
>> +       (PUF) driver on AMD Versal devices.
>> +       It is a configurable driver to generate PUF KEK source either by using
>> +       PUF registration or regeneration command.
>> +
>> +       If unsure, say N.
> Module name?
>
>> +
>>   config MISC_RTSX
>
>
>>        tristate
>>        default MISC_RTSX_PCI || MISC_RTSX_USB
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index fed47c7672b9..66d1ea5d1f13 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -55,6 +55,7 @@ obj-$(CONFIG_BCM_VK)                += bcm-vk/
>>   obj-y                                += cardreader/
>>   obj-$(CONFIG_PVPANIC)        += pvpanic/
>>   obj-$(CONFIG_UACCE)          += uacce/
>> +obj-$(CONFIG_XILINX_PUF)     += xilinx_puf.o
>>   obj-$(CONFIG_XILINX_SDFEC)   += xilinx_sdfec.o
>>   obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
>>   obj-$(CONFIG_NTSYNC)         += ntsync.o
>> diff --git a/drivers/misc/xilinx_puf.c b/drivers/misc/xilinx_puf.c
>> new file mode 100644
>> index 000000000000..ad8ada58b105
>> --- /dev/null
>> +++ b/drivers/misc/xilinx_puf.c
>> @@ -0,0 +1,377 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for Xilinx PUF device.
>> + *
>> + * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
>> + *
>> + * Description:
>> + * This driver is developed for PUF registration and regeneration support.
>> + */
>> +
>> +#include <linux/dma-mapping.h>
>> +#include <linux/firmware/xlnx-zynqmp.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <uapi/misc/xilinx_puf.h>
>> +
>> +static bool puf_clear;
>> +module_param(puf_clear, bool, 0600);
>> +MODULE_PARM_DESC(puf_clear, "Flag to enable clearing of PUF ID and key.");
> This is not the 1990's, please do not add module parameters for things
> that should be done other ways.  Especially for a device-specific thing.
>
>> +
>> +/**
>> + * struct puf_params - parameters for PUF
>> + * @pufoperation: PUF registration or regeneration operation
>> + * @globalvarfilter: global variation filter
>> + * @readoption: option to read PUF data from efuse cache or ram address
>> + * @reserved: explicit padding to match the Versal PLM PUF parameter layout.
>> + * @shuttervalue: shutter value for PUF registration/regeneration
>> + * @readsyndromeaddr: address to store the syndrome data during registration
>> + * @chashaddr: CHASH address
>> + * @auxaddr: AUX address
>> + * @pufidaddr: PUF ID address
>> + * @writesyndromeaddr: address where syndrome data is present and it is passed to the user
>> + * @trimsyndataaddr: trimmed syndrome data will be stored
>> + */
>> +struct puf_params {
>> +     u8 pufoperation;
>> +     u8 globalvarfilter;
>> +     u8 readoption;
>> +     u8 reserved;
>> +     u32 shuttervalue;
>> +     u64 readsyndromeaddr;
>> +     u64 chashaddr;
>> +     u64 auxaddr;
>> +     u64 pufidaddr;
>> +     u64 writesyndromeaddr;
>> +     u64 trimsyndataaddr;
>> +};
>> +
>> +/**
>> + * struct xpuf_dev - Driver data for PUF
>> + * @dev: pointer to device struct
>> + * @miscdev: misc device handle
>> + */
>> +struct xpuf_dev {
>> +     struct device *dev;
>> +     struct miscdevice miscdev;
>> +};
>> +
>> +static int xlnx_puf_regis(struct xpuf_dev *puf, struct puf_usrparams *pufreq)
>> +{
>> +     struct puf_params *pufin;
>> +     struct pufdata *pufdat;
>> +     dma_addr_t dma_addr_data;
>> +     dma_addr_t dma_addr_in;
>> +     u32 buflen;
>> +     void *buf;
>> +     int ret;
>> +
>> +     if (pufreq->pufoperation != PUF_REGIS)
>> +             return -EINVAL;
>> +
>> +     if (pufreq->readoption != PUF_READ_FROM_RAM &&
>> +         pufreq->readoption != PUF_READ_FROM_EFUSE_CACHE)
>> +             return -EINVAL;
>> +
>> +     buflen = sizeof(struct puf_params) + sizeof(struct pufdata);
>> +     buf = kzalloc(buflen, GFP_KERNEL);
>> +     if (!buf)
>> +             return -ENOMEM;
>> +
>> +     pufin = buf;
>> +     pufin->pufoperation = pufreq->pufoperation;
>> +     pufin->globalvarfilter = pufreq->globalvarfilter;
>> +     pufin->readoption = pufreq->readoption;
>> +     pufin->shuttervalue = pufreq->shuttervalue;
>> +
>> +     pufdat = buf + sizeof(struct puf_params);
>> +     dma_addr_in = dma_map_single(puf->dev, buf, buflen, DMA_BIDIRECTIONAL);
>> +     if (dma_mapping_error(puf->dev, dma_addr_in)) {
>> +             ret = -ENOMEM;
>> +             goto cleanup;
>> +     }
>> +
>> +     dma_addr_data = dma_addr_in + sizeof(struct puf_params);
>> +     pufin->readsyndromeaddr = (u64)dma_addr_data;
>> +     pufin->chashaddr = (u64)(pufin->readsyndromeaddr + sizeof(pufdat->pufhd.syndata));
>> +     pufin->auxaddr = (u64)(pufin->chashaddr + sizeof(pufdat->pufhd.chash));
>> +     pufin->pufidaddr = (u64)(pufin->auxaddr + sizeof(pufdat->pufhd.aux));
>> +     pufin->trimsyndataaddr = (u64)(pufin->pufidaddr + sizeof(pufdat->pufid));
>> +
>> +     dma_sync_single_for_device(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
>> +     ret = versal_pm_puf_registration(dma_addr_in);
>> +     dma_unmap_single(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
>> +     if (ret)
>> +             goto cleanup;
>> +
>> +     if (copy_to_user(u64_to_user_ptr(pufreq->pufdataaddr), pufdat, sizeof(struct pufdata))) {
>> +             ret = -EFAULT;
>> +             goto cleanup;
>> +     }
>> +
>> +cleanup:
>> +     kfree(buf);
>> +
>> +     return ret;
>> +}
>> +
>> +static int xlnx_puf_regen_id(struct xpuf_dev *puf, struct puf_usrparams *pufreq)
>> +{
>> +     struct puf_helperdata *pufhd;
>> +     struct puf_params *pufin;
>> +     dma_addr_t dma_addr_data;
>> +     dma_addr_t dma_addr_in;
>> +     u32 buflen;
>> +     void *buf;
>> +     int ret;
>> +
>> +     if (pufreq->pufoperation != PUF_REGEN &&
>> +         pufreq->pufoperation != PUF_REGEN_ID)
>> +             return -EINVAL;
>> +
>> +     if (pufreq->readoption != PUF_READ_FROM_RAM &&
>> +         pufreq->readoption != PUF_READ_FROM_EFUSE_CACHE)
>> +             return -EINVAL;
>> +
>> +     buflen = sizeof(struct puf_params) + sizeof(struct puf_helperdata) +
>> +              PUF_ID_LEN_IN_BYTES;
>> +
>> +     buf = kzalloc(buflen, GFP_KERNEL);
>> +     if (!buf)
>> +             return -ENOMEM;
>> +
>> +     pufin = buf;
>> +     pufin->pufoperation = pufreq->pufoperation;
>> +     pufin->globalvarfilter = pufreq->globalvarfilter;
>> +     pufin->shuttervalue = pufreq->shuttervalue;
>> +     pufin->readoption = pufreq->readoption;
>> +
>> +     pufhd = buf + sizeof(struct puf_params);
>> +     if (copy_from_user(pufhd, u64_to_user_ptr(pufreq->pufdataaddr),
>> +                        sizeof(struct puf_helperdata))) {
>> +             ret = -EFAULT;
>> +             goto cleanup;
>> +     }
>> +
>> +     dma_addr_in = dma_map_single(puf->dev, buf, buflen, DMA_BIDIRECTIONAL);
>> +     if (dma_mapping_error(puf->dev, dma_addr_in)) {
>> +             ret = -ENOMEM;
>> +             goto cleanup;
>> +     }
>> +
>> +     dma_addr_data = dma_addr_in + sizeof(struct puf_params);
>> +     pufin->writesyndromeaddr = (u64)dma_addr_data;
>> +     pufin->chashaddr = (u64)(pufin->writesyndromeaddr + sizeof(pufhd->syndata));
>> +     pufin->auxaddr = (u64)(pufin->chashaddr + sizeof(pufhd->chash));
>> +     pufin->pufidaddr = (u64)(pufin->auxaddr + sizeof(pufhd->aux));
>> +
>> +     dma_sync_single_for_device(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
>> +     ret = versal_pm_puf_regeneration(dma_addr_in);
>> +     dma_unmap_single(puf->dev, dma_addr_in, buflen, DMA_BIDIRECTIONAL);
>> +     if (ret)
>> +             goto cleanup;
>> +
>> +     if (copy_to_user(u64_to_user_ptr(pufreq->pufidaddr), ((char *)pufhd +
>> +                      sizeof(struct puf_helperdata)),
>> +                      PUF_ID_LEN_IN_BYTES)) {
>> +             ret = -EFAULT;
>> +             goto cleanup;
>> +     }
>> +
>> +cleanup:
>> +     kfree(buf);
>> +
>> +     return ret;
>> +}
>> +
>> +static long xlnx_puf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>> +{
>> +     struct xpuf_dev *puf = file->private_data;
>> +     struct puf_usrparams pufreq;
>> +     void __user *data = NULL;
>> +     int ret;
>> +
>> +     if (_IOC_TYPE(cmd) != PUF_IOC_MAGIC)
>> +             return -ENOTTY;
>> +
>> +     /* check if ioctl argument is present and valid */
>> +     if (_IOC_DIR(cmd) != _IOC_NONE) {
>> +             data = (void __user *)arg;
>> +             if (!data)
>> +                     return -EINVAL;
>> +
>> +             if (copy_from_user(&pufreq, data, sizeof(struct puf_usrparams)))
>> +                     return -EFAULT;
>> +     }
>> +
>> +     switch (cmd) {
>> +     case PUF_REGISTRATION:
>> +             ret = xlnx_puf_regis(puf, &pufreq);
>> +             break;
>> +     case PUF_REGENERATION:
>> +     case PUF_REGEN_ID_ONLY:
>> +             ret = xlnx_puf_regen_id(puf, &pufreq);
>> +             break;
>> +     case PUF_CLEAR_ID:
>> +             if (!puf_clear) {
>> +                     ret = -EOPNOTSUPP;
>> +                     break;
>> +             }
>> +             ret = versal_pm_puf_clear_id();
>> +             break;
>> +     case PUF_CLEAR_KEY:
>> +             if (!puf_clear) {
>> +                     ret = -EOPNOTSUPP;
>> +                     break;
>> +             }
>> +             ret = versal_pm_aes_init();
>> +             if (!ret)
>> +                     ret = versal_pm_puf_key_zero();
>> +             break;
>> +     default:
>> +             return -ENOTTY;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * xlnx_puf_open - open puf device
>> + * @inode:   inode object
>> + * @file:    file object
>> + *
>> + * Return:   0 if successful; otherwise -errno
>> + */
>> +static int xlnx_puf_open(struct inode *inode, struct file *file)
>> +{
>> +     struct xpuf_dev *xpuf;
>> +
>> +     xpuf = container_of(file->private_data, struct xpuf_dev, miscdev);
>> +     file->private_data = xpuf;
>> +
>> +     dev_dbg(xpuf->dev, "device /dev/xpuf opened\n");
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * xlnx_puf_release - release puf resources
>> + * @inode:   inode object
>> + * @file:    file object
>> + *
>> + * Return:   0 if successful; otherwise -errno
>> + */
>> +static int xlnx_puf_release(struct inode *inode, struct file *file)
>> +{
>> +     struct xpuf_dev *xpuf = file->private_data;
>> +
>> +     dev_dbg(xpuf->dev, "device /dev/xpuf closed\n");
>> +
>> +     return 0;
>> +}
> This does nothing, why is it needed at all?
>
>> +
>> +static const struct file_operations xlnx_puf_fops = {
>> +     .owner          = THIS_MODULE,
>> +     .open           = xlnx_puf_open,
>> +     .release        = xlnx_puf_release,
>> +     .unlocked_ioctl = xlnx_puf_ioctl,
>> +     .compat_ioctl   = compat_ptr_ioctl,
>> +};
>> +
>> +/**
>> + * xlnx_puf_probe - probe puf device
>> + * @pdev: Pointer to puf platform device structure
>> + *
>> + * Return: 0 if successful; otherwise -errno
>> + */
>> +static int xlnx_puf_probe(struct platform_device *pdev)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     struct xpuf_dev *xpuf;
>> +     int ret;
>> +
>> +     xpuf = devm_kzalloc(dev, sizeof(*xpuf), GFP_KERNEL);
>> +     if (!xpuf)
>> +             return -ENOMEM;
>> +
>> +     xpuf->dev = dev;
>> +
>> +     ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
>> +     if (ret < 0) {
>> +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>> +             if (ret < 0) {
>> +                     dev_err(dev, "no usable DMA configuration\n");
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     xpuf->miscdev.minor = MISC_DYNAMIC_MINOR;
>> +     xpuf->miscdev.name = "xpuf";
>> +     xpuf->miscdev.fops = &xlnx_puf_fops;
>> +     xpuf->miscdev.parent = dev;
>> +
>> +     ret = misc_register(&xpuf->miscdev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     platform_set_drvdata(pdev, xpuf);
>> +
>> +     dev_dbg(dev, "puf registered as /dev/xpuf successfully\n");
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * xlnx_puf_remove - clean up structures
>> + * @pdev:    The structure containing the device's details
>> + */
>> +static void xlnx_puf_remove(struct platform_device *pdev)
>> +{
>> +     struct xpuf_dev *xpuf = platform_get_drvdata(pdev);
>> +
>> +     misc_deregister(&xpuf->miscdev);
>> +
>> +     dev_dbg(xpuf->dev, "device /dev/xpuf removed\n");
> ftrace is your friend :)
>
>> +}
>> +
>> +static struct platform_driver xlnx_puf_drv = {
>> +     .probe = xlnx_puf_probe,
>> +     .remove = xlnx_puf_remove,
>> +     .driver = {
>> +             .name = "xlnx-puf",
>> +     },
>> +};
>> +
>> +static struct platform_device *xlnx_puf_pdev;
>> +
>> +static int __init xlnx_puf_driver_init(void)
>> +{
>> +     int ret;
>> +
>> +     ret = platform_driver_register(&xlnx_puf_drv);
>> +     if (ret)
>> +             return ret;
>> +
>> +     xlnx_puf_pdev = platform_device_register_simple(xlnx_puf_drv.driver.name,
>> +                                                     0, NULL, 0);
> Why isn't this using the faux bus instead?  This isn't a real platform
> device at all as you have no resources for it that describe it.
>
>
>
>
>> +     if (IS_ERR(xlnx_puf_pdev)) {
>> +             ret = PTR_ERR(xlnx_puf_pdev);
>> +             platform_driver_unregister(&xlnx_puf_drv);
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +static void __exit xlnx_puf_driver_exit(void)
>> +{
>> +     platform_device_unregister(xlnx_puf_pdev);
>> +     platform_driver_unregister(&xlnx_puf_drv);
>> +}
>> +
>> +module_init(xlnx_puf_driver_init);
>> +module_exit(xlnx_puf_driver_exit);
>> +
>> +MODULE_AUTHOR("AMD");
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("Xilinx Versal PUF driver");
>> diff --git a/include/uapi/misc/xilinx_puf.h b/include/uapi/misc/xilinx_puf.h
>> new file mode 100644
>> index 000000000000..32565a748df3
>> --- /dev/null
>> +++ b/include/uapi/misc/xilinx_puf.h
>> @@ -0,0 +1,82 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Driver for Xilinx PUF device.
>> + *
>> + * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
>> + *
>> + * Description:
>> + * This driver is developed for PUF registration and regeneration support.
>> + */
>> +
>> +#ifndef _UAPI_MISC_XILINX_PUF_H_
>> +#define _UAPI_MISC_XILINX_PUF_H_
>> +
>> +#include <linux/ioctl.h>
>> +#include <linux/types.h>
>> +
>> +#define PUF_MAX_SYNDROME_DATA_LEN_IN_WORDS   140
>> +#define PUF_EFUSE_TRIM_SYN_DATA_IN_WORDS     127
>> +#define PUF_ID_LEN_IN_WORDS                  8
>> +#define PUF_ID_LEN_IN_BYTES                  32
>> +#define PUF_REGIS                            0
>> +#define PUF_REGEN                            1
>> +#define PUF_REGEN_ID                         2
>> +
>> +/**
>> + * struct puf_usrparams - user parameters for PUF from user space.
>> + * @pufoperation: PUF registration or regeneration operation
>> + * @globalvarfilter: global variation filter
>> + * @readoption: option to read PUF data from efuse cache or ram address
>> + * @reserved: explicit padding to match the Versal PLM PUF parameter layout
>> + * @shuttervalue: shutter value for PUF registration/regeneration
>> + * @pufdataaddr: address to store/get the puf data during registration/regeneration
>> + * @pufidaddr: puf id will be stored either during registration/regeneration
>> + */
>> +struct puf_usrparams {
>> +     __u8 pufoperation;
>> +     __u8 globalvarfilter;
>> +     __u8 readoption;
>> +     __u8 reserved;
>> +     __u32 shuttervalue;
>> +     __u64 pufdataaddr;
>> +     __u64 pufidaddr;
>> +};
>> +
>> +/**
>> + * struct puf_helperdata - parameters for puf helper data.
>> + * @syndata: PUF syndrome data
>> + * @chash: PUF chash
>> + * @aux: PUF aux
>> + */
>> +struct puf_helperdata {
>> +     __u32 syndata[PUF_MAX_SYNDROME_DATA_LEN_IN_WORDS];
>> +     __u32 chash;
>> +     __u32 aux;
>> +};
>> +
>> +/**
>> + * struct pufdata - parameters for puf data.
>> + * @pufhd: puf helper data of type struct puf_helperdata
>> + * @pufid: PUF id
>> + * @efusesyndata: PUF efuse syndrome data
>> + */
>> +struct pufdata {
>> +     struct puf_helperdata pufhd;
>> +     __u32 pufid[PUF_ID_LEN_IN_WORDS];
>> +     __u32 efusesyndata[PUF_EFUSE_TRIM_SYN_DATA_IN_WORDS];
>> +};
>> +
>> +enum pufreadoption {
>> +     PUF_READ_FROM_RAM = 0,
>> +     PUF_READ_FROM_EFUSE_CACHE = 1
>> +};
>> +
>> +#define PUF_IOC_MAGIC 'P'
> Why are you allowed to use "P" for your ioctl?  Isn't that already used
> by other drivers?
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-07 10:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07  9:56 [PATCH 0/2] Add Versal PUF support Harsh Jain
2026-07-07  9:56 ` [PATCH 1/2] firmware: xilinx: Add Versal PUF PM API support Harsh Jain
2026-07-07  9:56 ` [PATCH 2/2] misc: Add Xilinx PUF driver Harsh Jain
2026-07-07 10:04   ` Arnd Bergmann
2026-07-07 10:41     ` Jain, Harsh (AECG-SSW)
2026-07-07 10:12   ` Greg KH
2026-07-07 10:52     ` Jain, Harsh (AECG-SSW)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox