Re: KVM: pvmmu breakage with gcc 4.3.0

Bill Davidsen
2008-06-27T16:12:01Z

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