AMD General Hi Simon,
On 2026-08-25T14:44:21, Desapogu Jayaramudu <[email protected]> wrote: > x86: cpu: x86_64: Detect CPU vendor and device ID using cpuid > > When U-Boot is configured as a coreboot payload on x86_64, the current > code leaves the CPU identity at the default Intel vendor and device ID > 0x0, even on non-Intel platforms. > > Read CPUID leaf 0 and build the 12-byte vendor string from EBX:EDX:ECX > to identify the CPU vendor at runtime. Set gd->arch.x86_vendor to > Intel/AMD when matched, with fallback to X86_VENDOR_ANY for unknown > vendors. Also store cpuid_eax(1) in gd->arch.x86_device so later x86 > code can use the detected CPU identity. > > This avoids relying on a fixed vendor value and keeps vendor-sensitive > paths (e.g. TSC calibration) aligned with the actual CPU. > > Signed-off-by: Desapogu Jayaramudu <[email protected]> > > arch/x86/cpu/Makefile | 1 + > arch/x86/cpu/i386/cpu.c | 51 ++------------------------------------ > arch/x86/cpu/vendor.c | 52 > +++++++++++++++++++++++++++++++++++++++ > arch/x86/cpu/x86_64/cpu.c | 20 ++++++++++++--- > arch/x86/include/asm/u-boot-x86.h | 16 ++++++++++++ > 5 files changed, 88 insertions(+), 52 deletions(-) > diff --git a/arch/x86/cpu/vendor.c b/arch/x86/cpu/vendor.c @@ -0,0 > +1,52 @@ > +int x86_vendor_name_to_id(const char *name) { > + int i; > + > + for (i = 0; i < ARRAY_SIZE(x86_vendors) / sizeof(x86_vendors[0]); i++) { > + if (!memcmp(name, x86_vendors[i].name, 12)) > + return x86_vendors[i].vendor; > + } > + > + return X86_VENDOR_ANY; > +} ARRAY_SIZE() already expands to sizeof(arr) / sizeof(arr[0]), so dividing again gives 11 / 16 == 0 and the loop never runs - every caller gets X86_VENDOR_ANY back, including GenuineIntel and AuthenticAMD. Please drop the extra division. > diff --git a/arch/x86/cpu/vendor.c b/arch/x86/cpu/vendor.c @@ -0,0 > +1,52 @@ > + return X86_VENDOR_ANY; > +} identify_cpu() in i386/cpu.c uses X86_VENDOR_UNKNOWN as its fallback, and folding that call site into this helper silently changes it to X86_VENDOR_ANY. Please have the helper return X86_VENDOR_UNKNOWN and let the x86_64 caller substitute X86_VENDOR_ANY itself if it wants that. > diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c @@ > -48,11 +49,23 @@ static void setup_sse_features(void) > + if (cpuid_level >= 0x00000001) > + gd->arch.x86_device = cpuid_eax(0x00000001); > + else > + gd->arch.x86_device = 0x000004000; Where does 0x000004000 come from? It not look like any encoded family/model value I recognise - identify_cpu() in i386/cpu.c just sets c->x86 = 4 in the equivalent path. Please either use a named constant with a comment explaining the encoding, or drop the else branch (leaving x86_device at 0 is no worse than a bogus value). I have addressed all your comments in v4 patch (https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/). Thanks for the review. Regards, Jayaram
