On Tue, Aug 06, 2013 at 11:22:27AM +0800, Gavin Shan wrote: > On Mon, Aug 05, 2013 at 04:21:35PM -0600, Bjorn Helgaas wrote: > >On Mon, Aug 05, 2013 at 02:09:27PM -0700, Yinghai Lu wrote: > >> On Mon, Aug 5, 2013 at 1:59 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > >> >> then, we should drop that 4k capping. > >> >> I was thinking there could be strange or wild res with bigger than 4k. > >> > > >> > If there *were* an I/O BAR larger than 4KB, how should it be handled? > >> > I don't think capping the alignment to 4KB sounds like the best way. > >> > For example, a 16KB I/O BAR would still need to be aligned on 16KB. > >> > > >> > And I think capping to 4KB as you did above will break the powerpc > >> > pcibios_window_alignment() implementation. For example, if > >> > pcibios_window_alignment() returned 16KB, and we later capped it to > >> > 4KB, we're going to allocate space for the bridge window with the > >> > wrong alignment. > >> > >> Agree. > > > >OK. Can you guys try this out and see whether it fixes the problem? > >I don't know what the actual problem *is*, so I can't tell whether > >this is a possible fix. > > > > The code looks simpler, but it potentially breaks PowerNV platforms. > Lets have inline description if I understand everything here. > > >diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > >index d4f1ad9..8333c92 100644 > >--- a/drivers/pci/setup-bus.c > >+++ b/drivers/pci/setup-bus.c > >@@ -749,12 +749,12 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > > struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO); > > resource_size_t size = 0, size0 = 0, size1 = 0; > > resource_size_t children_add_size = 0; > >- resource_size_t min_align, io_align, align; > >+ resource_size_t min_align, align; > > > > if (!b_res) > > return; > > > >- io_align = min_align = window_alignment(bus, IORESOURCE_IO); > >+ min_align = window_alignment(bus, IORESOURCE_IO); > > list_for_each_entry(dev, &bus->devices, bus_list) { > > int i; > > > > "min_align" here indicates the IO segment size on PowerNV platform. > On PowerNV platform, the IO range of specific PHB (PCI controller) > is divided evenly and each piece of that is called IO segment. For > example, the IO segment size ("min_align") is 64K initially. If one > of specific IO BAR has size of 96K (it's possible?), "min_align" > becomes 96K eventually, which isn't IO segment aligned. 96K is not a valid size for any PCI BAR. BARs are always a power-of-2 size. If PowerNV has an I/O segment size of 64K and we have an I/O BAR of 128K (illegal per spec, but assume we did for the sake of argument), we would allocate a 128K-aligned area, which does satisfy the PowerNV alignment restriction. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html