All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Borislav Petkov <bp@alien8.de>, "H . Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@redhat.com>
Cc: "Mika Westerberg" <mika.westerberg@linux.intel.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Myron Stowe" <myron.stowe@redhat.com>,
	"Juha-Pekka Heikkila" <juhapekka.heikkila@gmail.com>,
	"Benoit Grégoire" <benoitg@coeus.ca>,
	"Hui Wang" <hui.wang@canonical.com>,
	"Kai-Heng Feng" <kai.heng.feng@canonical.com>,
	linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	"Bjorn Helgaas" <bhelgaas@google.com>
Subject: [PATCH 2/3] x86/PCI: Log host bridge window clipping for E820 regions
Date: Thu,  3 Mar 2022 21:51:10 -0600	[thread overview]
Message-ID: <20220304035110.988712-3-helgaas@kernel.org> (raw)
In-Reply-To: <20220304035110.988712-1-helgaas@kernel.org>

From: Bjorn Helgaas <bhelgaas@google.com>

ACPI firmware advertises PCI host bridge resources via PNP0A03 _CRS
methods.  Some BIOSes include non-window address space in _CRS, and if we
allocate that non-window space for PCI devices, they don't work.

4dc2287c1805 ("x86: avoid E820 regions when allocating address space")
works around this issue by clipping out any regions mentioned in the E820
table in the allocate_resource() path, but the implementation has several
issues:

  - The clipping is done for *all* allocations, not just those for PCI
    address space,

  - The clipping is done at each allocation instead of being done once when
    setting up the host bridge windows, and

  - The host bridge windows logged in dmesg do not reflect the clipping,
    and in fact there is *no* indication in dmesg, which complicates
    debugging.

Rework the implementation so we only clip PCI host bridge windows, we do it
once when setting them up, we a log message when a window is clipped, and
we reflect the clip when printing the host bridge windows.

I intend this only to improve the logging, not to fix any issues.

Example output changes:

    BIOS-e820: [mem 0x00000000b0000000-0x00000000c00fffff] reserved
  + acpi PNP0A08:00: clipped [mem 0xc0000000-0xfebfffff window] to [mem 0xc0100000-0xfebfffff window] for e820 entry [mem 0xb0000000-0xc00fffff]
  - pci_bus 0000:00: root bus resource [mem 0xc0000000-0xfebfffff window]
  + pci_bus 0000:00: root bus resource [mem 0xc0100000-0xfebfffff window]

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 arch/x86/include/asm/e820/api.h |  5 +++++
 arch/x86/kernel/resource.c      | 17 ++++++++++++-----
 arch/x86/pci/acpi.c             |  5 +++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
index e8f58ddd06d9..5a39ed59b6db 100644
--- a/arch/x86/include/asm/e820/api.h
+++ b/arch/x86/include/asm/e820/api.h
@@ -4,6 +4,9 @@
 
 #include <asm/e820/types.h>
 
+struct device;
+struct resource;
+
 extern struct e820_table *e820_table;
 extern struct e820_table *e820_table_kexec;
 extern struct e820_table *e820_table_firmware;
@@ -43,6 +46,8 @@ extern void e820__register_nosave_regions(unsigned long limit_pfn);
 
 extern int  e820__get_entry_type(u64 start, u64 end);
 
+extern void remove_e820_regions(struct device *dev, struct resource *avail);
+
 /*
  * Returns true iff the specified range [start,end) is completely contained inside
  * the ISA region.
diff --git a/arch/x86/kernel/resource.c b/arch/x86/kernel/resource.c
index 8ffe68437744..7378ea146976 100644
--- a/arch/x86/kernel/resource.c
+++ b/arch/x86/kernel/resource.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/dev_printk.h>
 #include <linux/ioport.h>
 #include <asm/e820/api.h>
 
@@ -23,18 +24,27 @@ static void resource_clip(struct resource *res, resource_size_t start,
 		res->start = end + 1;
 }
 
-static void remove_e820_regions(struct resource *avail)
+void remove_e820_regions(struct device *dev, struct resource *avail)
 {
+	struct resource orig = *avail;
 	int i;
 	struct e820_entry *entry;
 	u64 e820_start, e820_end;
 
+	if (!(avail->flags & IORESOURCE_MEM))
+		return;
+
 	for (i = 0; i < e820_table->nr_entries; i++) {
 		entry = &e820_table->entries[i];
 		e820_start = entry->addr;
 		e820_end = entry->addr + entry->size - 1;
 
 		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",
+				 &orig, avail, e820_start, e820_end);
+			orig = *avail;
+		}
 	}
 }
 
@@ -45,9 +55,6 @@ void arch_remove_reservations(struct resource *avail)
 	 * the low 1MB unconditionally, as this area is needed for some ISA
 	 * cards requiring a memory range, e.g. the i82365 PCMCIA controller.
 	 */
-	if (avail->flags & IORESOURCE_MEM) {
+	if (avail->flags & IORESOURCE_MEM)
 		resource_clip(avail, BIOS_ROM_BASE, BIOS_ROM_END);
-
-		remove_e820_regions(avail);
-	}
 }
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 052f1d78a562..562c81a51ea0 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -8,6 +8,7 @@
 #include <linux/pci-acpi.h>
 #include <asm/numa.h>
 #include <asm/pci_x86.h>
+#include <asm/e820/api.h>
 
 struct pci_root_info {
 	struct acpi_pci_root_info common;
@@ -299,6 +300,10 @@ static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
 	int status;
 
 	status = acpi_pci_probe_root_resources(ci);
+
+	resource_list_for_each_entry(entry, &ci->resources)
+		remove_e820_regions(&device->dev, entry->res);
+
 	if (pci_use_crs) {
 		resource_list_for_each_entry_safe(entry, tmp, &ci->resources)
 			if (resource_is_pcicfg_ioport(entry->res))
-- 
2.25.1


  parent reply	other threads:[~2022-03-04  3:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04  3:51 [PATCH 0/3] x86/PCI: Clip only partial E820 overlaps Bjorn Helgaas
2022-03-04  3:51 ` [PATCH 1/3] x86/PCI: Eliminate remove_e820_regions() common subexpressions Bjorn Helgaas
2022-03-04  3:51 ` Bjorn Helgaas [this message]
2022-03-04  3:51 ` [PATCH 3/3] x86/PCI: Preserve host bridge windows completely covered by E820 Bjorn Helgaas
2022-03-04 14:16   ` Hans de Goede
2022-03-04 15:32     ` Bjorn Helgaas
2022-03-04 15:46       ` Hans de Goede
2022-03-04 18:34         ` Bjorn Helgaas
2022-03-05 10:37         ` Hans de Goede
2022-03-07 10:02           ` Hans de Goede
2022-03-08 14:52             ` Rafael J. Wysocki
2022-03-09 18:15           ` Bjorn Helgaas
2022-03-10 12:28             ` Hans de Goede
2022-03-11  7:52               ` Hans de Goede
2022-03-11 16:24                 ` Bjorn Helgaas
2022-03-11 15:13         ` Hans de Goede
2022-03-04 14:15 ` [PATCH 0/3] x86/PCI: Clip only partial E820 overlaps Hans de Goede
2022-03-04 15:21   ` Mika Westerberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220304035110.988712-3-helgaas@kernel.org \
    --to=helgaas@kernel.org \
    --cc=benoitg@coeus.ca \
    --cc=bhelgaas@google.com \
    --cc=bp@alien8.de \
    --cc=hdegoede@redhat.com \
    --cc=hpa@zytor.com \
    --cc=hui.wang@canonical.com \
    --cc=juhapekka.heikkila@gmail.com \
    --cc=kai.heng.feng@canonical.com \
    --cc=kw@linux.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mingo@redhat.com \
    --cc=myron.stowe@redhat.com \
    --cc=rjw@rjwysocki.net \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.