Skip to contentSkip to conversation

Fix coalescing host bridge windows in arch/x86/pci/acpi.c

PATCH In review Alexey Neyman 3 participants 8 messages Sep 23, 2013 – Oct 9, 2013

Subsystems (3)
x86 ACPI PCI
lore
Share & tools
lore

Alexey Neyman

author
#0 lore raw

PATCH 1/1 Fix coalescing host bridge windows in arch/x86/pci/acpi.c

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3. The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2 (regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Signed-off-by Alexey Neyman <[email protected]>
arch/x86/pci/acpi.c +4 -4
index b30e937..7fb24e5 100644
@@ -354,12 +354,12 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
 			 * the kernel resource tree doesn't allow overlaps.
 			 */
 			if (resource_overlaps(res1, res2)) {
-				res1->start = min(res1->start, res2->start);
-				res1->end = max(res1->end, res2->end);
+				res2->start = min(res1->start, res2->start);
+				res2->end = max(res1->end, res2->end);
 				dev_info(&info->bridge->dev,
 					 "host bridge window expanded to %pR; %pR ignored\n",
-					 res1, res2);
-				res2->flags = 0;
+					 res2, res1);
+				res1->flags = 0;
 			}
 		}
 	}
#1 lore raw
Quoted 26 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3. The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

Quoted 7 lines

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2 (regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Signed-off-by Alexey Neyman <[email protected]>

It looks fine to me, but I have no platform to test it. :)

Signature · 2 lines
Thanks!
Yijing
author
#2 lore raw
Quoted 34 lines · Yijing Wang
Quoted 33 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3.
The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2
(regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Quoted 3 lines
Quoted 1 line
Signed-off-by Alexey Neyman <[email protected]>

It looks fine to me, but I have no platform to test it. :)

Thanks for a review. Could anybody push it into the tree? :) For convenience, patch attached again.

Regards,
Alexey.

arch/x86/pci/acpi.c +4 -4
index b30e937..7fb24e5 100644
@@ -354,12 +354,12 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
 			 * the kernel resource tree doesn't allow overlaps.
 			 */
 			if (resource_overlaps(res1, res2)) {
-				res1->start = min(res1->start, res2->start);
-				res1->end = max(res1->end, res2->end);
+				res2->start = min(res1->start, res2->start);
+				res2->end = max(res1->end, res2->end);
 				dev_info(&info->bridge->dev,
 					 "host bridge window expanded to %pR; %pR ignored\n",
-					 res1, res2);
-				res2->flags = 0;
+					 res2, res1);
+				res1->flags = 0;
 			}
 		}
 	}
author
#3 lore raw
In reply to Yijing Wang

[Patch ping #4...]

Quoted 34 lines · Yijing Wang
Quoted 33 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3.
The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2
(regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Quoted 3 lines
Quoted 1 line
Signed-off-by Alexey Neyman <[email protected]>

It looks fine to me, but I have no platform to test it. :)

Thanks for a review. Could anybody push it into the tree? :) For convenience, patch attached again.

Regards,
Alexey.

arch/x86/pci/acpi.c +4 -4
index b30e937..7fb24e5 100644
@@ -354,12 +354,12 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
 			 * the kernel resource tree doesn't allow overlaps.
 			 */
 			if (resource_overlaps(res1, res2)) {
-				res1->start = min(res1->start, res2->start);
-				res1->end = max(res1->end, res2->end);
+				res2->start = min(res1->start, res2->start);
+				res2->end = max(res1->end, res2->end);
 				dev_info(&info->bridge->dev,
 					 "host bridge window expanded to %pR; %pR ignored\n",
-					 res1, res2);
-				res2->flags = 0;
+					 res2, res1);
+				res1->flags = 0;
 			}
 		}
 	}
maintainer
#4 lore raw
In reply to Alexey Neyman
Quoted 26 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3. The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

Quoted 6 lines

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2 (regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Signed-off-by Alexey Neyman <[email protected]>

Can you please open a report at http://bugzilla.kernel.org, assign it to drivers/pci, and attach a complete dmesg log?

Thanks,
Bjorn

author
#5 lore raw
Quoted 33 lines · Bjorn Helgaas
Quoted 32 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3. The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2 (regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Quoted 4 lines
Quoted 1 line
Signed-off-by Alexey Neyman <[email protected]>

Can you please open a report at http://bugzilla.kernel.org, assign it to drivers/pci, and attach a complete dmesg log?

https://bugzilla.kernel.org/show_bug.cgi?id=62511

I am not at liberty to disclose full hardware configuration of the board, as it is a proprietary prototype board not in production. I've provided the relevant messages from dmesg though.

Regards,
Alexey.

author
#6 lore raw

[Patch ping #5]

Quoted 37 lines · Alexey Neyman
Quoted 36 lines · Bjorn Helgaas
Quoted 34 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3. The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2 (regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Signed-off-by Alexey Neyman <[email protected]>
Quoted 8 lines
Quoted 2 lines

Can you please open a report at http://bugzilla.kernel.org, assign it to drivers/pci, and attach a complete dmesg log?

https://bugzilla.kernel.org/show_bug.cgi?id=62511

I am not at liberty to disclose full hardware configuration of the board, as it is a proprietary prototype board not in production. I've provided the relevant messages from dmesg though.

Could anybody review/commit?

Regards,
Alexey.

maintainer
#7 lore raw
In reply to Alexey Neyman

[+cc linux-pci]

Quoted 26 lines · Alexey Neyman

[Resending due to no response to the original message in a week]

Hi all,

I have a board with a BIOS bug that reports the following I/O port regions in _CRS on one of the host bridges:

0x0000-0x03af // #0
0x03e0-0x0cf7 // #1
0x03b0-0x03bb // #2
0x03c0-0x03df // #3
0x0000-0xdfff // #4
0xf000-0xffff // #5

Obviously, region number #4 is erroneous as it overlaps with regions #0..3. The code in coalesce_windows() in arch/x86/pci/acpi.c attempts to recover from such kind of BIOS bugs by merging the overlapping regions. Current code expands region #0 to 0x0000-0xdffff and makes region #4 ignored. As a result, overlap of the expanded region #0 with regions #1..3 remains undetected (as the inner loop already compared them with region #0). As a result, regions #1..3 are inserted into the resource tree even though they overlap with adjusted region #0 - which later results in resource conflicts for PCI devices with IO ports in one of those regions (e.g., for an PCI IDE controller in legacy mode - which has port 0x3f6). The kernel then refuses to initialize these devices.

Quoted 6 lines

The fix: instead of expanding res1 and ignoring res2, do the opposite. The res2 window is yet to be compared against all windows between res1 and res2 (regions #1..3 in the above example), so the resulting resource map will include just the expanded region - and will ignore any overlapping ones.

Signed-off-by Alexey Neyman <[email protected]>

I added the bugzilla reference (thanks for that) and applied the following patch to my pci/misc branch for v3.13.

Sorry this took so long; part of the reason was that it wasn't on linux-pci, so it didn't show up in the PCI patchwork. I also added a MAINTAINERS patch so in the future, get_maintainers.pl will include linux-pci and me as co-maintainer of arch/x86/pci/*. Also, it helps if you include the patch in-line rather than as an attachment because I have to manually combine the in-line changelog with the attached patch.

Thanks for the fix and your persistence!

Bjorn

x86/PCI: Coalesce multiple overlapping host bridge windows

From: Alexey Neyman <[email protected]>

Previously we coalesced windows by expanding the first overlapping one and making the second invalid. But we never look at the expanded first window again, so we fail to notice other windows that overlap it. For example, we coalesced these:

  [io  0x0000-0x03af] // #0
  [io  0x03e0-0x0cf7] // #1
  [io  0x0000-0xdfff] // #2

into these, which still overlap:

[io 0x0000-0xdfff] // #0
[io 0x03e0-0x0cf7] // #1

The fix is to expand the *second* overlapping resource and ignore the first, so we get this instead with no overlaps:

[io 0x0000-0xdfff] // #2

[bhelgaas: changelog]
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62511

Signed-off-by Alexey Neyman <[email protected]>
Signed-off-by Bjorn Helgaas <[email protected]>
---
 arch/x86/pci/acpi.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/pci/acpi.c +4 -4
index b30e937..7fb24e5 100644
@@ -354,12 +354,12 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
 			 * the kernel resource tree doesn't allow overlaps.
 			 */
 			if (resource_overlaps(res1, res2)) {
-				res1->start = min(res1->start, res2->start);
-				res1->end = max(res1->end, res2->end);
+				res2->start = min(res1->start, res2->start);
+				res2->end = max(res1->end, res2->end);
 				dev_info(&info->bridge->dev,
 					 "host bridge window expanded to %pR; %pR ignored\n",
-					 res1, res2);
-				res2->flags = 0;
+					 res2, res1);
+				res1->flags = 0;
 			}
 		}
 	}