Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 8 Apr 2002 20:11:13 +0100
From:      David Malone <dwmalone@maths.tcd.ie>
To:        Christopher Masto <chris@netmonger.net>
Cc:        Jordan Hubbard <jkh@winston.freebsd.org>, Murray Stokely <murray@FreeBSD.ORG>, Greg 'groggy' Lehey <grog@FreeBSD.ORG>, "David E. O'Brien" <obrien@FreeBSD.ORG>, cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG
Subject:   Re: Configuring XFree 4 (was: cvs commit: src/usr.sbin/sysinstall Makefile dispatch.c dist.c install.c menus.c sysinstall.8)
Message-ID:  <20020408191113.GA56982@walton.maths.tcd.ie>
In-Reply-To: <20020408182647.GA67395@netmonger.net>
References:  <20020404115539.GE2303@freebsdmall.com> <32305.1017961490@winston.freebsd.org> <20020408182647.GA67395@netmonger.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Apr 08, 2002 at 02:26:47PM -0400, Christopher Masto wrote:
> On Thu, Apr 04, 2002 at 03:04:50PM -0800, Jordan Hubbard wrote:
> > Oh yeah, I think I've mentioned this before but just to bring it back
> > into focus so we're not blind-sided by other reports, XFree86 4.x will
> > bring you to grief if you have one of those Athlon-based Compaq
> > machines
> 
> I wonder if it's the same issue as PRs 28418 and 25958.
> 
> If this is the same problem on a variety of hardware, I tend to think
> it's not the BIOS writers on crack, but rather a misunderstanding in
> FreeBSD's MTRR code.

*suddenly wakes up*

I've some patches which makes the MTRR code a bit more robust, but
my BIOS is still acting funny and unfortunately the AMD docs which
are likely to describe this stuff are under NDA.

If anyone would like to try these patches, please do and let me
know how you get on. On my machine it makes X work fine, but there
are problems then at shutdown (probably related to ACPI and the
BIOS getting cranky about the changes I've made). There are some
debugging printfs so you can see what MTRR values are being changed.

(If someone would like to review these patches so I can commit them
then please do. They certainly are an improvement over a panic when
you run X.)

	David.


Index: sys/i386/i386/i686_mem.c
===================================================================
RCS file: /cvs/FreeBSD-CVS/src/sys/i386/i386/i686_mem.c,v
retrieving revision 1.13
diff -u -r1.13 i686_mem.c
--- sys/i386/i386/i686_mem.c	27 Apr 2001 19:28:19 -0000	1.13
+++ sys/i386/i386/i686_mem.c	8 Apr 2002 07:35:56 -0000
@@ -79,6 +79,8 @@
 						 struct mem_range_desc *mrd);
 static void			i686_mrfetch(struct mem_range_softc *sc);
 static int			i686_mtrrtype(int flags);
+static int			i686_mrt2mtrr(int flags, int oldval);
+static int			i686_mtrrconflict(int flag1, int flag2);
 static void			i686_mrstore(struct mem_range_softc *sc);
 static void			i686_mrstoreone(void *arg);
 static struct mem_range_desc	*i686_mtrrfixsearch(struct mem_range_softc *sc,
@@ -94,29 +96,35 @@
 static int i686_mtrrtomrt[] = {
     MDF_UNCACHEABLE,
     MDF_WRITECOMBINE,
-    0,
-    0,
+    MDF_UNKNOWN,
+    MDF_UNKNOWN,
     MDF_WRITETHROUGH,
     MDF_WRITEPROTECT,
     MDF_WRITEBACK
 };
 
+#define MTRRTOMRTLEN (sizeof(i686_mtrrtomrt) / sizeof(i686_mtrrtomrt[0]))
+
+static int
+i686_mtrr2mrt(int val) {
+	if (val < 0 || val >= MTRRTOMRTLEN)
+		return MDF_UNKNOWN;
+	return i686_mtrrtomrt[val];
+}
+
 /* 
- * i686 MTRR conflict matrix for overlapping ranges 
- *
- * Specifically, this matrix allows writeback and uncached ranges
- * to overlap (the overlapped region is uncached).  The array index
- * is the translated i686 code for the flags (because they map well).
+ * i686 MTRR conflicts. Writeback and uncachable may overlap.
  */
-static int i686_mtrrconflict[] = {
-    MDF_WRITECOMBINE | MDF_WRITETHROUGH | MDF_WRITEPROTECT,
-    MDF_ATTRMASK,
-    0,
-    0,
-    MDF_ATTRMASK,
-    MDF_ATTRMASK,
-    MDF_WRITECOMBINE | MDF_WRITETHROUGH | MDF_WRITEPROTECT
-};
+static int
+i686_mtrrconflict(int flag1, int flag2) {
+	flag1 &= MDF_ATTRMASK;
+	flag2 &= MDF_ATTRMASK;
+	if (flag1 == flag2 ||
+	    (flag1 == MDF_WRITEBACK && flag2 == MDF_UNCACHEABLE) ||
+	    (flag2 == MDF_WRITEBACK && flag1 == MDF_UNCACHEABLE))
+		return 0;
+	return 1;
+}
 
 /*
  * Look for an exactly-matching range.
@@ -155,7 +163,7 @@
 	    msrv = rdmsr(msr);
 	    for (j = 0; j < 8; j++, mrd++) {
 		mrd->mr_flags = (mrd->mr_flags & ~MDF_ATTRMASK) |
-		    i686_mtrrtomrt[msrv & 0xff] |
+		    i686_mtrr2mrt(msrv & 0xff) |
 		    MDF_ACTIVE;
 		if (mrd->mr_owner[0] == 0)
 		    strcpy(mrd->mr_owner, mem_owner_bios);
@@ -167,7 +175,7 @@
 	    msrv = rdmsr(msr);
 	    for (j = 0; j < 8; j++, mrd++) {
 		mrd->mr_flags = (mrd->mr_flags & ~MDF_ATTRMASK) |
-		    i686_mtrrtomrt[msrv & 0xff] |
+		    i686_mtrr2mrt(msrv & 0xff) |
 		    MDF_ACTIVE;
 		if (mrd->mr_owner[0] == 0)
 		    strcpy(mrd->mr_owner, mem_owner_bios);
@@ -179,7 +187,7 @@
 	    msrv = rdmsr(msr);
 	    for (j = 0; j < 8; j++, mrd++) {
 		mrd->mr_flags = (mrd->mr_flags & ~MDF_ATTRMASK) |
-		    i686_mtrrtomrt[msrv & 0xff] |
+		    i686_mtrr2mrt(msrv & 0xff) |
 		    MDF_ACTIVE;
 		if (mrd->mr_owner[0] == 0)
 		    strcpy(mrd->mr_owner, mem_owner_bios);
@@ -193,7 +201,7 @@
     for (; (mrd - sc->mr_desc) < sc->mr_ndesc; msr += 2, mrd++) {
 	msrv = rdmsr(msr);
 	mrd->mr_flags = (mrd->mr_flags & ~MDF_ATTRMASK) |
-	    i686_mtrrtomrt[msrv & 0xff];
+	    i686_mtrr2mrt(msrv & 0xff);
 	mrd->mr_base = msrv & 0x0000000ffffff000LL;
 	msrv = rdmsr(msr + 1);
 	mrd->mr_flags = (msrv & 0x800) ? 
@@ -219,8 +227,8 @@
 
     flags &= MDF_ATTRMASK;
 
-    for (i = 0; i < (sizeof(i686_mtrrtomrt) / sizeof(i686_mtrrtomrt[0])); i++) {
-	if (i686_mtrrtomrt[i] == 0)
+    for (i = 0; i < MTRRTOMRTLEN; i++) {
+	if (i686_mtrrtomrt[i] == MDF_UNKNOWN)
 	    continue;
 	if (flags == i686_mtrrtomrt[i])
 	    return(i);
@@ -228,6 +236,16 @@
     return(-1);
 }
 
+static int
+i686_mrt2mtrr(int flags, int oldval)
+{
+	int val;
+
+	if ((val = i686_mtrrtype(flags)) == -1)
+		return oldval & 0xff;
+	return val & 0xff;
+}
+
 /*
  * Update running CPU(s) MTRRs to match the ranges in the descriptor
  * list.
@@ -262,7 +280,7 @@
 {
     struct mem_range_softc 	*sc = (struct mem_range_softc *)arg;
     struct mem_range_desc	*mrd;
-    u_int64_t			msrv;
+    u_int64_t			omsrv, msrv;
     int				i, j, msr;
     u_int			cr4save;
 
@@ -280,30 +298,39 @@
 	msr = MSR_MTRR64kBase;
 	for (i = 0; i < (MTRR_N64K / 8); i++, msr++) {
 	    msrv = 0;
+	    omsrv = rdmsr(msr);
 	    for (j = 7; j >= 0; j--) {
 		msrv = msrv << 8;
-		msrv |= (i686_mtrrtype((mrd + j)->mr_flags) & 0xff);
+		msrv |= i686_mrt2mtrr((mrd + j)->mr_flags, omsrv >> (j*8));
 	    }
+	    if (rdmsr(msr) != msrv)
+                printf("MSR %x, old=%llx new=%llx\n", msr, rdmsr(msr), msrv);
 	    wrmsr(msr, msrv);
 	    mrd += 8;
 	}
 	msr = MSR_MTRR16kBase;
 	for (i = 0; i < (MTRR_N16K / 8); i++, msr++) {
 	    msrv = 0;
+	    omsrv = rdmsr(msr);
 	    for (j = 7; j >= 0; j--) {
 		msrv = msrv << 8;
-		msrv |= (i686_mtrrtype((mrd + j)->mr_flags) & 0xff);
+		msrv |= i686_mrt2mtrr((mrd + j)->mr_flags, omsrv >> (j*8));
 	    }
+	    if (rdmsr(msr) != msrv)
+                printf("MSR %x, old=%llx new=%llx\n", msr, rdmsr(msr), msrv);
 	    wrmsr(msr, msrv);
 	    mrd += 8;
 	}
 	msr = MSR_MTRR4kBase;
 	for (i = 0; i < (MTRR_N4K / 8); i++, msr++) {
 	    msrv = 0;
+	    omsrv = rdmsr(msr);
 	    for (j = 7; j >= 0; j--) {
 		msrv = msrv << 8;
-		msrv |= (i686_mtrrtype((mrd + j)->mr_flags) & 0xff);
+		msrv |= i686_mrt2mtrr((mrd + j)->mr_flags, omsrv >> (j*8));
 	    }
+	    if (rdmsr(msr) != msrv)
+                printf("MSR %x, old=%llx new=%llx\n", msr, rdmsr(msr), msrv);
 	    wrmsr(msr, msrv);
 	    mrd += 8;
 	}
@@ -313,9 +340,10 @@
     msr = MSR_MTRRVarBase;
     for (; (mrd - sc->mr_desc) < sc->mr_ndesc; msr += 2, mrd++) {
 	/* base/type register */
+	omsrv = rdmsr(msr);
 	if (mrd->mr_flags & MDF_ACTIVE) {
 	    msrv = mrd->mr_base & 0x0000000ffffff000LL;
-	    msrv |= (i686_mtrrtype(mrd->mr_flags) & 0xff);
+	    msrv |= i686_mrt2mtrr(mrd->mr_flags, omsrv);
 	} else {
 	    msrv = 0;
 	}
@@ -416,8 +444,7 @@
 	    /* non-exact overlap ? */
 	    if (mroverlap(curr_md, mrd)) {
 		/* between conflicting region types? */
-		if ((i686_mtrrconflict[i686_mtrrtype(curr_md->mr_flags)] & mrd->mr_flags) ||
-		    (i686_mtrrconflict[i686_mtrrtype(mrd->mr_flags)] & curr_md->mr_flags))
+		if (i686_mtrrconflict(curr_md->mr_flags, mrd->mr_flags))
 		    return(EINVAL);
 	    }
 	} else if (free_md == NULL) {
@@ -450,7 +477,7 @@
     case MEMRANGE_SET_UPDATE:
 	/* make sure that what's being asked for is even possible at all */
 	if (!mrvalid(mrd->mr_base, mrd->mr_len) ||
-	    (i686_mtrrtype(mrd->mr_flags & MDF_ATTRMASK) == -1))
+	    i686_mtrrtype(mrd->mr_flags) == -1)
 	    return(EINVAL);
 
 #define FIXTOP	((MTRR_N64K * 0x10000) + (MTRR_N16K * 0x4000) + (MTRR_N4K * 0x1000))
Index: sys/sys/memrange.h
===================================================================
RCS file: /cvs/FreeBSD-CVS/src/sys/sys/memrange.h,v
retrieving revision 1.4
diff -u -r1.4 memrange.h
--- sys/sys/memrange.h	29 Dec 1999 04:24:44 -0000	1.4
+++ sys/sys/memrange.h	7 Mar 2002 22:49:40 -0000
@@ -10,6 +10,7 @@
 #define MDF_WRITETHROUGH	(1<<2)	/* write-through cached */
 #define MDF_WRITEBACK		(1<<3)	/* write-back cached */
 #define MDF_WRITEPROTECT	(1<<4)	/* read-only region */
+#define MDF_UNKNOWN		(1<<5)	/* any state we don't understand */
 #define MDF_ATTRMASK		(0x00ffffff)
 
 #define MDF_FIXBASE		(1<<24)	/* fixed base */
Index: usr.sbin/memcontrol/memcontrol.c
===================================================================
RCS file: /cvs/FreeBSD-CVS/src/usr.sbin/memcontrol/memcontrol.c,v
retrieving revision 1.6
diff -u -r1.6 memcontrol.c
--- usr.sbin/memcontrol/memcontrol.c	24 Jun 2001 23:41:44 -0000	1.6
+++ usr.sbin/memcontrol/memcontrol.c	8 Mar 2002 15:08:47 -0000
@@ -50,6 +50,7 @@
     {"write-through",	MDF_WRITETHROUGH,	MDF_SETTABLE},
     {"write-back",	MDF_WRITEBACK,		MDF_SETTABLE},
     {"write-protect",	MDF_WRITEPROTECT,	MDF_SETTABLE},
+    {"unknown",		MDF_UNKNOWN,		0},
     {"fixed-base",	MDF_FIXBASE,		0},
     {"fixed-length",	MDF_FIXLEN,		0},
     {"set-by-firmware",	MDF_FIRMWARE,		0},

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020408191113.GA56982>