Re: [PATCH v2] accel/amdxdna: Load the aie4 firmware variant the PSP reports
From: Mario Limonciello
Date: Thu Nov 05 2026 - 12:52:37 EST
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 trustneed to add entries for "_p" images.
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@xxxxxxx>
---
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@xxxxxxx
---
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");
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@xxxxxxx>