Re: [PATCH v6 5/5] x86/CPU/AMD: Print AGESA string from DMI additional information entry

[Jean Delvare <jdelvare@suse.de>]

On Sat,  7 Mar 2026 08:10:24 -0600, Mario Limonciello (AMD) wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> Type 40 entries (Additional Information) are summarized in section 7.41
> as part of the SMBIOS specification.  Generally, these entries aren't
> interesting to save.
> 
> However on some AMD Zen systems, the AGESA version is stored here. This
> is useful to save to the kernel message logs for debugging. It can be
> used to cross-reference issues.
> 
> Implement an iterator for the Additional Information entries. Use this
> to find and print the AGESA string. Do so in AMD code, since the use
> case is AMD-specific.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Co-developed-by: "Mario Limonciello (AMD)" <superm1@kernel.org>
> Signed-off-by: "Mario Limonciello (AMD)" <superm1@kernel.org>

Looks good to me.

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

I only have one question:

> (...)
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> (...)
> +static void __init dmi_scan_additional(const struct dmi_header *d, void *p)
> +{
> +	struct dmi_a_info *info = (struct dmi_a_info *)d;
> +	void *next, *end;
> +
> +	if (!IS_ENABLED(CONFIG_DMI))
> +		return;
> +
> +	if (info->header.type != DMI_ENTRY_ADDITIONAL ||
> +	    info->header.length < DMI_A_INFO_MIN_SIZE ||
> +	    info->count < 1)
> +		return;
> +
> +	next = (void *)(info + 1);
> +	end  = (void *)info + info->header.length;
> +
> +	do {
> +		struct dmi_a_info_entry *entry;
> +		const char *string_ptr;
> +
> +		entry = (struct dmi_a_info_entry *)next;
> +
> +		/*
> +		 * Not much can be done to validate data. At least the entry
> +		 * length shouldn't be 0.
> +		 */
> +		if (!entry->length)
> +			return;
> +
> +		string_ptr = dmi_string_nosave(&info->header, entry->str_num);
> +
> +		/* Sample string: AGESA!V9 StrixKrackanPI-FP8 1.1.0.0c */
> +		if (!strncmp(string_ptr, "AGESA!V9", 8) && strlen(string_ptr) > 9) {

Isn't "V9" going to become "V10" at some point in the future, and then
the AGESA string will no longer be displayed until the code is
adjusted? Or will AMD move the information to a more appropriate place
by then?

Also, I know this was discussed before, but the current print strategy
omits "V9". Is this not something we are going to regret once more
versions are available and we may have to differentiate between them?

> +			pr_info("AGESA: %s\n", string_ptr + 9);
> +			break;
> +		}
> +
> +		next += entry->length;
> +	} while (end - next >= DMI_A_INFO_ENT_MIN_SIZE);
> +}

-- 
Jean Delvare
SUSE L3 Support