| Submitter | Jonathan A. Kollasch |
|---|---|
| Date | 2011-05-22 15:12:06 |
| Message ID | <20110522151206.GI24800@tarantulon.kollasch.net> |
| Download | mbox | patch |
| Permalink | /patch/2982/ |
| State | New |
| Headers | show |
Comments
On 5/22/11 8:12 AM, Jonathan A. Kollasch wrote: > Optionally treat ck804 memory-mapped PCI configuration space window as a > resource. Not enabled by default because the resource should be > non-posted and there's no way to express that to the resource allocator. > > Signed-off-by: Jonathan Kollasch<jakllsch@kollasch.net> Please don't make stuff like that a Kconfig option. All memory used by devices should be resources, so it's fine to always enable this, given that it's tested and works. Why would the resource allocator have to worry about posted vs non-posted PCI transactions?
On Sun, May 22, 2011 at 12:28:41PM -0700, Stefan Reinauer wrote: > On 5/22/11 8:12 AM, Jonathan A. Kollasch wrote: > >Optionally treat ck804 memory-mapped PCI configuration space window as a > >resource. Not enabled by default because the resource should be > >non-posted and there's no way to express that to the resource allocator. > > > >Signed-off-by: Jonathan Kollasch<jakllsch@kollasch.net> > Please don't make stuff like that a Kconfig option. All memory used > by devices should be resources, so it's fine to always enable this, > given that it's tested and works. Aren't there fam10h boards that have a ck804 southbridge? How do we make sure the appropriate mmconf implementation is used? > Why would the resource allocator have to worry about posted vs > non-posted PCI transactions? How else will the northbridge be told to make the region non-posted? Jonathan Kollasch
On 5/22/11 1:07 PM, Jonathan A. Kollasch wrote: > On Sun, May 22, 2011 at 12:28:41PM -0700, Stefan Reinauer wrote: >> On 5/22/11 8:12 AM, Jonathan A. Kollasch wrote: >>> Optionally treat ck804 memory-mapped PCI configuration space window as a >>> resource. Not enabled by default because the resource should be >>> non-posted and there's no way to express that to the resource allocator. >>> >>> Signed-off-by: Jonathan Kollasch<jakllsch@kollasch.net> >> Please don't make stuff like that a Kconfig option. All memory used >> by devices should be resources, so it's fine to always enable this, >> given that it's tested and works. > Aren't there fam10h boards that have a ck804 southbridge? > How do we make sure the appropriate mmconf implementation is used? > We have defines for the cpu/northbridge/southbridge already ... why not use those, if needed? >> Why would the resource allocator have to worry about posted vs >> non-posted PCI transactions? > How else will the northbridge be told to make the region non-posted? Why would it have to? Not sure I understand your concern. Your patch does not address that issue at all.
Stefan Reinauer wrote: ]>> Why would the resource allocator have to worry about posted vs ]>> non-posted PCI transactions? ]> How else will the northbridge be told to make the region non-posted? ]Why would it have to? Not sure I understand your concern. Your patch ]does not address that issue at all. This subject came up recently: http://www.coreboot.org/pipermail/coreboot/2010-October/061288.html AMD processors have 8 mmio base/limit register pairs. Each pair defines an address range as posted mmio or as non-posted mmio. Somehow posted and non-posted ranges have to be allocated so that they can be consolidated into a total of 8 or fewer ranges. Then the mmio base/limit registers can be programmed to cover the ranges. Thanks, ScottD
"Jonathan A. Kollasch" <jakllsch@kollasch.net> writes: > Optionally treat ck804 memory-mapped PCI configuration space window as a > resource. Not enabled by default because the resource should be > non-posted and there's no way to express that to the resource allocator. > > Signed-off-by: Jonathan Kollasch <jakllsch@kollasch.net> Can you expand on why the resource should be non-posted? I didn't think non-posted was a requirement of pci config space. Where are you configuring posted vs non-posted behavior? Do writes to extended pci config space actually work for you. A coworker hacked something like this together a while ago for one of the boards we support and there is a bug where I can read extended pcie config space but not write to it. Except for a bit of trivial looking I haven't had a chance to dig into that problem. Eric
On Mon, May 23, 2011 at 03:01:16AM -0700, Eric W. Biederman wrote: > "Jonathan A. Kollasch" <jakllsch@kollasch.net> writes: > > > Optionally treat ck804 memory-mapped PCI configuration space window as a > > resource. Not enabled by default because the resource should be > > non-posted and there's no way to express that to the resource allocator. > > > > Signed-off-by: Jonathan Kollasch <jakllsch@kollasch.net> > > Can you expand on why the resource should be non-posted? I didn't think > non-posted was a requirement of pci config space. I don't remember if I ever saw that in writing. However, k8resdump when booted in the original BIOS shows: MMIO map: #6 0x00e0000000 - 0x00e05fffff Access: R/W NonPosted Dstnode:0 DstLink 0 > Where are you configuring posted vs non-posted behavior? I'm not. There isn't a good way to do it yet. > Do writes to extended pci config space actually work for you. A > coworker hacked something like this together a while ago for one of the > boards we support and there is a bug where I can read extended pcie > config space but not write to it. Except for a bit of trivial looking > I haven't had a chance to dig into that problem. I'll check. Jonathan Kollasch
On Mon, May 23, 2011 at 10:56:29AM +0000, Jonathan A. Kollasch wrote: > On Mon, May 23, 2011 at 03:01:16AM -0700, Eric W. Biederman wrote: > > Do writes to extended pci config space actually work for you. A > > coworker hacked something like this together a while ago for one of the > > boards we support and there is a bug where I can read extended pcie > > config space but not write to it. Except for a bit of trivial looking > > I haven't had a chance to dig into that problem. > > I'll check. Appears to work. A read/zero/restore operation on the PCI CSR shadow at 0x404 in the 88E8052 on this board responds as expected. (This is under coreboot with posted memory for the mmconf window.) Jonathan Kollasch
Patch
Index: src/southbridge/nvidia/ck804/Kconfig =================================================================== --- src/southbridge/nvidia/ck804/Kconfig (revision 6607) +++ src/southbridge/nvidia/ck804/Kconfig (working copy) @@ -43,4 +43,12 @@ int default 1 +config CK804_MMCONF_SUPPORT + bool + default n + +config CK804_MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + endif Index: src/southbridge/nvidia/ck804/ht.c =================================================================== --- src/southbridge/nvidia/ck804/ht.c (revision 6607) +++ src/southbridge/nvidia/ck804/ht.c (working copy) @@ -25,9 +25,47 @@ #include <device/pci_ops.h> #include "ck804.h" +static void ht_read_resources(device_t dev) +{ +#if CONFIG_CK804_MMCONF_SUPPORT + struct resource *res; +#endif + + pci_dev_read_resources(dev); + +#if CONFIG_CK804_MMCONF_SUPPORT + res = new_resource(dev, 0x90); /* MMCONF */ + res->base = CONFIG_CK804_MMCONF_BASE_ADDRESS; + res->size = 0x10000000; + res->align = 28; + res->gran = 28; + res->limit = 0xffffffff; + res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; +#endif +} + +static void ht_set_resources(device_t dev) +{ +#if CONFIG_CK804_MMCONF_SUPPORT + struct resource *res; +#endif + + pci_dev_set_resources(dev); + +#if CONFIG_CK804_MMCONF_SUPPORT + /* enable MMCONF */ + res = find_resource(dev, 0x90); + if (res) { + pci_write_config16(dev, 0x90, 1<<12|((res->base >> 28)&0xf)); + res->flags |= IORESOURCE_STORED; + report_resource_stored(dev, res, ""); + } +#endif +} + static struct device_operations ht_ops = { - .read_resources = pci_dev_read_resources, - .set_resources = pci_dev_set_resources, + .read_resources = ht_read_resources, + .set_resources = ht_set_resources, .enable_resources = pci_dev_enable_resources, .init = 0, .scan_bus = 0,
Optionally treat ck804 memory-mapped PCI configuration space window as a resource. Not enabled by default because the resource should be non-posted and there's no way to express that to the resource allocator. Signed-off-by: Jonathan Kollasch <jakllsch@kollasch.net>