From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B462F41C317 for ; Wed, 21 Jan 2026 20:50:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769028650; cv=none; b=h+HT9oOWeUa53JGCyEVj54r8tln7/ET/dPJlnHeb3jeSckVdjKxp7eOHC55DrxLYYsTUs1ubUodwFYHWAOZv5yqPIpx4YFf57Bv2wzKGvLgE7hoN7A/zE7wvhFPZh4jXvuie8rcejOXafYVYj9swVSbdv602vc5K/WhaipDTygQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769028650; c=relaxed/simple; bh=q8PaMK3gWkUm+OBOvpWJvwInabeswwUlaZK03lmcErU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=PNW9KqJ4Xe01iaNaUhwfMDgV0I94gUcFGsgzOLsFQ2VuFQ8RzF4g4wTX9DsOSf8YbST197/rViI1edWCux8LAS1lz008JHzZboKzm2rAIphPS3fbMLhOKE3l5Rd/5Vc7ubtamZJgSuF5Ns3qirPc6qWSaTW4u97G0aN8CZKIB5M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ohmZMiae; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ohmZMiae" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E5395C4CEF1; Wed, 21 Jan 2026 20:50:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769028650; bh=q8PaMK3gWkUm+OBOvpWJvwInabeswwUlaZK03lmcErU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=ohmZMiae6gniKWjPmSsybhkFBqj1pgiP6VZJ/Cx43lgTMtOJloSpm78O77rhtpCGd p/VQ0yQYKJ3wbXe30j1e3Zw0oa5FXm1wtoydeVQ3uFiLOVkvZ/N5KpjlKUQHl1Y/BI rLXd81hAuw0uABmqkQRu+Qz/A4jAC6K89laqD+e85mOuUbEEIvs2IUSVS0Z136vp+s IoFfLRyLJKNDMXIGV/IG8CyueAs3jlD73bWNvJ7EC2UN/5HNPQa5/Gd0HeOMqYFt0V OJnvZkd0FxbQxzUvS+/JPiCtiYv9anMndSvVCNwZL0CEFoPnlYixIE1GWDnC1o84WS jFKza1pueRMQA== Message-ID: <3ec8b151-5f3a-48e4-bcb2-3e8524d85083@kernel.org> Date: Wed, 21 Jan 2026 14:50:49 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 6/6] x86/CPU/AMD: Print AGESA string from DMI additional information entry To: Yazen Ghannam Cc: Jean Delvare , linux-kernel@vger.kernel.org, Borislav Petkov References: <20260121060431.432350-1-superm1@kernel.org> <20260121060431.432350-7-superm1@kernel.org> <20260121142654.GA1784626@yaz-khff2.amd.com> Content-Language: en-US From: Mario Limonciello In-Reply-To: <20260121142654.GA1784626@yaz-khff2.amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 1/21/26 8:26 AM, Yazen Ghannam wrote: > On Wed, Jan 21, 2026 at 12:04:31AM -0600, Mario Limonciello (AMD) wrote: >> From: Yazen Ghannam >> >> 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 >> Co-developed-by: "Mario Limonciello (AMD)" >> Signed-off-by: "Mario Limonciello (AMD)" >> --- >> v4: >> * New patch (based upon older versions though) >> --- >> arch/x86/kernel/cpu/amd.c | 53 +++++++++++++++++++++++++++++++++++++ >> drivers/firmware/dmi_scan.c | 3 ++- >> include/linux/dmi.h | 18 +++++++++++++ >> 3 files changed, 73 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c >> index c19c4ee74dd1f..5cd60855a85b0 100644 >> --- a/arch/x86/kernel/cpu/amd.c >> +++ b/arch/x86/kernel/cpu/amd.c >> @@ -3,6 +3,7 @@ >> >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1406,3 +1407,55 @@ static __init int print_s5_reset_status_mmio(void) >> return 0; >> } >> late_initcall(print_s5_reset_status_mmio); >> + >> +static void __init amd_dmi_scan_additional(const struct dmi_header *d, void *p) >> +{ >> + struct dmi_a_info *info = (struct dmi_a_info *)d; >> + void *next, *end; >> + >> + /* >> + * DMI Additional Info table has a 'count' field. But it's not very >> + * helpful since the entries are variable length. So don't use it. >> + */ >> + if (info->header.type != DMI_ENTRY_ADDITIONAL || >> + info->header.length < DMI_A_INFO_MIN_SIZE) >> + return; >> + >> + /* >> + * Get the first entry. >> + * The minimum table size guarantees at least one entry is present. >> + */ >> + 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); >> + >> + /* Only one AGESA string is expected. */ >> + if (!strncmp(string_ptr, "AGESA", 5)) { >> + pr_info("%s\n", string_ptr); >> + break; >> + } >> + >> + next += entry->length; >> + } while (end - next >= DMI_A_INFO_ENT_MIN_SIZE); >> +} >> + >> +static __init int print_dmi_agesa(void) >> +{ >> + dmi_walk(amd_dmi_scan_additional, NULL); >> + return 0; >> +} >> +late_initcall(print_dmi_agesa); >> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c >> index ed6235ac576b6..a3f7dabd49554 100644 >> --- a/drivers/firmware/dmi_scan.c >> +++ b/drivers/firmware/dmi_scan.c >> @@ -47,7 +47,7 @@ static struct dmi_memdev_info { >> static int dmi_memdev_nr; >> static int dmi_memdev_populated_nr __initdata; >> >> -static const char * __init dmi_string_nosave(const struct dmi_header *dm, u8 s) >> +const char *dmi_string_nosave(const struct dmi_header *dm, u8 s) >> { >> const u8 *bp = ((u8 *) dm) + dm->length; >> const u8 *nsp; >> @@ -66,6 +66,7 @@ static const char * __init dmi_string_nosave(const struct dmi_header *dm, u8 s) >> >> return dmi_empty_string; >> } >> +EXPORT_SYMBOL_GPL(dmi_string_nosave); >> >> static const char * __init dmi_string(const struct dmi_header *dm, u8 s) >> { >> diff --git a/include/linux/dmi.h b/include/linux/dmi.h >> index 2eedf44e68012..b7865fa387ccb 100644 >> --- a/include/linux/dmi.h >> +++ b/include/linux/dmi.h >> @@ -91,6 +91,21 @@ struct dmi_device { >> void *device_data; /* Type specific data */ >> }; >> >> +#define DMI_A_INFO_ENT_MIN_SIZE 0x6 >> +struct dmi_a_info_entry { >> + u8 length; >> + u16 handle; >> + u8 offset; >> + u8 str_num; >> + u8 value[]; >> +} __packed; >> + >> +#define DMI_A_INFO_MIN_SIZE 0xB >> +struct dmi_a_info { >> + struct dmi_header header; >> + u8 count; >> +} __packed; >> + >> #ifdef CONFIG_DMI >> >> struct dmi_dev_onboard { >> @@ -120,6 +135,7 @@ extern void dmi_memdev_name(u16 handle, const char **bank, const char **device); >> extern u64 dmi_memdev_size(u16 handle); >> extern u8 dmi_memdev_type(u16 handle); >> extern u16 dmi_memdev_handle(int slot); >> +const char *dmi_string_nosave(const struct dmi_header *dm, u8 s); >> >> #else >> >> @@ -153,6 +169,8 @@ static inline u8 dmi_memdev_type(u16 handle) { return 0x0; } >> static inline u16 dmi_memdev_handle(int slot) { return 0xffff; } >> static inline const struct dmi_system_id * >> dmi_first_match(const struct dmi_system_id *list) { return NULL; } >> +static inline const char * >> + dmi_string_nosave(const struct dmi_header *dm, u8 s) { return dmi_empty_string; } >> > > The dmi_empty_string needs to be moved to this header file from > dmi_scan.c. > > Otherwise, there's a build issue as the test bot reported. > > Thanks, > Yazen I don't think it's actually appropriate to move dmi_empty_string in this case. It's a static variable, shouldn't really be in a header. I would think it's better to just return NULL.