Re: Detect on-die temp sensor for Atom E6xx on amd64

2013-02-11 Thread Kurt Miller
On Wednesday 30 January 2013 3:49:34 am Mark Kettenis wrote:
  Date: Tue, 29 Jan 2013 17:35:13 -0500
  From: Matt Dainty m...@bodgit-n-scarper.com
  
  * Christian Weisgerber na...@mips.inka.de [2013-01-24 13:03:43]:
   I think it's dubious that we match the CPU brand name for this at
   all.  Shouldn't this be properly handled with CPUID?
  
  Here's a slightly simpler diff that checks the vendor rather than the
  CPU brand.
  
  I'm not sure if it's safe to check CPUID regardless of vendor, plus
  there is the CLFLUSH code in that block as well, I figure that's
  confined to Intel for a reason?
 
 Look at the equivalent i386 code.

Looking at the the equivalent i386 code shows that ci_cflushsz is
only set when the vendor is Intel and the cpu features has CFLUSH
set. Also core update sensors are only checked when vendor is
Intel and cpuid_level = 0x06. The following diff changes amd64
to follow these rules, allows the Atom E6XX sensors to attach and
brings amd64 closer to i386 for these items.

-Kurt

Index: sys/arch/amd64/amd64/identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.43
diff -u -p -r1.43 identcpu.c
--- sys/arch/amd64/amd64/identcpu.c 10 Nov 2012 09:45:05 -  1.43
+++ sys/arch/amd64/amd64/identcpu.c 12 Feb 2013 04:21:35 -
@@ -506,20 +506,24 @@ identifycpu(struct cpu_info *ci)
if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
replacesmap();
}
-   if (!strncmp(mycpu_model, Intel, 5)) {
-   u_int32_t cflushsz;
+   if (!strcmp(cpu_vendor, GenuineIntel)) {
+   if (ci-ci_feature_flags  CPUID_CFLUSH) {
+   u_int32_t cflushsz;
 
-   CPUID(0x01, dummy, cflushsz, dummy, dummy);
-   /* cflush cacheline size is equal to bits 15-8 of ebx * 8 */
-   ci-ci_cflushsz = ((cflushsz  8)  0xff) * 8;
-   CPUID(0x06, val, dummy, dummy, dummy);
-   if (val  0x1) {
-   strlcpy(ci-ci_sensordev.xname, ci-ci_dev-dv_xname,
-   sizeof(ci-ci_sensordev.xname));
-   ci-ci_sensor.type = SENSOR_TEMP;
-   sensor_task_register(ci, intelcore_update_sensor, 5);
-   sensor_attach(ci-ci_sensordev, ci-ci_sensor);
-   sensordev_install(ci-ci_sensordev);
+   CPUID(0x01, dummy, cflushsz, dummy, dummy);
+   /* cflush cacheline size is equal to bits 15-8 of ebx * 
8 */
+   ci-ci_cflushsz = ((cflushsz  8)  0xff) * 8;
+   }
+   if (cpuid_level = 0x06) {
+   CPUID(0x06, val, dummy, dummy, dummy);
+   if (val  0x1) {
+   strlcpy(ci-ci_sensordev.xname, 
ci-ci_dev-dv_xname,
+   sizeof(ci-ci_sensordev.xname));
+   ci-ci_sensor.type = SENSOR_TEMP;
+   sensor_task_register(ci, 
intelcore_update_sensor, 5);
+   sensor_attach(ci-ci_sensordev, 
ci-ci_sensor);
+   sensordev_install(ci-ci_sensordev);
+   }
}
}
 



Re: Detect on-die temp sensor for Atom E6xx on amd64

2013-01-30 Thread Mark Kettenis
 Date: Tue, 29 Jan 2013 17:35:13 -0500
 From: Matt Dainty m...@bodgit-n-scarper.com
 
 * Christian Weisgerber na...@mips.inka.de [2013-01-24 13:03:43]:
  Matt Dainty m...@bodgit-n-scarper.com wrote:
  
   --- sys/arch/amd64/amd64/identcpu.c.orig  2013-01-14 22:23:43.0 
   +
   +++ sys/arch/amd64/amd64/identcpu.c   2013-01-14 22:33:21.0 
   +
   @@ -506,7 +506,8 @@
 if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
 replacesmap();
 }
   - if (!strncmp(mycpu_model, Intel, 5)) {
   + if (!strncmp(mycpu_model, Intel, 5) ||
   + !strncmp(mycpu_model, Genuine Intel, 13)) {
 u_int32_t cflushsz;

 CPUID(0x01, dummy, cflushsz, dummy, dummy);
   
  
  I think it's dubious that we match the CPU brand name for this at
  all.  Shouldn't this be properly handled with CPUID?
 
 Here's a slightly simpler diff that checks the vendor rather than the
 CPU brand.
 
 I'm not sure if it's safe to check CPUID regardless of vendor, plus
 there is the CLFLUSH code in that block as well, I figure that's
 confined to Intel for a reason?

Look at the equivalent i386 code.



Re: Detect on-die temp sensor for Atom E6xx on amd64

2013-01-30 Thread Matt Dainty
* Mark Kettenis mark.kette...@xs4all.nl [2013-01-30 03:49:45]:
  Date: Tue, 29 Jan 2013 17:35:13 -0500
  From: Matt Dainty m...@bodgit-n-scarper.com
  
  * Christian Weisgerber na...@mips.inka.de [2013-01-24 13:03:43]:
   Matt Dainty m...@bodgit-n-scarper.com wrote:
   
--- sys/arch/amd64/amd64/identcpu.c.orig2013-01-14 
22:23:43.0 +
+++ sys/arch/amd64/amd64/identcpu.c 2013-01-14 22:33:21.0 
+
@@ -506,7 +506,8 @@
if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
replacesmap();
}
-   if (!strncmp(mycpu_model, Intel, 5)) {
+   if (!strncmp(mycpu_model, Intel, 5) ||
+   !strncmp(mycpu_model, Genuine Intel, 13)) {
u_int32_t cflushsz;
 
CPUID(0x01, dummy, cflushsz, dummy, dummy);

   
   I think it's dubious that we match the CPU brand name for this at
   all.  Shouldn't this be properly handled with CPUID?
  
  Here's a slightly simpler diff that checks the vendor rather than the
  CPU brand.
  
  I'm not sure if it's safe to check CPUID regardless of vendor, plus
  there is the CLFLUSH code in that block as well, I figure that's
  confined to Intel for a reason?
 
 Look at the equivalent i386 code.

It looks like the i386 code ultimately does the same thing, looking
through i386/machdep.c, the CPUID is only checked in
intel686_cpusensors_setup() which is traced back to the big
i386_cpuid_cpus[] struct and intel686_cpu_setup()/intel686_p4_cpu_setup()
which to my eyes only gets called for Intel CPUs.

Matt



Re: Detect on-die temp sensor for Atom E6xx on amd64

2013-01-29 Thread Matt Dainty
* Christian Weisgerber na...@mips.inka.de [2013-01-24 13:03:43]:
 Matt Dainty m...@bodgit-n-scarper.com wrote:
 
  --- sys/arch/amd64/amd64/identcpu.c.orig2013-01-14 22:23:43.0 
  +
  +++ sys/arch/amd64/amd64/identcpu.c 2013-01-14 22:33:21.0 +
  @@ -506,7 +506,8 @@
  if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
  replacesmap();
  }
  -   if (!strncmp(mycpu_model, Intel, 5)) {
  +   if (!strncmp(mycpu_model, Intel, 5) ||
  +   !strncmp(mycpu_model, Genuine Intel, 13)) {
  u_int32_t cflushsz;
   
  CPUID(0x01, dummy, cflushsz, dummy, dummy);
  
 
 I think it's dubious that we match the CPU brand name for this at
 all.  Shouldn't this be properly handled with CPUID?

Here's a slightly simpler diff that checks the vendor rather than the
CPU brand.

I'm not sure if it's safe to check CPUID regardless of vendor, plus
there is the CLFLUSH code in that block as well, I figure that's
confined to Intel for a reason?

Matt

--- sys/arch/amd64/amd64/identcpu.c.orig2013-01-14 22:23:43.0 
+
+++ sys/arch/amd64/amd64/identcpu.c 2013-01-29 22:17:44.0 +
@@ -506,7 +506,7 @@
if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
replacesmap();
}
-   if (!strncmp(mycpu_model, Intel, 5)) {
+   if (!strcmp(cpu_vendor, GenuineIntel)) {
u_int32_t cflushsz;
 
CPUID(0x01, dummy, cflushsz, dummy, dummy);



Re: Detect on-die temp sensor for Atom E6xx on amd64

2013-01-24 Thread Christian Weisgerber
Matt Dainty m...@bodgit-n-scarper.com wrote:

 --- sys/arch/amd64/amd64/identcpu.c.orig  2013-01-14 22:23:43.0 
 +
 +++ sys/arch/amd64/amd64/identcpu.c   2013-01-14 22:33:21.0 +
 @@ -506,7 +506,8 @@
   if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
   replacesmap();
   }
 - if (!strncmp(mycpu_model, Intel, 5)) {
 + if (!strncmp(mycpu_model, Intel, 5) ||
 + !strncmp(mycpu_model, Genuine Intel, 13)) {
   u_int32_t cflushsz;
  
   CPUID(0x01, dummy, cflushsz, dummy, dummy);
 

I think it's dubious that we match the CPU brand name for this at
all.  Shouldn't this be properly handled with CPUID?

-- 
Christian naddy Weisgerber  na...@mips.inka.de



Detect on-die temp sensor for Atom E6xx on amd64

2013-01-14 Thread Matt Dainty
The following patch fixes the the detection of the on-die temp sensor
for Atom E6xx CPUs (and possibly other Atom CPUs). The code was only
matching processors where the model began with Intel but this Atom
CPU model begins with Genuine Intel so I've updated the logic to
match.

Now I get:

# sysctl hw
hw.machine=amd64
hw.model=Genuine Intel(R) CPU @ 1.60GHz
hw.ncpu=2
hw.byteorder=1234
hw.pagesize=4096
hw.disknames=sd0:937db626fa67a9f0
hw.diskcount=1
hw.sensors.cpu0.temp0=66.00 degC
hw.sensors.cpu1.temp0=66.00 degC
hw.cpuspeed=600
hw.setperf=0
hw.physmem=2146304000
hw.usermem=2146287616
hw.ncpufound=2
hw.allowpowerdown=1

This only seems to affect the amd64 port, I can't find similar logic in
the i386 port.

Matt

--- sys/arch/amd64/amd64/identcpu.c.orig2013-01-14 22:23:43.0 
+
+++ sys/arch/amd64/amd64/identcpu.c 2013-01-14 22:33:21.0 +
@@ -506,7 +506,8 @@
if (ci-ci_feature_sefflags  SEFF0EBX_SMAP)
replacesmap();
}
-   if (!strncmp(mycpu_model, Intel, 5)) {
+   if (!strncmp(mycpu_model, Intel, 5) ||
+   !strncmp(mycpu_model, Genuine Intel, 13)) {
u_int32_t cflushsz;
 
CPUID(0x01, dummy, cflushsz, dummy, dummy);