| Message ID | 1375435866-16332-5-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: > In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), > it introduce a new method to calculate the window alignment of P2P bridge. > > When the io_window_1k is set, the calculation for the io resource alignment > is different from the original one. In the original logic before 462d9303, > the alignment is no bigger than 4K even the io_window_1k is set. The logic > introduced in 462d9303 will limit the alignment to 1k in this case. > > This patch fix this issue. > > Here is an example for this case. > > Assume: > 1. pcibios_window_alignment() return 1. > 2. window_alignment() return PCI_P2P_DEFAULT_IO_ALIGN_1K. > 3. one of the child device has an IO resource with size of 2K. > > Result comparison: > > Before 462d9303 After 462d9303 > min_align 1k 1k > | > after loop | > V > min_align 2k 2k > | > check boundary | > V > min_align 2k 1k > > After applying the change: > > Result comparison: > > with 462d9303 with this patch > min_align 1k 1k > io_align 1k 4k > | > after loop | > V > min_align 2k 2k > io_align 1k 4k > | > check boundary | > V > min_align 1k 2k > io_align 1k 1k > > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> Gavin, can you ack this, since you wrote the original commit involved? > --- > drivers/pci/setup-bus.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index d4f1ad9..6c111e9 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -755,6 +755,10 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > return; > > io_align = min_align = window_alignment(bus, IORESOURCE_IO); > + /* Don't exceed 4KiB for windows requesting 1KiB alignment */ > + if (bus->self->io_window_1k && io_align == PCI_P2P_DEFAULT_IO_ALIGN_1K) > + io_align = PCI_P2P_DEFAULT_IO_ALIGN; > + > list_for_each_entry(dev, &bus->devices, bus_list) { > int i; > > -- > 1.7.5.4 > -- 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
[+cc Yinghai] On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: > In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), > it introduce a new method to calculate the window alignment of P2P bridge. > > When the io_window_1k is set, the calculation for the io resource alignment > is different from the original one. In the original logic before 462d9303, > the alignment is no bigger than 4K even the io_window_1k is set. The logic > introduced in 462d9303 will limit the alignment to 1k in this case. > > This patch fix this issue. Presumably this fixes a bug, but you don't say what it is. "different from the original" is not a description of a problem. You need something like "with the current code, we allocate the wrong window size in situation X, or we allocate a window with incorrect alignment in situation Y, etc." > Here is an example for this case. > > Assume: > 1. pcibios_window_alignment() return 1. > 2. window_alignment() return PCI_P2P_DEFAULT_IO_ALIGN_1K. > 3. one of the child device has an IO resource with size of 2K. > > Result comparison: > > Before 462d9303 After 462d9303 > min_align 1k 1k > | > after loop | > V > min_align 2k 2k > | > check boundary | > V > min_align 2k 1k > > After applying the change: > > Result comparison: > > with 462d9303 with this patch > min_align 1k 1k > io_align 1k 4k > | > after loop | > V > min_align 2k 2k > io_align 1k 4k > | > check boundary | > V > min_align 1k 2k > io_align 1k 1k > > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> > --- > drivers/pci/setup-bus.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index d4f1ad9..6c111e9 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -755,6 +755,10 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > return; > > io_align = min_align = window_alignment(bus, IORESOURCE_IO); I don't think it makes sense to set "min_align = window_alignment(bus, IORESOURCE_IO)" before the loop. "io_align" is a function of the bridge and is independent of downstream devices and is computed by window_alignment(). "min_align" is a function of the downstream devices and bridges and should be zero when entering the loop. > + /* Don't exceed 4KiB for windows requesting 1KiB alignment */ This comment doesn't make sense to me. The code changes io_align from 1024 to 4096, so I don't understand where the "don't exceed 4KiB" part comes from. io_align doesn't exceed 4096 anyway, unless pcibios_window_alignment() gives us a larger alignment, and in that case, this patch has no effect because io_align is not 1024. > + if (bus->self->io_window_1k && io_align == PCI_P2P_DEFAULT_IO_ALIGN_1K) > + io_align = PCI_P2P_DEFAULT_IO_ALIGN; > + > list_for_each_entry(dev, &bus->devices, bus_list) { > int i; This list_for_each_entry() loop computes the alignment required by downstream devices and bridges. The result is "min_align". After the loop we have this (added by Yinghai in fd5913411): if (min_align > io_align) min_align = io_align; I don't understand this. There are three cases: 1) No downstream bridges: min_align <= 256 because I/O BARs are limited to 256 bytes. 2) All downstream bridges have 1K extension: min_align = 1024. 3) At least one downstream bridge requires 4K alignment: min_align = 4096. "io_align" is the minimum alignment enforced by the bridge. This is independent of any devices below the bridge, so "io_align >= 1024" in all cases. Therefore, the "min_align = io_align" assignment only happens in case 3, when a downstream bridge requires 4K alignment and *this* bridge supports the 1K extension. But that seems wrong. The downstream bridge's 4K requirement also applies to all bridges all the way upstream. It looks to me like this test should instead be: if (min_align < io_align) min_align = io_align; > -- > 1.7.5.4 > -- 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 Mon, Aug 5, 2013 at 10:58 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > [+cc Yinghai] > > After the loop we have this (added by Yinghai in fd5913411): > > if (min_align > io_align) > min_align = io_align; > > I don't understand this. There are three cases: > > 1) No downstream bridges: min_align <= 256 because I/O BARs are > limited to 256 bytes. > 2) All downstream bridges have 1K extension: min_align = 1024. > 3) At least one downstream bridge requires 4K alignment: min_align = 4096. > > "io_align" is the minimum alignment enforced by the bridge. This is > independent of any devices below the bridge, so "io_align >= 1024" in > all cases. > > Therefore, the "min_align = io_align" assignment only happens in case > 3, when a downstream bridge requires 4K alignment and *this* bridge > supports the 1K extension. But that seems wrong. The downstream > bridge's 4K requirement also applies to all bridges all the way > upstream. > > It looks to me like this test should instead be: > > if (min_align < io_align) > min_align = io_align; before my change, min_align always set to 4096. in the loop to get min_align, dev resource could have size bigger than 4k, those resource will have SIZEALIGN, so final min_align could be more than 4096. so at last we cap it to 4096 again. But according to your findings: "I/O BARs are limited to 256 bytes" we should not worry about that. So we should just drop io_align and checking like attached patch. (min_align is already set to 1k or 4k before the looping.) and that should solve Wei Yang's problem. Thanks Yinghai
On Mon, Aug 05, 2013 at 12:05:03PM -0700, Yinghai Lu wrote: > On Mon, Aug 5, 2013 at 10:58 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > [+cc Yinghai] > > > > After the loop we have this (added by Yinghai in fd5913411): > > > > if (min_align > io_align) > > min_align = io_align; > > > > I don't understand this. There are three cases: > > > > 1) No downstream bridges: min_align <= 256 because I/O BARs are > > limited to 256 bytes. > > 2) All downstream bridges have 1K extension: min_align = 1024. > > 3) At least one downstream bridge requires 4K alignment: min_align = 4096. > > > > "io_align" is the minimum alignment enforced by the bridge. This is > > independent of any devices below the bridge, so "io_align >= 1024" in > > all cases. > > > > Therefore, the "min_align = io_align" assignment only happens in case > > 3, when a downstream bridge requires 4K alignment and *this* bridge > > supports the 1K extension. But that seems wrong. The downstream > > bridge's 4K requirement also applies to all bridges all the way > > upstream. > > > > It looks to me like this test should instead be: > > > > if (min_align < io_align) > > min_align = io_align; > > before my change, min_align always set to 4096. > in the loop to get min_align, dev resource could have size bigger than > 4k, those resource will have SIZEALIGN, so final min_align could be > more than 4096. > so at last we cap it to 4096 again. To make sure I understand this, I think (correct me if I'm wrong): - Every device BAR resource will have IORESOURCE_SIZEALIGN set since it must be naturally aligned on its size (sec 6.2.5.1 of PCI spec r3.0). - Bridge window resources will have IORESOURCE_STARTALIGN set and res->start contains the required alignment, i.e., 1MB for MEM windows, 4K for architected I/O windows, 1K for bridges with 1K extension, or larger values for arch-specific requirements. > But according to your findings: "I/O BARs are limited to 256 bytes" > we should not worry about that. This is also per sec 6.2.5.1 of PCI spec r3.0: "Devices that map control functions into I/O Space must not consume more than 256 bytes per I/O Base Address register." > So we should just drop io_align and checking like attached patch. > (min_align is already set to 1k or 4k before the looping.) > > and that should solve Wei Yang's problem. [I inlined your patch for commenting. You're welcome :)] > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 64a7de2..edaa9e8 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -816,12 +816,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); > unsigned long 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); I like this change. > list_for_each_entry(dev, &bus->devices, bus_list) { > int i; > > @@ -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); -- 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 Mon, Aug 5, 2013 at 12:51 PM, Bjorn Helgaas <bhelgaas@google.com> 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. Thanks Yinghai -- 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 Mon, Aug 5, 2013 at 2:52 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Mon, Aug 5, 2013 at 12:51 PM, Bjorn Helgaas <bhelgaas@google.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 5, 2013 at 1:59 PM, Bjorn Helgaas <bhelgaas@google.com> 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. -- 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 Mon, Aug 05, 2013 at 11:58:50AM -0600, Bjorn Helgaas wrote: >[+cc Yinghai] > >On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: >> In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), >> it introduce a new method to calculate the window alignment of P2P bridge. >> >> When the io_window_1k is set, the calculation for the io resource alignment >> is different from the original one. In the original logic before 462d9303, >> the alignment is no bigger than 4K even the io_window_1k is set. The logic >> introduced in 462d9303 will limit the alignment to 1k in this case. >> >> This patch fix this issue. > >Presumably this fixes a bug, but you don't say what it is. "different >from the original" is not a description of a problem. You need >something like "with the current code, we allocate the wrong window >size in situation X, or we allocate a window with incorrect alignment >in situation Y, etc." > With current code, we allocate the wrong window size when upstream bridge could be 1k aligned and one of the downstream port is 4k aligned. In this case, the "min_align" should be 4k. But the current code set "min_align" to 1k. -- 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 Tue, Aug 06, 2013 at 02:19:24PM +0800, Wei Yang wrote: > On Mon, Aug 05, 2013 at 11:58:50AM -0600, Bjorn Helgaas wrote: > >[+cc Yinghai] > > > >On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: > >> In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), > >> it introduce a new method to calculate the window alignment of P2P bridge. > >> > >> When the io_window_1k is set, the calculation for the io resource alignment > >> is different from the original one. In the original logic before 462d9303, > >> the alignment is no bigger than 4K even the io_window_1k is set. The logic > >> introduced in 462d9303 will limit the alignment to 1k in this case. > >> > >> This patch fix this issue. > > > >Presumably this fixes a bug, but you don't say what it is. "different > >from the original" is not a description of a problem. You need > >something like "with the current code, we allocate the wrong window > >size in situation X, or we allocate a window with incorrect alignment > >in situation Y, etc." > > > > With current code, we allocate the wrong window size when upstream bridge > could be 1k aligned and one of the downstream port is 4k aligned. > > In this case, the "min_align" should be 4k. But the current code set > "min_align" to 1k. I would expect that we allocate a window with incorrect *alignment*, not incorrect size. If you include a dmesg log and lspci output and point out the problem, that will avoid a lot of confusion. Bjorn -- 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 Tue, Aug 06, 2013 at 07:44:21AM -0600, Bjorn Helgaas wrote: >On Tue, Aug 06, 2013 at 02:19:24PM +0800, Wei Yang wrote: >> On Mon, Aug 05, 2013 at 11:58:50AM -0600, Bjorn Helgaas wrote: >> >[+cc Yinghai] >> > >> >On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: >> >> In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), >> >> it introduce a new method to calculate the window alignment of P2P bridge. >> >> >> >> When the io_window_1k is set, the calculation for the io resource alignment >> >> is different from the original one. In the original logic before 462d9303, >> >> the alignment is no bigger than 4K even the io_window_1k is set. The logic >> >> introduced in 462d9303 will limit the alignment to 1k in this case. >> >> >> >> This patch fix this issue. >> > >> >Presumably this fixes a bug, but you don't say what it is. "different >> >from the original" is not a description of a problem. You need >> >something like "with the current code, we allocate the wrong window >> >size in situation X, or we allocate a window with incorrect alignment >> >in situation Y, etc." >> > >> >> With current code, we allocate the wrong window size when upstream bridge >> could be 1k aligned and one of the downstream port is 4k aligned. >> >> In this case, the "min_align" should be 4k. But the current code set >> "min_align" to 1k. > Hmm... sorry I should say. With current code, we allocate the wrong window size and alignment when upstream bridge could be 1k aligned and one of the downstream port is 4k aligned. In this case, the "min_align" should be 4k. But the current code set "min_align" to 1k. >I would expect that we allocate a window with incorrect *alignment*, >not incorrect size. If you include a dmesg log and lspci output and point >out the problem, that will avoid a lot of confusion. Agree. I notice this potential problem during the code reading and found the io_window_1k is set only for Intel P64H2 bridge. Currently I don't have this device, not sure how to create this senario. Sorry for the confusion, I should state the problem more clearly. > >Bjorn
On Tue, Aug 06, 2013 at 11:47:42PM +0800, Wei Yang wrote: > On Tue, Aug 06, 2013 at 07:44:21AM -0600, Bjorn Helgaas wrote: > >On Tue, Aug 06, 2013 at 02:19:24PM +0800, Wei Yang wrote: > >> On Mon, Aug 05, 2013 at 11:58:50AM -0600, Bjorn Helgaas wrote: > >> >[+cc Yinghai] > >> > > >> >On Fri, Aug 2, 2013 at 3:31 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: > >> >> In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), > >> >> it introduce a new method to calculate the window alignment of P2P bridge. > >> >> > >> >> When the io_window_1k is set, the calculation for the io resource alignment > >> >> is different from the original one. In the original logic before 462d9303, > >> >> the alignment is no bigger than 4K even the io_window_1k is set. The logic > >> >> introduced in 462d9303 will limit the alignment to 1k in this case. > >> >> > >> >> This patch fix this issue. > >> > > >> >Presumably this fixes a bug, but you don't say what it is. "different > >> >from the original" is not a description of a problem. You need > >> >something like "with the current code, we allocate the wrong window > >> >size in situation X, or we allocate a window with incorrect alignment > >> >in situation Y, etc." > >> > > >> > >> With current code, we allocate the wrong window size when upstream bridge > >> could be 1k aligned and one of the downstream port is 4k aligned. > >> > >> In this case, the "min_align" should be 4k. But the current code set > >> "min_align" to 1k. > > > > Hmm... sorry I should say. > > With current code, we allocate the wrong window size and alignment when upstream > bridge could be 1k aligned and one of the downstream port is 4k aligned. > > In this case, the "min_align" should be 4k. But the current code set > "min_align" to 1k. Actually, I think only the alignment is wrong (not the size). But I do agree that this looks like a problem in the current code. I'll write this up and post the patch soon. Bjorn -- 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/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index d4f1ad9..6c111e9 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -755,6 +755,10 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, return; io_align = min_align = window_alignment(bus, IORESOURCE_IO); + /* Don't exceed 4KiB for windows requesting 1KiB alignment */ + if (bus->self->io_window_1k && io_align == PCI_P2P_DEFAULT_IO_ALIGN_1K) + io_align = PCI_P2P_DEFAULT_IO_ALIGN; + list_for_each_entry(dev, &bus->devices, bus_list) { int i;
In commit 462d9303 ("PCI: Align P2P windows using pcibios_window_alignment()"), it introduce a new method to calculate the window alignment of P2P bridge. When the io_window_1k is set, the calculation for the io resource alignment is different from the original one. In the original logic before 462d9303, the alignment is no bigger than 4K even the io_window_1k is set. The logic introduced in 462d9303 will limit the alignment to 1k in this case. This patch fix this issue. Here is an example for this case. Assume: 1. pcibios_window_alignment() return 1. 2. window_alignment() return PCI_P2P_DEFAULT_IO_ALIGN_1K. 3. one of the child device has an IO resource with size of 2K. Result comparison: Before 462d9303 After 462d9303 min_align 1k 1k | after loop | V min_align 2k 2k | check boundary | V min_align 2k 1k After applying the change: Result comparison: with 462d9303 with this patch min_align 1k 1k io_align 1k 4k | after loop | V min_align 2k 2k io_align 1k 4k | check boundary | V min_align 1k 2k io_align 1k 1k Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> --- drivers/pci/setup-bus.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)