WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] x86: fix variable_test_bit() asm constraints

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] x86: fix variable_test_bit() asm constraints
From: "Jan Beulich" <jbeulich@xxxxxxxxxx>
Date: Fri, 14 Mar 2008 11:23:47 +0000
Delivery-date: Fri, 14 Mar 2008 04:23:26 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
I just sent a (much bigger, see below) patch to the same effect to the
x86 Linux maintainers - in Xen, all the operations modifying bits do
have "memory" clobbers, so it's just the test_bit() constraint that's
wrong.

However, I wonder whether the non-atomic ops aren't limiting things too
much by having "memory" clobbers, they would much better be restricted
to indicate just the changing memory location. This, however, would
probably require some additional consideration given that Xen (other
than Linux) isn't using -fno-strict-aliasing. Furthermore, these
non-atomic operations, according to their comments, can be re-ordered,
which contradicts the use of __asm__ __volatile__ (but note that
removing this would probably require extra precautions to meet strict
aliasing rules).

Further, using 'void *' for the 'addr' parameter appears dangerous,
since bt{,c,r,s} access the full 32 bits (if 'unsigned long' was used
properly here, 64 bits for x86-64) pointed at, so invalid uses like
referencing a 'char' array cannot currently be caught.

Finally I find the leading 'd' constraints in the 'nr' assembly
operands quite odd - what is the purpose of that? Linux is using just
"Ir" here...

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

Index: 2008-03-05/xen/include/asm-x86/bitops.h
===================================================================
--- 2008-03-05.orig/xen/include/asm-x86/bitops.h        2007-09-14 
11:03:32.000000000 +0200
+++ 2008-03-05/xen/include/asm-x86/bitops.h     2008-03-13 10:20:34.000000000 
+0100
@@ -254,7 +254,8 @@ static __inline__ int variable_test_bit(
        __asm__ __volatile__(
                "btl %2,%1\n\tsbbl %0,%0"
                :"=r" (oldbit)
-               :"m" (CONST_ADDR),"dIr" (nr));
+               :"m" (CONST_ADDR), "dIr" (nr),
+                "m" (((const volatile int *)addr)[nr >> 5]));
        return oldbit;
 }
 




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