[LinuxBIOS] [Patch] Fix processor name string for Rev. F CPUs

[Sven Kapferer]

This patch fixes the processor name string for Rev F. CPUs.
It moves the complete naming functionality to
src/cpu/amd/model_fxx/processor_name.c

Signed-off-by: Sven Kapferer <[e-mail]>
---

The current code sets the processor name string twice for Rev. F CPUs.

In src/cpu/amd/model_fxx/model_fxx_init.c the function
amd_set_name_string_f is called first. Several lines later
init_processor_name is called which doesn't recognize newer CPUs and
actually programs incorrect values, thus overwriting the previously set
CPU name. For example, this resulted in identifying an Opteron 2218 as a
Turion processor.

This patch removes the amd_set_name_string_f function from
src/cpu/amd/model_fxx/model_fxx_init.c and adds support for Rev. F CPUs
to src/cpu/amd/model_fxx/processor_name.c as described in the Revision
Guide for AMD NPT Family 0Fh Processors, AMD Document ID 33610 Rev 3.00,
October 2006.


Regards,
- Sven

-- 
Sven Kapferer
University of Mannheim - Computer Architecture Group
Address : B6 26, 68131 Mannheim, Germany
Web     : http://ra.ti.uni-mannheim.de
Email   : [e-mail]
Phone   : [phone]   Fax: [phone]

Index: src/cpu/amd/model_fxx/processor_name.c
===================================================================
--- src/cpu/amd/model_fxx/processor_name.c	(revision 2627)
+++ src/cpu/amd/model_fxx/processor_name.c	(working copy)
@@ -9,6 +9,10 @@
  *
  * (C) 2006 by coresystems GmbH <[e-mail]>
  *
+ * Updated by Sven Kapferer <[e-mail]> using
+ * the Revision Guide for AMD NPT Family 0Fh Processors
+ * Document ID 33610 Rev 3.00, October 2006 (Public Version)
+ *
  * This file is subject to the terms and conditions of the GNU General
  * Public License. See the file COPYING in the main directory of this
  * archive for more details.
@@ -25,6 +29,7 @@
  * your mainboard will not be posted on the AMD Recommended Motherboard Website
  */
 
+#if K8_REV_F_SUPPORT == 0
 static char *processor_names[]={
 	/* 0x00 */ "AMD Engineering Sample",
 	/* 0x01-0x03 */ NULL, NULL, NULL,
@@ -80,6 +85,7 @@
 	/* 0x3A */ "Dual Core AMD Opteron(tm) Processor 8RR"
 #define MAX_CPU_NUMBER 0x3A
 };
+#endif
 
 /* wrmsr_amd() is from yhlu's changes to model_fxx_init.c */
 
@@ -92,6 +98,17 @@
                 );
 }
 
+static inline unsigned int cpuid_eax(unsigned int op)
+{
+        unsigned int eax;
+
+        __asm__("cpuid"
+                : "=a" (eax)
+                : "0" (op)
+                : "ebx", "ecx", "edx");
+        return eax;
+}
+
 static inline unsigned int cpuid_ebx(unsigned int op)
 {
         unsigned int eax, ebx;
@@ -103,6 +120,17 @@
         return ebx;
 }
 
+static inline unsigned int cpuid_ecx(unsigned int op)
+{
+        unsigned int eax, ecx;
+
+        __asm__("cpuid"
+                : "=a" (eax), "=c" (ecx)
+                : "0" (op)
+                : "ebx", "edx" );
+        return ecx;
+}
+
 static inline void strcpy(char *dst, char *src) 
 {
 	while (*src) *dst++ = *src++;
@@ -123,6 +151,7 @@
 	char program_string[48];
 	unsigned int *program_values = (unsigned int *)program_string;
 
+#if K8_REV_F_SUPPORT == 0
 	/* Find out which CPU brand it is */
 	EightBitBrandId = cpuid_ebx(0x00000001) & 0xff;
 	BrandId = cpuid_ebx(0x80000001) & 0xffff;
@@ -144,7 +173,104 @@
 
 	if (!processor_name_string)
 		processor_name_string = "AMD Processor model unknown";
+#endif
 
+#if K8_REV_F_SUPPORT == 1
+	u32 Socket;
+	u32 CmpCap;
+	u32 PwrLmt;
+
+	BrandId = cpuid_ebx(0x80000001) & 0xffff;
+	Socket = (cpuid_eax(0x80000001) & 0x00000030) >> 4; // 00b = S1g1, 01b = F (1207), 11b = AM2
+	CmpCap = cpuid_ecx(0x80000008) & 0x03; // Number of CPU cores
+
+	PwrLmt = ((BrandId >> 14) & 0x01) | ((BrandId >> 5) & 0x0e); // BrandId[8:6,14]
+	BrandTableIndex = (BrandId >> 9) & 0x1f; // BrandId[13:9]
+	NN = ((BrandId >> 15) & 0x01) | (BrandId & 0x3f); // BrandId[15,5:0]
+
+	if (((BrandTableIndex == 0) && (PwrLmt == 0)) || (NN == 0)) {
+		processor_name_string = "AMD Engineering Sample";
+	} else {
+		/* Use all fields to identify CPU */
+		switch ((Socket << 16) | (CmpCap << 12) | (BrandTableIndex << 4)
+			| PwrLmt) {
+		/* Socket F */
+		case 0x11016:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 22RR HE";
+			break;
+		case 0x1101a:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 22RR";
+			break;
+		case 0x1101c:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 22RR SE";
+			break;
+		case 0x11046:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 82RR HE";
+			break;
+		case 0x1104a:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 82RR";
+			break;
+		case 0x1104c:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 82RR SE";
+			break;
+		/* Socket AM2 */
+		case 0x30044:
+			processor_name_string =
+			    "AMD Athlon(tm) 64 Processor TT00+";
+			break;
+		case 0x30048:
+			processor_name_string =
+			    "AMD Athlon(tm) 64 Processor TT00+";
+			break;
+		case 0x30064:
+			processor_name_string =
+			    "AMD Sempron(tm) Processor TT00+";
+			break;
+		case 0x30068:
+			processor_name_string =
+			    "AMD Sempron(tm) Processor TT00+";
+			break;
+		case 0x3101a:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 12RR";
+			break;
+		case 0x3101c:
+			processor_name_string =
+			    "Dual-Core AMD Opteron(tm) Processor 12RR SE";
+			break;
+		case 0x31042:
+			processor_name_string =
+			    "AMD Athlon(tm) 64 X2 Dual Core Processor TT00+";
+			break;
+		case 0x31046:
+			processor_name_string =
+			    "AMD Athlon(tm) 64 X2 Dual Core Processor TT00+";
+			break;
+		case 0x31048:
+			processor_name_string =
+			    "AMD Athlon(tm) 64 X2 Dual Core Processor TT00+";
+			break;
+		case 0x3105c:
+			processor_name_string =
+			    "AMD Athlon(tm) 64 FX-ZZ Dual Core Processor";
+			break;
+		/* Socket S1g1 */
+		case 0x0102c:
+			processor_name_string =
+			    "AMD Turion(tm) 64 X2 Mobile Technology TL-YY";
+			break;
+		default:
+			processor_name_string = "AMD Processor model unknown";
+		}
+	}
+#endif
+
 	memset(program_string, 0, 48);
 	strcpy(program_string, processor_name_string);
 	
@@ -156,12 +282,22 @@
 	for (i=0; i<47; i++) { // 48 -1 
 		if(program_string[i] == program_string[i+1]) {
 			switch (program_string[i]) {
+#if K8_REV_F_SUPPORT == 0
 			case 'X': ModelNumber = 22+ NN; break;
 			case 'Y': ModelNumber = 38 + (2*NN); break;
 			case 'Z':
 			case 'T': ModelNumber = 24 + NN; break;
 			case 'R': ModelNumber = 45 + (5*NN); break;
 			case 'V': ModelNumber =  9 + NN; break;
+#endif
+
+#if K8_REV_F_SUPPORT == 1
+			case 'R': ModelNumber = NN - 1; break;
+			case 'P': ModelNumber = 26 + NN; break;
+			case 'T': ModelNumber = 15 + (CmpCap * 10) + NN; break;
+			case 'Z': ModelNumber = 57 + NN; break;
+			case 'Y': ModelNumber = 29 + NN; break;
+#endif
 			}
 			
 			if(ModelNumber && ModelNumber < 100) {
Index: src/cpu/amd/model_fxx/model_fxx_init.c
===================================================================
--- src/cpu/amd/model_fxx/model_fxx_init.c	(revision 2627)
+++ src/cpu/amd/model_fxx/model_fxx_init.c	(working copy)
@@ -462,87 +462,6 @@
 
 }
 
-#if K8_REV_F_SUPPORT == 1
-static void amd_set_name_string_f(device_t dev)
-{
-	unsigned socket;
-	unsigned cmpCap;
-	unsigned pwrLmt;
-	unsigned brandId;
-	unsigned brandTableIndex;
-	unsigned nN;
-	unsigned unknown = 1;
-
-	uint8_t str[48];
-	uint32_t *p;
-
-	msr_t msr;
-	unsigned i;
-	
-	brandId = cpuid_ebx(0x80000001) & 0xffff;
-
-	printk_debug("brandId=%04x\n", brandId);	
-	pwrLmt = ((brandId>>14) & 1) | ((brandId>>5) & 0x0e);
-	brandTableIndex = (brandId>>9) & 0x1f;
-	nN = (brandId & 0x3f) | ((brandId>>(15-6)) &(1<<6));
-
-	socket = (dev->device >> 4) & 0x3;
-
-	cmpCap = cpuid_ecx(0x80000008) & 0xff;
-
-
-        if((brandTableIndex == 0) && (pwrLmt == 0)) {
-        	memset(str, 0, 48);
-                sprintf(str, "AMD Engineering Sample");
-                unknown = 0;
-        } else {
-
-		memset(str, 0, 48);
-		sprintf(str, "AMD Processor model unknown");
-
-	#if CPU_SOCKET_TYPE == 0x10 
-		if(socket == 0x01) { // socket F
-			if ((cmpCap == 1) && ((brandTableIndex==0) ||(brandTableIndex ==1) ||(brandTableIndex == 4)) ) {
-				uint8_t pc[2];
-				unknown = 0;
-				switch (pwrLmt) {
-					case   2: pc[0]= 'E'; pc[1] = 'E'; break;
-					case   6: pc[0]= 'H'; pc[1] = 'E'; break;
-					case 0xa: pc[0]= ' '; pc[1] = ' '; break;
-					case 0xc: pc[0]= 'S'; pc[1] = 'E'; break;
-					default: unknown = 1;
-					
-				}
-				if(!unknown) {
-					memset(str, 0, 48);
-					sprintf(str, "Dual-Core AMD Opteron(tm) Processor %1d2%2d %c%c", brandTableIndex<<1, (nN-1)&0x3f, pc[0], pc[1]);
-				}
-			}
-		}
-	#else
-		#if CPU_SOCKET_TYPE == 0x11
-		if(socket == 0x00) { // socket AM2
-			if(cmpCap == 0) {
-				sprintf(str, "Athlon 64");	
-			} else {
-				sprintf(str, "Athlon 64 Dual Core");
-			}
-
-		}
-		#endif
-	#endif
-	}
-
-	p = str; 
-	for(i=0;i<6;i++) {
-		msr.lo = *p;  p++; msr.hi = *p; p++;
-		wrmsr(0xc0010030+i, msr);
-	}
-	
-
-}
-#endif
-
 extern void model_fxx_update_microcode(unsigned cpu_deviceid);
 int init_processor_name(void);
 
@@ -561,10 +480,6 @@
 	struct cpuinfo_x86 c;
 	
 	get_fms(&c, dev->device);
-
-	if((c.x86_model & 0xf0) == 0x40) {
-		amd_set_name_string_f(dev);
-	} 
 #endif
 
 #if CONFIG_USBDEBUG_DIRECT

[Ward Vandewege]

On Fri, May 04, 2007 at 08:56:20PM +0200, Sven Kapferer wrote:
> In src/cpu/amd/model_fxx/model_fxx_init.c the function
> amd_set_name_string_f is called first. Several lines later
> init_processor_name is called which doesn't recognize newer CPUs and
> actually programs incorrect values, thus overwriting the previously set
> CPU name. For example, this resulted in identifying an Opteron 2218 as a
> Turion processor.

Confirmed - I have seen that behavior too (X2 cpus being misidentified as
Turion when booted with LinuxBIOS).

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Stefan Reinauer]

* Sven Kapferer <[e-mail]> [070504 20:56]:
> This patch fixes the processor name string for Rev F. CPUs.
> It moves the complete naming functionality to
> src/cpu/amd/model_fxx/processor_name.c

Why does REV_F throw out all the old code? Is REV_F always AM2 or newer?

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[yhlu]

Yes

On 5/4/07, Stefan Reinauer <[e-mail]> wrote:
> * Sven Kapferer <[e-mail]> [070504 20:56]:
> > This patch fixes the processor name string for Rev F. CPUs.
> > It moves the complete naming functionality to
> > src/cpu/amd/model_fxx/processor_name.c
>
> Why does REV_F throw out all the old code? Is REV_F always AM2 or newer?

[Sven Kapferer]

yhlu wrote:
> Yes
>
> On 5/4/07, Stefan Reinauer <[e-mail]> wrote:
> > * Sven Kapferer <[e-mail]> [070504 20:56]:
> > > This patch fixes the processor name string for Rev F. CPUs.
> > > It moves the complete naming functionality to
> > > src/cpu/amd/model_fxx/processor_name.c
> > > Why does REV_F throw out all the old code? Is REV_F always AM2 or newer?

AM2 for Athlons, Socket F (1207) for Opterons and Socket S1g1 for Turions.

Regards,
- Sven

-- 
Sven Kapferer
University of Mannheim - Computer Architecture Group
Address : B6 26, 68131 Mannheim, Germany
Web     : http://ra.ti.uni-mannheim.de
Email   : [e-mail]
Phone   : [phone]   Fax: [phone]

[Ward Vandewege]

Hi Sven,

On Fri, May 04, 2007 at 08:56:20PM +0200, Sven Kapferer wrote:
> This patch fixes the processor name string for Rev F. CPUs.

I've just tried this patch on a m57sli-s4 system with an 

   AMD Athlon 64 X2 3800+(65W) Windsor 2.0GHz 2 x 512KB L2 Cache Socket AM2

and it doesn't help detection for me; the cpu is still listed as a

  Mobile AMD Sempron(tm) Processor 3700+

I've attached my cpuinfo output from the proprietary BIOS, and from LB with
your patch.

Interestingly enough, the proprietary BIOS gets the model name right, but not
the cpu clock frequency (!).

Thoughts?

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Stefan Reinauer]

* Ward Vandewege <[e-mail]> [070521 21:30]:
> I've attached my cpuinfo output from the proprietary BIOS, and from LB with
> your patch.

Did you set K8_REV_F_SUPPORT=1? Otherwise Sven's patch will not work.
> Interestingly enough, the proprietary BIOS gets the model name right, but not
> the cpu clock frequency (!).

Maybe power now or something?

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[Ward Vandewege]

On Mon, May 21, 2007 at 09:37:28PM +0200, Stefan Reinauer wrote:
> * Ward Vandewege <[e-mail]> [070521 21:30]:
> > I've attached my cpuinfo output from the proprietary BIOS, and from LB with
> > your patch.
>
> Did you set K8_REV_F_SUPPORT=1? Otherwise Sven's patch will not work.

Ah, I did not. I tried adding it to the mainboard Options.lb:

--- Options.lb  (revision 2680)
+++ Options.lb  (working copy)
@@ -85,6 +85,7 @@
 uses HW_MEM_HOLE_SIZEK
 uses HW_MEM_HOLE_SIZE_AUTO_INC
 uses K8_HT_FREQ_1G_SUPPORT
+uses K8_REV_F_SUPPORT
 
 uses HT_CHAIN_UNITID_BASE
 uses HT_CHAIN_END_UNITID_BASE
@@ -209,6 +210,9 @@
 #Opteron K8 1G HT Support
 default K8_HT_FREQ_1G_SUPPORT=1
 
+#Opteron Rev F (DDR2) support
+default K8_REV_F_SUPPORT=1
+


But that did not seem to have any effect. Is this the right place?
> > Interestingly enough, the proprietary BIOS gets the model name right, but not
> > the cpu clock frequency (!).
>
> Maybe power now or something?

Yeah, you're right, that's why that is.

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Ward Vandewege]

On Mon, May 21, 2007 at 04:02:07PM -0400, Ward Vandewege wrote:
> Ah, I did not. I tried adding it to the mainboard Options.lb:
>
> --- Options.lb  (revision 2680)
> +++ Options.lb  (working copy)
> @@ -85,6 +85,7 @@
>  uses HW_MEM_HOLE_SIZEK
>  uses HW_MEM_HOLE_SIZE_AUTO_INC
>  uses K8_HT_FREQ_1G_SUPPORT
> +uses K8_REV_F_SUPPORT
>
> uses HT_CHAIN_UNITID_BASE
>  uses HT_CHAIN_END_UNITID_BASE
> @@ -209,6 +210,9 @@
>  #Opteron K8 1G HT Support
>  default K8_HT_FREQ_1G_SUPPORT=1
>
> +#Opteron Rev F (DDR2) support
> +default K8_REV_F_SUPPORT=1
> +
>
> But that did not seem to have any effect. Is this the right place?

Uhm, never mind that. I was modifying a different source tree than the one I
was building :(

It works fine:

  model name      : AMD Athlon(tm) 64 X2 Dual Core Processor 3800+

I'll ack Sven's patch, and submit a patch for the m57sli's Options.lb.

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Ward Vandewege]

On Fri, May 04, 2007 at 08:56:20PM +0200, Sven Kapferer wrote:
> This patch fixes the processor name string for Rev F. CPUs.
> It moves the complete naming functionality to
> src/cpu/amd/model_fxx/processor_name.c
>
> Signed-off-by: Sven Kapferer <[e-mail]>

Acked-by: Ward Vandewege <[e-mail]>

I've tested this on the m57sli-s4, and it fixes the processor name string (if
K8_REV_F_SUPPORT=1 is set in Options.lb).

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Stefan Reinauer]

* Ward Vandewege <[e-mail]> [070521 22:25]:
> On Fri, May 04, 2007 at 08:56:20PM +0200, Sven Kapferer wrote:
> > This patch fixes the processor name string for Rev F. CPUs.
> > It moves the complete naming functionality to
> > src/cpu/amd/model_fxx/processor_name.c
> >
> > Signed-off-by: Sven Kapferer <[e-mail]>
>
> Acked-by: Ward Vandewege <[e-mail]>
>
> I've tested this on the m57sli-s4, and it fixes the processor name string (if
> K8_REV_F_SUPPORT=1 is set in Options.lb).

Does it make sense to have the code decide during runtime rather than
during compile time what to do? REV_F_SUPPORT is somewhat misleading, it
should rather say SOCKET_AM2 or SOCKET_WHATEVER_HAS_REV_F_IN_IT to make
the decision require less knowledge from people porting boards.

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[Sven Kapferer]

Ward Vandewege wrote:
> On Fri, May 04, 2007 at 08:56:20PM +0200, Sven Kapferer wrote:
> > This patch fixes the processor name string for Rev F. CPUs.
> > It moves the complete naming functionality to
> > src/cpu/amd/model_fxx/processor_name.c
> >
> > Signed-off-by: Sven Kapferer <[e-mail]>
>
> Acked-by: Ward Vandewege <[e-mail]>
>
> I've tested this on the m57sli-s4, and it fixes the processor name string (if
> K8_REV_F_SUPPORT=1 is set in Options.lb).
>
> Thanks,
> Ward.

This is strange, because the compiler option -DK8_REV_F_SUPPORT='1' was 
already set by including cpu/amd/socket_AM2. I just verified this by 
compiling the gigabyte board with an svn version that doesn't include 
your changes to Options.lb

Regards,
- Sven

-- 
Sven Kapferer
University of Mannheim - Computer Architecture Group
Address : B6 26, 68131 Mannheim, Germany
Web     : http://ra.ti.uni-mannheim.de
Email   : [e-mail]
Phone   : [phone]   Fax: [phone]

[Ward Vandewege]

On Tue, May 22, 2007 at 09:15:54AM +0200, Sven Kapferer wrote:
> This is strange, because the compiler option -DK8_REV_F_SUPPORT='1' was 
> already set by including cpu/amd/socket_AM2. I just verified this by 
> compiling the gigabyte board with an svn version that doesn't include 
> your changes to Options.lb

Yes, confirmed, YH signalled this too yesterday. My patch is superfluous.
Yours should go in though, can someone else review and commit it?

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Ward Vandewege]

On Mon, May 21, 2007 at 04:25:45PM -0400, Ward Vandewege wrote:
> On Fri, May 04, 2007 at 08:56:20PM +0200, Sven Kapferer wrote:
> > This patch fixes the processor name string for Rev F. CPUs.
> > It moves the complete naming functionality to
> > src/cpu/amd/model_fxx/processor_name.c
> >
> > Signed-off-by: Sven Kapferer <[e-mail]>
>
> Acked-by: Ward Vandewege <[e-mail]>
>
> I've tested this on the m57sli-s4, and it fixes the processor name string.

Can someone commit this please, if there is no objection?

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior System Administrator

[Uwe Hermann]

On Fri, May 25, 2007 at 08:15:06AM -0400, Ward Vandewege wrote:
> Can someone commit this please, if there is no objection?

Yep, it's about time. Committed in r2699. Sorry for the delay...

I cannot test the patch, so I hope it doesn't break random stuff ;-) As
nobody reported problems so far I think it's ok to commit.

I'll post an additional patch to fix/adapt the copyright header.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org