x86, AMD, cacheinfo: L3 CID fixes

From: Borislav Petkov <[email protected]>

Hi all,

here are three fixes for the AMD L3 cache code that sprang up lately after testing different enterprise backports. Ingo, hpa, it would be cool if they went to urgent and to Linus before .39 is out, if possible.

For that, 3/3 needs ACKs from Greg/Randy before, though.

Patchset is also at

git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git l3-fixes

Thanks.

From: Borislav Petkov <[email protected]>

732eacc0542d0aa48797f675888b85d6065af837 converted code around the kernel using nested max() macros to use the new max3 macro but forgot to remove the old line in intel_cacheinfo.c. Fix it.

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

index 1ce1af2..31590a0 100644
@@ -327,7 +327,6 @@ static void __cpuinit amd_calc_l3_indices(struct amd_l3_cache *l3)
 	l3->subcaches[2] = sc2 = !(val & BIT(8))  + !(val & BIT(9));
 	l3->subcaches[3] = sc3 = !(val & BIT(12)) + !(val & BIT(13));
 
-	l3->indices = (max(max(max(sc0, sc1), sc2), sc3) << 10) - 1;
 	l3->indices = (max(max3(sc0, sc1, sc2), sc3) << 10) - 1;
 }
 

1.7.4.rc2

Please also add:

here to resolve the regression found in the last .38 release.

thanks,

greg k-h

I think you mean this regression:

http://marc.info/?l=linux-kernel&m=130541471818831

If so, we're working on that too. The patch above is just a small fixlet which shouldn't go to stable.

Thanks.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

So are you asking for this patchset to go into .39 or .40?

-hpa

.39 please, if possible.

Thanks.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

At this point, ".39 and not stable" is basically an oxymoron (especially since .39 might very well be out before this patch makes it to Linus), so please clarify what you mean, then.

-hpa

Strictly speaking, path 2/3 needs to be backported to stable because it is a bugfix. The other two not so much, call them fixlets if you like.

However, if .39 is so close, queuing them for .40 so that they go in with the upcoming merge window might be the more sensible thing to do - it's not like we're oopsing or cannot boot without them.

So yeah, please queue them for .40, I'll put them on my backporting TODO-list.

Thanks.

Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Argl, sorry for that!

Commit-ID:  50e7534427283afd997d58481778c07bea79eb63
Gitweb:     http://git.kernel.org/tip/50e7534427283afd997d58481778c07bea79eb63
Author:     Borislav Petkov <[email protected]>
AuthorDate: Mon, 16 May 2011 15:39:46 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 16 May 2011 11:24:23 -0700

x86, AMD, cacheinfo: Fix fallout caused by max3 conversion

732eacc0542d0aa48797f675888b85d6065af837 converted code around the kernel using nested max() macros to use the new max3 macro but forgot to remove the old line in intel_cacheinfo.c. Fix it.

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

index 1ce1af2..31590a0 100644
@@ -327,7 +327,6 @@ static void __cpuinit amd_calc_l3_indices(struct amd_l3_cache *l3)
 	l3->subcaches[2] = sc2 = !(val & BIT(8))  + !(val & BIT(9));
 	l3->subcaches[3] = sc3 = !(val & BIT(12)) + !(val & BIT(13));
 
-	l3->indices = (max(max(max(sc0, sc1), sc2), sc3) << 10) - 1;
 	l3->indices = (max(max3(sc0, sc1, sc2), sc3) << 10) - 1;
 }
 

From: Frank Arnold <[email protected]>

We provide two slots to disable cache indices, and have a check to prevent both slots to be used for the same index.

If the user disables the same index on different subcaches, both slots will hold the same index, e.g.

  $ echo 2047 > /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_0
  $ cat /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_0
  2047
  $ echo 1050623 > /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_1
  $ cat /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_1
  2047

due to the fact that the check was looking only at index bits [11:0] and was ignoring writes to bits outside that range. The more correct fix is to simply check whether the index is within the bounds of [0..l3->indices].

While at it, cleanup comments and drop now-unused local macros.

---
 arch/x86/kernel/cpu/intel_cacheinfo.c |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

index 31590a0..c105c53 100644
@@ -453,27 +453,16 @@ int amd_set_l3_disable_slot(struct amd_l3_cache *l3, int cpu, unsigned slot,
 {
 	int ret = 0;
 
-#define SUBCACHE_MASK	(3UL << 20)
-#define SUBCACHE_INDEX	0xfff
-
-	/*
-	 * check whether this slot is already used or
-	 * the index is already disabled
-	 */
+	/*  check if @slot is already used or the index is already disabled */
 	ret = amd_get_l3_disable_slot(l3, slot);
 	if (ret >= 0)
 		return -EINVAL;
 
-	/*
-	 * check whether the other slot has disabled the
-	 * same index already
-	 */
-	if (index == amd_get_l3_disable_slot(l3, !slot))
+	if (index > l3->indices)
 		return -EINVAL;
 
-	/* do not allow writes outside of allowed bits */
-	if ((index & ~(SUBCACHE_MASK | SUBCACHE_INDEX)) ||
-	    ((index & SUBCACHE_INDEX) > l3->indices))
+	/* check whether the other slot has disabled the same index already */
+	if (index == amd_get_l3_disable_slot(l3, !slot))
 		return -EINVAL;
 
 	amd_l3_disable_index(l3, cpu, slot, index);

1.7.4.rc2

Commit-ID:  42be450565b0fc4607fae3e3a7da038d367a23ed
Gitweb:     http://git.kernel.org/tip/42be450565b0fc4607fae3e3a7da038d367a23ed
Author:     Frank Arnold <[email protected]>
AuthorDate: Mon, 16 May 2011 15:39:47 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 16 May 2011 11:24:27 -0700

x86, AMD, cacheinfo: Fix L3 cache index disable checks

We provide two slots to disable cache indices, and have a check to prevent both slots to be used for the same index.

If the user disables the same index on different subcaches, both slots will hold the same index, e.g.

  $ echo 2047 > /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_0
  $ cat /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_0
  2047
  $ echo 1050623 > /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_1
  $ cat /sys/devices/system/cpu/cpu0/cache/index3/cache_disable_1
  2047

due to the fact that the check was looking only at index bits [11:0] and was ignoring writes to bits outside that range. The more correct fix is to simply check whether the index is within the bounds of [0..l3->indices].

While at it, cleanup comments and drop now-unused local macros.

---
 arch/x86/kernel/cpu/intel_cacheinfo.c |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

index 31590a0..c105c53 100644
@@ -453,27 +453,16 @@ int amd_set_l3_disable_slot(struct amd_l3_cache *l3, int cpu, unsigned slot,
 {
 	int ret = 0;
 
-#define SUBCACHE_MASK	(3UL << 20)
-#define SUBCACHE_INDEX	0xfff
-
-	/*
-	 * check whether this slot is already used or
-	 * the index is already disabled
-	 */
+	/*  check if @slot is already used or the index is already disabled */
 	ret = amd_get_l3_disable_slot(l3, slot);
 	if (ret >= 0)
 		return -EINVAL;
 
-	/*
-	 * check whether the other slot has disabled the
-	 * same index already
-	 */
-	if (index == amd_get_l3_disable_slot(l3, !slot))
+	if (index > l3->indices)
 		return -EINVAL;
 
-	/* do not allow writes outside of allowed bits */
-	if ((index & ~(SUBCACHE_MASK | SUBCACHE_INDEX)) ||
-	    ((index & SUBCACHE_INDEX) > l3->indices))
+	/* check whether the other slot has disabled the same index already */
+	if (index == amd_get_l3_disable_slot(l3, !slot))
 		return -EINVAL;
 
 	amd_l3_disable_index(l3, cpu, slot, index);

From: Borislav Petkov <[email protected]>

Change contact person to AMD kernel mailing list, update text and external references, drop "Users:" tag.

---
 Documentation/ABI/testing/sysfs-devices-system-cpu |   34 ++++++++++----------
 1 files changed, 17 insertions(+), 17 deletions(-)

index 7564e88..e7be75b 100644
@@ -183,21 +183,21 @@ Description:	Discover and change clock speed of CPUs
 		to learn how to control the knobs.
 
 
-What:      /sys/devices/system/cpu/cpu*/cache/index*/cache_disable_X
-Date:      August 2008
+What:		/sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
+Date:		August 2008
 KernelVersion:	2.6.27
-Contact:	[email protected]
-Description:	These files exist in every cpu's cache index directories.
-		There are currently 2 cache_disable_# files in each
-		directory.  Reading from these files on a supported
-		processor will return that cache disable index value
-		for that processor and node.  Writing to one of these
-		files will cause the specificed cache index to be disabled.
-
-		Currently, only AMD Family 10h Processors support cache index
-		disable, and only for their L3 caches.  See the BIOS and
-		Kernel Developer's Guide at
-		http://support.amd.com/us/Embedded_TechDocs/31116-Public-GH-BKDG_3-28_5-28-09.pdf	
-		for formatting information and other details on the
-		cache index disable.
-Users:    [email protected]
+Contact:	[email protected]
+Description:	Disable L3 cache indices
+
+		These files exist in every CPU's cache/index3 directory. Each
+		cache_disable_{0,1} file corresponds to one disable slot which
+		can be used to disable a cache index. Reading from these files
+		on a processor with this functionality will return the currently
+		disabled index for that node. There is one L3 structure per
+		node, or per internal node on MCM machines. Writing a valid
+		index to one of these files will cause the specificed cache
+		index to be disabled.
+
+		All AMD processors with L3 caches provide this functionality.
+		For details, see BKDGs at
+		http://developer.amd.com/documentation/guides/Pages/default.aspx

1.7.4.rc2

thanks for the quick response.

greg k-h

Commit-ID:  eecaaba5b2e4ae762b4726fae2e3b22630e137ec
Gitweb:     http://git.kernel.org/tip/eecaaba5b2e4ae762b4726fae2e3b22630e137ec
Author:     Borislav Petkov <[email protected]>
AuthorDate: Mon, 16 May 2011 15:39:48 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 16 May 2011 11:24:30 -0700

Documentation, ABI: Update L3 cache index disable text

Change contact person to AMD kernel mailing list, update text and external references, drop "Users:" tag.

---
 Documentation/ABI/testing/sysfs-devices-system-cpu |   34 ++++++++++----------
 1 files changed, 17 insertions(+), 17 deletions(-)

index 7564e88..e7be75b 100644
@@ -183,21 +183,21 @@ Description:	Discover and change clock speed of CPUs
 		to learn how to control the knobs.
 
 
-What:      /sys/devices/system/cpu/cpu*/cache/index*/cache_disable_X
-Date:      August 2008
+What:		/sys/devices/system/cpu/cpu*/cache/index3/cache_disable_{0,1}
+Date:		August 2008
 KernelVersion:	2.6.27
-Contact:	[email protected]
-Description:	These files exist in every cpu's cache index directories.
-		There are currently 2 cache_disable_# files in each
-		directory.  Reading from these files on a supported
-		processor will return that cache disable index value
-		for that processor and node.  Writing to one of these
-		files will cause the specificed cache index to be disabled.
-
-		Currently, only AMD Family 10h Processors support cache index
-		disable, and only for their L3 caches.  See the BIOS and
-		Kernel Developer's Guide at
-		http://support.amd.com/us/Embedded_TechDocs/31116-Public-GH-BKDG_3-28_5-28-09.pdf	
-		for formatting information and other details on the
-		cache index disable.
-Users:    [email protected]
+Contact:	[email protected]
+Description:	Disable L3 cache indices
+
+		These files exist in every CPU's cache/index3 directory. Each
+		cache_disable_{0,1} file corresponds to one disable slot which
+		can be used to disable a cache index. Reading from these files
+		on a processor with this functionality will return the currently
+		disabled index for that node. There is one L3 structure per
+		node, or per internal node on MCM machines. Writing a valid
+		index to one of these files will cause the specificed cache
+		index to be disabled.
+
+		All AMD processors with L3 caches provide this functionality.
+		For details, see BKDGs at
+		http://developer.amd.com/documentation/guides/Pages/default.aspx