Subject: Re: Powernow support?
To: None <port-amd64@netbsd.org, port-i386@netbsd.org>
From: TAMURA Kent <kent@NetBSD.org>
List: port-amd64
Date: 07/31/2006 17:30:47
> Anyone wants to review the patch or try to fix the code if something is wrong?
> http://www.xtrarom.org/~juan/powernow_k8.patch

I moved powernow_k8.c into sys/arch/x86/x86, modified some files
for i386, and tested it.  Unfortunately, it didn't work because
the BIOS of my machine doesn't provide AMDK7PNOW! PSB entry.  I
guess the machine provides such information just via ACPI.

The following patch fixes uninitialized cstate->n_states
dereference and syctl_createv() failures.

-- 
TAMURA Kent <kent_2006 at hauN.org> <kent at NetBSD.org>


--- arch/amd64/amd64/powernow_k8.c	2006-07-31 17:20:19.000000000 +0900
+++ arch/x86/x86/powernow_k8.c	2006-07-31 17:21:15.000000000 +0900
@@ -415,7 +415,7 @@
 {
 	uint64_t status;
 	u_int maxfid, maxvid, i;
-	const struct sysctlnode *node, *pnownode, *freqnode;
+	const struct sysctlnode *node, *pnownode;
 	struct k8pnow_cpu_state *cstate;
 	struct k8pnow_state *state;
 	struct cpu_info *ci;
@@ -448,18 +448,6 @@
 	else
 		techname = "Cool`n'Quiet";
 
-	freq_names_len = cstate->n_states *(sizeof("9999 ")-1) + 1;
-	freq_names = malloc(freq_names_len, M_SYSCTLDATA, M_WAITOK);
-
-	if (freq_names == NULL)
-		panic("%s: freq_names alloc not possible", __func__);
-
-	freq_table = malloc(sizeof(struct k8pnow_state) * 
-	    cstate->n_states , M_TEMP, M_WAITOK);
-
-	if (freq_table == NULL)
-		panic("%s: freq_table alloc not possible", __func__);
-
 	if (k8pnow_states(cstate, ci->ci_signature, maxfid, maxvid)) {
 		if (cstate->n_states) {
 			for (i = cstate->n_states; i > 0; i--) {
@@ -476,6 +464,19 @@
 			    cstate->n_states);
 		}
 	}
+	if (k8pnow_current_state == NULL) {
+		return;
+	}
+
+	freq_names_len = cstate->n_states *(sizeof("9999 ")-1) + 1;
+	freq_names = malloc(freq_names_len, M_SYSCTLDATA, M_WAITOK);
+	if (freq_names == NULL)
+		panic("%s: freq_names alloc not possible", __func__);
+
+	freq_table = malloc(sizeof(struct k8pnow_state) * 
+	    cstate->n_states , M_TEMP, M_WAITOK);
+	if (freq_table == NULL)
+		panic("%s: freq_table alloc not possible", __func__);
 
 	/* Create sysctl machdep.powernow.frequency. */
 	if ((rc = sysctl_createv(SYSCTLLOG, 0, NULL, &node,
@@ -492,7 +493,7 @@
 	    CTL_CREATE, CTL_EOL)) != 0)
 		goto err;
 
-	if ((rc = sysctl_createv(SYSCTLLOG, 0, &freqnode, &node,
+	if ((rc = sysctl_createv(SYSCTLLOG, 0, &pnownode, &node,
 	    CTLFLAG_READWRITE,
 	    CTLTYPE_INT, "target", NULL,
 	    powernow_sysctl_helper, 0, NULL, 0,
@@ -501,7 +502,7 @@
 
 	powernow_node_target = node->sysctl_num;
 
-	if ((rc = sysctl_createv(SYSCTLLOG, 0, &freqnode, &node,
+	if ((rc = sysctl_createv(SYSCTLLOG, 0, &pnownode, &node,
 	    0,
 	    CTLTYPE_INT, "current", NULL,
 	    powernow_sysctl_helper, 0, NULL, 0,
@@ -510,7 +511,7 @@
 
 	powernow_node_current = node->sysctl_num;
 
-	if ((rc = sysctl_createv(SYSCTLLOG, 0, &freqnode, &node,
+	if ((rc = sysctl_createv(SYSCTLLOG, 0, &pnownode, &node,
 	    0,
 	    CTLTYPE_STRING, "available", NULL,
 	    NULL, 0, freq_names, freq_names_len,