[PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[Mario Limonciello]

Some Platforms ship BIOS with either a default or an alternative trust
authority. The PSP validates the NPU firmware, so it must be signed with
the matching scheme. Firmware for the alternative scheme is named with
an "_l" suffix before the extension. A preproduction part needs firmware
of its own, named with a "_p" suffix.

The PSP reports the signing type in bit 6 of C2PMSG_63, and in bit 28
whether this is a preproduction part. A preproduction part that also
reports the alternative scheme is not expected, so warn about it and
use the default firmware.

Add MODULE_FIRMWARE() entries for the "_l" variants of the npu and cert
images of the aie4 devices so that initramfs generators include them.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
Changes in v2:
- Initialize val in aie4_psp_fw_variant() so a failed SMN read can't leave
  it uninitialized
- Only read the PSP firmware variant after the npufw/certfw path check, so
  VFs (which have no firmware paths) skip the SMN access
- Drop the redundant all-ones check, amd_smn_read() already returns -ENODEV
- Link to v1: https://patch.msgid.link/20261104-superm1-lms-npu-v1-1-dbffcc5abc23@amd.com
---
 drivers/accel/amdxdna/Kconfig           |   1 +
 drivers/accel/amdxdna/aie4_pci.c        | 162 ++++++++++++++++++++++++++------
 drivers/accel/amdxdna/amdxdna_pci_drv.c |   8 ++
 3 files changed, 141 insertions(+), 30 deletions(-)

diff --git a/drivers/accel/amdxdna/Kconfig b/drivers/accel/amdxdna/Kconfig
index eddff490b4f75..cd17a89cf4146 100644
--- a/drivers/accel/amdxdna/Kconfig
+++ b/drivers/accel/amdxdna/Kconfig
@@ -6,6 +6,7 @@ config DRM_ACCEL_AMDXDNA
 	depends on DRM_ACCEL
 	depends on PCI && HAS_IOMEM
 	depends on X86_64
+	depends on AMD_NODE
 	depends on AMD_PMF || !AMD_PMF
 	select DRM_SCHED
 	select DRM_GEM_SHMEM_HELPER
diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/aie4_pci.c
index 02dcb2f813d45..26c6162893760 100644
--- a/drivers/accel/amdxdna/aie4_pci.c
+++ b/drivers/accel/amdxdna/aie4_pci.c
@@ -11,6 +11,8 @@
 #include <linux/debugfs.h>
 #include <linux/firmware.h>
 #include <linux/sizes.h>
+#include <linux/string.h>
+#include <asm/amd/node.h>
 
 #include "aie.h"
 #include "aie4_msg_priv.h"
@@ -558,46 +560,124 @@ static void aie4_classic_hw_stop(struct amdxdna_dev_hdl *ndev)
 	aie4_fw_stop(ndev);
 }
 
-static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
-				 const struct firmware **npufw,
-				 const struct firmware **certfw)
+/*
+ * PSP C2PMSG_63 (SMN) bit 6: 0 = default firmware signing, 1 = alternative
+ * PSP C2PMSG_63 (SMN) bit 28: 0 = production part, 1 = preproduction part
+ */
+#define AIE4_PSP_C2PMSG_63_SMN_ADDR	0x38109fc
+#define AIE4_PSP_FW_SIGN_ALT		BIT(6)
+#define AIE4_PSP_FW_PREPROD		BIT(28)
+
+enum aie4_fw_variant {
+	AIE4_FW_DEFAULT,
+	AIE4_FW_ALT,		/* "_l" images */
+	AIE4_FW_PREPROD,	/* "_p" images */
+};
+
+/*
+ * The PSP only trusts firmware signed with the scheme the platform firmware
+ * uses and a preproduction part needs firmware of its own. Reading which one is
+ * in use is optional and only used as a hint for which images to try first:
+ * anything but a clear answer is treated as default.
+ */
+static enum aie4_fw_variant aie4_psp_fw_variant(struct amdxdna_dev *xdna)
 {
-	struct amdxdna_dev *xdna = ndev->aie.xdna;
-	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
-	char fw_name[128];
+	u32 val = 0;
 	int ret;
 
-	ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
-		       pdev->device, pdev->revision, ndev->priv->npufw_path);
-	if (ret >= sizeof(fw_name)) {
-		XDNA_ERR(xdna, "npu firmware path is truncated");
-		return -EINVAL;
+	ret = amd_smn_read(0, AIE4_PSP_C2PMSG_63_SMN_ADDR, &val);
+	if (ret) {
+		XDNA_DBG(xdna, "PSP firmware variant not available, ret %d", ret);
+		return AIE4_FW_DEFAULT;
 	}
 
-	ret = request_firmware(npufw, fw_name, &pdev->dev);
-	if (ret) {
-		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", fw_name, ret);
-		return ret;
+	if ((val & AIE4_PSP_FW_SIGN_ALT) && (val & AIE4_PSP_FW_PREPROD)) {
+		XDNA_WARN(xdna,
+			  "Preproduction part with alternative signing, using default firmware");
+		return AIE4_FW_DEFAULT;
 	}
 
-	ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
-		       pdev->device, pdev->revision, ndev->priv->certfw_path);
-	if (ret >= sizeof(fw_name)) {
-		XDNA_ERR(xdna, "cert firmware path is truncated");
-		ret = -EINVAL;
-		goto release_npufw;
+	if (val & AIE4_PSP_FW_PREPROD)
+		return AIE4_FW_PREPROD;
+
+	return (val & AIE4_PSP_FW_SIGN_ALT) ? AIE4_FW_ALT : AIE4_FW_DEFAULT;
+}
+
+/*
+ * Images for the other variants have a suffix before the ".sbin" extension.
+ * Returns -ENOENT if @path has no such variant.
+ */
+static int aie4_fw_name(struct amdxdna_dev *xdna, char *name, size_t size,
+			const char *path, enum aie4_fw_variant variant)
+{
+	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
+	const char *suffix;
+	size_t len, base;
+	int ret;
+
+	ret = snprintf(name, size, "amdnpu/%04x_%02x/%s",
+		       pdev->device, pdev->revision, path);
+	if (ret >= size) {
+		XDNA_ERR(xdna, "firmware path %s is truncated", path);
+		return -EINVAL;
 	}
 
-	ret = request_firmware(certfw, fw_name, &pdev->dev);
-	if (ret) {
-		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", fw_name, ret);
-		goto release_npufw;
+	switch (variant) {
+	case AIE4_FW_ALT:
+		suffix = "_l.sbin";
+		break;
+	case AIE4_FW_PREPROD:
+		suffix = "_p.sbin";
+		break;
+	default:
+		return 0;
 	}
 
+	len = ret;
+	if (len < strlen(".sbin") || strcmp(name + len - strlen(".sbin"), ".sbin"))
+		return -ENOENT;
+
+	base = len - strlen(".sbin");
+	if (strscpy(name + base, suffix, size - base) < 0)
+		return -EOVERFLOW;
+
 	return 0;
+}
+
+static int aie4_request_one(struct amdxdna_dev_hdl *ndev, const struct firmware **fw,
+			    const char *path, enum aie4_fw_variant variant)
+{
+	struct amdxdna_dev *xdna = ndev->aie.xdna;
+	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
+	char name[128];
+	int ret;
+
+	ret = aie4_fw_name(xdna, name, sizeof(name), path, variant);
+	if (ret)
+		return ret;
 
-release_npufw:
-	release_firmware(*npufw);
+	ret = firmware_request_nowarn(fw, name, &pdev->dev);
+	if (ret)
+		XDNA_DBG(xdna, "firmware %s not loaded, ret %d", name, ret);
+
+	return ret;
+}
+
+/* The NPU and cert images are validated together, so both use the same variant */
+static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
+				 const struct firmware **npufw,
+				 const struct firmware **certfw,
+				 enum aie4_fw_variant variant)
+{
+	int ret;
+
+	ret = aie4_request_one(ndev, npufw, ndev->priv->npufw_path, variant);
+	if (ret)
+		return ret;
+
+	ret = aie4_request_one(ndev, certfw, ndev->priv->certfw_path, variant);
+	if (ret)
+		release_firmware(*npufw);
 
 	return ret;
 }
@@ -645,18 +725,40 @@ static int aie4_prepare_firmware(struct amdxdna_dev_hdl *ndev,
 	return 0;
 }
 
+/*
+ * The firmware can only be started once. It is the PSP that decides whether it
+ * accepts the images, and it does that when the firmware is started, so there
+ * is no trying a variant and falling back. Pick the variant that the PSP
+ * reported (the default one if it can't be read) and use the others only if
+ * its images are missing.
+ */
 static int aie4_load_fw(struct amdxdna_dev_hdl *ndev,
 			void __iomem *tbl[PCI_NUM_RESOURCES])
 {
+	struct amdxdna_dev *xdna = ndev->aie.xdna;
 	const struct firmware *npufw, *certfw;
-	int ret;
+	enum aie4_fw_variant order[3], hint;
+	int i, n = 0, ret = -ENOENT;
 
 	if (!ndev->priv->npufw_path && !ndev->priv->certfw_path)
 		return 0;
 
-	ret = aie4_request_firmware(ndev, &npufw, &certfw);
-	if (ret)
+	hint = aie4_psp_fw_variant(xdna);
+	order[n++] = hint;
+	if (hint != AIE4_FW_DEFAULT)
+		order[n++] = AIE4_FW_DEFAULT;
+	if (hint != AIE4_FW_ALT)
+		order[n++] = AIE4_FW_ALT;
+
+	for (i = 0; i < n; i++) {
+		ret = aie4_request_firmware(ndev, &npufw, &certfw, order[i]);
+		if (!ret)
+			break;
+	}
+	if (ret) {
+		XDNA_ERR(xdna, "failed to load firmware, ret %d", ret);
 		return ret;
+	}
 
 	ret = aie4_prepare_firmware(ndev, npufw, certfw, tbl);
 	aie4_release_firmware(ndev, npufw, certfw);
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index f233eb70a2957..dab8603e2381f 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -37,6 +37,14 @@ MODULE_FIRMWARE("amdnpu/17f1_13/npu.sbin");
 MODULE_FIRMWARE("amdnpu/17f1_13/cert.sbin");
 MODULE_FIRMWARE("amdnpu/17f2_13/npu.sbin");
 MODULE_FIRMWARE("amdnpu/17f2_13/cert.sbin");
+MODULE_FIRMWARE("amdnpu/17f1_10/npu_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f1_10/cert_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f2_10/npu_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f2_10/cert_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f1_13/npu_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f1_13/cert_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f2_13/npu_l.sbin");
+MODULE_FIRMWARE("amdnpu/17f2_13/cert_l.sbin");
 
 /*
  * 0.0: Initial version

---
base-commit: e32e83a6c5ec031a7f59d4794113d8484b220254
change-id: 20261104-superm1-lms-npu-6bc3bbe38c29

Best regards,
--  
Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[Lizhi Hou]

On 11/5/26 07:51, Mario Limonciello wrote:
> Some Platforms ship BIOS with either a default or an alternative trust
> authority. The PSP validates the NPU firmware, so it must be signed with
> the matching scheme. Firmware for the alternative scheme is named with
> an "_l" suffix before the extension. A preproduction part needs firmware
> of its own, named with a "_p" suffix.
>
> The PSP reports the signing type in bit 6 of C2PMSG_63, and in bit 28
> whether this is a preproduction part. A preproduction part that also
> reports the alternative scheme is not expected, so warn about it and
> use the default firmware.
>
> Add MODULE_FIRMWARE() entries for the "_l" variants of the npu and cert
> images of the aie4 devices so that initramfs generators include them.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Changes in v2:
> - Initialize val in aie4_psp_fw_variant() so a failed SMN read can't leave
>    it uninitialized
> - Only read the PSP firmware variant after the npufw/certfw path check, so
>    VFs (which have no firmware paths) skip the SMN access
> - Drop the redundant all-ones check, amd_smn_read() already returns -ENODEV
> - Link to v1: https://patch.msgid.link/20261104-superm1-lms-npu-v1-1-dbffcc5abc23@amd.com
> ---
>   drivers/accel/amdxdna/Kconfig           |   1 +
>   drivers/accel/amdxdna/aie4_pci.c        | 162 ++++++++++++++++++++++++++------
>   drivers/accel/amdxdna/amdxdna_pci_drv.c |   8 ++
>   3 files changed, 141 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/Kconfig b/drivers/accel/amdxdna/Kconfig
> index eddff490b4f75..cd17a89cf4146 100644
> --- a/drivers/accel/amdxdna/Kconfig
> +++ b/drivers/accel/amdxdna/Kconfig
> @@ -6,6 +6,7 @@ config DRM_ACCEL_AMDXDNA
>   	depends on DRM_ACCEL
>   	depends on PCI && HAS_IOMEM
>   	depends on X86_64
> +	depends on AMD_NODE
>   	depends on AMD_PMF || !AMD_PMF
>   	select DRM_SCHED
>   	select DRM_GEM_SHMEM_HELPER
> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/aie4_pci.c
> index 02dcb2f813d45..26c6162893760 100644
> --- a/drivers/accel/amdxdna/aie4_pci.c
> +++ b/drivers/accel/amdxdna/aie4_pci.c
> @@ -11,6 +11,8 @@
>   #include <linux/debugfs.h>
>   #include <linux/firmware.h>
>   #include <linux/sizes.h>
> +#include <linux/string.h>
> +#include <asm/amd/node.h>
>   
>   #include "aie.h"
>   #include "aie4_msg_priv.h"
> @@ -558,46 +560,124 @@ static void aie4_classic_hw_stop(struct amdxdna_dev_hdl *ndev)
>   	aie4_fw_stop(ndev);
>   }
>   
> -static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
> -				 const struct firmware **npufw,
> -				 const struct firmware **certfw)
> +/*
> + * PSP C2PMSG_63 (SMN) bit 6: 0 = default firmware signing, 1 = alternative
> + * PSP C2PMSG_63 (SMN) bit 28: 0 = production part, 1 = preproduction part
> + */
> +#define AIE4_PSP_C2PMSG_63_SMN_ADDR	0x38109fc
> +#define AIE4_PSP_FW_SIGN_ALT		BIT(6)
> +#define AIE4_PSP_FW_PREPROD		BIT(28)
> +
> +enum aie4_fw_variant {
> +	AIE4_FW_DEFAULT,
> +	AIE4_FW_ALT,		/* "_l" images */
> +	AIE4_FW_PREPROD,	/* "_p" images */
> +};
> +
> +/*
> + * The PSP only trusts firmware signed with the scheme the platform firmware
> + * uses and a preproduction part needs firmware of its own. Reading which one is
> + * in use is optional and only used as a hint for which images to try first:
> + * anything but a clear answer is treated as default.
> + */
> +static enum aie4_fw_variant aie4_psp_fw_variant(struct amdxdna_dev *xdna)
>   {
> -	struct amdxdna_dev *xdna = ndev->aie.xdna;
> -	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> -	char fw_name[128];
> +	u32 val = 0;
>   	int ret;
>   
> -	ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
> -		       pdev->device, pdev->revision, ndev->priv->npufw_path);
> -	if (ret >= sizeof(fw_name)) {
> -		XDNA_ERR(xdna, "npu firmware path is truncated");
> -		return -EINVAL;
> +	ret = amd_smn_read(0, AIE4_PSP_C2PMSG_63_SMN_ADDR, &val);
> +	if (ret) {
> +		XDNA_DBG(xdna, "PSP firmware variant not available, ret %d", ret);
> +		return AIE4_FW_DEFAULT;
>   	}
>   
> -	ret = request_firmware(npufw, fw_name, &pdev->dev);
> -	if (ret) {
> -		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", fw_name, ret);
> -		return ret;
> +	if ((val & AIE4_PSP_FW_SIGN_ALT) && (val & AIE4_PSP_FW_PREPROD)) {
> +		XDNA_WARN(xdna,
> +			  "Preproduction part with alternative signing, using default firmware");
> +		return AIE4_FW_DEFAULT;
>   	}
>   
> -	ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
> -		       pdev->device, pdev->revision, ndev->priv->certfw_path);
> -	if (ret >= sizeof(fw_name)) {
> -		XDNA_ERR(xdna, "cert firmware path is truncated");
> -		ret = -EINVAL;
> -		goto release_npufw;
> +	if (val & AIE4_PSP_FW_PREPROD)
> +		return AIE4_FW_PREPROD;
> +
> +	return (val & AIE4_PSP_FW_SIGN_ALT) ? AIE4_FW_ALT : AIE4_FW_DEFAULT;
> +}
> +
> +/*
> + * Images for the other variants have a suffix before the ".sbin" extension.
> + * Returns -ENOENT if @path has no such variant.
> + */
> +static int aie4_fw_name(struct amdxdna_dev *xdna, char *name, size_t size,
> +			const char *path, enum aie4_fw_variant variant)
> +{
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	const char *suffix;
> +	size_t len, base;
> +	int ret;
> +
> +	ret = snprintf(name, size, "amdnpu/%04x_%02x/%s",
> +		       pdev->device, pdev->revision, path);
> +	if (ret >= size) {
> +		XDNA_ERR(xdna, "firmware path %s is truncated", path);
> +		return -EINVAL;
>   	}
>   
> -	ret = request_firmware(certfw, fw_name, &pdev->dev);
> -	if (ret) {
> -		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", fw_name, ret);
> -		goto release_npufw;
> +	switch (variant) {
> +	case AIE4_FW_ALT:
> +		suffix = "_l.sbin";
> +		break;
> +	case AIE4_FW_PREPROD:
> +		suffix = "_p.sbin";
> +		break;
> +	default:
> +		return 0;
>   	}
>   
> +	len = ret;
> +	if (len < strlen(".sbin") || strcmp(name + len - strlen(".sbin"), ".sbin"))
> +		return -ENOENT;
> +
> +	base = len - strlen(".sbin");
> +	if (strscpy(name + base, suffix, size - base) < 0)
> +		return -EOVERFLOW;
> +
>   	return 0;
> +}
> +
> +static int aie4_request_one(struct amdxdna_dev_hdl *ndev, const struct firmware **fw,
> +			    const char *path, enum aie4_fw_variant variant)
> +{
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	char name[128];
> +	int ret;
> +
> +	ret = aie4_fw_name(xdna, name, sizeof(name), path, variant);
> +	if (ret)
> +		return ret;
>   
> -release_npufw:
> -	release_firmware(*npufw);
> +	ret = firmware_request_nowarn(fw, name, &pdev->dev);
> +	if (ret)
> +		XDNA_DBG(xdna, "firmware %s not loaded, ret %d", name, ret);
> +
> +	return ret;
> +}
> +
> +/* The NPU and cert images are validated together, so both use the same variant */
> +static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
> +				 const struct firmware **npufw,
> +				 const struct firmware **certfw,
> +				 enum aie4_fw_variant variant)
> +{
> +	int ret;
> +
> +	ret = aie4_request_one(ndev, npufw, ndev->priv->npufw_path, variant);
> +	if (ret)
> +		return ret;
> +
> +	ret = aie4_request_one(ndev, certfw, ndev->priv->certfw_path, variant);
> +	if (ret)
> +		release_firmware(*npufw);
>   
>   	return ret;
>   }
> @@ -645,18 +725,40 @@ static int aie4_prepare_firmware(struct amdxdna_dev_hdl *ndev,
>   	return 0;
>   }
>   
> +/*
> + * The firmware can only be started once. It is the PSP that decides whether it
> + * accepts the images, and it does that when the firmware is started, so there
> + * is no trying a variant and falling back. Pick the variant that the PSP
> + * reported (the default one if it can't be read) and use the others only if
> + * its images are missing.
> + */
>   static int aie4_load_fw(struct amdxdna_dev_hdl *ndev,
>   			void __iomem *tbl[PCI_NUM_RESOURCES])
>   {
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
>   	const struct firmware *npufw, *certfw;
> -	int ret;
> +	enum aie4_fw_variant order[3], hint;
> +	int i, n = 0, ret = -ENOENT;
>   
>   	if (!ndev->priv->npufw_path && !ndev->priv->certfw_path)
>   		return 0;
>   
> -	ret = aie4_request_firmware(ndev, &npufw, &certfw);
> -	if (ret)
> +	hint = aie4_psp_fw_variant(xdna);
> +	order[n++] = hint;
> +	if (hint != AIE4_FW_DEFAULT)
> +		order[n++] = AIE4_FW_DEFAULT;
> +	if (hint != AIE4_FW_ALT)
> +		order[n++] = AIE4_FW_ALT;
> +
> +	for (i = 0; i < n; i++) {
> +		ret = aie4_request_firmware(ndev, &npufw, &certfw, order[i]);
> +		if (!ret)
> +			break;
> +	}
> +	if (ret) {
> +		XDNA_ERR(xdna, "failed to load firmware, ret %d", ret);
>   		return ret;
> +	}
>   
>   	ret = aie4_prepare_firmware(ndev, npufw, certfw, tbl);
>   	aie4_release_firmware(ndev, npufw, certfw);
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index f233eb70a2957..dab8603e2381f 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -37,6 +37,14 @@ MODULE_FIRMWARE("amdnpu/17f1_13/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f1_13/cert.sbin");
>   MODULE_FIRMWARE("amdnpu/17f2_13/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f2_13/cert.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_10/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_10/cert_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_10/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_10/cert_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_13/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_13/cert_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_13/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_13/cert_l.sbin");
need to add entries for "_p" images.
>   
>   /*
>    * 0.0: Initial version
>
> ---
> base-commit: e32e83a6c5ec031a7f59d4794113d8484b220254
> change-id: 20261104-superm1-lms-npu-6bc3bbe38c29
>
> Best regards,
> --
> Mario Limonciello <mario.limonciello@amd.com>
>

Re: [PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[Mario Limonciello]

On 11/5/26 11:47, Lizhi Hou wrote:
> 
> On 11/5/26 07:51, Mario Limonciello wrote:
>> Some Platforms ship BIOS with either a default or an alternative trust
>> authority. The PSP validates the NPU firmware, so it must be signed with
>> the matching scheme. Firmware for the alternative scheme is named with
>> an "_l" suffix before the extension. A preproduction part needs firmware
>> of its own, named with a "_p" suffix.
>>
>> The PSP reports the signing type in bit 6 of C2PMSG_63, and in bit 28
>> whether this is a preproduction part. A preproduction part that also
>> reports the alternative scheme is not expected, so warn about it and
>> use the default firmware.
>>
>> Add MODULE_FIRMWARE() entries for the "_l" variants of the npu and cert
>> images of the aie4 devices so that initramfs generators include them.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> Changes in v2:
>> - Initialize val in aie4_psp_fw_variant() so a failed SMN read can't 
>> leave
>>    it uninitialized
>> - Only read the PSP firmware variant after the npufw/certfw path 
>> check, so
>>    VFs (which have no firmware paths) skip the SMN access
>> - Drop the redundant all-ones check, amd_smn_read() already returns - 
>> ENODEV
>> - Link to v1: https://patch.msgid.link/20261104-superm1-lms-npu-v1-1- 
>> dbffcc5abc23@amd.com
>> ---
>>   drivers/accel/amdxdna/Kconfig           |   1 +
>>   drivers/accel/amdxdna/aie4_pci.c        | 162 ++++++++++++++++++++++ 
>> ++++------
>>   drivers/accel/amdxdna/amdxdna_pci_drv.c |   8 ++
>>   3 files changed, 141 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/Kconfig b/drivers/accel/amdxdna/ 
>> Kconfig
>> index eddff490b4f75..cd17a89cf4146 100644
>> --- a/drivers/accel/amdxdna/Kconfig
>> +++ b/drivers/accel/amdxdna/Kconfig
>> @@ -6,6 +6,7 @@ config DRM_ACCEL_AMDXDNA
>>       depends on DRM_ACCEL
>>       depends on PCI && HAS_IOMEM
>>       depends on X86_64
>> +    depends on AMD_NODE
>>       depends on AMD_PMF || !AMD_PMF
>>       select DRM_SCHED
>>       select DRM_GEM_SHMEM_HELPER
>> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/ 
>> aie4_pci.c
>> index 02dcb2f813d45..26c6162893760 100644
>> --- a/drivers/accel/amdxdna/aie4_pci.c
>> +++ b/drivers/accel/amdxdna/aie4_pci.c
>> @@ -11,6 +11,8 @@
>>   #include <linux/debugfs.h>
>>   #include <linux/firmware.h>
>>   #include <linux/sizes.h>
>> +#include <linux/string.h>
>> +#include <asm/amd/node.h>
>>   #include "aie.h"
>>   #include "aie4_msg_priv.h"
>> @@ -558,46 +560,124 @@ static void aie4_classic_hw_stop(struct 
>> amdxdna_dev_hdl *ndev)
>>       aie4_fw_stop(ndev);
>>   }
>> -static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
>> -                 const struct firmware **npufw,
>> -                 const struct firmware **certfw)
>> +/*
>> + * PSP C2PMSG_63 (SMN) bit 6: 0 = default firmware signing, 1 = 
>> alternative
>> + * PSP C2PMSG_63 (SMN) bit 28: 0 = production part, 1 = preproduction 
>> part
>> + */
>> +#define AIE4_PSP_C2PMSG_63_SMN_ADDR    0x38109fc
>> +#define AIE4_PSP_FW_SIGN_ALT        BIT(6)
>> +#define AIE4_PSP_FW_PREPROD        BIT(28)
>> +
>> +enum aie4_fw_variant {
>> +    AIE4_FW_DEFAULT,
>> +    AIE4_FW_ALT,        /* "_l" images */
>> +    AIE4_FW_PREPROD,    /* "_p" images */
>> +};
>> +
>> +/*
>> + * The PSP only trusts firmware signed with the scheme the platform 
>> firmware
>> + * uses and a preproduction part needs firmware of its own. Reading 
>> which one is
>> + * in use is optional and only used as a hint for which images to try 
>> first:
>> + * anything but a clear answer is treated as default.
>> + */
>> +static enum aie4_fw_variant aie4_psp_fw_variant(struct amdxdna_dev 
>> *xdna)
>>   {
>> -    struct amdxdna_dev *xdna = ndev->aie.xdna;
>> -    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>> -    char fw_name[128];
>> +    u32 val = 0;
>>       int ret;
>> -    ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
>> -               pdev->device, pdev->revision, ndev->priv->npufw_path);
>> -    if (ret >= sizeof(fw_name)) {
>> -        XDNA_ERR(xdna, "npu firmware path is truncated");
>> -        return -EINVAL;
>> +    ret = amd_smn_read(0, AIE4_PSP_C2PMSG_63_SMN_ADDR, &val);
>> +    if (ret) {
>> +        XDNA_DBG(xdna, "PSP firmware variant not available, ret %d", 
>> ret);
>> +        return AIE4_FW_DEFAULT;
>>       }
>> -    ret = request_firmware(npufw, fw_name, &pdev->dev);
>> -    if (ret) {
>> -        XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", 
>> fw_name, ret);
>> -        return ret;
>> +    if ((val & AIE4_PSP_FW_SIGN_ALT) && (val & AIE4_PSP_FW_PREPROD)) {
>> +        XDNA_WARN(xdna,
>> +              "Preproduction part with alternative signing, using 
>> default firmware");
>> +        return AIE4_FW_DEFAULT;
>>       }
>> -    ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
>> -               pdev->device, pdev->revision, ndev->priv->certfw_path);
>> -    if (ret >= sizeof(fw_name)) {
>> -        XDNA_ERR(xdna, "cert firmware path is truncated");
>> -        ret = -EINVAL;
>> -        goto release_npufw;
>> +    if (val & AIE4_PSP_FW_PREPROD)
>> +        return AIE4_FW_PREPROD;
>> +
>> +    return (val & AIE4_PSP_FW_SIGN_ALT) ? AIE4_FW_ALT : AIE4_FW_DEFAULT;
>> +}
>> +
>> +/*
>> + * Images for the other variants have a suffix before the ".sbin" 
>> extension.
>> + * Returns -ENOENT if @path has no such variant.
>> + */
>> +static int aie4_fw_name(struct amdxdna_dev *xdna, char *name, size_t 
>> size,
>> +            const char *path, enum aie4_fw_variant variant)
>> +{
>> +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>> +    const char *suffix;
>> +    size_t len, base;
>> +    int ret;
>> +
>> +    ret = snprintf(name, size, "amdnpu/%04x_%02x/%s",
>> +               pdev->device, pdev->revision, path);
>> +    if (ret >= size) {
>> +        XDNA_ERR(xdna, "firmware path %s is truncated", path);
>> +        return -EINVAL;
>>       }
>> -    ret = request_firmware(certfw, fw_name, &pdev->dev);
>> -    if (ret) {
>> -        XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", 
>> fw_name, ret);
>> -        goto release_npufw;
>> +    switch (variant) {
>> +    case AIE4_FW_ALT:
>> +        suffix = "_l.sbin";
>> +        break;
>> +    case AIE4_FW_PREPROD:
>> +        suffix = "_p.sbin";
>> +        break;
>> +    default:
>> +        return 0;
>>       }
>> +    len = ret;
>> +    if (len < strlen(".sbin") || strcmp(name + len - strlen(".sbin"), 
>> ".sbin"))
>> +        return -ENOENT;
>> +
>> +    base = len - strlen(".sbin");
>> +    if (strscpy(name + base, suffix, size - base) < 0)
>> +        return -EOVERFLOW;
>> +
>>       return 0;
>> +}
>> +
>> +static int aie4_request_one(struct amdxdna_dev_hdl *ndev, const 
>> struct firmware **fw,
>> +                const char *path, enum aie4_fw_variant variant)
>> +{
>> +    struct amdxdna_dev *xdna = ndev->aie.xdna;
>> +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>> +    char name[128];
>> +    int ret;
>> +
>> +    ret = aie4_fw_name(xdna, name, sizeof(name), path, variant);
>> +    if (ret)
>> +        return ret;
>> -release_npufw:
>> -    release_firmware(*npufw);
>> +    ret = firmware_request_nowarn(fw, name, &pdev->dev);
>> +    if (ret)
>> +        XDNA_DBG(xdna, "firmware %s not loaded, ret %d", name, ret);
>> +
>> +    return ret;
>> +}
>> +
>> +/* The NPU and cert images are validated together, so both use the 
>> same variant */
>> +static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
>> +                 const struct firmware **npufw,
>> +                 const struct firmware **certfw,
>> +                 enum aie4_fw_variant variant)
>> +{
>> +    int ret;
>> +
>> +    ret = aie4_request_one(ndev, npufw, ndev->priv->npufw_path, 
>> variant);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = aie4_request_one(ndev, certfw, ndev->priv->certfw_path, 
>> variant);
>> +    if (ret)
>> +        release_firmware(*npufw);
>>       return ret;
>>   }
>> @@ -645,18 +725,40 @@ static int aie4_prepare_firmware(struct 
>> amdxdna_dev_hdl *ndev,
>>       return 0;
>>   }
>> +/*
>> + * The firmware can only be started once. It is the PSP that decides 
>> whether it
>> + * accepts the images, and it does that when the firmware is started, 
>> so there
>> + * is no trying a variant and falling back. Pick the variant that the 
>> PSP
>> + * reported (the default one if it can't be read) and use the others 
>> only if
>> + * its images are missing.
>> + */
>>   static int aie4_load_fw(struct amdxdna_dev_hdl *ndev,
>>               void __iomem *tbl[PCI_NUM_RESOURCES])
>>   {
>> +    struct amdxdna_dev *xdna = ndev->aie.xdna;
>>       const struct firmware *npufw, *certfw;
>> -    int ret;
>> +    enum aie4_fw_variant order[3], hint;
>> +    int i, n = 0, ret = -ENOENT;
>>       if (!ndev->priv->npufw_path && !ndev->priv->certfw_path)
>>           return 0;
>> -    ret = aie4_request_firmware(ndev, &npufw, &certfw);
>> -    if (ret)
>> +    hint = aie4_psp_fw_variant(xdna);
>> +    order[n++] = hint;
>> +    if (hint != AIE4_FW_DEFAULT)
>> +        order[n++] = AIE4_FW_DEFAULT;
>> +    if (hint != AIE4_FW_ALT)
>> +        order[n++] = AIE4_FW_ALT;
>> +
>> +    for (i = 0; i < n; i++) {
>> +        ret = aie4_request_firmware(ndev, &npufw, &certfw, order[i]);
>> +        if (!ret)
>> +            break;
>> +    }
>> +    if (ret) {
>> +        XDNA_ERR(xdna, "failed to load firmware, ret %d", ret);
>>           return ret;
>> +    }
>>       ret = aie4_prepare_firmware(ndev, npufw, certfw, tbl);
>>       aie4_release_firmware(ndev, npufw, certfw);
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/ 
>> amdxdna/amdxdna_pci_drv.c
>> index f233eb70a2957..dab8603e2381f 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> @@ -37,6 +37,14 @@ MODULE_FIRMWARE("amdnpu/17f1_13/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f1_13/cert.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f2_13/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f2_13/cert.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_10/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_10/cert_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_10/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_10/cert_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_13/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_13/cert_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_13/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_13/cert_l.sbin");
> need to add entries for "_p" images.

Let me tell you my thought process in excluding them.

They are only for pre-prod hardware, and so *most people* who use this 
driver after the product launches shouldn't need them in their 
initramfs; they'll just bloat the initramfs.

If you think they should still be there; I'll spin a v3 an add them.

>>   /*
>>    * 0.0: Initial version
>>
>> ---
>> base-commit: e32e83a6c5ec031a7f59d4794113d8484b220254
>> change-id: 20261104-superm1-lms-npu-6bc3bbe38c29
>>
>> Best regards,
>> -- 
>> Mario Limonciello <mario.limonciello@amd.com>
>>

Re: [PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[Lizhi Hou]

On 11/5/26 09:52, Mario Limonciello wrote:
>
>
> On 11/5/26 11:47, Lizhi Hou wrote:
>>
>> On 11/5/26 07:51, Mario Limonciello wrote:
>>> Some Platforms ship BIOS with either a default or an alternative trust
>>> authority. The PSP validates the NPU firmware, so it must be signed 
>>> with
>>> the matching scheme. Firmware for the alternative scheme is named with
>>> an "_l" suffix before the extension. A preproduction part needs 
>>> firmware
>>> of its own, named with a "_p" suffix.
>>>
>>> The PSP reports the signing type in bit 6 of C2PMSG_63, and in bit 28
>>> whether this is a preproduction part. A preproduction part that also
>>> reports the alternative scheme is not expected, so warn about it and
>>> use the default firmware.
>>>
>>> Add MODULE_FIRMWARE() entries for the "_l" variants of the npu and cert
>>> images of the aie4 devices so that initramfs generators include them.
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>> Changes in v2:
>>> - Initialize val in aie4_psp_fw_variant() so a failed SMN read can't 
>>> leave
>>>    it uninitialized
>>> - Only read the PSP firmware variant after the npufw/certfw path 
>>> check, so
>>>    VFs (which have no firmware paths) skip the SMN access
>>> - Drop the redundant all-ones check, amd_smn_read() already returns 
>>> - ENODEV
>>> - Link to v1: 
>>> https://patch.msgid.link/20261104-superm1-lms-npu-v1-1- 
>>> dbffcc5abc23@amd.com
>>> ---
>>>   drivers/accel/amdxdna/Kconfig           |   1 +
>>>   drivers/accel/amdxdna/aie4_pci.c        | 162 
>>> ++++++++++++++++++++++ ++++------
>>>   drivers/accel/amdxdna/amdxdna_pci_drv.c |   8 ++
>>>   3 files changed, 141 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/accel/amdxdna/Kconfig b/drivers/accel/amdxdna/ 
>>> Kconfig
>>> index eddff490b4f75..cd17a89cf4146 100644
>>> --- a/drivers/accel/amdxdna/Kconfig
>>> +++ b/drivers/accel/amdxdna/Kconfig
>>> @@ -6,6 +6,7 @@ config DRM_ACCEL_AMDXDNA
>>>       depends on DRM_ACCEL
>>>       depends on PCI && HAS_IOMEM
>>>       depends on X86_64
>>> +    depends on AMD_NODE
>>>       depends on AMD_PMF || !AMD_PMF
>>>       select DRM_SCHED
>>>       select DRM_GEM_SHMEM_HELPER
>>> diff --git a/drivers/accel/amdxdna/aie4_pci.c 
>>> b/drivers/accel/amdxdna/ aie4_pci.c
>>> index 02dcb2f813d45..26c6162893760 100644
>>> --- a/drivers/accel/amdxdna/aie4_pci.c
>>> +++ b/drivers/accel/amdxdna/aie4_pci.c
>>> @@ -11,6 +11,8 @@
>>>   #include <linux/debugfs.h>
>>>   #include <linux/firmware.h>
>>>   #include <linux/sizes.h>
>>> +#include <linux/string.h>
>>> +#include <asm/amd/node.h>
>>>   #include "aie.h"
>>>   #include "aie4_msg_priv.h"
>>> @@ -558,46 +560,124 @@ static void aie4_classic_hw_stop(struct 
>>> amdxdna_dev_hdl *ndev)
>>>       aie4_fw_stop(ndev);
>>>   }
>>> -static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
>>> -                 const struct firmware **npufw,
>>> -                 const struct firmware **certfw)
>>> +/*
>>> + * PSP C2PMSG_63 (SMN) bit 6: 0 = default firmware signing, 1 = 
>>> alternative
>>> + * PSP C2PMSG_63 (SMN) bit 28: 0 = production part, 1 = 
>>> preproduction part
>>> + */
>>> +#define AIE4_PSP_C2PMSG_63_SMN_ADDR    0x38109fc
>>> +#define AIE4_PSP_FW_SIGN_ALT        BIT(6)
>>> +#define AIE4_PSP_FW_PREPROD        BIT(28)
>>> +
>>> +enum aie4_fw_variant {
>>> +    AIE4_FW_DEFAULT,
>>> +    AIE4_FW_ALT,        /* "_l" images */
>>> +    AIE4_FW_PREPROD,    /* "_p" images */
>>> +};
>>> +
>>> +/*
>>> + * The PSP only trusts firmware signed with the scheme the platform 
>>> firmware
>>> + * uses and a preproduction part needs firmware of its own. Reading 
>>> which one is
>>> + * in use is optional and only used as a hint for which images to 
>>> try first:
>>> + * anything but a clear answer is treated as default.
>>> + */
>>> +static enum aie4_fw_variant aie4_psp_fw_variant(struct amdxdna_dev 
>>> *xdna)
>>>   {
>>> -    struct amdxdna_dev *xdna = ndev->aie.xdna;
>>> -    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>>> -    char fw_name[128];
>>> +    u32 val = 0;
>>>       int ret;
>>> -    ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
>>> -               pdev->device, pdev->revision, ndev->priv->npufw_path);
>>> -    if (ret >= sizeof(fw_name)) {
>>> -        XDNA_ERR(xdna, "npu firmware path is truncated");
>>> -        return -EINVAL;
>>> +    ret = amd_smn_read(0, AIE4_PSP_C2PMSG_63_SMN_ADDR, &val);
>>> +    if (ret) {
>>> +        XDNA_DBG(xdna, "PSP firmware variant not available, ret 
>>> %d", ret);
>>> +        return AIE4_FW_DEFAULT;
>>>       }
>>> -    ret = request_firmware(npufw, fw_name, &pdev->dev);
>>> -    if (ret) {
>>> -        XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", 
>>> fw_name, ret);
>>> -        return ret;
>>> +    if ((val & AIE4_PSP_FW_SIGN_ALT) && (val & AIE4_PSP_FW_PREPROD)) {
>>> +        XDNA_WARN(xdna,
>>> +              "Preproduction part with alternative signing, using 
>>> default firmware");
>>> +        return AIE4_FW_DEFAULT;
>>>       }
>>> -    ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
>>> -               pdev->device, pdev->revision, ndev->priv->certfw_path);
>>> -    if (ret >= sizeof(fw_name)) {
>>> -        XDNA_ERR(xdna, "cert firmware path is truncated");
>>> -        ret = -EINVAL;
>>> -        goto release_npufw;
>>> +    if (val & AIE4_PSP_FW_PREPROD)
>>> +        return AIE4_FW_PREPROD;
>>> +
>>> +    return (val & AIE4_PSP_FW_SIGN_ALT) ? AIE4_FW_ALT : 
>>> AIE4_FW_DEFAULT;
>>> +}
>>> +
>>> +/*
>>> + * Images for the other variants have a suffix before the ".sbin" 
>>> extension.
>>> + * Returns -ENOENT if @path has no such variant.
>>> + */
>>> +static int aie4_fw_name(struct amdxdna_dev *xdna, char *name, 
>>> size_t size,
>>> +            const char *path, enum aie4_fw_variant variant)
>>> +{
>>> +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>>> +    const char *suffix;
>>> +    size_t len, base;
>>> +    int ret;
>>> +
>>> +    ret = snprintf(name, size, "amdnpu/%04x_%02x/%s",
>>> +               pdev->device, pdev->revision, path);
>>> +    if (ret >= size) {
>>> +        XDNA_ERR(xdna, "firmware path %s is truncated", path);
>>> +        return -EINVAL;
>>>       }
>>> -    ret = request_firmware(certfw, fw_name, &pdev->dev);
>>> -    if (ret) {
>>> -        XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", 
>>> fw_name, ret);
>>> -        goto release_npufw;
>>> +    switch (variant) {
>>> +    case AIE4_FW_ALT:
>>> +        suffix = "_l.sbin";
>>> +        break;
>>> +    case AIE4_FW_PREPROD:
>>> +        suffix = "_p.sbin";
>>> +        break;
>>> +    default:
>>> +        return 0;
>>>       }
>>> +    len = ret;
>>> +    if (len < strlen(".sbin") || strcmp(name + len - 
>>> strlen(".sbin"), ".sbin"))
>>> +        return -ENOENT;
>>> +
>>> +    base = len - strlen(".sbin");
>>> +    if (strscpy(name + base, suffix, size - base) < 0)
>>> +        return -EOVERFLOW;
>>> +
>>>       return 0;
>>> +}
>>> +
>>> +static int aie4_request_one(struct amdxdna_dev_hdl *ndev, const 
>>> struct firmware **fw,
>>> +                const char *path, enum aie4_fw_variant variant)
>>> +{
>>> +    struct amdxdna_dev *xdna = ndev->aie.xdna;
>>> +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>>> +    char name[128];
>>> +    int ret;
>>> +
>>> +    ret = aie4_fw_name(xdna, name, sizeof(name), path, variant);
>>> +    if (ret)
>>> +        return ret;
>>> -release_npufw:
>>> -    release_firmware(*npufw);
>>> +    ret = firmware_request_nowarn(fw, name, &pdev->dev);
>>> +    if (ret)
>>> +        XDNA_DBG(xdna, "firmware %s not loaded, ret %d", name, ret);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +/* The NPU and cert images are validated together, so both use the 
>>> same variant */
>>> +static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
>>> +                 const struct firmware **npufw,
>>> +                 const struct firmware **certfw,
>>> +                 enum aie4_fw_variant variant)
>>> +{
>>> +    int ret;
>>> +
>>> +    ret = aie4_request_one(ndev, npufw, ndev->priv->npufw_path, 
>>> variant);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    ret = aie4_request_one(ndev, certfw, ndev->priv->certfw_path, 
>>> variant);
>>> +    if (ret)
>>> +        release_firmware(*npufw);
>>>       return ret;
>>>   }
>>> @@ -645,18 +725,40 @@ static int aie4_prepare_firmware(struct 
>>> amdxdna_dev_hdl *ndev,
>>>       return 0;
>>>   }
>>> +/*
>>> + * The firmware can only be started once. It is the PSP that 
>>> decides whether it
>>> + * accepts the images, and it does that when the firmware is 
>>> started, so there
>>> + * is no trying a variant and falling back. Pick the variant that 
>>> the PSP
>>> + * reported (the default one if it can't be read) and use the 
>>> others only if
>>> + * its images are missing.
>>> + */
>>>   static int aie4_load_fw(struct amdxdna_dev_hdl *ndev,
>>>               void __iomem *tbl[PCI_NUM_RESOURCES])
>>>   {
>>> +    struct amdxdna_dev *xdna = ndev->aie.xdna;
>>>       const struct firmware *npufw, *certfw;
>>> -    int ret;
>>> +    enum aie4_fw_variant order[3], hint;
>>> +    int i, n = 0, ret = -ENOENT;
>>>       if (!ndev->priv->npufw_path && !ndev->priv->certfw_path)
>>>           return 0;
>>> -    ret = aie4_request_firmware(ndev, &npufw, &certfw);
>>> -    if (ret)
>>> +    hint = aie4_psp_fw_variant(xdna);
>>> +    order[n++] = hint;
>>> +    if (hint != AIE4_FW_DEFAULT)
>>> +        order[n++] = AIE4_FW_DEFAULT;
>>> +    if (hint != AIE4_FW_ALT)
>>> +        order[n++] = AIE4_FW_ALT;
>>> +
>>> +    for (i = 0; i < n; i++) {
>>> +        ret = aie4_request_firmware(ndev, &npufw, &certfw, order[i]);
>>> +        if (!ret)
>>> +            break;
>>> +    }
>>> +    if (ret) {
>>> +        XDNA_ERR(xdna, "failed to load firmware, ret %d", ret);
>>>           return ret;
>>> +    }
>>>       ret = aie4_prepare_firmware(ndev, npufw, certfw, tbl);
>>>       aie4_release_firmware(ndev, npufw, certfw);
>>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c 
>>> b/drivers/accel/ amdxdna/amdxdna_pci_drv.c
>>> index f233eb70a2957..dab8603e2381f 100644
>>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>>> @@ -37,6 +37,14 @@ MODULE_FIRMWARE("amdnpu/17f1_13/npu.sbin");
>>>   MODULE_FIRMWARE("amdnpu/17f1_13/cert.sbin");
>>>   MODULE_FIRMWARE("amdnpu/17f2_13/npu.sbin");
>>>   MODULE_FIRMWARE("amdnpu/17f2_13/cert.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f1_10/npu_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f1_10/cert_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f2_10/npu_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f2_10/cert_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f1_13/npu_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f1_13/cert_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f2_13/npu_l.sbin");
>>> +MODULE_FIRMWARE("amdnpu/17f2_13/cert_l.sbin");
>> need to add entries for "_p" images.
>
> Let me tell you my thought process in excluding them.
>
> They are only for pre-prod hardware, and so *most people* who use this 
> driver after the product launches shouldn't need them in their 
> initramfs; they'll just bloat the initramfs.
>
> If you think they should still be there; I'll spin a v3 an add them.

Ok, sounds good to me.

Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>

Lizhi

>
>>>   /*
>>>    * 0.0: Initial version
>>>
>>> ---
>>> base-commit: e32e83a6c5ec031a7f59d4794113d8484b220254
>>> change-id: 20261104-superm1-lms-npu-6bc3bbe38c29
>>>
>>> Best regards,
>>> -- 
>>> Mario Limonciello <mario.limonciello@amd.com>
>>>
>

Re: [PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[Zhang, Yidong (David)]

On 11/5/2026 7:51 AM, Mario Limonciello wrote:
> Some Platforms ship BIOS with either a default or an alternative trust
> authority. The PSP validates the NPU firmware, so it must be signed with
> the matching scheme. Firmware for the alternative scheme is named with
> an "_l" suffix before the extension. A preproduction part needs firmware
> of its own, named with a "_p" suffix.
>
> The PSP reports the signing type in bit 6 of C2PMSG_63, and in bit 28
> whether this is a preproduction part. A preproduction part that also
> reports the alternative scheme is not expected, so warn about it and
> use the default firmware.
>
> Add MODULE_FIRMWARE() entries for the "_l" variants of the npu and cert
> images of the aie4 devices so that initramfs generators include them.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Changes in v2:
> - Initialize val in aie4_psp_fw_variant() so a failed SMN read can't leave
>    it uninitialized
> - Only read the PSP firmware variant after the npufw/certfw path check, so
>    VFs (which have no firmware paths) skip the SMN access
> - Drop the redundant all-ones check, amd_smn_read() already returns -ENODEV
> - Link to v1: https://patch.msgid.link/20261104-superm1-lms-npu-v1-1-dbffcc5abc23@amd.com
> ---
>   drivers/accel/amdxdna/Kconfig           |   1 +
>   drivers/accel/amdxdna/aie4_pci.c        | 162 ++++++++++++++++++++++++++------
>   drivers/accel/amdxdna/amdxdna_pci_drv.c |   8 ++
>   3 files changed, 141 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/Kconfig b/drivers/accel/amdxdna/Kconfig
> index eddff490b4f75..cd17a89cf4146 100644
> --- a/drivers/accel/amdxdna/Kconfig
> +++ b/drivers/accel/amdxdna/Kconfig
> @@ -6,6 +6,7 @@ config DRM_ACCEL_AMDXDNA
>   	depends on DRM_ACCEL
>   	depends on PCI && HAS_IOMEM
>   	depends on X86_64
> +	depends on AMD_NODE
>   	depends on AMD_PMF || !AMD_PMF
>   	select DRM_SCHED
>   	select DRM_GEM_SHMEM_HELPER
> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/aie4_pci.c
> index 02dcb2f813d45..26c6162893760 100644
> --- a/drivers/accel/amdxdna/aie4_pci.c
> +++ b/drivers/accel/amdxdna/aie4_pci.c
> @@ -11,6 +11,8 @@
>   #include <linux/debugfs.h>
>   #include <linux/firmware.h>
>   #include <linux/sizes.h>
> +#include <linux/string.h>
> +#include <asm/amd/node.h>
>   
>   #include "aie.h"
>   #include "aie4_msg_priv.h"
> @@ -558,46 +560,124 @@ static void aie4_classic_hw_stop(struct amdxdna_dev_hdl *ndev)
>   	aie4_fw_stop(ndev);
>   }
>   
> -static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
> -				 const struct firmware **npufw,
> -				 const struct firmware **certfw)
> +/*
> + * PSP C2PMSG_63 (SMN) bit 6: 0 = default firmware signing, 1 = alternative
> + * PSP C2PMSG_63 (SMN) bit 28: 0 = production part, 1 = preproduction part
> + */
> +#define AIE4_PSP_C2PMSG_63_SMN_ADDR	0x38109fc
> +#define AIE4_PSP_FW_SIGN_ALT		BIT(6)
> +#define AIE4_PSP_FW_PREPROD		BIT(28)
> +
> +enum aie4_fw_variant {
> +	AIE4_FW_DEFAULT,
> +	AIE4_FW_ALT,		/* "_l" images */
> +	AIE4_FW_PREPROD,	/* "_p" images */
> +};
> +
> +/*
> + * The PSP only trusts firmware signed with the scheme the platform firmware
> + * uses and a preproduction part needs firmware of its own. Reading which one is
> + * in use is optional and only used as a hint for which images to try first:
> + * anything but a clear answer is treated as default.
> + */
> +static enum aie4_fw_variant aie4_psp_fw_variant(struct amdxdna_dev *xdna)
>   {
> -	struct amdxdna_dev *xdna = ndev->aie.xdna;
> -	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> -	char fw_name[128];
> +	u32 val = 0;
>   	int ret;
>   
> -	ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
> -		       pdev->device, pdev->revision, ndev->priv->npufw_path);
> -	if (ret >= sizeof(fw_name)) {
> -		XDNA_ERR(xdna, "npu firmware path is truncated");
> -		return -EINVAL;
> +	ret = amd_smn_read(0, AIE4_PSP_C2PMSG_63_SMN_ADDR, &val);
> +	if (ret) {
> +		XDNA_DBG(xdna, "PSP firmware variant not available, ret %d", ret);
> +		return AIE4_FW_DEFAULT;
>   	}
>   
> -	ret = request_firmware(npufw, fw_name, &pdev->dev);
> -	if (ret) {
> -		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", fw_name, ret);
> -		return ret;
> +	if ((val & AIE4_PSP_FW_SIGN_ALT) && (val & AIE4_PSP_FW_PREPROD)) {
> +		XDNA_WARN(xdna,
> +			  "Preproduction part with alternative signing, using default firmware");
> +		return AIE4_FW_DEFAULT;
>   	}
>   
> -	ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
> -		       pdev->device, pdev->revision, ndev->priv->certfw_path);
> -	if (ret >= sizeof(fw_name)) {
> -		XDNA_ERR(xdna, "cert firmware path is truncated");
> -		ret = -EINVAL;
> -		goto release_npufw;
> +	if (val & AIE4_PSP_FW_PREPROD)
> +		return AIE4_FW_PREPROD;
> +
> +	return (val & AIE4_PSP_FW_SIGN_ALT) ? AIE4_FW_ALT : AIE4_FW_DEFAULT;
> +}
> +
> +/*
> + * Images for the other variants have a suffix before the ".sbin" extension.
> + * Returns -ENOENT if @path has no such variant.
> + */
> +static int aie4_fw_name(struct amdxdna_dev *xdna, char *name, size_t size,
> +			const char *path, enum aie4_fw_variant variant)
> +{
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	const char *suffix;
> +	size_t len, base;
> +	int ret;
> +
> +	ret = snprintf(name, size, "amdnpu/%04x_%02x/%s",
> +		       pdev->device, pdev->revision, path);
> +	if (ret >= size) {
> +		XDNA_ERR(xdna, "firmware path %s is truncated", path);
> +		return -EINVAL;
>   	}
>   
> -	ret = request_firmware(certfw, fw_name, &pdev->dev);
> -	if (ret) {
> -		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", fw_name, ret);
> -		goto release_npufw;
> +	switch (variant) {
> +	case AIE4_FW_ALT:
> +		suffix = "_l.sbin";
> +		break;
> +	case AIE4_FW_PREPROD:
> +		suffix = "_p.sbin";
> +		break;
> +	default:
> +		return 0;
>   	}
>   
> +	len = ret;
> +	if (len < strlen(".sbin") || strcmp(name + len - strlen(".sbin"), ".sbin"))
> +		return -ENOENT;
> +
> +	base = len - strlen(".sbin");
> +	if (strscpy(name + base, suffix, size - base) < 0)
> +		return -EOVERFLOW;
> +
>   	return 0;
> +}
> +
> +static int aie4_request_one(struct amdxdna_dev_hdl *ndev, const struct firmware **fw,
> +			    const char *path, enum aie4_fw_variant variant)
> +{
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	char name[128];
> +	int ret;
> +
> +	ret = aie4_fw_name(xdna, name, sizeof(name), path, variant);
> +	if (ret)
> +		return ret;
>   
> -release_npufw:
> -	release_firmware(*npufw);
> +	ret = firmware_request_nowarn(fw, name, &pdev->dev);
> +	if (ret)
> +		XDNA_DBG(xdna, "firmware %s not loaded, ret %d", name, ret);
> +
> +	return ret;
> +}
> +
> +/* The NPU and cert images are validated together, so both use the same variant */
> +static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
> +				 const struct firmware **npufw,
> +				 const struct firmware **certfw,
> +				 enum aie4_fw_variant variant)
> +{
> +	int ret;
> +
> +	ret = aie4_request_one(ndev, npufw, ndev->priv->npufw_path, variant);
> +	if (ret)
> +		return ret;
> +
> +	ret = aie4_request_one(ndev, certfw, ndev->priv->certfw_path, variant);
> +	if (ret)
> +		release_firmware(*npufw);
>   
>   	return ret;
>   }
> @@ -645,18 +725,40 @@ static int aie4_prepare_firmware(struct amdxdna_dev_hdl *ndev,
>   	return 0;
>   }
>   
> +/*
> + * The firmware can only be started once. It is the PSP that decides whether it
> + * accepts the images, and it does that when the firmware is started, so there
> + * is no trying a variant and falling back. Pick the variant that the PSP
> + * reported (the default one if it can't be read) and use the others only if
> + * its images are missing.
> + */
>   static int aie4_load_fw(struct amdxdna_dev_hdl *ndev,
>   			void __iomem *tbl[PCI_NUM_RESOURCES])
>   {
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
>   	const struct firmware *npufw, *certfw;
> -	int ret;
> +	enum aie4_fw_variant order[3], hint;
> +	int i, n = 0, ret = -ENOENT;
>   
>   	if (!ndev->priv->npufw_path && !ndev->priv->certfw_path)
>   		return 0;
>   
> -	ret = aie4_request_firmware(ndev, &npufw, &certfw);
> -	if (ret)
> +	hint = aie4_psp_fw_variant(xdna);
> +	order[n++] = hint;
> +	if (hint != AIE4_FW_DEFAULT)
> +		order[n++] = AIE4_FW_DEFAULT;
> +	if (hint != AIE4_FW_ALT)
> +		order[n++] = AIE4_FW_ALT;
> +
> +	for (i = 0; i < n; i++) {
> +		ret = aie4_request_firmware(ndev, &npufw, &certfw, order[i]);
> +		if (!ret)
> +			break;
> +	}
> +	if (ret) {
> +		XDNA_ERR(xdna, "failed to load firmware, ret %d", ret);
>   		return ret;
> +	}
>   
>   	ret = aie4_prepare_firmware(ndev, npufw, certfw, tbl);
>   	aie4_release_firmware(ndev, npufw, certfw);
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index f233eb70a2957..dab8603e2381f 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -37,6 +37,14 @@ MODULE_FIRMWARE("amdnpu/17f1_13/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f1_13/cert.sbin");
>   MODULE_FIRMWARE("amdnpu/17f2_13/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f2_13/cert.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_10/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_10/cert_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_10/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_10/cert_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_13/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f1_13/cert_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_13/npu_l.sbin");
> +MODULE_FIRMWARE("amdnpu/17f2_13/cert_l.sbin");


I tested against both _l, _p and default sbins. All passed.

Reviewed-by: David Zhang <yidong.zhang@amd.com>

Tested-by: David Zhang <yidong.zhang@amd.com>

/David

>   
>   /*
>    * 0.0: Initial version
>
> ---
> base-commit: e32e83a6c5ec031a7f59d4794113d8484b220254
> change-id: 20261104-superm1-lms-npu-6bc3bbe38c29
>
> Best regards,
> --
> Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports

[Mario Limonciello]

On 11/5/26 13:19, Zhang, Yidong (David) wrote:
> 
> On 11/5/2026 7:51 AM, Mario Limonciello wrote:
>> Some Platforms ship BIOS with either a default or an alternative trust
>> authority. The PSP validates the NPU firmware, so it must be signed with
>> the matching scheme. Firmware for the alternative scheme is named with
>> an "_l" suffix before the extension. A preproduction part needs firmware
>> of its own, named with a "_p" suffix.
>>
>> The PSP reports the signing type in bit 6 of C2PMSG_63, and in bit 28
>> whether this is a preproduction part. A preproduction part that also
>> reports the alternative scheme is not expected, so warn about it and
>> use the default firmware.
>>
>> Add MODULE_FIRMWARE() entries for the "_l" variants of the npu and cert
>> images of the aie4 devices so that initramfs generators include them.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> Changes in v2:
>> - Initialize val in aie4_psp_fw_variant() so a failed SMN read can't 
>> leave
>>    it uninitialized
>> - Only read the PSP firmware variant after the npufw/certfw path 
>> check, so
>>    VFs (which have no firmware paths) skip the SMN access
>> - Drop the redundant all-ones check, amd_smn_read() already returns - 
>> ENODEV
>> - Link to v1: https://patch.msgid.link/20261104-superm1-lms-npu-v1-1- 
>> dbffcc5abc23@amd.com
>> ---
>>   drivers/accel/amdxdna/Kconfig           |   1 +
>>   drivers/accel/amdxdna/aie4_pci.c        | 162 ++++++++++++++++++++++ 
>> ++++------
>>   drivers/accel/amdxdna/amdxdna_pci_drv.c |   8 ++
>>   3 files changed, 141 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/Kconfig b/drivers/accel/amdxdna/ 
>> Kconfig
>> index eddff490b4f75..cd17a89cf4146 100644
>> --- a/drivers/accel/amdxdna/Kconfig
>> +++ b/drivers/accel/amdxdna/Kconfig
>> @@ -6,6 +6,7 @@ config DRM_ACCEL_AMDXDNA
>>       depends on DRM_ACCEL
>>       depends on PCI && HAS_IOMEM
>>       depends on X86_64
>> +    depends on AMD_NODE
>>       depends on AMD_PMF || !AMD_PMF
>>       select DRM_SCHED
>>       select DRM_GEM_SHMEM_HELPER
>> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/ 
>> aie4_pci.c
>> index 02dcb2f813d45..26c6162893760 100644
>> --- a/drivers/accel/amdxdna/aie4_pci.c
>> +++ b/drivers/accel/amdxdna/aie4_pci.c
>> @@ -11,6 +11,8 @@
>>   #include <linux/debugfs.h>
>>   #include <linux/firmware.h>
>>   #include <linux/sizes.h>
>> +#include <linux/string.h>
>> +#include <asm/amd/node.h>
>>   #include "aie.h"
>>   #include "aie4_msg_priv.h"
>> @@ -558,46 +560,124 @@ static void aie4_classic_hw_stop(struct 
>> amdxdna_dev_hdl *ndev)
>>       aie4_fw_stop(ndev);
>>   }
>> -static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
>> -                 const struct firmware **npufw,
>> -                 const struct firmware **certfw)
>> +/*
>> + * PSP C2PMSG_63 (SMN) bit 6: 0 = default firmware signing, 1 = 
>> alternative
>> + * PSP C2PMSG_63 (SMN) bit 28: 0 = production part, 1 = preproduction 
>> part
>> + */
>> +#define AIE4_PSP_C2PMSG_63_SMN_ADDR    0x38109fc
>> +#define AIE4_PSP_FW_SIGN_ALT        BIT(6)
>> +#define AIE4_PSP_FW_PREPROD        BIT(28)
>> +
>> +enum aie4_fw_variant {
>> +    AIE4_FW_DEFAULT,
>> +    AIE4_FW_ALT,        /* "_l" images */
>> +    AIE4_FW_PREPROD,    /* "_p" images */
>> +};
>> +
>> +/*
>> + * The PSP only trusts firmware signed with the scheme the platform 
>> firmware
>> + * uses and a preproduction part needs firmware of its own. Reading 
>> which one is
>> + * in use is optional and only used as a hint for which images to try 
>> first:
>> + * anything but a clear answer is treated as default.
>> + */
>> +static enum aie4_fw_variant aie4_psp_fw_variant(struct amdxdna_dev 
>> *xdna)
>>   {
>> -    struct amdxdna_dev *xdna = ndev->aie.xdna;
>> -    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>> -    char fw_name[128];
>> +    u32 val = 0;
>>       int ret;
>> -    ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
>> -               pdev->device, pdev->revision, ndev->priv->npufw_path);
>> -    if (ret >= sizeof(fw_name)) {
>> -        XDNA_ERR(xdna, "npu firmware path is truncated");
>> -        return -EINVAL;
>> +    ret = amd_smn_read(0, AIE4_PSP_C2PMSG_63_SMN_ADDR, &val);
>> +    if (ret) {
>> +        XDNA_DBG(xdna, "PSP firmware variant not available, ret %d", 
>> ret);
>> +        return AIE4_FW_DEFAULT;
>>       }
>> -    ret = request_firmware(npufw, fw_name, &pdev->dev);
>> -    if (ret) {
>> -        XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", 
>> fw_name, ret);
>> -        return ret;
>> +    if ((val & AIE4_PSP_FW_SIGN_ALT) && (val & AIE4_PSP_FW_PREPROD)) {
>> +        XDNA_WARN(xdna,
>> +              "Preproduction part with alternative signing, using 
>> default firmware");
>> +        return AIE4_FW_DEFAULT;
>>       }
>> -    ret = snprintf(fw_name, sizeof(fw_name), "amdnpu/%04x_%02x/%s",
>> -               pdev->device, pdev->revision, ndev->priv->certfw_path);
>> -    if (ret >= sizeof(fw_name)) {
>> -        XDNA_ERR(xdna, "cert firmware path is truncated");
>> -        ret = -EINVAL;
>> -        goto release_npufw;
>> +    if (val & AIE4_PSP_FW_PREPROD)
>> +        return AIE4_FW_PREPROD;
>> +
>> +    return (val & AIE4_PSP_FW_SIGN_ALT) ? AIE4_FW_ALT : AIE4_FW_DEFAULT;
>> +}
>> +
>> +/*
>> + * Images for the other variants have a suffix before the ".sbin" 
>> extension.
>> + * Returns -ENOENT if @path has no such variant.
>> + */
>> +static int aie4_fw_name(struct amdxdna_dev *xdna, char *name, size_t 
>> size,
>> +            const char *path, enum aie4_fw_variant variant)
>> +{
>> +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>> +    const char *suffix;
>> +    size_t len, base;
>> +    int ret;
>> +
>> +    ret = snprintf(name, size, "amdnpu/%04x_%02x/%s",
>> +               pdev->device, pdev->revision, path);
>> +    if (ret >= size) {
>> +        XDNA_ERR(xdna, "firmware path %s is truncated", path);
>> +        return -EINVAL;
>>       }
>> -    ret = request_firmware(certfw, fw_name, &pdev->dev);
>> -    if (ret) {
>> -        XDNA_ERR(xdna, "failed to request_firmware %s, ret %d", 
>> fw_name, ret);
>> -        goto release_npufw;
>> +    switch (variant) {
>> +    case AIE4_FW_ALT:
>> +        suffix = "_l.sbin";
>> +        break;
>> +    case AIE4_FW_PREPROD:
>> +        suffix = "_p.sbin";
>> +        break;
>> +    default:
>> +        return 0;
>>       }
>> +    len = ret;
>> +    if (len < strlen(".sbin") || strcmp(name + len - strlen(".sbin"), 
>> ".sbin"))
>> +        return -ENOENT;
>> +
>> +    base = len - strlen(".sbin");
>> +    if (strscpy(name + base, suffix, size - base) < 0)
>> +        return -EOVERFLOW;
>> +
>>       return 0;
>> +}
>> +
>> +static int aie4_request_one(struct amdxdna_dev_hdl *ndev, const 
>> struct firmware **fw,
>> +                const char *path, enum aie4_fw_variant variant)
>> +{
>> +    struct amdxdna_dev *xdna = ndev->aie.xdna;
>> +    struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
>> +    char name[128];
>> +    int ret;
>> +
>> +    ret = aie4_fw_name(xdna, name, sizeof(name), path, variant);
>> +    if (ret)
>> +        return ret;
>> -release_npufw:
>> -    release_firmware(*npufw);
>> +    ret = firmware_request_nowarn(fw, name, &pdev->dev);
>> +    if (ret)
>> +        XDNA_DBG(xdna, "firmware %s not loaded, ret %d", name, ret);
>> +
>> +    return ret;
>> +}
>> +
>> +/* The NPU and cert images are validated together, so both use the 
>> same variant */
>> +static int aie4_request_firmware(struct amdxdna_dev_hdl *ndev,
>> +                 const struct firmware **npufw,
>> +                 const struct firmware **certfw,
>> +                 enum aie4_fw_variant variant)
>> +{
>> +    int ret;
>> +
>> +    ret = aie4_request_one(ndev, npufw, ndev->priv->npufw_path, 
>> variant);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = aie4_request_one(ndev, certfw, ndev->priv->certfw_path, 
>> variant);
>> +    if (ret)
>> +        release_firmware(*npufw);
>>       return ret;
>>   }
>> @@ -645,18 +725,40 @@ static int aie4_prepare_firmware(struct 
>> amdxdna_dev_hdl *ndev,
>>       return 0;
>>   }
>> +/*
>> + * The firmware can only be started once. It is the PSP that decides 
>> whether it
>> + * accepts the images, and it does that when the firmware is started, 
>> so there
>> + * is no trying a variant and falling back. Pick the variant that the 
>> PSP
>> + * reported (the default one if it can't be read) and use the others 
>> only if
>> + * its images are missing.
>> + */
>>   static int aie4_load_fw(struct amdxdna_dev_hdl *ndev,
>>               void __iomem *tbl[PCI_NUM_RESOURCES])
>>   {
>> +    struct amdxdna_dev *xdna = ndev->aie.xdna;
>>       const struct firmware *npufw, *certfw;
>> -    int ret;
>> +    enum aie4_fw_variant order[3], hint;
>> +    int i, n = 0, ret = -ENOENT;
>>       if (!ndev->priv->npufw_path && !ndev->priv->certfw_path)
>>           return 0;
>> -    ret = aie4_request_firmware(ndev, &npufw, &certfw);
>> -    if (ret)
>> +    hint = aie4_psp_fw_variant(xdna);
>> +    order[n++] = hint;
>> +    if (hint != AIE4_FW_DEFAULT)
>> +        order[n++] = AIE4_FW_DEFAULT;
>> +    if (hint != AIE4_FW_ALT)
>> +        order[n++] = AIE4_FW_ALT;
>> +
>> +    for (i = 0; i < n; i++) {
>> +        ret = aie4_request_firmware(ndev, &npufw, &certfw, order[i]);
>> +        if (!ret)
>> +            break;
>> +    }
>> +    if (ret) {
>> +        XDNA_ERR(xdna, "failed to load firmware, ret %d", ret);
>>           return ret;
>> +    }
>>       ret = aie4_prepare_firmware(ndev, npufw, certfw, tbl);
>>       aie4_release_firmware(ndev, npufw, certfw);
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/ 
>> amdxdna/amdxdna_pci_drv.c
>> index f233eb70a2957..dab8603e2381f 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> @@ -37,6 +37,14 @@ MODULE_FIRMWARE("amdnpu/17f1_13/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f1_13/cert.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f2_13/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f2_13/cert.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_10/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_10/cert_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_10/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_10/cert_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_13/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f1_13/cert_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_13/npu_l.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f2_13/cert_l.sbin");
> 
> 
> I tested against both _l, _p and default sbins. All passed.
> 
> Reviewed-by: David Zhang <yidong.zhang@amd.com>
> 
> Tested-by: David Zhang <yidong.zhang@amd.com>
> 
> /David
> 
>>   /*
>>    * 0.0: Initial version
>>
>> ---
>> base-commit: e32e83a6c5ec031a7f59d4794113d8484b220254
>> change-id: 20261104-superm1-lms-npu-6bc3bbe38c29
>>
>> Best regards,
>> -- 
>> Mario Limonciello <mario.limonciello@amd.com>

applied to drm-misc-next