[RFC] address space annotation in the server

Jesse Barnes jbarnes at virtuousgeek.org
Wed Jan 3 21:26:41 PST 2007


Ok, my eyes are still bleeding from looking at hw/xfree86/common/compiler.h,
so the patch is really incomplete, but it should give you the general idea.

It adds a new __iomem type specifier for pointers, which is used by sparse
to do extra type checking on dereferences and parameter passing.  Having
something like this for MMIO regions should help avoid annoying bugs that
manifest themselves on platforms where memory mapping is a little funky
since sparse will complain when you dereference an __iomem pointer without
going through an appropriate MMIO wrapper.

However, as I mentioned the patch is incomplete.  Routines that map MMIO
space and return pointers should have the new __iomem specifier added, and
any functions expecting MMIO pointers need to be updated as well.  Given
that much of this code is changing when we switch over to the new PCI
layer (right Ian?), that might be a good time to start annotating things
and fixing up the bugs it uncovers.  Thoughts?

We might also consider adding annotations for things like fbmem, agp
space or port space, but I'm not sure how helpful they'll be in
terms of finding bugs (they may make the code clearer though).

There are a few other sparse features we could play with too, like
context annotations (could be useful for lock/unlock or alloc/free
stuff), but I haven't played with them myself yet...

Thanks,
Jesse

diff --git a/hw/xfree86/common/compiler.h b/hw/xfree86/common/compiler.h
index ea995ed..e1c1a4d 100644
--- a/hw/xfree86/common/compiler.h
+++ b/hw/xfree86/common/compiler.h
@@ -53,6 +53,10 @@
 
 # define _COMPILER_H
 
+#ifdef __CHECKER__
+#define __iomem __attribute__((noderef, address_space(1)))
+#endif
+
 #if defined(__SUNPRO_C)
 # define DO_PROTOTYPES
 #endif
@@ -633,7 +637,7 @@ inl(unsigned long port)
 }
 
 static __inline__ unsigned char
-xf86ReadMmio8(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio8(__volatile__ void __iomem *base, const unsigned long offset)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
 	unsigned char ret;
@@ -645,7 +649,7 @@ xf86ReadMmio8(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned short
-xf86ReadMmio16Be(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio16Be(__volatile__ void __iomem *base, const unsigned long offset)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
 	unsigned short ret;
@@ -657,7 +661,7 @@ xf86ReadMmio16Be(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned short
-xf86ReadMmio16Le(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio16Le(__volatile__ void __iomem *base, const unsigned long offset)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
 	unsigned short ret;
@@ -669,7 +673,7 @@ xf86ReadMmio16Le(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned int
-xf86ReadMmio32Be(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio32Be(__volatile__ void __iomem *base, const unsigned long offset)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
 	unsigned int ret;
@@ -681,7 +685,7 @@ xf86ReadMmio32Be(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned int
-xf86ReadMmio32Le(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio32Le(__volatile__ void __iomem *base, const unsigned long offset)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
 	unsigned int ret;
@@ -693,7 +697,7 @@ xf86ReadMmio32Le(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ void
-xf86WriteMmio8(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio8(__volatile__ void __iomem *base, const unsigned long offset,
 	       const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -705,7 +709,7 @@ xf86WriteMmio8(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio16Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio16Be(__volatile__ void __iomem *base, const unsigned long offset,
 		  const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -717,7 +721,7 @@ xf86WriteMmio16Be(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio16Le(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio16Le(__volatile__ void __iomem *base, const unsigned long offset,
 		  const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -729,7 +733,7 @@ xf86WriteMmio16Le(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio32Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32Be(__volatile__ void __iomem *base, const unsigned long offset,
 		  const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -741,7 +745,7 @@ xf86WriteMmio32Be(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio32Le(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32Le(__volatile__ void __iomem *base, const unsigned long offset,
 		  const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -753,7 +757,7 @@ xf86WriteMmio32Le(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio8NB(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio8NB(__volatile__ void __iomem *base, const unsigned long offset,
 		 const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -764,7 +768,7 @@ xf86WriteMmio8NB(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio16BeNB(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio16BeNB(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -775,7 +779,7 @@ xf86WriteMmio16BeNB(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio16LeNB(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio16LeNB(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -786,7 +790,7 @@ xf86WriteMmio16LeNB(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio32BeNB(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32BeNB(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -797,7 +801,7 @@ xf86WriteMmio32BeNB(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio32LeNB(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32LeNB(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -1008,7 +1012,7 @@ static __inline__ void stl_u(unsigned long val, unsigned int *p)
 
 #       if X_BYTE_ORDER == X_BIG_ENDIAN
 static __inline__ unsigned int
-xf86ReadMmio32Be(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio32Be(__volatile__ void __iomem *base, const unsigned long offset)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
 	unsigned int ret;
@@ -1020,7 +1024,7 @@ xf86ReadMmio32Be(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ void
-xf86WriteMmio32Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32Be(__volatile__ void __iomem *base, const unsigned long offset,
 		  const unsigned int val)
 {
 	unsigned long addr = ((unsigned long)base) + offset;
@@ -1088,7 +1092,7 @@ extern volatile unsigned char *ioBase;
 #endif /* eieio */
 
 static __inline__ unsigned char
-xf86ReadMmio8(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio8(__volatile__ void __iomem *base, const unsigned long offset)
 {
         register unsigned char val;
         __asm__ __volatile__(
@@ -1101,7 +1105,7 @@ xf86ReadMmio8(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned short
-xf86ReadMmio16Be(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio16Be(__volatile__ void __iomem *base, const unsigned long offset)
 {
         register unsigned short val;
         __asm__ __volatile__(
@@ -1114,7 +1118,7 @@ xf86ReadMmio16Be(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned short
-xf86ReadMmio16Le(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio16Le(__volatile__ void __iomem *base, const unsigned long offset)
 {
         register unsigned short val;
         __asm__ __volatile__(
@@ -1127,7 +1131,7 @@ xf86ReadMmio16Le(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned int
-xf86ReadMmio32Be(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio32Be(__volatile__ void __iomem *base, const unsigned long offset)
 {
         register unsigned int val;
         __asm__ __volatile__(
@@ -1140,7 +1144,7 @@ xf86ReadMmio32Be(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ unsigned int
-xf86ReadMmio32Le(__volatile__ void *base, const unsigned long offset)
+xf86ReadMmio32Le(__volatile__ void __iomem *base, const unsigned long offset)
 {
         register unsigned int val;
         __asm__ __volatile__(
@@ -1153,7 +1157,7 @@ xf86ReadMmio32Le(__volatile__ void *base, const unsigned long offset)
 }
 
 static __inline__ void
-xf86WriteMmioNB8(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmioNB8(__volatile__ void __iomem *base, const unsigned long offset,
 		 const unsigned char val)
 {
         __asm__ __volatile__(
@@ -1163,7 +1167,7 @@ xf86WriteMmioNB8(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmioNB16Le(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmioNB16Le(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned short val)
 {
         __asm__ __volatile__(
@@ -1173,7 +1177,7 @@ xf86WriteMmioNB16Le(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmioNB16Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmioNB16Be(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned short val)
 {
         __asm__ __volatile__(
@@ -1183,7 +1187,7 @@ xf86WriteMmioNB16Be(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmioNB32Le(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmioNB32Le(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned int val)
 {
         __asm__ __volatile__(
@@ -1193,7 +1197,7 @@ xf86WriteMmioNB32Le(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmioNB32Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmioNB32Be(__volatile__ void __iomem *base, const unsigned long offset,
 		    const unsigned int val)
 {
         __asm__ __volatile__(
@@ -1203,7 +1207,7 @@ xf86WriteMmioNB32Be(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio8(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio8(__volatile__ void __iomem *base, const unsigned long offset,
                const unsigned char val)
 {
         xf86WriteMmioNB8(base, offset, val);
@@ -1211,7 +1215,7 @@ xf86WriteMmio8(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio16Le(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio16Le(__volatile__ void __iomem *base, const unsigned long offset,
                   const unsigned short val)
 {
         xf86WriteMmioNB16Le(base, offset, val);
@@ -1219,7 +1223,7 @@ xf86WriteMmio16Le(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio16Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio16Be(__volatile__ void __iomem *base, const unsigned long offset,
                   const unsigned short val)
 {
         xf86WriteMmioNB16Be(base, offset, val);
@@ -1227,7 +1231,7 @@ xf86WriteMmio16Be(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio32Le(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32Le(__volatile__ void __iomem *base, const unsigned long offset,
                   const unsigned int val)
 {
         xf86WriteMmioNB32Le(base, offset, val);
@@ -1235,7 +1239,7 @@ xf86WriteMmio32Le(__volatile__ void *base, const unsigned long offset,
 }
 
 static __inline__ void
-xf86WriteMmio32Be(__volatile__ void *base, const unsigned long offset,
+xf86WriteMmio32Be(__volatile__ void __iomem *base, const unsigned long offset,
                   const unsigned int val)
 {
         xf86WriteMmioNB32Be(base, offset, val);
@@ -1594,7 +1598,7 @@ extern int (*xf86ReadMmio32)(void *, unsigned long);
 #  else
 /* Some DRI 3D drivers need MMIO_IN32. */
 static __inline__ int
-xf86ReadMmio32(void *Base, unsigned long Offset)
+xf86ReadMmio32(void __iomem *Base, unsigned long Offset)
 {
 	__asm__ __volatile__("mb"  : : : "memory");
 	return *(volatile unsigned int*)((unsigned long)Base+(Offset));



More information about the xorg mailing list