Hi Desapogu,

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).

Regards,
Simon

Reply via email to