Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Apr 2002 21:02:30 +0100
From:      David Malone <dwmalone@maths.tcd.ie>
To:        Jordan Hubbard <jkh@winston.freebsd.org>
Cc:        Christopher Masto <chris@netmonger.net>, Murray Stokely <murray@FreeBSD.ORG>, Greg 'groggy' Lehey <grog@FreeBSD.ORG>, msmith@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:   <200204102102.aa16821@salmon.maths.tcd.ie>
In-Reply-To: Your message of "Wed, 10 Apr 2002 11:49:38 PDT." <51324.1018464578@winston.freebsd.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
> > Excilent. Any complications when shutting down? I've tracked the
> > complications I'm seeing, and it is something to do with ACPI calls
> > causing the machine to hang up - I think my BIOS is just nuts actually...

> No, no complications - it shuts down as always.

-current or -stable?

> Well, he's pretty busy too so you might end up just having to go ahead
> with them too and let the general population test it. :)

OK. I talked the problem through with Ian before writing the patches,
so I'll see if he has time to review them. Failing that, I'll commit
them and wait for the screams ;-)

> > Should I aim to get these into 4.6?

> That's up to Murray and co, but it certainly would make *my* machine
> happy. :-)

If it means it's easier to ship XFree 4.2 then it would
probably be a good thing.

I've included the patches below, with explaination. If anyone
has time to comment on them please do.

	David.




The problem with the old MTRR code is that it only expects to find
documented values in the bytes of MTRR registers. To convert the
MTRR byte into a FreeBSD "Memory Range Type" (mrt) it uses the
byte value and looks it up in an array. If the value is not in range
then the mrt value ends up containing random junk.

This isn't an immediate problem. The mrt value is only used later
when rewriting the MTRR registers. When we finally go to write a
value back again, the function i686_mtrrtype() searches for the
junk value and returns -1 when it fails to find it. This is converted
to a byte (0xff) and written back to the register, causing a GPF
as 0xff is an illegal value for a MTRR byte.

To work around this problem I've added a new mrt flag MDF_UNKNOWN.
We set this when we read a MTRR byte which we do not understand.
If we try to convert a MDF_UNKNOWN back into a MTRR value, then the
new function, i686_mrt2mtrr, just returns the old value of the MTRR
byte. This leaves the memory range type unchanged.

I've checked k6_mem.c and it doesn't seem to need any corrisponding
changes as we understand all possible values of its equivelent of
MTRRs. Alpha, ia64 and powerpc don't seem to support these sorts
of memory range ioctls so I don't think they need any changes.


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	10 Apr 2002 19:30:27 -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,9 +298,10 @@
 	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));
 	    }
 	    wrmsr(msr, msrv);
 	    mrd += 8;
@@ -290,9 +309,10 @@
 	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));
 	    }
 	    wrmsr(msr, msrv);
 	    mrd += 8;
@@ -300,9 +320,10 @@
 	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));
 	    }
 	    wrmsr(msr, msrv);
 	    mrd += 8;
@@ -313,9 +334,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 +438,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 +471,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? <200204102102.aa16821>