Subject: Re: Dom0 build broken
To: Andrew Doran <bouyer@netbsd.org>
From: Christoph Egger <Christoph_Egger@gmx.de>
List: port-xen
Date: 09/03/2007 12:15:20
--Boundary-00=_4692G3skSPkivFJ
Content-Type: text/plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Monday 03 September 2007 11:45:34 Christoph Egger wrote:
> On Friday 31 August 2007 19:15:26 Christoph Egger wrote:
> > On Friday 31 August 2007 17:34:08 Andrew Doran wrote:
> > > Hi Christoph,
> > >
> > > On Fri, Aug 31, 2007 at 11:19:26AM +0200, Christoph Egger wrote:
> > > > Your last vmlocking merge in -current broke Xen Dom0 build:
> > > > Xen DomU still builds.
> > > >
> > > >
> > > > cc1: warnings being treated as errors
> > > > ../../../../arch/x86/x86/bus_dma.c: In function '_bus_dmamem_map':
> > > > ../../../../arch/x86/x86/bus_dma.c:1053: warning: implicit
> > > > declaration of function 'x86_atomic_setbits_l'
> > > > ../../../../arch/x86/x86/bus_dma.c: In function '_bus_dmamem_unmap':
> > > > ../../../../arch/x86/x86/bus_dma.c:1098: warning: implicit
> > > > declaration of function 'x86_atomic_clearbits_l'
> > > > *** [bus_dma.o] Error code 1
> > > > 1 error
> > >
> > > Odd. Do you have local modifications, and is your tree up to date? It
> > > builds for me and my source tree has no diffs.
>
> My XEN3_DOM0 has a local modification:
>
>
> Index: ../i386/conf/XEN3_DOM0
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/i386/conf/XEN3_DOM0,v
> retrieving revision 1.6
> diff -u -p -r1.6 XEN3_DOM0
> --- ../i386/conf/XEN3_DOM0      17 Oct 2006 19:57:24 -0000      1.6
> +++ ../i386/conf/XEN3_DOM0      3 Sep 2007 09:42:23 -0000
> @@ -23,6 +23,10 @@ options      ACPIVERBOSE
>  #options        PCI_BUS_FIXUP          # fixup PCI bus numbering
>  #options        PCI_INTR_FIXUP         # fixup PCI interrupt routing
>
> +options                LOCKDEBUG
> +options                VNODE_LOCKDEBUG
> +options                MALLOC_DEBUG
> +options                UVMHIST
>  ioapic*                at mainbus? apid ?
>
>  # ACPI devices
>
> > > Andrew
> >
> > The attached patch makes this one file build again. But there is more to
> > do to make the dom0 kernel build again.
>
> The arch/xen/i386/pmap.c does not build, because pvh_lock is now a kmutex_t
> but simple_lock() expects a volatile struct simplelock.
>
> This leads to warnings like these:
>
> ../../../../arch/xen/i386/pmap.c: In function 'pmap_lock_pvhs':
> ../../../../arch/xen/i386/pmap.c:1599: warning: passing argument 1
> of '_simple_lock' from incompatible pointer type
> ../../../../arch/xen/i386/pmap.c:1604: warning: passing argument 1
> of '_simple_lock' from incompatible pointer type

Attached is a patch which fixes these build errors
(and is much less intrusive than the big patch I sent you last friday in 
private).

Christoph


--Boundary-00=_4692G3skSPkivFJ
Content-Type: text/x-diff;
  charset="iso-8859-15";
  name="xen_pmap.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="xen_pmap.diff"

Index: i386/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/i386/pmap.c,v
retrieving revision 1.29
diff -u -p -r1.29 pmap.c
--- i386/pmap.c	17 May 2007 14:51:36 -0000	1.29
+++ i386/pmap.c	3 Sep 2007 10:11:19 -0000
@@ -1596,16 +1596,16 @@ pmap_lock_pvhs(struct pv_head *pvh1, str
 {
 
 	if (pvh2 == NULL) {
-		simple_lock(&pvh1->pvh_lock);
+		mutex_enter(&pvh1->pvh_lock);
 		return;
 	}
 
 	if (pvh1 < pvh2) {
-		simple_lock(&pvh1->pvh_lock);
-		simple_lock(&pvh2->pvh_lock);
+		mutex_enter(&pvh1->pvh_lock);
+		mutex_enter(&pvh2->pvh_lock);
 	} else {
-		simple_lock(&pvh2->pvh_lock);
-		simple_lock(&pvh1->pvh_lock);
+		mutex_enter(&pvh2->pvh_lock);
+		mutex_enter(&pvh1->pvh_lock);
 	}
 }
 
@@ -2003,14 +2003,14 @@ pmap_fork(pmap1, pmap2)
 		sel = -1;
 	}
 
-	simple_lock(&pmap1->pm_obj.vmobjlock);
-	simple_lock(&pmap2->pm_obj.vmobjlock);
+	mutex_enter(&pmap1->pm_obj.vmobjlock);
+	mutex_enter(&pmap2->pm_obj.vmobjlock);
 
 	/* Copy the LDT, if necessary. */
 	if (pmap1->pm_flags & PMF_USER_LDT) {
 		if (len != pmap1->pm_ldt_len * sizeof(union descriptor)) {
-			simple_unlock(&pmap2->pm_obj.vmobjlock);
-			simple_unlock(&pmap1->pm_obj.vmobjlock);
+			mutex_exit(&pmap2->pm_obj.vmobjlock);
+			mutex_exit(&pmap1->pm_obj.vmobjlock);
 			if (len != -1) {
 				ldt_free(sel);
 				uvm_km_free(kernel_map, (vaddr_t)new_ldt,
@@ -2027,8 +2027,8 @@ pmap_fork(pmap1, pmap2)
 		len = -1;
 	}
 
-	simple_unlock(&pmap2->pm_obj.vmobjlock);
-	simple_unlock(&pmap1->pm_obj.vmobjlock);
+	mutex_exit(&pmap2->pm_obj.vmobjlock);
+	mutex_exit(&pmap1->pm_obj.vmobjlock);
 
 	if (len != -1) {
 		ldt_free(sel);
@@ -2055,7 +2055,7 @@ pmap_ldt_cleanup(l)
 	size_t len = 0;
 	int sel = -1;
 
-	simple_lock(&pmap->pm_obj.vmobjlock);
+	mutex_enter(&pmap->pm_obj.vmobjlock);
 
 	if (pmap->pm_flags & PMF_USER_LDT) {
 		sel = pmap->pm_ldt_sel;
@@ -2070,7 +2070,7 @@ pmap_ldt_cleanup(l)
 		pmap->pm_flags &= ~PMF_USER_LDT;
 	}
 
-	simple_unlock(&pmap->pm_obj.vmobjlock);
+	mutex_exit(&pmap->pm_obj.vmobjlock);
 
 	if (old_ldt != NULL)
 		uvm_km_free(kernel_map, (vaddr_t)old_ldt, len, UVM_KMF_WIRED);
@@ -2655,10 +2655,10 @@ pmap_remove_ptes(pmap, ptp, ptpva, start
 		mdpg = &pg->mdpage;
 
 		/* sync R/M bits */
-		simple_lock(&mdpg->mp_pvhead.pvh_lock);
+		mutex_enter(&mdpg->mp_pvhead.pvh_lock);
 		mdpg->mp_attrs |= (opte & (PG_U|PG_M));
 		pve = pmap_remove_pv(&mdpg->mp_pvhead, pmap, startva);
-		simple_unlock(&mdpg->mp_pvhead.pvh_lock);
+		mutex_exit(&mdpg->mp_pvhead.pvh_lock);
 
 		if (pve) {
 			SPLAY_RIGHT(pve, pv_node) = pv_tofree;
@@ -2747,10 +2747,10 @@ pmap_remove_pte(pmap, ptp, pte, va, cpum
 	mdpg = &pg->mdpage;
 
 	/* sync R/M bits */
-	simple_lock(&mdpg->mp_pvhead.pvh_lock);
+	mutex_enter(&mdpg->mp_pvhead.pvh_lock);
 	mdpg->mp_attrs |= (opte & (PG_U|PG_M));
 	pve = pmap_remove_pv(&mdpg->mp_pvhead, pmap, va);
-	simple_unlock(&mdpg->mp_pvhead.pvh_lock);
+	mutex_exit(&mdpg->mp_pvhead.pvh_lock);
 
 	if (pve)
 		pmap_free_pv(pmap, pve);
@@ -3031,7 +3031,7 @@ pmap_page_remove(pg)
 	curpmap = ci->ci_pmap;
 
 	/* XXX: needed if we hold head->map lock? */
-	simple_lock(&pvh->pvh_lock);
+	mutex_enter(&pvh->pvh_lock);
 
 	for (pve = SPLAY_MIN(pvtree, &pvh->pvh_root); pve != NULL; pve = npve) {
 		npve = SPLAY_NEXT(pvtree, &pvh->pvh_root, pve);
@@ -3118,7 +3118,7 @@ pmap_page_remove(pg)
 		killlist = pve;
 	}
 	pmap_free_pvs(NULL, killlist);
-	simple_unlock(&pvh->pvh_lock);
+	mutex_exit(&pvh->pvh_lock);
 	PMAP_HEAD_TO_MAP_UNLOCK();
 	pmap_tlb_shootnow(cpumask);
 
@@ -3179,7 +3179,7 @@ pmap_test_attrs(pg, testbits)
 	/* nope, gonna have to do it the hard way */
 	PMAP_HEAD_TO_MAP_LOCK();
 	/* XXX: needed if we hold head->map lock? */
-	simple_lock(&pvh->pvh_lock);
+	mutex_enter(&pvh->pvh_lock);
 
 	for (pve = SPLAY_MIN(pvtree, &pvh->pvh_root);
 	     pve != NULL && (*myattrs & testbits) == 0;
@@ -3195,7 +3195,7 @@ pmap_test_attrs(pg, testbits)
 	 * we have found the bits we are testing for.
 	 */
 
-	simple_unlock(&pvh->pvh_lock);
+	mutex_exit(&pvh->pvh_lock);
 	PMAP_HEAD_TO_MAP_UNLOCK();
 	return((*myattrs & testbits) != 0);
 }
@@ -3233,7 +3233,7 @@ pmap_clear_attrs(pg, clearbits)
 	PMAP_HEAD_TO_MAP_LOCK();
 	pvh = &mdpg->mp_pvhead;
 	/* XXX: needed if we hold head->map lock? */
-	simple_lock(&pvh->pvh_lock);
+	mutex_enter(&pvh->pvh_lock);
 
 	myattrs = &mdpg->mp_attrs;
 	result = *myattrs & clearbits;
@@ -3294,7 +3294,7 @@ no_tlb_shootdown:
 		pmap_unmap_ptes(pve->pv_pmap);		/* unlocks pmap */
 	}
 
-	simple_unlock(&pvh->pvh_lock);
+	mutex_exit(&pvh->pvh_lock);
 	PMAP_HEAD_TO_MAP_UNLOCK();
 
 	pmap_tlb_shootnow(cpumask);
@@ -3592,9 +3592,9 @@ pmap_enter_ma(struct pmap *pmap, vaddr_t
 #endif
 			mdpg = &pg->mdpage;
 			old_pvh = &mdpg->mp_pvhead;
-			simple_lock(&old_pvh->pvh_lock);
+			mutex_enter(&old_pvh->pvh_lock);
 			mdpg->mp_attrs |= opte;
-			simple_unlock(&old_pvh->pvh_lock);
+			mutex_exit(&old_pvh->pvh_lock);
 		}
 		goto shootdown_now;
 	}
@@ -3672,10 +3672,10 @@ pmap_enter_ma(struct pmap *pmap, vaddr_t
 
 			if (new_pvh) {
 				pmap_enter_pv(new_pvh, pve, pmap, va, ptp);
-				simple_unlock(&new_pvh->pvh_lock);
+				mutex_exit(&new_pvh->pvh_lock);
 			} else
 				pmap_free_pv(pmap, pve);
-			simple_unlock(&old_pvh->pvh_lock);
+			mutex_exit(&old_pvh->pvh_lock);
 
 			goto shootdown_test;
 		}
@@ -3686,9 +3686,9 @@ pmap_enter_ma(struct pmap *pmap, vaddr_t
 	}
 
 	if (new_pvh) {
-		simple_lock(&new_pvh->pvh_lock);
+		mutex_enter(&new_pvh->pvh_lock);
 		pmap_enter_pv(new_pvh, pve, pmap, va, ptp);
-		simple_unlock(&new_pvh->pvh_lock);
+		mutex_exit(&new_pvh->pvh_lock);
 	}
 
 	XENPRINTK(("pmap initial setup\n"));
Index: include/pmap.h
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/include/pmap.h,v
retrieving revision 1.11
diff -u -p -r1.11 pmap.h
--- include/pmap.h	29 Aug 2007 23:38:06 -0000	1.11
+++ include/pmap.h	3 Sep 2007 10:11:19 -0000
@@ -52,6 +52,8 @@
 #include <machine/xenfunc.h>
 #include <machine/xenpmap.h>
 #include <machine/segments.h>
+#include <machine/atomic.h>
+
 #include <uvm/uvm_object.h>
 
 /*

--Boundary-00=_4692G3skSPkivFJ--