| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3ec8b151-5f3a-48e4-bcb2-3e8524d85083@kernel.org>
Date: Wed, 21 Jan 2026 14:50:49 -0600
From: Mario Limonciello <superm1@...nel.org>
To: Yazen Ghannam <yazen.ghannam@....com>
Cc: Jean Delvare <jdelvare@...e.com>, linux-kernel@...r.kernel.org,
Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v4 6/6] x86/CPU/AMD: Print AGESA string from DMI
additional information entry
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 <yazen.ghannam@....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@....com>
>> Co-developed-by: "Mario Limonciello (AMD)" <superm1@...nel.org>
>> Signed-off-by: "Mario Limonciello (AMD)" <superm1@...nel.org>
>> ---
>> 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 <linux/export.h>
>> #include <linux/bitops.h>
>> +#include <linux/dmi.h>
>> #include <linux/elf.h>
>> #include <linux/mm.h>
>> #include <linux/kvm_types.h>
>> @@ -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.
Powered by blists - more mailing lists