2010-01-22 15:00:59

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

From: Borislav Petkov <[email protected]>

Changelog:

-v3: Add smp WBINVD functionality in a less uglier way :)

-v2: Split smp functionality into a different module

===
these are a bunch of fixes which correct the disabling of L3 cache
indexes. This is needed, for example, in the case when excessive
correctable L3 error rates are observed. In a later step, this
functionality will be connected to the mcheck mechanism and used to
evaluate occurring L3 tag and data array errors and then disable the
respective failing L3 cache regions in order to defer system failure
before a sysop can intervene.

Those patches are also good -stable candidates.

Please apply,
thanks.


2010-01-22 15:01:01

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 3/5] x86, cacheinfo: Fix disabling of L3 cache indices

From: Borislav Petkov <[email protected]>

* Correct the masks used for writing the cache index disable indices.
* Do not turn off L3 scrubber - it is not necessary.
* Make sure wbinvd is executed on the same node where the L3 is.
* Check for out-of-bounds values written to the registers.
* Make show_cache_disable hex values unambiguous
* Check for Erratum #388

Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/kernel/cpu/intel_cacheinfo.c | 34 ++++++++++++++++++++------------
1 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index fc6c8ef..08c91ab 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -18,6 +18,7 @@
#include <asm/processor.h>
#include <linux/smp.h>
#include <asm/k8.h>
+#include <asm/smp.h>

#define LVL_1_INST 1
#define LVL_1_DATA 2
@@ -299,8 +300,10 @@ amd_check_l3_disable(int index, struct _cpuid4_info_regs *this_leaf)
if (boot_cpu_data.x86 == 0x11)
return;

- /* see erratum #382 */
- if ((boot_cpu_data.x86 == 0x10) && (boot_cpu_data.x86_model < 0x8))
+ /* see errata #382 and #388 */
+ if ((boot_cpu_data.x86 == 0x10) &&
+ ((boot_cpu_data.x86_model < 0x9) ||
+ (boot_cpu_data.x86_mask < 0x1)))
return;

this_leaf->can_disable = 1;
@@ -726,12 +729,12 @@ static ssize_t show_cache_disable(struct _cpuid4_info *this_leaf, char *buf,
return -EINVAL;

pci_read_config_dword(dev, 0x1BC + index * 4, &reg);
- return sprintf(buf, "%x\n", reg);
+ return sprintf(buf, "0x%08x\n", reg);
}

#define SHOW_CACHE_DISABLE(index) \
static ssize_t \
-show_cache_disable_##index(struct _cpuid4_info *this_leaf, char *buf) \
+show_cache_disable_##index(struct _cpuid4_info *this_leaf, char *buf) \
{ \
return show_cache_disable(this_leaf, buf, index); \
}
@@ -745,7 +748,9 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf,
int node = cpu_to_node(cpu);
struct pci_dev *dev = node_to_k8_nb_misc(node);
unsigned long val = 0;
- unsigned int scrubber = 0;
+
+#define SUBCACHE_MASK (3UL << 20)
+#define SUBCACHE_INDEX 0xfff

if (!this_leaf->can_disable)
return -EINVAL;
@@ -759,21 +764,24 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf,
if (strict_strtoul(buf, 10, &val) < 0)
return -EINVAL;

- val |= 0xc0000000;
-
- pci_read_config_dword(dev, 0x58, &scrubber);
- scrubber &= ~0x1f000000;
- pci_write_config_dword(dev, 0x58, scrubber);
+ /* do not allow writes outside of allowed bits */
+ if (val & ~(SUBCACHE_MASK | SUBCACHE_INDEX))
+ return -EINVAL;

- pci_write_config_dword(dev, 0x1BC + index * 4, val & ~0x40000000);
- wbinvd();
+ val |= BIT(30);
pci_write_config_dword(dev, 0x1BC + index * 4, val);
+ /*
+ * We need to WBINVD on a core on the node containing the L3 cache which
+ * indices we disable therefore a simple wbinvd() is not sufficient.
+ */
+ wbinvd_on_cpu(cpu);
+ pci_write_config_dword(dev, 0x1BC + index * 4, val | BIT(31));
return count;
}

#define STORE_CACHE_DISABLE(index) \
static ssize_t \
-store_cache_disable_##index(struct _cpuid4_info *this_leaf, \
+store_cache_disable_##index(struct _cpuid4_info *this_leaf, \
const char *buf, size_t count) \
{ \
return store_cache_disable(this_leaf, buf, count, index); \
--
1.6.6

2010-01-22 15:01:08

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 5/5] x86, cacheinfo: Calculate L3 indices

From: Borislav Petkov <[email protected]>

We need to know the valid L3 indices interval when disabling them over
/sysfs. Do that when the core is brought online and add boundary checks
to the sysfs .store attribute.

Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/kernel/cpu/intel_cacheinfo.c | 35 +++++++++++++++++++++++++++++---
1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index 3976ce9..589b705 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -151,7 +151,8 @@ struct _cpuid4_info {
union _cpuid4_leaf_ebx ebx;
union _cpuid4_leaf_ecx ecx;
unsigned long size;
- unsigned long can_disable;
+ bool can_disable;
+ unsigned int l3_indices;
DECLARE_BITMAP(shared_cpu_map, NR_CPUS);
};

@@ -161,7 +162,8 @@ struct _cpuid4_info_regs {
union _cpuid4_leaf_ebx ebx;
union _cpuid4_leaf_ecx ecx;
unsigned long size;
- unsigned long can_disable;
+ bool can_disable;
+ unsigned int l3_indices;
};

unsigned short num_cache_leaves;
@@ -291,6 +293,29 @@ amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
(ebx->split.ways_of_associativity + 1) - 1;
}

+static unsigned int __cpuinit amd_calc_l3_indices(void)
+{
+ /*
+ * We're called over smp_call_function_single() and therefore
+ * are on the correct cpu.
+ */
+ int cpu = smp_processor_id();
+ int node = cpu_to_node(cpu);
+ struct pci_dev *dev = node_to_k8_nb_misc(node);
+ unsigned int sc0, sc1, sc2, sc3;
+ u32 val;
+
+ pci_read_config_dword(dev, 0x1C4, &val);
+
+ /* calculate subcache sizes */
+ sc0 = !(val & BIT(0));
+ sc1 = !(val & BIT(4));
+ sc2 = !(val & BIT(8)) + !(val & BIT(9));
+ sc3 = !(val & BIT(12)) + !(val & BIT(13));
+
+ return (max(max(max(sc0, sc1), sc2), sc3) << 10) - 1;
+}
+
static void __cpuinit
amd_check_l3_disable(int index, struct _cpuid4_info_regs *this_leaf)
{
@@ -306,7 +331,8 @@ amd_check_l3_disable(int index, struct _cpuid4_info_regs *this_leaf)
(boot_cpu_data.x86_mask < 0x1)))
return;

- this_leaf->can_disable = 1;
+ this_leaf->can_disable = true;
+ this_leaf->l3_indices = amd_calc_l3_indices();
}

static int
@@ -765,7 +791,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf,
return -EINVAL;

/* do not allow writes outside of allowed bits */
- if (val & ~(SUBCACHE_MASK | SUBCACHE_INDEX))
+ if ((val & ~(SUBCACHE_MASK | SUBCACHE_INDEX)) ||
+ ((val & SUBCACHE_INDEX) > this_leaf->l3_indices))
return -EINVAL;

val |= BIT(30);
--
1.6.6

2010-01-22 15:01:29

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 4/5] x86, cacheinfo: Add cache index disable sysfs attrs only to L3 caches

From: Borislav Petkov <[email protected]>

The cache_disable_[01] attribute in

/sys/devices/system/cpu/cpu?/cache/index[0-3]/

is enabled on all cache levels although only L3 supports it. Add it only
to the cache level that actually supports it.

Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/kernel/cpu/intel_cacheinfo.c | 35 ++++++++++++++++++++++++--------
1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index 08c91ab..3976ce9 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -814,16 +814,24 @@ static struct _cache_attr cache_disable_0 = __ATTR(cache_disable_0, 0644,
static struct _cache_attr cache_disable_1 = __ATTR(cache_disable_1, 0644,
show_cache_disable_1, store_cache_disable_1);

+#define DEFAULT_SYSFS_CACHE_ATTRS \
+ &type.attr, \
+ &level.attr, \
+ &coherency_line_size.attr, \
+ &physical_line_partition.attr, \
+ &ways_of_associativity.attr, \
+ &number_of_sets.attr, \
+ &size.attr, \
+ &shared_cpu_map.attr, \
+ &shared_cpu_list.attr
+
static struct attribute *default_attrs[] = {
- &type.attr,
- &level.attr,
- &coherency_line_size.attr,
- &physical_line_partition.attr,
- &ways_of_associativity.attr,
- &number_of_sets.attr,
- &size.attr,
- &shared_cpu_map.attr,
- &shared_cpu_list.attr,
+ DEFAULT_SYSFS_CACHE_ATTRS,
+ NULL
+};
+
+static struct attribute *default_l3_attrs[] = {
+ DEFAULT_SYSFS_CACHE_ATTRS,
&cache_disable_0.attr,
&cache_disable_1.attr,
NULL
@@ -916,6 +924,7 @@ static int __cpuinit cache_add_dev(struct sys_device * sys_dev)
unsigned int cpu = sys_dev->id;
unsigned long i, j;
struct _index_kobject *this_object;
+ struct _cpuid4_info *this_leaf;
int retval;

retval = cpuid4_cache_sysfs_init(cpu);
@@ -934,6 +943,14 @@ static int __cpuinit cache_add_dev(struct sys_device * sys_dev)
this_object = INDEX_KOBJECT_PTR(cpu, i);
this_object->cpu = cpu;
this_object->index = i;
+
+ this_leaf = CPUID4_INFO_IDX(cpu, i);
+
+ if (this_leaf->can_disable)
+ ktype_cache.default_attrs = default_l3_attrs;
+ else
+ ktype_cache.default_attrs = default_attrs;
+
retval = kobject_init_and_add(&(this_object->kobj),
&ktype_cache,
per_cpu(ici_cache_kobject, cpu),
--
1.6.6

2010-01-22 15:01:41

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 2/5] intel-agp: Switch to wbinvd_on_all_cpus

From: Borislav Petkov <[email protected]>

Simplify if-statement while at it.

Cc: Dave Jones <[email protected]>
Cc: David Airlie <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/char/agp/intel-agp.c | 14 +++-----------
1 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 30c36ac..28660b4 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -815,12 +815,6 @@ static void intel_i830_setup_flush(void)
intel_i830_fini_flush();
}

-static void
-do_wbinvd(void *null)
-{
- wbinvd();
-}
-
/* The chipset_flush interface needs to get data that has already been
* flushed out of the CPU all the way out to main memory, because the GPU
* doesn't snoop those buffers.
@@ -837,12 +831,10 @@ static void intel_i830_chipset_flush(struct agp_bridge_data *bridge)

memset(pg, 0, 1024);

- if (cpu_has_clflush) {
+ if (cpu_has_clflush)
clflush_cache_range(pg, 1024);
- } else {
- if (on_each_cpu(do_wbinvd, NULL, 1) != 0)
- printk(KERN_ERR "Timed out waiting for cache flush.\n");
- }
+ else if (wbinvd_on_all_cpus() != 0)
+ printk(KERN_ERR "Timed out waiting for cache flush.\n");
}

/* The intel i830 automatically initializes the agp aperture during POST.
--
1.6.6

2010-01-22 15:01:55

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH 1/5] x86, lib: Add wbinvd smp helpers

From: Borislav Petkov <[email protected]>

Add wbinvd_on_cpu and wbinvd_on_all_cpus stubs for executing wbinvd on a
particular CPU.

Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/smp.h | 5 +++++
arch/x86/lib/Makefile | 1 +
arch/x86/lib/smp.c | 19 +++++++++++++++++++
3 files changed, 25 insertions(+), 0 deletions(-)
create mode 100644 arch/x86/lib/smp.c

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 1e79678..82e675b 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -135,6 +135,8 @@ int native_cpu_disable(void);
void native_cpu_die(unsigned int cpu);
void native_play_dead(void);
void play_dead_common(void);
+void wbinvd_on_cpu(int cpu);
+int wbinvd_on_all_cpus(void);

void native_send_call_func_ipi(const struct cpumask *mask);
void native_send_call_func_single_ipi(int cpu);
@@ -147,6 +149,9 @@ static inline int num_booting_cpus(void)
{
return cpumask_weight(cpu_callout_mask);
}
+#else /* !CONFIG_SMP */
+#define wbinvd_on_cpu(cpu) wbinvd()
+#define wbinvd_on_all_cpus() wbinvd()
#endif /* CONFIG_SMP */

extern unsigned disabled_cpus __cpuinitdata;
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index cffd754..171cb14 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -15,6 +15,7 @@ $(obj)/inat.o: $(obj)/inat-tables.c
clean-files := inat-tables.c

obj-$(CONFIG_SMP) += msr-smp.o
+obj-$(CONFIG_SMP) += smp.o

lib-y := delay.o
lib-y += thunk_$(BITS).o
diff --git a/arch/x86/lib/smp.c b/arch/x86/lib/smp.c
new file mode 100644
index 0000000..a3c6688
--- /dev/null
+++ b/arch/x86/lib/smp.c
@@ -0,0 +1,19 @@
+#include <linux/smp.h>
+#include <linux/module.h>
+
+static void __wbinvd(void *dummy)
+{
+ wbinvd();
+}
+
+void wbinvd_on_cpu(int cpu)
+{
+ smp_call_function_single(cpu, __wbinvd, NULL, 1);
+}
+EXPORT_SYMBOL(wbinvd_on_cpu);
+
+int wbinvd_on_all_cpus(void)
+{
+ return on_each_cpu(__wbinvd, NULL, 1);
+}
+EXPORT_SYMBOL(wbinvd_on_all_cpus);
--
1.6.6

2010-01-22 17:25:45

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

On 01/22/2010 07:01 AM, Borislav Petkov wrote:
> these are a bunch of fixes which correct the disabling of L3 cache
> indexes. This is needed, for example, in the case when excessive
> correctable L3 error rates are observed. In a later step, this
> functionality will be connected to the mcheck mechanism and used to
> evaluate occurring L3 tag and data array errors and then disable the
> respective failing L3 cache regions in order to defer system failure
> before a sysop can intervene.
>
> Those patches are also good -stable candidates.

Hmmm... I'm not sure I see a strong justification for a late -rc push
into Linus/stable push for for these... I think you would have to
explicitly make the case if you want them to be considered as such.

-hpa

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

2010-01-22 17:40:44

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

On Fri, Jan 22, 2010 at 09:24:28AM -0800, H. Peter Anvin wrote:
> On 01/22/2010 07:01 AM, Borislav Petkov wrote:
> > these are a bunch of fixes which correct the disabling of L3 cache
> > indexes. This is needed, for example, in the case when excessive
> > correctable L3 error rates are observed. In a later step, this
> > functionality will be connected to the mcheck mechanism and used to
> > evaluate occurring L3 tag and data array errors and then disable the
> > respective failing L3 cache regions in order to defer system failure
> > before a sysop can intervene.
> >
> > Those patches are also good -stable candidates.
>
> Hmmm... I'm not sure I see a strong justification for a late -rc push
> into Linus/stable push for for these... I think you would have to
> explicitly make the case if you want them to be considered as such.

Well, on the one hand, they fix real bugs in the L3 cache index disable
code and since they're bugfixes, they are eligible late -rc candidates.

On the other hand, however and more importantly, the machines which
have that feature are not selling yet so postponing the patches for the
next merge window is still ok. I'll backport them then to .32 for the
distro kernels and .33 and I think we are going to be fine this way.

So queueing them for .34 is still fine with me, thanks.

--
Regards/Gruss,
ボリス.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

2010-01-22 17:50:06

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

On 01/22/2010 09:40 AM, Borislav Petkov wrote:
>>>
>>> Those patches are also good -stable candidates.
>>
>> Hmmm... I'm not sure I see a strong justification for a late -rc push
>> into Linus/stable push for for these... I think you would have to
>> explicitly make the case if you want them to be considered as such.
>
> Well, on the one hand, they fix real bugs in the L3 cache index disable
> code and since they're bugfixes, they are eligible late -rc candidates.
>

Bugfixes are *early* -rc candidates. Regression fixes are *late* -rc
candidates, at least that seems to be the policy Linus currently
implements. -stable seems to use slightly less strict criteria (the
whole point is that -final needs to be a stabilization point, backported
fixes/drivers can then come onto a stable base) which is why you seem
some patches which are "straight to .1".

> On the other hand, however and more importantly, the machines which
> have that feature are not selling yet so postponing the patches for the
> next merge window is still ok. I'll backport them then to .32 for the
> distro kernels and .33 and I think we are going to be fine this way.
>
> So queueing them for .34 is still fine with me, thanks.

OK. You can check with -stable if they want to take the backport post-.33.

-hpa

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

Subject: [tip:x86/cpu] x86, lib: Add wbinvd smp helpers

Commit-ID: a7b480e7f30b3813353ec009f10f2ac7a6669f3b
Gitweb: http://git.kernel.org/tip/a7b480e7f30b3813353ec009f10f2ac7a6669f3b
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 22 Jan 2010 16:01:03 +0100
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 22 Jan 2010 16:05:42 -0800

x86, lib: Add wbinvd smp helpers

Add wbinvd_on_cpu and wbinvd_on_all_cpus stubs for executing wbinvd on a
particular CPU.

[ hpa: renamed lib/smp.c to lib/cache-smp.c ]
[ hpa: wbinvd_on_all_cpus() returns int, but wbinvd() returns
void. Thus, the former cannot be a macro for the latter,
replace with an inline function. ]

Signed-off-by: Borislav Petkov <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/include/asm/smp.h | 9 +++++++++
arch/x86/lib/Makefile | 2 +-
arch/x86/lib/cache-smp.c | 19 +++++++++++++++++++
3 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 1e79678..4cfc908 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -135,6 +135,8 @@ int native_cpu_disable(void);
void native_cpu_die(unsigned int cpu);
void native_play_dead(void);
void play_dead_common(void);
+void wbinvd_on_cpu(int cpu);
+int wbinvd_on_all_cpus(void);

void native_send_call_func_ipi(const struct cpumask *mask);
void native_send_call_func_single_ipi(int cpu);
@@ -147,6 +149,13 @@ static inline int num_booting_cpus(void)
{
return cpumask_weight(cpu_callout_mask);
}
+#else /* !CONFIG_SMP */
+#define wbinvd_on_cpu(cpu) wbinvd()
+static inline int wbinvd_on_all_cpus(void)
+{
+ wbinvd();
+ return 0;
+}
#endif /* CONFIG_SMP */

extern unsigned disabled_cpus __cpuinitdata;
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index cffd754..d85e0e4 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -14,7 +14,7 @@ $(obj)/inat.o: $(obj)/inat-tables.c

clean-files := inat-tables.c

-obj-$(CONFIG_SMP) += msr-smp.o
+obj-$(CONFIG_SMP) += msr-smp.o cache-smp.o

lib-y := delay.o
lib-y += thunk_$(BITS).o
diff --git a/arch/x86/lib/cache-smp.c b/arch/x86/lib/cache-smp.c
new file mode 100644
index 0000000..a3c6688
--- /dev/null
+++ b/arch/x86/lib/cache-smp.c
@@ -0,0 +1,19 @@
+#include <linux/smp.h>
+#include <linux/module.h>
+
+static void __wbinvd(void *dummy)
+{
+ wbinvd();
+}
+
+void wbinvd_on_cpu(int cpu)
+{
+ smp_call_function_single(cpu, __wbinvd, NULL, 1);
+}
+EXPORT_SYMBOL(wbinvd_on_cpu);
+
+int wbinvd_on_all_cpus(void)
+{
+ return on_each_cpu(__wbinvd, NULL, 1);
+}
+EXPORT_SYMBOL(wbinvd_on_all_cpus);

Subject: [tip:x86/cpu] intel-agp: Switch to wbinvd_on_all_cpus

Commit-ID: 48a719c238bcbb72d6da79de9c5b3b93ab472107
Gitweb: http://git.kernel.org/tip/48a719c238bcbb72d6da79de9c5b3b93ab472107
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 22 Jan 2010 16:01:04 +0100
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 22 Jan 2010 16:06:30 -0800

intel-agp: Switch to wbinvd_on_all_cpus

Simplify if-statement while at it.

[ hpa: we need to #include <asm/smp.h> ]

Cc: Dave Jones <[email protected]>
Cc: David Airlie <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
drivers/char/agp/intel-agp.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 3999a5f..8a713f1 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -8,6 +8,7 @@
#include <linux/kernel.h>
#include <linux/pagemap.h>
#include <linux/agp_backend.h>
+#include <asm/smp.h>
#include "agp.h"

/*
@@ -815,12 +816,6 @@ static void intel_i830_setup_flush(void)
intel_i830_fini_flush();
}

-static void
-do_wbinvd(void *null)
-{
- wbinvd();
-}
-
/* The chipset_flush interface needs to get data that has already been
* flushed out of the CPU all the way out to main memory, because the GPU
* doesn't snoop those buffers.
@@ -837,12 +832,10 @@ static void intel_i830_chipset_flush(struct agp_bridge_data *bridge)

memset(pg, 0, 1024);

- if (cpu_has_clflush) {
+ if (cpu_has_clflush)
clflush_cache_range(pg, 1024);
- } else {
- if (on_each_cpu(do_wbinvd, NULL, 1) != 0)
- printk(KERN_ERR "Timed out waiting for cache flush.\n");
- }
+ else if (wbinvd_on_all_cpus() != 0)
+ printk(KERN_ERR "Timed out waiting for cache flush.\n");
}

/* The intel i830 automatically initializes the agp aperture during POST.

Subject: [tip:x86/cpu] x86, cacheinfo: Fix disabling of L3 cache indices

Commit-ID: dcf39daf3d6d97f8741e82f0b9fb7554704ed2d1
Gitweb: http://git.kernel.org/tip/dcf39daf3d6d97f8741e82f0b9fb7554704ed2d1
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 22 Jan 2010 16:01:05 +0100
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 22 Jan 2010 16:06:31 -0800

x86, cacheinfo: Fix disabling of L3 cache indices

* Correct the masks used for writing the cache index disable indices.
* Do not turn off L3 scrubber - it is not necessary.
* Make sure wbinvd is executed on the same node where the L3 is.
* Check for out-of-bounds values written to the registers.
* Make show_cache_disable hex values unambiguous
* Check for Erratum #388

Signed-off-by: Borislav Petkov <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/cpu/intel_cacheinfo.c | 34 ++++++++++++++++++++------------
1 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index fc6c8ef..08c91ab 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -18,6 +18,7 @@
#include <asm/processor.h>
#include <linux/smp.h>
#include <asm/k8.h>
+#include <asm/smp.h>

#define LVL_1_INST 1
#define LVL_1_DATA 2
@@ -299,8 +300,10 @@ amd_check_l3_disable(int index, struct _cpuid4_info_regs *this_leaf)
if (boot_cpu_data.x86 == 0x11)
return;

- /* see erratum #382 */
- if ((boot_cpu_data.x86 == 0x10) && (boot_cpu_data.x86_model < 0x8))
+ /* see errata #382 and #388 */
+ if ((boot_cpu_data.x86 == 0x10) &&
+ ((boot_cpu_data.x86_model < 0x9) ||
+ (boot_cpu_data.x86_mask < 0x1)))
return;

this_leaf->can_disable = 1;
@@ -726,12 +729,12 @@ static ssize_t show_cache_disable(struct _cpuid4_info *this_leaf, char *buf,
return -EINVAL;

pci_read_config_dword(dev, 0x1BC + index * 4, &reg);
- return sprintf(buf, "%x\n", reg);
+ return sprintf(buf, "0x%08x\n", reg);
}

#define SHOW_CACHE_DISABLE(index) \
static ssize_t \
-show_cache_disable_##index(struct _cpuid4_info *this_leaf, char *buf) \
+show_cache_disable_##index(struct _cpuid4_info *this_leaf, char *buf) \
{ \
return show_cache_disable(this_leaf, buf, index); \
}
@@ -745,7 +748,9 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf,
int node = cpu_to_node(cpu);
struct pci_dev *dev = node_to_k8_nb_misc(node);
unsigned long val = 0;
- unsigned int scrubber = 0;
+
+#define SUBCACHE_MASK (3UL << 20)
+#define SUBCACHE_INDEX 0xfff

if (!this_leaf->can_disable)
return -EINVAL;
@@ -759,21 +764,24 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf,
if (strict_strtoul(buf, 10, &val) < 0)
return -EINVAL;

- val |= 0xc0000000;
-
- pci_read_config_dword(dev, 0x58, &scrubber);
- scrubber &= ~0x1f000000;
- pci_write_config_dword(dev, 0x58, scrubber);
+ /* do not allow writes outside of allowed bits */
+ if (val & ~(SUBCACHE_MASK | SUBCACHE_INDEX))
+ return -EINVAL;

- pci_write_config_dword(dev, 0x1BC + index * 4, val & ~0x40000000);
- wbinvd();
+ val |= BIT(30);
pci_write_config_dword(dev, 0x1BC + index * 4, val);
+ /*
+ * We need to WBINVD on a core on the node containing the L3 cache which
+ * indices we disable therefore a simple wbinvd() is not sufficient.
+ */
+ wbinvd_on_cpu(cpu);
+ pci_write_config_dword(dev, 0x1BC + index * 4, val | BIT(31));
return count;
}

#define STORE_CACHE_DISABLE(index) \
static ssize_t \
-store_cache_disable_##index(struct _cpuid4_info *this_leaf, \
+store_cache_disable_##index(struct _cpuid4_info *this_leaf, \
const char *buf, size_t count) \
{ \
return store_cache_disable(this_leaf, buf, count, index); \

Subject: [tip:x86/cpu] x86, cacheinfo: Add cache index disable sysfs attrs only to L3 caches

Commit-ID: 897de50e08937663912c86fb12ad7f708af2386c
Gitweb: http://git.kernel.org/tip/897de50e08937663912c86fb12ad7f708af2386c
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 22 Jan 2010 16:01:06 +0100
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 22 Jan 2010 16:06:31 -0800

x86, cacheinfo: Add cache index disable sysfs attrs only to L3 caches

The cache_disable_[01] attribute in

/sys/devices/system/cpu/cpu?/cache/index[0-3]/

is enabled on all cache levels although only L3 supports it. Add it only
to the cache level that actually supports it.

Signed-off-by: Borislav Petkov <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/cpu/intel_cacheinfo.c | 35 ++++++++++++++++++++++++--------
1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index 08c91ab..3976ce9 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -814,16 +814,24 @@ static struct _cache_attr cache_disable_0 = __ATTR(cache_disable_0, 0644,
static struct _cache_attr cache_disable_1 = __ATTR(cache_disable_1, 0644,
show_cache_disable_1, store_cache_disable_1);

+#define DEFAULT_SYSFS_CACHE_ATTRS \
+ &type.attr, \
+ &level.attr, \
+ &coherency_line_size.attr, \
+ &physical_line_partition.attr, \
+ &ways_of_associativity.attr, \
+ &number_of_sets.attr, \
+ &size.attr, \
+ &shared_cpu_map.attr, \
+ &shared_cpu_list.attr
+
static struct attribute *default_attrs[] = {
- &type.attr,
- &level.attr,
- &coherency_line_size.attr,
- &physical_line_partition.attr,
- &ways_of_associativity.attr,
- &number_of_sets.attr,
- &size.attr,
- &shared_cpu_map.attr,
- &shared_cpu_list.attr,
+ DEFAULT_SYSFS_CACHE_ATTRS,
+ NULL
+};
+
+static struct attribute *default_l3_attrs[] = {
+ DEFAULT_SYSFS_CACHE_ATTRS,
&cache_disable_0.attr,
&cache_disable_1.attr,
NULL
@@ -916,6 +924,7 @@ static int __cpuinit cache_add_dev(struct sys_device * sys_dev)
unsigned int cpu = sys_dev->id;
unsigned long i, j;
struct _index_kobject *this_object;
+ struct _cpuid4_info *this_leaf;
int retval;

retval = cpuid4_cache_sysfs_init(cpu);
@@ -934,6 +943,14 @@ static int __cpuinit cache_add_dev(struct sys_device * sys_dev)
this_object = INDEX_KOBJECT_PTR(cpu, i);
this_object->cpu = cpu;
this_object->index = i;
+
+ this_leaf = CPUID4_INFO_IDX(cpu, i);
+
+ if (this_leaf->can_disable)
+ ktype_cache.default_attrs = default_l3_attrs;
+ else
+ ktype_cache.default_attrs = default_attrs;
+
retval = kobject_init_and_add(&(this_object->kobj),
&ktype_cache,
per_cpu(ici_cache_kobject, cpu),

Subject: [tip:x86/cpu] x86, cacheinfo: Calculate L3 indices

Commit-ID: 048a8774ca43488d78605031f11cc206d7a2682a
Gitweb: http://git.kernel.org/tip/048a8774ca43488d78605031f11cc206d7a2682a
Author: Borislav Petkov <[email protected]>
AuthorDate: Fri, 22 Jan 2010 16:01:07 +0100
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 22 Jan 2010 16:06:31 -0800

x86, cacheinfo: Calculate L3 indices

We need to know the valid L3 indices interval when disabling them over
/sysfs. Do that when the core is brought online and add boundary checks
to the sysfs .store attribute.

Signed-off-by: Borislav Petkov <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/cpu/intel_cacheinfo.c | 35 +++++++++++++++++++++++++++++---
1 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index 3976ce9..589b705 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -151,7 +151,8 @@ struct _cpuid4_info {
union _cpuid4_leaf_ebx ebx;
union _cpuid4_leaf_ecx ecx;
unsigned long size;
- unsigned long can_disable;
+ bool can_disable;
+ unsigned int l3_indices;
DECLARE_BITMAP(shared_cpu_map, NR_CPUS);
};

@@ -161,7 +162,8 @@ struct _cpuid4_info_regs {
union _cpuid4_leaf_ebx ebx;
union _cpuid4_leaf_ecx ecx;
unsigned long size;
- unsigned long can_disable;
+ bool can_disable;
+ unsigned int l3_indices;
};

unsigned short num_cache_leaves;
@@ -291,6 +293,29 @@ amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
(ebx->split.ways_of_associativity + 1) - 1;
}

+static unsigned int __cpuinit amd_calc_l3_indices(void)
+{
+ /*
+ * We're called over smp_call_function_single() and therefore
+ * are on the correct cpu.
+ */
+ int cpu = smp_processor_id();
+ int node = cpu_to_node(cpu);
+ struct pci_dev *dev = node_to_k8_nb_misc(node);
+ unsigned int sc0, sc1, sc2, sc3;
+ u32 val;
+
+ pci_read_config_dword(dev, 0x1C4, &val);
+
+ /* calculate subcache sizes */
+ sc0 = !(val & BIT(0));
+ sc1 = !(val & BIT(4));
+ sc2 = !(val & BIT(8)) + !(val & BIT(9));
+ sc3 = !(val & BIT(12)) + !(val & BIT(13));
+
+ return (max(max(max(sc0, sc1), sc2), sc3) << 10) - 1;
+}
+
static void __cpuinit
amd_check_l3_disable(int index, struct _cpuid4_info_regs *this_leaf)
{
@@ -306,7 +331,8 @@ amd_check_l3_disable(int index, struct _cpuid4_info_regs *this_leaf)
(boot_cpu_data.x86_mask < 0x1)))
return;

- this_leaf->can_disable = 1;
+ this_leaf->can_disable = true;
+ this_leaf->l3_indices = amd_calc_l3_indices();
}

static int
@@ -765,7 +791,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf,
return -EINVAL;

/* do not allow writes outside of allowed bits */
- if (val & ~(SUBCACHE_MASK | SUBCACHE_INDEX))
+ if ((val & ~(SUBCACHE_MASK | SUBCACHE_INDEX)) ||
+ ((val & SUBCACHE_INDEX) > this_leaf->l3_indices))
return -EINVAL;

val |= BIT(30);

2010-01-23 07:00:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes


* H. Peter Anvin <[email protected]> wrote:

> On 01/22/2010 09:40 AM, Borislav Petkov wrote:
> >>>
> >>> Those patches are also good -stable candidates.
> >>
> >> Hmmm... I'm not sure I see a strong justification for a late -rc push
> >> into Linus/stable push for for these... I think you would have to
> >> explicitly make the case if you want them to be considered as such.
> >
> > Well, on the one hand, they fix real bugs in the L3 cache index disable
> > code and since they're bugfixes, they are eligible late -rc candidates.
>
> Bugfixes are *early* -rc candidates. Regression fixes are *late* -rc
> candidates, at least that seems to be the policy Linus currently implements.
> -stable seems to use slightly less strict criteria (the whole point is that
> -final needs to be a stabilization point, backported fixes/drivers can then
> come onto a stable base) which is why you seem some patches which are
> "straight to .1".

Yes.

Ingo

2010-01-23 08:11:40

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

On Sat, Jan 23, 2010 at 07:59:53AM +0100, Ingo Molnar wrote:
>
> * H. Peter Anvin <[email protected]> wrote:
>
> > On 01/22/2010 09:40 AM, Borislav Petkov wrote:
> > >>>
> > >>> Those patches are also good -stable candidates.
> > >>
> > >> Hmmm... I'm not sure I see a strong justification for a late -rc push
> > >> into Linus/stable push for for these... I think you would have to
> > >> explicitly make the case if you want them to be considered as such.
> > >
> > > Well, on the one hand, they fix real bugs in the L3 cache index disable
> > > code and since they're bugfixes, they are eligible late -rc candidates.
> >
> > Bugfixes are *early* -rc candidates. Regression fixes are *late* -rc
> > candidates, at least that seems to be the policy Linus currently implements.
> > -stable seems to use slightly less strict criteria (the whole point is that
> > -final needs to be a stabilization point, backported fixes/drivers can then
> > come onto a stable base) which is why you seem some patches which are
> > "straight to .1".
>
> Yes.

Ok, thanks for the clarification - my only trouble was that the current
code is b0rked as is and those fixes are needed. However, backporting
them at a later point seems much more riskfree and I will do so later.

Thanks.

--
Regards/Gruss,
Boris.

2010-01-23 09:01:49

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes


* Borislav Petkov <[email protected]> wrote:

> On Sat, Jan 23, 2010 at 07:59:53AM +0100, Ingo Molnar wrote:
> >
> > * H. Peter Anvin <[email protected]> wrote:
> >
> > > On 01/22/2010 09:40 AM, Borislav Petkov wrote:
> > > >>>
> > > >>> Those patches are also good -stable candidates.
> > > >>
> > > >> Hmmm... I'm not sure I see a strong justification for a late -rc push
> > > >> into Linus/stable push for for these... I think you would have to
> > > >> explicitly make the case if you want them to be considered as such.
> > > >
> > > > Well, on the one hand, they fix real bugs in the L3 cache index disable
> > > > code and since they're bugfixes, they are eligible late -rc candidates.
> > >
> > > Bugfixes are *early* -rc candidates. Regression fixes are *late* -rc
> > > candidates, at least that seems to be the policy Linus currently implements.
> > > -stable seems to use slightly less strict criteria (the whole point is that
> > > -final needs to be a stabilization point, backported fixes/drivers can then
> > > come onto a stable base) which is why you seem some patches which are
> > > "straight to .1".
> >
> > Yes.
>
> Ok, thanks for the clarification - my only trouble was that the current
> code is b0rked as is and those fixes are needed. However, backporting
> them at a later point seems much more riskfree and I will do so later.
>
> Thanks.

Well, if there's a crasher in there, then a minimal fix to address just that
is preferred for .33 - and that can be tagged for -stable immediately.

Anything more complex (these handful of patches) should go via the usual route
of .34-rc1 and then -stable if it's problem-free.

Ingo

2010-01-23 16:32:34

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

On Sat, Jan 23, 2010 at 10:01:35AM +0100, Ingo Molnar wrote:
>
> * Borislav Petkov <[email protected]> wrote:
>
> > On Sat, Jan 23, 2010 at 07:59:53AM +0100, Ingo Molnar wrote:
> > >
> > > * H. Peter Anvin <[email protected]> wrote:
> > >
> > > > On 01/22/2010 09:40 AM, Borislav Petkov wrote:
> > > > >>>
> > > > >>> Those patches are also good -stable candidates.
> > > > >>
> > > > >> Hmmm... I'm not sure I see a strong justification for a late -rc push
> > > > >> into Linus/stable push for for these... I think you would have to
> > > > >> explicitly make the case if you want them to be considered as such.
> > > > >
> > > > > Well, on the one hand, they fix real bugs in the L3 cache index disable
> > > > > code and since they're bugfixes, they are eligible late -rc candidates.
> > > >
> > > > Bugfixes are *early* -rc candidates. Regression fixes are *late* -rc
> > > > candidates, at least that seems to be the policy Linus currently implements.
> > > > -stable seems to use slightly less strict criteria (the whole point is that
> > > > -final needs to be a stabilization point, backported fixes/drivers can then
> > > > come onto a stable base) which is why you seem some patches which are
> > > > "straight to .1".
> > >
> > > Yes.
> >
> > Ok, thanks for the clarification - my only trouble was that the current
> > code is b0rked as is and those fixes are needed. However, backporting
> > them at a later point seems much more riskfree and I will do so later.
> >
> > Thanks.
>
> Well, if there's a crasher in there, then a minimal fix to address just that
> is preferred for .33 - and that can be tagged for -stable immediately.

As I said earlier, I don't believe we have machines in the wild to
practically support the feature yet so we should be OK without a fix.
Let me verify that next week and get back to you with a minimal fix in
case we have any affected configurations.

Thanks.

--
Regards/Gruss,
Boris.

Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes

Hi Ingo,

On Sat, Jan 23, 2010 at 05:32:23PM +0100, Borislav Petkov wrote:
> > Well, if there's a crasher in there, then a minimal fix to address just that
> > is preferred for .33 - and that can be tagged for -stable immediately.
>
> As I said earlier, I don't believe we have machines in the wild to
> practically support the feature yet so we should be OK without a fix.
> Let me verify that next week and get back to you with a minimal fix in
> case we have any affected configurations.

I could verify that we don't need a minimal fix for .33 for the reason
above so the 5 fixes for .34 merge window should suffice completely.

Thanks.

--
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center

2010-01-27 12:17:57

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -v3 0/5] x86, cacheinfo, amd: L3 Cache Index Disable fixes


* Borislav Petkov <[email protected]> wrote:

> Hi Ingo,
>
> On Sat, Jan 23, 2010 at 05:32:23PM +0100, Borislav Petkov wrote:
> > > Well, if there's a crasher in there, then a minimal fix to address just that
> > > is preferred for .33 - and that can be tagged for -stable immediately.
> >
> > As I said earlier, I don't believe we have machines in the wild to
> > practically support the feature yet so we should be OK without a fix.
> > Let me verify that next week and get back to you with a minimal fix in
> > case we have any affected configurations.
>
> I could verify that we don't need a minimal fix for .33 for the reason above
> so the 5 fixes for .34 merge window should suffice completely.

Great - thanks for confirming that.

Ingo