On Wed, Dec 10, 2003 at 07:39:43PM -0500, David Dawes wrote:
> On Thu, Dec 11, 2003 at 12:25:25AM +0000, Alan Hourihane wrote:
> >On Wed, Dec 10, 2003 at 07:13:39PM -0500, David Dawes wrote:
> >> On Wed, Dec 10, 2003 at 11:06:01PM +0100, Linus Gasser wrote:
> >> >On Wednesday 10 December 2003 19:52, David Dawes wrote:
> >> >> On Wed, Dec 10, 2003 at 09:30:12AM +0100, Linus Gasser wrote:
> >> >> >As I wrote, I installed a fresh, new XFree 4.3.99.901, cvs from 6th
> >> >> > December, and I also checked that the file that has been compiled in the
> >> >> > XFree-tree is the same as in
> >> >> >/usr/X11R6/lib/modules/dri/r200_dri.so
> >> >> >but I'll check again this evening, just to be sure.
> >> >>
> >> >> What platform are you building on? It looks like the INREG() code in
> >> >> r200_screen.c could expand to this on some platforms. Some code in the
> >> >> radeon_dri.so module that uses INREG is #ifdef'd out for Alpha platforms.
> >> >> Not all of it though, and not the instances of it in the r200_dri.so
> >> >> module.
> >> >>
> >> >> David
> >> >
> >> >It's an alpha pc164 (as stated in my first mail ;-), and I stumbled already
> >> >over a first error that got fixed quite fast (#938, thanks to Alan
> >> >Hourihane), obviously I should've filed a second bug. I've done so now, and
> >> >also added the diff with the INREGs #ifdef'd out for Alpha (there was still
> >> >some in radeon_screen.c, r128_ioctl.c). The bug-# is 967, the attachement is
> >> >there. Now it works for me!
> >> >
> >> >thanks for your fast reply and for the right hint!
> >>
> >> I don't know that disabling it is a real solution though. It is
> >> clearly better than it was, but some apps are likely to fail.
> >>
> >> Maybe the DRM driver should be providing this information? I'm
> >> not even sure what the security implications are of allowing the
> >> MMIO area to be mapped into an app. Doesn't that mean that a rogue
> >> DRI app could potentially reprogram the video hardware, unless it
> >> is mapped read-only?
> >
> >It is mapped read-only.
>
> Good :-)
>
> >I guess the only way to handle this is to export the memory read/write
> >functions in a library to allow DRI drivers to link against. So we'd
> >have a library that contains xf86ReadMmio32(), xf86ReadMmio16()...
> >
> >Does that sound plausible David ?
>
> It looks complicated on Alpha because of the sparse/dense mapping
> modes, so the XFree86 server has to determine at run-time how to
> do MMIO. However, if only 32-bit access is used (and you don't
> need Jensen support) just declaring a local xf86ReadMmio32 function
> pointer and initialising it to readDense32() should be good enough.
> I think readDense32() is provided in libc on Alpha.
>
> All other platforms should get handled via the inline's provided
> in compiler.h.
In fact, regardless of dense or sparse mapping the read is exactly the
same for 32bit access. It only differs for 8 or 16 bit access. That's
even true for JENSEN. And all readDense32() does is a mem_barrier then
a volatile read - just like normal.
I'm thinking of committing this patch...
Alan.
Index: compiler.h
===================================================================
RCS file: /X11R6/x-cvs/xc/programs/Xserver/hw/xfree86/common/compiler.h,v
retrieving revision 3.104
diff -u -r3.104 compiler.h
--- compiler.h 3 Nov 2003 05:11:01 -0000 3.104
+++ compiler.h 11 Dec 2003 01:05:08 -0000
@@ -1616,7 +1616,7 @@
extern int (*xf86ReadMmio32)(void *, unsigned long);
extern void (*xf86WriteMmio8)(int, void *, unsigned long);
extern void (*xf86WriteMmio16)(int, void *, unsigned long);
-extern void (*xf86WriteMmio32)(int, void *, unsigned long);
+/* extern void (*xf86WriteMmio32)(int, void *, unsigned long); */
extern void (*xf86WriteMmioNB8)(int, void *, unsigned long);
extern void (*xf86WriteMmioNB16)(int, void *, unsigned long);
extern void (*xf86WriteMmioNB32)(int, void *, unsigned long);
@@ -1625,11 +1625,19 @@
extern void xf86SlowBCopyFromBus(unsigned char *, unsigned char *, int);
extern void xf86SlowBCopyToBus(unsigned char *, unsigned char *, int);
+/* regardless of the type of alpha we have, a 32bit read is always the same */
+static __inline__ unsigned int
+xf86ReadMmio32(pointer Base, register unsigned long Offset)
+{
+ mem_barrier();
+ return *(volatile unsigned int *)((unsigned long)Base+(Offset));
+}
+
/* Some macros to hide the system dependencies for MMIO accesses */
/* Changed to kill noise generated by gcc's -Wcast-align */
# define MMIO_IN8(base, offset) (*xf86ReadMmio8)(base, offset)
# define MMIO_IN16(base, offset) (*xf86ReadMmio16)(base, offset)
-# define MMIO_IN32(base, offset) (*xf86ReadMmio32)(base, offset)
+# define MMIO_IN32(base, offset) xf86ReadMmio32(base, offset)
# if defined (JENSEN_SUPPORT)
# define MMIO_OUT32(base, offset, val) \
_______________________________________________
XFree86 mailing list
XFree86@xxxxxxxxxxx
http://XFree86.Org/mailman/listinfo/xfree86
