Broken CPUID brand string?

3

I am printing some information about CPU in my OS using CPUID instruction.

Reading and printing vendor string(GenuineIntel) works well, but reading brand string gives me little strange string.

ok cpu-info <= Run command
CPU Vendor name: GenuineIntel <= Vendor string is good
CPU Brand: D:  l(R)  Core(TMD:   CPU       MD:  <= What..?
ok

Vendor string supposed to be:

Intel(R) Core(TM) i5 CPU       M 540

But what I got is:

 D:  l(R)  Core(TMD:   CPU       MD:

C++ code:

            char vendorString[13] = { 0, };
            Dword eax, ebx, ecx, edx;
            ACpuid(0, &eax, &ebx, &ecx, &edx);
            *((Dword*)vendorString) = ebx;
            *((Dword*)vendorString + 1) = edx;
            *((Dword*)vendorString + 2) = ecx;
            Console::Output.Write(L"CPU vendor name: ");
            for (int i = 0; i < 13; i++) {
                Console::Output.Write((wchar_t)(vendorString[i]));
            }
            Console::Output.WriteLine();

            char brandString[48] = { 0, };
            ACpuid(0x80000002, &eax, &ebx, &ecx, &edx);
            *((Dword*)brandString) = eax;
            *((Dword*)brandString + 1) = ebx;
            *((Dword*)brandString + 2) = ecx;
            *((Dword*)brandString + 3) = edx;
            ACpuid(0x80000003, &eax, &ebx, &ecx, &edx);
            *((Dword*)brandString + 4) = eax;
            *((Dword*)brandString + 5) = ebx;
            *((Dword*)brandString + 6) = ecx;
            *((Dword*)brandString + 7) = edx;
            ACpuid(0x80000004, &eax, &ebx, &ecx, &edx);
            *((Dword*)brandString + 8) = eax;
            *((Dword*)brandString + 9) = ebx;
            *((Dword*)brandString + 10) = ecx;
            *((Dword*)brandString + 11) = edx;
            Console::Output.Write(L"CPU brand: ");
            for (int i = 0; i < 48; i++) {
                Console::Output.Write((wchar_t) brandString[i]);
            }
            Console::Output.WriteLine();

NOTE:

  1. This program is UEFI application. No problem with permissions.

  2. Console is an wrapper class for EFI console. Not C# stuff.

  3. Dword = unsigned 32bit integer

Assembly code(MASM):

;Cpuid command
;ACpuid(Type, pEax, pEbx, pEcx, pEdx)
ACpuid Proc
    ;Type => Rcx
    ;pEax => Rdx
    ;pEbx => R8
    ;pEcx => R9
    ;pEdx => [ rbp + 48 ] ?
    push rbp
    mov rbp, rsp
    push rax
    push rsi

    mov rax, rcx
    cpuid
    mov [ rdx ], eax
    mov [ r8 ], ebx
    mov [ r9 ], ecx
    mov rsi, [ rbp + 48 ]
    mov [ rsi ], rdx

    pop rsi
    pop rax
    pop rbp
    ret
ACpuid Endp
c++
assembly
x86-64
masm
uefi
asked on Stack Overflow Apr 10, 2016 by Gippeumi • edited Apr 10, 2016 by Michael Petch

1 Answer

5

I agree with Ross Ridge that you should use the compiler intrinsic __cpuid. As for why your code likely doesn't work as is - there are some bugs that will cause problems.


CPUID destroys the contents of RAX, RBX, RCX, and RDX and yet you do this in your code:

cpuid
mov [ rdx ], eax

RDX has been destroyed by the time mov [ rdx ], eax is executed, rendering the pointer in RDX invalid. You'll need to move RDX to another register before using the CPUID instruction.

Per the Windows 64-bit Calling Convention these are the volatile registers that need to be preserved by the caller:

The registers RAX, RCX, RDX, R8, R9, R10, R11 are considered volatile and must be considered destroyed on function calls (unless otherwise safety-provable by analysis such as whole program optimization).

These are the non-volatile ones that need to be preserved by the callee:

The registers RBX, RBP, RDI, RSI, RSP, R12, R13, R14, and R15 are considered nonvolatile and must be saved and restored by a function that uses them.

We can use R10 (a volatile register) to store RDX temporarily. Rather than use RSI in the code we can reuse R10 for updating the value at pEdx. We won't need to preserve RSI if we don't use it. CPUID does destroy RBX, and RBX is non-volatile, so we need to preserve it. RAX is volatile so we don't need to preserve it.


In your code you have this line:

mov [ rsi ], rdx

RSI is a memory address (pEdx) provided by the caller to store the value in EDX. The code you have would move the contents of the 8-byte register RDX to a memory location that was expecting a 4-byte DWORD. This could potentially trash data in the caller. This really should have been:

mov [ rsi ], edx

With all of the above in mind we could code the ACpuid routine this way:

option casemap:none
.code

;Cpuid command
;ACpuid(Type, pEax, pEbx, pEcx, pEdx)
ACpuid Proc
    ;Type => Rcx
    ;pEax => Rdx
    ;pEbx => R8
    ;pEcx => R9
    ;pEdx => [ rbp + 48 ] ?
    push rbp
    mov rbp, rsp
    push rbx         ; Preserve RBX (destroyed by CPUID)

    mov r10, rdx     ; Save RDX before CPUID
    mov rax, rcx
    cpuid
    mov [ r10 ], eax
    mov [ r8 ], ebx
    mov [ r9 ], ecx
    mov r10, [ rbp + 48 ]
    mov [ r10 ], edx ; Last parameter is pointer to 32-bit DWORD, 
                     ;  Move EDX to the memory location, not RDX

    pop rbx
    pop rbp
    ret
ACpuid Endp

end
answered on Stack Overflow Apr 10, 2016 by Michael Petch • edited Apr 11, 2016 by Michael Petch

User contributions licensed under CC BY-SA 3.0