Re: [PATCH -v2 0/5] AMD L3 cache index disable fixes for .35

[Borislav Petkov <bp@amd64.org>]

From: "H. Peter Anvin" <hpa@zytor.com>
Date: Fri, Apr 23, 2010 at 11:06:15AM -0700

> On 04/23/2010 07:09 AM, Borislav Petkov wrote:
> > 
> > Ok, looking at the k8.o object it is 507 bytes so I don't think
> > compiling it in would hurt embedded people too much:
> > 
> >    text    data     bss     dec     hex filename
> >     379     104      24     507     1fb arch/x86/kernel/k8.o
> > 
> > So how about the following? It should apply cleanly on top and it
> > survived a bunch of randconfigs here so far.
> > 
> 
> I have to say I think that's pretty ridiculous for someone who cares so
> much about size that they have disabled CONFIG_PCI that they can just
> add another half-kilobyte of code that is going to do absolutely
> nothing.  Think about the kind of x86 CPUs that could even consider
> disabling CONFIG_PCI -- we're talking pretty deep embedded by now.

Yep, true story.

> So, no, I don't think this is an option.  Force-enabling CONFIG_PCI on
> x86 would be a more realistic option, and I honestly don't know how many
> people would object to that, but not right now.

Well, after looking at 1ac97018169c5a13feaa90d9671f2d6ba2d9e86e,
grepping for '!PCI' in arch/x86/ returns nothing now.

>From all the x86 flavors which didn't need PCI

1) There used to be a X86_VOYAGER MCA-based 32-bit arch from NCR which
seems to be gone now

2) X86_VISWS "SGI 320/540 (Visual Workstation)" depended on !PCI but
depends on PCI now, so which is it?

3) X86_VSMP ("ScaleMP vSMP") used to depend on !PCI but depends on PCI
now, [/me puzzled].

all seem fine with PCI currently (especially the Voyager :)). So it
really looks like we could enable it by default on x86. Maybe for the
.35 merge window and see how much fallout we generate. Or at least put
it up for a flamewar on lkml since we like those so much :).

> The obvious answer instead is to augment the list of stubs in
> <asm/k8.h>.  In particular, move num_k8_northbridges into the #ifdef and
> just #define num_k8_northbridges 0 in the other clause.

Right, it couldn't be simpler, see below. With it, the only warning I
get doing randconfig builds is

arch/x86/kernel/cpu/intel_cacheinfo.c:498: warning: ‘cache_disable_0’ defined but not used
arch/x86/kernel/cpu/intel_cacheinfo.c:500: warning: ‘cache_disable_1’ defined but not used

which is caused by !CONFIG_SYSFS but this is another senseless case.

---
From: Borislav Petkov <borislav.petkov@amd.com>
Date: Sat, 24 Apr 2010 09:56:53 +0200
Subject: [PATCH] x86, k8-nb: Fix build error when K8_NB is disabled

K8_NB depends on PCI and when the last is disabled (allnoconfig) we fail
at the final linking stage due to missing exported num_k8_northbridges.
Add a header stub for that.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/include/asm/k8.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/k8.h b/arch/x86/include/asm/k8.h
index f70e600..af00bd1 100644
--- a/arch/x86/include/asm/k8.h
+++ b/arch/x86/include/asm/k8.h
@@ -16,11 +16,16 @@ extern int k8_numa_init(unsigned long start_pfn, unsigned long end_pfn);
 extern int k8_scan_nodes(void);
 
 #ifdef CONFIG_K8_NB
+extern int num_k8_northbridges;
+
 static inline struct pci_dev *node_to_k8_nb_misc(int node)
 {
 	return (node < num_k8_northbridges) ? k8_northbridges[node] : NULL;
 }
+
 #else
+#define num_k8_northbridges 0
+
 static inline struct pci_dev *node_to_k8_nb_misc(int node)
 {
 	return NULL;
-- 
1.6.4.4


-- 
Regards/Gruss,
Boris.

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