Re: [PATCH] Add memory clobber to hypercalls (v2)

[Avi Kivity <avi@qumranet.com>]

Hollis Blanchard wrote:
> On Sat, 2008-06-28 at 06:43 +0300, Avi Kivity wrote:
>   
>> Anthony Liguori wrote:
>>     
>>> Hypercalls can modify arbitrary regions of memory.  Make sure to indicate this
>>> in the clobber list.  This fixes a hang when using KVM_GUEST kernel built with
>>> GCC 4.3.0.
>>>
>>> This was originally spotted and analyzed by Marcelo.
>>>
>>> Since v1, I've also added a "m" constraint for the inputs to the hypercall.
>>> This was suggested by Christian since it's not entirely clear whether a memory
>>> clobber will force the data to be in memory before the asm statement.  In the
>>> very least, it helps to be more conservative.
>>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>>
>>> @@ -80,7 +81,9 @@ static inline long kvm_hypercall1(unsigned int nr, unsigned long p1)
>>>  	long ret;
>>>  	asm volatile(KVM_HYPERCALL
>>>  		     : "=a"(ret)
>>> -		     : "a"(nr), "b"(p1));
>>> +		     : "a"(nr), "b"(p1),
>>> +		       "m"(*(char *)p1)
>>> +		     : "memory");
>>>  	return ret;
>>>  }
>>>  
>>>   
>>>       
>> Those are physical addresses, not virtual, and on i386 the addresses are 
>> split across multiple registers.
>>
>> However a small test program shows that the memory clobber does work 
>> with gcc 4.3, so I'll pick the earlier patch.
>>     
>
> What about gcc 4.4? 4.5? 5.0?
>
>   

Alexandre Oliva's idea of adding a constraint in __pa() to tell gcc that 
the memory there must be written seems to be the best option here.  
Though perhaps gcc should consider all memory pointed to by a pointer 
that is cast to an integer as escaped.


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