Am Donnerstag, 26. Juni 2008 schrieb Anthony Liguori:
> 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.
The thing is, that your patch has "memory" in the clobber list. I dont know
if clobber constraints give any guarantees about input constraints. Thats why
I think that "b" plus "m" as input constraints are better. Does gcc actually
accept "memory" as input contraint?
I think I have to ask our compiler developers, but in 5 minutes I will be
offline until monday...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
