Re: [XenPPC] [RFC] fix stupid grant table flags

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

On Jun 22, 2006, at 5:27 PM, Hollis Blanchard wrote:

> On Wed, 2006-06-21 at 13:49 -0500, Hollis Blanchard wrote:
> > diff -r 6f3d44537b76 include/asm-powerpc/xen/asm/synch_bitops.h
> > --- a/include/asm-powerpc/xen/asm/synch_bitops.h        Fri Jun 16  
> > 16:07:38 2006 -0500
> > +++ b/include/asm-powerpc/xen/asm/synch_bitops.h        Wed Jun 21  
> > 13:42:56 2006 -0500
> > @@ -6,6 +6,8 @@
> >
> >  #ifdef CONFIG_SMP
> >  #include <asm/bitops.h>
> > +
> > +#define gnttab_cmpxchg_flags(ptr,o,n) synch_cmpxchg((int *) 
> > (ptr),o,n)
> >  #define synch_change_bit(a,b) change_bit(a,b)
> >  #define synch_clear_bit(a,b) clear_bit(a,b)
> Of course, this won't work. This should be better:
>
> static inline __u16 gnttab_cmpxchg_flags(__u16 *ptr, __u16 o, __u16 n)
> {
>         unsigned long *laddr;
cmpxchg will actually allow a u32 so there is no need to take it to a  
ulong, less bits to mess with and we expect the other short (the  
domid) to be relatively static, if fact xen uses cmpxch on both  
fields simultaneously.
>         old = ((unsigned long)o << shift) | (*laddr & ((1UL<<shift)-1));
>         new = ((unsigned long)n << shift) | (*laddr & ((1UL<<shift)-1));
Good Catch!
It might be prudent to deref laddr only once and make it volatile.

BTW: I'm not sure if you did, but just in case, this endian issue  
will need addressing (cscope):
  common/grant_table.c gnttab_prepare_for_transfer 567 scombo =  
((u32)sdom << 16) | (u32)sflags;
I plan on getting it eventually, I mention it so it does not get  
dropped.
-JX

_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel