| Message ID | 20130805222135.GA29875@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
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@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. > >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. > Thanks all for the comments, this makes me re-consider the cases. Let me do a summary. Maybe I misunderstand the idea, please fix me~ Requirement from PCI Spec ============================================================================ 1. I/O BAR for non-bridge PCI devices are limited to 256 bytes 2. Most I/O window for PCI bridge is 4k aligned 3. Some bridge support 1k aligned I/O window Ancient time -- when 1k aligned I/O window is not there ============================================================================ The alignment is 4k in all cases. (As it is hard coded.) For devices under this bridge, I/O BAR is less then 256. For downstream port, I/O window is 4k aligned, since the IORESOURCE_STARTALIGN is set. This means even the downstream port connects other bridge, the alignment is still 4k. Middle Age -- when 1k aligned I/O window is introduced ============================================================================ This introduce two other cases: 1. All downstream port is 1k aligned 2. One of the downstream port is 4k aligned. Case 1: the "min_align" should be set to 1k. This could save some I/O resource. Case 2: the "min_align" should be set to 4k, even itself anounced could support 1k alignment. ^--- Fix me, if not correct. The "min_align" could be set to other value? Previously, I thought it could be, for example 2k. Now I think no, the list_for_each_entry loop will iterate on two kinds of resources: 1. Device I/O BAR; 2. Bridge I/O window. Device I/O BAR is less then 256 bytes, it won't contribut to the alignment. Bridge I/O window will be 1k or 4k aligned. This means only two possible value for "min_align": 1k or 4k. ^--- Fix me, if not correct. Back to Yinghai's commit(fd5913411), the "min_align" is set to 1k at the beginning. During the list_for_each_entry loop, (align > min_align) is true means align is 4k. And this (min_align > 4096) will never be true, since at most "min_align" is 4k. So, I think, this comparison could be removed in commit(fd5913411). ^--- Fix me, if not correct. Present Age -- when architecture require specific alignment for bridge window ============================================================================ This introduce 3 cases: 1. 1k < 4k < arch_align 2. 1k < arch_align < 4k 3. arch_align < 1k < 4k Case 1: the "min_align" should be arch_align. Case 2: this is a little complicated. downstream port could be all 1k aligned or one of the downstream port is 4k aligned. a. all 1k aligned, the "min_align" should be arch_align b. one is 4k aligned, the "min_align" should be 4k Case 3: this is the same as before The final result of "min_align" in these three cases are all the biggest one of upstream/downstream/arch alignment. So the algorithm could be changed to calculate the biggest one of the three. Personal Conclusion ============================================================================ I think Bjorn's patch works. Will test on powernv platform and give the result. Last but not the least, please fix me, if I am not correct. :-)
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@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. >> >>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. I think this caes will not happen. As I analysised in previous letter. During the list_for_each_entry loop, the resources could be in two cased: 1. bridge I/O window(IORESOURCE_STARTALIGN); 2. device I/O BAR(IORESOURCE_SIZEALIGN). Bridge I/O window is 64k aligned, as the platform required. Device I/O BAR is less than 256 bytes according to the specification. So the 96k size is not possible. Please fix me, if I am not correct. > >>@@ -781,9 +781,6 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, >> } >> } >> >>- if (min_align > io_align) >>- min_align = io_align; >>- >> size0 = calculate_iosize(size, min_size, size1, >> resource_size(b_res), min_align); >> if (children_add_size > add_size) >> > >Thanks, >Gavin
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@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. > > > >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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 06, 2013 at 02:15:34PM +0800, Wei Yang 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@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. > > > >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. > > > > Thanks all for the comments, this makes me re-consider the cases. Let me do a > summary. Maybe I misunderstand the idea, please fix me~ > > Requirement from PCI Spec > ============================================================================ > 1. I/O BAR for non-bridge PCI devices are limited to 256 bytes > 2. Most I/O window for PCI bridge is 4k aligned > 3. Some bridge support 1k aligned I/O window > > Ancient time -- when 1k aligned I/O window is not there > ============================================================================ > The alignment is 4k in all cases. (As it is hard coded.) > > For devices under this bridge, I/O BAR is less then 256. > > For downstream port, I/O window is 4k aligned, since the IORESOURCE_STARTALIGN > is set. This means even the downstream port connects other bridge, the > alignment is still 4k. Right. > Middle Age -- when 1k aligned I/O window is introduced > ============================================================================ > This introduce two other cases: > 1. All downstream port is 1k aligned > 2. One of the downstream port is 4k aligned. > > Case 1: the "min_align" should be set to 1k. This could save some I/O resource. > Case 2: the "min_align" should be set to 4k, even itself anounced could support > 1k alignment. > ^--- Fix me, if not correct. Right. > The "min_align" could be set to other value? Previously, I thought it could > be, for example 2k. Now I think no, the list_for_each_entry loop will iterate > on two kinds of resources: 1. Device I/O BAR; 2. Bridge I/O window. > > Device I/O BAR is less then 256 bytes, it won't contribut to the alignment. > Bridge I/O window will be 1k or 4k aligned. > > This means only two possible value for "min_align": 1k or 4k. > ^--- Fix me, if not correct. Right. > Back to Yinghai's commit(fd5913411), the "min_align" is set to 1k at the > beginning. During the list_for_each_entry loop, (align > min_align) is true > means align is 4k. > > And this (min_align > 4096) will never be true, since at most "min_align" is > 4k. So, I think, this comparison could be removed in commit(fd5913411). > ^--- Fix me, if not correct. Right. I think Yinghai was considering the case of an I/O BAR larger than 4K. But that's illegal per spec, and I think we would want to keep the larger alignment even if it were legal. > Present Age -- when architecture require specific alignment for bridge window > ============================================================================ > This introduce 3 cases: > 1. 1k < 4k < arch_align > 2. 1k < arch_align < 4k > 3. arch_align < 1k < 4k > > Case 1: the "min_align" should be arch_align. > Case 2: this is a little complicated. downstream port could be all 1k aligned > or one of the downstream port is 4k aligned. > a. all 1k aligned, the "min_align" should be arch_align > b. one is 4k aligned, the "min_align" should be 4k > Case 3: this is the same as before > > The final result of "min_align" in these three cases are all the biggest one > of upstream/downstream/arch alignment. So the algorithm could be changed to > calculate the biggest one of the three. Right. > Personal Conclusion > ============================================================================ > I think Bjorn's patch works. > Will test on powernv platform and give the result. Great, let me know what happens. 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 02:26:24PM +0800, Wei Yang wrote: > 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@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. > >> > >>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. > > I think this caes will not happen. > > As I analysised in previous letter. During the list_for_each_entry loop, the > resources could be in two cased: 1. bridge I/O window(IORESOURCE_STARTALIGN); > 2. device I/O BAR(IORESOURCE_SIZEALIGN). > > Bridge I/O window is 64k aligned, as the platform required. > Device I/O BAR is less than 256 bytes according to the specification. > > So the 96k size is not possible. It's not possible because of the power-of-2 size requirement. The spec does say I/O BARs should be 256 bytes or smaller, but one could imagine a BAR that violated that requirement. But the power-of-2 requirement is more fundamental because of the way BARs are sized (low-order bits are hardwired to zero, and the number of hardwired bits determines the size) and PCI really cannot support a non-power-of-2 sized BAR at all. > Please fix me, if I am not correct. > > > > >>@@ -781,9 +781,6 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > >> } > >> } > >> > >>- if (min_align > io_align) > >>- min_align = io_align; > >>- > >> size0 = calculate_iosize(size, min_size, size1, > >> resource_size(b_res), min_align); > >> if (children_add_size > add_size) > >> > > > >Thanks, > >Gavin > > -- > Richard Yang > Help you, Help me > -- 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:39:29AM -0600, Bjorn Helgaas wrote: >> Personal Conclusion >> ============================================================================ >> I think Bjorn's patch works. >> Will test on powernv platform and give the result. > >Great, let me know what happens. In both case, with/with out your patch, the assignment result is the same. Below is the /proc/ioports file. 00010000-0080ffff : /io-hub@3efe00000000/pciex@3efe00080000 00010000-00010fff : Legacy IO 00020000-0003ffff : PCI Bus 0000:01 00020000-0003ffff : PCI Bus 0000:02 00020000-0002ffff : PCI Bus 0000:40 00030000-0003ffff : PCI Bus 0000:a0 00820000-0101ffff : /io-hub@3efe00000000/pciex@3efe00090000 00820000-00820fff : Legacy IO 00830000-0083ffff : PCI Bus 0001:01 01030000-0182ffff : /io-hub@3efe00000000/pciex@3efe000a0000 01030000-01030fff : Legacy IO 01040000-0104ffff : PCI Bus 0002:01 01840000-0203ffff : /io-hub@3efe00000000/pciex@3efe000b0000 01840000-01840fff : Legacy IO 01850000-0185ffff : PCI Bus 0003:01 02050000-0284ffff : /io-hub@3efe00000000/pciex@3efe000c0000 02050000-02050fff : Legacy IO 02060000-0206ffff : PCI Bus 0004:01 02860000-0305ffff : /io-hub@3efe00000000/pciex@3efe000d0000 02860000-02860fff : Legacy IO 02870000-0287ffff : PCI Bus 0005:01 The arch_align is 0x10000. When sizing for Bus 0000:01, its child has a I/O bridge window with size 0x20000/alignment 0x10000. And the result [0x20000,0x3ffff] looks like size 0x20000/alignment 0x20000 or size 0x20000/alignment 0x10000. But I still think the alignment is 0x10000. > >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 11:34:10PM +0800, Wei Yang wrote: > On Tue, Aug 06, 2013 at 07:39:29AM -0600, Bjorn Helgaas wrote: > >> Personal Conclusion > >> ============================================================================ > >> I think Bjorn's patch works. > >> Will test on powernv platform and give the result. > > > >Great, let me know what happens. > > In both case, with/with out your patch, the assignment result is the same. > Below is the /proc/ioports file. > > 00010000-0080ffff : /io-hub@3efe00000000/pciex@3efe00080000 > 00010000-00010fff : Legacy IO > 00020000-0003ffff : PCI Bus 0000:01 > 00020000-0003ffff : PCI Bus 0000:02 > 00020000-0002ffff : PCI Bus 0000:40 > 00030000-0003ffff : PCI Bus 0000:a0 > 00820000-0101ffff : /io-hub@3efe00000000/pciex@3efe00090000 > 00820000-00820fff : Legacy IO > 00830000-0083ffff : PCI Bus 0001:01 > 01030000-0182ffff : /io-hub@3efe00000000/pciex@3efe000a0000 > 01030000-01030fff : Legacy IO > 01040000-0104ffff : PCI Bus 0002:01 > 01840000-0203ffff : /io-hub@3efe00000000/pciex@3efe000b0000 > 01840000-01840fff : Legacy IO > 01850000-0185ffff : PCI Bus 0003:01 > 02050000-0284ffff : /io-hub@3efe00000000/pciex@3efe000c0000 > 02050000-02050fff : Legacy IO > 02060000-0206ffff : PCI Bus 0004:01 > 02860000-0305ffff : /io-hub@3efe00000000/pciex@3efe000d0000 > 02860000-02860fff : Legacy IO > 02870000-0287ffff : PCI Bus 0005:01 > > The arch_align is 0x10000. When sizing for Bus 0000:01, its child has a I/O > bridge window with size 0x20000/alignment 0x10000. And the result > [0x20000,0x3ffff] looks like size 0x20000/alignment 0x20000 or size > 0x20000/alignment 0x10000. Obviously, an address with alignment of 0x20000 is *also* aligned to 0x10000. > But I still think the alignment is 0x10000. Yes, as it should be. These are all bridge windows, which never have to be aligned at more than 1K (if supported), 4K, or the arch alignment, whichever is largest. I *think* you're saying that the patch works correctly. 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 11:58:56AM -0600, Bjorn Helgaas wrote: >On Tue, Aug 06, 2013 at 11:34:10PM +0800, Wei Yang wrote: >> On Tue, Aug 06, 2013 at 07:39:29AM -0600, Bjorn Helgaas wrote: >> >> Personal Conclusion >> >> ============================================================================ >> >> I think Bjorn's patch works. >> >> Will test on powernv platform and give the result. >> > >> >Great, let me know what happens. >> >> In both case, with/with out your patch, the assignment result is the same. >> Below is the /proc/ioports file. >> >> 00010000-0080ffff : /io-hub@3efe00000000/pciex@3efe00080000 >> 00010000-00010fff : Legacy IO >> 00020000-0003ffff : PCI Bus 0000:01 >> 00020000-0003ffff : PCI Bus 0000:02 >> 00020000-0002ffff : PCI Bus 0000:40 >> 00030000-0003ffff : PCI Bus 0000:a0 >> 00820000-0101ffff : /io-hub@3efe00000000/pciex@3efe00090000 >> 00820000-00820fff : Legacy IO >> 00830000-0083ffff : PCI Bus 0001:01 >> 01030000-0182ffff : /io-hub@3efe00000000/pciex@3efe000a0000 >> 01030000-01030fff : Legacy IO >> 01040000-0104ffff : PCI Bus 0002:01 >> 01840000-0203ffff : /io-hub@3efe00000000/pciex@3efe000b0000 >> 01840000-01840fff : Legacy IO >> 01850000-0185ffff : PCI Bus 0003:01 >> 02050000-0284ffff : /io-hub@3efe00000000/pciex@3efe000c0000 >> 02050000-02050fff : Legacy IO >> 02060000-0206ffff : PCI Bus 0004:01 >> 02860000-0305ffff : /io-hub@3efe00000000/pciex@3efe000d0000 >> 02860000-02860fff : Legacy IO >> 02870000-0287ffff : PCI Bus 0005:01 >> >> The arch_align is 0x10000. When sizing for Bus 0000:01, its child has a I/O >> bridge window with size 0x20000/alignment 0x10000. And the result >> [0x20000,0x3ffff] looks like size 0x20000/alignment 0x20000 or size >> 0x20000/alignment 0x10000. > >Obviously, an address with alignment of 0x20000 is *also* aligned to >0x10000. > >> But I still think the alignment is 0x10000. > >Yes, as it should be. These are all bridge windows, which never have to be >aligned at more than 1K (if supported), 4K, or the arch alignment, >whichever is largest. I add some printk which shows the alignment is 0x10000 instead of 0x20000. > >I *think* you're saying that the patch works correctly. Yes. > >Bjorn
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; @@ -781,9 +781,6 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, } } - if (min_align > io_align) - min_align = io_align; - size0 = calculate_iosize(size, min_size, size1, resource_size(b_res), min_align); if (children_add_size > add_size)