Hi Simon,
On Fri, Nov 7, 2014 at 10:22 AM, Simon Glass <s...@chromium.org> wrote:
> Hi Bin,
>
> On 4 November 2014 07:58, Bin Meng <bmeng...@gmail.com> wrote:
>> Currently only basic CPU information (x86 or x86_64) is displayed
>> on boot. This commit adds more detailed information output including
>> CPU vendor name, device id, family, model and stepping as well as
>> the CPU brand string, all of which are extracted from CPUID result.
>>
>> The CPU identification happens in x86_cpu_init_f() and corresponding
>> fields are saved in the global data. Later print_cpuinfo() just uses
>> these fields to display CPU information without the need to probe
>> again in real time.
>>
>> Signed-off-by: Bin Meng <bmeng...@gmail.com>
>> ---
>> arch/x86/cpu/cpu.c | 282
>> +++++++++++++++++++++++++++++++------
>> arch/x86/include/asm/cpu.h | 142 +++++++++++++++++++
>> arch/x86/include/asm/global_data.h | 5 +
>> 3 files changed, 385 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
>> index 2e25253..e9058f7 100644
>> --- a/arch/x86/cpu/cpu.c
>> +++ b/arch/x86/cpu/cpu.c
>> @@ -13,6 +13,9 @@
>> * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>> * Alex Zuepke <a...@sysgo.de>
>> *
>> + * Part of this file is adapted from coreboot
>> + * src/arch/x86/lib/cpu.c
>> + *
>> * SPDX-License-Identifier: GPL-2.0+
>> */
>>
>> @@ -27,6 +30,8 @@
>> #include <asm/interrupt.h>
>> #include <linux/compiler.h>
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> /*
>> * Constructor for a conventional segment GDT (or LDT) entry
>> * This is a macro so it can be used in initialisers
>> @@ -43,6 +48,51 @@ struct gdt_ptr {
>> u32 ptr;
>> } __packed;
>>
>> +struct cpu_device_id {
>> + unsigned vendor;
>> + unsigned device;
>> +};
>> +
>> +struct cpuinfo_x86 {
>> + uint8_t x86; /* CPU family */
>> + uint8_t x86_vendor; /* CPU vendor */
>> + uint8_t x86_model;
>> + uint8_t x86_mask;
>> +};
>> +
>> +/* List of cpu vendor strings along with their normalized
>
> Can we put /* on its own line?
>
> /*
> * List of CPU vendor strings ...
> * ...
> */
>
Sure.
>> + * id values.
>> + */
>> +static struct {
>> + int vendor;
>> + const char *name;
>> +} x86_vendors[] = {
>> + { X86_VENDOR_INTEL, "GenuineIntel", },
>> + { X86_VENDOR_CYRIX, "CyrixInstead", },
>> + { X86_VENDOR_AMD, "AuthenticAMD", },
>> + { X86_VENDOR_UMC, "UMC UMC UMC ", },
>> + { X86_VENDOR_NEXGEN, "NexGenDriven", },
>> + { X86_VENDOR_CENTAUR, "CentaurHauls", },
>> + { X86_VENDOR_RISE, "RiseRiseRise", },
>> + { X86_VENDOR_TRANSMETA, "GenuineTMx86", },
>> + { X86_VENDOR_TRANSMETA, "TransmetaCPU", },
>> + { X86_VENDOR_NSC, "Geode by NSC", },
>> + { X86_VENDOR_SIS, "SiS SiS SiS ", },
>> +};
>> +
>> +static const char *x86_vendor_name[] = {
>> + [X86_VENDOR_INTEL] = "Intel",
>> + [X86_VENDOR_CYRIX] = "Cyrix",
>> + [X86_VENDOR_AMD] = "AMD",
>> + [X86_VENDOR_UMC] = "UMC",
>> + [X86_VENDOR_NEXGEN] = "NexGen",
>> + [X86_VENDOR_CENTAUR] = "Centaur",
>> + [X86_VENDOR_RISE] = "Rise",
>> + [X86_VENDOR_TRANSMETA] = "Transmeta",
>> + [X86_VENDOR_NSC] = "NSC",
>> + [X86_VENDOR_SIS] = "SiS",
>> +};
>> +
>> static void load_ds(u32 segment)
>> {
>> asm volatile("movl %0, %%ds" : : "r" (segment * X86_GDT_ENTRY_SIZE));
>> @@ -115,6 +165,131 @@ int __weak x86_cleanup_before_linux(void)
>> return 0;
>> }
>>
>> +/*
>> + * Cyrix CPUs without cpuid or with cpuid not yet enabled can be detected
>> + * by the fact that they preserve the flags across the division of 5/2.
>> + * PII and PPro exhibit this behavior too, but they have cpuid available.
>> + */
>> +
>> +/*
>> + * Perform the Cyrix 5/2 test. A Cyrix won't change
>> + * the flags, while other 486 chips will.
>> + */
>> +static inline int test_cyrix_52div(void)
>> +{
>> + unsigned int test;
>> +
>> + __asm__ __volatile__(
>> + "sahf\n\t" /* clear flags (%eax = 0x0005) */
>> + "div %b2\n\t" /* divide 5 by 2 */
>> + "lahf" /* store flags into %ah */
>> + : "=a" (test)
>> + : "0" (5), "q" (2)
>> + : "cc");
>> +
>> + /* AH is 0x02 on Cyrix after the divide.. */
>> + return (unsigned char) (test >> 8) == 0x02;
>> +}
>> +
>> +/*
>> + * Detect a NexGen CPU running without BIOS hypercode new enough
>> + * to have CPUID. (Thanks to Herbert Oppmann)
>> + */
>> +
>> +static int deep_magic_nexgen_probe(void)
>> +{
>> + int ret;
>> +
>> + __asm__ __volatile__ (
>> + " movw $0x5555, %%ax\n"
>> + " xorw %%dx,%%dx\n"
>> + " movw $2, %%cx\n"
>> + " divw %%cx\n"
>> + " movl $0, %%eax\n"
>> + " jnz 1f\n"
>> + " movl $1, %%eax\n"
>> + "1:\n"
>> + : "=a" (ret) : : "cx", "dx" );
>> + return ret;
>> +}
>> +
>> +static bool has_cpuid(void)
>> +{
>> + return flag_is_changeable_p(X86_EFLAGS_ID);
>> +}
>> +
>> +static void identify_cpu(struct cpu_device_id *cpu)
>> +{
>> + char vendor_name[16];
>> + int i;
>> +
>> + vendor_name[0] = '\0'; /* Unset */
>> +
>> + /* Find the id and vendor_name */
>> + if (!has_cpuid()) {
>> + /* Its a 486 if we can modify the AC flag */
>> + if (flag_is_changeable_p(X86_EFLAGS_AC)) {
>> + cpu->device = 0x00000400; /* 486 */
>> + } else {
>> + cpu->device = 0x00000300; /* 386 */
>> + }
>> + if ((cpu->device == 0x00000400) && test_cyrix_52div()) {
>> + memcpy(vendor_name, "CyrixInstead", 13);
>> + /* If we ever care we can enable cpuid here */
>> + }
>> + /* Detect NexGen with old hypercode */
>> + else if (deep_magic_nexgen_probe()) {
>> + memcpy(vendor_name, "NexGenDriven", 13);
>> + }
>> + }
>> + if (has_cpuid()) {
>> + int cpuid_level;
>> + struct cpuid_result result;
>
> blank line between declaration &code
OK.
>> + result = cpuid(0x00000000);
>> + cpuid_level = result.eax;
>> + vendor_name[ 0] = (result.ebx >> 0) & 0xff;
>> + vendor_name[ 1] = (result.ebx >> 8) & 0xff;
>> + vendor_name[ 2] = (result.ebx >> 16) & 0xff;
>> + vendor_name[ 3] = (result.ebx >> 24) & 0xff;
>> + vendor_name[ 4] = (result.edx >> 0) & 0xff;
>> + vendor_name[ 5] = (result.edx >> 8) & 0xff;
>> + vendor_name[ 6] = (result.edx >> 16) & 0xff;
>> + vendor_name[ 7] = (result.edx >> 24) & 0xff;
>> + vendor_name[ 8] = (result.ecx >> 0) & 0xff;
>> + vendor_name[ 9] = (result.ecx >> 8) & 0xff;
>> + vendor_name[10] = (result.ecx >> 16) & 0xff;
>> + vendor_name[11] = (result.ecx >> 24) & 0xff;
>
> I think the byte loop approach is better (see cpu_get_name() in my
> series). Can it be done? If you put this name bit in a separate
> function it will be cleaner, too.
Seems you were confused by the codes here. The cpu_get_name() in your
series is the same function as the fill_processor_name() in my patch,
but it is not equal to the code logic here. The code here is to
retrive the correct vendor name string from ebx/ecx/edx as the result
in ebx/ecx/edx is disordered.
>> + vendor_name[12] = '\0';
>> +
>> + /* Intel-defined flags: level 0x00000001 */
>> + if (cpuid_level >= 0x00000001) {
>> + cpu->device = cpuid_eax(0x00000001);
>> + }
>> + else {
>> + /* Have CPUID level 0 only unheard of */
>> + cpu->device = 0x00000400;
>> + }
>> + }
>> + cpu->vendor = X86_VENDOR_UNKNOWN;
>> + for(i = 0; i < ARRAY_SIZE(x86_vendors); i++) {
>> + if (memcmp(vendor_name, x86_vendors[i].name, 12) == 0) {
>> + cpu->vendor = x86_vendors[i].vendor;
>> + break;
>> + }
>> + }
>> +}
>> +
>> +static inline void get_fms(struct cpuinfo_x86 *c, uint32_t tfms)
>> +{
>> + c->x86 = (tfms >> 8) & 0xf;
>> + c->x86_model = (tfms >> 4) & 0xf;
>> + c->x86_mask = tfms & 0xf;
>> + if (c->x86 == 0xf)
>> + c->x86 += (tfms >> 20) & 0xff;
>> + if (c->x86 >= 0x6)
>> + c->x86_model += ((tfms >> 16) & 0xF) << 4;
>> +}
>> +
>> int x86_cpu_init_f(void)
>> {
>> const u32 em_rst = ~X86_CR0_EM;
>> @@ -128,6 +303,20 @@ int x86_cpu_init_f(void)
>> "movl %%eax, %%cr0\n" \
>> : : "i" (em_rst), "i" (mp_ne_set) : "eax");
>>
>> + /* identify CPU via cpuid and store the decoded info into gd->arch */
>> + if (has_cpuid()) {
>
> Why not move this into print_cpuinfo()? Then you can print debug
> information if the CPU is not known. Or will it always be known? For
> some reason for me it always says '<invalid cpu vendor>'
The following patch "x86: Do TSC MSR calibration only for
known/supported CPUs" needs to access gd->arch.x86 and
gd->arch.x86_model so the cpu identification is put here. I can move
this into print_cpu_info() but that means only after print_cpu_info()
is called can TSC timer driver (the udelay API) be available for use.
Is this an acceptable limitation?
'<invalid cpu vendor>' on you side looks weird to me. Do you make sure
x86_cpu_init_f() gets called by U-Boot? The print_cpu_info() later
will use gd->arch.x86_vendor which is initialized here by cpu.vendor.
>> + struct cpu_device_id cpu;
>> + struct cpuinfo_x86 c;
>> +
>> + identify_cpu(&cpu);
>> + get_fms(&c, cpu.device);
>> + gd->arch.x86 = c.x86;
>> + gd->arch.x86_vendor = cpu.vendor;
>> + gd->arch.x86_model = c.x86_model;
>> + gd->arch.x86_mask = c.x86_mask;
>> + gd->arch.x86_device = cpu.device;
>> + }
>> +
>> return 0;
>> }
>> int cpu_init_f(void) __attribute__((weak, alias("x86_cpu_init_f")));
>> @@ -279,55 +468,14 @@ void cpu_disable_paging_pae(void)
>> : "eax");
>> }
>>
>> -static bool has_cpuid(void)
>> -{
>> - unsigned long flag;
>> -
>> - asm volatile("pushf\n" \
>> - "pop %%eax\n"
>> - "mov %%eax, %%ecx\n" /* ecx = flags */
>> - "xor %1, %%eax\n"
>> - "push %%eax\n"
>> - "popf\n" /* flags ^= $2 */
>> - "pushf\n"
>> - "pop %%eax\n" /* eax = flags */
>> - "push %%ecx\n"
>> - "popf\n" /* flags = ecx */
>> - "xor %%ecx, %%eax\n"
>> - "mov %%eax, %0"
>> - : "=r" (flag)
>> - : "i" (1 << 21)
>> - : "eax", "ecx", "memory");
>> -
>> - return flag != 0;
>> -}
>> -
>> static bool can_detect_long_mode(void)
>> {
>> - unsigned long flag;
>> -
>> - asm volatile("mov $0x80000000, %%eax\n"
>> - "cpuid\n"
>> - "mov %%eax, %0"
>> - : "=r" (flag)
>> - :
>> - : "eax", "ebx", "ecx", "edx", "memory");
>> -
>> - return flag > 0x80000000UL;
>> + return cpuid_eax(0x80000000) > 0x80000000UL;
>> }
>>
>> static bool has_long_mode(void)
>> {
>> - unsigned long flag;
>> -
>> - asm volatile("mov $0x80000001, %%eax\n"
>> - "cpuid\n"
>> - "mov %%edx, %0"
>> - : "=r" (flag)
>> - :
>> - : "eax", "ebx", "ecx", "edx", "memory");
>> -
>> - return flag & (1 << 29) ? true : false;
>> + return cpuid_edx(0x80000001) & (1 << 29) ? true : false;
>> }
>>
>> int cpu_has_64bit(void)
>> @@ -336,9 +484,55 @@ int cpu_has_64bit(void)
>> has_long_mode();
>> }
>>
>> +static const char *cpu_vendor_name(int vendor)
>> +{
>> + const char *name;
>> + name = "<invalid cpu vendor>";
>> + if ((vendor < (ARRAY_SIZE(x86_vendor_name))) &&
>> + (x86_vendor_name[vendor] != 0))
>> + {
>> + name = x86_vendor_name[vendor];
>> + }
>> + return name;
>> +}
>> +
>> +static void fill_processor_name(char *processor_name)
>> +{
>> + struct cpuid_result regs;
>> + char temp_processor_name[49];
>> + char *processor_name_start;
>> + unsigned int *name_as_ints = (unsigned int *)temp_processor_name;
>> + int i;
>> +
>> + for (i = 0; i < 3; i++) {
>> + regs = cpuid(0x80000002 + i);
>> + name_as_ints[i * 4 + 0] = regs.eax;
>> + name_as_ints[i * 4 + 1] = regs.ebx;
>> + name_as_ints[i * 4 + 2] = regs.ecx;
>> + name_as_ints[i * 4 + 3] = regs.edx;
>> + }
>> +
>> + temp_processor_name[48] = 0;
>> +
>> + /* Skip leading spaces. */
>> + processor_name_start = temp_processor_name;
>> + while (*processor_name_start == ' ')
>> + processor_name_start++;
>> +
>> + memset(processor_name, 0, 49);
>> + strcpy(processor_name, processor_name_start);
>> +}
>> +
>> int print_cpuinfo(void)
>> {
>> - printf("CPU: %s\n", cpu_has_64bit() ? "x86_64" : "x86");
>> + char processor_name[49];
>> +
>> + printf("CPU: %s, vendor %s, device %xh\n", cpu_has_64bit() ?
>> "x86_64" : "x86",
>> + cpu_vendor_name(gd->arch.x86_vendor), gd->arch.x86_device);
>> + printf("CPU: family %02xh, model %02xh, stepping %02xh\n",
>> + gd->arch.x86, gd->arch.x86_model, gd->arch.x86_mask);
>> + fill_processor_name(processor_name);
>> + printf("CPU: %s\n", processor_name);
>>
>> return 0;
>> }
>> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
>> index 6c6774a..6cd9f5b 100644
>> --- a/arch/x86/include/asm/cpu.h
>> +++ b/arch/x86/include/asm/cpu.h
>> @@ -1,12 +1,154 @@
>> /*
>> * Copyright (c) 2014 The Chromium OS Authors.
>> *
>> + * Part of this file is adapted from coreboot
>> + * src/arch/x86/include/arch/cpu.h and
>> + * src/arch/x86/lib/cpu.c
>> + *
>> * SPDX-License-Identifier: GPL-2.0+
>> */
>>
>> #ifndef __X86_CPU_H
>> #define __X86_CPU_H
>
> _ASM_CPU_H
OK
>> +#define X86_VENDOR_INVALID 0
>> +#define X86_VENDOR_INTEL 1
>> +#define X86_VENDOR_CYRIX 2
>> +#define X86_VENDOR_AMD 3
>> +#define X86_VENDOR_UMC 4
>> +#define X86_VENDOR_NEXGEN 5
>> +#define X86_VENDOR_CENTAUR 6
>> +#define X86_VENDOR_RISE 7
>> +#define X86_VENDOR_TRANSMETA 8
>> +#define X86_VENDOR_NSC 9
>> +#define X86_VENDOR_SIS 10
>> +#define X86_VENDOR_ANY 0xfe
>> +#define X86_VENDOR_UNKNOWN 0xff
>
> Can we use an enum for this?
Sure.
>> +
>> +struct cpuid_result {
>> + uint32_t eax;
>> + uint32_t ebx;
>> + uint32_t ecx;
>> + uint32_t edx;
>> +};
>> +
>> +/*
>> + * Generic CPUID function
>> + */
>> +static inline struct cpuid_result cpuid(int op)
>> +{
>> + struct cpuid_result result;
>> + asm volatile(
>> + "mov %%ebx, %%edi;"
>> + "cpuid;"
>> + "mov %%ebx, %%esi;"
>> + "mov %%edi, %%ebx;"
>> + : "=a" (result.eax),
>> + "=S" (result.ebx),
>> + "=c" (result.ecx),
>> + "=d" (result.edx)
>> + : "0" (op)
>> + : "edi");
>> + return result;
>> +}
>> +
>> +/*
>> + * Generic Extended CPUID function
>> + */
>> +static inline struct cpuid_result cpuid_ext(int op, unsigned ecx)
>> +{
>> + struct cpuid_result result;
>> + asm volatile(
>> + "mov %%ebx, %%edi;"
>> + "cpuid;"
>> + "mov %%ebx, %%esi;"
>> + "mov %%edi, %%ebx;"
>> + : "=a" (result.eax),
>> + "=S" (result.ebx),
>> + "=c" (result.ecx),
>> + "=d" (result.edx)
>> + : "0" (op), "2" (ecx)
>> + : "edi");
>> + return result;
>> +}
>> +
>> +/*
>> + * CPUID functions returning a single datum
>> + */
>> +static inline unsigned int cpuid_eax(unsigned int op)
>> +{
>> + unsigned int eax;
>> +
>> + __asm__("mov %%ebx, %%edi;"
>> + "cpuid;"
>> + "mov %%edi, %%ebx;"
>> + : "=a" (eax)
>> + : "0" (op)
>> + : "ecx", "edx", "edi");
>> + return eax;
>> +}
>> +
>> +static inline unsigned int cpuid_ebx(unsigned int op)
>> +{
>> + unsigned int eax, ebx;
>> +
>> + __asm__("mov %%ebx, %%edi;"
>> + "cpuid;"
>> + "mov %%ebx, %%esi;"
>> + "mov %%edi, %%ebx;"
>> + : "=a" (eax), "=S" (ebx)
>> + : "0" (op)
>> + : "ecx", "edx", "edi");
>> + return ebx;
>> +}
>> +
>> +static inline unsigned int cpuid_ecx(unsigned int op)
>> +{
>> + unsigned int eax, ecx;
>> +
>> + __asm__("mov %%ebx, %%edi;"
>> + "cpuid;"
>> + "mov %%edi, %%ebx;"
>> + : "=a" (eax), "=c" (ecx)
>> + : "0" (op)
>> + : "edx", "edi");
>> + return ecx;
>> +}
>> +
>> +static inline unsigned int cpuid_edx(unsigned int op)
>> +{
>> + unsigned int eax, edx;
>> +
>> + __asm__("mov %%ebx, %%edi;"
>> + "cpuid;"
>> + "mov %%edi, %%ebx;"
>> + : "=a" (eax), "=d" (edx)
>> + : "0" (op)
>> + : "ecx", "edi");
>> + return edx;
>> +}
>> +
>> +/* Standard macro to see if a specific flag is changeable */
>> +static inline int flag_is_changeable_p(uint32_t flag)
>> +{
>> + uint32_t f1, f2;
>> +
>> + asm(
>> + "pushfl\n\t"
>> + "pushfl\n\t"
>> + "popl %0\n\t"
>> + "movl %0,%1\n\t"
>> + "xorl %2,%0\n\t"
>> + "pushl %0\n\t"
>> + "popfl\n\t"
>> + "pushfl\n\t"
>> + "popl %0\n\t"
>> + "popfl\n\t"
>> + : "=&r" (f1), "=&r" (f2)
>> + : "ir" (flag));
>> + return ((f1^f2) & flag) != 0;
>> +}
>
> Does this have to be inline?
I don't think so, but both coreboot and linux are using inline, should
we keep the same?
[snip]
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
