[PATCH] v2: fix PCIe/MMCONF config space access

[Stefan Reinauer]

See patch

[Myles Watson]

It seems like these two snippets should be related in some way.  Do
you have to set them equal?

Index: src/config/Options.lb
===================================================================
--- src/config/Options.lb	(.../branches/upstream/coreboot-v2)	
+++ src/config/Options.lb	(.../trunk/coreboot-v2)	
@@ -983,6 +983,13 @@
    comment "enable mmconfig for pci conf"
 end

+define MMCONF_BASE_ADDRESS
+	default none
+	format "0x%x"
+	export used
+	comment "enable mmconfig base address"
+end
+

from /src/northbridge/intel/i945/northbridge.c:

/* We use 0xcf as an unused index for our PCIe bar so that we find it again */
    resource = new_resource(dev, 0xcf);
    resource->base = DEFAULT_PCIEXBAR;
    resource->size = 64 * 1024 * 1024;	/* 64MB hard coded PCIe config space */
    resource->flags =
        IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |
        IORESOURCE_ASSIGNED;
    printk_debug("Adding PCIe enhanced config space BAR 0x%08x-0x%08x.\n",
    	     resource->base, (resource->base + resource->size));

I didn't see anywhere that you enabled the PCI decode space in the
fam10 code.  Is it fixed?  How does resource allocation avoid it?

Thanks,
Myles

[Stefan Reinauer]

On 06.03.2009 19:20 Uhr, Myles Watson wrote:
> It seems like these two snippets should be related in some way.  Do
> you have to set them equal?

Yes, the two are related. They're not yet unified - there will be more
patches coming to clean things up, but I need to get my patch queue
smaller in order to not drift away from the public tree.
> Index: src/config/Options.lb
>
> --- src/config/Options.lb	(.../branches/upstream/coreboot-v2)	
> +++ src/config/Options.lb	(.../trunk/coreboot-v2)	
> @@ -983,6 +983,13 @@
>    comment "enable mmconfig for pci conf"
>  end
>
> +define MMCONF_BASE_ADDRESS
> default none
> format "0x%x"
> export used
> comment "enable mmconfig base address"
>
> +end
>
> from /src/northbridge/intel/i945/northbridge.c:
>
> /* We use 0xcf as an unused index for our PCIe bar so that we find it again */
>    resource = new_resource(dev, 0xcf);
>    resource->base = DEFAULT_PCIEXBAR;
>    resource->size = 64 * 1024 * 1024;	/* 64MB hard coded PCIe config space */
>    resource->flags =
>        IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |
>        IORESOURCE_ASSIGNED;
>    printk_debug("Adding PCIe enhanced config space BAR 0x%08x-0x%08x.\n",
>    	     resource->base, (resource->base + resource->size));
>
> I didn't see anywhere that you enabled the PCI decode space in the
> fam10 code.  Is it fixed?  How does resource allocation avoid it?

I didn't touch any of the fam10 code, except the obvious MMCONF ->
MMCONF_DEFAULT, because that's what the the code that is there really is
supposed to do.
Hopefully someone can jump in on fam10 and help getting that part
straight. At the moment, I'm focussed on getting it right for the Intel
code and try to fix up some of the AMD parts as far as I can be sure not
to break anything there or make it worse.

Stefan

[Myles Watson]

On Fri, Mar 6, 2009 at 11:45 AM, Stefan Reinauer  wrote:
> On 06.03.2009 19:20 Uhr, Myles Watson wrote:
> > It seems like these two snippets should be related in some way.  Do
> > you have to set them equal?
>
> Yes, the two are related. They're not yet unified - there will be more
> patches coming to clean things up, but I need to get my patch queue
> smaller in order to not drift away from the public tree.

Sure.  I just wanted to make sure that was the direction we're heading in.

Acked-by: Myles Watson

Thanks,
Myles

[Carl-Daniel Hailfinger]

On 06.03.2009 19:07, Stefan Reinauer wrote:
> Fix mmconf (PCIe memory mapped config space access) support in v2. It was
> horribly broken and thus never used by any platform. This needs to get
> straightened out so current chipsets drivers can use the full feature set.
>
> Create wrapper functions similar to the io pci config space ones.
>
> Signed-off-by: Stefan Reinauer

IIRC the AMD DBM690T code uses mmconf accesses for PCIe. Then again,
maybe it was only in the ACPI code.

Regards,
Carl-Daniel

[Stefan Reinauer]

On 06.03.2009 19:39 Uhr, Carl-Daniel Hailfinger wrote:
> On 06.03.2009 19:07, Stefan Reinauer wrote:
> > Fix mmconf (PCIe memory mapped config space access) support in v2. It was
> > horribly broken and thus never used by any platform. This needs to get
> > straightened out so current chipsets drivers can use the full feature set.
> >
> > Create wrapper functions similar to the io pci config space ones.
> >
> > Signed-off-by: Stefan Reinauer
>
> IIRC the AMD DBM690T code uses mmconf accesses for PCIe.

No, it does not. MMCONF is unused. I am absolutely certain that it never
worked. (As you can be, when you look at my patch)