History

x86/PCI: Revert "x86/PCI: Clip only host bridge windows for E820 regions"

This reverts commit 4c5e242d3e93. Prior to 4c5e242d3e93 ("x86/PCI: Clip only host bridge windows for E820 regions"), E820 regions did not affect PCI host bridge windows. We only looked at E820 regions and avoided them when allocating new MMIO space. If firmware PCI bridge window and BAR assignments used E820 regions, we left them alone. After 4c5e242d3e93, we removed E820 regions from the PCI host bridge windows before looking at BARs, so firmware assignments in E820 regions looked like errors, and we moved things around to fit in the space left (if any) after removing the E820 regions. This unnecessary BAR reassignment broke several machines. Guilherme reported that Steam Deck fails to boot after 4c5e242d3e93. We clipped the window that contained most 32-bit BARs: BIOS-e820: [mem 0x00000000a0000000-0x00000000a00fffff] reserved acpi PNP0A08:00: clipped [mem 0x80000000-0xf7ffffff window] to [mem 0xa0100000-0xf7ffffff window] for e820 entry [mem 0xa0000000-0xa00fffff] which forced us to reassign all those BARs, for example, this NVMe BAR: pci 0000:00:01.2: PCI bridge to [bus 01] pci 0000:00:01.2: bridge window [mem 0x80600000-0x806fffff] pci 0000:01:00.0: BAR 0: [mem 0x80600000-0x80603fff 64bit] pci 0000:00:01.2: can't claim window [mem 0x80600000-0x806fffff]: no compatible bridge window pci 0000:01:00.0: can't claim BAR 0 [mem 0x80600000-0x80603fff 64bit]: no compatible bridge window pci 0000:00:01.2: bridge window: assigned [mem 0xa0100000-0xa01fffff] pci 0000:01:00.0: BAR 0: assigned [mem 0xa0100000-0xa0103fff 64bit] All the reassignments were successful, so the devices should have been functional at the new addresses, but some were not. Andy reported a similar failure on an Intel MID platform. Benjamin reported a similar failure on a VMWare Fusion VM. Note: this is not a clean revert; this revert keeps the later change to make the clipping dependent on a new pci_use_e820 bool, moving the checking of this bool to arch_remove_reservations(). [bhelgaas: commit log, add more reporters and testers] BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216109 Reported-by: Guilherme G. Piccoli <gpiccoli@igalia.com> Reported-by: Andy Shevchenko <andy.shevchenko@gmail.com> Reported-by: Benjamin Coddington <bcodding@redhat.com> Reported-by: Jongman Heo <jongman.heo@gmail.com> Fixes: 4c5e242d3e93 ("x86/PCI: Clip only host bridge windows for E820 regions") Link: https://lore.kernel.org/r/20220612144325.85366-1-hdegoede@redhat.com Tested-by: Guilherme G. Piccoli <gpiccoli@igalia.com> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Tested-by: Benjamin Coddington <bcodding@redhat.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Hans de Goede committed 4 years ago

Commit a2b36ff1 parent f2906aa4 files · +18 −17Browse files

Unified · Split

4 files
  1. arch/x86/include/asm/e820/api.h +0 −5
  2. arch/x86/include/asm/pci_x86.h +8 −0
  3. arch/x86/kernel/resource.c +9 −5
  4. arch/x86/pci/acpi.c +1 −7
arch/x86/include/asm/e820/api.h+0−5
@@ -4,9 +4,6 @@ · 3 unmodified lines
4 4
5#include <asm/e820/types.h>5#include <asm/e820/types.h>
6 6
7struct device;
8struct resource;
9
10extern struct e820_table *e820_table;7extern struct e820_table *e820_table;
11extern struct e820_table *e820_table_kexec;8extern struct e820_table *e820_table_kexec;
12extern struct e820_table *e820_table_firmware;9extern struct e820_table *e820_table_firmware;
@@ -46,8 +43,6 @@ extern void e820__register_nosave_regions(unsigned long limit_pfn); · 33 unmodified lines
46 43
47extern int e820__get_entry_type(u64 start, u64 end);44extern int e820__get_entry_type(u64 start, u64 end);
48 45
49extern void remove_e820_regions(struct device *dev, struct resource *avail);
50
51/*46/*
52 * Returns true iff the specified range [start,end) is completely contained inside47 * Returns true iff the specified range [start,end) is completely contained inside
53 * the ISA region.48 * the ISA region.
arch/x86/include/asm/pci_x86.h+8−0
@@ -69,6 +69,8 @@ void pcibios_scan_specific_bus(int busn); · 68 unmodified lines
69 69
70/* pci-irq.c */70/* pci-irq.c */
71 71
72struct pci_dev;
73
72struct irq_info {74struct irq_info {
73 u8 bus, devfn; /* Bus, device and function */75 u8 bus, devfn; /* Bus, device and function */
74 struct {76 struct {
@@ -246,3 +248,9 @@ static inline void mmio_config_writel(void __iomem *pos, u32 val) · 171 unmodified lines
246# define x86_default_pci_init_irq NULL248# define x86_default_pci_init_irq NULL
247# define x86_default_pci_fixup_irqs NULL249# define x86_default_pci_fixup_irqs NULL
248#endif250#endif
251
252#if defined(CONFIG_PCI) && defined(CONFIG_ACPI)
253extern bool pci_use_e820;
254#else
255#define pci_use_e820 false
256#endif
arch/x86/kernel/resource.c+9−5
@@ -1,7 +1,8 @@
1// SPDX-License-Identifier: GPL-2.01// SPDX-License-Identifier: GPL-2.0
2#include <linux/dev_printk.h>
3#include <linux/ioport.h>2#include <linux/ioport.h>
3#include <linux/printk.h>
4#include <asm/e820/api.h>4#include <asm/e820/api.h>
5#include <asm/pci_x86.h>
5 6
6static void resource_clip(struct resource *res, resource_size_t start,7static void resource_clip(struct resource *res, resource_size_t start,
7 resource_size_t end)8 resource_size_t end)
@@ -24,14 +25,14 @@ static void resource_clip(struct resource *res, resource_size_t start, · 16 unmodified lines
24 res->start = end + 1;25 res->start = end + 1;
25}26}
26 27
27void remove_e820_regions(struct device *dev, struct resource *avail)28static void remove_e820_regions(struct resource *avail)
28{29{
29 int i;30 int i;
30 struct e820_entry *entry;31 struct e820_entry *entry;
31 u64 e820_start, e820_end;32 u64 e820_start, e820_end;
32 struct resource orig = *avail;33 struct resource orig = *avail;
33 34
34 if (!(avail->flags & IORESOURCE_MEM))35 if (!pci_use_e820)
35 return;36 return;
36 37
37 for (i = 0; i < e820_table->nr_entries; i++) {38 for (i = 0; i < e820_table->nr_entries; i++) {
@@ -41,7 +42,7 @@ void remove_e820_regions(struct device *dev, struct resource *avail) · 3 unmodified lines
41 42
42 resource_clip(avail, e820_start, e820_end);43 resource_clip(avail, e820_start, e820_end);
43 if (orig.start != avail->start || orig.end != avail->end) {44 if (orig.start != avail->start || orig.end != avail->end) {
44 dev_info(dev, "clipped %pR to %pR for e820 entry [mem %#010Lx-%#010Lx]\n",45 pr_info("clipped %pR to %pR for e820 entry [mem %#010Lx-%#010Lx]\n",
45 &orig, avail, e820_start, e820_end);46 &orig, avail, e820_start, e820_end);
46 orig = *avail;47 orig = *avail;
47 }48 }
@@ -55,6 +56,9 @@ void arch_remove_reservations(struct resource *avail) · 7 unmodified lines
55 * the low 1MB unconditionally, as this area is needed for some ISA56 * the low 1MB unconditionally, as this area is needed for some ISA
56 * cards requiring a memory range, e.g. the i82365 PCMCIA controller.57 * cards requiring a memory range, e.g. the i82365 PCMCIA controller.
57 */58 */
58 if (avail->flags & IORESOURCE_MEM)59 if (avail->flags & IORESOURCE_MEM) {
59 resource_clip(avail, BIOS_ROM_BASE, BIOS_ROM_END);60 resource_clip(avail, BIOS_ROM_BASE, BIOS_ROM_END);
61
62 remove_e820_regions(avail);
63 }
60}64}
arch/x86/pci/acpi.c+1−7
@@ -8,7 +8,6 @@ · 7 unmodified lines
8#include <linux/pci-acpi.h>8#include <linux/pci-acpi.h>
9#include <asm/numa.h>9#include <asm/numa.h>
10#include <asm/pci_x86.h>10#include <asm/pci_x86.h>
11#include <asm/e820/api.h>
12 11
13struct pci_root_info {12struct pci_root_info {
14 struct acpi_pci_root_info common;13 struct acpi_pci_root_info common;
@@ -20,7 +19,7 @@ struct pci_root_info { · 5 unmodified lines
20#endif19#endif
21};20};
22 21
23static bool pci_use_e820 = true;22bool pci_use_e820 = true;
24static bool pci_use_crs = true;23static bool pci_use_crs = true;
25static bool pci_ignore_seg;24static bool pci_ignore_seg;
26 25
@@ -387,11 +386,6 @@ static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) · 360 unmodified lines
387 386
388 status = acpi_pci_probe_root_resources(ci);387 status = acpi_pci_probe_root_resources(ci);
389 388
390 if (pci_use_e820) {
391 resource_list_for_each_entry(entry, &ci->resources)
392 remove_e820_regions(&device->dev, entry->res);
393 }
394
395 if (pci_use_crs) {389 if (pci_use_crs) {
396 resource_list_for_each_entry_safe(entry, tmp, &ci->resources)390 resource_list_for_each_entry_safe(entry, tmp, &ci->resources)
397 if (resource_is_pcicfg_ioport(entry->res))391 if (resource_is_pcicfg_ioport(entry->res))