Re: [patch 1/5] x86: Add AMD64 Barcelona PMU MSR definitions

Re: [patch 1/5] x86: Add AMD64 Barcelona PMU MSR definitions

[Andi Kleen]

On Thursday 30 August 2007 19:43:12 Robert Richter wrote:
> Already added to the -mm tree.  Its filename is
>      i386-add-amd64-barcelona-pmu-msr-definitions.patch


Patch doesn't apply to -rc5 + ff tree.

Hunk #1 FAILED at 73.
Hunk #2 FAILED at 107.
2 out of 2 hunks FAILED -- rejects in file include/asm-i386/msr-index.h

If you want to do random white space changes and
code movement please do it in a separate patch in the future.

-Andi

[patch 0/5] (resent) x86: PCI extended config space access on AMD Barcelona CPUs

[Robert Richter]

(resent due to mail server issues)

Hello,

the following series of patches adds support for PCI extended
configuration space access of AMD's Barcelona CPUs (family 10h). It
modifies the CF8/CFC IO register access method and sets the size of
the CPU's PCI devices to 4096 bytes.

Regards,
Robert

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

[patch 1/5] x86: Add AMD64 Barcelona PMU MSR definitions

[Robert Richter]

[-- Attachment #1: i386-add-amd64-barcelona-pmu-msr-definitions.patch --]
[-- Type: text/plain, Size: 2622 bytes --]

Already added to the -mm tree.  Its filename is
     i386-add-amd64-barcelona-pmu-msr-definitions.patch

Signed-off-by: Robert Richter <robert.richter@amd.com>

---
 include/asm-i386/msr-index.h |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

Index: linux-2.6/include/asm-i386/msr-index.h
===================================================================
--- linux-2.6.orig/include/asm-i386/msr-index.h
+++ linux-2.6/include/asm-i386/msr-index.h
@@ -73,8 +73,32 @@
 #define MSR_P6_EVNTSEL0			0x00000186
 #define MSR_P6_EVNTSEL1			0x00000187
 
-/* K7/K8 MSRs. Not complete. See the architecture manual for a more
+/* AMD64 MSRs. Not complete. See the architecture manual for a more
    complete list. */
+
+#define MSR_AMD64_IBSFETCHCTL		0xc0011030
+#define MSR_AMD64_IBSFETCHLINAD		0xc0011031
+#define MSR_AMD64_IBSFETCHPHYSAD	0xc0011032
+#define MSR_AMD64_IBSOPCTL		0xc0011033
+#define MSR_AMD64_IBSOPRIP		0xc0011034
+#define MSR_AMD64_IBSOPDATA		0xc0011035
+#define MSR_AMD64_IBSOPDATA2		0xc0011036
+#define MSR_AMD64_IBSOPDATA3		0xc0011037
+#define MSR_AMD64_IBSDCLINAD		0xc0011038
+#define MSR_AMD64_IBSDCPHYSAD		0xc0011039
+#define MSR_AMD64_IBSCTL		0xc001103a
+
+/* K8 MSRs */
+#define MSR_K8_TOP_MEM1			0xc001001a
+#define MSR_K8_TOP_MEM2			0xc001001d
+#define MSR_K8_SYSCFG			0xc0010010
+#define MSR_K8_HWCR			0xc0010015
+#define MSR_K8_ENABLE_C1E		0xc0010055
+#define K8_MTRRFIXRANGE_DRAM_ENABLE	0x00040000 /* MtrrFixDramEn bit    */
+#define K8_MTRRFIXRANGE_DRAM_MODIFY	0x00080000 /* MtrrFixDramModEn bit */
+#define K8_MTRR_RDMEM_WRMEM_MASK	0x18181818 /* Mask: RdMem|WrMem    */
+
+/* K7 MSRs */
 #define MSR_K7_EVNTSEL0			0xc0010000
 #define MSR_K7_PERFCTR0			0xc0010004
 #define MSR_K7_EVNTSEL1			0xc0010001
@@ -83,20 +107,10 @@
 #define MSR_K7_PERFCTR2			0xc0010006
 #define MSR_K7_EVNTSEL3			0xc0010003
 #define MSR_K7_PERFCTR3			0xc0010007
-#define MSR_K8_TOP_MEM1			0xc001001a
 #define MSR_K7_CLK_CTL			0xc001001b
-#define MSR_K8_TOP_MEM2			0xc001001d
-#define MSR_K8_SYSCFG			0xc0010010
-
-#define K8_MTRRFIXRANGE_DRAM_ENABLE	0x00040000 /* MtrrFixDramEn bit    */
-#define K8_MTRRFIXRANGE_DRAM_MODIFY	0x00080000 /* MtrrFixDramModEn bit */
-#define K8_MTRR_RDMEM_WRMEM_MASK	0x18181818 /* Mask: RdMem|WrMem    */
-
 #define MSR_K7_HWCR			0xc0010015
-#define MSR_K8_HWCR			0xc0010015
 #define MSR_K7_FID_VID_CTL		0xc0010041
 #define MSR_K7_FID_VID_STATUS		0xc0010042
-#define MSR_K8_ENABLE_C1E		0xc0010055
 
 /* K6 MSRs */
 #define MSR_K6_EFER			0xc0000080

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

[patch 2/5] x86: Add AMD64 Barcelona NB cfg MSR definitions

[Robert Richter]

[-- Attachment #1: i386-add-amd64-barcelona-nb-cfg-msr-definitions.patch --]
[-- Type: text/plain, Size: 821 bytes --]

This patch adds MSR definitions for the northbridge configuration
register for AMD's Barcelona CPUs.

Signed-off-by: Robert Richter <robert.richter@amd.com>

---
 include/asm-i386/msr-index.h |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/include/asm-i386/msr-index.h
===================================================================
--- linux-2.6.orig/include/asm-i386/msr-index.h
+++ linux-2.6/include/asm-i386/msr-index.h
@@ -76,6 +76,7 @@
 /* AMD64 MSRs. Not complete. See the architecture manual for a more
    complete list. */
 
+#define MSR_AMD64_NB_CFG		0xc001001f
 #define MSR_AMD64_IBSFETCHCTL		0xc0011030
 #define MSR_AMD64_IBSFETCHLINAD		0xc0011031
 #define MSR_AMD64_IBSFETCHPHYSAD	0xc0011032

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

[patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Robert Richter]

[-- Attachment #1: fam10h-pci-ext-cfg-io.diff --]
[-- Type: text/plain, Size: 7415 bytes --]

This patch implements PCI extended configuration space access for
AMD's Barcelona CPUs. It extends the method using CF8/CFC IO
addresses. An x86 capability bit has been introduced that is set for
CPUs supporting PCI extended config space accesses.

Signed-off-by: Robert Richter <robert.richter@amd.com>

---
 arch/i386/kernel/cpu/amd.c    |   11 +++++-
 arch/i386/pci/direct.c        |   67 +++++++++++++++++++++++++++++++++++-------
 arch/x86_64/kernel/setup.c    |   12 ++++++-
 include/asm-i386/cpufeature.h |    2 +
 4 files changed, 77 insertions(+), 15 deletions(-)

Index: linux-2.6/arch/i386/pci/direct.c
===================================================================
--- linux-2.6.orig/arch/i386/pci/direct.c
+++ linux-2.6/arch/i386/pci/direct.c
@@ -8,18 +8,22 @@
 #include "pci.h"
 
 /*
- * Functions for accessing PCI configuration space with type 1 accesses
+ * Functions for accessing PCI base (first 256 bytes) and extended
+ * (4096 bytes per PCI function) configuration space with type 1
+ * accesses.
  */
 
 #define PCI_CONF1_ADDRESS(bus, devfn, reg) \
-	(0x80000000 | (bus << 16) | (devfn << 8) | (reg & ~3))
+	(0x80000000 | ((reg & 0xF00) << 16) | (bus << 16) \
+	| (devfn << 8) | (reg & 0xFC))
 
-int pci_conf1_read(unsigned int seg, unsigned int bus,
-			  unsigned int devfn, int reg, int len, u32 *value)
+static inline int
+_pci_conf1ext_read(unsigned int seg, unsigned int bus,
+		   unsigned int devfn, int reg, int len, u32 *value)
 {
 	unsigned long flags;
 
-	if ((bus > 255) || (devfn > 255) || (reg > 255)) {
+	if ((bus > 255) || (devfn > 255) || (reg > 4095)) {
 		*value = -1;
 		return -EINVAL;
 	}
@@ -45,12 +49,29 @@ int pci_conf1_read(unsigned int seg, uns
 	return 0;
 }
 
-int pci_conf1_write(unsigned int seg, unsigned int bus,
-			   unsigned int devfn, int reg, int len, u32 value)
+int pci_conf1ext_read(unsigned int seg, unsigned int bus,
+		      unsigned int devfn, int reg, int len, u32 *value)
+{
+	return _pci_conf1ext_read(seg, bus, devfn, reg, len, value);
+}
+
+int pci_conf1_read(unsigned int seg, unsigned int bus,
+			  unsigned int devfn, int reg, int len, u32 *value)
+{
+	if (reg > 255) {
+		*value = -1;
+		return -EINVAL;
+	}
+	return _pci_conf1ext_read(seg, bus, devfn, reg, len, value);
+}
+
+static inline int
+_pci_conf1ext_write(unsigned int seg, unsigned int bus,
+		    unsigned int devfn, int reg, int len, u32 value)
 {
 	unsigned long flags;
 
-	if ((bus > 255) || (devfn > 255) || (reg > 255)) 
+	if ((bus > 255) || (devfn > 255) || (reg > 4095))
 		return -EINVAL;
 
 	spin_lock_irqsave(&pci_config_lock, flags);
@@ -74,6 +95,20 @@ int pci_conf1_write(unsigned int seg, un
 	return 0;
 }
 
+int pci_conf1ext_write(unsigned int seg, unsigned int bus,
+		       unsigned int devfn, int reg, int len, u32 value)
+{
+	return _pci_conf1ext_write(seg, bus, devfn, reg, len, value);
+}
+
+int pci_conf1_write(unsigned int seg, unsigned int bus,
+		 unsigned int devfn, int reg, int len, u32 value)
+{
+	if (reg > 255)
+		return -EINVAL;
+	return _pci_conf1ext_write(seg, bus, devfn, reg, len, value);
+}
+
 #undef PCI_CONF1_ADDRESS
 
 struct pci_raw_ops pci_direct_conf1 = {
@@ -81,6 +116,11 @@ struct pci_raw_ops pci_direct_conf1 = {
 	.write =	pci_conf1_write,
 };
 
+struct pci_raw_ops pci_direct_conf1ext = {
+	.read =		pci_conf1ext_read,
+	.write =	pci_conf1ext_write,
+};
+
 
 /*
  * Functions for accessing PCI configuration space with type 2 accesses
@@ -259,10 +299,15 @@ void __init pci_direct_init(int type)
 	if (type == 0)
 		return;
 	printk(KERN_INFO "PCI: Using configuration type %d\n", type);
-	if (type == 1)
-		raw_pci_ops = &pci_direct_conf1;
-	else
+	if (type != 1) {
 		raw_pci_ops = &pci_direct_conf2;
+	} else if (cpu_has_pci_ext_cfg) {
+		printk(KERN_INFO
+		       "PCI: Extended configuration space enabled\n");
+		raw_pci_ops = &pci_direct_conf1ext;
+	} else {
+		raw_pci_ops = &pci_direct_conf1;
+	}
 }
 
 int __init pci_direct_probe(void)
Index: linux-2.6/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86_64/kernel/setup.c
+++ linux-2.6/arch/x86_64/kernel/setup.c
@@ -65,6 +65,8 @@
 #include <asm/sections.h>
 #include <asm/dmi.h>
 
+#define ENABLE_CF8_EXT_CFG		(1ULL << 46)
+
 /*
  * Machine setup..
  */
@@ -549,10 +551,9 @@ static void __init amd_detect_cmp(struct
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
 	unsigned level;
-
-#ifdef CONFIG_SMP
 	unsigned long value;
 
+#ifdef CONFIG_SMP
 	/*
 	 * Disable TLB flush filter by setting HWCR.FFDIS on K8
 	 * bit 6 of msr C001_0015
@@ -617,6 +618,13 @@ static void __cpuinit init_amd(struct cp
 	/* Family 10 doesn't support C states in MWAIT so don't use it */
 	if (c->x86 == 0x10 && !force_mwait)
 		clear_bit(X86_FEATURE_MWAIT, &c->x86_capability);
+
+	/* Access to PCI extended config space? */
+	if (c->x86 == 0x10) {
+		rdmsrl(MSR_AMD64_NB_CFG, value);
+		if (value & ENABLE_CF8_EXT_CFG)
+			set_bit(X86_FEATURE_PCI_EXT_CFG, &c->x86_capability);
+	}
 }
 
 static void __cpuinit detect_ht(struct cpuinfo_x86 *c)
Index: linux-2.6/include/asm-i386/cpufeature.h
===================================================================
--- linux-2.6.orig/include/asm-i386/cpufeature.h
+++ linux-2.6/include/asm-i386/cpufeature.h
@@ -82,6 +82,7 @@
 /* 14 free */
 #define X86_FEATURE_SYNC_RDTSC	(3*32+15)  /* RDTSC synchronizes the CPU */
 #define X86_FEATURE_REP_GOOD   (3*32+16) /* rep microcode works well on this CPU */
+#define X86_FEATURE_PCI_EXT_CFG	(3*32+17) /* PCI extended cfg access */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* Streaming SIMD Extensions-3 */
@@ -164,6 +165,7 @@
 #define cpu_has_pebs 		boot_cpu_has(X86_FEATURE_PEBS)
 #define cpu_has_clflush		boot_cpu_has(X86_FEATURE_CLFLSH)
 #define cpu_has_bts 		boot_cpu_has(X86_FEATURE_BTS)
+#define cpu_has_pci_ext_cfg	boot_cpu_has(X86_FEATURE_PCI_EXT_CFG)
 
 #endif /* __ASM_I386_CPUFEATURE_H */
 
Index: linux-2.6/arch/i386/kernel/cpu/amd.c
===================================================================
--- linux-2.6.orig/arch/i386/kernel/cpu/amd.c
+++ linux-2.6/arch/i386/kernel/cpu/amd.c
@@ -32,6 +32,7 @@ __asm__(".align 4\nvide: ret");
 #define CPUID_XFAM_11H          0x00200000
 #define CPUID_XMOD              0x000f0000
 #define CPUID_XMOD_REV_F        0x00040000
+#define ENABLE_CF8_EXT_CFG      (1ULL << 46)
 
 /* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
 static __cpuinit int amd_apic_timer_broken(void)
@@ -63,10 +64,9 @@ static void __cpuinit init_amd(struct cp
 	u32 l, h;
 	int mbytes = num_physpages >> (20-PAGE_SHIFT);
 	int r;
-
-#ifdef CONFIG_SMP
 	unsigned long long value;
 
+#ifdef CONFIG_SMP
 	/* Disable TLB flush filter by setting HWCR.FFDIS on K8
 	 * bit 6 of msr C001_0015
 	 *
@@ -296,6 +296,13 @@ static void __cpuinit init_amd(struct cp
 	/* K6s reports MCEs but don't actually have all the MSRs */
 	if (c->x86 < 6)
 		clear_bit(X86_FEATURE_MCE, c->x86_capability);
+
+	/* Access to PCI extended config space? */
+	if (c->x86 == 0x10) {
+		rdmsrl(MSR_AMD64_NB_CFG, value);
+		if (value & ENABLE_CF8_EXT_CFG)
+			set_bit(X86_FEATURE_PCI_EXT_CFG, c->x86_capability);
+	}
 }
 
 static unsigned int __cpuinit amd_size_cache(struct cpuinfo_x86 * c, unsigned int size)

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

[patch 4/5] x86: Add PCI IDs for AMD Barcelona PCI devices

[Robert Richter]

[-- Attachment #1: fam10h-pci-ids.diff --]
[-- Type: text/plain, Size: 936 bytes --]

Signed-off-by: Robert Richter <robert.richter@amd.com>

---
 include/linux/pci_ids.h |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6/include/linux/pci_ids.h
===================================================================
--- linux-2.6.orig/include/linux/pci_ids.h
+++ linux-2.6/include/linux/pci_ids.h
@@ -501,6 +501,11 @@
 #define PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP	0x1101
 #define PCI_DEVICE_ID_AMD_K8_NB_MEMCTL	0x1102
 #define PCI_DEVICE_ID_AMD_K8_NB_MISC	0x1103
+#define PCI_DEVICE_ID_AMD_FAM10H_HT	0x1200
+#define PCI_DEVICE_ID_AMD_FAM10H_MAP	0x1201
+#define PCI_DEVICE_ID_AMD_FAM10H_DRAM	0x1202
+#define PCI_DEVICE_ID_AMD_FAM10H_MISC	0x1203
+#define PCI_DEVICE_ID_AMD_FAM10H_LINK	0x1204
 #define PCI_DEVICE_ID_AMD_LANCE		0x2000
 #define PCI_DEVICE_ID_AMD_LANCE_HOME	0x2001
 #define PCI_DEVICE_ID_AMD_SCSI		0x2020

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

[patch 5/5] x86: Set PCI config space size to extended for AMD Barcelona

[Robert Richter]

[-- Attachment #1: fam10h-pci-ext-cfg-dev-fixup.diff --]
[-- Type: text/plain, Size: 1526 bytes --]

This patch sets the config space size for AMD Barcelona PCI devices to
4096.

Signed-off-by: Robert Richter <robert.richter@amd.com>

---
 arch/i386/pci/fixup.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

Index: linux-2.6/arch/i386/pci/fixup.c
===================================================================
--- linux-2.6.orig/arch/i386/pci/fixup.c
+++ linux-2.6/arch/i386/pci/fixup.c
@@ -8,6 +8,7 @@
 #include <linux/init.h>
 #include "pci.h"
 
+#define PCI_CFG_SPACE_EXP_SIZE	4096
 
 static void __devinit pci_fixup_i450nx(struct pci_dev *d)
 {
@@ -444,3 +445,16 @@ static void __devinit pci_siemens_interr
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SIEMENS, 0x0015,
 			  pci_siemens_interrupt_controller);
+
+/*
+ * Extend size of PCI configuration space for AMD CPUs
+ */
+static void __devinit pci_ext_cfg_space_access(struct pci_dev *dev)
+{
+	dev->cfg_size = PCI_CFG_SPACE_EXP_SIZE;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_HT,   pci_ext_cfg_space_access);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_MAP,  pci_ext_cfg_space_access);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_DRAM, pci_ext_cfg_space_access);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_MISC, pci_ext_cfg_space_access);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_LINK, pci_ext_cfg_space_access);

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

Re: [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Arjan van de Ven]

On Mon, 03 Sep 2007 10:17:39 +0200
"Robert Richter" <robert.richter@amd.com> wrote:

> This patch implements PCI extended configuration space access for
> AMD's Barcelona CPUs. It extends the method using CF8/CFC IO
> addresses. An x86 capability bit has been introduced that is set for
> CPUs supporting PCI extended config space accesses.
> 


No offence but this feels a bit wrong to me.

PCI is sort of more a chipset property than a cpu property (I realize
that this boundary is changing of course).

I'd like to ask you to at least rename some of the feature bits to
indicate that the extended config space is for the IO access method;
after all Linux already supports the MMIO method for accessing extended
config space since a really long time; not marking the feature bit to
indicate it's the IO method is going to be extremely confusing and
cause bugs I bet.

(we probably need a global function that drivers can use to find out of
extended config space is accessible; however that for sure isn't a CPU
capability bit. However the current naming etc sort of makes me fear
drivers will abuse this thing while thinking it's the right API)

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Andreas Herrmann]

On Mon, Sep 03, 2007 at 01:31:57AM -0700, Arjan van de Ven wrote:
> On Mon, 03 Sep 2007 10:17:39 +0200
> "Robert Richter" <robert.richter@amd.com> wrote:
> 
> > This patch implements PCI extended configuration space access for
> > AMD's Barcelona CPUs. It extends the method using CF8/CFC IO
> > addresses. An x86 capability bit has been introduced that is set for
> > CPUs supporting PCI extended config space accesses.
> > 
> 
> 
> No offence but this feels a bit wrong to me.
> 
> PCI is sort of more a chipset property than a cpu property (I realize
> that this boundary is changing of course).
> 
> I'd like to ask you to at least rename some of the feature bits to
> indicate that the extended config space is for the IO access method;
> after all Linux already supports the MMIO method for accessing extended
> config space since a really long time; not marking the feature bit to
> indicate it's the IO method is going to be extremely confusing and
> cause bugs I bet.

Hmm, yes the naming  of the CPU capability bit seems wrong.
Guess, Robert will fix it.

> (we probably need a global function that drivers can use to find out of
> extended config space is accessible; however that for sure isn't a CPU
> capability bit.

IMHO this is already available. Just check pci_dev->cfg_size which
is 256 if PCI ECS access is not possible (see pci_cfg_space_size()).

> However the current naming etc sort of makes me fear
> drivers will abuse this thing while thinking it's the right API)

Do you see any other issues besides the naming of the bit?


Regards,

Andreas

-- 
Operating | AMD Saxony Limited Liability Company & Co. KG,
  System  | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 Research | Register Court Dresden: HRA 4896, General Partner authorized
  Center  | to represent: AMD Saxony LLC (Wilmington, Delaware, US)
  (OSRC)  | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Arjan van de Ven]

On Mon, 3 Sep 2007 11:17:18 +0200
"Andreas Herrmann" <andreas.herrmann3@amd.com> wrote:
\> 
> Do you see any other issues besides the naming of the bit?

I wonder if we should key this off a PCI ID of the chipset rather than
the cpu id... I mean, how sure are you that all via chipsets connected
to the barcelona cpu will deal well?

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Andreas Herrmann]

On Mon, Sep 03, 2007 at 04:33:19AM -0700, Arjan van de Ven wrote:
> On Mon, 3 Sep 2007 11:17:18 +0200
> "Andreas Herrmann" <andreas.herrmann3@amd.com> wrote:
> \> 
> > Do you see any other issues besides the naming of the bit?
> 
> I wonder if we should key this off a PCI ID of the chipset rather than
> the cpu id... I mean, how sure are you that all via chipsets connected
> to the barcelona cpu will deal well?

In general they should be able to deal with those accesses. They result in
extended type 0/1 configuration cycles which are defined already in HT I/O Link
Spec 1.10. So if unexpectedly problems arise then it is time to add a pci-quirk.

But at the moment there is no need for further discussion on this subject
because Andi refuses to add support for Barcelona CF8/CFC ECS access.


Regards,

Andreas

-- 
Operating | AMD Saxony Limited Liability Company & Co. KG,
  System  | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 Research | Register Court Dresden: HRA 4896, General Partner authorized
  Center  | to represent: AMD Saxony LLC (Wilmington, Delaware, US)
  (OSRC)  | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [patch 5/5] x86: Set PCI config space size to extended for AMD Barcelona

[dean gaudet]

it's so very unfortunate the PCI standard has no feature bit to indicate 
the presence of ECS.

FWIW in my testing on a range of machines spanning 7 or 8 years i could 
read config space reg 256... and get 0xffffffff when the device didn't 
support ECS, and get valid data when the device did support ECS... granted 
there may be some system out there which behaves really badly when you do 
this.

perhaps someone could write a userspace program and test that concept on a 
far wider range of machines.

-dean

On Mon, 3 Sep 2007, Robert Richter wrote:

> This patch sets the config space size for AMD Barcelona PCI devices to
> 4096.
> 
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> 
> ---
>  arch/i386/pci/fixup.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> Index: linux-2.6/arch/i386/pci/fixup.c
> ===================================================================
> --- linux-2.6.orig/arch/i386/pci/fixup.c
> +++ linux-2.6/arch/i386/pci/fixup.c
> @@ -8,6 +8,7 @@
>  #include <linux/init.h>
>  #include "pci.h"
>  
> +#define PCI_CFG_SPACE_EXP_SIZE	4096
>  
>  static void __devinit pci_fixup_i450nx(struct pci_dev *d)
>  {
> @@ -444,3 +445,16 @@ static void __devinit pci_siemens_interr
>  }
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SIEMENS, 0x0015,
>  			  pci_siemens_interrupt_controller);
> +
> +/*
> + * Extend size of PCI configuration space for AMD CPUs
> + */
> +static void __devinit pci_ext_cfg_space_access(struct pci_dev *dev)
> +{
> +	dev->cfg_size = PCI_CFG_SPACE_EXP_SIZE;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_HT,   pci_ext_cfg_space_access);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_MAP,  pci_ext_cfg_space_access);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_DRAM, pci_ext_cfg_space_access);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_MISC, pci_ext_cfg_space_access);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FAM10H_LINK, pci_ext_cfg_space_access);
> 
> -- 
> AMD Saxony, Dresden, Germany
> Operating System Research Center
> email: robert.richter@amd.com
> 
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [patch 5/5] x86: Set PCI config space size to extended for AMD Barcelona

[Robert Richter]

On 03.09.07 09:48:15, dean gaudet wrote:
> it's so very unfortunate the PCI standard has no feature bit to indicate 
> the presence of ECS.

Right. Not nice.

> FWIW in my testing on a range of machines spanning 7 or 8 years i could 
> read config space reg 256... and get 0xffffffff when the device didn't 
> support ECS, and get valid data when the device did support ECS... granted 
> there may be some system out there which behaves really badly when you do 
> this.
> 
> perhaps someone could write a userspace program and test that concept on a 
> far wider range of machines.

A user space programm could map the config space (if MMCONFIG is usable)
and scan the PCI devices directly using memory access.

This patch sets pcidev->cfg_size to 4096 for certain devices so that
the ECS is also available using proc/sys fs or lspci -xxxx. In this
case the Kernel access functions are used. This works with both
methods, CF8 and MMCONFIG. For CF8, all patches of these series must
be used.

Note also, that ECS access is not only for CPU devices available, but
also of other PCI Express devices on the bus.

-Robert

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com

Re: [patch 5/5] x86: Set PCI config space size to extended for AMD Barcelona

[Martin Mares]

Hi!

> it's so very unfortunate the PCI standard has no feature bit to indicate 
> the presence of ECS.
> 
> FWIW in my testing on a range of machines spanning 7 or 8 years i could 
> read config space reg 256... and get 0xffffffff when the device didn't 
> support ECS, and get valid data when the device did support ECS... granted 
> there may be some system out there which behaves really badly when you do 
> this.
> 
> perhaps someone could write a userspace program and test that concept on a 
> far wider range of machines.

If you want to experiment with this in user space, it's easy to write
a couple of access functions for the libpci in pciutils. (Patches welcome,
as usually :) )

				Have a nice fortnight
-- 
Martin `MJ' Mares                          <mj@ucw.cz>   http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
God is real, unless declared integer.

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[H. Peter Anvin]

Andreas Herrmann wrote:
> On Mon, Sep 03, 2007 at 04:33:19AM -0700, Arjan van de Ven wrote:
>> On Mon, 3 Sep 2007 11:17:18 +0200
>> "Andreas Herrmann" <andreas.herrmann3@amd.com> wrote:
>> \> 
>>> Do you see any other issues besides the naming of the bit?
>> I wonder if we should key this off a PCI ID of the chipset rather than
>> the cpu id... I mean, how sure are you that all via chipsets connected
>> to the barcelona cpu will deal well?
> 
> In general they should be able to deal with those accesses. They result in
> extended type 0/1 configuration cycles which are defined already in HT I/O Link
> Spec 1.10. So if unexpectedly problems arise then it is time to add a pci-quirk.
> 
> But at the moment there is no need for further discussion on this subject
> because Andi refuses to add support for Barcelona CF8/CFC ECS access.
> 

Well, they don't add any functionality, do they?  As such, I would agree 
with Andi -- we only need one method which can (correctly) access the 
full configuration space, since it'll look the same on the bus anyway.

	-hpa

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Robert Richter]

On 05.09.07 06:58:58, H. Peter Anvin wrote:

> >But at the moment there is no need for further discussion on this subject
> >because Andi refuses to add support for Barcelona CF8/CFC ECS access.
> >
> 
> Well, they don't add any functionality, do they?  As such, I would agree 
> with Andi -- we only need one method which can (correctly) access the 
> full configuration space, since it'll look the same on the bus anyway.

PCI Devices will not be the same on the bus since PCI read/write
functions will have different behavior. Without the patches you will
get an error when accessing ECS with CF8. We need ECS access for
patches that setups local interrupt vectors. This patches will be
released soon.

Btw, this patch fixes also config space access with proc/sys fs and
lspci. I see this as an added functionality as well.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[H. Peter Anvin]

Robert Richter wrote:
> On 05.09.07 06:58:58, H. Peter Anvin wrote:
> 
>>> But at the moment there is no need for further discussion on this subject
>>> because Andi refuses to add support for Barcelona CF8/CFC ECS access.
>>>
>> Well, they don't add any functionality, do they?  As such, I would agree 
>> with Andi -- we only need one method which can (correctly) access the 
>> full configuration space, since it'll look the same on the bus anyway.
> 
> PCI Devices will not be the same on the bus since PCI read/write
> functions will have different behavior. Without the patches you will
> get an error when accessing ECS with CF8. We need ECS access for
> patches that setups local interrupt vectors. This patches will be
> released soon.

You're missing the point.   How will the PCI bus transactions be 
different when using MMCONFIG versus your extended CF8 version?

> Btw, this patch fixes also config space access with proc/sys fs and
> lspci. I see this as an added functionality as well.

The latter implies the former.  Again, how does this differ from MMCONFIG?

	-hpa

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Robert Richter]

On 05.09.07 11:12:00, H. Peter Anvin wrote:
> >PCI Devices will not be the same on the bus since PCI read/write
> >functions will have different behavior. Without the patches you will
> >get an error when accessing ECS with CF8. We need ECS access for
> >patches that setups local interrupt vectors. This patches will be
> >released soon.
> 
> You're missing the point.   How will the PCI bus transactions be 
> different when using MMCONFIG versus your extended CF8 version?

Misunderstood you, with this patch there will be the same behavior,
that's the intention. There might be slightly differences in ordering
rules for read/write cycles. IO config cycles are serialized while
ordering rules for MMIO config cycles may result in unexpected
behavior for PCI devices on the bus.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Arne Georg Gleditsch]

"H. Peter Anvin" <hpa@zytor.com> writes:
> You're missing the point.   How will the PCI bus transactions be
> different when using MMCONFIG versus your extended CF8 version?

Conceivably this is useful if the IO hub does not support MMCONFIG
accesses.  The AMD 8111 does not, as far as I can see.  At least I
have an Opteron system where MMCONFIG is not supported, and I assume
it's because the 8111 doesn't provide it.  I don't know if this is
going to be a realistic scenario for Barcelona systems.

-- 
								Arne.

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Andreas Herrmann]

On Wed, Sep 05, 2007 at 06:58:58AM +0100, H. Peter Anvin wrote:
> Well, they don't add any functionality, do they?

They allow CF8/CFC to access ECS in cases where mmcfg is not working.

> As such, I would agree with Andi -- we only 
> need one method which can (correctly) access the full configuration space,

Right, we need to be able to "correctly access the full config space".

> since it'll look the same on the bus anyway.

Sure, on the bus we should only see pci configuration requests in both
cases.


To summarize it:
- mmcfg needs support by BIOS ("PCI services in ACPI")
- CF8/CFC ECS access does not have that dependency
- For base configuration space access we already have two methods -
  type 1 and mmcfg  (type1 as fallback if there is no mmcfg).
- So what's the benefit in not allowing CF8/CFC ECS  access ("extended type1")
  if the hardware supports it and if mmcfg is not suitable?


One thing that comes out of that fruitless discussion:
For IBS Robert might have to implement CF8/CFC ECS access directly in the IBS
code, or in a new driver for ECS access of NB functions. Just to ensure that
IBS is working if there is no mmcfg. And this is kind of ugly.


Regards,

Andreas

-- 
Operating | AMD Saxony Limited Liability Company & Co. KG,
  System  | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 Research | Register Court Dresden: HRA 4896, General Partner authorized
  Center  | to represent: AMD Saxony LLC (Wilmington, Delaware, US)
  (OSRC)  | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Andreas Herrmann]

On Wed, Sep 05, 2007 at 01:05:25PM +0200, Arne Georg Gleditsch wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> > You're missing the point.   How will the PCI bus transactions be
> > different when using MMCONFIG versus your extended CF8 version?
> 
> Conceivably this is useful if the IO hub does not support MMCONFIG
> accesses.  The AMD 8111 does not, as far as I can see.  At least I
> have an Opteron system where MMCONFIG is not supported, and I assume
> it's because the 8111 doesn't provide it.  I don't know if this is
> going to be a realistic scenario for Barcelona systems.
> 
> -- 
> 								Arne.

Not sure how many AMD 8111 based systems are out there which are
upgradeable to Barcelona.

I guess the BIOS won't setup MCFG for systems with 8111/8131 
because both lack ECS MMIO Base Address Register. 
(Because 8111 and 8131 are not Mode2-PCI-X and PCI-express
capable.)

So if such a board is upgradeable, the way to access extended
config space of the northbridge functions is to use CF8/CFC
ECS access.


Regards,

Andreas

-- 
Operating | AMD Saxony Limited Liability Company & Co. KG,
  System  | Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 Research | Register Court Dresden: HRA 4896, General Partner authorized
  Center  | to represent: AMD Saxony LLC (Wilmington, Delaware, US)
  (OSRC)  | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Yinghai Lu]

On 9/5/07, Andreas Herrmann <andreas.herrmann3@amd.com> wrote:
> On Wed, Sep 05, 2007 at 01:05:25PM +0200, Arne Georg Gleditsch wrote:
> > "H. Peter Anvin" <hpa@zytor.com> writes:
> > > You're missing the point.   How will the PCI bus transactions be
> > > different when using MMCONFIG versus your extended CF8 version?
> >
> > Conceivably this is useful if the IO hub does not support MMCONFIG
> > accesses.  The AMD 8111 does not, as far as I can see.  At least I
> > have an Opteron system where MMCONFIG is not supported, and I assume
> > it's because the 8111 doesn't provide it.  I don't know if this is
> > going to be a realistic scenario for Barcelona systems.

mmconfig is set in NB ( in new CPU), Do we still need to set mmconfig
in SB like mcp55?

YH

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Arne Georg Gleditsch]

"Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> mmconfig is set in NB ( in new CPU), Do we still need to set mmconfig
> in SB like mcp55?

I wasn't aware that the family 10h-chips had MSRs for setting the
mmconfig address space directly in the NB (core?).  Please disregard
my previous comment...

-- 
								Arne.

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[H. Peter Anvin]

Arne Georg Gleditsch wrote:
> "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
>> mmconfig is set in NB ( in new CPU), Do we still need to set mmconfig
>> in SB like mcp55?
> 
> I wasn't aware that the family 10h-chips had MSRs for setting the
> mmconfig address space directly in the NB (core?).  Please disregard
> my previous comment...
> 

Well, to a first order of approximations, *all* northbridges have some 
sort of hardware registers to set mmconfig.  We were talking yesterday 
that it might just make more sense to have code for various northbridges 
to configure mmconfig directly, just like we do for IRQ routing (we 
can't trust the BIOS there, either.)

	-hpa

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Arjan van de Ven]

On Wed, 5 Sep 2007 17:00:27 +0200
"Andreas Herrmann" <andreas.herrmann3@amd.com> wrote:

> On Wed, Sep 05, 2007 at 06:58:58AM +0100, H. Peter Anvin wrote:
> > Well, they don't add any functionality, do they?
> 
> They allow CF8/CFC to access ECS in cases where mmcfg is not working.
> 

just for the record; I have absolutely no problem with allowing the new
cf8 method (I'm assuming that it's obvious that if mmio is available,
that will be used instead for performance reasons). What I was/am
concerned/uncomfortable about is keying this off the CPU level rather
than a PCI level property; I much rather would see this keyed of, say,
the PCI ID of the root bridge, or ideally from some PCI property.

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Jesse Barnes]

On Thursday, September 6, 2007 2:48 am H. Peter Anvin wrote:
> Arne Georg Gleditsch wrote:
> > "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> >> mmconfig is set in NB ( in new CPU), Do we still need to set
> >> mmconfig in SB like mcp55?
> >
> > I wasn't aware that the family 10h-chips had MSRs for setting the
> > mmconfig address space directly in the NB (core?).  Please
> > disregard my previous comment...
>
> Well, to a first order of approximations, *all* northbridges have
> some sort of hardware registers to set mmconfig.  We were talking
> yesterday that it might just make more sense to have code for various
> northbridges to configure mmconfig directly, just like we do for IRQ
> routing (we can't trust the BIOS there, either.)

The problem with doing that is it would mean reserving some address 
space for mmconfig usage.  If the BIOS doesn't completely describe all 
the reserved regions via e820 or similar (apparently a common problem) 
we may end up making mmconfig overlap with another important area...  
It's pretty hard not to trust the BIOS here without completely 
replacing big chunks of it.

Jesse

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Yinghai Lu]

On 9/6/07, H. Peter Anvin <hpa@zytor.com> wrote:
> Well, to a first order of approximations, *all* northbridges have some
> sort of hardware registers to set mmconfig.  We were talking yesterday
> that it might just make more sense to have code for various northbridges
> to configure mmconfig directly, just like we do for IRQ routing (we
> can't trust the BIOS there, either.)

that is good idea.

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Jesse Barnes]

On Thursday, September 6, 2007 10:50 am Yinghai Lu wrote:
> On 9/6/07, Jesse Barnes <jesse.barnes@intel.com> wrote:
> > The problem with doing that is it would mean reserving some address
> > space for mmconfig usage.  If the BIOS doesn't completely describe
> > all the reserved regions via e820 or similar (apparently a common
> > problem) we may end up making mmconfig overlap with another
> > important area... It's pretty hard not to trust the BIOS here
> > without completely replacing big chunks of it.
>
> for Family 10h, you can use mmio range high beyond than the RAM
> range...( just don't conflict with HT reserved range...)

Sure, and on some Intel platforms we can configure things similarly.  
But any such changes will be platform specific.  I'm just not sure 
allocating mmconfig space ourselves is worth the trouble.  Besides, 
most machines made in the last few years can do it, as long as we look 
at the right BIOS bits (i.e. MCFG not e820) and remember to disable PCI 
device decode when probing BAR sizes for example. :)

Jesse

Re: [patches] [patch 3/5] x86: Add PCI extended config space access for AMD Barcelona

[Yinghai Lu]

On 9/6/07, Jesse Barnes <jesse.barnes@intel.com> wrote:
> The problem with doing that is it would mean reserving some address
> space for mmconfig usage.  If the BIOS doesn't completely describe all
> the reserved regions via e820 or similar (apparently a common problem)
> we may end up making mmconfig overlap with another important area...
> It's pretty hard not to trust the BIOS here without completely
> replacing big chunks of it.

for Family 10h, you can use mmio range high beyond than the RAM
range...( just don't conflict with HT reserved range...)

YH