>Number:         8405
>Category:       port-powerpc
>Synopsis:       pmap.new clean-up incomplete for powerpc
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    port-powerpc-maintainer (NetBSD/powerpc Portmasters)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Sep 13 23:05:01 1999
>Last-Modified:
>Originator:     Erik Bertelsen
>Organization:
	
>Release:        NetBSD-current 13 Sep 1999
>Environment:
	
System: NetBSD rlanarh108.uni-c.dk 1.4K NetBSD 1.4K (HJEMME) #185: Mon Sep 13 18:10:11 CEST 1999 erik@rlanarh108.uni-c.dk:/home/src/sys/arch/macppc/compile/HJEMME macppc


>Description:
	Configuring a kernel with sources of 13 Sep 1999, i.e. after the pmap.new
	clean-up is incomplete with respect to arch/powerpc and leaves the kernel
	uncompilable:

cc  -O2 -pipe -Werror -Wreturn-type -Wpointer-arith -Wno-main -msoft-float -I. -I../../../../arch -I../../../.. -nostdinc -DDIAGNOSTIC -DUSB_DEBUG -DMSGBUFSIZE="0x10000" -DMAXUSERS=32 -D_KERNEL -Dmacppc  -c ../../../../arch/powerpc/powerpc/pmap.c
../../../../arch/powerpc/powerpc/pmap.c: In function `ptebits':
../../../../arch/powerpc/powerpc/pmap.c:1270: argument `pa' doesn't match prototype
powerpc/pmap.h:79: prototype declaration
cc1: warnings being treated as errors
../../../../arch/powerpc/powerpc/pmap.c: In function `pmap_page_protect':
../../../../arch/powerpc/powerpc/pmap.c:1344: warning: passing arg 1 of `ptemodify' makes pointer from integer without a cast
*** Error code 1


	The main point seem to be related to the signature of ptebits, and the patches
	below are not meant to be a suggested fix, but they do point to the areas
	with problems.

	While looking at powerpc/include/pte.h, I also wondered whether pte_hi
	and pte_lo in struct pte should be u_int or u_int32 for explicitness' sake.

	I also noted that just changing ptebits to take a paddr_t as in the patch
	below causes problems with pmap_is_modified in powerpc/include/pmap.h and its
	uses in several uvm files.

	Anyway, I hope that I've provided enough information to make it easier for
	a person with in-depth knowledge about the pmap stuff to fix it in a matter
	of half-minutes :-)

	ps: the patch below is certainly wrong, but it actually produces a running
	kernel...
>How-To-Repeat:
	
>Fix:
Index: arch/powerpc/include/pmap.h
===================================================================
RCS file: /home/cvs-base/src/sys/arch/powerpc/include/pmap.h,v
retrieving revision 1.1.1.11
diff -c -r1.1.1.11 pmap.h
*** arch/powerpc/include/pmap.h	1999/09/12 14:50:27	1.1.1.11
--- arch/powerpc/include/pmap.h	1999/09/13 15:54:37
***************
*** 76,82 ****
  void pmap_bootstrap __P((u_int kernelstart, u_int kernelend));
  boolean_t pmap_extract __P((struct pmap *, vaddr_t, paddr_t *));
  boolean_t ptemodify __P((struct vm_page *, u_int, u_int));
! int ptebits __P((struct vm_page *, int));
  
  #define PMAP_NEED_PROCWR
  void pmap_procwr __P((struct proc *, vaddr_t, size_t));
--- 76,82 ----
  void pmap_bootstrap __P((u_int kernelstart, u_int kernelend));
  boolean_t pmap_extract __P((struct pmap *, vaddr_t, paddr_t *));
  boolean_t ptemodify __P((struct vm_page *, u_int, u_int));
! int ptebits __P((paddr_t, int));
  
  #define PMAP_NEED_PROCWR
  void pmap_procwr __P((struct proc *, vaddr_t, size_t));
Index: arch/powerpc/powerpc/pmap.c
===================================================================
RCS file: /home/cvs-base/src/sys/arch/powerpc/powerpc/pmap.c,v
retrieving revision 1.1.1.22
diff -c -r1.1.1.22 pmap.c
*** arch/powerpc/powerpc/pmap.c	1999/09/12 14:50:28	1.1.1.22
--- arch/powerpc/powerpc/pmap.c	1999/09/14 05:30:47
***************
*** 1341,1347 ****
  
  	pa &= ~ADDR_POFF;
  	if (prot & VM_PROT_READ) {
! 		ptemodify(pa, PTE_PP, PTE_RO);
  		return;
  	}
  
--- 1341,1347 ----
  
  	pa &= ~ADDR_POFF;
  	if (prot & VM_PROT_READ) {
! 		ptemodify(pg, PTE_PP, PTE_RO);
  		return;
  	}
  
Index: uvm/uvm_page_i.h
===================================================================
RCS file: /home/cvs-base/src/sys/uvm/uvm_page_i.h,v
retrieving revision 1.1.1.10
diff -c -r1.1.1.10 uvm_page_i.h
*** uvm/uvm_page_i.h	1999/09/12 14:50:03	1.1.1.10
--- uvm/uvm_page_i.h	1999/09/13 16:02:11
***************
*** 226,232 ****
  		pg->pqflags |= PQ_INACTIVE;
  		uvmexp.inactive++;
  		pmap_clear_reference(pg);
! 		if (pmap_is_modified(pg))
  			pg->flags &= ~PG_CLEAN;
  	}
  }
--- 226,232 ----
  		pg->pqflags |= PQ_INACTIVE;
  		uvmexp.inactive++;
  		pmap_clear_reference(pg);
! 		if (pmap_is_modified((paddr_t)pg))
  			pg->flags &= ~PG_CLEAN;
  	}
  }
Index: uvm/uvm_pager.c
===================================================================
RCS file: /home/cvs-base/src/sys/uvm/uvm_pager.c,v
retrieving revision 1.1.1.21
diff -c -r1.1.1.21 uvm_pager.c
*** uvm/uvm_pager.c	1999/09/12 14:50:04	1.1.1.21
--- uvm/uvm_pager.c	1999/09/13 16:03:29
***************
*** 328,334 ****
  				if ((pclust->flags & PG_CLEANCHK) == 0) {
  					if ((pclust->flags & (PG_CLEAN|PG_BUSY))
  					   == PG_CLEAN &&
! 					   pmap_is_modified(pclust))
  						pclust->flags &= ~PG_CLEAN;
  
  					/* now checked */
--- 328,334 ----
  				if ((pclust->flags & PG_CLEANCHK) == 0) {
  					if ((pclust->flags & (PG_CLEAN|PG_BUSY))
  					   == PG_CLEAN &&
! 					   pmap_is_modified((paddr_t)pclust))
  						pclust->flags &= ~PG_CLEAN;
  
  					/* now checked */
Index: uvm/uvm_pdaemon.c
===================================================================
RCS file: /home/cvs-base/src/sys/uvm/uvm_pdaemon.c,v
retrieving revision 1.1.1.15
diff -c -r1.1.1.15 uvm_pdaemon.c
*** uvm/uvm_pdaemon.c	1999/09/12 14:50:04	1.1.1.15
--- uvm/uvm_pdaemon.c	1999/09/13 16:04:26
***************
*** 402,408 ****
  			 * inactive pages shouldn't have any valid mappings
  			 * and we cleared reference before deactivating).
  			 */
! 			if (pmap_is_referenced(p)) {
  				uvm_pageactivate(p);
  				uvmexp.pdreact++;
  				continue;
--- 402,408 ----
  			 * inactive pages shouldn't have any valid mappings
  			 * and we cleared reference before deactivating).
  			 */
! 			if (pmap_is_referenced((paddr_t)p)) {
  				uvm_pageactivate(p);
  				uvmexp.pdreact++;
  				continue;
Index: uvm/uvm_vnode.c
===================================================================
RCS file: /home/cvs-base/src/sys/uvm/uvm_vnode.c,v
retrieving revision 1.1.1.24
diff -c -r1.1.1.24 uvm_vnode.c
*** uvm/uvm_vnode.c	1999/09/12 14:50:05	1.1.1.24
--- uvm/uvm_vnode.c	1999/09/13 16:05:08
***************
*** 977,983 ****
  			    (pp->pqflags & PQ_ACTIVE) != 0)
  				pmap_page_protect(pp, VM_PROT_NONE);
  			if ((pp->flags & PG_CLEAN) != 0 &&
! 			    pmap_is_modified(pp))
  				pp->flags &= ~(PG_CLEAN);
  			pp->flags |= PG_CLEANCHK;	/* update "hint" */
  
--- 977,983 ----
  			    (pp->pqflags & PQ_ACTIVE) != 0)
  				pmap_page_protect(pp, VM_PROT_NONE);
  			if ((pp->flags & PG_CLEAN) != 0 &&
! 			    pmap_is_modified((paddr_t)pp))
  				pp->flags &= ~(PG_CLEAN);
  			pp->flags |= PG_CLEANCHK;	/* update "hint" */
  


regards
Erik Bertelsen
>Audit-Trail:
>Unformatted: