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

[Mario Limonciello]

On 3/15/26 8:14 AM, Borislav Petkov wrote:
> On Fri, Jan 23, 2026 at 03:30:56PM -0600, Mario Limonciello (AMD) wrote:
> > From: Yazen Ghannam <yazen.ghannam@xxxxxxx>
> >
> > 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.
>
> Look what I said the last time:

FYI - you're replying to the v5 not the v6 [1].

Link: 
https://lore.kernel.org/all/20260307141024.819807-1-superm1@xxxxxxxxxx/ [1]

> https://lore.kernel.org/r/20260126130755.GIaXdnK65MTIiXqg-l@fat_crate.local
>
> I'll have to teach AI to give feedback for commit messages explaining the
> diff.

Sorry about that one, I missed it even in v6.

> > Signed-off-by: Yazen Ghannam <yazen.ghannam@xxxxxxx>
> > Co-developed-by: "Mario Limonciello (AMD)" <superm1@xxxxxxxxxx>
> > Signed-off-by: "Mario Limonciello (AMD)" <superm1@xxxxxxxxxx>
> > ---
> > v5:
> >   * Return "" in !CONFIG_DMI case (LKP robot, Yazen)
> > 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 bc94ff1e250ad..aa04a27aeb107 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
> >   #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>
> > @@ -1404,3 +1405,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;
>
>  From last time:
>
> "This end needs to be corrected to some sane upper limit because we don't trust
> firmware one bit."

I did add a check for info->count < 1, but that's not what you were 
actually looking for.  Do you have a suggestion for an upper limit to 
bound things?

I was thinking something like this:

#define DMI_LIMIT 10
int limit = info -> count > DMI_LIMIT ? DMI_LIMIT : info->count;
end  = (void *)info + limit * info->header.length;