On Mon, Aug 5, 2013 at 2:52 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: > On Mon, Aug 5, 2013 at 12:51 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: >>> @@ -848,8 +848,8 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, >>> } >>> } >>> >>> - if (min_align > io_align) >>> - min_align = io_align; >>> + if (min_align > PCI_P2P_DEFAULT_IO_ALIGN) >>> + min_align = PCI_P2P_DEFAULT_IO_ALIGN; >> >> But I still don't understand this one. As far as I can tell, >> "min_align > 4096" can happen only for arch-specific I/O window >> requirements. It can never happen because of a large device I/O BAR. >> >> Let's assume "min_align = window_alignment(bus, IORESOURCE_IO)" above set >> min_align to 16KB because that's what pnv_pci_window_alignment() returned. >> Why would we cap it to 4KB here? I think we should set this: >> >> b_res->start = 16384; >> b_res->end = b_res->start + size0 - 1; >> b_res->flags |= IORESOURCE_STARTALIGN; >> >> to indicate that the bridge needs a 16KB-aligned I/O window. >> >> In what case do we need to cap min_align to 4KB? >> >>> size0 = calculate_iosize(size, min_size, size1, >>> resource_size(b_res), min_align); > > 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. 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
