Re: CVS commit: src/sys/arch/x86/x86

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

(2013/12/09 0:24), SAITOH Masanobu wrote:
> (2013/12/08 19:21), Jukka Ruohonen wrote:
> > On Sun, Dec 08, 2013 at 04:07:38AM +0000, SAITOH Masanobu wrote:
> > > Module Name:    src
> > > Committed By:   msaitoh
> > > Date:           Sun Dec  8 04:07:38 UTC 2013
> > >
> > > Modified Files:
> > >         src/sys/arch/x86/x86: tsc.c
> > >
> > > Log Message:
> > >   Update invariant TSC detect code from both Intel and AMD documents.
> > > The best way to check whether the TSC counter is invariant or not is to check
> > > CPUID 80000007.
> >
> > It might be worth to refactor this as a function, given that the code is
> > replicated in arch/x86/acpi/acpi_cpu_md.c.
> >
> > - Jukkal.
>
>   Yes, the code can be shared. I'm new to x86 and I'm not familiar
> with acpi, so please do it if you can :)
>
>   For last few months, I have been reading documents and code around
> CPUID and MSR registers. While reading the code, I knew I had to check
> ACPI stuff, but I've not done it yet :-<


 How about the following patch? Is it ok to commit?

Index: sys/arch/x86/acpi/acpi_cpu_md.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/acpi/acpi_cpu_md.c,v
retrieving revision 1.74
diff -u -r1.74 acpi_cpu_md.c
--- sys/arch/x86/acpi/acpi_cpu_md.c     20 Nov 2013 13:52:30 -0000      1.74
+++ sys/arch/x86/acpi/acpi_cpu_md.c     10 Dec 2013 04:55:11 -0000
@@ -43,6 +43,7 @@
 #include <x86/cpuvar.h>
 #include <x86/cpu_msr.h>
 #include <x86/machdep.h>
+#include <x86/x86/tsc.h>
 
 #include <dev/acpi/acpica.h>
 #include <dev/acpi/acpi_cpu.h>
@@ -162,6 +163,16 @@
         */
        val |= ACPICPU_FLAG_C_APIC | ACPICPU_FLAG_C_TSC;
 
+	/*
+        * Detect whether TSC is invariant. If it is not, we keep the flag to
+        * note that TSC will not run at constant rate. Depending on the CPU,
+        * this may affect P- and T-state changes, but especially relevant
+        * are C-states; with variant TSC, states larger than C1 may
+        * completely stop the counter.
+        */
+       if (tsc_is_invariant())
+               val &= ~ACPICPU_FLAG_C_TSC;
+
        switch (cpu_vendor) {
 
 	case CPUVENDOR_IDT:
@@ -214,24 +225,6 @@
                                val &= ~ACPICPU_FLAG_C_APIC;
                }
 
-		/*
-                * Detect whether TSC is invariant. If it is not,
-                * we keep the flag to note that TSC will not run
-                * at constant rate. Depending on the CPU, this may
-                * affect P- and T-state changes, but especially
-                * relevant are C-states; with variant TSC, states
-                * larger than C1 may completely stop the counter.
-                */
-               x86_cpuid(0x80000000, regs);
-
-               if (regs[0] >= 0x80000007) {
-
-                       x86_cpuid(0x80000007, regs);
-
-                       if ((regs[3] & __BIT(8)) != 0)
-                               val &= ~ACPICPU_FLAG_C_TSC;
-               }
-
                break;
 
 	case CPUVENDOR_AMD:
@@ -284,13 +277,10 @@
                case 0x15: /* AMD Bulldozer */
 
 			/*
-                        * Like with Intel, detect invariant TSC,
-                        * MSR-based P-states, and AMD's "turbo"
-                        * (Core Performance Boost), respectively.
+                        * Like with Intel, detect MSR-based P-states,
+                        * and AMD's "turbo" (Core Performance Boost),
+                        * respectively.
                         */
-                       if ((regs[3] & CPUID_APM_TSC) != 0)
-                               val &= ~ACPICPU_FLAG_C_TSC;
-
                        if ((regs[3] & CPUID_APM_HWP) != 0)
                                val |= ACPICPU_FLAG_P_FFH;
 
Index: sys/arch/x86/x86/tsc.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/tsc.c,v
retrieving revision 1.34
diff -u -r1.34 tsc.c
--- sys/arch/x86/x86/tsc.c      8 Dec 2013 04:07:38 -0000       1.34
+++ sys/arch/x86/x86/tsc.c      10 Dec 2013 04:55:11 -0000
@@ -63,23 +63,19 @@
        .tc_quality = 3000,
 };
 
-void
-tsc_tc_init(void)
+bool
+tsc_is_invariant(void)
 {
        struct cpu_info *ci;
        uint32_t descs[4];
        uint32_t family;
        bool invariant;
 
-	if (!cpu_hascounter()) {
-               return;
-       }
+       if (!cpu_hascounter())
+               return false;
 
 	ci = curcpu();
        invariant = false;
-       tsc_freq = ci->ci_data.cpu_cc_freq;
-       tsc_good = (cpu_feature[0] & CPUID_MSR) != 0 &&
-           (rdmsr(MSR_TSC) != 0 || rdmsr(MSR_TSC) != 0);
 
 	if (cpu_vendor == CPUVENDOR_INTEL) {
                /*
@@ -140,6 +136,24 @@
                }
        }
 
+	return invariant;
+}
+
+void
+tsc_tc_init(void)
+{
+       struct cpu_info *ci;
+       bool invariant;
+
+       if (!cpu_hascounter())
+               return;
+
+       ci = curcpu();
+       tsc_freq = ci->ci_data.cpu_cc_freq;
+       tsc_good = (cpu_feature[0] & CPUID_MSR) != 0 &&
+           (rdmsr(MSR_TSC) != 0 || rdmsr(MSR_TSC) != 0);
+
+       invariant = tsc_is_invariant();
        if (!invariant) {
                aprint_debug("TSC not known invariant on this CPU\n");
                tsc_timecounter.tc_quality = -100;
Index: sys/arch/x86/x86/tsc.h
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/tsc.h,v
retrieving revision 1.4
diff -u -r1.4 tsc.h
--- sys/arch/x86/x86/tsc.h      10 May 2008 16:12:32 -0000      1.4
+++ sys/arch/x86/x86/tsc.h      10 Dec 2013 04:55:11 -0000
@@ -26,6 +26,7 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+bool	tsc_is_invariant(void);
 void   tsc_tc_init(void);
 void   tsc_sync_ap(struct cpu_info *);
 void   tsc_sync_bp(struct cpu_info *);


--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)