Re: [PATCH] Fix coalescing host bridge windows in arch/x86/pci/acpi.c

[Alexey Neyman <stilor@att.net>]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

[Patch ping #4...]

On Tuesday, September 24, 2013 11:47:21 AM Yijing Wang wrote:
> On 2013/9/23 14:15, Alexey Neyman wrote:
> > [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 <stilor@att.net>
> 
> 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.

[-- Attachment #2: acpi.c.diff --]
[-- Type: text/x-patch, Size: 722 bytes --]

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index b30e937..7fb24e5 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -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;
 			}
 		}
 	}