12954 Rewrite coretemp to use the ksensor framework

12955 coretemp had the wrong granularity
Reviewed by: Ryan Zezeski <ryan@zinascii.com>
Reviewed by: Garrett D'Amore <garrett@damore.org>
Approved by: Dan McDonald <danmcd@joyent.com>

1 files changed, 168 insertions, 410 deletions

diff --git a/usr/src/uts/intel/io/coretemp/coretemp.c b/usr/src/uts/intel/io/coretemp/coretemp.c
index e21d385991..f6ba1ac38e 100644
--- a/usr/src/uts/intel/io/coretemp/coretemp.c
+++ b/usr/src/uts/intel/io/coretemp/coretemp.c
@@ -11,6 +11,7 @@
 
 /*
  * Copyright 2019, Joyent, Inc.
+ * Copyright 2020 Oxide Computer Company
  */
 
 /*
@@ -22,7 +23,7 @@
  * basis. The temperature sensor exposes a reading that's relative to the
  * processor's maximum junction temperature, often referred to as Tj. We
  * currently only support models where we can determine that junction
- * temperature programatically. For older processors, we would need to track
+ * temperature programmatically. For older processors, we would need to track
  * down the datasheet. Unfortunately, the values here are often on a per-brand
  * string basis. As in two CPUs with the same model and stepping, but have
  * binned differently have different temperatures.
@@ -46,7 +47,6 @@
 #include <sys/list.h>
 #include <sys/stddef.h>
 #include <sys/cmn_err.h>
-#include <sys/id_space.h>
 #include <sys/x86_archext.h>
 #include <sys/cpu_module.h>
 #include <sys/ontrap.h>
@@ -54,37 +54,44 @@
 #include <sys/x_call.h>
 #include <sys/sensors.h>
 
-#define	CORETEMP_MINOR_MIN	1
-#define	CORETEMP_MINOR_MAX	INT32_MAX
-
-typedef struct coretemp_core {
-	list_node_t		ctc_link;
-	id_t			ctc_core_minor;
-	id_t			ctc_pkg_minor;
-	enum cmi_hdl_class	ctc_class;
-	uint_t			ctc_chip;
-	uint_t			ctc_core;
-	uint_t			ctc_strand;
-	uint_t			ctc_tjmax;
-	hrtime_t		ctc_last_read;
-	uint64_t		ctc_core_status;
-	uint64_t		ctc_core_intr;
-	uint64_t		ctc_pkg_status;
-	uint64_t		ctc_pkg_intr;
-	uint64_t		ctc_invalid_reads;
+/*
+ * The Intel SDM says that the measurements we get are always in degrees
+ * Celsius.
+ */
+#define	CORETEMP_GRANULARITY	1
+
+typedef enum coretemp_sensor_type {
+	CORETEMP_S_CORE,
+	CORETEMP_S_SOCKET
+} coretemp_sensor_type_t;
+
+typedef struct coretemp_sensor {
+	list_node_t		cs_link;
+	struct coretemp		*cs_coretemp;
+	char			cs_name[128];
+	id_t			cs_sensor;
+	coretemp_sensor_type_t	cs_type;
+	enum cmi_hdl_class	cs_class;
+	uint_t			cs_chip;
+	uint_t			cs_core;
+	uint_t			cs_strand;
+	uint_t			cs_tjmax;
+	uint_t			cs_status_msr;
+	uint_t			cs_intr_msr;
+	hrtime_t		cs_last_read;
+	uint64_t		cs_status;
+	uint64_t		cs_intr;
 	/* The following fields are derived from above */
-	uint_t			ctc_temperature;
-	uint_t			ctc_resolution;
-	uint_t			ctc_pkg_temperature;
-} coretemp_core_t;
+	uint_t			cs_temperature;
+	uint_t			cs_resolution;
+} coretemp_sensor_t;
 
 typedef struct coretemp {
 	dev_info_t	*coretemp_dip;
-	id_space_t	*coretemp_ids;
 	cpuset_t	*coretemp_cpuset;
 	boolean_t	coretemp_pkg;
 	kmutex_t	coretemp_mutex;
-	list_t		coretemp_cores;
+	list_t		coretemp_sensors;
 } coretemp_t;
 
 coretemp_t *coretemp;
@@ -165,7 +172,7 @@ coretemp_cmi_errno(cmi_errno_t e)
  *   o The CPU family is 6, which is usually implicit from the above
  *   o We can determine its junction temperature through an MSR
  *
- * If we can't determine the junction temperature programatically, then we need
+ * If we can't determine the junction temperature programmatically, then we need
  * to set up tables of CPUs to do so. This can be fleshed out and improved.
  */
 static boolean_t
@@ -195,30 +202,6 @@ coretemp_supported(void)
 	return (B_TRUE);
 }
 
-static coretemp_core_t *
-coretemp_lookup_core(coretemp_t *ct, minor_t minor)
-{
-	coretemp_core_t *ctc;
-
-	ASSERT(MUTEX_HELD(&ct->coretemp_mutex));
-
-	if (minor < CORETEMP_MINOR_MIN || minor > CORETEMP_MINOR_MAX) {
-		return (NULL);
-	}
-
-	for (ctc = list_head(&ct->coretemp_cores); ctc != NULL;
-	    ctc = list_next(&ct->coretemp_cores, ctc)) {
-		if (ctc->ctc_core_minor == (id_t)minor ||
-		    (ctc->ctc_pkg_minor >= CORETEMP_MINOR_MIN &&
-		    ctc->ctc_pkg_minor == (id_t)minor)) {
-			return (ctc);
-		}
-	}
-
-	return (NULL);
-}
-
-
 /*
  * We need to determine the value of Tj Max as all temperature sensors are
  * derived from this value. The ease of this depends on how old the processor in
@@ -230,199 +213,71 @@ coretemp_lookup_core(coretemp_t *ct, minor_t minor)
  * At the moment, we only support this on processors that have that MSR.
  */
 static int
-coretemp_calculate_tjmax(coretemp_t *ct, coretemp_core_t *ctc, cmi_hdl_t hdl)
+coretemp_calculate_tjmax(coretemp_t *ct, cmi_hdl_t hdl, uint_t *tjmax)
 {
 	cmi_errno_t e;
-	int err = 0;
 	uint64_t val = 0;
 
 	e = coretemp_rdmsr(ct, hdl, MSR_TEMPERATURE_TARGET, &val);
-	if (e == CMI_SUCCESS && val != 0) {
-		ctc->ctc_tjmax = MSR_TEMPERATURE_TARGET_TARGET(val);
+	if (e != CMI_SUCCESS) {
+		return (coretemp_cmi_errno(e));
 	} else if (val == 0) {
-		err = EINVAL;
-	} else {
-		err = coretemp_cmi_errno(e);
+		return (EINVAL);
 	}
 
-	return (err);
+	*tjmax = MSR_TEMPERATURE_TARGET_TARGET(val);
+	return (0);
 }
 
 static int
-coretemp_read(coretemp_t *ct, coretemp_core_t *ctc, cmi_hdl_t hdl)
+coretemp_update(coretemp_t *ct, coretemp_sensor_t *sensor, cmi_hdl_t hdl)
 {
 	cmi_errno_t e;
 	int err = 0;
-	uint64_t val = 0;
-
-	ctc->ctc_last_read = gethrtime();
+	uint64_t intr, status;
 
-	e = coretemp_rdmsr(ct, hdl, MSR_IA32_THERM_STATUS, &val);
-	if (e == CMI_SUCCESS) {
-		ctc->ctc_core_status = val;
-	} else {
+	if ((e = coretemp_rdmsr(ct, hdl, sensor->cs_status_msr, &status)) !=
+	    CMI_SUCCESS) {
 		err = coretemp_cmi_errno(e);
-		dev_err(ct->coretemp_dip, CE_WARN, "!failed to get core "
-		    "thermal status on %u/%u: %d", ctc->ctc_chip, ctc->ctc_core,
-		    err);
+		dev_err(ct->coretemp_dip, CE_WARN, "!failed to get thermal "
+		    "status on %s: %d", sensor->cs_name, err);
 		return (err);
 	}
 
-	e = coretemp_rdmsr(ct, hdl, MSR_IA32_THERM_INTERRUPT, &val);
-	if (e == CMI_SUCCESS) {
-		ctc->ctc_core_intr = val;
-	} else {
+	if ((e = coretemp_rdmsr(ct, hdl, sensor->cs_intr_msr, &intr)) !=
+	    CMI_SUCCESS) {
 		err = coretemp_cmi_errno(e);
-		dev_err(ct->coretemp_dip, CE_WARN, "!failed to get core "
-		    "thermal interrupt on %u/%u: %d", ctc->ctc_chip,
-		    ctc->ctc_core, err);
+		dev_err(ct->coretemp_dip, CE_WARN, "!failed to get thermal "
+		    "interrupt on %s: %d", sensor->cs_name, err);
 		return (err);
 	}
 
-	/*
-	 * If the last read wasn't valid, then we should keep the current state.
-	 */
-	if ((ctc->ctc_core_status & IA32_THERM_STATUS_READ_VALID) != 0) {
-		uint_t diff;
-		diff = IA32_THERM_STATUS_READING(ctc->ctc_core_status);
-
-		if (diff >= ctc->ctc_tjmax) {
-			dev_err(ct->coretemp_dip, CE_WARN, "!found invalid "
-			    "core temperature on %u/%u: readout: %u, Tjmax: "
-			    "%u, raw: 0x%" PRIx64, ctc->ctc_chip,
-			    ctc->ctc_core, diff, ctc->ctc_tjmax,
-			    ctc->ctc_core_status);
-			ctc->ctc_invalid_reads++;
-		} else {
-			ctc->ctc_temperature = ctc->ctc_tjmax - diff;
-		}
-	} else {
-		ctc->ctc_invalid_reads++;
-	}
-
-	ctc->ctc_resolution =
-	    IA32_THERM_STATUS_RESOLUTION(ctc->ctc_core_status);
-
-	/*
-	 * If we have package support and this is core zero, then update the
-	 * package data.
-	 */
-	if (ct->coretemp_pkg && ctc->ctc_core == 0) {
-		uint_t diff;
-
-		e = coretemp_rdmsr(ct, hdl, MSR_IA32_PACKAGE_THERM_STATUS,
-		    &val);
-		if (e == CMI_SUCCESS) {
-			ctc->ctc_pkg_status = val;
-		} else {
-			err = coretemp_cmi_errno(e);
-			dev_err(ct->coretemp_dip, CE_WARN, "!failed to get "
-			    "package thermal status on %u: %d", ctc->ctc_chip,
-			    err);
-			return (err);
-		}
-
-		e = coretemp_rdmsr(ct, hdl, MSR_IA32_PACKAGE_THERM_INTERRUPT,
-		    &val);
-		if (e == CMI_SUCCESS) {
-			ctc->ctc_pkg_intr = val;
-		} else {
-			err = coretemp_cmi_errno(e);
-			dev_err(ct->coretemp_dip, CE_WARN, "!failed to get "
-			    "package thermal interrupt on %u: %d",
-			    ctc->ctc_chip, err);
-			return (err);
-		}
-
-		diff = IA32_PKG_THERM_STATUS_READING(ctc->ctc_pkg_status);
-		if (diff >= ctc->ctc_tjmax) {
-			dev_err(ct->coretemp_dip, CE_WARN, "!found invalid "
-			    "package temperature on %u: readout: %u, tjmax: "
-			    "%u, raw: 0x%" PRIx64, ctc->ctc_chip, diff,
-			    ctc->ctc_tjmax, ctc->ctc_pkg_status);
-			ctc->ctc_invalid_reads++;
-
-		} else {
-			ctc->ctc_pkg_temperature = ctc->ctc_tjmax - diff;
-		}
-	}
-
+	sensor->cs_status = status;
+	sensor->cs_intr = intr;
+	sensor->cs_last_read = gethrtime();
 	return (0);
 }
 
 static int
-coretemp_open(dev_t *devp, int flags, int otype, cred_t *credp)
+coretemp_read(void *arg, sensor_ioctl_temperature_t *sit)
 {
-	coretemp_t *ct = coretemp;
-
-	if (crgetzoneid(credp) != GLOBAL_ZONEID || drv_priv(credp)) {
-		return (EPERM);
-	}
-
-	if ((flags & (FEXCL | FNDELAY | FWRITE)) != 0) {
-		return (EINVAL);
-	}
-
-	if (otype != OTYP_CHR) {
-		return (EINVAL);
-	}
-
-	/*
-	 * Sanity check the minor
-	 */
-	mutex_enter(&ct->coretemp_mutex);
-	if (coretemp_lookup_core(ct, getminor(*devp)) == NULL) {
-		mutex_exit(&ct->coretemp_mutex);
-		return (ENXIO);
-	}
-	mutex_exit(&ct->coretemp_mutex);
-
-	return (0);
-}
-
-static int
-coretemp_ioctl_kind(intptr_t arg, int mode)
-{
-	sensor_ioctl_kind_t kind;
-
-	bzero(&kind, sizeof (kind));
-	kind.sik_kind = SENSOR_KIND_TEMPERATURE;
-
-	if (ddi_copyout((void *)&kind, (void *)arg, sizeof (kind),
-	    mode & FKIOCTL) != 0) {
-		return (EFAULT);
-	}
-
-	return (0);
-}
-
-static int
-coretemp_ioctl_temp(coretemp_t *ct, minor_t minor, intptr_t arg, int mode)
-{
-	coretemp_core_t *ctc;
+	coretemp_sensor_t *sensor = arg;
+	coretemp_t *ct = sensor->cs_coretemp;
 	hrtime_t diff;
-	sensor_ioctl_temperature_t temp;
-
-	bzero(&temp, sizeof (temp));
+	uint_t reading, resolution;
 
 	mutex_enter(&ct->coretemp_mutex);
-	ctc = coretemp_lookup_core(ct, minor);
-	if (ctc == NULL) {
-		mutex_exit(&ct->coretemp_mutex);
-		return (ENXIO);
-	}
-
-	diff = NSEC2MSEC(gethrtime() - ctc->ctc_last_read);
+	diff = NSEC2MSEC(gethrtime() - sensor->cs_last_read);
 	if (diff > 0 && diff > (hrtime_t)coretemp_cache_ms) {
 		int ret;
 		cmi_hdl_t hdl;
 
-		if ((hdl = cmi_hdl_lookup(ctc->ctc_class, ctc->ctc_chip,
-		    ctc->ctc_core, ctc->ctc_strand)) == NULL) {
+		if ((hdl = cmi_hdl_lookup(sensor->cs_class, sensor->cs_chip,
+		    sensor->cs_core, sensor->cs_strand)) == NULL) {
 			mutex_exit(&ct->coretemp_mutex);
 			return (ENXIO);
 		}
-		ret = coretemp_read(ct, ctc, hdl);
+		ret = coretemp_update(ct, sensor, hdl);
 		cmi_hdl_rele(hdl);
 		if (ret != 0) {
 			mutex_exit(&ct->coretemp_mutex);
@@ -430,106 +285,124 @@ coretemp_ioctl_temp(coretemp_t *ct, minor_t minor, intptr_t arg, int mode)
 		}
 	}
 
-	temp.sit_unit = SENSOR_UNIT_CELSIUS;
-	if ((id_t)minor == ctc->ctc_core_minor) {
-		temp.sit_temp = ctc->ctc_temperature;
-	} else {
-		temp.sit_temp = ctc->ctc_pkg_temperature;
-	}
-
-	/*
-	 * The resolution field is in whole units of degrees Celsius.
-	 */
-	temp.sit_gran = ctc->ctc_resolution;
-	if (ctc->ctc_resolution > 1) {
-		temp.sit_gran *= -1;
-	}
-	mutex_exit(&ct->coretemp_mutex);
-
-	if (ddi_copyout(&temp, (void *)arg, sizeof (temp),
-	    mode & FKIOCTL) != 0) {
-		return (EFAULT);
+	switch (sensor->cs_type) {
+	case CORETEMP_S_CORE:
+		if ((sensor->cs_status & IA32_THERM_STATUS_READ_VALID) == 0) {
+			mutex_exit(&ct->coretemp_mutex);
+			return (EIO);
+		}
+		reading = IA32_THERM_STATUS_READING(sensor->cs_status);
+		resolution = IA32_THERM_STATUS_RESOLUTION(sensor->cs_status);
+		break;
+	case CORETEMP_S_SOCKET:
+		reading = IA32_PKG_THERM_STATUS_READING(sensor->cs_status);
+		resolution = 0;
+		break;
+	default:
+		mutex_exit(&ct->coretemp_mutex);
+		return (ENXIO);
 	}
-
-	return (0);
-}
-
-static int
-coretemp_ioctl(dev_t dev, int cmd, intptr_t arg, int mode, cred_t *credp,
-    int *rvalp)
-{
-	coretemp_t *ct = coretemp;
-
-	if ((mode & FREAD) == 0) {
-		return (EINVAL);
+	if (reading >= sensor->cs_tjmax) {
+		dev_err(ct->coretemp_dip, CE_WARN, "!found invalid temperature "
+		    "on sensor %s: readout: %u, tjmax: %u, raw: 0x%"
+		    PRIx64, sensor->cs_name, reading, sensor->cs_tjmax,
+		    sensor->cs_status);
+		mutex_exit(&ct->coretemp_mutex);
+		return (EIO);
 	}
+	sensor->cs_temperature = sensor->cs_tjmax - reading;
+	sensor->cs_resolution = resolution;
 
-	switch (cmd) {
-	case SENSOR_IOCTL_TYPE:
-		return (coretemp_ioctl_kind(arg, mode));
-	case SENSOR_IOCTL_TEMPERATURE:
-		return (coretemp_ioctl_temp(ct, getminor(dev), arg, mode));
-	default:
-		return (ENOTTY);
-	}
-}
+	sit->sit_unit = SENSOR_UNIT_CELSIUS;
+	sit->sit_temp = sensor->cs_temperature;
+	sit->sit_gran = CORETEMP_GRANULARITY;
+	sit->sit_prec = sensor->cs_resolution;
+	mutex_exit(&ct->coretemp_mutex);
 
-/*
- * We don't really do any state tracking on close, so for now, just allow it to
- * always succeed.
- */
-static int
-coretemp_close(dev_t dev, int flags, int otype, cred_t *credp)
-{
 	return (0);
 }
 
-static void
-coretemp_fini_core(coretemp_t *ct, coretemp_core_t *ctc)
-{
-	if (ctc->ctc_core_minor > 0)
-		id_free(ct->coretemp_ids, ctc->ctc_core_minor);
-	if (ctc->ctc_pkg_minor > 0)
-		id_free(ct->coretemp_ids, ctc->ctc_pkg_minor);
-	kmem_free(ctc, sizeof (coretemp_core_t));
-}
+static const ksensor_ops_t coretemp_temp_ops = {
+	.kso_kind = ksensor_kind_temperature,
+	.kso_temp = coretemp_read
+};
 
 static void
 coretemp_destroy(coretemp_t *ct)
 {
-	coretemp_core_t *ctc;
-
-	ddi_remove_minor_node(ct->coretemp_dip, NULL);
+	coretemp_sensor_t *sensor;
 
-	while ((ctc = list_remove_head(&ct->coretemp_cores)) != NULL) {
-		coretemp_fini_core(ct, ctc);
+	(void) ksensor_remove(ct->coretemp_dip, KSENSOR_ALL_IDS);
+	while ((sensor = list_remove_head(&ct->coretemp_sensors)) != NULL) {
+		kmem_free(sensor, sizeof (coretemp_sensor_t));
 	}
-	list_destroy(&ct->coretemp_cores);
+	list_destroy(&ct->coretemp_sensors);
 
 	if (ct->coretemp_cpuset != NULL) {
 		cpuset_free(ct->coretemp_cpuset);
 	}
 
-	if (ct->coretemp_ids != NULL) {
-		id_space_destroy(ct->coretemp_ids);
-	}
-
 	mutex_destroy(&ct->coretemp_mutex);
 	kmem_free(ct, sizeof (coretemp_t));
 }
 
+static boolean_t
+coretemp_create_sensor(coretemp_t *ct, cmi_hdl_t hdl, uint_t tjmax,
+    coretemp_sensor_type_t type)
+{
+	int err;
+	coretemp_sensor_t *sensor;
+
+	sensor = kmem_zalloc(sizeof (coretemp_sensor_t), KM_SLEEP);
+	sensor->cs_coretemp = ct;
+	sensor->cs_type = type;
+	sensor->cs_class = cmi_hdl_class(hdl);
+	sensor->cs_chip = cmi_hdl_chipid(hdl);
+	sensor->cs_core = cmi_hdl_coreid(hdl);
+	sensor->cs_strand = 0;
+	sensor->cs_tjmax = tjmax;
+
+	switch (sensor->cs_type) {
+	case CORETEMP_S_CORE:
+		if (snprintf(sensor->cs_name, sizeof (sensor->cs_name),
+		    "chip%u.core%u", sensor->cs_chip, sensor->cs_core) >=
+		    sizeof (sensor->cs_name)) {
+			goto err;
+		}
+		sensor->cs_status_msr = MSR_IA32_THERM_STATUS;
+		sensor->cs_intr_msr = MSR_IA32_THERM_INTERRUPT;
+		break;
+	case CORETEMP_S_SOCKET:
+		if (snprintf(sensor->cs_name, sizeof (sensor->cs_name),
+		    "chip%u", sensor->cs_chip) >= sizeof (sensor->cs_name)) {
+			goto err;
+		}
+		sensor->cs_status_msr = MSR_IA32_PACKAGE_THERM_STATUS;
+		sensor->cs_intr_msr = MSR_IA32_PACKAGE_THERM_INTERRUPT;
+		break;
+	}
+
+	if ((err = ksensor_create(ct->coretemp_dip, &coretemp_temp_ops, sensor,
+	    sensor->cs_name, DDI_NT_SENSOR_TEMP_CPU, &sensor->cs_sensor)) !=
+	    0) {
+		dev_err(ct->coretemp_dip, CE_WARN, "failed to create ksensor "
+		    "for %s: %d", sensor->cs_name, err);
+	}
+
+	return (B_TRUE);
+err:
+	kmem_free(sensor, sizeof (coretemp_sensor_t));
+	return (B_FALSE);
+}
+
 static int
-coretemp_init_core(cmi_hdl_t hdl, void *arg1, void *arg2, void *arg3)
+coretemp_walk(cmi_hdl_t hdl, void *arg1, void *arg2, void *arg3)
 {
 	coretemp_t *ct = arg1;
 	boolean_t *walkerr = arg2;
-	coretemp_core_t *ctc;
-	uint_t chip, core;
+	uint_t tjmax;
 	int err;
 
-	chip = cmi_hdl_chipid(hdl);
-	core = cmi_hdl_coreid(hdl);
-
 	/*
 	 * The temperature sensor only exists on a per-core basis. Therefore we
 	 * ignore any non-zero strand.
@@ -538,78 +411,26 @@ coretemp_init_core(cmi_hdl_t hdl, void *arg1, void *arg2, void *arg3)
 		return (CMI_HDL_WALK_NEXT);
 	}
 
-	ctc = kmem_zalloc(sizeof (coretemp_core_t), KM_SLEEP);
-	ctc->ctc_class = cmi_hdl_class(hdl);
-	ctc->ctc_chip = chip;
-	ctc->ctc_core = core;
-	ctc->ctc_strand = 0;
-	ctc->ctc_core_minor = id_alloc(ct->coretemp_ids);
-	if (ct->coretemp_pkg && ctc->ctc_core == 0) {
-		ctc->ctc_pkg_minor = id_alloc(ct->coretemp_ids);
-	}
-
-	if ((err = coretemp_calculate_tjmax(ct, ctc, hdl)) != 0) {
+	if ((err = coretemp_calculate_tjmax(ct, hdl, &tjmax)) != 0) {
 		dev_err(ct->coretemp_dip, CE_WARN,
-		    "failed to read Tj Max on %u/%u: %d", chip, core, err);
+		    "failed to read Tj Max on %u/%u: %d", cmi_hdl_chipid(hdl),
+		    cmi_hdl_coreid(hdl), err);
 		*walkerr = B_TRUE;
-		coretemp_fini_core(ct, ctc);
 		return (CMI_HDL_WALK_DONE);
 	}
 
-	if ((err = coretemp_read(ct, ctc, hdl)) != 0) {
-		dev_err(ct->coretemp_dip, CE_WARN,
-		    "failed to take initial temperature reading on %u/%u: %d",
-		    chip, core, err);
+	if (!coretemp_create_sensor(ct, hdl, tjmax, CORETEMP_S_CORE)) {
 		*walkerr = B_TRUE;
-		coretemp_fini_core(ct, ctc);
 		return (CMI_HDL_WALK_DONE);
 	}
 
-	list_insert_tail(&ct->coretemp_cores, ctc);
-
-	return (CMI_HDL_WALK_NEXT);
-}
-
-static boolean_t
-coretemp_create_minors(coretemp_t *ct)
-{
-	coretemp_core_t *ctc;
-
-	for (ctc = list_head(&ct->coretemp_cores); ctc != NULL;
-	    ctc = list_next(&ct->coretemp_cores, ctc)) {
-		int ret;
-		char buf[128];
-
-		if (snprintf(buf, sizeof (buf), "chip%u.core%u", ctc->ctc_chip,
-		    ctc->ctc_core) >= sizeof (buf)) {
-			return (B_FALSE);
-		}
-		ret = ddi_create_minor_node(ct->coretemp_dip, buf, S_IFCHR,
-		    ctc->ctc_core_minor, DDI_NT_SENSOR_TEMP_CPU, 0);
-		if (ret != DDI_SUCCESS) {
-			dev_err(ct->coretemp_dip, CE_WARN, "!failed to create "
-			    "minor node %s", buf);
-			return (B_FALSE);
-		}
-
-		if (ctc->ctc_core != 0)
-			continue;
-
-		if (snprintf(buf, sizeof (buf), "chip%u", ctc->ctc_chip) >=
-		    sizeof (buf)) {
-			return (B_FALSE);
-		}
-
-		ret = ddi_create_minor_node(ct->coretemp_dip, buf, S_IFCHR,
-		    ctc->ctc_pkg_minor, DDI_NT_SENSOR_TEMP_CPU, 0);
-		if (ret != DDI_SUCCESS) {
-			dev_err(ct->coretemp_dip, CE_WARN, "!failed to create "
-			    "minor node %s", buf);
-			return (B_FALSE);
-		}
+	if (ct->coretemp_pkg && cmi_hdl_coreid(hdl) == 0 &&
+	    !coretemp_create_sensor(ct, hdl, tjmax, CORETEMP_S_SOCKET)) {
+		*walkerr = B_TRUE;
+		return (CMI_HDL_WALK_DONE);
 	}
 
-	return (B_TRUE);
+	return (CMI_HDL_WALK_NEXT);
 }
 
 static int
@@ -619,13 +440,8 @@ coretemp_attach(dev_info_t *dip, ddi_attach_cmd_t cmd)
 	coretemp_t *ct = NULL;
 
 	if (cmd == DDI_RESUME) {
-		/*
-		 * Currently suspend and resume for this driver are nops.
-		 */
 		return (DDI_SUCCESS);
-	}
-
-	if (cmd != DDI_ATTACH) {
+	} else if (cmd != DDI_ATTACH) {
 		return (DDI_FAILURE);
 	}
 
@@ -636,29 +452,20 @@ coretemp_attach(dev_info_t *dip, ddi_attach_cmd_t cmd)
 	ct = kmem_zalloc(sizeof (coretemp_t), KM_SLEEP);
 	ct->coretemp_dip = dip;
 	ct->coretemp_pkg = is_x86_feature(x86_featureset, X86FSET_PKG_THERMAL);
-	list_create(&ct->coretemp_cores, sizeof (coretemp_core_t),
-	    offsetof(coretemp_core_t, ctc_link));
+	list_create(&ct->coretemp_sensors, sizeof (coretemp_sensor_t),
+	    offsetof(coretemp_sensor_t, cs_link));
 	mutex_init(&ct->coretemp_mutex, NULL, MUTEX_DRIVER, NULL);
 	ct->coretemp_cpuset = cpuset_alloc(KM_SLEEP);
-	if ((ct->coretemp_ids = id_space_create("coretemp_minors", 1,
-	    INT32_MAX)) == NULL) {
-		goto fail;
-	}
 
 	mutex_enter(&ct->coretemp_mutex);
 	walkerr = B_FALSE;
-	cmi_hdl_walk(coretemp_init_core, ct, &walkerr, NULL);
+	cmi_hdl_walk(coretemp_walk, ct, &walkerr, NULL);
 
 	if (walkerr) {
 		mutex_exit(&ct->coretemp_mutex);
 		goto fail;
 	}
 
-	if (!coretemp_create_minors(ct)) {
-		mutex_exit(&ct->coretemp_mutex);
-		goto fail;
-	}
-
 	coretemp = ct;
 	mutex_exit(&ct->coretemp_mutex);
 	return (DDI_SUCCESS);
@@ -670,38 +477,11 @@ fail:
 }
 
 static int
-coretemp_getinfo(dev_info_t *dip, ddi_info_cmd_t cmd, void *arg,
-    void **resultp)
-{
-	int ret;
-
-	switch (cmd) {
-	case DDI_INFO_DEVT2DEVINFO:
-		*resultp = coretemp->coretemp_dip;
-		ret = DDI_SUCCESS;
-		break;
-	case DDI_INFO_DEVT2INSTANCE:
-		*resultp = (void *)0;
-		ret = DDI_SUCCESS;
-		break;
-	default:
-		ret = DDI_FAILURE;
-		break;
-	}
-
-	return (ret);
-}
-
-static int
 coretemp_detach(dev_info_t *dip, ddi_detach_cmd_t cmd)
 {
-	coretemp_t *ct;
-
 	if (cmd == DDI_SUSPEND) {
 		return (DDI_SUCCESS);
-	}
-
-	if (cmd != DDI_DETACH) {
+	} else if (cmd != DDI_DETACH) {
 		return (DDI_FAILURE);
 	}
 
@@ -709,45 +489,23 @@ coretemp_detach(dev_info_t *dip, ddi_detach_cmd_t cmd)
 		return (DDI_FAILURE);
 	}
 
-	ct = coretemp;
+	coretemp_destroy(coretemp);
 	coretemp = NULL;
-	coretemp_destroy(ct);
 
 	return (DDI_SUCCESS);
 }
 
-static struct cb_ops coretemp_cb_ops = {
-	.cb_open = coretemp_open,
-	.cb_close = coretemp_close,
-	.cb_strategy = nodev,
-	.cb_print = nodev,
-	.cb_dump = nodev,
-	.cb_read = nodev,
-	.cb_write = nodev,
-	.cb_ioctl = coretemp_ioctl,
-	.cb_devmap = nodev,
-	.cb_mmap = nodev,
-	.cb_segmap = nodev,
-	.cb_chpoll = nochpoll,
-	.cb_prop_op = ddi_prop_op,
-	.cb_flag = D_MP,
-	.cb_rev = CB_REV,
-	.cb_aread = nodev,
-	.cb_awrite = nodev
-};
-
 static struct dev_ops coretemp_dev_ops = {
 	.devo_rev = DEVO_REV,
 	.devo_refcnt = 0,
-	.devo_getinfo = coretemp_getinfo,
+	.devo_getinfo = nodev,
 	.devo_identify = nulldev,
 	.devo_probe = nulldev,
 	.devo_attach = coretemp_attach,
 	.devo_detach = coretemp_detach,
 	.devo_reset = nodev,
 	.devo_power = ddi_power,
-	.devo_quiesce = ddi_quiesce_not_needed,
-	.devo_cb_ops = &coretemp_cb_ops
+	.devo_quiesce = ddi_quiesce_not_needed
 };
 
 static struct modldrv coretemp_modldrv = {