[PATCH 3/3] x86/PCI: Preserve host bridge windows completely covered by E820

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>

Many folks have reported PCI devices not working.  It could affect any
device, but most reports are for Thunderbolt controllers on Lenovo Yoga and
Clevo Barebone laptops and the touchpad on Lenovo IdeaPads.

In every report, a region in the E820 table entirely encloses a PCI host
bridge window from _CRS, and because of 4dc2287c1805 ("x86: avoid E820
regions when allocating address space"), we ignore the entire window,
preventing us from assigning space to PCI devices.

For example, the dmesg log [2] from bug report [1] shows:

  BIOS-e820: [mem 0x000000004bc50000-0x00000000cfffffff] reserved
  pci_bus 0000:00: root bus resource [mem 0x65400000-0xbfffffff window]
  pci 0000:00:15.0: BAR 0: no space for [mem size 0x00001000 64bit]

The efi=debug dmesg log [3] from the same report shows the EFI memory map
entries that created the E820 map:

  efi: mem47: [Reserved |   |WB|WT|WC|UC] range=[0x4bc50000-0x5fffffff]
  efi: mem48: [Reserved |   |WB|  |  |UC] range=[0x60000000-0x60ffffff]
  efi: mem49: [Reserved |   |  |  |  |  ] range=[0x61000000-0x653fffff]
  efi: mem50: [MMIO     |RUN|  |  |  |UC] range=[0x65400000-0xcfffffff]

4dc2287c1805 ("x86: avoid E820 regions when allocating address space")
works around issues where _CRS contains non-window address space that can't
be used for PCI devices.  It does this by removing E820 regions from host
bridge windows.  But in these reports, the E820 region covers the entire
window, so 4dc2287c1805 makes it completely unusable.

Per UEFI v2.8, sec 7.2, the EfiMemoryMappedIO type means:

  Used by system firmware to request that a memory-mapped IO region be
  mapped by the OS to a virtual address so it can be accessed by EFI
  runtime services.

A host bridge window is definitely a memory-mapped IO region, and EFI
runtime services may need to access it, so I don't think we can argue that
this is a firmware defect.

Instead, change the 4dc2287c1805 strategy so it only removes E820 regions
when they overlap *part* of a host bridge window on the assumption that a
partial overlap is really register space, not part of the window proper.

If an E820 region covers the entire window from _CRS, assume the _CRS
window is correct and do nothing.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1868899
[2] https://bugzilla.redhat.com/attachment.cgi?id=1711424
[3] https://bugzilla.redhat.com/attachment.cgi?id=1861407

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=206459
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214259
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1868899
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1871793
BugLink: https://bugs.launchpad.net/bugs/1878279
BugLink: https://bugs.launchpad.net/bugs/1931715
BugLink: https://bugs.launchpad.net/bugs/1932069
BugLink: https://bugs.launchpad.net/bugs/1921649
Fixes: 4dc2287c1805 ("x86: avoid E820 regions when allocating address space")
Link: https://lore.kernel.org/r/20220228105259.230903-1-hdegoede@xxxxxxxxxx
Based-on-patch-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Reported-by: Benoit Grégoire <benoitg@xxxxxxxx>   # BZ 206459
Reported-by: wse@xxxxxxxxxxxxxxxxxxx              # BZ 214259
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
---
 arch/x86/kernel/resource.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/kernel/resource.c b/arch/x86/kernel/resource.c
index 7378ea146976..405f0af53e3d 100644
--- a/arch/x86/kernel/resource.c
+++ b/arch/x86/kernel/resource.c
@@ -39,6 +39,17 @@ void remove_e820_regions(struct device *dev, struct resource *avail)
 		e820_start = entry->addr;
 		e820_end = entry->addr + entry->size - 1;
 
+		/*
+		 * If an E820 entry covers just part of the resource, we
+		 * assume E820 is telling us about something like host
+		 * bridge register space that is unavailable for PCI
+		 * devices.  But if it covers the *entire* resource, it's
+		 * more likely just telling us that this is MMIO space, and
+		 * that doesn't need to be removed.
+		 */
+		if (e820_start <= avail->start && avail->end <= e820_end)
+			continue;
+
 		resource_clip(avail, e820_start, e820_end);
 		if (orig.start != avail->start || orig.end != avail->end) {
 			dev_info(dev, "clipped %pR to %pR for e820 entry [mem %#010Lx-%#010Lx]\n",
-- 
2.25.1




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux