>Number:         180588
>Category:       kern
>Synopsis:       cpufreq cannot be loaded as kernel module
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Jul 16 04:40:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Jia-Shiun Li
>Release:        10.0-current
>Organization:
>Environment:
FreeBSD 4cbsd 10.0-CURRENT FreeBSD 10.0-CURRENT #0 r252467: Wed Jul  3 12:21:20 
CST 2013     jsli@4cbsd:/usr/obj/usr/src/sys/Minimal  amd64
>Description:
cpufreq, though can be compiled as kernel module, does not actually load. It 
currently rely on gathering feature requests of sub-drivers and requesting to 
ACPI at boot time. If it is disconnected from boot time ACPI logic, it cannot 
tell ACPI to expose needed methods.

Details described in:
http://lists.freebsd.org/pipermail/freebsd-hackers/2013-January/041636.html

>How-To-Repeat:
1. compile and run a kernel without 
  device cpufreq
in kernel config file

2. kldload cpufreq

>Fix:
Remove get_features method of acpi_if.m and relevant code. Instead directly 
specify feature flags OSPM will use to ACPI.

Patch attached with submission follows:

Index: sys/dev/acpica/acpi_cpu.c
===================================================================
--- sys/dev/acpica/acpi_cpu.c   (revision 252537)
+++ sys/dev/acpica/acpi_cpu.c   (working copy)
@@ -279,9 +279,7 @@
     struct acpi_cpu_softc *sc;
     struct acpi_softc    *acpi_sc;
     ACPI_STATUS                   status;
-    u_int                 features;
-    int                           cpu_id, drv_count, i;
-    driver_t             **drivers;
+    int                           cpu_id;
     uint32_t              cap_set[3];
 
     /* UUID needed by _OSC evaluation */
@@ -344,13 +342,12 @@
      * SMP control where each CPU can have different settings.
      */
     sc->cpu_features = ACPI_CAP_SMP_SAME | ACPI_CAP_SMP_SAME_C3;
-    if (devclass_get_drivers(acpi_cpu_devclass, &drivers, &drv_count) == 0) {
-       for (i = 0; i < drv_count; i++) {
-           if (ACPI_GET_FEATURES(drivers[i], &features) == 0)
-               sc->cpu_features |= features;
-       }
-       free(drivers, M_TEMP);
-    }
+    /* est */
+    sc->cpu_features |= ACPI_CAP_PERF_MSRS | ACPI_CAP_C1_IO_HALT;
+    /* p4tcc */
+    sc->cpu_features |= ACPI_CAP_THR_MSRS;
+    /* hwpstate */
+    sc->cpu_features |= ACPI_CAP_PERF_MSRS;
 
     /*
      * CPU capabilities are specified in
Index: sys/dev/acpica/acpi_if.m
===================================================================
--- sys/dev/acpica/acpi_if.m    (revision 252537)
+++ sys/dev/acpica/acpi_if.m    (working copy)
@@ -159,19 +159,6 @@
 };
 
 #
-# Query a given driver for its supported feature(s).  This should be
-# called by the parent bus before the driver is probed.
-#
-# driver_t *driver:  child driver
-#
-# u_int *features:  returned bitmask of all supported features
-#
-STATICMETHOD int get_features {
-       driver_t        *driver;
-       u_int           *features;
-};
-
-#
 # Read embedded controller (EC) address space
 #
 # device_t dev:  EC device
Index: sys/x86/cpufreq/est.c
===================================================================
--- sys/x86/cpufreq/est.c       (revision 252537)
+++ sys/x86/cpufreq/est.c       (working copy)
@@ -899,7 +899,6 @@
 };
 
 static void    est_identify(driver_t *driver, device_t parent);
-static int     est_features(driver_t *driver, u_int *features);
 static int     est_probe(device_t parent);
 static int     est_attach(device_t parent);
 static int     est_detach(device_t parent);
@@ -928,9 +927,6 @@
        DEVMETHOD(cpufreq_drv_type,     est_type),
        DEVMETHOD(cpufreq_drv_settings, est_settings),
 
-       /* ACPI interface */
-       DEVMETHOD(acpi_get_features,    est_features),
-
        {0, 0}
 };
 
@@ -943,18 +939,6 @@
 static devclass_t est_devclass;
 DRIVER_MODULE(est, cpu, est_driver, est_devclass, 0, 0);
 
-static int
-est_features(driver_t *driver, u_int *features)
-{
-
-       /*
-        * Notify the ACPI CPU that we support direct access to MSRs.
-        * XXX C1 "I/O then Halt" seems necessary for some broken BIOS.
-        */
-       *features = ACPI_CAP_PERF_MSRS | ACPI_CAP_C1_IO_HALT;
-       return (0);
-}
-
 static void
 est_identify(driver_t *driver, device_t parent)
 {
Index: sys/x86/cpufreq/hwpstate.c
===================================================================
--- sys/x86/cpufreq/hwpstate.c  (revision 252537)
+++ sys/x86/cpufreq/hwpstate.c  (working copy)
@@ -112,7 +112,6 @@
 static int     hwpstate_settings(device_t dev, struct cf_setting *sets, int 
*count);
 static int     hwpstate_type(device_t dev, int *type);
 static int     hwpstate_shutdown(device_t dev);
-static int     hwpstate_features(driver_t *driver, u_int *features);
 static int     hwpstate_get_info_from_acpi_perf(device_t dev, device_t 
perf_dev);
 static int     hwpstate_get_info_from_msr(device_t dev);
 static int     hwpstate_goto_pstate(device_t dev, int pstate_id);
@@ -136,9 +135,6 @@
        DEVMETHOD(cpufreq_drv_settings, hwpstate_settings),
        DEVMETHOD(cpufreq_drv_type,     hwpstate_type),
 
-       /* ACPI interface */
-       DEVMETHOD(acpi_get_features,    hwpstate_features),
-
        {0, 0}
 };
 
@@ -493,12 +489,3 @@
        /* hwpstate_goto_pstate(dev, 0); */
        return (0);
 }
-
-static int
-hwpstate_features(driver_t *driver, u_int *features)
-{
-
-       /* Notify the ACPI CPU that we support direct access to MSRs */
-       *features = ACPI_CAP_PERF_MSRS;
-       return (0);
-}
Index: sys/x86/cpufreq/p4tcc.c
===================================================================
--- sys/x86/cpufreq/p4tcc.c     (revision 252537)
+++ sys/x86/cpufreq/p4tcc.c     (working copy)
@@ -69,7 +69,6 @@
 #define TCC_REG_OFFSET         1
 #define TCC_SPEED_PERCENT(x)   ((10000 * (x)) / TCC_NUM_SETTINGS)
 
-static int     p4tcc_features(driver_t *driver, u_int *features);
 static void    p4tcc_identify(driver_t *driver, device_t parent);
 static int     p4tcc_probe(device_t dev);
 static int     p4tcc_attach(device_t dev);
@@ -93,9 +92,6 @@
        DEVMETHOD(cpufreq_drv_type,     p4tcc_type),
        DEVMETHOD(cpufreq_drv_settings, p4tcc_settings),
 
-       /* ACPI interface */
-       DEVMETHOD(acpi_get_features,    p4tcc_features),
-
        {0, 0}
 };
 
@@ -108,15 +104,6 @@
 static devclass_t p4tcc_devclass;
 DRIVER_MODULE(p4tcc, cpu, p4tcc_driver, p4tcc_devclass, 0, 0);
 
-static int
-p4tcc_features(driver_t *driver, u_int *features)
-{
-
-       /* Notify the ACPI CPU that we support direct access to MSRs */
-       *features = ACPI_CAP_THR_MSRS;
-       return (0);
-}
-
 static void
 p4tcc_identify(driver_t *driver, device_t parent)
 {


>Release-Note:
>Audit-Trail:
>Unformatted:
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"

Reply via email to