Re: KVM: pvmmu breakage with gcc 4.3.0

[Bill Davidsen <davidsen@tmr.com>]

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Marcelo Tosatti wrote:
>>> Some pvmmu functions store their commands on stack, and newer GCC
>>> versions conclude that these commands are unused.
>>>
>>> So stick an inline asm statement to convince the compiler otherwise.
>>>
>>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>>
>>>
>>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>>> index 8b7a3cf..c892752 100644
>>> --- a/arch/x86/kernel/kvm.c
>>> +++ b/arch/x86/kernel/kvm.c
>>> @@ -55,6 +55,12 @@ static void kvm_mmu_op(void *buffer, unsigned len)
>>>      int r;
>>>      unsigned long a1, a2;
>>>  
>>> +    /*
>>> +        * GCC 4.3.0 concludes that on-stack kvm_mmu_op* is unused and
>>> +        * optimizes its initialization away.
>>> +      */
>>> +        asm ("" : : "p" (buffer));
>>> +
>>>   
>>
>> I don't think "p" should force the contents into memory?  Perhaps 
>> "m"(*(char *)buffer)?
>>
>> Anthony, I don't see why a memory clobber would tell gcc that the 
>> variables is actually used.
> 
> It doesn't, but it seems to me that it should force GCC to assume 
> everything is live.  It's a big stick to hit the problem with but it 
> seems like the right thing semantically.
> 
In general the right stick to hit a compiler with is 'volatile,' which 
should tell the compiler that outside forces are in play. Without really 
digging into the code I don't want to suggest just where this should be 
used, but clearly Christian had the same thought.

>>   The problem is with the void * -> unsigned long cast (__pa), once 
>> that happens gcc loses track.  It's probably needed anyway since 
>> hypercalls _do_ clobber memory.
> 
> Right, it's not telling GCC that we touch a particular variable, but 
> rather that we may have touched any variable.
> 

-- 
Bill Davidsen <davidsen@tmr.com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot