Re: KVM: pvmmu breakage with gcc 4.3.0

[Anthony Liguori <anthony@codemonkey.ws>]

Christian Borntraeger wrote:
> Am Donnerstag, 26. Juni 2008 schrieb Avi Kivity:
>   
>> 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.  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.
>>     
>
> I I think about that again, the correct solution should be to use 2 input 
> constraints for parameters together with the memory clobber on hypercall.
>
> I think something like the following covers all cases:
>
> static inline long kvm_hypercall1(unsigned int nr, unsigned long p1)
> {
> 	long ret;
> 	asm volatile(KVM_HYPERCALL
> 		     : "=a"(ret)
> 		     : "a"(nr), "b"(p1), "m"(*(char *) p1)
> 		     : "memory" );
> 	return ret;
> }
>   

I don't know exactly what the rules are when you cast from pointer to 
unsigned long.  What I'm concerned about is that if there are a few 
layers of indirection, GCC won't be able to propagate the liveness of 
the pointer p1.  It has to be able to figure out that p1 here was really 
that point on the stack.  From the perspective of being conservative, it 
certainly can't hurt, but I'm not sure it solves the problem by itself.

This is why I think the "memory" constraint is really the right 
solution.  That should force GCC to be very conservative about syncing 
everything to memory.

Regards,

Anthony Liguori

> The address and the memory content of p1 (if it is a pointer) is not ommitted 
> by gcc. Furthermore, the memory clobbering nature of a hypercall is specified 
> as well.
>
> Christian
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>