Clean up x86 CPU auto-loading

This series fixes a number of apparent bugs in the recent changes to enable auto-loading by x86 CPU ID.

Ben.

Ben Hutchings (5):
x86/cpu: Fix overrun check in arch_print_cpu_modalias() x86/cpu: Clean up modalias feature matching
intel_idle: Fix ID for Nehalem-EX Xeon in device ID table
intel_idle: Revert change of auto_demotion_flags for Nehalem
powernow-k7: Fix CPU family number

 arch/x86/kernel/cpu/match.c   |    5 ++---
 drivers/cpufreq/powernow-k7.c |    2 +-
 drivers/idle/intel_idle.c     |   12 ++++--------
 scripts/mod/file2alias.c      |    5 +++--
 4 files changed, 10 insertions(+), 14 deletions(-)

1.7.9

snprintf() does not return a negative value when truncating.

---
 arch/x86/kernel/cpu/match.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 940e2d4..2dfa52b 100644
@@ -67,7 +67,7 @@ ssize_t arch_print_cpu_modalias(struct device *dev,
 	for (i = 0; i < NCAPINTS*32; i++) {
 		if (boot_cpu_has(i)) {
 			n = snprintf(buf, size, ",%04X", i);
-			if (n < 0) {
+			if (n >= size) {
 				WARN(1, "x86 features overflow page\n");
 				break;
 			}

1.7.9

We currently include commas on both sides of the feature ID in a modalias, but this prevents the lowest numbered feature of a CPU from being matched. Since all feature IDs have the same length, we do not need to worry about substring matches, so omit commas from the modalias entirely.

Avoid generating multiple adjacent wildcards when there is no feature ID to match.

---
 arch/x86/kernel/cpu/match.c |    3 +--
 scripts/mod/file2alias.c    |    5 +++--
 2 files changed, 4 insertions(+), 4 deletions(-)

index 2dfa52b..5502b28 100644
@@ -63,7 +63,7 @@ ssize_t arch_print_cpu_modalias(struct device *dev,
 		boot_cpu_data.x86_model);
 	size -= n;
 	buf += n;
-	size -= 2;
+	size -= 1;
 	for (i = 0; i < NCAPINTS*32; i++) {
 		if (boot_cpu_has(i)) {
 			n = snprintf(buf, size, ",%04X", i);

@@ -75,7 +75,6 @@ ssize_t arch_print_cpu_modalias(struct device *dev,
 			buf += n;
 		}
 	}
-	*buf++ = ',';
 	*buf++ = '\n';
 	return buf - bufptr;
 }

index a468af0..78fd81f 100644
@@ -1021,8 +1021,9 @@ static int do_x86cpu_entry(const char *filename, struct x86_cpu_id *id,
 	ADD(alias, "vendor:",  id->vendor != X86_VENDOR_ANY, id->vendor);
 	ADD(alias, ":family:", id->family != X86_FAMILY_ANY, id->family);
 	ADD(alias, ":model:",  id->model  != X86_MODEL_ANY,  id->model);
-	ADD(alias, ":feature:*,", id->feature != X86_FEATURE_ANY, id->feature);
-	strcat(alias, ",*");
+	strcat(alias, ":feature:*");
+	if (id->feature != X86_FEATURE_ANY)
+		sprintf(alias + strlen(alias), "%04X*", id->feature);
 	return 1;
 }
 ADD_TO_DEVTABLE("x86cpu", struct x86_cpu_id, do_x86cpu_entry);

1.7.9

Commit b66b8b9a4a79087dde1b358a016e5c8739ccf186 ('intel-idle: convert to x86_cpu_id auto probing') put two entries for model 0x2f (Westmere-EX Xeon) in the device ID table and left out model 0x2e (Nehalem-EX Xeon).

---
 drivers/idle/intel_idle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 237fe57..a238649 100644
@@ -360,7 +360,7 @@ static const struct x86_cpu_id intel_idle_ids[] = {
 	ICPU(0x1f, idle_cpu_nehalem),
 	ICPU(0x25, idle_cpu_westmere),
 	ICPU(0x2c, idle_cpu_westmere),
-	ICPU(0x2f, idle_cpu_westmere),
+	ICPU(0x2e, idle_cpu_westmere),
 	ICPU(0x1c, idle_cpu_atom),
 	ICPU(0x26, idle_cpu_lincroft),
 	ICPU(0x2f, idle_cpu_westmere),

1.7.9

Commit b66b8b9a4a79087dde1b358a016e5c8739ccf186 ('intel-idle: convert to x86_cpu_id auto probing') added a distinction between Nehalem and Westemere processors and changed auto_demotion_flags for the former to
0. This was not explained in the commit message, so change it back.

---
I have very little idea what I'm doing here...

Ben.

 drivers/idle/intel_idle.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

index a238649..1c15e9b 100644
@@ -331,10 +331,6 @@ static void auto_demotion_disable(void *dummy)
 
 static const struct idle_cpu idle_cpu_nehalem = {
 	.state_table = nehalem_cstates,
-};
-
-static const struct idle_cpu idle_cpu_westmere = {
-	.state_table = nehalem_cstates,
 	.auto_demotion_disable_flags = NHM_C1_AUTO_DEMOTE | NHM_C3_AUTO_DEMOTE,
 };
 

@@ -358,12 +354,12 @@ static const struct x86_cpu_id intel_idle_ids[] = {
 	ICPU(0x1a, idle_cpu_nehalem),
 	ICPU(0x1e, idle_cpu_nehalem),
 	ICPU(0x1f, idle_cpu_nehalem),
-	ICPU(0x25, idle_cpu_westmere),
-	ICPU(0x2c, idle_cpu_westmere),
-	ICPU(0x2e, idle_cpu_westmere),
+	ICPU(0x25, idle_cpu_nehalem),
+	ICPU(0x2c, idle_cpu_nehalem),
+	ICPU(0x2e, idle_cpu_nehalem),
 	ICPU(0x1c, idle_cpu_atom),
 	ICPU(0x26, idle_cpu_lincroft),
-	ICPU(0x2f, idle_cpu_westmere),
+	ICPU(0x2f, idle_cpu_nehalem),
 	ICPU(0x2a, idle_cpu_snb),
 	ICPU(0x2d, idle_cpu_snb),
 	{}

1.7.9

Yep, it looks like the change slipped in unintentionally.

Thomas

Commit fa8031aefec0cf7ea6c2387c93610d99d9659aa2 ('cpufreq: Add support for x86 cpuinfo auto loading v4') seems to have inadvertently changed the matched CPU family number from 6 to 7. Change it back.

---
 drivers/cpufreq/powernow-k7.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 501d167..cf7e1ee 100644
@@ -112,7 +112,7 @@ static int check_fsb(unsigned int fsbspeed)
 }
 
 static const struct x86_cpu_id powernow_k7_cpuids[] = {
-	{ X86_VENDOR_AMD, 7, },
+	{ X86_VENDOR_AMD, 6, },
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, powernow_k7_cpuids);

1.7.9

These all look correct.
I should have spotted the intel_idle modifications myself. Good catch(es).

Greg: I expect these should get queued in your driver-core-next branch?

Thanks,

Thomas

Either that or we can carry them in -tip as x86 patches, since there isn't a pending conflict this time.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Ok, if there are no dependancies on the driver-core-next tree, please take this through the x86 trees.

thanks,

greg k-h

My mistake; I thought these changes were upstream already. So yes, please take them.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

I can take them, unless the x86 maintainers object and want to take them through their tree.

thanks,

greg k-h

Why don't we take them... I think it makes more sense long-term.

-hpa

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Ok, but for some reason I thought they depended on the patches already in my driver-core-next tree. If that's not the case, please feel free to take them through yours.

greg k-h