[PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Yinghai Lu]

[PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used.

reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

 arch/i386/pci/fixup.c |   13 +++++++++++++
 drivers/pci/probe.c   |   11 ++++++++++-
 include/linux/pci.h   |    1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

===================================================================
Index: linux-2.6/arch/i386/pci/fixup.c
===================================================================
--- linux-2.6.orig/arch/i386/pci/fixup.c	2007-09-12 19:00:56.000000000 -0700
+++ linux-2.6/arch/i386/pci/fixup.c	2007-09-12 19:01:32.000000000 -0700
@@ -444,3 +444,16 @@
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SIEMENS, 0x0015,
 			  pci_siemens_interrupt_controller);
+
+/**
+ * Regular PCI devices have 256 bytes, but AMD Family 10h Opteron ext config
+ * have 4096 bytes.  Even if the device is capable, that doesn't mean we can
+ * access it.  Maybe we don't have a way to generate extended config space
+ * accesses.   So check it
+ */
+static void fam10h_pci_cfg_space_size(struct pci_dev *dev)
+{
+        dev->cfg_size = pci_cfg_space_size_ext(dev, 0);
+}
+
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_ANY_ID, fam10h_pci_cfg_space_size);
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c	2007-09-12 19:00:56.000000000 -0700
+++ linux-2.6/drivers/pci/probe.c	2007-09-12 19:01:32.000000000 -0700
@@ -831,11 +831,14 @@
  * reading the dword at 0x100 which must either be 0 or a valid extended
  * capability header.
  */
-int pci_cfg_space_size(struct pci_dev *dev)
+int pci_cfg_space_size_ext(struct pci_dev *dev, unsigned check_exp_pcix)
 {
 	int pos;
 	u32 status;
 
+	if (!check_exp_pcix)
+		goto skip;
+
 	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
 	if (!pos) {
 		pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
@@ -847,6 +850,7 @@
 			goto fail;
 	}
 
+ skip:
 	if (pci_read_config_dword(dev, 256, &status) != PCIBIOS_SUCCESSFUL)
 		goto fail;
 	if (status == 0xffffffff)
@@ -858,6 +862,11 @@
 	return PCI_CFG_SPACE_SIZE;
 }
 
+int pci_cfg_space_size(struct pci_dev *dev)
+{
+	return pci_cfg_space_size_ext(dev, 1);
+}
+
 static void pci_release_bus_bridge_dev(struct device *dev)
 {
 	kfree(dev);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h	2007-09-12 19:00:56.000000000 -0700
+++ linux-2.6/include/linux/pci.h	2007-09-12 19:01:32.000000000 -0700
@@ -626,6 +626,7 @@
 
 void pci_walk_bus(struct pci_bus *top, void (*cb)(struct pci_dev *, void *),
 		  void *userdata);
+int pci_cfg_space_size_ext(struct pci_dev *dev, unsigned check_exp_pcix);
 int pci_cfg_space_size(struct pci_dev *dev);
 unsigned char pci_bus_max_busnr(struct pci_bus* bus);
 

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Andrew Morton]

On Wed, 12 Sep 2007 19:21:43 -0700 Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> +/**
> + * Regular PCI devices have 256 bytes, but AMD Family 10h Opteron ext config
> + * have 4096 bytes.  Even if the device is capable, that doesn't mean we can
> + * access it.  Maybe we don't have a way to generate extended config space
> + * accesses.   So check it
> + */

Please don't use the kerneldoc leadin "/**" for non-kerneldoc comments.

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Andi Kleen]

On Thursday 13 September 2007 04:21, Yinghai Lu wrote:
> [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used.
>
> reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.

I just rejected a similar patch -- this should be done using MMCONFIG

-Andi

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Greg KH]

On Thu, Sep 13, 2007 at 11:47:42AM +0200, Andi Kleen wrote:
> On Thursday 13 September 2007 04:21, Yinghai Lu wrote:
> > [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used.
> >
> > reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.
> 
> I just rejected a similar patch -- this should be done using MMCONFIG

But what about for broken machines without the proper MMCONFIG entries?
They still need a way to get access to this config space, so why should
we not also work properly for them?

Until we can somehow flog all BIOS developers until they fix all of
their bugs, we have to be able to handle things like this :(

thanks,

greg k-h

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Andi Kleen]

On Thursday 13 September 2007 12:47, Greg KH wrote:
> On Thu, Sep 13, 2007 at 11:47:42AM +0200, Andi Kleen wrote:
> > On Thursday 13 September 2007 04:21, Yinghai Lu wrote:
> > > [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is
> > > used.
> > >
> > > reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.
> >
> > I just rejected a similar patch -- this should be done using MMCONFIG
>
> But what about for broken machines without the proper MMCONFIG entries?
> They still need a way to get access to this config space,

If there are any devices that need it. IBS and PCI-E error handling 
do, but they're quite obscure.

Also so far we don't know of any Fam 10h systems without MMCONFIG
entries. Fam10h requires a new BIOS so it's reasonable to assume
that the new BIOSes will do better.

-Andi

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Yinghai Lu]

On 9/13/07, Andi Kleen <ak@suse.de> wrote:
> On Thursday 13 September 2007 12:47, Greg KH wrote:
> > On Thu, Sep 13, 2007 at 11:47:42AM +0200, Andi Kleen wrote:
> > > On Thursday 13 September 2007 04:21, Yinghai Lu wrote:
> > > > [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is
> > > > used.
> > > >
> > > > reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.
> > >
> > > I just rejected a similar patch -- this should be done using MMCONFIG
> >
> > But what about for broken machines without the proper MMCONFIG entries?
> > They still need a way to get access to this config space,
>
> If there are any devices that need it. IBS and PCI-E error handling
> do, but they're quite obscure.
>
> Also so far we don't know of any Fam 10h systems without MMCONFIG
> entries. Fam10h requires a new BIOS so it's reasonable to assume
> that the new BIOSes will do better.

BIOS guys also said that fam 10h need mmconfig via eax accessing, may
need OS do sth, so it is safe to stay with MCFG entry for SB like
mcp55...

but latest kernel already have that workaround to make mmconfig via eax...

YH

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> BIOS guys also said that fam 10h need mmconfig via eax accessing, may
> need OS do sth, so it is safe to stay with MCFG entry for SB like
> mcp55...
> 
> but latest kernel already have that workaround to make mmconfig via eax...
> 

This is actually a good point.  Since the CPU vendor managed to
completely fuck up the operation of MMCONFIG itself on this CPU (it's a
*MEMORY REFERENCE*, guys!), it is actually to be expected and prudent
that BIOS vendors will drop the MCFG entry.  MMCONFIG doesn't actually
work on this CPU for any system software which doesn't already know to
work around this particular piece of severe braindamage;
standards-complicant MMCONFIG isn't supported at all.

	-hpa

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Yinghai Lu]

On 9/13/07, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
> >
> > BIOS guys also said that fam 10h need mmconfig via eax accessing, may
> > need OS do sth, so it is safe to stay with MCFG entry for SB like
> > mcp55...
> >
> > but latest kernel already have that workaround to make mmconfig via eax...
> >
>
> This is actually a good point.  Since the CPU vendor managed to
> completely fuck up the operation of MMCONFIG itself on this CPU (it's a
> *MEMORY REFERENCE*, guys!), it is actually to be expected and prudent
> that BIOS vendors will drop the MCFG entry.  MMCONFIG doesn't actually
> work on this CPU for any system software which doesn't already know to
> work around this particular piece of severe braindamage;
> standards-complicant MMCONFIG isn't supported at all.

if other mmconfig device on x86 is sick of accessing via eax, may need
to provide one fam10h_pci_mmcfg to be assigned to raw_pci_ops instead
and leave pci_mmcfg to default behavior ( *MEMORY REFERENCE*).

YH

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[H. Peter Anvin]

Yinghai Lu wrote:
> On 9/13/07, H. Peter Anvin <hpa@zytor.com> wrote:
>> Yinghai Lu wrote:
>>> BIOS guys also said that fam 10h need mmconfig via eax accessing, may
>>> need OS do sth, so it is safe to stay with MCFG entry for SB like
>>> mcp55...
>>>
>>> but latest kernel already have that workaround to make mmconfig via eax...
>>>
>> This is actually a good point.  Since the CPU vendor managed to
>> completely fuck up the operation of MMCONFIG itself on this CPU (it's a
>> *MEMORY REFERENCE*, guys!), it is actually to be expected and prudent
>> that BIOS vendors will drop the MCFG entry.  MMCONFIG doesn't actually
>> work on this CPU for any system software which doesn't already know to
>> work around this particular piece of severe braindamage;
>> standards-complicant MMCONFIG isn't supported at all.
> 
> if other mmconfig device on x86 is sick of accessing via eax, may need
> to provide one fam10h_pci_mmcfg to be assigned to raw_pci_ops instead
> and leave pci_mmcfg to default behavior ( *MEMORY REFERENCE*).
> 

We already have the workaround.  WE do.  However, you could easily see
why a BIOS vendor would want to leave out the MCFG ACPI info, since fam
10h's mmconfig is broken.

	-hpa

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Andreas Herrmann]

On Thu, Sep 13, 2007 at 10:20:56AM -0700, H. Peter Anvin wrote:
> Yinghai Lu wrote:
> > 
> > BIOS guys also said that fam 10h need mmconfig via eax accessing, may
> > need OS do sth, so it is safe to stay with MCFG entry for SB like
> > mcp55...
> > 
> > but latest kernel already have that workaround to make mmconfig via eax...
> > 
> 
> This is actually a good point.  Since the CPU vendor managed to
> completely fuck up the operation of MMCONFIG itself on this CPU (it's a
> *MEMORY REFERENCE*, guys!), it is actually to be expected and prudent
> that BIOS vendors will drop the MCFG entry.  MMCONFIG doesn't actually
> work on this CPU for any system software which doesn't already know to
> work around this particular piece of severe braindamage;

But wait, isn't it true that Vista is using MCFG?
So, poor BIOS guys what should they do now?
If you have a special problem here why not upgrading your Linux kernel?

> standards-complicant MMCONFIG isn't supported at all.

And yes, it is complicated to follow your argumentation.


Andreas

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[H. Peter Anvin]

Andreas Herrmann wrote:
> On Thu, Sep 13, 2007 at 10:20:56AM -0700, H. Peter Anvin wrote:
>> Yinghai Lu wrote:
>>> BIOS guys also said that fam 10h need mmconfig via eax accessing, may
>>> need OS do sth, so it is safe to stay with MCFG entry for SB like
>>> mcp55...
>>>
>>> but latest kernel already have that workaround to make mmconfig via eax...
>>>
>> This is actually a good point.  Since the CPU vendor managed to
>> completely fuck up the operation of MMCONFIG itself on this CPU (it's a
>> *MEMORY REFERENCE*, guys!), it is actually to be expected and prudent
>> that BIOS vendors will drop the MCFG entry.  MMCONFIG doesn't actually
>> work on this CPU for any system software which doesn't already know to
>> work around this particular piece of severe braindamage;
> 
> But wait, isn't it true that Vista is using MCFG?
> So, poor BIOS guys what should they do now?
> If you have a special problem here why not upgrading your Linux kernel?
> 

I'm not talking about Linux here.  I'm talking about any random system
software (which may or may not be Vista, and may nor may not even be an
OS.)  If they advertise MCFG, then a random OS could try to use it, in
accordance with the spec, without the workaround, with serious
malfunction as a result.

If they don't include MCFG, then the worst thing that can happen is that
the OS doesn't see the extended space.  An OS that has chip-specific
workarounds can be aware that this chip is Special and enable MMCONFIG
and use it anyway.

In RFC-speak:

Since this chip doesn't implement standards-conformant mmconfig, a BIOS
MUST NOT enable MCFG in ACPI.  As a result, OSes that have chip-specific
workarounds, like Linux has, SHOULD detect it and use ad hoc code to
enable it anyway.

	-hpa

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG

[Andreas Herrmann]

Usually one shouldn't reply to such mails. But I cannot resist.
Because partially there was told such nonsense ...
Of course the following is just my personal view.

On 2007-09-13 20:54:41, H. Peter Anvin wrote:
> Andreas Herrmann wrote:
> > On Thu, Sep 13, 2007 at 10:20:56AM -0700, H. Peter Anvin wrote:
> >> Yinghai Lu wrote:
> >>> BIOS guys also said that fam 10h need mmconfig via eax accessing, may
> >>> need OS do sth, so it is safe to stay with MCFG entry for SB like
> >>> mcp55...
> >>>
> >>> but latest kernel already have that workaround to make mmconfig via eax...
> >>>
> >> This is actually a good point.  Since the CPU vendor managed to
> >> completely fuck up the operation of MMCONFIG itself on this CPU (it's a
> >> *MEMORY REFERENCE*, guys!), it is actually to be expected and prudent
> >> that BIOS vendors will drop the MCFG entry.  MMCONFIG doesn't actually
> >> work on this CPU for any system software which doesn't already know to
> >> work around this particular piece of severe braindamage;
> > 
> > But wait, isn't it true that Vista is using MCFG?
> > So, poor BIOS guys what should they do now?
> > If you have a special problem here why not upgrading your Linux kernel?
> > 
>
> I'm not talking about Linux here.  I'm talking about any random system
> software (which may or may not be Vista, and may nor may not even be an
> OS.)

Are you serious? You are worried about Vista compatibility issues for
new hardware? Here on LKML? I thought this is a Linux mailing list.

But if this is your worry:
Aren't you aware that most OSes have included patches in order to
support this new hardware? Making use of the new pstates stuff is one
example. And aren't you aware that even for proprietary OSes there are
updates?

But if that is not functioning for you why don't you just get rid of your
proprietary hoax?

>  If they advertise MCFG, then a random OS could try to use it, in
> accordance with the spec, without the workaround, with serious
> malfunction as a result.

Have you looked into the BKDG of that CPU?
You can download it here
http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/31116.pdf

BTW, in this version it is even recommended to set up MCFG. So why should a
BIOS vendor drop it?

As I have stated in some mail yesterday there are actually 3 ECS access methods
possible. For mmio config space accesses you have the choices that the
translation is done either by CPU/NB or by the chipset.
So if you do not enable MMIO config space access within CPU/NB, the CPU will
not convert the config accesses to config cycles on the link.

Which means that:
- your tiny "random OS" should work as usual
- you have no mmio config access to the northbridge functions.

Said that, it seems obvious that the MMIO configuration requirements
are just needed if the translation is done by the CPU/NB. Which makes
somehow sense, because I think the CPU has to intercept somehow the
MMIO traffic to decide whether config space should be accessed or not.
And maybe the hardware designers had some reason to impose those
requirements?

So here comes one funny part out of that discussion.

It seems that a workaround went into the kernel
(commit  3320ad994afb2c44ad34b3b34c3c5cf0da297331)
for a feature that is not yet switched on by the OS.

Because Yinghai's patch is rejected. Or did I miss something?
Did Andi rejected just the third patch and not the first two? Hm, ...

(And obviously Dean Gaudet must have had a system where MSR 0xc0010058
was accordingly configured  - by the BIOS or himself?)

This shows just one thing. Code in this area needs more review and testing
before it goes upstream. So maybe there is some problem with the
"development process" for x86_64?

> If they don't include MCFG, then the worst thing that can happen is that
> the OS doesn't see the extended space.  An OS that has chip-specific
> workarounds can be aware that this chip is Special and enable MMCONFIG
> and use it anyway.

Why then not add code for this "fam 10h-chip" - to decide it is special
and to set up the MmioConfigBase MSR in such a case?

> In RFC-speak:
> 
> Since this chip doesn't implement standards-conformant mmconfig, a BIOS
> MUST NOT enable MCFG in ACPI.  As a result, OSes that have chip-specific
> workarounds, like Linux has, SHOULD detect it and use ad hoc code to
> enable it anyway.


#1: So if you all think that the new mmconfig stuff (done in the CPU/NB) is a crux,
then make it right and double-check in the OS whether the MmioConfigBase MSR
is set and clear it if you like!
(And I can even imagine that you would send patches to do this ;-(

And your statement is wrong again. If #1 is done.
Then you should still have MCFG set up in the BIOS to tell the OS the config space
address range such that mmconfig can be done via the chipset.


And some final notes:

This stuff was long documented in some preliminary NDA specs. And some kernel
developer(s) had access to it. So if it is such a big deal, why didn't they speak
up earlier?

Why I didn't speak up earlier?
Because I have to admit that I didn't care about that stuff until 2-3 weeks ago.

Because I am not as standards-compliant as you are, I should say: "To err is human".
So I've done my best to verify that config space stuff with real code on real hardware
last week. But as always, it is possible that I am wrong with certain statements.

Do you think it is a good idea to discuss that MMCONFIG topic with
some designers at AMD? I could print out that mail or forward it to them.
Do you think they will be happy to listen to your words if you say that
they are "fucking up" stuff and come up with "braindamaged" ideas?
I appreciate the code you write, but not the communication style you chose.


Regards,

Andreas

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG

[H. Peter Anvin]

Andreas Herrmann wrote:
>>>
>> I'm not talking about Linux here.  I'm talking about any random system
>> software (which may or may not be Vista, and may nor may not even be an
>> OS.)
> 
> Are you serious? You are worried about Vista compatibility issues for
> new hardware? Here on LKML? I thought this is a Linux mailing list.
> 

No, I'm not concerned about Vista.  Read: MAY OR MAY NOT EVEN BE AN OS.
 In fact, non-OS system software, such as bootloaders or diagnostic
systems, are in fact much more difficult in this than OSes.

>>  If they advertise MCFG, then a random OS could try to use it, in
>> accordance with the spec, without the workaround, with serious
>> malfunction as a result.
> 
> Have you looked into the BKDG of that CPU?
> You can download it here
> http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/31116.pdf
> 
> BTW, in this version it is even recommended to set up MCFG. So why should a
> BIOS vendor drop it?

They probably won't, which is highly unfortunate.  AMD is unlikely to
admit that they have a serious bug in their compliance, and BIOS vendors
are unlikely to catch on.  It's still broken.

> Because I am not as standards-compliant as you are, I should say: "To err is human".
> So I've done my best to verify that config space stuff with real code on real hardware
> last week. But as always, it is possible that I am wrong with certain statements.

To err is human (and, in silicon manufacturing, both common and
extremely expensive.)  It's now you deal with the fallout that matters,
and it's quite unforgiving, because we're dealing with computer
protocols here.  You MUST NOT advertise compliance with a protocol that
you don't actually have (MCFG).  You then SHOULD promulgate workarounds
that say "oh, I'm on chip X, that means that I can enable mmconfig as
long as I play within these chip-specific rules."

> Do you think it is a good idea to discuss that MMCONFIG topic with
> some designers at AMD? I could print out that mail or forward it to them.
> Do you think they will be happy to listen to your words if you say that
> they are "fucking up" stuff and come up with "braindamaged" ideas?
> I appreciate the code you write, but not the communication style you chose.

Uhm, it's hardware.  Bugs happen.  What gets ugly is when marketing gets
involved, and says "we can't afford to say we don't have <feature X>, so
let's advertise it anyway."

	-hpa

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG

[Yinghai Lu]

On 9/14/07, Andreas Herrmann <aherrman@arcor.de> wrote:
>
> This stuff was long documented in some preliminary NDA specs. And some kernel
> developer(s) had access to it. So if it is such a big deal, why didn't they speak
> up earlier?

I did ask about that when I was porting linuxbios for it. they said
that it is need for ...

YH

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG

[Yinghai Lu]

On 9/14/07, Andreas Herrmann <aherrman@arcor.de> wrote:
> It seems that a workaround went into the kernel
> (commit  3320ad994afb2c44ad34b3b34c3c5cf0da297331)
> for a feature that is not yet switched on by the OS.
>
> Because Yinghai's patch is rejected. Or did I miss something?
> Did Andi rejected just the third patch and not the first two? Hm, ...
>
> (And obviously Dean Gaudet must have had a system where MSR 0xc0010058
> was accordingly configured  - by the BIOS or himself?)
>

the three patches are independent...

[PATCH] x86_64: check MSR to get MMCONFIG for AMD Family 10h Opteron
http://lkml.org/lkml/2007/9/12/247
[PATCH] x86_64: check and enable MMCONFIG for AMD Family 10h Opteron
http://lkml.org/lkml/2007/9/12/248
[PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used
http://lkml.org/lkml/2007/9/12/352

the first two is doing as the way as hpa think that we should do.
the third one: ak doesn't like it, but greg said it is needed for
broken MCFG....

and andrew already put them into -mm.

one side fact is, I could use mmconfig when acpi=off too.

YH

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Andi Kleen]

On Thu, Sep 13, 2007 at 10:20:56AM -0700, H. Peter Anvin wrote:
> Yinghai Lu wrote:
> > 
> > BIOS guys also said that fam 10h need mmconfig via eax accessing, may
> > need OS do sth, so it is safe to stay with MCFG entry for SB like
> > mcp55...
> > 
> > but latest kernel already have that workaround to make mmconfig via eax...
> > 
> 
> This is actually a good point.  Since the CPU vendor managed to
> completely fuck up the operation of MMCONFIG itself on this CPU (it's a
> *MEMORY REFERENCE*, guys!), it is actually to be expected and prudent

It's quite possible that Windows MCFG code always ends up using EAX. 
In fact many older Linux kernels ended up doing this by chance too.

-Andi

Re: [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Andreas Herrmann]

On Thu, Sep 13, 2007 at 01:53:15PM +0200, Andi Kleen wrote:
> On Thursday 13 September 2007 12:47, Greg KH wrote:
> > On Thu, Sep 13, 2007 at 11:47:42AM +0200, Andi Kleen wrote:
> > > On Thursday 13 September 2007 04:21, Yinghai Lu wrote:
> > > > [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is
> > > > used.
> > > >
> > > > reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.
> > >
> > > I just rejected a similar patch -- this should be done using MMCONFIG
> >
> > But what about for broken machines without the proper MMCONFIG entries?
> > They still need a way to get access to this config space,
> 
> If there are any devices that need it. IBS and PCI-E error handling 
> do, but they're quite obscure.
> 
> Also so far we don't know of any Fam 10h systems without MMCONFIG
> entries. Fam10h requires a new BIOS so it's reasonable to assume
> that the new BIOSes will do better.

IMHO support for all config space access methods should be added to the kernel.
And it should be added at a central point. Not waiting for drivers to do it
their own way if they need to use it.

The more complicated/important question is how to decide which access method
should be used for a device. (Something similar to the pci_mmcfg_fallback_slots
stuff that exists already for K8 NB.)
But that needs some more thinking.

Here a summary wrt family 10h extended config space access methods.
(Most of this stuff I verified with some patched kernels. I didn't
eavesdropping on the physical links though ...)

For family 10h we have 3 methods
(1) "legacy" MMCONFIG access (via south bridge)
(2) mmconfig access via CPU/NB (new with fam10h, using the new MMIO
    config base MSR)
(3) CF8/CFC ECS access (new with fam10h, has to be enabled in the NB_CFG MSR)

- If (1) is used  and (2)+(3) are not configurred there is no way to access
  NB config space with mmconfig accesses. Which implies that NB ECS cannot
  be accessed.
  So you can only access NB standard config space via "legacy" CF8/CFC.
  This equals what we have with K8.
  => ECS is accessible just for external devices (not the NB ECS).

- If (2) is used, mmconfig accesses are translated by CPU/NB. So this will
  result in (extended) config cycles on the link to the south bridge.
  This works for both NB config space and config space for normal PCI(e) devices.
  => ECS is accessible for all devices via that method.

- And finally with (3) you can also access both NB config space and the config
  space of external devices. The CPU/NB does the translation and will also
  create (extended) config cycles here.
  => ECS is accessible for all devices via that method.
  (But of course not in the same "atomic" manner like with mmconfig.)

Then we have different prereqs for those methods:
- For (1) the ACPI stuff is required.
- For (2) and (3) some MSRs have to be set up.

Ah, yes in case I didn't mention that: ;-)
Using (2) you haven't such a strict dependency on that ACPI stuff.
Would be cool to disable MCFG at all and configure mmconfig just in
the CPU, no?


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] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is used

[Yinghai Lu]

Andreas Herrmann wrote:
> On Thu, Sep 13, 2007 at 01:53:15PM +0200, Andi Kleen wrote:
>> On Thursday 13 September 2007 12:47, Greg KH wrote:
>>> On Thu, Sep 13, 2007 at 11:47:42AM +0200, Andi Kleen wrote:
>>>> On Thursday 13 September 2007 04:21, Yinghai Lu wrote:
>>>>> [PATCH] x86_64: set cfg_size for AMD Family 10h in case MMCONFIG is
>>>>> used.
>>>>>
>>>>> reuse pci_cfg_space_size but skip check pci express and pci-x CAP ID.
>>>> I just rejected a similar patch -- this should be done using MMCONFIG
>>> But what about for broken machines without the proper MMCONFIG entries?
>>> They still need a way to get access to this config space,
>> If there are any devices that need it. IBS and PCI-E error handling 
>> do, but they're quite obscure.
>>
>> Also so far we don't know of any Fam 10h systems without MMCONFIG
>> entries. Fam10h requires a new BIOS so it's reasonable to assume
>> that the new BIOSes will do better.
> 
> IMHO support for all config space access methods should be added to the kernel.
> And it should be added at a central point. Not waiting for drivers to do it
> their own way if they need to use it.
> 
> The more complicated/important question is how to decide which access method
> should be used for a device. (Something similar to the pci_mmcfg_fallback_slots
> stuff that exists already for K8 NB.)
> But that needs some more thinking.
> 
> Here a summary wrt family 10h extended config space access methods.
> (Most of this stuff I verified with some patched kernels. I didn't
> eavesdropping on the physical links though ...)
> 
> For family 10h we have 3 methods
> (1) "legacy" MMCONFIG access (via south bridge)
> (2) mmconfig access via CPU/NB (new with fam10h, using the new MMIO
>     config base MSR)
> (3) CF8/CFC ECS access (new with fam10h, has to be enabled in the NB_CFG MSR)

ECS access in NB_CFG_MSR is always enabled. because BIOS need to use that to do mem initialization.

mmconfig enablement in BIOS is depending ....

YH