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

[Avi Kivity <avi@qumranet.com>]

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone