Re: [PATCH]: Fix MSR_K7_EVNTSEL{0,3} for SVM

Avi Kivity
2008-04-24T15:20:24Z

Chris Lalancette wrote:
> Avi Kivity wrote:
>
>> You mean the gcc generates wrong code? It seems fine here (though
>> wonderfully obfuscated).
>>
>> Can you attach an objdump -Sr svm.o? Also, what gcc version are you using?
>>
>>
>
> (sending attachment in private mail, so I don't spam the whole list with 189K of
> objdump). Note that this is an objdump -Sr of the current code, with my patch
> *not* applied.
>
> gcc is gcc-4.3.0-7 in Fedora 9.
>
>

It's a gcc bug. svm_set_msr() places ecx in %rsi, and consistently uses
%esi to refer to the first 32 bits. But when it compiles this bit:

> case MSR_K7_EVNTSEL0:
> case MSR_K7_EVNTSEL1:
> case MSR_K7_EVNTSEL2:
> case MSR_K7_EVNTSEL3:
> /*
> * only support writing 0 to the performance counters for now
> * to make Windows happy. Should be replaced by a real
> * performance counter emulation later.
> */
> if (data != 0)
> goto unhandled;
> break;

(where MSR_K7_EVENTSEL[0123] == 0xc001000[0123])

it compiles it into

> 1811: 8d 86 00 00 ff 3f lea 0x3fff0000(%rsi),%eax
> 1817: 83 f8 03 cmp $0x3,%eax
> 181a: 0f 87 e2 01 00 00 ja 1a02 <svm_set_msr+0x27f>
Now it uses %rsi instead of %esi, and any junk in the upper bits will
cause the ja to be taken.

We need to get a reduced testcase to the gcc folks, this is a serious
bug. Any changes in the code to work around this would be fragile.

--
error compiling committee.c: too many arguments to function