Subject: Re: 1.6.1 lack of stability on cats
To: Chris Gilbert <chris@dokein.co.uk>
From: Richard Earnshaw <rearnsha@arm.com>
List: port-arm
Date: 02/12/2003 11:10:14
This is a multipart MIME message.

--==_Exmh_-3770734110
Content-Type: text/plain; charset=us-ascii

> 
> rearnsha@buzzard.freeserve.co.uk said:
> >  It's early days yet, and I've cried wolf on this issue once already,
> > but  this does seem like it might be it this time.
> 
> > So could folks who are in a possition to test 1.6.1 on any ARM
> > platform  please try pulling up arch/arm/arm32/vm_machdep.c 1.22->1.23
> > and give it a  try. 
> 
> Forget that.  No sooner do I press send on the email that the *)(^^%*^%&^% 
> thing dumps core again.
> 
> Back to the drawing board :-(

And the drawing board sayeth:

Thou shalt revert ticket 712, namely

  Module Name:    syssrc
  Committed By:   thorpej
  Date:           Wed Aug 21 18:34:34 UTC 2002

  Modified Files:
        syssrc/sys/arch/acorn32/acorn32: rpc_machdep.c
        syssrc/sys/arch/arm/arm32: pmap.c
        syssrc/sys/arch/cats/cats: cats_machdep.c
        syssrc/sys/arch/evbarm/integrator: integrator_machdep.c
        syssrc/sys/arch/evbarm/iq80310: iq80310_machdep.c
        syssrc/sys/arch/evbarm/iq80321: iq80321_machdep.c
        syssrc/sys/arch/evbarm/ixm1200: ixm1200_machdep.c
        syssrc/sys/arch/hpcarm/hpcarm: hpc_machdep.c
        syssrc/sys/arch/netwinder/netwinder: netwinder_machdep.c
        syssrc/sys/arch/shark/ofw: ofw.c

  Log Message:
  Do cached memory access to L1 tables, making sure to write-back the
  cache after any L1 table modifications.

With this patch removed I was able to build the X distribution without any 
further problems (well, strictly speaking, it was still building when I 
left for work this morning, but it had been building for 9 hours without 
any problems and was in the process of creating the tgz files when I left 
for work this morning.

Jason has told me that this change was a performance improvement rather 
than a bug fix.  So I think we should revert it.

Here are links to the relevant commits

http://mail-index.netbsd.org/source-changes/2002/11/21/0035.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0036.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0037.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0038.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0039.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0040.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0041.html
http://mail-index.netbsd.org/source-changes/2002/11/21/0042.html
http://mail-index.netbsd.org/source-changes/2002/11/30/0044.html

Note that reverting the patch to pmap.c may cause conflicts.  The attached 
patch can be used instead.

I haven't submitted a request to releng yet.  Ignatios, is there any 
chance that you could try this on your shark and see if it solves the 
problems you reported?

It would also be helpful if folks could test this on other systems as well 
and report any problems observed.

R.



--==_Exmh_-3770734110
Content-Type: text/x-patch ; name="pmap.patch"; charset=us-ascii
Content-Description: pmap.patch
Content-Disposition: attachment; filename="pmap.patch"

Index: arm/arm32/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/arm/arm32/pmap.c,v
retrieving revision 1.97.4.5
diff -p -r1.97.4.5 pmap.c
*** arm/arm32/pmap.c	2002/12/07 20:44:23	1.97.4.5
--- arm/arm32/pmap.c	2003/02/11 23:57:04
*************** pmap_map_in_l1(struct pmap *pmap, vaddr_
*** 973,979 ****
  	pmap->pm_pdir[ptva + 1] = L1_C_PROTO | (l2pa + 0x400);
  	pmap->pm_pdir[ptva + 2] = L1_C_PROTO | (l2pa + 0x800);
  	pmap->pm_pdir[ptva + 3] = L1_C_PROTO | (l2pa + 0xc00);
- 	cpu_dcache_wb_range((vaddr_t) &pmap->pm_pdir[ptva + 0], 16);
  
  	/* Map the page table into the page table area. */
  	if (selfref)
--- 973,978 ----
*************** pmap_unmap_in_l1(struct pmap *pmap, vadd
*** 995,1001 ****
  	pmap->pm_pdir[ptva + 1] = 0;
  	pmap->pm_pdir[ptva + 2] = 0;
  	pmap->pm_pdir[ptva + 3] = 0;
- 	cpu_dcache_wb_range((vaddr_t) &pmap->pm_pdir[ptva + 0], 16);
  
  	/* Unmap the page table from the page table area. */
  	*((pt_entry_t *)(pmap->pm_vptpt + ptva)) = 0;
--- 994,999 ----
*************** pmap_alloc_l1pt(void)
*** 1336,1341 ****
--- 1334,1340 ----
  	struct l1pt *pt;
  	int error;
  	struct vm_page *m;
+ 	pt_entry_t *pte;
  
  	/* Allocate virtual address space for the L1 page table */
  	va = uvm_km_valloc(kernel_map, L1_TABLE_SIZE);
*************** pmap_alloc_l1pt(void)
*** 1373,1380 ****
  	m = TAILQ_FIRST(&pt->pt_plist);
  	while (m && va < (pt->pt_va + L1_TABLE_SIZE)) {
  		pa = VM_PAGE_TO_PHYS(m);
  
! 		pmap_kenter_pa(va, pa, VM_PROT_READ|VM_PROT_WRITE);
  
  		va += NBPG;
  		m = m->pageq.tqe_next;
--- 1372,1389 ----
  	m = TAILQ_FIRST(&pt->pt_plist);
  	while (m && va < (pt->pt_va + L1_TABLE_SIZE)) {
  		pa = VM_PAGE_TO_PHYS(m);
+ 
+ 		pte = vtopte(va);
+ 
+ 		/*
+ 		 * Assert that the PTE is invalid.  If it's invalid,
+ 		 * then we are guaranteed that there won't be an entry
+ 		 * for this VA in the TLB.
+ 		 */
+ 		KDASSERT(pmap_pte_v(pte) == 0);
  
! 		*pte = L2_S_PROTO | VM_PAGE_TO_PHYS(m) |
! 		    L2_S_PROT(PTE_KERNEL, VM_PROT_READ|VM_PROT_WRITE);
  
  		va += NBPG;
  		m = m->pageq.tqe_next;
*************** pmap_allocpagedir(struct pmap *pmap)
*** 1543,1553 ****
  	pmap->pm_pdir = (pd_entry_t *)pt->pt_va;
  
  	/* Clean the L1 if it is dirty */
! 	if (!(pt->pt_flags & PTFLAG_CLEAN)) {
  		bzero((void *)pmap->pm_pdir, (L1_TABLE_SIZE - KERNEL_PD_SIZE));
- 		cpu_dcache_wb_range((vaddr_t) pmap->pm_pdir,
- 		    (L1_TABLE_SIZE - KERNEL_PD_SIZE));
- 	}
  
  	/* Allocate a page table to map all the page tables for this pmap */
  	KASSERT(pmap->pm_vptpt == 0);
--- 1552,1559 ----
  	pmap->pm_pdir = (pd_entry_t *)pt->pt_va;
  
  	/* Clean the L1 if it is dirty */
! 	if (!(pt->pt_flags & PTFLAG_CLEAN))
  		bzero((void *)pmap->pm_pdir, (L1_TABLE_SIZE - KERNEL_PD_SIZE));
  
  	/* Allocate a page table to map all the page tables for this pmap */
  	KASSERT(pmap->pm_vptpt == 0);
*************** pmap_allocpagedir(struct pmap *pmap)
*** 1577,1590 ****
  	bcopy((char *)pmap_kernel()->pm_pdir + (L1_TABLE_SIZE - KERNEL_PD_SIZE),
  		(char *)pmap->pm_pdir + (L1_TABLE_SIZE - KERNEL_PD_SIZE),
  		KERNEL_PD_SIZE);
- 	cpu_dcache_wb_range((vaddr_t)pmap->pm_pdir +
- 	    (L1_TABLE_SIZE - KERNEL_PD_SIZE), KERNEL_PD_SIZE);
  
  	/* Wire in this page table */
  	pmap_map_in_l1(pmap, PTE_BASE, pmap->pm_pptpt, TRUE);
  
  	pt->pt_flags &= ~PTFLAG_CLEAN;	/* L1 is dirty now */
! 
  	LIST_INSERT_HEAD(&pmaps, pmap, pm_list);
  	simple_unlock(&pmaps_lock);
  	
--- 1583,1594 ----
  	bcopy((char *)pmap_kernel()->pm_pdir + (L1_TABLE_SIZE - KERNEL_PD_SIZE),
  		(char *)pmap->pm_pdir + (L1_TABLE_SIZE - KERNEL_PD_SIZE),
  		KERNEL_PD_SIZE);
  
  	/* Wire in this page table */
  	pmap_map_in_l1(pmap, PTE_BASE, pmap->pm_pptpt, TRUE);
  
  	pt->pt_flags &= ~PTFLAG_CLEAN;	/* L1 is dirty now */
! 	
  	LIST_INSERT_HEAD(&pmaps, pmap, pm_list);
  	simple_unlock(&pmaps_lock);
  	

--==_Exmh_-3770734110--