Date: Tue, 16 Jul 2013 04:36:50 GMT From: Jia-Shiun Li <jiashiun@gmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/180588: cpufreq cannot be loaded as kernel module Message-ID: <201307160436.r6G4aoQ3003325@oldred.freebsd.org> Resent-Message-ID: <201307160440.r6G4e04m018477@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307160436.r6G4aoQ3003325>