[PATCH v2 0/7] Parse SMBIOS additional entries

[PATCH v2 0/7] Parse SMBIOS additional entries

[Mario Limonciello (AMD)]

The SMBIOS additional entries on AMD Zen4+ systems running an AGESA
based BIOS contain information about the AGESA version which can be
useful for matching the software stack when debugging an issue.

Add support for parsing this from SMBIOS tables and output it into
the logs at bootup.  Additionally export the information to debugfs
in case users are interested in any other strings.

v2:
 * Move message to arch/x86/kernel/cpu/amd.c
 * Optimizations for LKP robot reported comments
 * Add tags
 * Add other (unused) missing enums that match spec

Mario Limonciello (AMD) (7):
  firmware: dmi: Correct an indexing error in dmi.h
  firmware: dmi: Adjust dmi_decode() to use enums
  firmware: dmi: Read additional information when decoding DMI table
  firmware: dmi: Add debugfs for additional information entries
  firmware: dmi: Add missing DMI entry types
  x86/CPU/AMD: Prefix messages with x86/amd
  x86/CPU/AMD: Output the AGESA version to the logs

 arch/x86/kernel/cpu/amd.c   | 22 ++++++++-
 drivers/firmware/dmi_scan.c | 92 ++++++++++++++++++++++++++++++++-----
 include/linux/dmi.h         | 12 +++++
 3 files changed, 114 insertions(+), 12 deletions(-)

-- 
2.43.0

[PATCH v2 1/7] firmware: dmi: Correct an indexing error in dmi.h

[Mario Limonciello (AMD)]

The entries later in `enum dmi_entry_type` don't match the SMBIOS
specification.  The entry for type 33: `64-Bit Memory Error Information`
is not present and thus the index for all later entries is incorrect.

Add the missing type 33 entry.

Fixes: 93c890dbe5287 ("firmware: Add DMI entry types to the headers")
Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0a.pdf
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 include/linux/dmi.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 927f8a8b7a1dd..a809b5095c259 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -60,6 +60,7 @@ enum dmi_entry_type {
 	DMI_ENTRY_OOB_REMOTE_ACCESS,
 	DMI_ENTRY_BIS_ENTRY,
 	DMI_ENTRY_SYSTEM_BOOT,
+	DMI_ENTRY_MEMORY_INFO,
 	DMI_ENTRY_MGMT_DEV,
 	DMI_ENTRY_MGMT_DEV_COMPONENT,
 	DMI_ENTRY_MGMT_DEV_THRES,
-- 
2.43.0

[PATCH v2 2/7] firmware: dmi: Adjust dmi_decode() to use enums

[Mario Limonciello (AMD)]

dmi_decode() has hardcoded values with comments for each DMI entry
type. The same information is already in dmi.h though, so drop the
comments and use the definitions instead.

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/firmware/dmi_scan.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 70d39adf50dca..80aded4c778bc 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -484,14 +484,14 @@ static void __init dmi_memdev_walk(void)
 static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
 {
 	switch (dm->type) {
-	case 0:		/* BIOS Information */
+	case DMI_ENTRY_BIOS:
 		dmi_save_ident(dm, DMI_BIOS_VENDOR, 4);
 		dmi_save_ident(dm, DMI_BIOS_VERSION, 5);
 		dmi_save_ident(dm, DMI_BIOS_DATE, 8);
 		dmi_save_release(dm, DMI_BIOS_RELEASE, 21);
 		dmi_save_release(dm, DMI_EC_FIRMWARE_RELEASE, 23);
 		break;
-	case 1:		/* System Information */
+	case DMI_ENTRY_SYSTEM:
 		dmi_save_ident(dm, DMI_SYS_VENDOR, 4);
 		dmi_save_ident(dm, DMI_PRODUCT_NAME, 5);
 		dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
@@ -500,33 +500,33 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
 		dmi_save_ident(dm, DMI_PRODUCT_SKU, 25);
 		dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
 		break;
-	case 2:		/* Base Board Information */
+	case DMI_ENTRY_BASEBOARD:
 		dmi_save_ident(dm, DMI_BOARD_VENDOR, 4);
 		dmi_save_ident(dm, DMI_BOARD_NAME, 5);
 		dmi_save_ident(dm, DMI_BOARD_VERSION, 6);
 		dmi_save_ident(dm, DMI_BOARD_SERIAL, 7);
 		dmi_save_ident(dm, DMI_BOARD_ASSET_TAG, 8);
 		break;
-	case 3:		/* Chassis Information */
+	case DMI_ENTRY_CHASSIS:
 		dmi_save_ident(dm, DMI_CHASSIS_VENDOR, 4);
 		dmi_save_type(dm, DMI_CHASSIS_TYPE, 5);
 		dmi_save_ident(dm, DMI_CHASSIS_VERSION, 6);
 		dmi_save_ident(dm, DMI_CHASSIS_SERIAL, 7);
 		dmi_save_ident(dm, DMI_CHASSIS_ASSET_TAG, 8);
 		break;
-	case 9:		/* System Slots */
+	case DMI_ENTRY_SYSTEM_SLOT:
 		dmi_save_system_slot(dm);
 		break;
-	case 10:	/* Onboard Devices Information */
+	case DMI_ENTRY_ONBOARD_DEVICE:
 		dmi_save_devices(dm);
 		break;
-	case 11:	/* OEM Strings */
+	case DMI_ENTRY_OEMSTRINGS:
 		dmi_save_oem_strings_devices(dm);
 		break;
-	case 38:	/* IPMI Device Information */
+	case DMI_ENTRY_IPMI_DEV:
 		dmi_save_ipmi_device(dm);
 		break;
-	case 41:	/* Onboard Devices Extended Information */
+	case DMI_ENTRY_ONBOARD_DEV_EXT:
 		dmi_save_extended_devices(dm);
 	}
 }
-- 
2.43.0

[PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Mario Limonciello (AMD)]

Type 40 entries (Additional information) are summarized in section
7.41 as part of the SMBIOS specification.  Save these entries when
decoding the DMI tables.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v2:
 * Drop some unneeded variables (LKP robot)
 * Allow any length strings, not just 5 and longer
---
 drivers/firmware/dmi_scan.c | 38 +++++++++++++++++++++++++++++++++++++
 include/linux/dmi.h         |  7 +++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 80aded4c778bc..ec84fe3935c1e 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -393,6 +393,40 @@ static void __init dmi_save_dev_pciaddr(int instance, int segment, int bus,
 	list_add(&dev->dev.list, &dmi_devices);
 }
 
+static void __init dmi_save_additional(const struct dmi_additional_info *info)
+{
+	const u8 *data;
+	int i;
+
+	if (!info || info->header.length < 5 + info->count * 5)
+		return;
+
+	data = info->entries;
+
+	for (i = 0; i < info->count; i++) {
+		u8 string_num = data[i * 5 + 4];
+		const char *string_ptr;
+		char *value;
+		int len;
+
+		string_ptr = dmi_string_nosave(&info->header, string_num);
+		if (!string_ptr || !*string_ptr)
+			continue;
+
+		len = strlen(string_ptr);
+		if (len == 0)
+			continue;
+
+		value = dmi_alloc(len + 1);
+		if (!value)
+			continue;
+
+		strscpy(value, string_ptr, len + 1);
+
+		dmi_save_one_device(DMI_DEV_TYPE_ADDITIONAL, value);
+	}
+}
+
 static void __init dmi_save_extended_devices(const struct dmi_header *dm)
 {
 	const char *name;
@@ -526,8 +560,12 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
 	case DMI_ENTRY_IPMI_DEV:
 		dmi_save_ipmi_device(dm);
 		break;
+	case DMI_ENTRY_ADDITIONAL:
+		dmi_save_additional((const struct dmi_additional_info *)dm);
+		break;
 	case DMI_ENTRY_ONBOARD_DEV_EXT:
 		dmi_save_extended_devices(dm);
+		break;
 	}
 }
 
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index a809b5095c259..3fc3d334b321d 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -24,6 +24,7 @@ enum dmi_device_type {
 	DMI_DEV_TYPE_OEM_STRING = -2,
 	DMI_DEV_TYPE_DEV_ONBOARD = -3,
 	DMI_DEV_TYPE_DEV_SLOT = -4,
+	DMI_DEV_TYPE_ADDITIONAL = -5,
 };
 
 enum dmi_entry_type {
@@ -87,6 +88,12 @@ struct dmi_device {
 	void *device_data;	/* Type specific data */
 };
 
+struct dmi_additional_info {
+	struct dmi_header header;
+	u8 count;
+	u8 entries[];
+} __packed;
+
 #ifdef CONFIG_DMI
 
 struct dmi_dev_onboard {
-- 
2.43.0

[PATCH v2 4/7] firmware: dmi: Add debugfs for additional information entries

[Mario Limonciello (AMD)]

Additional information entries are not exposed through standardized
information from sysfs, but can still contain valuable information.

For example on AMD systems this encodes the AGESA version.  Introduce
a debugfs file that will export this information.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/firmware/dmi_scan.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index ec84fe3935c1e..5361b8b8a39d5 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -4,6 +4,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/ctype.h>
+#include <linux/debugfs.h>
 #include <linux/dmi.h>
 #include <linux/efi.h>
 #include <linux/memblock.h>
@@ -30,6 +31,7 @@ static u32 dmi_len;
 static u16 dmi_num;
 static u8 smbios_entry_point[32];
 static int smbios_entry_point_size;
+static struct dentry *debugfs_dir;
 
 /* DMI system identification string used during boot */
 static char dmi_ids_string[128] __initdata;
@@ -180,6 +182,17 @@ static LIST_HEAD(dmi_devices);
 int dmi_available;
 EXPORT_SYMBOL_GPL(dmi_available);
 
+static int additional_show(struct seq_file *m, void *v)
+{
+	const struct dmi_device *dev = NULL;
+
+	while ((dev = dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, dev)))
+		seq_printf(m, "%s\n", dev->name);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(additional);
+
 /*
  *	Save a DMI string
  */
@@ -802,6 +815,20 @@ static void __init dmi_scan_machine(void)
 static __ro_after_init BIN_ATTR_SIMPLE_ADMIN_RO(smbios_entry_point);
 static __ro_after_init BIN_ATTR_SIMPLE_ADMIN_RO(DMI);
 
+static void __init dmi_create_debugfs(void)
+{
+	if (!arch_debugfs_dir)
+		return;
+	debugfs_dir = debugfs_create_dir("dmi", arch_debugfs_dir);
+	if (!debugfs_dir)
+		return;
+	if (!dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, NULL))
+		return;
+	debugfs_create_file("additional", 0444,
+			    debugfs_dir, NULL,
+			    &additional_fops);
+}
+
 static int __init dmi_init(void)
 {
 	struct kobject *tables_kobj;
@@ -837,9 +864,14 @@ static int __init dmi_init(void)
 	bin_attr_DMI.size = dmi_len;
 	bin_attr_DMI.private = dmi_table;
 	ret = sysfs_create_bin_file(tables_kobj, &bin_attr_DMI);
-	if (!ret)
-		return 0;
+	if (ret)
+		goto err_sysfs;
+
+	dmi_create_debugfs();
+
+	return 0;
 
+ err_sysfs:
 	sysfs_remove_bin_file(tables_kobj,
 			      &bin_attr_smbios_entry_point);
  err_unmap:
-- 
2.43.0

[PATCH v2 5/7] firmware: dmi: Add missing DMI entry types

[Mario Limonciello (AMD)]

Type 42 through type 46 entry types are missing from the
definitions in the dmi_entry_type enum.

Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.1.pdf
Suggested-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 include/linux/dmi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 3fc3d334b321d..b7d4fdc25bfde 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -71,6 +71,10 @@ enum dmi_entry_type {
 	DMI_ENTRY_ADDITIONAL,
 	DMI_ENTRY_ONBOARD_DEV_EXT,
 	DMI_ENTRY_MGMT_CONTROLLER_HOST,
+	DMI_ENTRY_TPM_DEVICE,
+	DMI_ENTRY_PROCESSOR_ADDITIONAL,
+	DMI_ENTRY_FIRMWARE_INVENTORY,
+	DMI_ENTRY_STRING_PROPERTY,
 	DMI_ENTRY_INACTIVE = 126,
 	DMI_ENTRY_END_OF_TABLE = 127,
 };
-- 
2.43.0

[PATCH v2 6/7] x86/CPU/AMD: Prefix messages with x86/amd

[Mario Limonciello (AMD)]

To clarify which messages come from arch/x86/kernel/cpu/amd.c add
a prefix to all messages instead of just the previous reset reason.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 arch/x86/kernel/cpu/amd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index bc94ff1e250ad..c19c4ee74dd1f 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1,4 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
+#define pr_fmt(fmt) "x86/amd: " fmt
+
 #include <linux/export.h>
 #include <linux/bitops.h>
 #include <linux/elf.h>
@@ -1396,7 +1398,7 @@ static __init int print_s5_reset_status_mmio(void)
 			continue;
 
 		if (s5_reset_reason_txt[i]) {
-			pr_info("x86/amd: Previous system reset reason [0x%08x]: %s\n",
+			pr_info("Previous system reset reason [0x%08x]: %s\n",
 				value, s5_reset_reason_txt[i]);
 		}
 	}
-- 
2.43.0

[PATCH v2 7/7] x86/CPU/AMD: Output the AGESA version to the logs

[Mario Limonciello (AMD)]

On AMD Zen platforms that are running AGESA, there is sometimes
DMI additional string for the AGESA version that can be helpful when
debugging an issue.  If this string is found output to kernel logs.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 arch/x86/kernel/cpu/amd.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index c19c4ee74dd1f..8f44439d3f993 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 #define pr_fmt(fmt) "x86/amd: " fmt
 
+#include <linux/dmi.h>
 #include <linux/export.h>
 #include <linux/bitops.h>
 #include <linux/elf.h>
@@ -1406,3 +1407,20 @@ static __init int print_s5_reset_status_mmio(void)
 	return 0;
 }
 late_initcall(print_s5_reset_status_mmio);
+
+#ifdef CONFIG_DMI
+static __init int print_agesa_dmi_info(void)
+{
+	const struct dmi_device *dev = NULL;
+
+	while ((dev = dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, dev))) {
+		if (!strncmp(dev->name, "AGESA", 5)) {
+			pr_info("%s\n", dev->name);
+			break;
+		}
+	}
+
+	return 0;
+}
+late_initcall(print_agesa_dmi_info);
+#endif
-- 
2.43.0

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Yazen Ghannam]

On Tue, Dec 16, 2025 at 06:33:50AM -0600, Mario Limonciello (AMD) wrote:
> Type 40 entries (Additional information) are summarized in section
> 7.41 as part of the SMBIOS specification.  Save these entries when
> decoding the DMI tables.
> 

Why can't an interested user just use dmidecode?

Thanks,
Yazen

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Mario Limonciello]

On 12/17/25 3:03 PM, Yazen Ghannam wrote:
> On Tue, Dec 16, 2025 at 06:33:50AM -0600, Mario Limonciello (AMD) wrote:
>> Type 40 entries (Additional information) are summarized in section
>> 7.41 as part of the SMBIOS specification.  Save these entries when
>> decoding the DMI tables.
>>
> 
> Why can't an interested user just use dmidecode?
> 
> Thanks,
> Yazen

They could.  The reason for doing it in this series is the same reason 
for the one that we did the S5 bit.

It shows up in the logs, you can tie regressions to the AGESA version at 
specifically at the time of the failure if they've done BIOS updates 
since then.

Re: [PATCH v2 6/7] x86/CPU/AMD: Prefix messages with x86/amd

[Yazen Ghannam]

On Tue, Dec 16, 2025 at 06:33:53AM -0600, Mario Limonciello (AMD) wrote:
> To clarify which messages come from arch/x86/kernel/cpu/amd.c add
> a prefix to all messages instead of just the previous reset reason.
> 
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen

Re: [PATCH v2 7/7] x86/CPU/AMD: Output the AGESA version to the logs

[Yazen Ghannam]

On Tue, Dec 16, 2025 at 06:33:54AM -0600, Mario Limonciello (AMD) wrote:
> On AMD Zen platforms that are running AGESA, there is sometimes
> DMI additional string for the AGESA version that can be helpful when
> debugging an issue.  If this string is found output to kernel logs.
> 
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  arch/x86/kernel/cpu/amd.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c19c4ee74dd1f..8f44439d3f993 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  #define pr_fmt(fmt) "x86/amd: " fmt
>  
> +#include <linux/dmi.h>
>  #include <linux/export.h>
>  #include <linux/bitops.h>
>  #include <linux/elf.h>
> @@ -1406,3 +1407,20 @@ static __init int print_s5_reset_status_mmio(void)
>  	return 0;
>  }
>  late_initcall(print_s5_reset_status_mmio);
> +
> +#ifdef CONFIG_DMI
> +static __init int print_agesa_dmi_info(void)
> +{
> +	const struct dmi_device *dev = NULL;
> +
> +	while ((dev = dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, dev))) {
> +		if (!strncmp(dev->name, "AGESA", 5)) {
> +			pr_info("%s\n", dev->name);
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +late_initcall(print_agesa_dmi_info);
> +#endif
> 

The Zen check is gone. Is that intentional?

If so, then I think the commit message should be tweaked to not mention
Zen systems specifically.

I don't think this is a problem. I think it's better to keep things
generic, if possible.

Thanks,
Yazen

Re: [PATCH v2 7/7] x86/CPU/AMD: Output the AGESA version to the logs

[Mario Limonciello]

On 12/17/25 3:18 PM, Yazen Ghannam wrote:
> On Tue, Dec 16, 2025 at 06:33:54AM -0600, Mario Limonciello (AMD) wrote:
>> On AMD Zen platforms that are running AGESA, there is sometimes
>> DMI additional string for the AGESA version that can be helpful when
>> debugging an issue.  If this string is found output to kernel logs.
>>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>>   arch/x86/kernel/cpu/amd.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index c19c4ee74dd1f..8f44439d3f993 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -1,6 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   #define pr_fmt(fmt) "x86/amd: " fmt
>>   
>> +#include <linux/dmi.h>
>>   #include <linux/export.h>
>>   #include <linux/bitops.h>
>>   #include <linux/elf.h>
>> @@ -1406,3 +1407,20 @@ static __init int print_s5_reset_status_mmio(void)
>>   	return 0;
>>   }
>>   late_initcall(print_s5_reset_status_mmio);
>> +
>> +#ifdef CONFIG_DMI
>> +static __init int print_agesa_dmi_info(void)
>> +{
>> +	const struct dmi_device *dev = NULL;
>> +
>> +	while ((dev = dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, dev))) {
>> +		if (!strncmp(dev->name, "AGESA", 5)) {
>> +			pr_info("%s\n", dev->name);
>> +			break;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +late_initcall(print_agesa_dmi_info);
>> +#endif
>>
> 
> The Zen check is gone. Is that intentional?
> 

No it's not, it's a good catch.  We don't have an AGESA version in 
SMBIOS data on pre-zen hardware.

I'll add in a check like we do for print_s5_reset_status_mmio() for the 
next spin.

         if (!cpu_feature_enabled(X86_FEATURE_ZEN))
                 return 0;

> If so, then I think the commit message should be tweaked to not mention
> Zen systems specifically.
> 
> I don't think this is a problem. I think it's better to keep things
> generic, if possible.
> 
> Thanks,
> Yazen

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Yazen Ghannam]

On Wed, Dec 17, 2025 at 03:09:33PM -0600, Mario Limonciello wrote:
> On 12/17/25 3:03 PM, Yazen Ghannam wrote:
> > On Tue, Dec 16, 2025 at 06:33:50AM -0600, Mario Limonciello (AMD) wrote:
> > > Type 40 entries (Additional information) are summarized in section
> > > 7.41 as part of the SMBIOS specification.  Save these entries when
> > > decoding the DMI tables.
> > > 
> > 
> > Why can't an interested user just use dmidecode?
> > 
> > Thanks,
> > Yazen
> 
> They could.  The reason for doing it in this series is the same reason for
> the one that we did the S5 bit.
> 
> It shows up in the logs, you can tie regressions to the AGESA version at
> specifically at the time of the failure if they've done BIOS updates since
> then.

Yes, right. Sorry, I mixed this up with the debugfs patch.

We need to save it here so the init code can find it.

But why do we need a debugfs entry for it?

Thanks,
Yazen

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Mario Limonciello]

On 12/17/25 3:21 PM, Yazen Ghannam wrote:
> On Wed, Dec 17, 2025 at 03:09:33PM -0600, Mario Limonciello wrote:
>> On 12/17/25 3:03 PM, Yazen Ghannam wrote:
>>> On Tue, Dec 16, 2025 at 06:33:50AM -0600, Mario Limonciello (AMD) wrote:
>>>> Type 40 entries (Additional information) are summarized in section
>>>> 7.41 as part of the SMBIOS specification.  Save these entries when
>>>> decoding the DMI tables.
>>>>
>>>
>>> Why can't an interested user just use dmidecode?
>>>
>>> Thanks,
>>> Yazen
>>
>> They could.  The reason for doing it in this series is the same reason for
>> the one that we did the S5 bit.
>>
>> It shows up in the logs, you can tie regressions to the AGESA version at
>> specifically at the time of the failure if they've done BIOS updates since
>> then.
> 
> Yes, right. Sorry, I mixed this up with the debugfs patch.
> 
> We need to save it here so the init code can find it.
> 
> But why do we need a debugfs entry for it?

Ah.  That one I don't feel strongly about.

I used it when I was getting the series working and it felt like a waste 
to toss.

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Yazen Ghannam]

On Wed, Dec 17, 2025 at 03:23:10PM -0600, Mario Limonciello wrote:
> On 12/17/25 3:21 PM, Yazen Ghannam wrote:
> > On Wed, Dec 17, 2025 at 03:09:33PM -0600, Mario Limonciello wrote:
> > > On 12/17/25 3:03 PM, Yazen Ghannam wrote:
> > > > On Tue, Dec 16, 2025 at 06:33:50AM -0600, Mario Limonciello (AMD) wrote:
> > > > > Type 40 entries (Additional information) are summarized in section
> > > > > 7.41 as part of the SMBIOS specification.  Save these entries when
> > > > > decoding the DMI tables.
> > > > > 
> > > > 
> > > > Why can't an interested user just use dmidecode?
> > > > 
> > > > Thanks,
> > > > Yazen
> > > 
> > > They could.  The reason for doing it in this series is the same reason for
> > > the one that we did the S5 bit.
> > > 
> > > It shows up in the logs, you can tie regressions to the AGESA version at
> > > specifically at the time of the failure if they've done BIOS updates since
> > > then.
> > 
> > Yes, right. Sorry, I mixed this up with the debugfs patch.
> > 
> > We need to save it here so the init code can find it.
> > 
> > But why do we need a debugfs entry for it?
> 
> Ah.  That one I don't feel strongly about.
> 
> I used it when I was getting the series working and it felt like a waste to
> toss.

Oh okay. Yeah, maybe "don't feel strongly about it" is a good reason to
drop it. Just trying to think like a more frugal reviewer. :)

Thanks,
Yazen

Re: [PATCH v2 7/7] x86/CPU/AMD: Output the AGESA version to the logs

[Yazen Ghannam]

On Wed, Dec 17, 2025 at 03:21:17PM -0600, Mario Limonciello wrote:
> On 12/17/25 3:18 PM, Yazen Ghannam wrote:
> > On Tue, Dec 16, 2025 at 06:33:54AM -0600, Mario Limonciello (AMD) wrote:
> > > On AMD Zen platforms that are running AGESA, there is sometimes
> > > DMI additional string for the AGESA version that can be helpful when
> > > debugging an issue.  If this string is found output to kernel logs.
> > > 
> > > Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> > > ---
> > >   arch/x86/kernel/cpu/amd.c | 18 ++++++++++++++++++
> > >   1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > > index c19c4ee74dd1f..8f44439d3f993 100644
> > > --- a/arch/x86/kernel/cpu/amd.c
> > > +++ b/arch/x86/kernel/cpu/amd.c
> > > @@ -1,6 +1,7 @@
> > >   // SPDX-License-Identifier: GPL-2.0-only
> > >   #define pr_fmt(fmt) "x86/amd: " fmt
> > > +#include <linux/dmi.h>
> > >   #include <linux/export.h>
> > >   #include <linux/bitops.h>
> > >   #include <linux/elf.h>
> > > @@ -1406,3 +1407,20 @@ static __init int print_s5_reset_status_mmio(void)
> > >   	return 0;
> > >   }
> > >   late_initcall(print_s5_reset_status_mmio);
> > > +
> > > +#ifdef CONFIG_DMI
> > > +static __init int print_agesa_dmi_info(void)
> > > +{
> > > +	const struct dmi_device *dev = NULL;
> > > +
> > > +	while ((dev = dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, dev))) {
> > > +		if (!strncmp(dev->name, "AGESA", 5)) {
> > > +			pr_info("%s\n", dev->name);
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +late_initcall(print_agesa_dmi_info);
> > > +#endif
> > > 
> > 
> > The Zen check is gone. Is that intentional?
> > 
> 
> No it's not, it's a good catch.  We don't have an AGESA version in SMBIOS
> data on pre-zen hardware.
> 
> I'll add in a check like we do for print_s5_reset_status_mmio() for the next
> spin.
> 
>         if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>                 return 0;
> 

In that case, please consider a wrapper function that checks for Zen and
calls the two functions: AGESA and S5_RESET_STATUS.

Thanks,
Yazen

Re: [PATCH v2 1/7] firmware: dmi: Correct an indexing error in dmi.h

[Jean Delvare]

Hi Mario,

On Tue, 16 Dec 2025 06:33:48 -0600, Mario Limonciello (AMD) wrote:
> The entries later in `enum dmi_entry_type` don't match the SMBIOS
> specification.  The entry for type 33: `64-Bit Memory Error Information`
> is not present and thus the index for all later entries is incorrect.
> 
> Add the missing type 33 entry.
> 
> Fixes: 93c890dbe5287 ("firmware: Add DMI entry types to the headers")
> Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0a.pdf
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  include/linux/dmi.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 927f8a8b7a1dd..a809b5095c259 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -60,6 +60,7 @@ enum dmi_entry_type {
>  	DMI_ENTRY_OOB_REMOTE_ACCESS,
>  	DMI_ENTRY_BIS_ENTRY,
>  	DMI_ENTRY_SYSTEM_BOOT,
> +	DMI_ENTRY_MEMORY_INFO,
>  	DMI_ENTRY_MGMT_DEV,
>  	DMI_ENTRY_MGMT_DEV_COMPONENT,
>  	DMI_ENTRY_MGMT_DEV_THRES,

Good catch. Do I understand correctly that this does not fix any actual
bug because none of the entry types after DMI_ENTRY_SYSTEM_BOOT was
used in code so far?

As for the name, considering that we used DMI_ENTRY_32_MEM_ERROR for
type 18 (32-Bit Memory Error Information), I believe we should use
DMI_ENTRY_64_MEM_ERROR for type 33 (64-Bit Memory Error Information)
for consistency.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 2/7] firmware: dmi: Adjust dmi_decode() to use enums

[Jean Delvare]

On Tue, 16 Dec 2025 06:33:49 -0600, Mario Limonciello (AMD) wrote:
> dmi_decode() has hardcoded values with comments for each DMI entry
> type. The same information is already in dmi.h though, so drop the
> comments and use the definitions instead.
> 
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  drivers/firmware/dmi_scan.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 70d39adf50dca..80aded4c778bc 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -484,14 +484,14 @@ static void __init dmi_memdev_walk(void)
>  static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
>  {
>  	switch (dm->type) {
> -	case 0:		/* BIOS Information */
> +	case DMI_ENTRY_BIOS:
>  		dmi_save_ident(dm, DMI_BIOS_VENDOR, 4);
>  		dmi_save_ident(dm, DMI_BIOS_VERSION, 5);
>  		dmi_save_ident(dm, DMI_BIOS_DATE, 8);
>  		dmi_save_release(dm, DMI_BIOS_RELEASE, 21);
>  		dmi_save_release(dm, DMI_EC_FIRMWARE_RELEASE, 23);
>  		break;
> -	case 1:		/* System Information */
> +	case DMI_ENTRY_SYSTEM:
>  		dmi_save_ident(dm, DMI_SYS_VENDOR, 4);
>  		dmi_save_ident(dm, DMI_PRODUCT_NAME, 5);
>  		dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
> @@ -500,33 +500,33 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
>  		dmi_save_ident(dm, DMI_PRODUCT_SKU, 25);
>  		dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
>  		break;
> -	case 2:		/* Base Board Information */
> +	case DMI_ENTRY_BASEBOARD:
>  		dmi_save_ident(dm, DMI_BOARD_VENDOR, 4);
>  		dmi_save_ident(dm, DMI_BOARD_NAME, 5);
>  		dmi_save_ident(dm, DMI_BOARD_VERSION, 6);
>  		dmi_save_ident(dm, DMI_BOARD_SERIAL, 7);
>  		dmi_save_ident(dm, DMI_BOARD_ASSET_TAG, 8);
>  		break;
> -	case 3:		/* Chassis Information */
> +	case DMI_ENTRY_CHASSIS:
>  		dmi_save_ident(dm, DMI_CHASSIS_VENDOR, 4);
>  		dmi_save_type(dm, DMI_CHASSIS_TYPE, 5);
>  		dmi_save_ident(dm, DMI_CHASSIS_VERSION, 6);
>  		dmi_save_ident(dm, DMI_CHASSIS_SERIAL, 7);
>  		dmi_save_ident(dm, DMI_CHASSIS_ASSET_TAG, 8);
>  		break;
> -	case 9:		/* System Slots */
> +	case DMI_ENTRY_SYSTEM_SLOT:
>  		dmi_save_system_slot(dm);
>  		break;
> -	case 10:	/* Onboard Devices Information */
> +	case DMI_ENTRY_ONBOARD_DEVICE:
>  		dmi_save_devices(dm);
>  		break;
> -	case 11:	/* OEM Strings */
> +	case DMI_ENTRY_OEMSTRINGS:
>  		dmi_save_oem_strings_devices(dm);
>  		break;
> -	case 38:	/* IPMI Device Information */
> +	case DMI_ENTRY_IPMI_DEV:
>  		dmi_save_ipmi_device(dm);
>  		break;
> -	case 41:	/* Onboard Devices Extended Information */
> +	case DMI_ENTRY_ONBOARD_DEV_EXT:
>  		dmi_save_extended_devices(dm);
>  	}
>  }

I'm fine with this change.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Jean Delvare]

Hi Yazen, Mario,

On Wed, 17 Dec 2025 16:21:34 -0500, Yazen Ghannam wrote:
> On Wed, Dec 17, 2025 at 03:09:33PM -0600, Mario Limonciello wrote:
> > On 12/17/25 3:03 PM, Yazen Ghannam wrote:  
> > > On Tue, Dec 16, 2025 at 06:33:50AM -0600, Mario Limonciello (AMD) wrote:  
> > > > Type 40 entries (Additional information) are summarized in section
> > > > 7.41 as part of the SMBIOS specification.  Save these entries when
> > > > decoding the DMI tables.
> > > 
> > > Why can't an interested user just use dmidecode?
> > 
> > They could.  The reason for doing it in this series is the same reason for
> > the one that we did the S5 bit.
> > 
> > It shows up in the logs, you can tie regressions to the AGESA version at
> > specifically at the time of the failure if they've done BIOS updates since
> > then.  
> 
> Yes, right. Sorry, I mixed this up with the debugfs patch.
> 
> We need to save it here so the init code can find it.
> 
> But why do we need a debugfs entry for it?

FWIW, I had the exact same feeling when first gazing at this patch
series. I believe that every problem that can be solved in user-space
should be solved in user-space. Now I get the reason (explained above)
for logging the information at boot time, and thus the requirement to
parse type 40 in the kernel, but if there's no strong reason to have a
debugfs interface then I'd just drop it.

Of course, we also want to improve support for type 40 in dmidecode. I
admit I didn't pay too much attention to it so far due to a lack of use
case. But now that there's a use case, I'll be happy to work on it (or
commit a contribution if someone else beats me to it).

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 5/7] firmware: dmi: Add missing DMI entry types

[Jean Delvare]

On Tue, 16 Dec 2025 06:33:52 -0600, Mario Limonciello (AMD) wrote:
> Type 42 through type 46 entry types are missing from the
> definitions in the dmi_entry_type enum.
> 
> Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.1.pdf
> Suggested-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  include/linux/dmi.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 3fc3d334b321d..b7d4fdc25bfde 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -71,6 +71,10 @@ enum dmi_entry_type {
>  	DMI_ENTRY_ADDITIONAL,
>  	DMI_ENTRY_ONBOARD_DEV_EXT,
>  	DMI_ENTRY_MGMT_CONTROLLER_HOST,
> +	DMI_ENTRY_TPM_DEVICE,
> +	DMI_ENTRY_PROCESSOR_ADDITIONAL,
> +	DMI_ENTRY_FIRMWARE_INVENTORY,
> +	DMI_ENTRY_STRING_PROPERTY,
>  	DMI_ENTRY_INACTIVE = 126,
>  	DMI_ENTRY_END_OF_TABLE = 127,
>  };

Certainly can't hurt.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Jean Delvare]

Hi Mario,

On Tue, 16 Dec 2025 06:33:50 -0600, Mario Limonciello (AMD) wrote:
> Type 40 entries (Additional information) are summarized in section
> 7.41 as part of the SMBIOS specification.  Save these entries when
> decoding the DMI tables.
> 
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v2:
>  * Drop some unneeded variables (LKP robot)
>  * Allow any length strings, not just 5 and longer
> ---
>  drivers/firmware/dmi_scan.c | 38 +++++++++++++++++++++++++++++++++++++
>  include/linux/dmi.h         |  7 +++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 80aded4c778bc..ec84fe3935c1e 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -393,6 +393,40 @@ static void __init dmi_save_dev_pciaddr(int instance, int segment, int bus,
>  	list_add(&dev->dev.list, &dmi_devices);
>  }
>  
> +static void __init dmi_save_additional(const struct dmi_additional_info *info)
> +{
> +	const u8 *data;
> +	int i;
> +
> +	if (!info || info->header.length < 5 + info->count * 5)

Evaluating the second condition will access info->count, which is not
guaranteed to exist if info->header.length < 5. Of course if the DMI
table is compliant, it will be fine, but you can't rely on
firmware-provided data being sane.

Second problem is that you seem to assume that every entry has size 5
bytes. The SMBIOS specification says that the minimum size is 6 bytes,
and the actual size is stored in the first byte of each entry. So you
can't validate the total length upfront. You need to check as you walk
the table that the next entry would fit in the DMI record.

(The actual length will be 6 bytes if adding a value for an 8-bit
field, which is certainly the most frequent use case. But if adding a
value for a 16-bit field for example, then the entry would be 7 byte
long.)

> +		return;
> +
> +	data = info->entries;
> +
> +	for (i = 0; i < info->count; i++) {
> +		u8 string_num = data[i * 5 + 4];
> +		const char *string_ptr;
> +		char *value;
> +		int len;
> +
> +		string_ptr = dmi_string_nosave(&info->header, string_num);
> +		if (!string_ptr || !*string_ptr)
> +			continue;
> +
> +		len = strlen(string_ptr);
> +		if (len == 0)
> +			continue;
> +
> +		value = dmi_alloc(len + 1);
> +		if (!value)
> +			continue;
> +
> +		strscpy(value, string_ptr, len + 1);

Not sure why you allocate memory for the string, considering that
dmi_save_one_device() below does it too?

> +
> +		dmi_save_one_device(DMI_DEV_TYPE_ADDITIONAL, value);
> +	}
> +}

I'm not thrilled by the interface this offers. You end up arbitrarily
saving only one piece of information from the record (the free-form
string). You omit the reference handle (which isn't super-useful per
se, but could be used to find out the base DMI type, which would be
useful), nor the field offset within that DMI type, nor the new value
associated with that field.

This might be OK with your immediate use case, but lacks flexibility if
anyone else ever needs data from such a record.

Even for your use case, this seems pretty fragile. You end up
considering that any string starting with "AGESA" in a type 40 DMI
record is what you are looking for. While you don't have any guarantee
that the record in question relates to the CPU in general nor to one
specific type 4 record field. This could result in false positives.

> +
>  static void __init dmi_save_extended_devices(const struct dmi_header *dm)
>  {
>  	const char *name;
> @@ -526,8 +560,12 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
>  	case DMI_ENTRY_IPMI_DEV:
>  		dmi_save_ipmi_device(dm);
>  		break;
> +	case DMI_ENTRY_ADDITIONAL:
> +		dmi_save_additional((const struct dmi_additional_info *)dm);
> +		break;
>  	case DMI_ENTRY_ONBOARD_DEV_EXT:
>  		dmi_save_extended_devices(dm);
> +		break;
>  	}
>  }
>  
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index a809b5095c259..3fc3d334b321d 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -24,6 +24,7 @@ enum dmi_device_type {
>  	DMI_DEV_TYPE_OEM_STRING = -2,
>  	DMI_DEV_TYPE_DEV_ONBOARD = -3,
>  	DMI_DEV_TYPE_DEV_SLOT = -4,
> +	DMI_DEV_TYPE_ADDITIONAL = -5,
>  };
>  
>  enum dmi_entry_type {
> @@ -87,6 +88,12 @@ struct dmi_device {
>  	void *device_data;	/* Type specific data */
>  };
>  
> +struct dmi_additional_info {
> +	struct dmi_header header;
> +	u8 count;
> +	u8 entries[];
> +} __packed;
> +
>  #ifdef CONFIG_DMI
>  
>  struct dmi_dev_onboard {


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 7/7] x86/CPU/AMD: Output the AGESA version to the logs

[Jean Delvare]

Hi Mario,

On Tue, 16 Dec 2025 06:33:54 -0600, Mario Limonciello (AMD) wrote:
> On AMD Zen platforms that are running AGESA, there is sometimes
> DMI additional string for the AGESA version that can be helpful when
> debugging an issue.  If this string is found output to kernel logs.

This last sentence sounds clumsy.

> 
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  arch/x86/kernel/cpu/amd.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c19c4ee74dd1f..8f44439d3f993 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  #define pr_fmt(fmt) "x86/amd: " fmt
>  
> +#include <linux/dmi.h>
>  #include <linux/export.h>
>  #include <linux/bitops.h>
>  #include <linux/elf.h>
> @@ -1406,3 +1407,20 @@ static __init int print_s5_reset_status_mmio(void)
>  	return 0;
>  }
>  late_initcall(print_s5_reset_status_mmio);
> +
> +#ifdef CONFIG_DMI
> +static __init int print_agesa_dmi_info(void)
> +{
> +	const struct dmi_device *dev = NULL;
> +
> +	while ((dev = dmi_find_device(DMI_DEV_TYPE_ADDITIONAL, NULL, dev))) {
> +		if (!strncmp(dev->name, "AGESA", 5)) {
> +			pr_info("%s\n", dev->name);
> +			break;
> +		}
> +	}

In theory, type 40 DMI records are supposed to be a temporary
workaround until a new definition makes its way to the SMBIOS
specification. There is supposed to be a field in a standard DMI record
which will hold the same information in the future, at which point
systems will no longer need to provide the additional information entry.

I'm therefore surprised that you look for the AGESA information only in
type 40 and not in the standard DMI type where this piece of
information is supposed to be added in a near future.

So I checked my collection of DMI table dumps to find examples. I found
a number of occurrences of such type 40 entries. And it's a mess, I'm
sorry to say.

Some properly reference the Firmware Information handle (0x000) but
others reference handle 0xFFFF instead (which is invalid). Some
reference offset 0x05 within that record (which corresponds to the
Firmware Version string) while others reference offset 0x00 (which is
invalid). All have a 4-byte data field at the end of the entry, with
value 0x00000000, which is needlessly large.

This really looks like an abuse of DMI type 40. These entries are
supposed to overlay enumerated values in other types. This applies to
numeric fields only, overlaying a string field with another string
field is a technical non-sense.

Not sure what AMD engineers had in mind when implementing this. If they
wanted to mention the AGESA version in the firmware string, they could
have done that in DMI type 0, firmware version string directly.
Alternatively, OEM-specific records can be implemented, where hardware
vendors can store any kind of information they like. OEM strings (type
11) would also have been a valid alternative. Using DMI type 40 was
clearly not the right way to do it.

> +
> +	return 0;
> +}
> +late_initcall(print_agesa_dmi_info);
> +#endif


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2 3/7] firmware: dmi: Read additional information when decoding DMI table

[Jean DELVARE]

Hi all,

On Tue, 2025-12-16 at 06:33 -0600, Mario Limonciello (AMD) wrote:
> Type 40 entries (Additional information) are summarized in section
> 7.41 as part of the SMBIOS specification.  Save these entries when
> decoding the DMI tables.
> 
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v2:
>  * Drop some unneeded variables (LKP robot)
>  * Allow any length strings, not just 5 and longer
> --- (...)

Having read the whole series now, my conclusion is that we don't want
to do that. The way AMD is storing this string in a type 40 record is
absolutely not how these records are supposed to be used. So having
generic code to store and retrieve strings from type 40 records is
neither needed nor desirable.

This is a hack from AMD, the best way to deal with it is with the
minimum amount of code in the kernel and smallest memory footprint.
Simply catch the string when you see it, log it, and be done with it.

Thanks,
-- 
Jean Delvare
SUSE L3 Support