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

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

[Andi Kleen]

On Thursday 30 August 2007 19:43:14 Robert Richter 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.

We shouldn't need this because extended config space should work
here and Linux should use it 
(especially after we added the ugly Barcelona workaround into  that code) 

The only exception would be if the user disables MMCONFIG in CONFIG, but that's
their own fault then.

Ok there might be buggy BIOS around with no or no usable MCFG table, but since
extended config space is not really a critical feature that's not a big issue.

So I don't think we want this special case code at all and should
just rely on MMCONFIG.

-Andi

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

[Andreas Herrmann]

On Sat, Sep 01, 2007 at 12:11:52PM +0200, Andi Kleen wrote:
> On Thursday 30 August 2007 19:43:14 Robert Richter 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.
> 
> We shouldn't need this because extended config space should work
> here and Linux should use it 
> (especially after we added the ugly Barcelona workaround into  that code) 

This patch is needed for all buggy BIOSes that don't correctly set up MCFG.

> The only exception would be if the user disables MMCONFIG in CONFIG, but that's
> their own fault then.

No, as you stated yourself there are two exceptions. And the more serious one is the
BIOS issue.

> Ok there might be buggy BIOS around with no or no usable MCFG table, but since
> extended config space is not really a critical feature that's not a big issue.

Not sure why you assume extended config space (ECS) is not critical.
Ok, for a lot of stuff it does not matter.
But it is needed for some devices for full functionality.
E.g. for perfmon (family 0x10/extended inerrupts) extended config space access
is a prerequisite.

> So I don't think we want this special case code at all and should
> just rely on MMCONFIG.

Rely on something unreliable (due to buggy/incomplete BIOS)?
I don't think we should do this.

IMHO it is best to try to use MMCONFIG if it's working and to use
a fallback (e.g. CF8 ECS access for family 0x10) if available.


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

[Andi Kleen]

> But it is needed for some devices for full functionality.

Examples? I can only think of PCI express error reporting, which
few drivers implement anyways and isn't really a show stopper
if it doesn't work. Besides I would be surprised if it even works
on the cheap desktop boards which have MCFG less BIOS.

> E.g. for perfmon (family 0x10/extended inerrupts) extended config space
> access is a prerequisite.

How so? 


>
> IMHO it is best to try to use MMCONFIG if it's working and to use
> a fallback (e.g. CF8 ECS access for family 0x10) if available.

We only put in workarounds if there is a serious problem otherwise (e.g. not 
booting etc.). I just don't see this here.

-Andi

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

[Robert Richter]

Andi,

On 03.09.07 12:15:03, Andi Kleen wrote:

> > But it is needed for some devices for full functionality.
> 
> Examples? I can only think of PCI express error reporting, which
> few drivers implement anyways and isn't really a show stopper
> if it doesn't work. Besides I would be surprised if it even works
> on the cheap desktop boards which have MCFG less BIOS.

As you say, there are BIOSs that do not support MMCONFIG. Thus, CF8
access is not only a workaround to boot a system. There are systems
that really use CF8 access. Also, MMCONFIG depends on many conditions
that must match. Cfg space must be mapped, the memory must be E820
reserved, MCFG table must be set, access to MMCONFIG must be enabled
for the device. Many things that may fail and can partly not be fixed
by the OS.

> > E.g. for perfmon (family 0x10/extended inerrupts) extended config space
> > access is a prerequisite.
> 
> How so? 

Recent (10h) and upcomming CPU families make heavy use of PCI ext cfg
space for certain CPU features. Setup of the extended interupt local
vector table for IBS (used with Perfmon2) is one example. CPU
designers do not take care anymore if a feature is in the base or
extended config space. So access to PCI ECS is essential.

> > IMHO it is best to try to use MMCONFIG if it's working and to use
> > a fallback (e.g. CF8 ECS access for family 0x10) if available.
> 
> We only put in workarounds if there is a serious problem otherwise (e.g. not 
> booting etc.). I just don't see this here.

As said above, I do not see CF8 access as a workaround. I expect my
system to work in the same way also if MMCONFIG is not available.

-Robert

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

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

[Andi Kleen]

On Monday 03 September 2007 13:27, Robert Richter wrote:

> On 03.09.07 12:15:03, Andi Kleen wrote:
> > > But it is needed for some devices for full functionality.
> >
> > Examples? I can only think of PCI express error reporting, which
> > few drivers implement anyways and isn't really a show stopper
> > if it doesn't work. Besides I would be surprised if it even works
> > on the cheap desktop boards which have MCFG less BIOS.
>
> As you say, there are BIOSs that do not support MMCONFIG. Thus, CF8
> access is not only a workaround to boot a system. 

We're talking about accessing the extended part of config spaces. I'm not 
aware of any case where that is required to boot a system.

> Recent (10h) and upcomming CPU families make heavy use of PCI ext cfg
> space for certain CPU features. Setup of the extended interupt local
> vector table for IBS (used with Perfmon2) is one example. CPU
> designers do not take care anymore if a feature is in the base or
> extended config space. So access to PCI ECS is essential.

I don't think it's a big issue if IBS doesn't work on a few buggy BIOS
(which should hopefully become fewer anyways because Vista is out
which actually uses MCFG) 


> > > IMHO it is best to try to use MMCONFIG if it's working and to use
> > > a fallback (e.g. CF8 ECS access for family 0x10) if available.
> >
> > We only put in workarounds if there is a serious problem otherwise (e.g.
> > not booting etc.). I just don't see this here.
>
> As said above, I do not see CF8 access as a workaround. I expect my
> system to work in the same way also if MMCONFIG is not available.

It should boot sure, but exotic stuff not working is not a major issue

-Andi

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

[Robert Richter]

Andi,

On 03.09.07 14:48:41, Andi Kleen wrote:

> > As said above, I do not see CF8 access as a workaround. I expect my
> > system to work in the same way also if MMCONFIG is not available.
>
> It should boot sure, but exotic stuff not working is not a major issue

It is not only about booting the system without MMCONFIG, it is also
about using the system. For this PCI ECS CF8 access is needed. So why
not adding support for this? Excluding major parts of CPU registers
when using CF8 base access only will cause writing code to workaround
this. Not very nice to handle.

-Robert

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

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

[Andi Kleen]

On Monday 03 September 2007 15:48, Robert Richter wrote:
> Andi,
>
> On 03.09.07 14:48:41, Andi Kleen wrote:
> > > As said above, I do not see CF8 access as a workaround. I expect my
> > > system to work in the same way also if MMCONFIG is not available.
> >
> > It should boot sure, but exotic stuff not working is not a major issue
>
> It is not only about booting the system without MMCONFIG, it is also
> about using the system

There are limits on how many BIOS bug workaround (and all this
patch kit is nothing more than a elaborate BIOS bug workaround) we can do.
IMHO you're far exceeding the threshold.

Better would be to invest that energy to get the BIOSes fixed.

> . For this PCI ECS CF8 access is needed.

AER and IBS are all totally optional features and the systems
will be completely usable without them.

-Andi

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

[Yinghai Lu]

On 9/3/07, Andi Kleen <ak@suse.de> wrote:
> On Monday 03 September 2007 13:27, Robert Richter wrote:
>
> > On 03.09.07 12:15:03, Andi Kleen wrote:
> > > > But it is needed for some devices for full functionality.
> > >
> > > Examples? I can only think of PCI express error reporting, which
> > > few drivers implement anyways and isn't really a show stopper
> > > if it doesn't work. Besides I would be surprised if it even works
> > > on the cheap desktop boards which have MCFG less BIOS.
> >
> > As you say, there are BIOSs that do not support MMCONFIG. Thus, CF8
> > access is not only a workaround to boot a system.
>
> We're talking about accessing the extended part of config spaces. I'm not
> aware of any case where that is required to boot a system.

in the bios for the new cpu, CF8 is used for ext conf space
accessing...(for ht and mem init).

for setting mmio to accessing pci conf space, need to set sth on MSR.
So the BIOS need to make it right..., otherwise OS need to set that
when booting every cpu...to workaround it to make MMCONFIG working.

YH

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

[Andi Kleen]

> for setting mmio to accessing pci conf space, need to set sth on MSR.
> So the BIOS need to make it right..., otherwise OS need to set that
> when booting every cpu...to workaround it to make MMCONFIG working.

Yes the BIOS has to get some things right. After all that's the BIOS job.
And the BIOS knows much more about the hardware than the generic
kernel.  x86 Linux is not trying to be a BIOS too.

-Andi

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

[Andreas Herrmann]

On Mon, Sep 03, 2007 at 12:15:03PM +0200, Andi Kleen wrote:
> 
> > But it is needed for some devices for full functionality.
> 
> Examples? I can only think of PCI express error reporting, which
> few drivers implement anyways and isn't really a show stopper
> if it doesn't work. Besides I would be surprised if it even works
> on the cheap desktop boards which have MCFG less BIOS.

Sure, AER is one example.
I don't see why all other stuff in ECS is not worth reading or writing.
And I am not sure whether all server boards set up MCFG in the correct way.

> > E.g. for perfmon (family 0x10/extended inerrupts) extended config space
> > access is a prerequisite.
> 
> How so? 

E.g. access to the IBS control register needs ECS access on Barcelona.
I guess we have to wait until Robert sends his patches which contain
all details.

> >
> > IMHO it is best to try to use MMCONFIG if it's working and to use
> > a fallback (e.g. CF8 ECS access for family 0x10) if available.
> 
> We only put in workarounds if there is a serious problem otherwise (e.g. not 
> booting etc.). I just don't see this here.

The serious problem is you can't access PCI ECS if the BIOS does
not take care to (correctly) set up MCFG.
And unfortunately this is too often the case.

See for instance Robert Hancock's patch http://lkml.org/lkml/2007/5/30/2
to enable MMCONFIG access in certain cases where BIOS did not correctly
set up MCFG. Why are people working on such stuff if it is not serious
enough?

Barcelona just adds another way to access PCI ECS (besides MMCONFIG) and I
can't understand why this shouldn't be supported by Linux.

Do you have any suggestion how else to add support for PCI ECS access via
IO instructions for Barcelona?


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

[Andi Kleen]

> And unfortunately this is too often the case.

On Barcelona systems? 

> See for instance Robert Hancock's patch http://lkml.org/lkml/2007/5/30/2
> to enable MMCONFIG access in certain cases where BIOS did not correctly
> set up MCFG. Why are people working on such stuff if it is not serious
> enough?

I don't know why. I'm just not aware of any serious problems that
are solved by extended config space access.

Originally we had trouble because MCFG was wrong and the systems
would not boot. Also there is one class of systems (some x86 Apples) 
where cf8 doesn't work, but MMCONFIG does.
But that's all specific to older systems. 

> Barcelona just adds another way to access PCI ECS (besides MMCONFIG) and I
> can't understand why this shouldn't be supported by Linux.

It will be supported when the BIOS gets it right.

> Do you have any suggestion how else to add support for PCI ECS access via
> IO instructions for Barcelona?

My suggestion is to rely on MMCONFIG for this.

-Andi

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