| Message ID | 20090423132202.GB13984@jurassic.park.msu.ru (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Ivan Kokshaysky wrote: > On Wed, Apr 22, 2009 at 07:10:29PM -0700, Yinghai Lu wrote: >> also on AMD system with two ht chain, or other system with pci=use_crs to get correct root default res, >> will get anonying >> >> PCI: allocations above 4G disabled >> >> even the system does support that. > > Yep, but it's easy to fix (patch applies on the top of the previous one). another case: one system with 8g ram install, one device on bus 0 does get res assigned, but one res from it does support 64bit mmio. and at that case the assign_unassigned code still could assign 64 res to it. but kernel will still print out that warning. > >> also will have problem with some calling like request_resource(&iomem_resource, ....) > > I don't think so. All critical resources are inserted much earlier than > mem64 one, and request_resource(&iomem_resource, ...) at later stages > would most likely fail regardless of mem64 thing. > Or am I missing something? at least the one in mm/memory_hotplug.c /* add this memory to iomem resource */ static struct resource *register_memory_resource(u64 start, u64 size) { struct resource *res; res = kzalloc(sizeof(struct resource), GFP_KERNEL); BUG_ON(!res); res->name = "System RAM"; res->start = start; res->end = start + size - 1; res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; if (request_resource(&iomem_resource, res) < 0) { printk("System RAM resource %llx - %llx cannot be added\n", (unsigned long long)res->start, (unsigned long long)res->end); kfree(res); res = NULL; } return res; } > > Ivan. > > diff --git a/arch/x86/pci/dac_64bit.c b/arch/x86/pci/dac_64bit.c > index ee03c4a..35ffee3 100644 > --- a/arch/x86/pci/dac_64bit.c > +++ b/arch/x86/pci/dac_64bit.c > @@ -33,12 +33,16 @@ void pcibios_pci64_setup(void) > void pcibios_pci64_verify(void) > { > struct pci_bus *b; > + int disabled = 0; > > if (mem64.flags & IORESOURCE_MEM64) > return; /* presumably DAC works */ > list_for_each_entry(b, &pci_root_buses, node) { > - if (b->resource[2] == &mem64) > + if (b->resource[2] == &mem64) { > b->resource[2] = NULL; > + disabled = 1; > + } > } > - printk(KERN_INFO "PCI: allocations above 4G disabled\n"); > + if (disabled) > + printk(KERN_INFO "PCI: allocations above 4G disabled\n"); > } -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 23, 2009 at 08:13:04AM -0700, Yinghai Lu wrote: > at least the one in mm/memory_hotplug.c > > /* add this memory to iomem resource */ > static struct resource *register_memory_resource(u64 start, u64 size) > { > struct resource *res; > res = kzalloc(sizeof(struct resource), GFP_KERNEL); > BUG_ON(!res); > > res->name = "System RAM"; > res->start = start; > res->end = start + size - 1; > res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; > if (request_resource(&iomem_resource, res) < 0) { > printk("System RAM resource %llx - %llx cannot be added\n", > (unsigned long long)res->start, (unsigned long long)res->end); > kfree(res); > res = NULL; > } > return res; > } Indeed. It's a really strong argument against mem64 resource approach. On the other hand, it shows that getting 64-bit allocations right is indeed a very complex issue - without well defined root bus ranges there is a high risk of unexpectedly breaking something, like that memory hotplug. Oh well... So if the main purpose is to prevent 32-bit allocations in the DAC area, some mix of your v2 and v3 patches seems to be the way to go. That is - keep IORESOURCE_MEM_64 bits from -v3, but drop iomem32_resource and iomem64_resource things; - pass "max" argument to allocate_resource() like you did in -v2, but *only* then allocating from iomem_resource (r == &iomem_resource). Also, instead of max = 0xffffffff use something like max = PCIBIOS_MAX_MEM_32. include/linux/pci.h: #ifndef PCIBIOS_MAX_MEM_32 #define PCIBIOS_MAX_MEM_32 (-1) #endif This should preserve the current behaviour of pci_bus_alloc_resource() on non-x86 arches; overridden in arch/x86/include/asm/pci.h: #define PCIBIOS_MAX_MEM_32 0xffffffff > check bits 0-3 and check PCI_PREF_BASE_UPPER32 register being r/w ? Yes. If these bits are zero, no further checks are needed - bridge is 32-bit. If they aren't, do additional check for PCI_PREF_BASE_UPPER32 being non-zero or writable, but *only* if this prefetch resource is not already allocated (res->parent == NULL), just for safety reasons - we don't want to disconnect the allocated range from the bus even for a short time. Ivan. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/pci/dac_64bit.c b/arch/x86/pci/dac_64bit.c index ee03c4a..35ffee3 100644 --- a/arch/x86/pci/dac_64bit.c +++ b/arch/x86/pci/dac_64bit.c @@ -33,12 +33,16 @@ void pcibios_pci64_setup(void) void pcibios_pci64_verify(void) { struct pci_bus *b; + int disabled = 0; if (mem64.flags & IORESOURCE_MEM64) return; /* presumably DAC works */ list_for_each_entry(b, &pci_root_buses, node) { - if (b->resource[2] == &mem64) + if (b->resource[2] == &mem64) { b->resource[2] = NULL; + disabled = 1; + } } - printk(KERN_INFO "PCI: allocations above 4G disabled\n"); + if (disabled) + printk(KERN_INFO "PCI: allocations above 4G disabled\n"); }