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

["Zhang, Yidong (David)" <yidong.zhang@amd.com>]

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>