>Number:         22487
>Category:       port-powerpc
>Synopsis:       powerpc mmmmap() in mem.c needs to return atop(off)???
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    port-powerpc-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 14 22:31:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     M L Riechers
>Release:        NetBSD 1.6P
>Organization:
	
>Environment:
	
	
System: NetBSD t982.rse.com 1.6P NetBSD 1.6P (EASTERN-1.6P.backside) #0: Fri Mar 21 18:07:11 EST 2003 mlr@t982.rse.com:/mnt2/usr/local/src/usr/src/sys/arch/macppc/compile/EASTERN-1.6P.backside macppc
Architecture: powerpc
Machine: macppc
>Description:
	

While hacking X for the mbx port, I noticed an inconsistancy between
/dev/mem and the mmap produced by e.g.:

         base = mmap(0, Size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, Base);

where fd is an open fd on /dev/mem.

This produced the result that the mmap'd area (which happend to be the
frame buffer) which was supposed to reflect Base didn't match 

	lseek(fd, Base + 0x00000, SEEK_SET);
	read(fd, &buckett, whatever_framebuf_size_is);

and the mmap didn't reflect the frame buffer, while the lseek/read
did.

So, I nosed around and noticed that i386, amongst others, was returning

	return (atop(off));

instead of 

	return (trunc_page((paddr_t)off));

in their respective mmmmap() in mem.c's, which means that
powerpc/powerpc/mem.c mmmmap() is off by a factor of page size.

I applied the patch under Fix; the /dev/mem read and the mmap then
matched, and X (ahem, with other hacks, eventually) worked.

>How-To-Repeat:
	
Search me.  Perhaps this is the source of some powerpc problems:

> From: Andreas Drewke <andreas_dr@gmx.de>
> To: <port-macppc@NetBSD.org>
> Subject: XFree86 4.3.0
> Message-ID: <29698.1060762518@www56.gmx.net>
> 
> ....
> 
> So now I spend some hours to get XFree86 4.3.0 built, but at the end GUESS
> what the problem is
> 
> coulnd not mmap screen [a=xxxxx, s=xxxxx] (Illegal argument)
> - the same I had with 4.2.0 -
>
> ....
>
> -A

>Fix:
	
I find it hard to believe that powerpc needs this patch; if it does,
it seems to me that lots of stuff, particularly X windows, ought to be
non-functional.  So I hesitate, but here's the patch: (please forgive
the non-functional junk at the end, but it's a hack, and I didn't want
to mess up the diff).


--- mem.c.orig  2002-10-23 06:04:09.000000000 -0400
+++ mem.c       2003-04-03 14:46:59.000000000 -0500
@@ -133,11 +133,38 @@
        int prot;
 {
        struct proc *p = curproc;
+       int ppn;
+
+       /*
+        * /dev/mem is the only one that makes sense through this
+        * interface.  For /dev/kmem any physaddr we return here
+        * could be transient and hence incorrect or invalid at
+        * a later time.  /dev/null just doesn't make any sense
+        * and /dev/zero is a hack that is handled via the default
+        * pager in mmap().
+        */

        if (minor(dev) != DEV_MEM)
                return (-1);
 
+       /* XXX This may botch our cacheing assumptions.  Do we care? */
+       ppn = atop(off);
+       if (ppn < physmem || suser(p->p_ucred, &p->p_acflag) == 0)
+               return ppn;
+       return -1;
+
        if (atop(off) >= physmem && suser(p->p_ucred, &p->p_acflag) != 0)
                return (-1);
        return (trunc_page((paddr_t)off));
+       /* XXX This may botch our cacheing assumptions.  Do we care? */
+       ppn = atop(off);
+       if (ppn >= 0 && ppn < physmem)
+               return ppn;
+       return -1;
+
+       /* XXX This may botch our cacheing assumptions.  Do we care? */
+       ppn = atop(off);
+       if (ppn < physmem || suser(p->p_ucred, &p->p_acflag) == 0)
+               return ppn;
+       return -1;
 }
>Release-Note:
>Audit-Trail:
>Unformatted: