1

I am writing a kernel module and it is about reading and writing MSRs. I wrote a simple program for testing but it still fails. All it is doing is writing to MSR, then reading it back. Here is the code:

static int __init test3_init(void)
{
    uint32_t hi,lo;
    hi=0; lo=0xb;
    asm volatile("mov %0,%%eax"::"r"(lo));
    asm volatile("mov %0,%%edx"::"r"(hi));
    asm volatile("mov $0x38d,%ecx");
    asm volatile("wrmsr");
    printk("exit_write: hi=%08x lo=%08x\n",hi,lo);
    asm volatile("mov $0x38d,%ecx");
    asm volatile("rdmsr":"=a"(lo),"=d"(hi));
    printk("exit_write2: hi=%08x lo=%08x\n",hi,lo);
    return 0;
}

The output looks like:

exit_write: hi=00000000 lo=0000000b

exit_write2: hi=00000000 lo=00000000

Can someone tell me why the return value is 0 in the second output, instead of the original? Is there something wrong with my code? Thanks a lot.

Alexey Frunze
  • 61,140
  • 12
  • 83
  • 180
Sean
  • 2,649
  • 3
  • 21
  • 27
  • Pls share the kernel log u mentioned also where is it crashing according to you ? – Jay D Jun 25 '12 at 05:10
  • Linux also lets your read and write MSRs from user space, via /dev/cpu/CPUNUM/msr: http://www.kernel.org/doc/man-pages/online/pages/man4/msr.4.html. If nothing else, you can look at the kernel source for the msr module. Just a thought... – paulsm4 Jun 25 '12 at 05:44
  • @paulsm4 I read that code before, it uses kernel module. But I need to use RDMSR and WRMSR, assembly code. I need to read the kernel code, maybe. – Sean Jun 25 '12 at 05:55

1 Answers1

5

The problem has to do with the fact that you don't fully tell gcc which registers you're using in inline assembly and how and you also expect that gcc doesn't do anything funky to the registers between the fragments of your inline assembly code. Related mov and xxmsr instructions should be in the same asm block.

Look what gcc does with your code (I've altered it a tiny bit to make it compilable as a regular program)...

Source:

// file: msr.c
#include <stdio.h>

typedef unsigned uint32_t;
#define printk printf
#define __init

static int __init test3_init(void)
{
    uint32_t hi,lo;
    hi=0; lo=0xb;
    asm volatile("mov %0,%%eax"::"r"(lo));
    asm volatile("mov %0,%%edx"::"r"(hi));
    asm volatile("mov $0x38d,%ecx");
    asm volatile("wrmsr");
    printk("exit_write: hi=%08x lo=%08x\n",hi,lo);
    asm volatile("mov $0x38d,%ecx");
    asm volatile("rdmsr":"=a"(lo),"=d"(hi));
    printk("exit_write2: hi=%08x lo=%08x\n",hi,lo);
    return 0;
}

int main(void)
{
  return test3_init();
}

Compiling (with MinGW gcc 4.6.2):

gcc msr.c -c -S -o msr.s

Disassembly of test3_init() from msr.s:

_test3_init:
        pushl   %ebp
        movl    %esp, %ebp
        pushl   %esi
        pushl   %ebx
        subl    $32, %esp
        movl    $0, -12(%ebp)
        movl    $11, -16(%ebp)
        movl    -16(%ebp), %eax
        mov %eax,%eax
        movl    -12(%ebp), %eax
        mov %eax,%edx
        mov $0x38d,%ecx
        wrmsr
        movl    -16(%ebp), %eax
        movl    %eax, 8(%esp)
        movl    -12(%ebp), %eax
        movl    %eax, 4(%esp)
        movl    $LC0, (%esp)
        call    _printf
        mov $0x38d,%ecx
        rdmsr
        movl    %edx, %ebx
        movl    %eax, %esi
        movl    %esi, -16(%ebp)
        movl    %ebx, -12(%ebp)
        movl    -16(%ebp), %eax
        movl    %eax, 8(%esp)
        movl    -12(%ebp), %eax
        movl    %eax, 4(%esp)
        movl    $LC1, (%esp)
        call    _printf
        movl    $0, %eax
        addl    $32, %esp
        popl    %ebx
        popl    %esi
        popl    %ebp
        ret

Note that when the CPU starts executing wrmsr it has ecx=0x38d (OK), edx=0 (OK), eax=0 (not 0xb, oops!). Follow the instructions to see it.

What you can and should write instead is something like the following, even shorter than it was:

static int __init test3_init2(void)
{
    uint32_t hi,lo;
    hi=0; lo=0xb;
    asm volatile("wrmsr"::"c"(0x38d),"a"(lo),"d"(hi));
    printk("exit_write: hi=%08x lo=%08x\n",hi,lo);
    asm volatile("rdmsr":"=a"(lo),"=d"(hi):"c"(0x38d));
    printk("exit_write2: hi=%08x lo=%08x\n",hi,lo);
    return 0;
}

Now, disassembly of test3_init2():

_test3_init2:
        pushl   %ebp
        movl    %esp, %ebp
        pushl   %esi
        pushl   %ebx
        subl    $48, %esp
        movl    $0, -12(%ebp)
        movl    $11, -16(%ebp)
        movl    $909, %ecx
        movl    -16(%ebp), %eax
        movl    -12(%ebp), %edx
        wrmsr
        movl    -16(%ebp), %eax
        movl    %eax, 8(%esp)
        movl    -12(%ebp), %eax
        movl    %eax, 4(%esp)
        movl    $LC0, (%esp)
        call    _printf
        movl    $909, -28(%ebp)
        movl    -28(%ebp), %ecx
        rdmsr
        movl    %edx, %ebx
        movl    %eax, %esi
        movl    %esi, -16(%ebp)
        movl    %ebx, -12(%ebp)
        movl    -16(%ebp), %eax
        movl    %eax, 8(%esp)
        movl    -12(%ebp), %eax
        movl    %eax, 4(%esp)
        movl    $LC1, (%esp)
        call    _printf
        movl    $0, %eax
        addl    $48, %esp
        popl    %ebx
        popl    %esi
        popl    %ebp
        ret

Also, remember that every CPU has its own MSR and you may want to set this MSR on all of them. Another important consideration is that the thread in which you're manipulating an MSR should not be moved between different CPUs until you're done with the MSR.

Alexey Frunze
  • 61,140
  • 12
  • 83
  • 180
  • Thank you very much for your response. I learned a little more about MSR from you. Actually, I found a solution last night. I should assign ecx first, then edx, last eax. It works but the ordering is really confusing, cuz it is not written in the manual or any where else. Now I understand I should set them all at one time. I will use your advise to rewrite my code. – Sean Jun 25 '12 at 18:37
  • WRMSR itself does not depend on the order in which you load registers, and so there's nothing else to document about this instruction. The problem was/is in your inline assembly code, it was/is broken. I showed a proper fix. – Alexey Frunze Jun 25 '12 at 19:41
  • Don't forget to upvote and accept answers wherever applicable. – Alexey Frunze Jun 25 '12 at 19:42