Re: [PATCH v2] x86: cpu: x86_64: Detect CPU vendor and device ID using cpuid

Hi Desapogu,

On 2026-08-14T17:18:29, 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
> stores cpuid_eax(1) in gd->arch.x86_device so later x86 code can use the
> detected CPU indentity.

Please use imperative tense - 'Also store' rather than 'Also stores' -
and fix the typo 'indentity' -> 'identity'. Also missing a space in
'paths(e.g.'.

>
> 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/x86_64/cpu.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

> diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
> @@ -50,8 +50,21 @@ static void setup_sse_features(void)
>  int x86_cpu_reinit_f(void)
>  {
> -     /* set the vendor to Intel so that native_calibrate_tsc() works */
> -     gd->arch.x86_vendor = X86_VENDOR_INTEL;
> +     struct cpuid_result res;
> +     char vendor[13];
> +
> +     res = cpuid(0x00000000);
> +     memcpy(&vendor[0], &res.ebx, 4);
> +     memcpy(&vendor[4], &res.edx, 4);
> +     memcpy(&vendor[8], &res.ecx, 4);
> +     vendor[12] = '\0';
> +     if (!strcmp(vendor, "GenuineIntel"))
> +             gd->arch.x86_vendor = X86_VENDOR_INTEL; /* native_calibrate_tsc() works*/
> +     else if (!strcmp(vendor, "AuthenticAMD"))
> +             gd->arch.x86_vendor = X86_VENDOR_AMD;
> +     else
> +             gd->arch.x86_vendor = X86_VENDOR_ANY;
> +     gd->arch.x86_device = cpuid_eax(1);

This duplicates logic in arch/x86/cpu/i386/cpu.c - see
x86_cpu_vendor_info() (declared in asm/u-boot-x86.h) and the
x86_vendors[] table. Please refactor so the vendor table and the
leaf-0 read live in a shared file built for both i386 and x86_64, and
call it from here. That way you pick up all the vendors (Cyrix,
Centaur, Transmeta, etc.) for free and there is only one place to
maintain the mapping.

Note that identify_cpu() in i386/cpu.c uses X86_VENDOR_UNKNOWN as the
fallback; only the TPL fast-path uses X86_VENDOR_ANY. Please stay
consistent with that.

The trailing '/* native_calibrate_tsc() works*/' is missing the space
before '*/' and reads oddly on the Intel branch - either drop it or
restore the original comment above the block.

Regards,
Simon